fix: handle pending and error states in CannedResponseList#38721
fix: handle pending and error states in CannedResponseList#38721sezallagwal wants to merge 4 commits into
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
|
No actionable comments were generated in the recent review. 🎉 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧠 Learnings (1)📚 Learning: 2025-11-19T18:20:07.720ZApplied to files:
🧬 Code graph analysis (1)apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/CannedResponseList.tsx (1)
🔇 Additional comments (3)
WalkthroughAdded explicit loading and error props to the canned responses list flow. The wrapper hook now surfaces Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/CannedResponseList.tsx`:
- Around line 108-120: The rendering logic in CannedResponseList is
inconsistent: it shows the error Callout only when error instanceof Error but
treats any truthy error as blocking the empty state with !error, which can leave
the UI blank for non-Error truthy values. Fix by normalizing the check so any
truthy error renders the Callout (replace the error instanceof Error check with
a truthy check on error) and render a safe string via error?.toString() ||
t('Unknown_error') for the Callout message; keep the empty-state guard as !error
&& !loading && itemCount === 0 so ContextualbarEmptyContent displays only when
there is no error. Reference: variables loading, error, itemCount and components
Callout and ContextualbarEmptyContent in CannedResponseList.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/CannedResponseList.tsxapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponseList.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/CannedResponseList.tsxapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponseList.tsx
🧠 Learnings (1)
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/CannedResponseList.tsx
🧬 Code graph analysis (2)
apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/CannedResponseList.tsx (1)
packages/ui-client/src/components/Contextualbar/index.ts (1)
ContextualbarEmptyContent(34-34)
apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponseList.tsx (1)
apps/meteor/client/views/omnichannel/hooks/useCannedResponseList.ts (1)
useCannedResponseList(7-59)
⏰ 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). (1)
- GitHub Check: cubic · AI code reviewer
🔇 Additional comments (3)
apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponseList.tsx (1)
34-34: LGTM — clean integration of pending/error states from the query hook.
isPendinganderrorfromuseInfiniteQueryare correctly destructured and forwarded.isPendingis the right choice here (true only when no cached data exists), as opposed toisFetchingwhich would also fire during background refetches and next-page loads.Also applies to: 74-75
apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/CannedResponseList.tsx (2)
27-28: LGTM on the new props.Typing
errorasunknownaligns well with what hooks like React Query return.
121-143: LGTM on the list rendering section.The existing list rendering remains correctly unaffected by the new loading/error states, and both can coexist (e.g., showing a spinner during a refetch while stale items are still visible).
Resolves the TODO in WrapCannedResponseList.tsx to handle pending and error states for the canned responses list.
Changes
WrapCannedResponseList.tsx
isPendinganderrorfrom useCannedResponseList()loadinganderrorprops to CannedResponseListCannedResponseList.tsx
loadinganderrorto component propsThrobberspinner while data is loadingCallout type='danger'when the API request failsNo Canned Responses) with!loading && !errorso it only appears when the request succeeds with zero resultsSummary by CodeRabbit