-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Zoho Desk - new help center actions #18708
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
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:
|
WalkthroughAdds Zoho Desk Help Center and Knowledge Base support: new actions to list help centers, list/search/get articles, and list root categories; extends the zoho_desk app with portalId/articleId/maxResults props, KB methods and streaming helpers; updates constants and bumps package version. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Action as Zoho Desk Action
participant App as zoho_desk.app
participant API as Zoho Desk API
User->>Action: invoke action (orgId, portalId, params/articleId)
Action->>App: call KB method (listHelpCenters / listKBArticles / getKBArticle / searchKBArticles / listRootCategories)
App->>API: HTTP request(s) using CORE_API_PATH or PORTAL_API_PATH
API-->>App: return item(s) or paginated responses
alt Streaming
App-->>Action: yield items (async iterator)
loop accumulate
Action->>Action: collect items until maxResults or end
end
else Single fetch
App-->>Action: return single entity
end
Action->>Action: $.export("$summary", ...)
Action-->>User: return collected data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)components/zoho_desk/zoho_desk.app.mjs (1)
⏰ 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). (5)
🔇 Additional comments (4)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
components/zoho_desk/zoho_desk.app.mjs (2)
24-46: Consider adding error handling in the async options.The
portalIdprop's async options function callslistHelpCenterswithout error handling. If the API call fails, the error could bubble up and cause issues for users trying to configure the component.Consider wrapping the API call in a try-catch block:
async options({ orgId }) { if (!orgId) { return []; } + try { const { data: helpCenters = [] } = await this.listHelpCenters({ params: { orgId, }, }); return helpCenters.map(({ portalId: value, name: label, }) => ({ value, label: label || value, })); + } catch (error) { + console.error("Error fetching help centers:", error); + return []; + } },
126-130: Consider enhancing articleId with async options.While the simple string approach works, adding async options (similar to
contactIdandticketIdprops) would improve user experience by allowing users to select from available articles rather than manually entering IDs.This would require additional context (e.g.,
orgIdandportalId) to fetch available articles, so it can be implemented as a future enhancement if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/zoho_desk/common/constants.mjs(2 hunks)components/zoho_desk/zoho_desk.app.mjs(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/zoho_desk/common/constants.mjs
🧰 Additional context used
🧬 Code graph analysis (1)
components/zoho_desk/zoho_desk.app.mjs (1)
components/zoho_desk/actions/list-help-centers/list-help-centers.mjs (1)
helpCenters(26-31)
⏰ 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)
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (2)
components/zoho_desk/zoho_desk.app.mjs (2)
295-368: LGTM! Well-structured knowledge base methods.The new help center and knowledge base methods are well-implemented:
- Consistent use of
makeRequestwithapiPrefix: constants.PORTAL_API_PATHfor portal API endpoints- Proper async generator patterns for streaming methods using
yield*andgetResourcesStream- Clear, RESTful path conventions (
/helpCenters,/kbArticles,/kbArticles/search,/kbRootCategories)- Correct parameter handling and destructuring in stream methods
- Implementation aligns with how they're consumed in the action files
The code follows the established patterns in the codebase and integrates cleanly with the existing infrastructure.
133-136: Constants definitions verified. CORE_API_PATH and PORTAL_API_PATH are defined in components/zoho_desk/common/constants.mjs with the expected values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good @verhovsky , thanks for the contribution! Left one comment
| const stream = this.zohoDesk.searchKnowledgeBaseArticlesStream({ | ||
| params, | ||
| }); | ||
| for await (const article of stream) { | ||
| articles.push(article); | ||
| if (maxResults && articles.length >= maxResults) { | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we have several components using the same pagination method, maybe we should move this logic to a paginate method in the app file. This is a common pattern for apps that have lots of paginated resources using the same standard pagination parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
components/zoho_desk/actions/search-articles/search-articles.mjs (1)
97-104: Note: Pagination pattern consistency.This implementation uses the streaming pattern from
getResourcesStreamin the app file, which is the correct approach. The for-await-of loop to accumulate results is a simple, readable pattern that's consistent with other actions in the codebase (list-articles, list-root-categories).The past review comment from GTFalcao about extracting common pagination logic has merit, but the current approach already centralizes the core pagination in
getResourcesStream. The remaining accumulation logic is minimal and action-specific.
🧹 Nitpick comments (2)
components/zoho_desk/zoho_desk.app.mjs (1)
295-374: Consider adding JSDoc documentation for the new knowledge base methods.While the method implementations are correct and follow the existing patterns in the file, adding JSDoc comments would improve maintainability and help other developers understand the expected parameters, return types, and usage. This aligns with the Pipedream component guidelines mentioned in the PR comments.
Example:
/** * Lists all help centers for the organization * @param {Object} args - Request arguments * @param {Object} args.params - Query parameters * @param {string} args.params.orgId - Organization ID * @returns {Promise<Object>} Response with data array of help centers */ listHelpCenters(args = {}) { // ... }Note: Since existing methods in this file don't use JSDoc either, this is a broader file-level improvement opportunity rather than a requirement for these specific additions.
Based on coding guidelines
components/zoho_desk/actions/search-articles/search-articles.mjs (1)
89-95: Consider filtering undefined parameter values.The params object includes all properties regardless of whether they have values. While the API likely ignores undefined values, filtering them out would make the request cleaner and reduce payload size slightly.
Example refactor:
const params = { portalId, searchStr, - categoryId, - sortBy, - searchKeyWordMatch, + ...(categoryId && { categoryId }), + ...(sortBy && { sortBy }), + ...(searchKeyWordMatch && { searchKeyWordMatch }), };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/zoho_desk/actions/list-articles/list-articles.mjs(1 hunks)components/zoho_desk/actions/list-root-categories/list-root-categories.mjs(1 hunks)components/zoho_desk/actions/search-articles/search-articles.mjs(1 hunks)components/zoho_desk/zoho_desk.app.mjs(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- components/zoho_desk/actions/list-root-categories/list-root-categories.mjs
- components/zoho_desk/actions/list-articles/list-articles.mjs
🧰 Additional context used
🧬 Code graph analysis (2)
components/zoho_desk/zoho_desk.app.mjs (1)
components/zoho_desk/actions/list-help-centers/list-help-centers.mjs (1)
helpCenters(26-31)
components/zoho_desk/actions/search-articles/search-articles.mjs (2)
components/zoho_desk/actions/list-articles/list-articles.mjs (3)
params(75-80)stream(83-86)articles(82-82)components/zoho_desk/actions/list-root-categories/list-root-categories.mjs (2)
params(85-92)stream(94-97)
⏰ 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)
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: pnpm publish
🔇 Additional comments (4)
components/zoho_desk/zoho_desk.app.mjs (3)
24-46: Well-implemented propDefinition with defensive checks.The
portalIdpropDefinition correctly handles the dependency onorgIdby returning an empty array whenorgIdis not provided. The label fallback (label: label || value) ensures a value is always displayed even if the help center name is missing.
133-175: LGTM! Dual API path support implemented correctly.The refactoring from
versionPathtoapiPrefixcleanly supports both core API endpoints (/api/v1/) and portal API endpoints (/portal/api/). The default value ofconstants.CORE_API_PATHmaintains backward compatibility with existing methods.
309-374: LGTM! Streaming methods follow a consistent pattern.The three async generator methods (
listKnowledgeBaseArticlesStream,searchKnowledgeBaseArticlesStream,listKnowledgeBaseRootCategoriesStream) correctly delegate to the existinggetResourcesStreamhelper, maintaining a clean and consistent approach to paginated results.components/zoho_desk/actions/search-articles/search-articles.mjs (1)
1-78: LGTM! Well-structured action with comprehensive properties.The action metadata and props are well-defined:
- Clear descriptions for all properties
- Appropriate use of
optionalfor non-required fields- Good UX with labeled options for
sortByandsearchKeyWordMatch- Proper dependency chain:
portalIddepends onorgId- Sensible defaults for
sortByandmaxResults
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
components/zoho_desk/zoho_desk.app.mjs (1)
24-46: Consider adding error handling to portalId options.If
listHelpCentersfails (network error, auth issue, etc.), the async options function will throw and the user will see a cryptic error. Consider wrapping the call in try-catch and returning an empty array or a helpful error message.Example:
async options({ orgId }) { if (!orgId) { return []; } + try { const { data: helpCenters = [] } = await this.listHelpCenters({ params: { orgId, }, }); return helpCenters.map(({ portalId: value, name: label, }) => ({ value, label: label || value, })); + } catch (error) { + console.error("Failed to load help centers:", error); + return []; + } },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/zoho_desk/actions/list-articles/list-articles.mjs(1 hunks)components/zoho_desk/actions/list-root-categories/list-root-categories.mjs(1 hunks)components/zoho_desk/actions/search-articles/search-articles.mjs(1 hunks)components/zoho_desk/zoho_desk.app.mjs(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/zoho_desk/actions/search-articles/search-articles.mjs
🧰 Additional context used
🧬 Code graph analysis (3)
components/zoho_desk/actions/list-root-categories/list-root-categories.mjs (2)
components/zoho_desk/actions/list-articles/list-articles.mjs (2)
params(72-77)stream(80-83)components/zoho_desk/actions/search-articles/search-articles.mjs (2)
params(86-92)stream(94-97)
components/zoho_desk/actions/list-articles/list-articles.mjs (3)
components/zoho_desk/actions/list-root-categories/list-root-categories.mjs (2)
params(82-89)stream(91-94)components/zoho_desk/actions/search-articles/search-articles.mjs (3)
params(86-92)articles(98-98)stream(94-97)components/zoho_desk/actions/get-article/get-article.mjs (1)
article(44-49)
components/zoho_desk/zoho_desk.app.mjs (1)
components/zoho_desk/actions/list-help-centers/list-help-centers.mjs (1)
helpCenters(26-31)
🪛 GitHub Actions: Pull Request Checks
components/zoho_desk/zoho_desk.app.mjs
[error] 134-134: ESLint: Strings must use doublequote (quotes). Command: 'pnpm exec eslint ...' to fix.
🪛 GitHub Check: Lint Code Base
components/zoho_desk/zoho_desk.app.mjs
[failure] 134-134:
Strings must use doublequote
⏰ 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: pnpm publish
- GitHub Check: Verify TypeScript components
🔇 Additional comments (4)
components/zoho_desk/zoho_desk.app.mjs (2)
140-142: LGTM: API prefix approach cleanly handles dual endpoint patterns.The
apiPrefixparameter elegantly addresses the different base paths (/api/v1/vs/portal/api/) mentioned in the past review comment. The default toCORE_API_PATHensures backward compatibility with existing methods while allowing KB methods to usePORTAL_API_PATH.Also applies to: 158-182
302-381: LGTM: Consistent streaming pattern across all KB methods.The three streaming methods (
listKnowledgeBaseArticlesStream,searchKnowledgeBaseArticlesStream,listKnowledgeBaseRootCategoriesStream) follow a consistent pattern by delegating togetResourcesStreamwith the appropriateresourceFn. This ensures predictable pagination behavior across all KB endpoints.components/zoho_desk/actions/list-root-categories/list-root-categories.mjs (1)
71-105: LGTM: Standard streaming action pattern.The implementation correctly:
- Streams results using the KB API
- Accumulates into an array for return
- Exports a summary with proper pluralization ("category" vs "categories")
- Follows the pattern established by other actions in this PR
components/zoho_desk/actions/list-articles/list-articles.mjs (1)
63-93: LGTM: Consistent with the established action pattern.The implementation follows the same streaming-to-array pattern as
list-root-categoriesand correctly handles pluralization in the summary export.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Closes #18644
The order is list-help-centers.mjs → list-root-categories.mjs → list-articles.mjs or search-articles.mjs → get-article.mjs
A nuance in testing this, when publishing a help article you need to set its visibility to something other than private, otherwise you will not see it in the API.
Summary by CodeRabbit