Refactor questionnaire response and migrated to TanStack's query #9995
Conversation
…re and add error message for loading questionnaire response
WalkthroughThis pull request introduces updates to the application's error handling and data fetching mechanisms. The changes primarily focus on modernizing the query management by migrating from a custom Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Finishing Touches
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 (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
@rithviknishad requesting for review, Thank you! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/components/Questionnaire/QuestionnaireForm.tsx (1)
Line range hint
171-238: Refactor submission logic for better maintainability and validation.The handleSubmit function is handling too many responsibilities and could benefit from being split into smaller, focused functions. Additionally, consider adding pre-submission validation.
+ const validateSubmission = () => { + if (!encounterId || !patientId) { + toast.error(t("missing_required_fields")); + return false; + } + return true; + }; + + const prepareStructuredRequests = ( + forms: QuestionnaireFormState[], + context: { patientId: string; encounterId: string } + ): BatchRequest[] => { + const requests: BatchRequest[] = []; + forms.forEach((form) => { + // ... structured request logic ... + }); + return requests; + }; + + const prepareQuestionnaireRequests = ( + forms: QuestionnaireFormState[] + ): BatchRequest[] => { + const requests: BatchRequest[] = []; + forms.forEach((form) => { + // ... questionnaire submission logic ... + }); + return requests; + }; const handleSubmit = async () => { setIsDirty(false); - if (hasErrors) return; + if (hasErrors || !validateSubmission()) return; const requests: BatchRequest[] = []; + const context = { patientId, encounterId: encounterId! }; + if (encounterId && patientId) { - const context = { patientId, encounterId }; - // First, collect all structured data requests... + requests.push(...prepareStructuredRequests(questionnaireForms, context)); } - // Then, add questionnaire submission requests... + requests.push(...prepareQuestionnaireRequests(questionnaireForms)); submitBatch({ requests }); };
🧹 Nitpick comments (5)
src/components/Questionnaire/QuestionnaireEditor.tsx (2)
63-69: LGTM! Well-structured query configuration.The query configuration follows TanStack Query best practices. Consider enhancing type safety for query keys.
Consider creating a type-safe query key factory:
const queryKeys = { questionnaireDetail: (id: string) => ['questionnaireDetail', id] as const } as const; // Usage useQuery({ queryKey: queryKeys.questionnaireDetail(id), queryFn: query(questionnaireApi.detail, { pathParams: { id }, }), });
Line range hint
1-1000: LGTM! Well-implemented questionnaire editor with robust features.The component demonstrates excellent organization, proper state management, and good accessibility practices.
Consider memoizing the question editor components to optimize performance with large questionnaires:
const MemoizedQuestionEditor = React.memo(QuestionEditor); // Usage in the mapping function {questionnaire.questions.map((question, index) => ( <Draggable key={question.id} draggableId={question.id} index={index}> {(provided) => ( <MemoizedQuestionEditor // ... props /> )} </Draggable> ))}src/components/Questionnaire/QuestionnaireForm.tsx (3)
77-84: Add type safety to error handling in useQuery.While the query configuration follows TanStack's best practices, consider adding explicit error typing for better type safety.
const { data: questionnaireData, isLoading: isQuestionnaireLoading, - error: questionnaireError, + error: questionnaireError, + } = useQuery<QuestionnaireDetail, Error>({ - }) = useQuery({ queryKey: ["questionnaireDetail", questionnaireSlug], queryFn: query(questionnaireApi.detail, { pathParams: { id: questionnaireSlug ?? "" }, }), enabled: !!questionnaireSlug && !FIXED_QUESTIONNAIRES[questionnaireSlug], });
Line range hint
115-124: Enhance error handling with retry mechanism and detailed error states.Consider implementing a more robust error handling strategy:
- Add retry mechanism for transient failures
- Handle network errors explicitly
- Display more detailed error information when available
if (questionnaireError) { + const isNetworkError = questionnaireError instanceof Error && + questionnaireError.message === 'Network Error'; return ( <Alert variant="destructive" className="m-4"> <AlertTitle>{t("questionnaire_error_loading")}</AlertTitle> - <AlertDescription>{t("questionnaire_not_exist")}</AlertDescription> + <AlertDescription> + {isNetworkError + ? t("network_error_try_again") + : t("questionnaire_not_exist")} + </AlertDescription> + <Button + onClick={() => window.location.reload()} + variant="outline" + className="mt-2" + > + {t("retry")} + </Button> </Alert> ); }
Line range hint
1-424: Consider implementing optimistic updates and caching strategy.While the migration to TanStack's react-query is well implemented, the component could benefit from additional react-query features:
- Implement optimistic updates for better UX
- Add proper cache invalidation strategy
- Consider implementing infinite query for large questionnaire lists
Example implementation for optimistic updates:
const { mutate: submitBatch } = useMutation({ mutationFn: mutate(routes.batchRequest, { silent: true }), onMutate: async (variables) => { // Cancel outgoing refetches await queryClient.cancelQueries({ queryKey: ['questionnaire'] }); // Snapshot the previous value const previousData = queryClient.getQueryData(['questionnaire']); // Optimistically update to the new value queryClient.setQueryData(['questionnaire'], (old) => ({ ...old, // Apply optimistic update })); // Return context with the snapshotted value return { previousData }; }, onError: (err, variables, context) => { // If the mutation fails, use the context we returned above queryClient.setQueryData(['questionnaire'], context.previousData); }, onSettled: () => { // Always refetch after error or success queryClient.invalidateQueries({ queryKey: ['questionnaire'] }); }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Questionnaire/QuestionnaireEditor.tsx(4 hunks)src/components/Questionnaire/QuestionnaireForm.tsx(3 hunks)
🧰 Additional context used
📓 Learnings (1)
src/components/Questionnaire/QuestionnaireForm.tsx (1)
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9838
File: src/components/Questionnaire/QuestionnaireForm.tsx:100-105
Timestamp: 2025-01-14T05:04:43.093Z
Learning: The QuestionnaireForm component in care_fe uses raviger's useNavigationPrompt hook for handling navigation prompts, replacing a previous custom hook implementation.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (3)
src/components/Questionnaire/QuestionnaireEditor.tsx (2)
2-2: LGTM! Clean migration to TanStack Query.The imports are well-structured and follow best practices for React Query integration.
Also applies to: 32-32
Line range hint
93-113: LGTM! Comprehensive state handling.The component properly handles all query states (loading, error, empty) with appropriate UI feedback.
src/components/Questionnaire/QuestionnaireForm.tsx (1)
1-1: LGTM! Import changes align with TanStack migration.The migration from local useQuery to TanStack's react-query is implemented correctly.
Note: There's a TODO comment about switching to tanstack router's useBlocker hook in the future, which would be a good follow-up improvement.
Also applies to: 19-19
…ed loading state handling
|
@rithviknishad I have made changes to one more file (index), which is a part of questionnaires. Sorry that I missed it out early. |
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/components/Questionnaire/index.tsx (1)
Pagination implementation is needed - API already supports it
The API endpoint returns
PaginatedResponse<QuestionnaireDetail>, but the component isn't utilizing this pagination capability. Implementing pagination in the frontend component is necessary to:
- Handle large datasets efficiently
- Improve initial load performance
- Match the backend's paginated response structure
🔗 Analysis chain
Line range hint
25-91: Consider implementing pagination for large questionnaire lists.The table implementation is clean and well-structured, but it might need pagination for better performance with large datasets. Consider:
- Implementing pagination using TanStack Query's built-in features
- Adding infinite scroll or load more functionality
Let's verify if pagination is needed by checking the API response structure:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for pagination-related fields in the API response type ast-grep --pattern 'interface QuestionnaireDetail { $$$ }'Length of output: 849
Script:
#!/bin/bash # Search for questionnaire list fetching logic rg -A 5 "questionnaireList" --type typescript # Also search for any API endpoints related to questionnaires ast-grep --pattern 'get($$$"/questionnaire"$$$)'Length of output: 131
Script:
#!/bin/bash # Search for questionnaire list fetching logic in TypeScript files rg -t ts -t tsx -A 5 "questionnaireList" # Search for API calls using fetch or axios rg -t ts -t tsx "(/api/questionnaire|/questionnaire)" -A 3 # Look for pagination-related terms rg -t ts -t tsx "(pageSize|currentPage|pagination|infinite.*scroll)"Length of output: 256
Script:
#!/bin/bash # Find all TypeScript/React files first fd "\.(ts|tsx)$" src/ # Search for API/fetch patterns in all files grep -r "fetch.*questionnaire\|axios.*questionnaire" src/ # Use ast-grep to find React component with questionnaireList ast-grep --pattern 'const $$ = $$$questionnaireList$$$'Length of output: 16466
Script:
#!/bin/bash # Check the questionnaire API implementation cat src/types/questionnaire/questionnaireApi.ts # Look for pagination usage in questionnaire components grep -r "PaginatedList\|Pagination" src/components/Questionnaire/Length of output: 2882
🧹 Nitpick comments (2)
src/components/Questionnaire/index.tsx (2)
15-18: Enhance query configuration for better cache management.Consider the following improvements:
- Use a more specific query key to prevent cache collisions
- Add error handling
- Configure retry and cache settings
const { data: response, isLoading } = useQuery({ - queryKey: ["questionnaire"], + queryKey: ["questionnaires", "list"], // More specific key queryFn: query(questionnaireApi.list), + retry: 2, + staleTime: 5 * 60 * 1000, // 5 minutes + onError: (error) => { + console.error("Failed to fetch questionnaires:", error); + // Add error notification/handling here + }, });
20-23: Consider enhancing loading and error states for better UX.While the current loading state handling works, consider implementing:
- A skeleton UI instead of a generic loading spinner
- Error state UI for failed queries
- Empty state UI when no questionnaires exist
- if (isLoading) { - return <Loading />; + if (isLoading) { + return <QuestionnaireListSkeleton />; // Add skeleton UI component + } + + if (error) { + return ( + <ErrorState + message="Failed to load questionnaires" + retry={() => refetch()} + /> + ); } + + if (!questionnaireList.length) { + return <EmptyState message="No questionnaires found" />; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Questionnaire/index.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (1)
src/components/Questionnaire/index.tsx (1)
1-1: LGTM! Clean migration to TanStack Query.The migration from a custom
useQueryhook to@tanstack/react-queryis a good modernization step. The imports are well-organized, and the addition of thequeryutility suggests proper separation of concerns.Also applies to: 7-7
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/hooks/useQuestionnaireOptions.ts (1)
20-20: LGTM! Consider adding a query key factory.The query key change to "questionnaires" (plural) aligns well with REST resource naming conventions. To improve maintainability, consider extracting query keys to a centralized factory.
Example implementation:
// src/utils/queryKeys.ts export const queryKeys = { questionnaires: { list: (slug: string) => ["questionnaires", slug] as const, }, } as const;Then use it as:
queryKey: queryKeys.questionnaires.list(slug),src/components/Questionnaire/index.tsx (1)
20-23: LGTM! Consider adding a loading skeleton.The loading state handling looks good. Consider using a skeleton loader instead of the generic loading component for a better user experience.
Example implementation:
if (isLoading) { return ( <div className="animate-pulse"> {[...Array(5)].map((_, i) => ( <div key={i} className="h-16 bg-gray-200 mb-2 rounded" /> ))} </div> ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Questionnaire/index.tsx(1 hunks)src/hooks/useQuestionnaireOptions.ts(1 hunks)
|
LGTM |
nihal467
added
tested
and removed
question
| @@ -17,7 +17,7 @@ const DEFAULT_OPTIONS: EditQuestionnaireOption[] = [ | |||
|
|
|||
| export default function useQuestionnaireOptions(slug: string) { | |||
| const { data } = useQuery({ | |||
| queryKey: ["questionnaire", slug] as const, | |||
| queryKey: ["questionnaires", slug] as const, | |||
|
@abhimanyurajeesh Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌 |
Proposed Changes
Part of #9837
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Refactor
@tanstack/react-query.useQueryanduseMutationhooks.Bug Fixes