Fix: Improper Error handling on shifting patient #9561
Conversation
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
WalkthroughThe pull request updates the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
|
Hey @rithviknishad, could you review this PR? It’s not fully done yet, but I’d appreciate your feedback on the changes. Am I on the right track, or is there something I might be missing? Error I am getting when data is passed :
|
![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 selected for processing (1)
src/components/Facility/DischargeModal.tsx(2 hunks)
🔇 Additional comments (3)
src/components/Facility/DischargeModal.tsx (3)
23-23: Looks good.
Importing useAppHistory is appropriate for handling in-app navigation callbacks.
96-97: Ensure graceful fallback if goBack is triggered multiple times.
While the introduction of goBack is logical, consider verifying that it won't lead to unexpected navigation behavior (e.g., multiple consecutive calls ending up at the wrong page or empty history).
98-109:
Address potential data type mismatch in the query.
You mentioned encountering a type error 'diagnoses' does not exist on type '(...) => Promise<ConsultationModel>'. Verify that ConsultationModel actually includes the diagnoses property. If not, update the interface or handle missing fields to avoid runtime or TypeScript errors.
…/github.com/AdityaJ2305/care_fe into issue/9061/improper_error_handling_shifting
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/Facility/DischargeModal.tsx (1)
105-107: Consider enhancing error handling.While the basic error handling is implemented, consider:
- Adding error type checking for more specific error messages
- Implementing error retry logic for transient failures
- if (error) { - Notification.Error({ msg: t("ERROR_FETCHING_CONSULTATION") }); - } + if (error) { + if (error instanceof Error) { + Notification.Error({ + msg: t("ERROR_FETCHING_CONSULTATION"), + description: error.message + }); + } else { + Notification.Error({ msg: t("ERROR_FETCHING_CONSULTATION") }); + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
public/locale/en.json(1 hunks)src/components/Facility/DischargeModal.tsx(4 hunks)
🔇 Additional comments (3)
src/components/Facility/DischargeModal.tsx (2)
1-1: LGTM: Import statements are correctly added.
The required imports for TanStack Query and the query utility are properly added.
Also applies to: 32-32
109-109: LGTM: Data and loading states are properly handled.
The implementation correctly:
- Handles undefined data with nullish coalescing
- Shows loading state while data is being fetched
Also applies to: 192-192
public/locale/en.json (1)
51-51: LGTM: Error message is properly added.
The error message follows the established conventions:
- Uses consistent key naming (all caps with underscores)
- Provides clear user feedback
- Is properly placed alphabetically in the file
|
LGTM |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/components/Facility/DischargeModal.tsx (1)
96-103: 🛠️ Refactor suggestionAlign query implementation with codebase patterns
The current implementation deviates from the established patterns in the codebase:
- Query key structure is simplified
- Direct usage of useQuery instead of the wrapper
Replace with:
- const { data, isLoading, error } = useQuery({ - queryKey: [routes.getConsultation.path, consultationData.id], - queryFn: query(routes.getConsultation, { - pathParams: { id: consultationData.id }, - silent: true, - }), - enabled: consultationData.id !== undefined, - }); + const { data, isLoading, error } = useTanStackQueryInstead( + routes.getConsultation, + { + pathParams: { id: consultationData.id }, + silent: true, + }, + { + enabled: consultationData.id !== undefined, + } + );
🧹 Nitpick comments (1)
src/components/Facility/DischargeModal.tsx (1)
111-111: Enhance type safety for diagnoses dataWhile the nullish coalescing operator provides a safe fallback, consider adding runtime type validation:
- const initialDiagnoses = data?.diagnoses ?? []; + const initialDiagnoses = Array.isArray(data?.diagnoses) ? data.diagnoses : [];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Facility/DischargeModal.tsx(4 hunks)
🔇 Additional comments (3)
src/components/Facility/DischargeModal.tsx (3)
1-1: LGTM: Import changes align with tanstack migration
The addition of useQuery and query imports supports the migration to @tanstack/react-query.
Also applies to: 32-32
105-109: LGTM: Error handling follows best practices
The error handling implementation:
- Correctly uses useEffect to prevent multiple notifications
- Properly utilizes translations for user-friendly error messages
194-194: LGTM: Loading state properly handled
The loading state implementation correctly uses the isLoading flag from useQuery and renders an appropriate loading component.
|
No longer relevant in form-field-v1 |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Bug Fixes