[Components] Topdesk - new components #18844
Conversation
| The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds a Topdesk integration: a full app module with HTTP/auth helpers, resource methods and pagination, package version/dependency update, and six new actions for creating, retrieving, listing, and updating incidents plus listing knowledge items and statuses. Changes
Sequence Diagram(s)sequenceDiagram participant User participant Action participant App participant TopdeskAPI User->>Action: Trigger action with props Action->>Action: Normalize props, build id-wrapped fields alt Paginated list actions Action->>App: app.paginate({ fn: listX, fnArgs, maxResults }) loop for pages App->>TopdeskAPI: GET /... (page_size, start, params) TopdeskAPI-->>App: paged items + page info App-->>Action: yield page items end else Single-record actions (create/get/update) Action->>App: app.createIncident / getIncident / updateIncident App->>TopdeskAPI: POST/GET/PATCH (payload, auth) TopdeskAPI-->>App: response App-->>Action: parsed response end Action->>Action: $.export("$summary", ...) Action-->>User: return response / items Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
jcortes force-pushed the topdesk-new-components branch from 7599331 to 3426e13 Compare ![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (10)
components/topdesk/actions/get-knowledge-item-statuses/get-knowledge-item-statuses.mjs (1)
30-41: Harden summary count against response shape differencesIf
listKnowledgeItemStatusesreturns an array (not{ results }), the count will read 0. Make the summary resilient.Apply:
- const statusCount = response.results?.length || 0; + const statusCount = Array.isArray(response) + ? response.length + : (response?.results?.length ?? 0);components/topdesk/actions/get-knowledge-items/get-knowledge-items.mjs (2)
80-84: Add idempotent hint for read-only actionThis action performs a GET; mark it as idempotent for consistency with other read actions.
annotations: { readOnlyHint: true, destructiveHint: false, openWorldHint: true, + idempotentHint: true, },
100-108: Field normalization is solid; minor cleanup optionalYou already accept both
string[]andstring. Consider extracting this join-to-string logic into a small helper intopdesk.app.mjsto DRY across actions (incidents uses same pattern).components/topdesk/actions/get-incidents/get-incidents.mjs (1)
67-74: Fields input type vs normalizationProp
fieldsis typed asstring(comma-separated), but runtime also supportsstring[]. Either keep as-is (tolerant) or switch prop type tostring[]withoptionsfor a curated list (like knowledge items) for better UX.components/topdesk/actions/create-incident/create-incident.mjs (2)
20-30: Enforce partial incident requirementsIf status “partial” is selected, Topdesk requires a mainIncident reference. Add a simple guard to prevent invalid payloads.
async run({ $ }) { const { app, callerLookupId, status, + // ... mainIncidentId, // ... } = this; + if (status === "partial" && !mainIncidentId) { + throw new Error("Main Incident ID is required when status is 'partial'."); + }Also applies to: 103-108
293-371: Deduplicate id-wrapping logic across actionsThis idFields → reduce pattern is duplicated in update-incident. Extract a small helper to keep both in sync.
+// helpers/id.js (new) +export const wrapIds = (pairs = []) => + pairs.reduce((acc, { value, key }) => value ? { ...acc, [key]: { id: value } } : acc, {});Then:
- ...idFields.reduce((acc, { value, key }) => ({ ...acc, ...value && { [key]: { id: value } } }), {}), + ...wrapIds(idFields),components/topdesk/actions/update-incident/update-incident.mjs (2)
17-22: Use propDefinition for caller lookup for consistent UXCreate action uses the app’s personId selector; this action takes a raw string. Unify to reuse dynamic options and labels.
- callerLookupId: { - type: "string", - label: "Caller Lookup ID", - description: "Lookup value for filling in a registered caller's contact details (UUID). Can only be set by operators.", - optional: true, - }, + callerLookupId: { + propDefinition: [app, "personId"], + label: "Caller Lookup ID", + description: "Lookup value for a registered caller (UUID). Can only be set by operators.", + optional: true, + },
392-429: Allow status update if API supports itIf the API allows updating status, expose a status prop and forward it like the create action does. Keeps parity for users moving an incident to second line, etc.
+ status: { + type: "string", + label: "Status", + optional: true, + options: ["firstLine","secondLine","partial"], + },And include in data.
- data: { + data: { + status, briefDescription,components/topdesk/topdesk.app.mjs (2)
37-61: Knowledge item option: description vs APIDescription says “UUID or number”, but app.getKnowledgeItem uses an ID path parameter. If numbers aren’t valid IDs, adjust description or implement number lookup via query.
139-166: Consider exposing GET helpers tooYou added post/patch convenience methods; adding get(opts) keeps call sites uniform.
post(opts = {}) { return this._makeRequest({ method: "POST", ...opts, }); }, + get(opts = {}) { + return this._makeRequest({ + method: "GET", + ...opts, + }); + }, patch(opts = {}) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
components/topdesk/actions/create-incident/create-incident.mjs(1 hunks)components/topdesk/actions/get-incident/get-incident.mjs(1 hunks)components/topdesk/actions/get-incidents/get-incidents.mjs(1 hunks)components/topdesk/actions/get-knowledge-item-statuses/get-knowledge-item-statuses.mjs(1 hunks)components/topdesk/actions/get-knowledge-items/get-knowledge-items.mjs(1 hunks)components/topdesk/actions/update-incident/update-incident.mjs(1 hunks)components/topdesk/package.json(2 hunks)components/topdesk/topdesk.app.mjs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
components/topdesk/actions/get-incident/get-incident.mjs (2)
components/topdesk/actions/create-incident/create-incident.mjs (1)
response(373-405)components/topdesk/actions/update-incident/update-incident.mjs (1)
response(392-429)
components/topdesk/actions/create-incident/create-incident.mjs (1)
components/topdesk/actions/update-incident/update-incident.mjs (2)
idFields(317-390)response(392-429)
components/topdesk/actions/get-knowledge-item-statuses/get-knowledge-item-statuses.mjs (4)
components/topdesk/actions/create-incident/create-incident.mjs (1)
response(373-405)components/topdesk/actions/get-incident/get-incident.mjs (1)
response(30-33)components/topdesk/actions/update-incident/update-incident.mjs (1)
response(392-429)components/topdesk/topdesk.app.mjs (2)
response(42-47)response(209-216)
components/topdesk/actions/update-incident/update-incident.mjs (1)
components/topdesk/actions/create-incident/create-incident.mjs (2)
idFields(294-371)response(373-405)
components/topdesk/actions/get-incidents/get-incidents.mjs (2)
components/topdesk/topdesk.app.mjs (1)
incidents(16-21)components/topdesk/actions/get-knowledge-items/get-knowledge-items.mjs (1)
paginator(95-112)
components/topdesk/actions/get-knowledge-items/get-knowledge-items.mjs (2)
components/topdesk/topdesk.app.mjs (2)
items(48-48)items(219-223)components/topdesk/actions/get-incidents/get-incidents.mjs (1)
paginator(60-77)
components/topdesk/topdesk.app.mjs (6)
components/topdesk/actions/get-incidents/get-incidents.mjs (1)
incidents(59-59)components/topdesk/actions/create-incident/create-incident.mjs (1)
response(373-405)components/topdesk/actions/get-incident/get-incident.mjs (1)
response(30-33)components/topdesk/actions/get-knowledge-item-statuses/get-knowledge-item-statuses.mjs (1)
response(30-35)components/topdesk/actions/update-incident/update-incident.mjs (1)
response(392-429)components/topdesk/actions/get-knowledge-items/get-knowledge-items.mjs (1)
items(94-94)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (6)
components/topdesk/package.json (1)
3-17: Version bump and dependency look finePackage version and dependency addition align with new actions using platform helpers. No issues from this diff.
components/topdesk/actions/get-incident/get-incident.mjs (1)
24-37: LGTMProp definition and call pattern are consistent. Summary uses
response.id, which matches expected payloads.If
propDefinitions.incidentIdsupports search / dynamic options, ensure it handles archived incidents as needed.components/topdesk/actions/get-incidents/get-incidents.mjs (1)
83-85: Good summary and return shapeReturning the aggregated array and exporting a count summary matches other actions.
components/topdesk/actions/get-knowledge-item-statuses/get-knowledge-item-statuses.mjs (1)
24-41: Verify response structure consistency across endpointsThe code assumes
listKnowledgeItemStatusesreturns{ results: [...] }(accessingresponse.results?.length), but other similar list methods likelistOperatorsandlistPersonsare treated as direct arrays in their usage. This inconsistency warrants manual verification:
- Check if "Searchlists" endpoints (knowledgeItemStatuses) have a different contract than other list endpoints
- Confirm whether the Topdesk API actually returns
{ results: [...] }for this endpoint, or if it should return an array directly like other list methods- If it returns an array, the code should be simplified to access the array length directly without the
.resultspropertycomponents/topdesk/actions/get-knowledge-items/get-knowledge-items.mjs (1)
73-78: No action required — language parameter is correctly implementedThe TOPdesk knowledge-base-v1 API supports filtering knowledge items by language using the language code, and the current implementation correctly passes the
languageparameter as a query parameter. The code will not produce silent no-ops; language filtering will work as intended.components/topdesk/actions/update-incident/update-incident.mjs (1)
6-8: PATCH is supported by TOPdesk API — no changes neededTOPdesk's Incident API supports both PATCH for partial updates and PUT for full replacement. The current implementation correctly uses PATCH at line 162 in
topdesk.app.mjs, called consistently from line 392 in the update-incident action. The documentation reference is also accurate.
| label: "Query", | ||
| description: `A FIQL query to filter the response. Use semicolons to combine multiple conditions. | ||
| **Available filter fields:** |
There was a problem hiding this comment.
I would suggest linking to the associated documentation page instead of listing all the filters here. It would keep the description more concise while also providing the information and guaranteeing it will be up-to-date.
jcortes force-pushed the topdesk-new-components branch from 3426e13 to 220a94c Compare 
jcortes force-pushed the topdesk-new-components branch from 220a94c to b0342a0 Compare There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
components/topdesk/actions/get-incidents/get-incidents.mjs (1)
59-77: Pagination param mismatch will break after first page.The
app.paginatehelper usesstart/page_sizeparameters (line 73), butapp.listIncidentsfield options in topdesk.app.mjs (lines 15-20) usepageStart/pageSize. This mismatch will cause pagination to stall after the first page since the paginator will pass parameters that don't match what the API expects.Apply this diff to use the correct parameter name:
- page_size: 100, + pageSize: 100,Alternatively, update
listIncidentsand the paginate helper to consistently usestart/page_sizeacross all endpoints (see the broader paginator fix needed in topdesk.app.mjs).components/topdesk/topdesk.app.mjs (3)
11-16: Destructuring undefinedprevContextwill throw.Line 12 destructures
pageStartfromprevContext, butprevContextcan beundefinedon the first call, causing a runtime exception.Apply this diff to safely access
prevContext:async options({ prevContext }) { - const { pageStart } = prevContext; + const pageStart = prevContext?.pageStart ?? 0; if (pageStart === null) { return []; }
86-101:personIdoptions returns array instead of pagination object.The
optionsfunction returns an array (lines 97-100), which prevents pagination from advancing. The Pipedream platform expects{ options, context }to support paginated dropdowns.Apply this diff to return the correct pagination structure:
async options({ prevContext }) { const persons = await this.listPersons({ params: { start: prevContext?.start || 0, page_size: 100, }, }); - return persons.map((person) => ({ - label: `${person.firstName} ${person.surName}`, - value: person.id, - })) || []; + return { + options: (persons || []).map((person) => ({ + label: `${person.firstName} ${person.surName}`, + value: person.id, + })), + context: { + start: persons?.length === 100 + ? (prevContext?.start || 0) + 100 + : null, + }, + }; },
654-705: Paginator skips the first page and has parameter naming inconsistencies.Line 668 sends
start: start + (fnArgs.params?.page_size || 100)on the first call. Sincestartis initialized to 0 (line 661), the first request usesstart=100, skipping the first page. Line 703 then incrementsstartagain, skipping subsequent pages as well.Additionally, the paginator hardcodes
start/page_sizeparameters, but several endpoints (listIncidents,listAssets) usepageStart/pageSizeinstead, causing pagination to fail for those endpoints.Apply this diff to fix both issues:
async *paginate({ fn, fnArgs = {}, maxResults = 600, dataField, }) { let resourcesCount = 0; - let start = 0; + const pageSize = fnArgs.params?.page_size ?? fnArgs.params?.pageSize ?? 100; + let start = fnArgs.params?.start ?? fnArgs.params?.pageStart ?? 0; + const startParam = fnArgs.params?.pageStart !== undefined ? "pageStart" : "start"; + const sizeParam = fnArgs.params?.pageSize !== undefined ? "pageSize" : "page_size"; - while (true) { + for (;;) { const response = await fn({ ...fnArgs, params: { ...fnArgs.params, - start: start + (fnArgs.params?.page_size || 100), - page_size: (fnArgs.params?.page_size || 100), + [startParam]: start, + [sizeParam]: pageSize, }, }); // Extract items from response based on dataField or use response directly const items = dataField - ? (response[dataField] || []) + ? (response?.[dataField] ?? []) : (Array.isArray(response) ? response : []); if (!items.length) { console.log("No items found"); return; } for (const item of items) { yield item; resourcesCount++; if (maxResults && resourcesCount >= maxResults) { console.log("Reached max results"); return; } } - const hasNextPage = response.next || (items.length === 100); + const hasNextPage = Boolean(response?.next) || (items.length === pageSize); if (!hasNextPage) { console.log("No more pages found"); return; } // Auto-increment pagination parameters - start += (fnArgs.params?.page_size || 100); + start += pageSize; } },
🧹 Nitpick comments (1)
components/topdesk/actions/get-knowledge-items/get-knowledge-items.mjs (1)
51-72: Consider condensing the filter documentation.The description lists all available filter fields in detail (22 lines). This could be more concise by primarily referencing the documentation link at line 72, while keeping a brief summary of common filters.
Example refactor:
description: `A FIQL query to filter the response. Use semicolons to combine multiple conditions. -**Available filter fields:** -- \`parent.id\`, \`parent.number\` -- \`visibility.sspVisibility\`, \`visibility.sspVisibleFrom\`, \`visibility.sspVisibleUntil\` -- \`visibility.sspVisibilityFilteredOnBranches\`, \`visibility.operatorVisibilityFilteredOnBranches\` -- \`visibility.publicKnowledgeItem\`, \`visibility.branches.id\`, \`visibility.branches.name\` -- \`manager.id\`, \`manager.name\` -- \`status.id\`, \`status.name\` -- \`standardSolution.id\`, \`standardSolution.name\` -- \`externalLink.id\`, \`externalLink.type\`, \`externalLink.date\` -- \`archived\`, \`news\` - -**Operators:** -- \`==\` (equals), \`!=\` (not equals) -- \`=gt=\` (greater than), \`=ge=\` (greater than or equal) -- \`=lt=\` (less than), \`=le=\` (less than or equal) -- \`=in=\` (in list), \`=out=\` (not in list) - **Example:** \`parent.id==3fa85f64-5717-4562-b3fc-2c963f66afa6;externalLink.id=in=(oneTool,otherTool)\` -[See the documentation](https://developers.topdesk.com/explorer/?page=knowledge-base#/Knowledge%20Items/getKnowledgeItems) for more information.`, +See the [FIQL documentation](https://developers.topdesk.com/explorer/?page=knowledge-base#/Knowledge%20Items/getKnowledgeItems) for available filter fields and operators.`,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
components/topdesk/actions/create-incident/create-incident.mjs(1 hunks)components/topdesk/actions/get-incident/get-incident.mjs(1 hunks)components/topdesk/actions/get-incidents/get-incidents.mjs(1 hunks)components/topdesk/actions/get-knowledge-item-statuses/get-knowledge-item-statuses.mjs(1 hunks)components/topdesk/actions/get-knowledge-items/get-knowledge-items.mjs(1 hunks)components/topdesk/actions/update-incident/update-incident.mjs(1 hunks)components/topdesk/package.json(2 hunks)components/topdesk/topdesk.app.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- components/topdesk/package.json
- components/topdesk/actions/create-incident/create-incident.mjs
- components/topdesk/actions/get-knowledge-item-statuses/get-knowledge-item-statuses.mjs
- components/topdesk/actions/get-incident/get-incident.mjs
🧰 Additional context used
🧬 Code graph analysis (4)
components/topdesk/actions/get-incidents/get-incidents.mjs (2)
components/topdesk/topdesk.app.mjs (1)
incidents(16-21)components/topdesk/actions/get-knowledge-items/get-knowledge-items.mjs (1)
paginator(97-114)
components/topdesk/actions/get-knowledge-items/get-knowledge-items.mjs (2)
components/topdesk/topdesk.app.mjs (4)
items(48-48)items(110-110)items(133-133)items(674-678)components/topdesk/actions/get-incidents/get-incidents.mjs (1)
paginator(60-77)
components/topdesk/actions/update-incident/update-incident.mjs (1)
components/topdesk/actions/create-incident/create-incident.mjs (2)
idFields(297-370)response(372-409)
components/topdesk/topdesk.app.mjs (4)
components/topdesk/actions/create-incident/create-incident.mjs (1)
response(372-409)components/topdesk/actions/get-incident/get-incident.mjs (1)
response(30-33)components/topdesk/actions/get-knowledge-item-statuses/get-knowledge-item-statuses.mjs (1)
response(30-35)components/topdesk/actions/update-incident/update-incident.mjs (1)
response(394-436)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (1)
components/topdesk/actions/update-incident/update-incident.mjs (1)
322-436: Well-structured data transformation logic.The idFields array and reduce pattern (lines 322-434) cleanly handles optional ID-based relations by conditionally wrapping values in
{ id: value }objects. This approach is consistent with the create-incident action and keeps the payload construction maintainable.
WHY
Resolves #18806
Summary by CodeRabbit
New Features
Chores