Prefill Structured Data; Add Edit Links for Structured #9992
Conversation
WalkthroughThis pull request introduces a comprehensive set of changes across multiple components and files, focusing on enhancing localization, improving type safety, and refactoring components related to patient information such as allergies, medications, diagnoses, and symptoms. The changes include adding new localization entries, removing an endpoint, updating component structures, introducing new layout components, and improving type definitions for allergy verification and clinical statuses. Changes
Suggested labels
Suggested reviewers
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (4)
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. |
Deploying care-fe with
|
| Latest commit: |
20ef2af
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0f04da46.care-fe.pages.dev |
| Branch Preview URL: | https://structured-prefills.care-fe.pages.dev |
CARE
|
||||||||||||||||||||||||||||
| Project |
CARE
|
| Branch Review |
structured_prefills
|
| Run status |
|
| Run duration | 02m 55s |
| Commit |
|
| Committer | Gigin George |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
5
|
| View all changes introduced in this branch ↗︎ | |
![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/Medicine/MedicationRequestTable/index.tsx (1)
Line range hint
211-218: Ensure consistent usage of status valuesThere's an inconsistency in the status value for "on hold": in the filtering logic, you use
"on_hold", but in thegetStatusVariantfunction, you check for"on-hold"(with a hyphen). This could lead to incorrect badge variants being displayed.Apply this diff to correct the status value in
getStatusVariant:switch (status) { case "active": return "default"; - case "on-hold": + case "on_hold": return "secondary"; case "cancelled": return "destructive"; case "completed": return "primary"; default: return "secondary"; }
🧹 Nitpick comments (8)
src/components/Medicine/MedicationRequestTable/index.tsx (3)
56-63: UsepatientIdinpathParamsfor consistencyIn the
useQuerycall, consider using the already definedpatientIdvariable instead ofencounter?.patient?.id || ""in thepathParams. This improves readability and ensures consistency throughout the code.Apply this diff:
- pathParams: { patientId: encounter?.patient?.id || "" }, + pathParams: { patientId: patientId || "" },
56-63: Add error handling for data fetchingConsider adding error handling to manage potential errors during data fetching with
useQuery. This ensures the component can gracefully handle and display error states to the user.
Line range hint
280-286: Implement the TODO for displaying dose and rateThere's a TODO comment indicating that the Medicine Administration Sheet needs to be rebuilt to properly display the dose and rate information. Implementing this functionality will enhance the completeness of the prescription entries.
Do you want me to help implement the dose and rate display or open a GitHub issue to track this task?
src/types/emr/allergyIntolerance/allergyIntolerance.ts (1)
41-47: Consider using i18n for verification status labels.The hardcoded English strings in
ALLERGY_VERIFICATION_STATUSshould be moved to i18n files for better internationalization support.-export const ALLERGY_VERIFICATION_STATUS = { - unconfirmed: "Unconfirmed", - confirmed: "Confirmed", - refuted: "Refuted", - presumed: "Presumed", - "entered-in-error": "Entered in Error", -}; +export const ALLERGY_VERIFICATION_STATUS = { + unconfirmed: t("allergy.verification.unconfirmed"), + confirmed: t("allergy.verification.confirmed"), + refuted: t("allergy.verification.refuted"), + presumed: t("allergy.verification.presumed"), + "entered-in-error": t("allergy.verification.entered_in_error"), +};src/components/Patient/symptoms/list.tsx (1)
69-97: Consider extracting common layout pattern.The layout component pattern is repeated across different list components. Consider creating a shared ListLayout component.
// src/components/shared/ListLayout.tsx interface ListLayoutProps { title: string; editHref?: string; facilityId?: string; children: ReactNode; } export const ListLayout = ({ title, editHref, facilityId, children, }: ListLayoutProps) => { return ( <Card> <CardHeader className="px-4 py-0 pt-4 flex justify-between flex-row"> <CardTitle>{t(title)}</CardTitle> {facilityId && editHref && ( <Link href={editHref} className="flex items-center gap-1 text-sm hover:text-gray-500" > <PencilIcon size={12} /> {t("edit")} </Link> )} </CardHeader> <CardContent>{children}</CardContent> </Card> ); };src/components/Patient/allergy/list.tsx (1)
114-119: Consider using a more explicit sorting function.The sorting logic could be more readable with a dedicated function and explicit return values.
- .sort((a, _b) => { - if (a.clinical_status === "inactive") { - return 1; - } - return -1; - }) + .sort((a, b) => { + const isAInactive = a.clinical_status === "inactive"; + const isBInactive = b.clinical_status === "inactive"; + return isAInactive === isBInactive ? 0 : isAInactive ? 1 : -1; + })src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx (1)
510-529: Enhance type safety with runtime validation.The desktop view implementation could benefit from additional type safety.
Consider this safer implementation:
onValueChange={(value) => { - if (value in ALLERGY_VERIFICATION_STATUS) { + if (Object.keys(ALLERGY_VERIFICATION_STATUS).includes(value)) { onUpdate?.({ verification_status: value as AllergyVerificationStatus, }); } }}src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (1)
77-101: Consider adding error handling for the query.The data fetching implementation looks good but could benefit from error handling.
Consider adding error handling:
-const { data: patientMedications } = useQuery({ +const { data: patientMedications, error } = useQuery({ queryKey: ["medications", patientId], queryFn: query(medicationRequestApi.list, { pathParams: { patientId }, }), }); useEffect(() => { + if (error) { + toast.error("Failed to fetch medications"); + return; + } if (patientMedications?.results && !medications.length) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
public/locale/en.json(1 hunks)src/Utils/request/api.tsx(0 hunks)src/components/Medicine/MedicationRequestTable/index.tsx(5 hunks)src/components/Patient/allergy/list.tsx(5 hunks)src/components/Patient/diagnosis/list.tsx(3 hunks)src/components/Patient/symptoms/list.tsx(3 hunks)src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx(4 hunks)src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx(4 hunks)src/components/Questionnaire/QuestionnaireForm.tsx(1 hunks)src/components/Questionnaire/data/StructuredFormData.tsx(1 hunks)src/pages/Encounters/PrintPrescription.tsx(2 hunks)src/pages/Encounters/tabs/EncounterMedicinesTab.tsx(1 hunks)src/pages/Encounters/tabs/EncounterUpdatesTab.tsx(2 hunks)src/types/emr/allergyIntolerance/allergyIntolerance.ts(2 hunks)src/types/emr/medicationRequest/medicationRequestApi.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/Utils/request/api.tsx
🧰 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.
🔇 Additional comments (26)
src/pages/Encounters/tabs/EncounterMedicinesTab.tsx (1)
1-8: Component replacement is correctly implementedThe
MedicationRequestTablecomponent replacesMedicineAdministrationSheetappropriately. The component is correctly imported and rendered with the requiredfacilityIdprop.src/types/emr/medicationRequest/medicationRequestApi.ts (1)
6-13: API module is correctly definedThe
medicationRequestApimodule is properly structured, and the endpoint for listing medication requests is correctly specified with the appropriate path, method, and response type.src/types/emr/allergyIntolerance/allergyIntolerance.ts (2)
4-11: LGTM! Well-defined type definitions enhance type safety.The introduction of
AllergyVerificationStatusandAllergyClinicalStatustypes helps prevent invalid values and improves code maintainability.
16-17: LGTM! Type-safe interface updates.Replacing string types with specific enum-like types improves type safety and provides better IntelliSense support.
Also applies to: 31-32
src/pages/Encounters/tabs/EncounterUpdatesTab.tsx (2)
10-13: LGTM! Consistent prop addition.The facilityId prop is correctly added to the component's props.
22-44: LGTM! Consistent prop passing to child components.The facilityId prop is consistently passed to all relevant list components.
src/components/Patient/symptoms/list.tsx (2)
18-18: LGTM! Clean props interface update.The facilityId prop is correctly added as optional.
Also applies to: 21-25
36-65: LGTM! Well-structured component refactoring.The use of SymptomListLayout improves code organization and reusability.
src/components/Patient/diagnosis/list.tsx (2)
69-97: Apply the same layout component suggestion.Similar to the symptoms list, consider using a shared ListLayout component here as well.
84-92: Verify the requirement for encounterId check.The edit link visibility differs from the symptoms component. Verify if both components should have the same conditions.
src/components/Questionnaire/data/StructuredFormData.tsx (5)
43-61: LGTM! Well-structured allergy questionnaire definition.The allergy intolerance questionnaire follows the established pattern and includes all required fields.
63-81: LGTM! Well-structured medication questionnaire definition.The medication statement questionnaire maintains consistency with other questionnaire definitions.
83-101: LGTM! Well-structured diagnosis questionnaire definition.The diagnosis questionnaire follows the same pattern as other questionnaires.
103-121: LGTM! Well-structured symptom questionnaire definition.The symptom questionnaire maintains consistency with other questionnaire definitions.
126-129: LGTM! Questionnaires properly exported.All new questionnaires are correctly added to the FIXED_QUESTIONNAIRES export.
src/components/Patient/allergy/list.tsx (3)
26-26: LGTM! Optional facilityId prop added.The facilityId prop is correctly marked as optional.
31-35: LGTM! Component signature updated.The AllergyList component signature properly includes the new facilityId prop.
166-194: LGTM! Well-structured layout component.The AllergyListLayout component properly handles the conditional rendering of the edit link based on facilityId and encounterId.
src/pages/Encounters/PrintPrescription.tsx (1)
27-27: LGTM! API import and usage updated.The medicationRequestApi import and its usage in the query function are correctly implemented.
Also applies to: 96-96
src/components/Questionnaire/QuestionnaireForm.tsx (1)
101-101: LGTM! Navigation prompt disabled in dev mode.Good improvement for developer experience. The change aligns with the retrieved learning about using raviger's useNavigationPrompt hook.
src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx (2)
50-53: LGTM! Good type safety improvements.The imports are well-organized and improve type safety by introducing dedicated types and constants for allergy verification statuses.
Line range hint
355-371: LGTM! Clean implementation of verification status handling.The mobile view implementation correctly uses type assertion and maps through the verification statuses dynamically.
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (3)
2-2: LGTM! Well-organized imports.The imports are properly organized and include necessary dependencies for data fetching and notifications.
Also applies to: 4-5
46-46: LGTM! Good separation of concerns.The API and utility imports maintain a clean separation of concerns.
Also applies to: 57-57
62-62: LGTM! Clear interface definition.The props interface is well-defined with the new patientId requirement.
Also applies to: 72-72
public/locale/en.json (1)
1346-1346: LGTM! Clear and consistent message.The new localization entry follows the established pattern and provides a clear message.
|
👋 Hi, @gigincg, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (3)
src/pages/Encounters/EncounterShow.tsx (2)
Line range hint
91-156: Clean up commented code and improve error handling.There's a significant amount of commented-out code that should be removed if no longer needed. Additionally, error handling could be improved.
Consider:
- Removing commented code if it's no longer needed
- Adding error boundaries for better error isolation
- Implementing comprehensive error states
- // const encounterQuery = useTanStackQueryInstead(routes.encounter.get, { - // pathParams: { id: consultationId }, - // }); - - // const consultationData = encounterQuery.data; - // const bedId = consultationData?.current_bed?.bed_object?.id;
Line range hint
179-285: Enhance accessibility in navigation and tab components.The navigation and tab components need accessibility improvements:
- Add proper ARIA attributes for tab navigation
- Ensure keyboard navigation works correctly
- Add screen reader support
Example improvements:
<nav className="flex space-x-6 overflow-x-auto pb-2 pl-2" id="encounter_tab_nav" + role="tablist" + aria-label={t('encounter_tabs')} > {keysOf(tabs).map((tab) => ( <Link key={tab} className={tabButtonClasses(props.tab === tab)} href={`/facility/${facilityId}/encounter/${encounterData.id}/${tab}`} + role="tab" + aria-selected={props.tab === tab} + aria-controls={`${tab}-panel`} > {t(`ENCOUNTER_TAB__${tab}`)} </Link> ))} </nav>src/components/Questionnaire/QuestionTypes/MedicationStatementQuestion.tsx (1)
Line range hint
105-122: Remove type casting and improve type safety in handleAddMedication.The FIXME comment indicates a type casting issue that should be addressed.
- value: newMedications as MedicationStatement[], // FIXME: Remove this cast + value: newMedications,Consider updating the MEDICATION_STATEMENT_INITIAL_VALUE type to ensure type safety without casting:
const MEDICATION_STATEMENT_INITIAL_VALUE: Omit<MedicationStatement, 'medication' | 'patient' | 'encounter' | 'id'> & { status: MedicationStatementStatus; } = { status: "active", reason: undefined, dosage_text: "", effective_period: undefined, information_source: MedicationStatementInformationSourceType.PATIENT, note: undefined, };
🧹 Nitpick comments (6)
src/pages/Encounters/EncounterShow.tsx (2)
Line range hint
22-34: Consider splitting component responsibilities and improving data flow.The component handles multiple concerns (data fetching, routing, UI). Consider:
- Extracting tab configuration into a separate module
- Creating a custom hook for encounter data fetching
- Re-evaluating the removal of
EncounterProvideras prop drilling might impact maintainabilityAlso applies to: 39-43
Line range hint
39-43: Improve type safety and centralize route handling.Consider:
- Creating typed route constants
- Centralizing URL construction
- Adding route parameter validation
Example implementation:
// routes.ts export const encounterRoutes = { tab: (params: { facilityId: string, encounterId: string, tab: keyof typeof defaultTabs }) => `/facility/${params.facilityId}/encounter/${params.encounterId}/${params.tab}`, // ... other routes } as const;Also applies to: 246-259
src/components/Medicine/MedicationRequestTable/index.tsx (1)
Line range hint
82-90: Enhance internationalization coverage.Several UI strings should be moved to i18n files to align with the PR objectives:
- "No matches found"
- "No Prescriptions"
- "No medications match"
- "No medications have been prescribed yet"
Consider moving these strings to the i18n files:
- <h3 className="font-medium"> - {searching ? "No matches found" : "No Prescriptions"} - </h3> - <p className="text-sm text-muted-foreground"> - {searching - ? `No medications match "${searchQuery}"` - : "No medications have been prescribed yet"} - </p> + <h3 className="font-medium"> + {searching ? t("no_matches_found") : t("no_prescriptions")} + </h3> + <p className="text-sm text-muted-foreground"> + {searching + ? t("no_medications_match", { query: searchQuery }) + : t("no_medications_prescribed")} + </p>Also applies to: 156-159
src/components/Questionnaire/QuestionTypes/MedicationStatementQuestion.tsx (1)
85-102: Consider adding loading state handling and race condition prevention.While the initialization logic works, there are a few improvements to consider:
- Add loading state handling to prevent UI flicker
- Add error handling for failed requests
- Prevent overwriting existing medications if they were manually added while fetching
useEffect(() => { - if (patientMedications?.results && !medications.length) { + if (patientMedications?.results && !medications.length && !isError) { updateQuestionnaireResponseCB({ ...questionnaireResponse, values: [ { type: "medication_statement", value: patientMedications.results, }, ], }); if (patientMedications.count > patientMedications.results.length) { toast.info( `Showing first ${patientMedications.results.length} of ${patientMedications.count} medication statements`, ); } + } else if (isError) { + toast.error("Failed to fetch existing medications"); } -}, [patientMedications]); +}, [patientMedications, isError]);src/types/emr/medicationStatement/medicationStatementApi.ts (2)
8-8: Add type safety for path parameters.The path parameter
{patientId}should be properly typed to ensure type safety when making API calls.Consider adding path parameter typing:
- path: "/api/v1/patient/{patientId}/medication/statement/", + path: "/api/v1/patient/{patientId}/medication/statement/" as const, + params: Type<{ patientId: string }>(),
6-12: Consider adding CRUD operations for complete structured data support.Since this PR aims to add edit support for structured fields, consider extending the API to include:
- POST endpoint for creating new statements
- PUT/PATCH endpoint for updating statements
- DELETE endpoint for removing statements
This would provide a complete set of operations for managing medication statements.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/components/Facility/ConsultationDetails/EncounterContext.tsx(0 hunks)src/components/Medicine/MedicationRequestTable/index.tsx(5 hunks)src/components/Questionnaire/QuestionTypes/MedicationStatementQuestion.tsx(5 hunks)src/pages/Encounters/EncounterShow.tsx(2 hunks)src/pages/Encounters/tabs/EncounterMedicinesTab.tsx(1 hunks)src/types/emr/medicationStatement/medicationStatementApi.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/components/Facility/ConsultationDetails/EncounterContext.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (11)
src/pages/Encounters/tabs/EncounterMedicinesTab.tsx (1)
1-1: LGTM! Clean component refactor.The switch to
MedicationRequestTablewith additional props improves type safety and data handling. Props are correctly derived from the parent component.Also applies to: 8-12
src/components/Medicine/MedicationRequestTable/index.tsx (3)
1-1: LGTM! Well-implemented data fetching with React Query.The implementation follows best practices:
- Type-safe props interface
- Proper query key structure
- Query enabled condition prevents unnecessary requests
Also applies to: 17-17, 20-20, 22-26, 56-63
107-114: LGTM! Edit functionality properly implemented.The edit button implementation:
- Follows UI/UX patterns
- Uses correct routing structure
- Maintains consistent styling
Line range hint
284-301: Track technical debt: Rebuild Medicine Administration Sheet.There's a TODO comment and commented-out code for dose_and_rate handling. Let's ensure this is tracked for future implementation.
Would you like me to:
- Create a GitHub issue to track this technical debt?
- Help implement the dose_and_rate handling?
src/components/Questionnaire/QuestionTypes/MedicationStatementQuestion.tsx (4)
Line range hint
1-48: LGTM! Import statements and prop interface changes are well-organized.The new imports and the addition of the
patientIdprop align well with the enhanced functionality for medication management.
78-83: LGTM! Data fetching implementation is correct.The query key and fetching logic are properly implemented using
@tanstack/react-query.
124-143: LGTM! Medication removal logic handles existing and new medications appropriately.The differentiation between existing and new medications is well implemented, preserving history for existing records while allowing complete removal of new ones.
238-264: LGTM! The remove button implementation with tooltips enhances user experience.The UI changes effectively communicate the difference between removing new medications and marking existing ones as entered-in-error. The tooltips and icons provide clear visual feedback.
src/types/emr/medicationStatement/medicationStatementApi.ts (3)
1-4: LGTM! Imports are well-structured.The imports are appropriate for the functionality, using proper type imports and consistent path aliasing.
14-14: LGTM! Export is appropriate.The default export of the API constant follows the module pattern consistently.
6-12: Verify the impact of switching from medicationRequest to medicationStatement.Based on the AI summary, this endpoint is replacing usage of
medicationRequestApi. This architectural change might require:
- Data migration plan for existing medication requests
- Update of dependent components
- Backwards compatibility consideration
Let's verify the impact:
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (3)
144-165: Improve type safety and maintainability of status handling.The status handling could be improved by using type assertions and constants for better maintainability.
+const MEDICATION_STATUS = { + ENTERED_IN_ERROR: 'entered_in_error' as const +} as const; const medication = medications[medicationToDelete]; if (medication.id) { // For existing records, update status to entered-in-error const newMedications = medications.map((med, i) => i === medicationToDelete - ? { ...med, status: "entered_in_error" as const } + ? { ...med, status: MEDICATION_STATUS.ENTERED_IN_ERROR } : med, );
519-526: Improve semantics of error state styling.The opacity styling for error state could be more semantic and maintainable.
- className={cn( - "grid grid-cols-1 lg:grid-cols-[280px,180px,170px,160px,300px,230px,180px,250px,180px,160px,48px] border-b hover:bg-gray-50/50", - { - "opacity-50": medication.status === "entered_in_error", - }, - )} + className={cn( + "grid grid-cols-1 lg:grid-cols-[280px,180px,170px,160px,300px,230px,180px,250px,180px,160px,48px] border-b hover:bg-gray-50/50", + "medication-row", + { + "medication-row--error": medication.status === MEDICATION_STATUS.ENTERED_IN_ERROR, + }, + )}Add the following CSS:
.medication-row--error { opacity: 0.5; pointer-events: none; }
319-344: Extract tooltip text to constants.The tooltip text should be extracted to constants for better maintainability and reusability.
+const TOOLTIP_TEXT = { + ENTERED_IN_ERROR: t("medication_already_marked_as_error"), + REMOVE: t("remove_medication"), +} as const; <TooltipContent> - {medication.status === "entered_in_error" - ? t("medication_already_marked_as_error") - : t("remove_medication")} + {medication.status === MEDICATION_STATUS.ENTERED_IN_ERROR + ? TOOLTIP_TEXT.ENTERED_IN_ERROR + : TOOLTIP_TEXT.REMOVE} </TooltipContent>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
| const { data: patientMedications } = useQuery({ | ||
| queryKey: ["medications", patientId], | ||
| queryFn: query(medicationRequestApi.list, { | ||
| pathParams: { patientId }, | ||
| }), | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error and loading state handling for the query.
The useQuery implementation lacks error handling and loading state management, which could lead to poor user experience when the API call fails or is in progress.
const { data: patientMedications } = useQuery({
+ enabled: !!patientId,
queryKey: ["medications", patientId],
queryFn: query(medicationRequestApi.list, {
pathParams: { patientId },
}),
});
+const isLoading = !patientMedications;
+if (isLoading) {
+ return <LoadingSpinner />;
+}Committable suggestion skipped: line range outside the PR's diff.
| useEffect(() => { | ||
| if (patientMedications?.results && !medications.length) { | ||
| updateQuestionnaireResponseCB({ | ||
| ...questionnaireResponse, | ||
| values: [ | ||
| { | ||
| type: "medication_request", | ||
| value: patientMedications.results, | ||
| }, | ||
| ], | ||
| }); | ||
| if (patientMedications.count > patientMedications.results.length) { | ||
| toast.info( | ||
| `Showing first ${patientMedications.results.length} of ${patientMedications.count} medications`, | ||
| ); | ||
| } | ||
| } | ||
| }, [patientMedications]); |
There was a problem hiding this comment.
Fix useEffect dependencies and potential race condition.
The useEffect hook is missing questionnaireResponse and updateQuestionnaireResponseCB in its dependency array, which could lead to stale closures. Also, there's a potential race condition if the medications are updated elsewhere while fetching.
useEffect(() => {
- if (patientMedications?.results && !medications.length) {
+ if (patientMedications?.results && !medications.length && !isLoading) {
updateQuestionnaireResponseCB({
...questionnaireResponse,
values: [
{
type: "medication_request",
value: patientMedications.results,
},
],
});
if (patientMedications.count > patientMedications.results.length) {
toast.info(
`Showing first ${patientMedications.results.length} of ${patientMedications.count} medications`,
);
}
}
-}, [patientMedications]);
+}, [patientMedications, medications.length, questionnaireResponse, updateQuestionnaireResponseCB]);Committable suggestion skipped: line range outside the PR's diff.
…structured_prefills
…structured_prefills
…structured_prefills
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/types/emr/diagnosis/diagnosis.ts (1)
Standardize to underscore_based naming convention for verification status
The codebase predominantly uses
entered_in_error(with underscores). For consistency, update the following files to use underscores:
- src/types/emr/diagnosis/diagnosis.ts
- src/types/emr/allergyIntolerance/allergyIntolerance.ts
🔗 Analysis chain
Line range hint
15-21: Verify consistency with symptom verification status naming.The symptom.ts file uses
entered_in_errorwhile this file usesentered-in-error. Consider standardizing the naming convention across both files.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for both versions of the status to check consistency echo "Searching for hyphenated version:" rg "entered-in-error" --type ts echo -e "\nSearching for underscore version:" rg "entered_in_error" --type tsLength of output: 3890
🧹 Nitpick comments (3)
src/components/Questionnaire/QuestionTypes/SymptomQuestion.tsx (1)
294-297: Internationalize the hardcoded toast message.The toast message informing users about the number of symptoms displayed is hardcoded in English. To support localization and maintain consistency, use the
tfunction for internationalization.Apply this diff to use the
tfunction:- toast.info( - `Showing first ${patientSymptoms.results.length} of ${patientSymptoms.count} symptoms`, - ); + toast.info( + t('showing_first_n_of_m_symptoms', { first: patientSymptoms.results.length, total: patientSymptoms.count }) + );Remember to add the translation key
showing_first_n_of_m_symptomsto your i18n files.src/components/Questionnaire/QuestionTypes/DiagnosisQuestion.tsx (1)
105-108: Internationalize the hardcoded toast message.The toast message is hardcoded in English, which may not cater to all users. Use the
tfunction to internationalize the message and provide a consistent experience for users in different locales.Apply this diff to use the
tfunction:- toast.info( - `Showing first ${patientDiagnoses.results.length} of ${patientDiagnoses.count} diagnoses`, - ); + toast.info( + t('showing_first_n_of_m_diagnoses', { first: patientDiagnoses.results.length, total: patientDiagnoses.count }) + );Ensure that the translation key
showing_first_n_of_m_diagnosesis added to your i18n files with appropriate translations.src/components/Questionnaire/structured/types.ts (1)
18-19: LGTM: Consistent type updates supporting bulk operations.The changes correctly update both maps to use request types, with
StructuredRequestMapsupporting bulk operations through the datapoints array wrapper.Consider adding a comment explaining the purpose of the datapoints array wrapper in
StructuredRequestMap, as it differs from the singular types inStructuredDataMap.Also applies to: 29-30
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
public/locale/en.json(14 hunks)src/components/Questionnaire/QuestionTypes/DiagnosisQuestion.tsx(10 hunks)src/components/Questionnaire/QuestionTypes/SymptomQuestion.tsx(4 hunks)src/components/Questionnaire/structured/handlers.ts(1 hunks)src/components/Questionnaire/structured/types.ts(3 hunks)src/types/emr/diagnosis/diagnosis.ts(2 hunks)src/types/emr/symptom/symptom.ts(3 hunks)src/types/emr/symptom/symptomApi.ts(2 hunks)src/types/questionnaire/form.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- public/locale/en.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (7)
src/components/Questionnaire/QuestionTypes/DiagnosisQuestion.tsx (1)
139-140: Ensure consistency of 'verification_status' values.The
verification_statusis set to"entered-in-error"with hyphens here, which is appropriate. Confirm that this value is consistently used across the codebase, especially inSymptomQuestion, to prevent any discrepancies.src/types/emr/symptom/symptomApi.ts (1)
17-22: Verify API endpoint alignment with backend specifications.Ensure that the new
upsertSymptomsendpoint aligns with the backend API specifications. Confirm that the path, HTTP method, request body structure (including the use of thedatapointskey), and response types are correctly defined and expected by the backend service.src/types/emr/diagnosis/diagnosis.ts (1)
36-36: LGTM: ID field additions support edit functionality.The addition of
idfields to both interfaces is well-structured - required for existing diagnoses and optional for requests.Also applies to: 48-48
src/types/emr/symptom/symptom.ts (1)
39-39: LGTM: ID field additions are consistent with diagnosis types.The addition of
idfields mirrors the structure in diagnosis.ts, maintaining consistency across similar entities.Also applies to: 52-52
src/types/questionnaire/form.ts (1)
2-2: LGTM: Consistent usage of request types for form responses.The changes correctly update the imports and response value types to use request types (
SymptomRequest[]andDiagnosisRequest[]) while maintaining the type discriminators.Also applies to: 6-6, 34-35
src/components/Questionnaire/structured/handlers.ts (2)
100-114: Reduce code duplication with symptoms handler.The diagnosis and symptoms handlers share identical logic. Consider extracting the common functionality into a shared utility function.
+ const createDatapointsRequest = ( + datapoints: unknown[], + encounterId: string, + referenceId: string, + endpoint: string + ) => ({ + url: endpoint, + method: "POST", + body: { + datapoints: datapoints.map((item) => ({ + ...item, + encounter: encounterId, + })), + }, + reference_id: referenceId, + }); symptom: { getRequests: (symptoms, { patientId, encounterId }) => { - return [{ - url: `/api/v1/patient/${patientId}/symptom/upsert/`, - method: "POST", - body: { - datapoints: symptoms.map((symptom) => ({ - ...symptom, - encounter: encounterId, - })), - }, - reference_id: "symptom", - }]; + if (!symptoms.length) return []; + return [ + createDatapointsRequest( + symptoms, + encounterId, + "symptom", + `/api/v1/patient/${patientId}/symptom/upsert/` + ), + ]; }, }, diagnosis: { getRequests: (diagnoses, { patientId, encounterId }) => { - return [{ - url: `/api/v1/patient/${patientId}/diagnosis/upsert/`, - method: "POST", - body: { - datapoints: diagnoses.map((diagnosis) => ({ - ...diagnosis, - encounter: encounterId, - })), - }, - reference_id: "diagnosis", - }]; + if (!diagnoses.length) return []; + return [ + createDatapointsRequest( + diagnoses, + encounterId, + "diagnosis", + `/api/v1/patient/${patientId}/diagnosis/upsert/` + ), + ]; }, },Verify patient ID requirement in diagnosis data.
Similar to symptoms, verify if omitting
patient: patientIdin the mapped data is intentional for diagnoses.
83-97: Add validation for empty symptoms array.Consider adding validation to handle empty arrays gracefully and prevent unnecessary API calls.
getRequests: (symptoms, { patientId, encounterId }) => { + if (!symptoms.length) return []; return [ {Verify patient ID requirement in symptom data.
Other handlers (e.g., medication) include
patient: patientIdin the mapped data. Verify if this is intentionally omitted for symptoms.
| <DropdownMenuItem | ||
| className="text-destructive focus:text-destructive" | ||
| onClick={onRemove} | ||
| disabled={verificationStatus === "entered_in_error"} |
There was a problem hiding this comment.
Inconsistent 'verification_status' values.
The verification_status value is set to "entered_in_error" with underscores in SymptomQuestion, while in DiagnosisQuestion, it is set to "entered-in-error" with hyphens. For consistency and compliance with standards (e.g., FHIR uses hyphens), please standardize the verification_status values across the codebase.
Apply this diff to correct the verification_status value:
Line 323:
- ? { ...s, verification_status: "entered_in_error" as const }
+ ? { ...s, verification_status: "entered-in-error" as const }
Line 122:
- disabled={verificationStatus === "entered_in_error"}
+ disabled={verificationStatus === "entered-in-error"}Also applies to: 323-323
| @@ -18,7 +18,7 @@ export const SYMPTOM_VERIFICATION_STATUS = [ | |||
| "differential", | |||
| "confirmed", | |||
| "refuted", | |||
| "entered-in-error", | |||
| "entered_in_error", | |||
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent verification status format needs standardization
The codebase shows mixed usage of hyphenated (entered-in-error) and underscore (entered_in_error) formats. This inconsistency could lead to runtime issues.
Key files requiring format standardization:
src/types/emr/diagnosis/diagnosis.ts: Uses hyphenatedsrc/types/emr/allergyIntolerance/allergyIntolerance.ts: Uses hyphenatedsrc/components/Questionnaire/QuestionTypes/DiagnosisQuestion.tsx: Uses hyphenated- Translation files in
public/locale/: Mixed usage
🔗 Analysis chain
Verify impact of verification status format change.
The change from hyphenated to underscore format (entered-in-error → entered_in_error) might affect existing code that relies on these values.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usages of both formats to assess impact
echo "Searching for verification status usage in components:"
rg -t tsx "verification_status.*entered[-_]in[-_]error"
echo -e "\nSearching for direct string usage:"
rg "entered[-_]in[-_]error"
Length of output: 5360
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
src/components/Questionnaire/QuestionTypes/MedicationStatementQuestion.tsx (1)
85-102: Enhance the pagination notification message.The auto-population logic is good, but the toast message could be more informative.
Consider enhancing the toast message:
- toast.info( - `Showing first ${patientMedications.results.length} of ${patientMedications.count} medication statements`, - ); + toast.info( + t("showing_x_of_y_medications", { + shown: patientMedications.results.length, + total: patientMedications.count, + remaining: patientMedications.count - patientMedications.results.length + }), + { + description: t("use_medication_history_to_view_all") + } + );src/types/emr/allergyIntolerance/allergyIntolerance.ts (1)
41-47: Consider using i18n for status labels.While the mapping is well-structured, consider moving these display strings to i18n files to support multiple languages, as mentioned in the PR checklist.
-export const ALLERGY_VERIFICATION_STATUS = { - unconfirmed: "Unconfirmed", - confirmed: "Confirmed", - refuted: "Refuted", - presumed: "Presumed", - entered_in_error: "Entered in Error", -}; +export const ALLERGY_VERIFICATION_STATUS = { + unconfirmed: t("allergy.verification_status.unconfirmed"), + confirmed: t("allergy.verification_status.confirmed"), + refuted: t("allergy.verification_status.refuted"), + presumed: t("allergy.verification_status.presumed"), + entered_in_error: t("allergy.verification_status.entered_in_error"), +};src/components/Questionnaire/QuestionTypes/DiagnosisQuestion.tsx (2)
134-163: Consider removing unnecessary type assertion.The type assertion
as DiagnosisRequest[]on line 141 might be unnecessary since the map operation should preserve the type. Consider removing it if TypeScript doesn't complain:- ) as DiagnosisRequest[]; + );
Line range hint
185-214: Consider adding explicit aria-labels.While the implementation is accessible through Radix UI components, consider adding explicit aria-labels to improve screen reader experience:
- <ValueSetSelect + <ValueSetSelect + aria-label={t("diagnosis_search")} system="system-condition-code" placeholder={t("search_for_diagnoses_to_add")} onSelect={handleAddDiagnosis} disabled={disabled} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
public/locale/en.json(17 hunks)public/locale/hi.json(2 hunks)public/locale/kn.json(2 hunks)public/locale/ml.json(2 hunks)public/locale/ta.json(2 hunks)src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx(5 hunks)src/components/Questionnaire/QuestionTypes/DiagnosisQuestion.tsx(10 hunks)src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx(7 hunks)src/components/Questionnaire/QuestionTypes/MedicationStatementQuestion.tsx(5 hunks)src/types/emr/allergyIntolerance/allergyIntolerance.ts(2 hunks)src/types/emr/diagnosis/diagnosis.ts(3 hunks)src/types/emr/symptom/symptom.ts(3 hunks)
✅ Files skipped from review due to trivial changes (3)
- public/locale/ta.json
- public/locale/hi.json
- public/locale/kn.json
🚧 Files skipped from review as they are similar to previous changes (4)
- src/types/emr/symptom/symptom.ts
- src/types/emr/diagnosis/diagnosis.ts
- src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx
- public/locale/en.json
🔇 Additional comments (13)
public/locale/ml.json (1)
905-905:⚠️ Potential issueRemove duplicate key "entered_in_error"
The key "entered_in_error" appears twice with different translations. This will cause inconsistency in the UI as only one translation will be used.
Keep the first translation "തെറ്റായി നൽകി" as it more accurately represents "entered in error" and remove the duplicate entry:
- "entered_in_error": "പിശകിൽ പ്രവേശിച്ചു",Likely invalid or redundant comment.
src/components/Questionnaire/QuestionTypes/MedicationStatementQuestion.tsx (3)
Line range hint
1-48: LGTM! Well-structured imports and prop definition.The new imports and the addition of the
patientIdprop are well-organized and properly typed, supporting the component's enhanced functionality.
124-143: LGTM! Well-implemented medication removal logic.The differentiated handling of new vs. existing medications is well thought out, maintaining data integrity by using "entered_in_error" status for existing records while allowing complete removal of new ones.
238-264: LGTM! Well-implemented tooltip UI with proper accessibility.The tooltip implementation provides clear user feedback with proper accessibility support through TooltipTrigger. The conditional rendering of different icons and messages based on the medication's state is a good UX enhancement.
src/types/emr/allergyIntolerance/allergyIntolerance.ts (3)
4-9: LGTM! Well-defined type following FHIR standards.The
AllergyVerificationStatustype correctly defines all standard FHIR allergy verification statuses.
11-11: LGTM! Well-defined type following FHIR standards.The
AllergyClinicalStatustype correctly defines the standard FHIR clinical status values.
16-17: Great improvement in type safety!Replacing
stringwith specific union types (AllergyClinicalStatusandAllergyVerificationStatus) enforces valid values at compile time, reducing potential runtime errors.Also applies to: 31-32
src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx (3)
365-371: LGTM! Good use of centralized constants.The implementation properly uses
ALLERGY_VERIFICATION_STATUSto maintain consistency in display values across the application.Also applies to: 523-529
510-516: Great addition of runtime type safety!The implementation adds both compile-time and runtime type checking for verification status updates:
- Compile-time through TypeScript's type assertion
- Runtime through the
inoperator check
342-344: Verify data migration for criticality value change.The change from
unable-to-assesstounable_to_assessis a breaking change that might affect existing data. Ensure that:
- A data migration script exists to update existing records
- API endpoints are updated to handle both old and new formats during transition
Also applies to: 503-503
src/components/Questionnaire/QuestionTypes/DiagnosisQuestion.tsx (3)
6-10: LGTM! The new imports enhance the component's capabilities.The additions of React Query for data fetching, date formatting utilities, internationalization support, and toast notifications align well with the component's enhanced functionality.
Also applies to: 32-32, 37-37, 39-39
44-44: Type safety improvement looks good.The addition of the
patientIdprop and the transition toDiagnosisRequesttype improves type safety and maintains consistency with the backend API contract.Also applies to: 50-55
86-115: Well-implemented data fetching and prefilling logic.Good use of React Query for data fetching and proper handling of partial results with user notification. The useEffect dependencies are correctly specified.
| const { data: patientMedications } = useQuery({ | ||
| queryKey: ["medication_statements", patientId], | ||
| queryFn: query(medicationStatementApi.list, { | ||
| pathParams: { patientId }, | ||
| }), | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for the API call.
The query setup looks good, but it's missing error handling for failed API calls.
Consider adding error handling:
const { data: patientMedications } = useQuery({
queryKey: ["medication_statements", patientId],
queryFn: query(medicationStatementApi.list, {
pathParams: { patientId },
}),
+ onError: (error) => {
+ toast.error(t("error_fetching_medications"));
+ },
});📝 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.
| const { data: patientMedications } = useQuery({ | |
| queryKey: ["medication_statements", patientId], | |
| queryFn: query(medicationStatementApi.list, { | |
| pathParams: { patientId }, | |
| }), | |
| }); | |
| const { data: patientMedications } = useQuery({ | |
| queryKey: ["medication_statements", patientId], | |
| queryFn: query(medicationStatementApi.list, { | |
| pathParams: { patientId }, | |
| }), | |
| onError: (error) => { | |
| toast.error(t("error_fetching_medications")); | |
| }, | |
| }); |
| {medication.status === "entered_in_error" | ||
| ? t("medication_already_marked_as_error") | ||
| : medication.id | ||
| ? t("mark_as_entered_in_error") | ||
| : t("remove_medication")} |
There was a problem hiding this comment.
💡 Codebase verification
Missing required translation key for error state message
The translation key "medication_already_marked_as_error" is missing from the locale files. This will cause issues when displaying the message for medications marked as entered in error.
- Add the missing translation key "medication_already_marked_as_error" to
public/locale/en.json
🔗 Analysis chain
Verify I18n entries for tooltip messages.
Ensure that the following translation keys exist in the I18n files:
- "medication_already_marked_as_error"
- "mark_as_entered_in_error"
- "remove_medication"
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if all required translation keys exist in I18n files
rg -l "medication_already_marked_as_error|mark_as_entered_in_error|remove_medication" public/locale/
Length of output: 124
Script:
#!/bin/bash
# Extract and display the specific translation entries
rg -U "\"(medication_already_marked_as_error|mark_as_entered_in_error|remove_medication)\".*" public/locale/en.json
Length of output: 217
| <div className="col-span-5">{t("diagnosis")}</div> | ||
| <div className="col-span-2 text-center">{t("date")}</div> | ||
| <div className="col-span-2 text-center">{t("status")}</div> | ||
| <div className="col-span-2 text-center">{t("verification")}</div> | ||
| <div className="col-span-1 text-center">{t("action")}</div> |
There was a problem hiding this comment.
💡 Codebase verification
Missing translations detected in locale files
Several translation keys are missing in non-English locale files:
verification(missing in ta, kn, ml, hi)status_placeholder(missing in all locales)verification_placeholder(missing in all locales)hide_notes(missing in ta, kn, ml, hi)remove_diagnosis(missing in ta, kn, ml, hi)add_notes_about_diagnosis(missing in ta, kn, ml, hi)
Only status and add_notes are properly translated across all locales.
🔗 Analysis chain
Verify translation key definitions.
The internationalization implementation is thorough. Please ensure all these translation keys are defined in the locale files:
- status
- verification
- status_placeholder
- verification_placeholder
- hide_notes
- add_notes
- remove_diagnosis
- add_notes_about_diagnosis
Also applies to: 258-258, 266-266, 274-276, 290-292, 303-303, 312-312, 320-320, 333-333, 342-342, 364-364, 372-372, 382-382
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if all translation keys are defined
rg -l "\"status\":|\"verification\":|\"status_placeholder\":|\"verification_placeholder\":|\"hide_notes\":|\"add_notes\":|\"remove_diagnosis\":|\"add_notes_about_diagnosis\":" public/locale/
Length of output: 286
Script:
#!/bin/bash
# Check each translation key individually and show their values
for key in "status" "verification" "status_placeholder" "verification_placeholder" "hide_notes" "add_notes" "remove_diagnosis" "add_notes_about_diagnosis"; do
echo "=== Checking key: $key ==="
rg "\"$key\":" public/locale/*.json
done
Length of output: 3965
| function convertToDiagnosisRequest(diagnosis: Diagnosis): DiagnosisRequest { | ||
| return { | ||
| id: diagnosis.id, | ||
| code: diagnosis.code, | ||
| clinical_status: diagnosis.clinical_status, | ||
| verification_status: diagnosis.verification_status, | ||
| onset: diagnosis.onset | ||
| ? { | ||
| ...diagnosis.onset, | ||
| onset_datetime: diagnosis.onset.onset_datetime | ||
| ? format(new Date(diagnosis.onset.onset_datetime), "yyyy-MM-dd") | ||
| : "", | ||
| } | ||
| : undefined, | ||
| recorded_date: diagnosis.recorded_date, | ||
| note: diagnosis.note, | ||
| encounter: "", // This will be set when submitting the form | ||
| }; | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Consider passing encounterId to the conversion function
The empty string approach is problematic as encounter is a required field in DiagnosisRequest. Instead of setting it empty and relying on later mutation:
- Pass the
encounterIdas a parameter toconvertToDiagnosisRequest - Set the encounter field directly during conversion, similar to other request handlers in the codebase
🔗 Analysis chain
Verify the empty encounter field handling.
While the conversion function is well-structured, the empty string for the encounter field might need verification:
encounter: "", // This will be set when submitting the formConsider making it optional in the type definition if it's meant to be populated later, rather than using an empty string.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Find DiagnosisRequest type definition
ast-grep --pattern 'type DiagnosisRequest = {$$$}'
ast-grep --pattern 'interface DiagnosisRequest {$$$}'
# Find usages of convertToDiagnosisRequest
rg "convertToDiagnosisRequest" -A 5
# Look for encounter field assignments
rg "encounter:" -A 2 -B 2
Length of output: 19776
…structured_prefills
bodhish
requested review from
rithviknishad and
khavinshankar
and removed request for
rithviknishad
…structured_prefills
…are_fe into questionnaire-update-rebuild
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (3)
src/pages/Encounters/EncounterShow.tsx (3)
Line range hint
179-285: Enhance error handling for query failures.The component only handles loading state and missing data. Consider adding comprehensive error handling for query failures to improve user experience.
- if (isLoading || !encounterData) { + const { data: encounterData, isLoading, error } = useQuery({...}); + + if (error) { + return <ErrorPage error={error} />; + } + + if (isLoading) { return <Loading />; + } + + if (!encounterData) { + return <ErrorPage error={new Error("No encounter data found")} />; }
Line range hint
179-285: Improve accessibility of tab navigation.The tab navigation should follow WAI-ARIA best practices for tabs. Consider implementing proper ARIA attributes and keyboard navigation.
<nav - className="flex space-x-6 overflow-x-auto pb-2 pl-2" + className="flex space-x-6 overflow-x-auto pb-2 pl-2" + role="tablist" id="encounter_tab_nav" > {keysOf(tabs).map((tab) => ( <Link key={tab} className={tabButtonClasses(props.tab === tab)} href={`/facility/${facilityId}/encounter/${encounterData.id}/${tab}`} + role="tab" + aria-selected={props.tab === tab} + aria-controls={`${tab}-panel`} > {t(`ENCOUNTER_TAB__${tab}`)} </Link> ))} </nav>
Pre-fill functionality issue confirmed.
The questionnaire form implementation uses local state without persistence, which explains the pre-fill issues when closing and reopening forms. The form state is reset when the component unmounts, and there's no mechanism to cache or restore previous responses. This matches the reported behavior in the PR comments.
Key findings:
- Form state is managed locally using
useState- No persistence mechanism between form sessions
- Form data is cleared on component unmount
🔗 Analysis chain
Line range hint
179-285: Verify pre-fill functionality across component lifecycle.Based on PR comments, there are issues with pre-fill functionality when closing and reopening questionnaire forms. While this component doesn't directly handle the pre-fill logic, it's important to verify the data flow to child components.
Let's analyze the data flow to questionnaire components:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for questionnaire form components and their data handling rg -A 5 "questionnaire.*form" src/Length of output: 16229
Script:
#!/bin/bash # Search for questionnaire form initialization and caching rg -A 10 "const \[questionnaireForms" src/ rg -A 10 "useEffect.*questionnaireForms" src/Length of output: 1070
Script:
#!/bin/bash # Search for questionnaire data loading and initialization rg -B 5 -A 15 "useEffect.*questionnaireData" src/ rg -B 5 -A 15 "const.*questionnaireData.*=" src/Length of output: 102
Script:
#!/bin/bash # Search for questionnaire API calls and hooks rg -A 10 "useQuery.*questionnaire" src/ rg -A 10 "api/v1/questionnaire" src/Length of output: 7365
🧹 Nitpick comments (3)
src/components/Patient/allergy/list.tsx (2)
114-120: Consider enhancing the sorting logic for better maintainability.The current sorting implementation could be improved for better readability and maintainability.
Consider this more explicit approach:
- .sort((a, _b) => { - if (a.clinical_status === "inactive") { - return 1; - } - return -1; - }) + .sort((a, b) => { + const statusOrder = { + active: 0, + inactive: 1, + }; + return (statusOrder[a.clinical_status?.toLowerCase() as keyof typeof statusOrder] ?? -1) - + (statusOrder[b.clinical_status?.toLowerCase() as keyof typeof statusOrder] ?? -1); + })This approach:
- Explicitly defines the order of different statuses
- Handles undefined/unknown statuses gracefully
- Makes it easier to add new status types in the future
169-196: Enhance accessibility for the edit link.While the layout implementation is clean, consider improving accessibility for better user experience.
Add ARIA attributes and keyboard interaction improvements:
<Link href={`/facility/${facilityId}/patient/${patientId}/encounter/${encounterId}/questionnaire/allergy_intolerance`} - className="flex items-center gap-1 text-sm hover:text-gray-500" + className="flex items-center gap-1 text-sm hover:text-gray-500 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:ring-primary" + role="button" + aria-label={t("edit_allergies")} > <PencilIcon size={12} /> {t("edit")} </Link>Also, consider adding a new translation key for the edit button's aria-label.
src/pages/Encounters/EncounterShow.tsx (1)
Line range hint
179-285: Clean up commented code.There are significant blocks of commented code that should be removed if they're no longer needed. This includes old data fetching logic, state management, and UI elements.
Let's verify if these code blocks are referenced elsewhere:
#!/bin/bash # Search for references to the commented functions and variables rg -l "showDoctors|patientData|activeShiftingData|getPatientGender|getPatientAddress|getPatientComorbidities|fetchData" src/
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
public/locale/en.json(12 hunks)src/components/Patient/allergy/list.tsx(5 hunks)src/pages/Encounters/EncounterShow.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- public/locale/en.json
🔇 Additional comments (4)
src/components/Patient/allergy/list.tsx (3)
3-5: LGTM! Clean imports and proper type definition.The new imports are well-organized and the optional
facilityIdprop is correctly typed.Also applies to: 27-27
32-36: LGTM! Consistent state handling with proper layout integration.The component properly handles all states (loading, empty, with data) while maintaining consistency in the layout structure.
Also applies to: 47-55, 61-69
184-192: Verify consistency with other structured field components.The edit link implementation looks good, but let's verify it follows the same pattern as other structured field components mentioned in the PR.
✅ Verification successful
Implementation matches other structured field components ✓
The edit link implementation follows the same pattern found in other structured field components (symptoms, diagnosis), using consistent icon size and translation keys.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for similar edit link implementations in other structured field components rg -U 'Link.*questionnaire.*\{t\("edit"\)\}' src/components/Patient/Length of output: 70
Script:
#!/bin/bash # Search for Link components in questionnaire-related paths echo "=== Searching for questionnaire-related Links ===" rg -U "Link.*questionnaire.*" src/components/Patient/ # Search for PencilIcon usage patterns echo -e "\n=== Searching for PencilIcon usage patterns ===" rg -U "PencilIcon.*size.*" src/components/Patient/ # Search for edit-related text in components echo -e "\n=== Searching for edit-related patterns ===" rg -U 't\("edit"\)' src/components/Patient/Length of output: 998
src/pages/Encounters/EncounterShow.tsx (1)
Line range hint
179-285: Consider documenting the architectural change from Context to Props.The removal of
EncounterProviderin favor of direct prop passing represents a significant architectural change. While this can simplify the data flow, it's important to ensure all consumers are updated accordingly.Let's verify if there are any remaining consumers of the removed context:
✅ Verification successful
The architectural change appears to be safe to proceed with
The verification process shows no remaining dependencies on the EncounterProvider context in the codebase. The change to direct prop passing aligns with the component hierarchy and maintains a clean data flow through the encounter-related components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for potential consumers of the removed EncounterProvider context rg -l "useEncounterContext|EncounterContext" src/Length of output: 51
Script:
#!/bin/bash # Broader search for encounter-related context/provider patterns echo "=== Searching for Encounter-related context patterns ===" rg -l "Encounter.*(?:Context|Provider)" -t tsx -t ts src/ echo -e "\n=== Looking for recently removed encounter context files ===" git log -p --full-history --all --glob="**/Encounter*Context*.ts*" -- src/ echo -e "\n=== Finding all encounter-related components ===" fd -e tsx -e ts "Encounter" src/Length of output: 1188
amjithtitus09
added
tested
needs review
and removed
test failed
reviewed
There was a problem hiding this comment.
Actionable comments posted: 13
🔭 Outside diff range comments (5)
src/components/Questionnaire/QuestionTypes/AppointmentQuestion.tsx (1)
Line range hint
73-93: Enhance error handling for API calls.The queries for resources and slots lack proper error handling and loading states. Given this is a medical appointment system, robust error handling is crucial.
Consider adding error boundaries and user-friendly error messages:
const resourcesQuery = useQuery({ queryKey: ["availableResources", facilityId], queryFn: query(scheduleApis.appointments.availableUsers, { pathParams: { facility_id: facilityId }, }), + onError: (error) => { + // Handle error appropriately + console.error('Failed to fetch resources:', error); + } });src/components/Patient/MedicationStatementList.tsx (1)
Edit functionality is missing from the implementation
The codebase contains display components and type definitions for medications, but there's no implementation of:
- Edit forms or modals
- Mutation endpoints for updating medication statements
- Update handlers or edit-related components
This confirms that the edit support for structured fields mentioned in the PR objectives is not yet implemented.
🔗 Analysis chain
Line range hint
1-202: Verify edit functionality implementation.While this component handles the display of medication data well, I don't see the edit functionality mentioned in the PR objectives. Please ensure that the edit support for structured fields is implemented in the relevant components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for edit-related components or handlers for medication statements rg -g '*.tsx' -g '*.ts' -l 'medication.*edit|edit.*medication' src/Length of output: 69
Script:
#!/bin/bash # Search for medication-related forms, modals, or mutation endpoints echo "=== Searching for medication forms and modals ===" rg -g '*.tsx' -g '*.ts' "MedicationStatement.*Form|MedicationStatement.*Modal" src/ echo -e "\n=== Searching for medication-related mutations ===" rg -g '*.ts' "mutation.*medication|updateMedication|createMedication" src/Utils/ echo -e "\n=== Checking component files with form/update keywords ===" fd -e tsx -e ts "(?i)medication.*?(form|update|edit)" src/components/Length of output: 599
Script:
#!/bin/bash # Check for any recent medication-related file changes echo "=== Recent medication-related files ===" fd -e tsx -e ts "medication" src/ echo -e "\n=== Checking for any mutation-related code ===" rg -g '*.ts' -g '*.tsx' "useMutation.*medication" src/Length of output: 762
src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx (1)
Line range hint
92-102: Improve pre-fill implementation to address potential issues.The current pre-fill implementation has several concerns:
- Missing dependencies in useEffect
- Potential race condition between encounter data and form state updates
- No cleanup on unmount
Consider this improved implementation:
useEffect(() => { if (encounterData) { - handleUpdateEncounter(encounterData as unknown as EncounterEditRequest); + // Only update if not already set from form state + setEncounter((current) => { + if (current.external_identifier) return current; + return encounterData as EncounterEditRequest; + }); } - }, [encounterData]); + }, [encounterData, organizations, patientId]); useEffect(() => { + let mounted = true; const formStateValue = (questionnaireResponse.values[0]?.value as any)?.[0]; if (formStateValue) { + if (!mounted) return; setEncounter(() => ({ ...formStateValue, })); } + return () => { + mounted = false; + }; }, [questionnaireResponse]);src/components/Patient/allergy/list.tsx (1)
Line range hint
39-45: Add error handling and loading state feedback for the query.The query implementation should handle errors and provide better loading state feedback.
- const { data: allergies, isLoading } = useQuery({ + const { data: allergies, isLoading, error } = useQuery({ queryKey: ["allergies", patientId, encounterId], queryFn: query(allergyIntoleranceApi.getAllergy, { pathParams: { patientId }, queryParams: encounterId ? { encounter: encounterId } : undefined, }), }); + if (error) { + console.error('Error fetching allergies:', error); + return ( + <AllergyListLayout + facilityId={facilityId} + patientId={patientId} + encounterId={encounterId} + > + <CardContent className="px-2 pb-2"> + <p className="text-destructive">{t("error_loading_allergies")}</p> + </CardContent> + </AllergyListLayout> + ); + }src/components/Questionnaire/QuestionTypes/DiagnosisQuestion.tsx (1)
Line range hint
284-293: Add date validation for onset datetime.Prevent selection of future dates for diagnosis onset.
<Input type="date" value={diagnosis.onset?.onset_datetime || ""} + max={new Date().toISOString().split('T')[0]} onChange={(e) => { + const selectedDate = new Date(e.target.value); + const today = new Date(); + if (selectedDate > today) { + alert(t("future_date_not_allowed")); + return; + } onUpdate?.({ onset: { onset_datetime: e.target.value }, }) }} disabled={disabled || !!diagnosis.id} className="h-8 md:h-9" />
♻️ Duplicate comments (1)
src/components/Questionnaire/QuestionTypes/MedicationStatementQuestion.tsx (1)
107-124:⚠️ Potential issueAdd error handling and fix useEffect dependencies.
The API integration needs error handling and the useEffect has missing dependencies.
Apply these changes:
const { data: patientMedications } = useQuery({ queryKey: ["medication_statements", patientId], queryFn: query(medicationStatementApi.list, { pathParams: { patientId }, queryParams: { limit: 100, }, }), + onError: (error) => { + toast.error(t("error_fetching_medications")); + }, }); useEffect(() => { if (patientMedications?.results) { updateQuestionnaireResponseCB( [{ type: "medication_statement", value: patientMedications.results }], questionnaireResponse.question_id, ); } -}, [patientMedications]); +}, [patientMedications, questionnaireResponse.question_id, updateQuestionnaireResponseCB]);
🧹 Nitpick comments (33)
src/components/Common/ComboboxQuantityInput.tsx (2)
106-106: LGTM! Complete disabled state implementation.The Popover's open state now correctly respects the disabled prop, ensuring the dropdown doesn't appear when the component is disabled.
However, consider adding error handling for invalid numeric inputs:
const handleInputChange = (e: React.ChangeEvent<HTMLInputElement>) => { if (disabled) return; const value = e.target.value; - if (value === "" || /^\d+$/.test(value)) { + if (value === "" || (/^\d+$/.test(value) && parseInt(value, 10) > 0)) { setInputValue(value); setOpen(true); setActiveIndex(0); if (value && selectedUnit) { onChange({ value: parseInt(value, 10), unit: selectedUnit, }); } + } else if (value !== "") { + // Optionally show an error message for invalid input } };
Line range hint
1-170: Consider adding accessibility improvements.While the disabled state handling is well implemented, there are opportunities to enhance accessibility:
- Add aria-label for the input field to describe its purpose
- Add aria-expanded attribute to indicate dropdown state
- Add aria-invalid attribute when input validation fails
<Input ref={inputRef} type="text" inputMode="numeric" pattern="\d*" value={inputValue} onChange={handleInputChange} onKeyDown={handleKeyDown} placeholder={placeholder} className={cn("w-full text-sm", selectedUnit && "pr-16")} disabled={disabled} autoFocus={autoFocus} + aria-label="Dosage quantity" + aria-expanded={open} + aria-invalid={inputValue !== "" && !/^\d+$/.test(inputValue)} />src/components/ui/date-time-picker.tsx (2)
Line range hint
91-147: Improve accessibility for time selection buttons.The time selection buttons lack proper accessibility attributes. Add aria-labels to help screen readers announce the time values correctly.
Example improvement for the hours section:
<Button key={hour} size="icon" variant={ value && value.getHours() % 12 === hour % 12 ? "default" : "ghost" } className="sm:w-full shrink-0 aspect-square" + aria-label={`Select ${hour} hour`} onClick={() => handleTimeChange("hour", hour.toString())} > {hour} </Button>Similar improvements should be made for minutes and AM/PM buttons.
Line range hint
91-147: Consider adding keyboard navigation for time selection.The time selection interface could benefit from keyboard navigation support, especially for accessibility. Consider implementing arrow key navigation between time options.
Would you like me to provide an example implementation of keyboard navigation for the time selection interface?
src/components/Questionnaire/QuestionTypes/MedicationStatementQuestion.tsx (1)
549-551: Consider memoizing the note update handler.The note update handler is recreated on every render, which could impact performance.
+const handleUpdateNote = React.useCallback( + (note: string) => { + onUpdate?.({ note }); + }, + [onUpdate] +); -handleUpdateNote={(note) => { - onUpdate?.({ note: note }); -}} +handleUpdateNote={handleUpdateNote}src/components/Questionnaire/QuestionTypes/NotesInput.tsx (2)
17-17: Great API simplification!The new
handleUpdateNotecallback provides a more focused and cleaner API compared to the previousupdateQuestionnaireResponseCB. This change follows the single responsibility principle by handling only note updates.This pattern of splitting complex callbacks into more focused ones improves maintainability and makes the component's contract clearer. Consider applying similar refactoring to other questionnaire components if they have overlapping concerns.
53-53: Consider adding debouncing and validation.While the event handler implementation is correct, consider these improvements:
- Add debouncing to prevent excessive updates during rapid typing
- Implement max length validation to prevent oversized notes
Example implementation:
+import { useMemo } from "react"; +import debounce from "lodash/debounce"; + +const MAX_NOTE_LENGTH = 1000; // adjust as needed export function NotesInput({ questionnaireResponse, handleUpdateNote, disabled, className, }: NotesInputProps) { const [open, setOpen] = useState(false); const notes = questionnaireResponse.note || ""; const hasNotes = notes.length > 0; + + const debouncedHandleUpdate = useMemo( + () => debounce((value: string) => handleUpdateNote(value), 300), + [handleUpdateNote] + ); + + const handleChange = (e: React.ChangeEvent<HTMLTextAreaElement>) => { + const value = e.target.value; + if (value.length <= MAX_NOTE_LENGTH) { + debouncedHandleUpdate(value); + } + }; // In the Textarea component: - onChange={(e) => handleUpdateNote(e.target.value)} + onChange={handleChange} + maxLength={MAX_NOTE_LENGTH}src/components/Questionnaire/QuestionTypes/SymptomQuestion.tsx (1)
67-86: Add date validation in convertToSymptomRequest.The function should validate the date before formatting to prevent potential runtime errors.
function convertToSymptomRequest(symptom: Symptom): SymptomRequest { + const parseDate = (dateStr: string) => { + const date = new Date(dateStr); + return !isNaN(date.getTime()) + ? format(date, "yyyy-MM-dd") + : ""; + }; return { // ...other fields onset: symptom.onset ? { ...symptom.onset, - onset_datetime: symptom.onset.onset_datetime - ? format(new Date(symptom.onset.onset_datetime), "yyyy-MM-dd") - : "", + onset_datetime: symptom.onset.onset_datetime + ? parseDate(symptom.onset.onset_datetime) + : "", } : undefined, }; }src/components/Questionnaire/QuestionTypes/DateTimeQuestion.tsx (1)
Line range hint
120-127: Consider adding error feedback for invalid time input.The time input could benefit from visual feedback when an invalid time is entered. Consider adding an error state to the Input component:
<Input type="time" className="w-[150px]" + aria-invalid={timeError} + onInvalid={(e) => { + e.preventDefault(); + setTimeError(true); + }} value={formatTime(currentValue)} onChange={handleTimeChange} disabled={disabled || !currentValue} />src/components/Questionnaire/QuestionTypes/QuestionInput.tsx (1)
199-205: Optimize note update implementation.The note update callback is duplicated and could benefit from optimization:
- Extract the callback to avoid code duplication
- Memoize it to prevent unnecessary rerenders
Consider this optimization:
+ const handleUpdateNote = useCallback( + (note: string) => { + updateQuestionnaireResponseCB( + [...questionnaireResponse.values], + questionnaireResponse.question_id, + note, + ); + }, + [questionnaireResponse.values, questionnaireResponse.question_id, updateQuestionnaireResponseCB], + ); // Then use handleUpdateNote in both NotesInput componentsDon't forget to import
useCallbackfrom React:+ import { useCallback } from "react";Also applies to: 229-235
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (4)
142-163: Enhance type safety and maintainability of status handling.Consider using constants for status values and improving type safety.
+const MEDICATION_STATUS = { + ENTERED_IN_ERROR: 'entered_in_error' as const, +} as const; const medication = medications[medicationToDelete]; if (medication.id) { // For existing records, update status to entered_in_error const newMedications = medications.map((med, i) => i === medicationToDelete - ? { ...med, status: "entered_in_error" as const } + ? { ...med, status: MEDICATION_STATUS.ENTERED_IN_ERROR } : med, );
389-389: Document and centralize read-only state management.Consider adding JSDoc comments explaining the read-only behavior and moving the state management to a custom hook.
+/** + * Determines if a medication is read-only based on its ID. + * Existing medications (with IDs) are read-only to maintain data integrity. + */ +const useMedicationEditState = (medication: MedicationRequest) => { + return { + isReadOnly: !!medication.id, + isRemovable: !medication.id || medication.status !== "entered_in_error", + }; +}; -const isReadOnly = !!medication.id; +const { isReadOnly, isRemovable } = useMedicationEditState(medication);
302-327: Enhance tooltip accessibility and consistency.Add consistent tooltip delay and improve accessibility.
-<TooltipProvider> +<TooltipProvider delayDuration={300}> <Tooltip> - <TooltipTrigger asChild> + <TooltipTrigger asChild aria-label={medication.status === "entered_in_error" ? t("medication_already_marked_as_error") : t("remove_medication")}>
504-511: Improve visual feedback and accessibility for status changes.Add transition animation and ARIA attributes for better user experience.
className={cn( "grid grid-cols-1 lg:grid-cols-[280px,180px,170px,160px,300px,180px,250px,180px,160px,200px,180px,48px] border-b hover:bg-gray-50/50", { - "opacity-40 pointer-events-none": + "opacity-40 pointer-events-none transition-opacity duration-300": medication.status === "entered_in_error", }, + // Add ARIA attributes + "role": "row", + "aria-disabled": medication.status === "entered_in_error" )}src/components/Questionnaire/QuestionTypes/QuestionGroup.tsx (1)
Line range hint
83-150: Add comprehensive tests for questionnaire state management.Given the complexity of the questionnaire logic and the reported pre-fill issues, consider adding comprehensive tests that cover:
- State management for nested question groups
- Pre-fill functionality persistence
- Enable/disable logic with the new response format
Would you like me to help create a test suite that covers these scenarios?
src/components/Questionnaire/QuestionTypes/NumberQuestion.tsx (2)
31-40: Add input validation and error handling.While the callback implementation is functionally correct, consider these improvements:
- Add NaN validation after numeric parsing
- Consider adding min/max value validation
- Handle edge cases for invalid numeric input
const handleChange = (value: string) => { const emptyValue = value === ""; const numericValue = question.type === "decimal" ? parseFloat(value) : parseInt(value); + + // Handle invalid numeric input + if (!emptyValue && Number.isNaN(numericValue)) { + return; + } + + // Optional: Add min/max validation + // if (!emptyValue && (numericValue < min || numericValue > max)) { + // return; + // } updateQuestionnaireResponseCB( [ { type: "number", value: emptyValue ? undefined : numericValue, }, ], questionnaireResponse.question_id, questionnaireResponse.note, ); };
Line range hint
44-52: Add defensive array access.While optional chaining is used for the value property, the array access could be made safer:
<Input type="number" - value={questionnaireResponse.values[0]?.value?.toString() || ""} + value={questionnaireResponse.values?.[0]?.value?.toString() ?? ""} onChange={(e) => handleChange(e.target.value)} step={question.type === "decimal" ? "0.01" : "1"} disabled={disabled} />Also consider adding
aria-labelfor better accessibility.src/components/Questionnaire/QuestionTypes/AppointmentQuestion.tsx (2)
61-70: Improve type safety in the response value construction.The current implementation has some type-related concerns:
- The use of
unknownin the type casting suggests a potential type system mismatch- The array wrapper
[appointment]adds unnecessary complexityConsider this safer implementation:
updateQuestionnaireResponseCB( [ { type: "appointment", - value: [appointment] as unknown as ResponseValue["value"], + value: [appointment] satisfies ResponseValue["value"], }, ], questionnaireResponse.question_id, questionnaireResponse.note, );Also, consider adding runtime validation for the note field if it's used in critical operations.
Line range hint
102-185: Improve form control accessibility.The form controls lack proper ARIA attributes and keyboard navigation support.
Consider these accessibility improvements:
- Add
aria-labelto form controls- Ensure proper focus management
- Add keyboard shortcuts for date selection
- Include error announcements for screen readers
Example:
<Textarea placeholder={t("reason_for_visit_placeholder")} value={value.reason_for_visit || ""} onChange={(e) => handleUpdate({ reason_for_visit: e.target.value })} disabled={disabled} + aria-label={t("reason_for_visit")} + aria-required="true" />src/components/Patient/MedicationStatementList.tsx (2)
47-99: Extract repeated Tooltip pattern into a reusable component.The Tooltip pattern is duplicated multiple times. Consider extracting it into a reusable component to improve maintainability and reduce code duplication.
+interface TooltipTextProps { + text: string; + className?: string; +} + +function TooltipText({ text, className }: TooltipTextProps) { + return ( + <Tooltip> + <TooltipTrigger asChild className={className ?? "max-w-60 truncate"}> + <p>{text}</p> + </TooltipTrigger> + <TooltipContent> + <p>{text}</p> + </TooltipContent> + </Tooltip> + ); +} // Usage in MedicationRow: <TableCell className="font-medium"> <TooltipText text={statement.medication.display ?? statement.medication.code} /> </TableCell>
175-185: Remove unnecessary Fragment wrapper.The Fragment wrapper is unnecessary since MedicationRow is the only child component. Also, the key prop is duplicated.
- return ( - <> - <MedicationRow - key={statement.id} - statement={statement} - isEnteredInError={isEnteredInError} - index={index} - /> - </> - ); + return ( + <MedicationRow + key={statement.id} + statement={statement} + isEnteredInError={isEnteredInError} + index={index} + /> + );src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx (3)
128-130: Consider making the limit configurable.The hard-coded limit of 100 allergies might need to be configurable based on different use cases or performance requirements.
246-252: Simplify nested ternary expressions.Consider extracting the style logic to a helper function for better readability:
- ${ - allergy.verification_status === "entered_in_error" - ? "opacity-40 pointer-events-none" - : allergy.clinical_status === "inactive" - ? "opacity-60" - : allergy.clinical_status === "resolved" - ? "line-through" - : "" - } + ${getAllergyRowClassName(allergy)} + function getAllergyRowClassName(allergy: AllergyIntoleranceRequest): string { + if (allergy.verification_status === "entered_in_error") { + return "opacity-40 pointer-events-none"; + } + if (allergy.clinical_status === "inactive") { + return "opacity-60"; + } + if (allergy.clinical_status === "resolved") { + return "line-through"; + } + return ""; + }
543-549: Improve type safety for verification status updates.The type check could be avoided by using better typing:
- if (value in ALLERGY_VERIFICATION_STATUS) { - onUpdate?.({ - verification_status: value as AllergyVerificationStatus, - }); - } + onUpdate?.({ + verification_status: value, + });And update the Select's type:
<Select<AllergyVerificationStatus>src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx (1)
Line range hint
1-338: Consider additional improvements for robustness.While the component is well-structured, consider these improvements:
- Add error boundaries to handle runtime errors gracefully
- Implement input validation for external_identifier and other user inputs
- Add loading states for all async operations, not just the initial load
- Add proper aria-labels for accessibility
src/components/Patient/symptoms/list.tsx (2)
76-87: Simplify the filtering logic.The current implementation uses spread operator and filter twice. This can be simplified for better readability and performance.
Apply this diff to simplify the filtering:
-symptoms={[ - ...filteredSymptoms.filter( - (symptom) => symptom.verification_status !== "entered_in_error", - ), - ...(showEnteredInError - ? filteredSymptoms.filter( - (symptom) => symptom.verification_status === "entered_in_error", - ) - : []), -]} +symptoms={filteredSymptoms.sort((a, b) => { + // Show entered_in_error records last + if (a.verification_status === "entered_in_error" && + b.verification_status !== "entered_in_error") return 1; + if (a.verification_status !== "entered_in_error" && + b.verification_status === "entered_in_error") return -1; + return 0; +})}
105-133: Consider moving SymptomListLayout to a separate file.The layout component is well-implemented but could benefit from:
- Moving to a separate file for better organization
- Using a proper interface for props
- Adding JSDoc documentation
Create a new file
src/components/Patient/symptoms/SymptomListLayout.tsx:import { ReactNode } from "react"; import { t } from "i18next"; import { PencilIcon } from "lucide-react"; import { Link } from "raviger"; import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; interface SymptomListLayoutProps { /** ID of the facility. If provided, shows edit link */ facilityId?: string; /** ID of the patient */ patientId: string; /** Optional encounter ID */ encounterId?: string; /** Content to render inside the card */ children: ReactNode; } export const SymptomListLayout = ({ facilityId, patientId, encounterId, children, }: SymptomListLayoutProps) => { return ( <Card> <CardHeader className="px-4 py-0 pt-4 flex justify-between flex-row"> <CardTitle>{t("symptoms")}</CardTitle> {facilityId && ( <Link href={`/facility/${facilityId}/patient/${patientId}/encounter/${encounterId}/questionnaire/symptom`} className="flex items-center gap-1 text-sm hover:text-gray-500" > <PencilIcon size={12} /> {t("edit")} </Link> )} </CardHeader> <CardContent className="px-2 pb-2">{children}</CardContent> </Card> ); };src/components/Patient/diagnosis/list.tsx (2)
77-89: Simplify the filtering logic.Similar to the symptoms list, the filtering logic can be simplified for better readability and performance.
Apply this diff to simplify the filtering:
-diagnoses={[ - ...filteredDiagnoses.filter( - (diagnosis) => diagnosis.verification_status !== "entered_in_error", - ), - ...(showEnteredInError - ? filteredDiagnoses.filter( - (diagnosis) => - diagnosis.verification_status === "entered_in_error", - ) - : []), -]} +diagnoses={filteredDiagnoses.sort((a, b) => { + // Show entered_in_error records last + if (a.verification_status === "entered_in_error" && + b.verification_status !== "entered_in_error") return 1; + if (a.verification_status !== "entered_in_error" && + b.verification_status === "entered_in_error") return -1; + return 0; +})}
107-135: Consider moving DiagnosisListLayout to a separate file.Similar to the symptoms list, the layout component could benefit from being moved to a separate file with proper typing and documentation.
Create a new file
src/components/Patient/diagnosis/DiagnosisListLayout.tsx:import { ReactNode } from "react"; import { t } from "i18next"; import { PencilIcon } from "lucide-react"; import { Link } from "raviger"; import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; interface DiagnosisListLayoutProps { /** ID of the facility. Required for edit functionality */ facilityId?: string; /** ID of the patient */ patientId: string; /** ID of the encounter. Required for edit functionality */ encounterId?: string; /** Content to render inside the card */ children: ReactNode; } export const DiagnosisListLayout = ({ facilityId, patientId, encounterId, children, }: DiagnosisListLayoutProps) => { return ( <Card> <CardHeader className="px-4 py-0 pt-4 flex justify-between flex-row"> <CardTitle>{t("diagnoses")}</CardTitle> {facilityId && encounterId && ( <Link href={`/facility/${facilityId}/patient/${patientId}/encounter/${encounterId}/questionnaire/diagnosis`} className="flex items-center gap-1 text-sm hover:text-gray-500" > <PencilIcon size={12} /> {t("edit")} </Link> )} </CardHeader> <CardContent className="px-2 pb-2">{children}</CardContent> </Card> ); };src/components/Patient/allergy/list.tsx (3)
61-68: Consider memoizing filtered results.The filtering operations could be memoized using
useMemoto prevent unnecessary recalculations on re-renders.- const filteredAllergies = allergies?.results?.filter( - (allergy) => - showEnteredInError || allergy.verification_status !== "entered_in_error", - ); - - const hasEnteredInErrorRecords = allergies?.results?.some( - (allergy) => allergy.verification_status === "entered_in_error", - ); + const { filteredAllergies, hasEnteredInErrorRecords } = useMemo(() => ({ + filteredAllergies: allergies?.results?.filter( + (allergy) => + showEnteredInError || allergy.verification_status !== "entered_in_error", + ), + hasEnteredInErrorRecords: allergies?.results?.some( + (allergy) => allergy.verification_status === "entered_in_error", + ), + }), [allergies?.results, showEnteredInError]);
108-162: Consider extracting badge styling logic into custom hooks.The
getStatusBadgeStyleandgetCategoryBadgeStylefunctions could be moved to custom hooks for better code organization and reusability.Example implementation:
const useAllergyStyling = () => { const getStatusBadgeStyle = useCallback((status: string | undefined) => { switch (status?.toLowerCase()) { case "active": return "bg-green-100 text-green-800 border-green-200"; case "inactive": return "bg-gray-100 text-gray-800 border-gray-200"; default: return "bg-gray-100 text-gray-800 border-gray-200"; } }, []); const getCategoryBadgeStyle = useCallback((category: string) => { switch (category?.toLowerCase()) { case "food": return "bg-orange-100 text-orange-800 border-orange-200"; case "medication": return "bg-blue-100 text-blue-800 border-blue-200"; case "environment": return "bg-green-100 text-green-800 border-green-200"; default: return "bg-gray-100 text-gray-800 border-gray-200"; } }, []); return { getStatusBadgeStyle, getCategoryBadgeStyle }; };
222-250: Consider adding error boundary protection.The layout component could benefit from error boundary protection to gracefully handle rendering failures.
+import { ErrorBoundary } from '@/components/Common/ErrorBoundary'; const AllergyListLayout = ({ facilityId, patientId, encounterId, children, }: { facilityId?: string; patientId: string; encounterId?: string; children: ReactNode; }) => { return ( + <ErrorBoundary fallback={<Card><CardHeader><CardTitle>{t("allergies")}</CardTitle></CardHeader></Card>}> <Card> <CardHeader className="px-4 py-0 pt-4 flex justify-between flex-row"> <CardTitle>{t("allergies")}</CardTitle> {facilityId && encounterId && ( <Link href={`/facility/${facilityId}/patient/${patientId}/encounter/${encounterId}/questionnaire/allergy_intolerance`} className="flex items-center gap-1 text-sm hover:text-gray-500" > <PencilIcon size={12} /> {t("edit")} </Link> )} </CardHeader> <CardContent className="px-2 pb-2">{children}</CardContent> </Card> + </ErrorBoundary> ); };src/components/Questionnaire/QuestionTypes/DiagnosisQuestion.tsx (1)
235-240: Enhance accessibility for disabled diagnoses.Add ARIA attributes to improve accessibility when diagnoses are marked as entered-in-error.
<div className={cn("group hover:bg-gray-50", { "opacity-40 pointer-events-none": diagnosis.verification_status === "entered_in_error", })} + aria-disabled={diagnosis.verification_status === "entered_in_error"} + role="listitem" + aria-label={diagnosis.verification_status === "entered_in_error" ? t("diagnosis_marked_as_error") : undefined} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
public/locale/en.json(17 hunks)src/components/Common/ComboboxQuantityInput.tsx(3 hunks)src/components/Patient/MedicationStatementList.tsx(5 hunks)src/components/Patient/allergy/list.tsx(4 hunks)src/components/Patient/diagnosis/DiagnosisTable.tsx(1 hunks)src/components/Patient/diagnosis/list.tsx(3 hunks)src/components/Patient/symptoms/SymptomTable.tsx(1 hunks)src/components/Patient/symptoms/list.tsx(3 hunks)src/components/Questionnaire/QuestionRenderer.tsx(4 hunks)src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx(12 hunks)src/components/Questionnaire/QuestionTypes/AppointmentQuestion.tsx(2 hunks)src/components/Questionnaire/QuestionTypes/BooleanQuestion.tsx(2 hunks)src/components/Questionnaire/QuestionTypes/ChoiceQuestion.tsx(2 hunks)src/components/Questionnaire/QuestionTypes/DateTimeQuestion.tsx(3 hunks)src/components/Questionnaire/QuestionTypes/DiagnosisQuestion.tsx(10 hunks)src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx(2 hunks)src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx(25 hunks)src/components/Questionnaire/QuestionTypes/MedicationStatementQuestion.tsx(14 hunks)src/components/Questionnaire/QuestionTypes/NotesInput.tsx(2 hunks)src/components/Questionnaire/QuestionTypes/NumberQuestion.tsx(2 hunks)src/components/Questionnaire/QuestionTypes/QuestionGroup.tsx(2 hunks)src/components/Questionnaire/QuestionTypes/QuestionInput.tsx(4 hunks)src/components/Questionnaire/QuestionTypes/SymptomQuestion.tsx(3 hunks)src/components/Questionnaire/QuestionTypes/TextQuestion.tsx(2 hunks)src/components/Questionnaire/QuestionnaireForm.tsx(5 hunks)src/components/ui/date-time-picker.tsx(1 hunks)src/types/emr/allergyIntolerance/allergyIntolerance.ts(2 hunks)src/types/emr/medicationStatement.ts(1 hunks)src/types/emr/symptom/symptom.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/types/emr/medicationStatement.ts
- src/components/Questionnaire/QuestionnaireForm.tsx
- src/types/emr/symptom/symptom.ts
- public/locale/en.json
🔇 Additional comments (44)
src/components/Common/ComboboxQuantityInput.tsx (2)
53-53: LGTM! Good defensive programming.The addition of the disabled check prevents any input changes when the component is disabled, which is the expected behavior.
69-69: LGTM! Consistent disabled state handling.The disabled check in handleKeyDown aligns with the component's disabled state behavior, preventing keyboard interactions when disabled.
src/components/Questionnaire/QuestionTypes/MedicationStatementQuestion.tsx (3)
145-166: LGTM! Well-structured medication removal logic.The implementation correctly handles both new and existing medications:
- Existing medications (with ID) are marked as "entered_in_error"
- New medications are removed completely
293-318: Verify translation key exists.The tooltip content uses the translation key "medication_already_marked_as_error" which was flagged as missing in previous reviews.
#!/bin/bash # Check if the translation key exists rg "medication_already_marked_as_error" public/locale/
409-409: LGTM! Consistent readonly state implementation.The implementation correctly disables form controls for existing medications using the
isReadOnlyflag.Also applies to: 480-480, 518-518
src/components/Questionnaire/QuestionTypes/NotesInput.tsx (1)
24-24: LGTM!The prop destructuring correctly reflects the interface changes.
src/components/Questionnaire/QuestionTypes/SymptomQuestion.tsx (5)
8-12: LGTM! Well-organized imports and type definitions.The imports are properly structured and the type definitions are comprehensive, providing good type safety.
Also applies to: 34-46
130-130: Inconsistent 'verification_status' values.The
verification_statusvalue uses underscores which is inconsistent with other components.
142-277: LGTM! Well-optimized SymptomRow component.The component is properly memoized with React.memo and all event handlers are optimized with useCallback.
324-343: LGTM! Well-structured symptom handling.The code properly differentiates between existing and new symptoms, applying appropriate actions for each case.
298-310:⚠️ Potential issueAdd missing dependencies to useEffect.
The effect depends on
updateQuestionnaireResponseCBandquestionnaireResponse.question_idbut they're not included in the dependency array.useEffect(() => { // ...effect code - }, [patientSymptoms]); + }, [ + patientSymptoms, + updateQuestionnaireResponseCB, + questionnaireResponse.question_id + ]);Likely invalid or redundant comment.
src/components/Questionnaire/QuestionTypes/BooleanQuestion.tsx (3)
4-7: LGTM! Clean type imports.The addition of
ResponseValuetype import is well-structured and follows the pattern of grouped imports.
13-17: LGTM! Props interface update.The updated callback signature provides more granular control over response updates and aligns with the broader refactoring pattern across questionnaire components.
34-43: LGTM! Clean callback implementation.The implementation correctly:
- Constructs a properly typed ResponseValue array
- Handles boolean conversion explicitly
- Passes through the question ID and note
src/components/Questionnaire/QuestionTypes/ChoiceQuestion.tsx (3)
12-15: LGTM! Consistent type imports.The import structure matches other questionnaire components, maintaining codebase consistency.
22-24: LGTM! Props interface update.The callback signature matches the pattern established in BooleanQuestion.
51-55: LGTM! Proper array handling.The implementation correctly preserves the array structure when updating values, which is crucial for maintaining response order.
src/components/Questionnaire/QuestionRenderer.tsx (2)
6-9: LGTM! Clean type imports.The grouped import structure maintains consistency with other components.
86-86: LGTM! Clean prop passing.Direct pass-through of
onResponseChangesimplifies the component hierarchy.src/components/Questionnaire/QuestionTypes/TextQuestion.tsx (3)
4-7: LGTM! Consistent type imports.The import structure maintains consistency with other questionnaire components.
13-17: LGTM! Props interface update.The callback signature matches the pattern established across other questionnaire components.
31-40: LGTM! Clean value handling.The implementation correctly:
- Constructs a properly typed ResponseValue array
- Preserves the string type for text inputs
- Passes through question ID and note consistently
src/components/Questionnaire/QuestionTypes/DateTimeQuestion.tsx (1)
53-62: LGTM! Callback usage preserves time and handles edge cases.The implementation correctly maintains the time component when changing dates and properly structures the response value.
src/components/Questionnaire/QuestionTypes/QuestionInput.tsx (2)
66-70: LGTM! Clean implementation of value addition.The implementation correctly preserves existing values and note while maintaining immutability.
35-37: LGTM! Verify all consumers of this callback.The new callback signature improves type safety by separating values, questionId, and notes. This is a good change that aligns with single responsibility principle.
Let's verify all consumers of this callback to ensure they've been updated:
✅ Verification successful
✅ All consumers verified and properly updated
The callback signature change has been consistently implemented across all 15 question type components and their parent QuestionRenderer. Each component correctly uses the new signature with proper parameter ordering:
- QuestionRenderer.tsx
- All question type components (AllergyQuestion, DateTimeQuestion, TextQuestion, etc.)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for components using updateQuestionnaireResponseCB ast-grep --pattern 'updateQuestionnaireResponseCB($$$)' -lLength of output: 166
Script:
#!/bin/bash # Search using both ast-grep and ripgrep for comprehensive coverage echo "=== AST-based search ===" ast-grep --lang ts --pattern 'updateQuestionnaireResponseCB($$$)' -l echo -e "\n=== Text-based search ===" rg -l "updateQuestionnaireResponseCB" --type ts --type tsx echo -e "\n=== Usage context ===" rg "updateQuestionnaireResponseCB" --type ts --type tsx -C 2Length of output: 537
Script:
#!/bin/bash # Search for callback usage with corrected parameters echo "=== AST-based search ===" ast-grep --lang typescript --pattern 'updateQuestionnaireResponseCB($$$)' -l echo -e "\n=== Text-based search ===" rg -l "updateQuestionnaireResponseCB" -g "*.{ts,tsx}" echo -e "\n=== Usage context ===" rg "updateQuestionnaireResponseCB" -g "*.{ts,tsx}" -C 2Length of output: 34679
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (2)
102-109: 🛠️ Refactor suggestionFix useEffect dependencies and potential race condition.
The useEffect hook is missing dependencies and could have race conditions with concurrent updates.
useEffect(() => { - if (patientMedications?.results) { + if (patientMedications?.results && !medications.length) { updateQuestionnaireResponseCB( [{ type: "medication_request", value: patientMedications.results }], questionnaireResponse.question_id, ); } -}, [patientMedications]); +}, [patientMedications, medications.length, questionnaireResponse.question_id, updateQuestionnaireResponseCB]);Likely invalid or redundant comment.
92-100: 🛠️ Refactor suggestionAdd error handling and loading state for the query.
The query implementation lacks error handling and loading state management, which could lead to poor user experience when the API call fails or is in progress.
-const { data: patientMedications } = useQuery({ +const { data: patientMedications, isLoading, error } = useQuery({ queryKey: ["medication_requests", patientId], queryFn: query(medicationRequestApi.list, { pathParams: { patientId }, queryParams: { limit: 100, }, }), }); + +if (error) { + return <div className="text-destructive">{t("error_loading_medications")}</div>; +} + +if (isLoading) { + return <div className="animate-pulse">{t("loading_medications")}</div>; +}Likely invalid or redundant comment.
src/components/Questionnaire/QuestionTypes/QuestionGroup.tsx (3)
8-11: LGTM! Clean type imports.The addition of
ResponseValuetype is well-organized and follows TypeScript best practices.
20-24: Verify consistent callback usage across components.The callback signature has been updated to support multiple values and additional context. Let's verify that all consuming components have been updated accordingly.
✅ Verification successful
Callback signature consistently implemented across components
The new callback signature with array values, questionId, and optional note parameters is correctly implemented in all question type components. No inconsistencies found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for components using updateQuestionnaireResponseCB to ensure consistent implementation echo "Searching for components using updateQuestionnaireResponseCB..." rg -t tsx "updateQuestionnaireResponseCB.*=>" ./src/components/Questionnaire -A 5 # Search for potential missed updates in test files echo "Checking test files for potential updates needed..." rg -t tsx "updateQuestionnaireResponseCB" ./src -g "*test*" -A 5Length of output: 454
Script:
#!/bin/bash # Search for all occurrences of updateQuestionnaireResponseCB echo "Searching for updateQuestionnaireResponseCB usage..." rg "updateQuestionnaireResponseCB" ./src -g "*.{ts,tsx}" -B 2 -A 5 # Search specifically for callback implementations echo -e "\nSearching for callback implementations..." rg "updateQuestionnaireResponseCB.*=>" ./src -g "*.{ts,tsx}" -B 2 -A 5 # Search for test files echo -e "\nChecking test files..." rg "updateQuestionnaireResponseCB" ./src -g "*test*.{ts,tsx}" -B 2 -A 5Length of output: 52671
Line range hint
33-82: Verify enable/disable logic with new response format.The
isQuestionEnabledfunction relies heavily on the response values structure. Given the changes to the response callback signature and the reported pre-fill issues, we should verify that the enable/disable logic works correctly with the new response format.Additionally, considering the pre-fill issues mentioned in the PR comments, please ensure that:
- The enable/disable logic correctly handles pre-filled values
- The state is properly maintained when closing and reopening questionnaires
src/components/Questionnaire/QuestionTypes/NumberQuestion.tsx (2)
3-6: LGTM! Clean type imports.The addition of ResponseValue type is well-organized alongside related questionnaire types.
12-16: Verify consistent callback signature across other question types.The improved callback signature with explicit parameters enhances type safety. However, ensure this signature change is consistently applied across all question type components.
✅ Verification successful
Callback signature is consistent across all question types ✅
The updated callback signature in
NumberQuestion.tsxmatches the implementation in all other question type components, maintaining consistency throughout the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other question type components and verify callback consistency ast-grep --pattern 'interface $_Props { $$$ updateQuestionnaireResponseCB: ($$$) => void; $$$ }' src/components/Questionnaire/QuestionTypes/Length of output: 148
Script:
#!/bin/bash # First, find all question type files echo "=== Question Type Components ===" fd . src/components/Questionnaire/QuestionTypes/ -e tsx echo -e "\n=== Callback Signatures ===" # Search for updateQuestionnaireResponseCB declarations rg "updateQuestionnaireResponseCB.*=>.*void" src/components/Questionnaire/QuestionTypes/ -A 1Length of output: 1261
Script:
#!/bin/bash # Search for Props interfaces in all question type components for file in $(fd . src/components/Questionnaire/QuestionTypes/ -e tsx); do echo "=== $file ===" ast-grep --pattern 'interface $_Props { $$$ }' "$file" doneLength of output: 5239
Script:
#!/bin/bash # Search for updateQuestionnaireResponseCB with more context rg "updateQuestionnaireResponseCB" src/components/Questionnaire/QuestionTypes/ -B 2 -A 2Length of output: 32871
src/components/Questionnaire/QuestionTypes/AppointmentQuestion.tsx (1)
36-40: LGTM! Verify all usage sites of this callback.The new callback signature provides better granularity and type safety. This aligns well with the standardization effort across questionnaire components.
Let's verify all usage sites of this component to ensure the breaking change has been properly handled:
✅ Verification successful
✅ Callback signature change is properly handled
The verification shows that
AppointmentQuestionis correctly integrated throughQuestionInput.tsx, which passes the callback directly via props spreading. This pattern is consistent with other question types, confirming that the signature change is properly handled at the component integration level.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for all imports and usages of AppointmentQuestion rg -l "AppointmentQuestion" --type ts --type tsxLength of output: 76
Script:
#!/bin/bash # Search for files containing AppointmentQuestion using file extensions echo "=== Files containing AppointmentQuestion ===" rg -l "AppointmentQuestion" -g "*.{ts,tsx}" echo -e "\n=== Import statements ===" rg "import.*AppointmentQuestion" -g "*.{ts,tsx}" echo -e "\n=== JSX usage patterns ===" rg "<AppointmentQuestion" -g "*.{ts,tsx}"Length of output: 1143
Script:
#!/bin/bash # Examine QuestionInput.tsx to understand callback handling echo "=== QuestionInput.tsx content ===" rg -A 10 "commonProps" src/components/Questionnaire/QuestionTypes/QuestionInput.tsx echo -e "\n=== Props type definition ===" rg -B 5 "commonProps" src/components/Questionnaire/QuestionTypes/QuestionInput.tsxLength of output: 3790
src/components/Patient/MedicationStatementList.tsx (1)
130-137: LGTM! Clean implementation of medication filtering.The filtering logic is well-implemented with good separation of concerns. The
hasEnteredInErrorRecordscheck efficiently determines whether to show the "view all" button.src/types/emr/allergyIntolerance/allergyIntolerance.ts (2)
4-17: Well-structured type definitions aligned with FHIR standards!The use of string literal unions for status types ensures type safety and prevents invalid values. The types cover all necessary states for allergy records.
48-54: Good practice: Centralized display text mapping!The
ALLERGY_VERIFICATION_STATUSconstant provides a single source of truth for displaying verification status values, making it easier to maintain consistent UI text.src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx (2)
135-144: Consider adding error handling.The effect doesn't handle potential errors from the API response.
160-189: Well-implemented audit trail for allergy removals!Good practice to mark existing allergies as "entered_in_error" instead of deletion, while allowing complete removal of new, unsaved entries.
src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx (2)
131-134: Review type casting inhandleUpdateEncounter.The type casting of
encounterRequesttoresponseValue.valueusingunknowncould hide potential type mismatches. Consider using a more type-safe approach.// Instead of: value: [encounterRequest] as unknown as typeof responseValue.value, // Consider defining a proper type guard or validation: type EncounterResponseValue = { type: "encounter"; value: [EncounterEditRequest]; }; // Then validate the type matches before assignment✅ Verification successful
Type casting approach is correct and consistent with codebase patterns
The
unknowntype casting is actually necessary and correct here because:
ResponseValueis a properly defined discriminated union type- The pattern is consistently used across similar components (e.g., AppointmentQuestion)
- The casting ensures type safety by maintaining the relationship between the
typefield and its correspondingvaluetype🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Search for type definitions related to encounter responses ast-grep --pattern 'type $_ = { $$$ value: $_ $$$ }' # Look for similar components and their type handling fd -e tsx -e ts "Question" --exec grep -l "as unknown" # Search for type validation patterns rg "type.*Encounter" -A 5Length of output: 2676
Script:
#!/bin/bash # Find ResponseValue type definition ast-grep --pattern 'type ResponseValue = { $$$ }' # Check AppointmentQuestion implementation cat src/components/Questionnaire/QuestionTypes/AppointmentQuestion.tsx # Look for type validation utilities rg "function.*validate" -A 3 rg "isValid.*Encounter" -A 3Length of output: 9081
43-47: Verify all consumers ofupdateQuestionnaireResponseCBare updated.The signature change from accepting a single
QuestionnaireResponseto multiple parameters is a breaking change. While the change aligns with adding edit support, we should ensure all consumers are updated.✅ Verification successful
All consumers of
updateQuestionnaireResponseCBare properly updatedThe breaking change has been consistently implemented across all question type components. Each component correctly uses the new signature with an array of response values and question ID, while safely omitting the optional note parameter.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for components using updateQuestionnaireResponseCB echo "Searching for components using updateQuestionnaireResponseCB..." rg -l "updateQuestionnaireResponseCB" src/components/Questionnaire # Check the usage pattern echo "\nChecking usage patterns..." rg "updateQuestionnaireResponseCB\(" src/components/QuestionnaireLength of output: 5099
Script:
#!/bin/bash # Check implementation details of updateQuestionnaireResponseCB calls echo "Checking SymptomQuestion implementation..." rg -A 2 "updateQuestionnaireResponseCB\(" src/components/Questionnaire/QuestionTypes/SymptomQuestion.tsx echo "\nChecking DiagnosisQuestion implementation..." rg -A 2 "updateQuestionnaireResponseCB\(" src/components/Questionnaire/QuestionTypes/DiagnosisQuestion.tsx echo "\nChecking MedicationStatementQuestion implementation..." rg -A 2 "updateQuestionnaireResponseCB\(" src/components/Questionnaire/QuestionTypes/MedicationStatementQuestion.tsx echo "\nChecking a simpler component (TextQuestion) for comparison..." rg -A 2 "updateQuestionnaireResponseCB\(" src/components/Questionnaire/QuestionTypes/TextQuestion.tsxLength of output: 2574
Script:
#!/bin/bash # Check remaining question type implementations echo "Checking remaining question types..." rg -A 2 "updateQuestionnaireResponseCB\(" src/components/Questionnaire/QuestionTypes/BooleanQuestion.tsx echo "\nChecking DateTimeQuestion..." rg -A 2 "updateQuestionnaireResponseCB\(" src/components/Questionnaire/QuestionTypes/DateTimeQuestion.tsx echo "\nChecking NumberQuestion..." rg -A 2 "updateQuestionnaireResponseCB\(" src/components/Questionnaire/QuestionTypes/NumberQuestion.tsx echo "\nChecking ChoiceQuestion..." rg -A 2 "updateQuestionnaireResponseCB\(" src/components/Questionnaire/QuestionTypes/ChoiceQuestion.tsxLength of output: 1065
src/components/Patient/diagnosis/DiagnosisTable.tsx (1)
77-86: LGTM! Well-implemented verification status.The implementation of verification status badges with translations and conditional styling based on error state is clean and follows best practices.
src/components/Patient/symptoms/SymptomTable.tsx (1)
51-51: LGTM! Well-structured table with consistent verification status handling.The implementation of the verification column and status badges with translations is clean and consistent with the diagnosis table implementation.
Also applies to: 78-99
src/components/Patient/diagnosis/list.tsx (1)
122-130: LGTM! Well-implemented edit link condition.The edit link's conditional rendering requiring both
facilityIdandencounterIdis a good practice that ensures the link is only shown when all required parameters are available.src/components/Patient/allergy/list.tsx (1)
27-37: LGTM! Well-structured component props and state management.The addition of
facilityIdprop andshowEnteredInErrorstate enhances the component's functionality by providing facility context and managing the visibility of entered-in-error records.
| // const handleResponseChange = ( | ||
| // updatedResponse: ResponseValue[], | ||
| // questionId: string, | ||
| // ) => { | ||
| // onResponseChange(newResponses, questionId); | ||
| // }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove commented code.
The commented-out handleResponseChange function should be removed if it's no longer needed. If it contains important logic that might be needed later, consider documenting it elsewhere.
- // const handleResponseChange = (
- // updatedResponse: ResponseValue[],
- // questionId: string,
- // ) => {
- // onResponseChange(newResponses, questionId);
- // };| updateQuestionnaireResponseCB( | ||
| [ | ||
| ...questionnaireResponse.values, | ||
| { | ||
| type: "dateTime", | ||
| value: date.toISOString(), | ||
| }, | ||
| ], | ||
| }); | ||
| questionnaireResponse.question_id, | ||
| questionnaireResponse.note, | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add time validation and improve error handling.
The current implementation has several areas for improvement:
- Spreading existing values could lead to duplicate entries
- Missing validation for hours (0-23) and minutes (0-59)
- No error handling for invalid time format
Consider this improved implementation:
const handleTimeChange = (event: React.ChangeEvent<HTMLInputElement>) => {
const [hours, minutes] = event.target.value.split(":").map(Number);
- if (isNaN(hours) || isNaN(minutes)) return;
+ if (isNaN(hours) || isNaN(minutes) ||
+ hours < 0 || hours > 23 ||
+ minutes < 0 || minutes > 59) {
+ return;
+ }
const date = currentValue || new Date();
date.setHours(hours);
date.setMinutes(minutes);
updateQuestionnaireResponseCB(
- [
- ...questionnaireResponse.values,
+ [{
+ type: "dateTime",
+ value: date.toISOString(),
+ }],
- {
- type: "dateTime",
- value: date.toISOString(),
- },
- ],
questionnaireResponse.question_id,
questionnaireResponse.note,
);
};Committable suggestion skipped: line range outside the PR's diff.
| import type { | ||
| QuestionnaireResponse, | ||
| ResponseValue, | ||
| } from "@/types/questionnaire/form"; |
There was a problem hiding this comment.
💡 Codebase verification
Confirmed: Spreading values in handleTimeChange creates duplicates
The implementation is inconsistent and problematic because:
- Component only uses
values[0]for the current value handleSelectcorrectly uses a single-element arrayhandleTimeChangeunnecessarily spreads existing values before adding new one
Fix needed in handleTimeChange:
updateQuestionnaireResponseCB(
[
{
type: "dateTime",
value: date.toISOString(),
},
],
questionnaireResponse.question_id,
questionnaireResponse.note,
);🔗 Analysis chain
Type changes improve safety but watch for value duplication.
The new type structure with ResponseValue and explicit questionId improves type safety. However, be cautious about spreading all values in handleTimeChange as it could lead to duplicate entries.
Consider updating handleTimeChange to only use the new value:
- [...questionnaireResponse.values,
+ [
{
type: "dateTime",
value: date.toISOString(),
},Also applies to: 23-27
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Find the complete DateTimeQuestion component implementation
ast-grep --pattern 'const DateTimeQuestion = ({ $$ }) => {
$$$
}'
# Look for similar handleTimeChange implementations
rg "handleTimeChange.*questionnaireResponse\.values" -A 5
# Find how other components handle response values
rg "updateQuestionnaireResponseCB.*values" -A 5
Length of output: 179
Script:
#!/bin/bash
# Get the full component implementation
cat src/components/Questionnaire/QuestionTypes/DateTimeQuestion.tsx
# Look for value spreading patterns in questionnaire components
rg "\[\.\.\..*values" --type ts
# Check how values are managed in the types
rg "type.*ResponseValue" --type ts -A 3
Length of output: 4761
| clinical_status: AllergyClinicalStatus; | ||
| verification_status: AllergyVerificationStatus; | ||
| category: string; | ||
| criticality: string; | ||
| code: Code; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Maintain type consistency across interfaces.
While clinical_status and verification_status use specific types, category and criticality are still using generic string types in AllergyIntoleranceRequest. Consider updating these for consistency:
clinical_status: AllergyClinicalStatus;
verification_status: AllergyVerificationStatus;
- category: string;
- criticality: string;
+ category: AllergyCategory;
+ criticality: AllergyCriticality;Committable suggestion skipped: line range outside the PR's diff.
| allergy.verification_status === "entered_in_error" | ||
| ? "opacity-40 pointer-events-none" | ||
| : allergy.clinical_status === "inactive" | ||
| ? "opacity-60" | ||
| : allergy.clinical_status === "resolved" | ||
| ? "line-through" | ||
| : "" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove duplicated style logic.
The style logic is duplicated between mobile and desktop views. Consider using the same helper function suggested earlier:
- const rowClassName = `group ${
- allergy.verification_status === "entered_in_error"
- ? "opacity-40 pointer-events-none"
- : allergy.clinical_status === "inactive"
- ? "opacity-60"
- : allergy.clinical_status === "resolved"
- ? "line-through"
- : ""
- }`;
+ const rowClassName = `group ${getAllergyRowClassName(allergy)}`;Committable suggestion skipped: line range outside the PR's diff.
| <> | ||
| <TableRow | ||
| key={index} | ||
| className={ | ||
| isEnteredInError ? "opacity-50 bg-gray-50/50" : undefined | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unnecessary fragment and fix key prop.
The fragment (<>...</>) is unnecessary as it wraps a single child. Also, using array index as a key prop can lead to rendering issues when the array items are reordered or modified.
Apply this diff to fix the issues:
-<>
- <TableRow
- key={index}
- className={
- isEnteredInError ? "opacity-50 bg-gray-50/50" : undefined
- }
- >
+<TableRow
+ key={diagnosis.id ?? index}
+ className={
+ isEnteredInError ? "opacity-50 bg-gray-50/50" : undefined
+ }
+>Committable suggestion skipped: line range outside the PR's diff.
| <> | ||
| <TableRow | ||
| key={index} | ||
| className={ | ||
| isEnteredInError ? "opacity-50 bg-gray-50/50" : undefined | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unnecessary fragment and fix key prop.
Similar to DiagnosisTable, the fragment (<>...</>) is unnecessary, and using array index as a key prop should be avoided.
Apply this diff to fix the issues:
-<>
- <TableRow
- key={index}
- className={
- isEnteredInError ? "opacity-50 bg-gray-50/50" : undefined
- }
- >
+<TableRow
+ key={symptom.id ?? index}
+ className={
+ isEnteredInError ? "opacity-50 bg-gray-50/50" : undefined
+ }
+>Committable suggestion skipped: line range outside the PR's diff.
| const handleRemoveDiagnosis = (index: number) => { | ||
| const newDiagnoses = diagnoses.filter((_, i) => i !== index); | ||
| updateQuestionnaireResponseCB({ | ||
| ...questionnaireResponse, | ||
| values: [{ type: "diagnosis", value: newDiagnoses }], | ||
| }); | ||
| const diagnosis = diagnoses[index]; | ||
| if (diagnosis.id) { | ||
| // For existing records, update verification status to entered_in_error | ||
| const newDiagnoses = diagnoses.map((d, i) => | ||
| i === index | ||
| ? { ...d, verification_status: "entered_in_error" as const } | ||
| : d, | ||
| ) as DiagnosisRequest[]; | ||
| updateQuestionnaireResponseCB( | ||
| [ | ||
| { | ||
| type: "diagnosis", | ||
| value: newDiagnoses, | ||
| }, | ||
| ], | ||
| questionnaireResponse.question_id, | ||
| ); | ||
| } else { | ||
| // For new records, remove them completely | ||
| const newDiagnoses = diagnoses.filter((_, i) => i !== index); | ||
| updateQuestionnaireResponseCB( | ||
| [ | ||
| { | ||
| type: "diagnosis", | ||
| value: newDiagnoses, | ||
| }, | ||
| ], | ||
| questionnaireResponse.question_id, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add confirmation dialog for diagnosis removal.
The removal of diagnoses, especially existing ones, should require user confirmation to prevent accidental deletions.
const handleRemoveDiagnosis = (index: number) => {
+ const confirmRemoval = window.confirm(t("confirm_remove_diagnosis"));
+ if (!confirmRemoval) return;
+
const diagnosis = diagnoses[index];
if (diagnosis.id) {
// For existing records, update verification status to entered_in_error
const newDiagnoses = diagnoses.map((d, i) =>📝 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.
| const handleRemoveDiagnosis = (index: number) => { | |
| const newDiagnoses = diagnoses.filter((_, i) => i !== index); | |
| updateQuestionnaireResponseCB({ | |
| ...questionnaireResponse, | |
| values: [{ type: "diagnosis", value: newDiagnoses }], | |
| }); | |
| const diagnosis = diagnoses[index]; | |
| if (diagnosis.id) { | |
| // For existing records, update verification status to entered_in_error | |
| const newDiagnoses = diagnoses.map((d, i) => | |
| i === index | |
| ? { ...d, verification_status: "entered_in_error" as const } | |
| : d, | |
| ) as DiagnosisRequest[]; | |
| updateQuestionnaireResponseCB( | |
| [ | |
| { | |
| type: "diagnosis", | |
| value: newDiagnoses, | |
| }, | |
| ], | |
| questionnaireResponse.question_id, | |
| ); | |
| } else { | |
| // For new records, remove them completely | |
| const newDiagnoses = diagnoses.filter((_, i) => i !== index); | |
| updateQuestionnaireResponseCB( | |
| [ | |
| { | |
| type: "diagnosis", | |
| value: newDiagnoses, | |
| }, | |
| ], | |
| questionnaireResponse.question_id, | |
| ); | |
| } | |
| const handleRemoveDiagnosis = (index: number) => { | |
| const confirmRemoval = window.confirm(t("confirm_remove_diagnosis")); | |
| if (!confirmRemoval) return; | |
| const diagnosis = diagnoses[index]; | |
| if (diagnosis.id) { | |
| // For existing records, update verification status to entered_in_error | |
| const newDiagnoses = diagnoses.map((d, i) => | |
| i === index | |
| ? { ...d, verification_status: "entered_in_error" as const } | |
| : d, | |
| ) as DiagnosisRequest[]; | |
| updateQuestionnaireResponseCB( | |
| [ | |
| { | |
| type: "diagnosis", | |
| value: newDiagnoses, | |
| }, | |
| ], | |
| questionnaireResponse.question_id, | |
| ); | |
| } else { | |
| // For new records, remove them completely | |
| const newDiagnoses = diagnoses.filter((_, i) => i !== index); | |
| updateQuestionnaireResponseCB( | |
| [ | |
| { | |
| type: "diagnosis", | |
| value: newDiagnoses, | |
| }, | |
| ], | |
| questionnaireResponse.question_id, | |
| ); | |
| } |
| // queryClient.invalidateQueries({ queryKey: ["allergies", patientId] }); | ||
| // queryClient.invalidateQueries({ queryKey: ["symptoms", patientId] }); | ||
| // queryClient.invalidateQueries({ queryKey: ["medications", patientId] }); | ||
| // queryClient.invalidateQueries({ queryKey: ["diagnoses", patientId] }); | ||
| // queryClient.invalidateQueries({ | ||
| // queryKey: ["medication_statements", patientId], | ||
| // }); | ||
| // queryClient.invalidateQueries({ | ||
| // queryKey: ["medication_requests", patientId], | ||
| // }); |
There was a problem hiding this comment.
don't we need to invalidate the queries?
…structured_prefills
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/components/Patient/MedicationStatementList.tsx (1)
172-185: Remove unnecessary fragment.The fragment (
<></>) is unnecessary sinceMedicationRowis the only child element.{filteredMedications.map((statement, index) => { const isEnteredInError = statement.status === "entered_in_error"; return ( - <> - <MedicationRow - key={statement.id} - statement={statement} - isEnteredInError={isEnteredInError} - index={index} - /> - </> + <MedicationRow + key={statement.id} + statement={statement} + isEnteredInError={isEnteredInError} + index={index} + /> ); })}src/components/Questionnaire/QuestionnaireForm.tsx (3)
25-28: Consider adding type validation for ResponseValue.The imported
ResponseValuetype is crucial for type safety. Consider adding runtime type validation using a schema validation library like Zod to ensure the response values match the expected structure at runtime.
95-103: Optimize query invalidation strategy.While the query invalidation is comprehensive, consider using a more efficient approach by grouping related queries under a common prefix:
- queryClient.invalidateQueries({ queryKey: ["allergies", patientId] }); - queryClient.invalidateQueries({ queryKey: ["symptoms", patientId] }); - queryClient.invalidateQueries({ queryKey: ["diagnoses", patientId] }); - queryClient.invalidateQueries({ - queryKey: ["medication_statements", patientId], - }); - queryClient.invalidateQueries({ - queryKey: ["medication_requests", patientId], - }); + queryClient.invalidateQueries({ + predicate: (query) => + query.queryKey[0] === "allergies" || + query.queryKey[0] === "symptoms" || + query.queryKey[0] === "diagnoses" || + query.queryKey[0] === "medication_statements" || + query.queryKey[0] === "medication_requests" && + query.queryKey[1] === patientId + });
118-118: Document the navigation prompt behavior.The navigation prompt is now disabled in development mode. Consider adding a comment explaining this behavior and the rationale behind it for better maintainability.
- useNavigationPrompt(isDirty && !import.meta.env.DEV, t("unsaved_changes")); + // Disable navigation prompt in development mode to avoid interrupting the development workflow + useNavigationPrompt(isDirty && !import.meta.env.DEV, t("unsaved_changes"));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/Medicine/MedicationRequestTable/index.tsx(4 hunks)src/components/Patient/MedicationStatementList.tsx(5 hunks)src/components/Questionnaire/QuestionnaireForm.tsx(6 hunks)src/pages/Encounters/PrintPrescription.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/Encounters/PrintPrescription.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (7)
src/components/Medicine/MedicationRequestTable/index.tsx (3)
1-2: LGTM! Props and imports are well-structured.The imports are properly organized, and the Props interface correctly defines the required fields with appropriate types.
Also applies to: 17-19, 21-26
28-32: Add error handling for the API call.The query setup is missing error handling for failed API calls.
Also applies to: 35-42
86-93: LGTM! Edit button implementation is clean and well-integrated.The edit button is properly implemented with clear visual feedback and correct routing parameters.
src/components/Patient/MedicationStatementList.tsx (4)
31-35: Define proper TypeScript interface for statement prop.The
statementprop is currently typed asany, which defeats TypeScript's type safety.
37-102: LGTM! MedicationRow component is well-structured.The component extraction improves code organization and reusability. The implementation includes proper tooltips for truncated content and consistent styling.
108-108: LGTM! State management and filtering logic are well-implemented.The filtering logic for entered_in_error records is clean and efficient, with proper state management.
Also applies to: 130-137
188-199: LGTM! "View All" button implementation is clean.The conditional rendering and styling of the button are well-implemented, with proper internationalization support.
| const { data: medications, isLoading: loading } = useQuery({ | ||
| queryKey: ["medication_requests", patientId], | ||
| queryFn: query(medicationRequestApi.list, { | ||
| pathParams: { patientId: patientId }, | ||
| queryParams: { encounter: encounterId }, | ||
| }), | ||
| enabled: !!patientId, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Include encounterId in the query key for proper caching.
The query key should include all variables that affect the query result. Since the query depends on both patientId and encounterId, both should be included in the query key.
const { data: medications, isLoading: loading } = useQuery({
- queryKey: ["medication_requests", patientId],
+ queryKey: ["medication_requests", patientId, encounterId],
queryFn: query(medicationRequestApi.list, {
pathParams: { patientId: patientId },
queryParams: { encounter: encounterId },
}),
enabled: !!patientId,
});📝 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.
| const { data: medications, isLoading: loading } = useQuery({ | |
| queryKey: ["medication_requests", patientId], | |
| queryFn: query(medicationRequestApi.list, { | |
| pathParams: { patientId: patientId }, | |
| queryParams: { encounter: encounterId }, | |
| }), | |
| enabled: !!patientId, | |
| }); | |
| const { data: medications, isLoading: loading } = useQuery({ | |
| queryKey: ["medication_requests", patientId, encounterId], | |
| queryFn: query(medicationRequestApi.list, { | |
| pathParams: { patientId: patientId }, | |
| queryParams: { encounter: encounterId }, | |
| }), | |
| enabled: !!patientId, | |
| }); |
| onResponseChange={( | ||
| values: ResponseValue[], | ||
| questionId: string, | ||
| note?: string, | ||
| ) => { | ||
| setQuestionnaireForms((existingForms) => | ||
| existingForms.map((formItem) => | ||
| formItem.questionnaire.id === form.questionnaire.id | ||
| ? { ...formItem, responses, errors: [] } | ||
| ? { | ||
| ...formItem, | ||
| responses: formItem.responses.map((r) => | ||
| r.question_id === questionId | ||
| ? { ...r, values, note: note } | ||
| : r, | ||
| ), | ||
| errors: [], | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Error clearing behavior needs improvement for consistency
The review comment is correct. The codebase shows inconsistent error handling patterns:
- The
onResponseChangehandler clears all errors - Other parts of the code already implement targeted error clearing by filtering on
questionId
Additionally, the form state is managed in-memory without persistence, which explains the pre-fill issues when closing/reopening the form.
Suggested changes:
- Follow the existing pattern to clear only question-specific errors in
onResponseChange - Consider implementing form state persistence if pre-fill functionality is required
🔗 Analysis chain
Verify error clearing behavior and pre-fill functionality.
- The current implementation clears all errors when any response changes. Consider clearing only the errors related to the changed question:
- errors: [],
+ errors: formItem.errors.filter(e => e.question_id !== questionId),- Based on the PR comments about pre-fill issues with symptoms and diagnosis, verify that the response handling properly maintains state when the form is closed and reopened.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for form state persistence logic
echo "Searching for form state persistence..."
rg -A 10 "setQuestionnaireForms|questionnaireForms" src/
Length of output: 14822
|
@gigincg 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
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Release Notes
New Features
Improvements
Refactoring
Chores