-
Notifications
You must be signed in to change notification settings - Fork 515
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Medication Request Enhancements #9962
Conversation
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces comprehensive changes to medication-related functionality across multiple components and types. The modifications primarily focus on enhancing dosage management, localization, and medication request handling. Key updates include a new Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
CARE Run #4314
Run Properties:
|
Project |
CARE
|
Branch Review |
medication_req_enhancements
|
Run status |
Passed #4314
|
Run duration | 01m 54s |
Commit |
18efb6ecca: Medication Request Enhancements
|
Committer | Amjith Titus |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
4
|
View all changes introduced in this branch ↗︎ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/pages/Encounters/PrintPrescription.tsx (3)
168-170
: Internationalize the hard-coded header text "PRESCRIPTION"The header text "PRESCRIPTION" is hard-coded and should be moved to the internationalization (i18n) files to support localization and maintain consistency across the application.
Apply this diff to internationalize the header text:
-<h2 className="text-center text-xl font-semibold text-[#046C4E] mb-4"> - PRESCRIPTION -</h2> +<h2 className="text-center text-xl font-semibold text-[#046C4E] mb-4"> + {t("prescription")} +</h2>
Line range hint
263-274
: Internationalize hard-coded footer textThe footer contains hard-coded text which should be moved to the internationalization files to ensure the application supports multiple languages.
Apply this diff to internationalize the footer text:
-<p className="text-xs text-muted-foreground"> - Sign of the Consulting Doctor -</p> ... -<p> - This is a computer generated prescription. It shall be issued to - the patient only after the concerned doctor has verified the - content and authorized the same by affixing signature. -</p> +<p className="text-xs text-muted-foreground"> + {t("sign_of_consulting_doctor")} +</p> ... +<p> + {t("computer_generated_prescription_notice")} +</p>Ensure that the corresponding translation keys
sign_of_consulting_doctor
andcomputer_generated_prescription_notice
are added to the i18n files.
195-230
: Refactor duplicated code for rendering medication rowsThe code for rendering normal medications and PRN medications is largely similar. Consider refactoring this into a shared component or function to reduce duplication and improve maintainability.
You can create a separate component for medication rows:
// Create a MedicationRow component const MedicationRow = ({ medication, index, offset = 0 }) => { const instruction = medication.dosage_instruction[0]; const frequency = getFrequencyDisplay(instruction?.timing); const dosage = formatDosage(instruction); const duration = instruction?.timing?.repeat?.bounds_duration; const remarks = instruction?.as_needed_boolean ? instruction?.as_needed_for?.display || t("as_needed_prn") : formatSig(instruction); return ( <TableRow key={medication.id} className="bg-white"> <TableCell className="text-center border-t"> {index + 1 + offset} </TableCell> <TableCell className="border-t"> <div className="font-medium"> {medication.medication?.display} </div> </TableCell> <TableCell className="border-t">{dosage}</TableCell> <TableCell className="border-t"> {medication.dosage_instruction[0]?.as_needed_boolean ? t("as_needed_prn") : frequency?.meaning} {instruction?.additional_instruction?.[0]?.display && ( <div className="text-sm text-gray-600"> {instruction.additional_instruction[0].display} </div> )} </TableCell> <TableCell className="border-t"> {duration ? `${duration.value} ${duration.unit}` : "-"} </TableCell> <TableCell className="border-t text-gray-600"> {remarks || "-"} </TableCell> </TableRow> ); }; // Then use it for both normal and PRN medications ... <TableBody> {normalMedications?.map((medication, index) => ( <MedicationRow medication={medication} index={index} key={medication.id} /> ))} {prnMedications?.map((medication, index) => ( <MedicationRow medication={medication} index={index} offset={normalMedications?.length || 0} key={medication.id} /> ))} </TableBody> ...Also applies to: 232-259
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (1)
611-617
: Ensure consistent ordering of frequency optionsWhen mapping over
MEDICATION_REQUEST_TIMING_OPTIONS
, the order of options might not be consistent due to the nature ofObject.entries
. Consider defining the options as an array or explicitly specifying the order to maintain a consistent user experience.Apply this modification:
-{Object.entries(MEDICATION_REQUEST_TIMING_OPTIONS).map( - ([key, option]) => ( +{MEDICATION_REQUEST_TIMING_ORDERED_KEYS.map((key) => { + const option = MEDICATION_REQUEST_TIMING_OPTIONS[key]; return ( <SelectItem key={key} value={key}> {option.display} </SelectItem> ); - ), -)} +})}Define
MEDICATION_REQUEST_TIMING_ORDERED_KEYS
to ensure the desired order.src/components/Common/ComboboxQuantityInput.tsx (1)
59-69
: Consider supporting decimal values in quantity inputCurrently, the input accepts only integers due to the regex
/^\d+$/
. If decimal quantities are valid, consider updating the regex to allow decimal numbers, such as/^\d*\.?\d*$/
. Additionally, update the parsing logic to handle decimals.Apply this diff to allow decimal input:
-if (value === "" || /^\d+$/.test(value)) { +if (value === "" || /^\d*\.?\d+$/.test(value)) {And modify the parsing at line 65:
-onChange({ - value: value ? parseInt(value, 10) : undefined, - unit: selectedUnit, -}); +onChange({ + value: value ? parseFloat(value) : undefined, + unit: selectedUnit, +});Ensure that all related regex patterns, input attributes, and parsing functions are updated accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
public/locale/en.json
(4 hunks)src/common/constants.tsx
(2 hunks)src/components/Common/ComboboxQuantityInput.tsx
(1 hunks)src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx
(10 hunks)src/pages/Encounters/PrintPrescription.tsx
(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: OSSAR-Scan
- GitHub Check: cypress-run (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (5)
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (2)
355-456
: Implementation ofDosageDialog
component is well-structuredThe
DosageDialog
component effectively handles the input of dosage ranges, providing a clear and user-friendly interface. The state management and update logic are correctly implemented.
482-567
: Efficient use ofComboboxQuantityInput
in dosage fieldThe integration of
ComboboxQuantityInput
for dosage input enhances usability and consistency. The logic for switching between single value and range input is well-handled.src/common/constants.tsx (2)
3-3
: LGTM: Import statement is appropriate.The import of
MedicationRequest
type is correctly added to support the timing options type definition.
914-1109
: Well-structured medication timing options with FHIR compliance.The implementation provides a comprehensive set of medication timing options following HL7 FHIR standards:
- Uses GTSAbbreviation code system for standardization
- Includes clear display strings for UI presentation
- Provides precise timing specifications
- Ensures type safety with const assertion and Record type
public/locale/en.json (1)
945-945
: LGTM: Localization keys added for dosage management.The new keys properly support medication dosage management features:
- Range specification with start/end doses
- Tapering and titration functionality
- Consistent naming pattern with existing keys
Also applies to: 1936-1936, 1969-1969, 2036-2037
pattern="\d*" | ||
value={inputValue} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update pattern
attribute to allow decimal input
If decimal quantities are permitted, the pattern
attribute should be updated to reflect the new input format.
Apply this diff to update the pattern
attribute:
- pattern="\d*"
+ pattern="\d*\.?\d*"
Note that when using the pattern
attribute, special characters like backslashes may need to be escaped differently in JSX.
📝 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.
pattern="\d*" | |
value={inputValue} | |
pattern="\d*\.?\d*" | |
value={inputValue} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (2)
489-539
: Consider extracting the dosage input component.The dosage input logic could be extracted into a separate component to improve maintainability and reusability.
Consider creating a
DosageInput
component that encapsulates the quantity input and range toggle functionality.
Line range hint
745-752
: Consider improving accessibility for the intent selector.While the capitalization improvements are good, consider adding aria-label for better screen reader support.
<SelectTrigger className="h-9 text-sm capitalize"> <SelectValue className="capitalize" placeholder={t("select_intent")} + aria-label={t("medication_request_intent")} /> </SelectTrigger>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx
(10 hunks)src/types/emr/medicationRequest.ts
(1 hunks)
🔇 Additional comments (4)
src/types/emr/medicationRequest.ts (2)
124-125
: LGTM! Clear and precise documentation.The updated comments provide better clarity on the distinction between 'ordered' and 'calculated' dose types.
127-131
: Consider adding validation for dose fields.While the type structure improvement allows for more flexible dosage specifications, there's a potential issue where both
dose_quantity
anddose_range
could be undefined or defined simultaneously.Consider adding a type constraint or runtime validation to ensure at least one of
dose_quantity
ordose_range
is defined, but not both. Example:dose_and_rate?: { type: "ordered" | "calculated"; } & ( | { dose_quantity: DosageQuantity; dose_range?: never } | { dose_range: DoseRange; dose_quantity?: never } );✅ Verification successful
The suggested type constraint is appropriate and aligns with codebase patterns
The suggestion to use discriminated union types for
dose_and_rate
aligns with:
- Existing type constraint patterns found in the codebase (e.g., in
schedule.ts
,question.ts
)- Current runtime implementation that already treats these fields as mutually exclusive
- Comment documentation stating "One of
dose_quantity
ordose_range
must be present"🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Find files importing or using medicationRequest rg -l "medicationRequest" # Look for usage of dose_quantity and dose_range rg "dose_quantity|dose_range" -A 2 -B 2 # Check for similar validation patterns in types ast-grep --pattern 'type $_ = $_ & ($_ | $_);' # Look for any runtime validation related to these fields rg "validate.*dose" -A 2 -B 2Length of output: 12359
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (2)
360-455
: Well-structured dosage dialog implementation!The dialog implementation provides a good user experience with:
- Clear separation of start and end doses
- Unit synchronization between fields
- Proper validation
- Responsive design considerations
583-589
: LGTM! Improved frequency selection implementation.The switch to
MEDICATION_REQUEST_TIMING_OPTIONS
provides better structure and maintainability.
export const reverseFrequencyOption = ( | ||
option: MedicationRequest["dosage_instruction"][0]["timing"], | ||
) => { | ||
return Object.entries(FREQUENCY_OPTIONS).find( | ||
([, value]) => | ||
value.timing.repeat.frequency === option?.repeat?.frequency && | ||
value.timing.repeat.period_unit === option?.repeat?.period_unit && | ||
value.timing.repeat.period === option?.repeat?.period, | ||
)?.[0] as keyof typeof FREQUENCY_OPTIONS; | ||
return Object.entries(MEDICATION_REQUEST_TIMING_OPTIONS).find( | ||
([key]) => key === option?.code?.code, | ||
)?.[0] as keyof typeof MEDICATION_REQUEST_TIMING_OPTIONS; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider adding error handling to reverseFrequencyOption.
The function might return undefined if no matching option is found.
export const reverseFrequencyOption = (
option: MedicationRequest["dosage_instruction"][0]["timing"],
) => {
- return Object.entries(MEDICATION_REQUEST_TIMING_OPTIONS).find(
+ const found = Object.entries(MEDICATION_REQUEST_TIMING_OPTIONS).find(
([key]) => key === option?.code?.code,
- )?.[0] as keyof typeof MEDICATION_REQUEST_TIMING_OPTIONS;
+ );
+ return found?.[0] as keyof typeof MEDICATION_REQUEST_TIMING_OPTIONS | undefined;
};
📝 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.
export const reverseFrequencyOption = ( | |
option: MedicationRequest["dosage_instruction"][0]["timing"], | |
) => { | |
return Object.entries(FREQUENCY_OPTIONS).find( | |
([, value]) => | |
value.timing.repeat.frequency === option?.repeat?.frequency && | |
value.timing.repeat.period_unit === option?.repeat?.period_unit && | |
value.timing.repeat.period === option?.repeat?.period, | |
)?.[0] as keyof typeof FREQUENCY_OPTIONS; | |
return Object.entries(MEDICATION_REQUEST_TIMING_OPTIONS).find( | |
([key]) => key === option?.code?.code, | |
)?.[0] as keyof typeof MEDICATION_REQUEST_TIMING_OPTIONS; | |
}; | |
export const reverseFrequencyOption = ( | |
option: MedicationRequest["dosage_instruction"][0]["timing"], | |
) => { | |
const found = Object.entries(MEDICATION_REQUEST_TIMING_OPTIONS).find( | |
([key]) => key === option?.code?.code, | |
); | |
return found?.[0] as keyof typeof MEDICATION_REQUEST_TIMING_OPTIONS | undefined; | |
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/components/Questionnaire/data/StructuredFormData.tsx (2)
26-26
: Consider version management strategy.The version "0.0.1" suggests this is an initial implementation. Consider documenting the versioning strategy for questionnaires to track breaking changes and migrations.
23-41
: Add JSDoc documentation for the questionnaire.Consider adding JSDoc documentation to describe the purpose and usage of this questionnaire, including:
- When to use this questionnaire
- Required fields and their formats
- Integration points with other components
src/components/Questionnaire/QuestionTypes/QuestionInput.tsx (1)
180-181
: Add visual regression tests for layout changes.Consider adding visual regression tests to ensure layout consistency is maintained across different question types and screen sizes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Questionnaire/QuestionTypes/QuestionInput.tsx
(1 hunks)src/components/Questionnaire/data/StructuredFormData.tsx
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (1)
src/components/Questionnaire/QuestionTypes/QuestionInput.tsx (1)
180-181
: Verify layout changes across all structured question types.The conditional class name change could affect the layout of existing structured question types. Please ensure this change maintains consistent layouts across all structured types:
- medication_request
- medication_statement
- allergy_intolerance
- symptom
- diagnosis
- appointment
- encounter
✅ Verification successful
Layout changes are correctly implemented and won't affect structured question types
The conditional class name change is intentionally designed to separate layout handling:
- Non-structured questions use the "flex w-full" layout
- Structured questions (medication, allergy, symptom, etc.) manage their layouts independently in their dedicated components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find all structured question type components to verify layout consistency echo "Searching for structured question type components..." rg -l "structured_type.*=.*['\"](\w+)['\"]" src/components/Questionnaire/QuestionTypes/Length of output: 208
Script:
#!/bin/bash # Search for structured_type in type definitions and component usage echo "Searching for structured_type in type definitions..." rg "structured_type" src/components/Questionnaire/QuestionTypes/ -A 2 -B 2 echo -e "\nSearching for specific structured types..." rg -i "(medication_request|medication_statement|allergy_intolerance|symptom|diagnosis|appointment|encounter)" src/components/Questionnaire/QuestionTypes/ -A 2 -B 2 echo -e "\nSearching for QuestionInput usage..." rg "QuestionInput" src/ -lLength of output: 75504
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@amjithtitus09 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! 🙌 |
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Release Notes
New Features
Improvements
Localization