Zendesk - Bug Fix to support "fields" prop without label #18960
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
components/zendesk/package.json(1 hunks)components/zendesk/sources/common/ticket.mjs(2 hunks)components/zendesk/sources/common/webhook.mjs(3 hunks)components/zendesk/sources/new-ticket-comment-added/new-ticket-comment-added.mjs(2 hunks)components/zendesk/sources/new-ticket/new-ticket.mjs(1 hunks)components/zendesk/sources/ticket-added-to-view/ticket-added-to-view.mjs(1 hunks)components/zendesk/sources/ticket-closed/ticket-closed.mjs(1 hunks)components/zendesk/sources/ticket-pended/ticket-pended.mjs(1 hunks)components/zendesk/sources/ticket-solved/ticket-solved.mjs(1 hunks)components/zendesk/sources/ticket-updated/ticket-updated.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-20T01:01:02.970Z
Learnt from: js07
Repo: PipedreamHQ/pipedream PR: 18744
File: components/slack_v2/actions/send-large-message/send-large-message.mjs:49-64
Timestamp: 2025-10-20T01:01:02.970Z
Learning: In components/slack_v2/actions/send-large-message/send-large-message.mjs, the metadata_event_payload prop is typed as string, so the code only needs to handle string-to-JSON parsing and does not need to handle object inputs.
Applied to files:
components/zendesk/sources/common/ticket.mjs
🧬 Code graph analysis (2)
components/zendesk/sources/common/ticket.mjs (1)
components/zendesk/sources/new-ticket-comment-added/new-ticket-comment-added.mjs (2)
key(73-76)payload(54-54)
components/zendesk/sources/new-ticket-comment-added/new-ticket-comment-added.mjs (1)
components/zendesk/sources/common/ticket.mjs (1)
payload(19-23)
⏰ 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: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (14)
components/zendesk/sources/ticket-updated/ticket-updated.mjs (1)
9-9: LGTM - Version bump is consistent.The version increment aligns with the broader package update and bug fix in the common ticket module.
components/zendesk/package.json (1)
3-3: LGTM - Appropriate patch version increment.The version bump from 0.10.0 to 0.10.1 correctly reflects a bug fix release.
components/zendesk/sources/ticket-added-to-view/ticket-added-to-view.mjs (1)
8-8: LGTM - Version bump is consistent.components/zendesk/sources/ticket-pended/ticket-pended.mjs (1)
9-9: LGTM - Version bump is consistent.components/zendesk/sources/new-ticket/new-ticket.mjs (1)
9-9: LGTM - Version bump is consistent.components/zendesk/sources/ticket-solved/ticket-solved.mjs (1)
9-9: LGTM - Version bump is consistent.components/zendesk/sources/ticket-closed/ticket-closed.mjs (1)
9-9: LGTM - Version bump is consistent.components/zendesk/sources/common/ticket.mjs (2)
14-17: LGTM - Async conversion is appropriate.Converting
getTriggerPayloadto async is necessary to fetch custom ticket fields. The early return forjsonBodyparsing is correctly preserved.
24-24: LGTM - Fetching custom fields for dynamic payload.Retrieving ticket field metadata enables dynamic placeholder generation for fields without explicit values.
components/zendesk/sources/common/webhook.mjs (3)
60-63: LGTM!The async call to
setupTriggerDatais properly awaited within the asyncactivatehook.
174-193: No issues found – async change is properly implemented and handled.The async signature change at line 174 is correct and justified. The caller at line 60 properly awaits
setupTriggerData(). The field metadata fetching and label handling fallback logic are correctly implemented in thegetTriggerPayload()method incommon/ticket.mjs(lines 24-32), which fetches custom fields and applies the fallback patternfield?.label || fieldto handle missing labels.
187-187: ****The original review concern is not valid. All components extending the webhook base properly implement
getTriggerPayloadas async functions:
- ticket.mjs: Overrides with
async getTriggerPayload()- Sources extending ticket.mjs (new-ticket, ticket-added-to-view, ticket-closed, ticket-pended, ticket-solved, ticket-updated): Inherit async implementation
- new-ticket-comment-added.mjs: Overrides with
async getTriggerPayload()and callsawait common.methods.getTriggerPayload.call(this)- locale-updated.mjs: Uses polling.mjs (completely separate), never calls
getTriggerPayloadThe base class throws an error on
getTriggerPayload(), enforcing that all implementations must override it. Theawaitat line 187 is safe—it's only executed on instances with proper async implementations.Likely an incorrect or invalid review comment.
components/zendesk/sources/new-ticket-comment-added/new-ticket-comment-added.mjs (2)
53-59: No issues found. Code is correct.The verification confirms that
common.methods.getTriggerPayloadincomponents/zendesk/sources/common/ticket.mjsis correctly defined as async (line 14), making the await at line 54 of the new-ticket-comment-added.mjs file appropriate and consistent. The implementation properly handles async parent calls and correctly merges the payload with the ticketComments field.
10-10: Review comment is partially valid; no undefined property fix is visible in this file.The version bump to 0.1.1 aligns with the visible change: making
getTriggerPayloadasync (line 33 now usesawait). However, the undefined error mentioned in PR objectives is not addressed here. The code atcomponents/zendesk/sources/common/ticket.mjs:26already handles undefined labels viafield?.label || fieldfallback. Either the fix is in a different file or the PR objectives may describe a separate concern.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/zendesk/sources/common/ticket.mjs (1)
26-26: Add explicit type validation before calling convertToCamelCase.The fix
field?.label || fieldaddresses the immediate bug for string fields, but doesn't handle all edge cases. Iffieldisnull,undefined, or an object without alabelproperty, the fallback could pass non-string values toconvertToCamelCase(), causing.replace()to fail on line 8.Add type validation to ensure a string is always passed:
- const key = this.convertToCamelCase(field?.label || field); + const labelOrField = typeof field === "object" && field?.label + ? field.label + : field; + if (!labelOrField || typeof labelOrField !== "string") { + console.warn(`Invalid field type: ${JSON.stringify(field)}`); + continue; + } + const key = this.convertToCamelCase(labelOrField);Alternatively, for a more compact solution that normalizes to string:
- const key = this.convertToCamelCase(field?.label || field); + const labelOrField = (typeof field === "object" ? field?.label : field) || ""; + if (!labelOrField) continue; + const key = this.convertToCamelCase(labelOrField);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/zendesk/sources/common/ticket.mjs(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-20T01:01:02.970Z
Learnt from: js07
Repo: PipedreamHQ/pipedream PR: 18744
File: components/slack_v2/actions/send-large-message/send-large-message.mjs:49-64
Timestamp: 2025-10-20T01:01:02.970Z
Learning: In components/slack_v2/actions/send-large-message/send-large-message.mjs, the metadata_event_payload prop is typed as string, so the code only needs to handle string-to-JSON parsing and does not need to handle object inputs.
Applied to files:
components/zendesk/sources/common/ticket.mjs
📚 Learning: 2025-08-27T17:25:10.425Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/utils/type-guards.ts:23-33
Timestamp: 2025-08-27T17:25:10.425Z
Learning: In the connect-react package, the isOptionWithLabel type guard intentionally restricts value types to string|number for runtime filtering purposes, even though LabelValueOption<T> allows any T. This runtime behavior should be preserved over type safety improvements.
Applied to files:
components/zendesk/sources/common/ticket.mjs
📚 Learning: 2024-10-08T16:42:59.225Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 14229
File: components/americommerce/actions/update-customer/update-customer.mjs:89-94
Timestamp: 2024-10-08T16:42:59.225Z
Learning: When defining boolean properties in AmeriCommerce components (e.g., in `update-customer.mjs`), ensure that the label and description are consistent and clearly indicate the intent, especially when using negations like "No Account", to avoid confusion.
Applied to files:
components/zendesk/sources/common/ticket.mjs
🧬 Code graph analysis (1)
components/zendesk/sources/common/ticket.mjs (2)
components/zendesk/sources/new-ticket-comment-added/new-ticket-comment-added.mjs (2)
key(73-76)payload(54-54)components/zendesk/sources/common/webhook.mjs (1)
payload(226-230)
⏰ 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: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (1)
components/zendesk/sources/common/ticket.mjs (1)
14-14: LGTM! Async promotion is necessary and correctly propagated.The method is correctly made async to support awaiting
listTicketFields()on line 24. The callers have been updated to await this method.
Resolves #18950
Summary by CodeRabbit