17497-zendesk #17499
17497-zendesk #17499
Conversation
|
@Afstkla is attempting to deploy a commit to the Pipedreamers Team on Vercel. A member of the Team first needs to authorize it. |
|
""" WalkthroughThe changes introduce two new optional properties, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ZendeskAction
participant ZendeskAPI
User->>ZendeskAction: Provide ticket update input (body, isHTML, public)
ZendeskAction->>ZendeskAction: Build comment payload (body/html_body, public)
ZendeskAction->>ZendeskAPI: Send ticket update with constructed comment
ZendeskAPI-->>ZendeskAction: Respond with update result
ZendeskAction-->>User: Return update outcome
Assessment against linked issues
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
components/zendesk/actions/delete-ticket/delete-ticket.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/zendesk/actions/get-ticket-info/get-ticket-info.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/zendesk/actions/list-tickets/list-tickets.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (14)
🚧 Files skipped from review as they are similar to previous changes (14)
⏰ 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). (4)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
|
Thank you so much for submitting this! We've added it to our backlog to review, and our team has been notified. |
|
Thanks for submitting this PR! When we review PRs, we follow the Pipedream component guidelines. If you're not familiar, here's a quick checklist:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 (3)
components/zendesk/actions/update-ticket/update-ticket.mjs(4 hunks)components/zendesk/package.json(2 hunks)components/zendesk/zendesk.app.mjs(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
components/zendesk/package.json (1)
Learnt from: jcortes
PR: PipedreamHQ/pipedream#14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like `fs` to `package.json` dependencies, as they are native modules provided by the Node.js runtime.
components/zendesk/zendesk.app.mjs (1)
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#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.
🔇 Additional comments (8)
components/zendesk/package.json (1)
3-3: LGTM - Appropriate version bump.The version increment from 0.7.0 to 0.7.1 is appropriate for the new optional properties added to the Zendesk integration.
components/zendesk/zendesk.app.mjs (3)
12-12: LGTM - Making categoryId optional.Making the Trigger Category ID optional provides better flexibility for users who may not need to specify a category.
128-134: LGTM - Well-defined HTML body property.The
ticketCommentBodyIsHTMLproperty is properly defined with:
- Clear label and description
- Appropriate default value (false) for backward compatibility
- Optional flag for flexibility
155-161: LGTM - Well-defined public visibility property.The
ticketCommentPublicproperty is properly defined with:
- Clear label and description
- Appropriate default value (true) matching typical comment behavior
- Optional flag for flexibility
components/zendesk/actions/update-ticket/update-ticket.mjs (4)
8-8: LGTM - Appropriate version bump.The version increment from 0.1.3 to 0.1.4 is appropriate for the new optional properties added to the update-ticket action.
23-28: LGTM - Proper prop definitions.The new properties are correctly defined using propDefinitions from the app, maintaining consistency with the existing pattern.
Also applies to: 47-52
82-90: LGTM - Correct conditional logic for HTML vs plain text.The implementation correctly uses either
html_bodyorbodybased on theticketCommentBodyIsHTMLflag, providing flexibility for rich content formatting while maintaining backward compatibility.
90-90: LGTM - Proper public visibility handling.The public attribute is correctly set on the comment object and used in the ticket update payload, providing control over comment visibility.
Also applies to: 98-98
components/zendesk/package.json
Outdated
| @@ -17,4 +17,4 @@ | |||
| "@pipedream/platform": "^3.0.3", | |||
| "crypto": "^1.0.1" | |||
There was a problem hiding this comment.
Remove built-in Node.js module from dependencies.
The crypto module is a built-in Node.js module and should not be included in package.json dependencies. This is similar to other built-in modules like fs that are provided by the Node.js runtime.
- "crypto": "^1.0.1"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "crypto": "^1.0.1" |
🤖 Prompt for AI Agents
In components/zendesk/package.json at line 18, remove the "crypto" dependency
entry because it is a built-in Node.js module and should not be listed in the
dependencies section of package.json.
andrewjschuang
moved this to Ready for PR Review
in Component (Source and Action) Backlog
jcortes
moved this from Ready for PR Review
to In Review
in Component (Source and Action) Backlog
|
Fixed the comments @andrewjschuang @adolfo-pd lmk if anything more is needed from my side. Difficult to test from my side but based on the docs & my experience with Zendesk, these changes seem sensible |
jcortes
moved this from In Review
to Ready for QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from Ready for QA
to Ready for Release
in Component (Source and Action) Backlog
|
Hi everyone, all test cases are passed! Ready for release! Test report |
|
@vunguyenhung would you also be so kind as to test the public / private boolean that's new? Your test report only covers external replies if I see it correctly |
|
@Afstkla thanks for notifying. The Comment Public = FALSE doesn't work. Moving it back to Changes Required |
vunguyenhung
moved this from Ready for Release
to Changes Required
in Component (Source and Action) Backlog
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check the test report below for more information |
@vunguyenhung the response says "successfully updated ticket with ID 6", whereas your screenshot shows ticket number 2. Are you sure you checked the right ticket? |
jcortes
moved this from Changes Required
to Ready for QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from Ready for QA
to In QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from In QA
to Ready for Release
in Component (Source and Action) Backlog
|
Hi everyone, all test cases are passed! Ready for release! Test report |
github-project-automation
bot
moved this from Ready for Release
to Done
in Component (Source and Action) Backlog
WHY
Resolves #17497 (if I did it correctly)
Summary by CodeRabbit
New Features
Chores