Medication Request redesign #9848
Conversation
…medication-request-figma
…medication-request-figma
WalkthroughThis pull request introduces extensive changes across various components related to medication management and questionnaire handling. The modifications primarily enhance the user interface for medication requests, improve localization support, and refine component layouts. Key updates include the addition of new internationalization keys, restructuring the medication request grid, introducing more flexible rendering for questionnaire components, and updating type definitions for medication timing and duration. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (3)
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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (1)
Line range hint
400-431: Correct the frequency options inFREQUENCY_OPTIONSThere are inaccuracies in the
FREQUENCY_OPTIONSconstants that need correction to ensure accurate medication timing and adherence to standard medical abbreviations. For example, the display names and timing parameters do not align with typical medical interpretations.Apply this diff to correct the
FREQUENCY_OPTIONS:- Q4H: { - display: "4th hourly", - timing: { repeat: { frequency: 4, period: 1, period_unit: "h" } }, - }, + Q4H: { + display: "Every 4 hours", + timing: { repeat: { frequency: 1, period: 4, period_unit: "h" } }, + }, - QID: { - display: "6th hourly", - timing: { repeat: { frequency: 6, period: 1, period_unit: "h" } }, - }, + QID: { + display: "Four times daily", + timing: { repeat: { frequency: 4, period: 1, period_unit: "d" } }, + }, - TID: { - display: "8th hourly", - timing: { repeat: { frequency: 8, period: 1, period_unit: "h" } }, - }, + TID: { + display: "Three times daily", + timing: { repeat: { frequency: 3, period: 1, period_unit: "d" } }, + }, - STAT: { - display: "Imediately", - timing: { repeat: { frequency: 1, period: 1, period_unit: "s" } }, - }, + STAT: { + display: "Immediately", + timing: { repeat: { frequency: 1, period: 0, period_unit: "s" } }, + },Do you want me to help verify and correct the
FREQUENCY_OPTIONSto ensure they match standard medical abbreviations and timing conventions?
🧹 Nitpick comments (1)
src/components/Questionnaire/QuestionRenderer.tsx (1)
70-70: Avoid assignments within expressions for better readabilityThe assignment within the expression in the
refprop may reduce code readability. Consider refactoring it to a function body for clarity.Apply this diff to improve code clarity:
- ref={(el) => (questionRefs.current[question.id] = el)} + ref={(el) => { + questionRefs.current[question.id] = el; + }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
public/locale/en.json(6 hunks)src/components/Medicine/MedicineAdministrationSheet/index.tsx(2 hunks)src/components/Questionnaire/QuestionRenderer.tsx(2 hunks)src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx(3 hunks)src/components/Questionnaire/QuestionnaireForm.tsx(4 hunks)src/components/Questionnaire/ValueSetSelect.tsx(3 hunks)src/types/emr/medicationRequest.ts(3 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Questionnaire/QuestionRenderer.tsx
[error] 72-72: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (15)
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (2)
102-169: Enhanced grid layout improves data organizationThe new grid layout with headers for medication attributes enhances readability and data organization. The use of internationalization with the
t()function is correctly implemented, providing better localization support.
Line range hint
173-423:MedicationRequestGridRowcomponent is well-structuredThe
MedicationRequestGridRowcomponent efficiently manages medication information and user interactions. The responsive design considerations and conditional rendering for different screen sizes are well-handled.src/components/Questionnaire/QuestionRenderer.tsx (2)
60-66: Utility functionisMedicationRequestis well implementedThe
isMedicationRequestfunction correctly identifies medication request questions by checking thetypeandstructured_typeproperties.
73-77: Conditional className application enhances renderingUsing the
cnutility with theisMedicationRequestfunction appropriately adjusts the styling of the question container based on its type. This ensures proper layout and overflow handling for medication requests.src/components/Questionnaire/ValueSetSelect.tsx (3)
36-36: New propwrapTextForSmallScreenenhances responsivenessThe introduction of the optional
wrapTextForSmallScreenprop allows better text wrapping on smaller screens, improving the user experience on mobile devices.
47-47: Default value forwrapTextForSmallScreenensures backward compatibilitySetting the default value of
wrapTextForSmallScreentofalsemaintains existing behavior for components not utilizing this new prop.
74-77: Conditionally applied class names improve UI on different screen sizesThe conditional class names based on
wrapTextForSmallScreeneffectively adjust text wrapping and layout, enhancing readability across various screen sizes.src/types/emr/medicationRequest.ts (3)
16-25: LGTM! Well-structured duration units mapping.The constant provides a clear mapping between duration unit abbreviations and their display labels, with good type safety using
as const.
80-84: LGTM! Well-designed duration interface.The
BoundsDurationinterface is well-structured with:
- Optional fields for flexibility
- Type-safe unit values using
keyof typeof BOUNDS_DURATION_UNITS
95-95: LGTM! Type-safe integration.The
bounds_durationproperty is correctly integrated into theTiminginterface using the newBoundsDurationtype.src/components/Medicine/MedicineAdministrationSheet/index.tsx (2)
20-23: LGTM! Clean import organization.Imports are well-organized, grouping related types together.
330-339: LGTM! Well-implemented duration display.The duration display implementation:
- Uses proper null checks with optional chaining
- Maintains consistent UI styling
- Correctly uses the duration units mapping
src/components/Questionnaire/QuestionnaireForm.tsx (2)
297-302: LGTM! Improved responsive layout.The layout changes enhance responsiveness while maintaining consistent spacing.
431-432: Verify the preview condition change.The preview rendering condition has been inverted from development-only to non-development-only. Please confirm if this change is intentional.
✅ Verification successful
Preview condition change is intentional and follows codebase patterns
The change aligns with other production/development environment checks in the codebase, where features like Sentry integration and specific routes are toggled based on the environment. The preview is now correctly enabled for production use.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for other preview conditions in the codebase rg -A 2 "import\.meta\.env\.DEV" --type ts --type tsxLength of output: 83
Script:
#!/bin/bash # Search for environment checks with proper file extensions rg -g "*.{ts,tsx}" -A 2 "import\.meta\.env" # Also search for any DEV environment checks with different patterns rg -g "*.{ts,tsx}" -A 2 "process\.env\.NODE_ENV.*development"Length of output: 1104
public/locale/en.json (1)
342-342: LGTM! Comprehensive localization additions.New localization keys are well-organized and provide clear labels for the medication request UI:
- Instructions and method labels
- Selection prompts
- PRN-related text
Also applies to: 399-399, 1050-1050, 1057-1057, 1198-1198, 1631-1631, 1639-1639, 1645-1645, 1647-1647, 1652-1652, 1657-1657, 1660-1660, 1662-1662
CARE
|
||||||||||||||||||||||||||||||||||
| Project |
CARE
|
| Branch Review |
medication-request-figma
|
| Run status |
|
| Run duration | 02m 38s |
| Commit |
|
| Committer | Amjith Titus |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
1
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
3
|
| View all changes introduced in this branch ↗︎ | |
Tests for review
cypress/e2e/facility_spec/facility_creation.cy.ts • 1 failed test
| Test | Artifacts | |
|---|---|---|
| Facility Management > Create a new facility using the admin role and verify validation errors |
Test Replay
Screenshots
|
|
…medication-request-figma
…medication-request-figma
Deploying care-fe with
|
| Latest commit: |
31a8f82
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://84891e37.care-fe.pages.dev |
| Branch Preview URL: | https://medication-request-figma.care-fe.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
src/components/Questionnaire/QuestionTypes/QuestionInput.tsx (1)
169-175: Consider accessibility improvements for the remove button layoutWhile the conditional styling improvements look good, consider adding
aria-labelto the remove button for better accessibility. The current layout changes appropriately handle the spacing and alignment.<Button variant="ghost" size="icon" onClick={() => removeValue(index)} className="h-10 w-10" disabled={disabled} + aria-label="Remove answer" >src/components/Questionnaire/QuestionRenderer.tsx (3)
73-75: Consider documenting the width adjustment rationaleWhile the conditional width adjustment using
cnis implemented correctly, it would be helpful to document why medication requests require different width handling.className={cn( + // Medication requests need full width for better form layout isMedicationRequest(question) ? "md:w-auto" : "max-w-4xl", )}
72-72: Refactor ref assignment to avoid expression assignment warningThe static analysis tool correctly flags the ref assignment in expression. Let's improve this by separating the assignment.
-ref={(el) => (questionRefs.current[question.id] = el)} +ref={(el) => { + questionRefs.current[question.id] = el; +}}🧰 Tools
🪛 Biome (1.9.4)
[error] 72-72: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
60-75: Consider extracting medication request specific styling to a dedicated componentThe current changes handle medication request styling through conditional classes. As the medication request redesign progresses, consider extracting this logic into a dedicated
MedicationRequestContainercomponent if more specific styling or behavior is needed. This would improve maintainability and keep theQuestionRenderermore focused.🧰 Tools
🪛 Biome (1.9.4)
[error] 72-72: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
src/components/Common/QuantityInput.tsx (1)
Line range hint
38-51: Consider adding input validation and error states.The numeric input could benefit from additional features:
- Input validation (min/max values)
- Error state handling
- Feedback for invalid inputs
Example enhancement:
interface Props<TUnit extends string> { quantity?: QuantityValue<TUnit> | null; onChange: (quantity: QuantityValue<TUnit>) => void; units: readonly TUnit[]; disabled?: boolean; placeholder?: string; autoFocus?: boolean; + min?: number; + max?: number; + error?: string; }src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (4)
108-108: Consider using CSS Grid'sminmax()for more flexible column widths.The hardcoded grid column widths might cause layout issues with different content lengths or translations. Consider using CSS Grid's
minmax()function for more flexible column sizing.-<div className="hidden lg:grid grid-cols-[280px,180px,170px,100px,300px,230px,180px,250px,180px,160px,48px] bg-gray-50 border-b text-sm font-medium text-gray-500"> +<div className="hidden lg:grid grid-cols-[minmax(200px,280px),minmax(150px,180px),minmax(150px,170px),minmax(80px,100px),minmax(200px,300px),minmax(180px,230px),minmax(150px,180px),minmax(200px,250px),minmax(150px,180px),minmax(130px,160px),48px] bg-gray-50 border-b text-sm font-medium text-gray-500">
241-255: Simplify frequency selection logic.The frequency selection logic could be simplified by extracting the PRN handling into a separate function.
+const handleFrequencyChange = (value: string, dosageInstruction: MedicationRequestDosageInstruction) => { + if (value === "PRN") { + return { + as_needed_boolean: true, + timing: undefined, + }; + } + return { + as_needed_boolean: false, + timing: FREQUENCY_OPTIONS[value as keyof typeof FREQUENCY_OPTIONS].timing, + }; +}; onValueChange={(value) => { - if (value === "PRN") { - handleUpdateDosageInstruction({ - as_needed_boolean: true, - timing: undefined, - }); - } else { - handleUpdateDosageInstruction({ - as_needed_boolean: false, - timing: - FREQUENCY_OPTIONS[value as keyof typeof FREQUENCY_OPTIONS] - .timing, - }); - } + handleUpdateDosageInstruction(handleFrequencyChange(value, dosageInstruction)); }}
Line range hint
469-521: Improve type safety of FREQUENCY_OPTIONS.The FREQUENCY_OPTIONS object could benefit from stricter typing:
- The display strings should be internationalized
- The timing units should be constrained to valid FHIR values
+type TimingUnit = 's' | 'min' | 'h' | 'd' | 'wk' | 'mo' | 'a'; + +interface FrequencyOption { + display: string; + timing: { + repeat: { + frequency: number; + period: number; + period_unit: TimingUnit; + }; + }; +} -const FREQUENCY_OPTIONS = { +const FREQUENCY_OPTIONS: Record<string, FrequencyOption> = { BD: { - display: "Twice daily", + display: t("twice_daily"), timing: { repeat: { frequency: 2, period: 1, period_unit: "d" } }, }, // ... rest of the options } as const;
Line range hint
1-521: Consider splitting the component for better maintainability.The component handles a lot of complex logic and UI rendering. Consider:
- Extracting the grid row into a separate file
- Moving frequency options to a separate configuration file
- Creating a custom hook for medication request logic
This would improve:
- Code organization
- Testability
- Reusability
- Maintenance
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
public/locale/en.json(6 hunks)src/components/Common/QuantityInput.tsx(3 hunks)src/components/Questionnaire/QuestionRenderer.tsx(2 hunks)src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx(3 hunks)src/components/Questionnaire/QuestionTypes/QuestionInput.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- public/locale/en.json
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Questionnaire/QuestionRenderer.tsx
[error] 72-72: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (6)
src/components/Questionnaire/QuestionTypes/QuestionInput.tsx (1)
1-1: LGTM: Clean utility importThe addition of the
cnutility function from@/lib/utilsfollows best practices for conditional class name composition.src/components/Questionnaire/QuestionRenderer.tsx (1)
60-65: LGTM: Clean helper function implementationThe
isMedicationRequesthelper function is well-implemented and improves code readability by encapsulating the type checking logic.src/components/Common/QuantityInput.tsx (3)
21-21: LGTM! Clean interface extension.The addition of the optional
autoFocusprop follows React and TypeScript best practices.
30-30: LGTM! Consistent props destructuring.The
autoFocusprop destructuring maintains consistency with the existing pattern.
49-49: Verify accessibility implications of autoFocus.While the implementation is correct, auto-focusing inputs can impact accessibility and user experience:
- It may disorient screen reader users
- It can be disruptive on mobile devices
- It might skip important content before the input
Consider documenting usage guidelines or adding validation to ensure this prop is used appropriately.
Let's verify where this component is being used with autoFocus:
✅ Verification successful
autoFocus implementation verified - no accessibility concerns found
The autoFocus prop is implemented correctly and currently not explicitly used in any component, defaulting to false. This means there are no immediate accessibility concerns to address.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for QuantityInput usage with autoFocus prop rg -U "QuantityInput.*autoFocus.*" -g "*.tsx" -g "*.jsx"Length of output: 58
Script:
#!/bin/bash # Search for all QuantityInput usages rg "QuantityInput" -g "*.tsx" -g "*.jsx" -A 3 -B 1 # Check the component implementation cat src/components/Common/QuantityInput.tsxLength of output: 3804
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (1)
Line range hint
466-466: Address the TODO comment regarding period_unit verification.The comment indicates uncertainty about the correctness of period_unit values. This should be verified and documented.
✅ Verification successful
Remove TODO comment - period_unit values are verified and correct
The period_unit values in FREQUENCY_OPTIONS are verified to be correct and consistent with the type definition (
"s" | "min" | "h" | "d" | "wk" | "mo" | "a"). The TODO comment can be safely removed.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for period_unit usage across the codebase to verify valid values echo "Searching for period_unit usage patterns..." rg -A 2 "period_unit.*[\"']([dhwmy]|min|s)[\"']" --type ts --type tsx echo "Searching for FHIR timing unit definitions..." rg -A 5 "timing.*unit" --type ts --type tsxLength of output: 375
Script:
#!/bin/bash # Search for period_unit usage and definitions echo "Searching for period_unit patterns..." rg -A 3 'period_unit' echo "Searching for timing unit related constants and types..." rg -A 5 'type.*Timing|interface.*Timing|TIMING|FREQUENCY' echo "Searching for FHIR timing related definitions..." rg -B 2 -A 2 'FHIR.*timing|timing.*FHIR'Length of output: 24990
| <Input | ||
| type="number" | ||
| disabled={ | ||
| disabled || medication.dosage_instruction[0]?.as_needed_boolean | ||
| } | ||
| value={ | ||
| medication.dosage_instruction[0]?.timing?.repeat?.bounds_duration | ||
| ?.value ?? "" | ||
| } | ||
| onChange={(e) => | ||
| handleUpdateDosageInstruction({ | ||
| timing: { | ||
| ...dosageInstruction?.timing, | ||
| repeat: { | ||
| frequency: | ||
| dosageInstruction?.timing?.repeat?.frequency ?? 1, | ||
| period: dosageInstruction?.timing?.repeat?.period ?? 1, | ||
| period_unit: | ||
| dosageInstruction?.timing?.repeat?.period_unit ?? "d", | ||
| bounds_duration: { | ||
| value: e.target.value | ||
| ? parseInt(e.target.value) | ||
| : undefined, | ||
| unit: "d", | ||
| }, | ||
| }, | ||
| }, | ||
| }) | ||
| } | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve days input validation and handling.
The days input needs additional validation and better type handling:
- No minimum value check
- No maximum value constraint
- Potential NaN issues with parseInt
<Input
type="number"
+ min="1"
+ max="365"
disabled={
disabled || medication.dosage_instruction[0]?.as_needed_boolean
}
value={
medication.dosage_instruction[0]?.timing?.repeat?.bounds_duration
?.value ?? ""
}
onChange={(e) => {
+ const value = e.target.value ? Math.max(1, Math.min(365, parseInt(e.target.value))) : undefined;
handleUpdateDosageInstruction({
timing: {
...dosageInstruction?.timing,
repeat: {
frequency:
dosageInstruction?.timing?.repeat?.frequency ?? 1,
period: dosageInstruction?.timing?.repeat?.period ?? 1,
period_unit:
dosageInstruction?.timing?.repeat?.period_unit ?? "d",
bounds_duration: {
- value: e.target.value
- ? parseInt(e.target.value)
- : undefined,
+ value,
unit: "d",
},
},
},
})
}}
/>📝 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.
| <Input | |
| type="number" | |
| disabled={ | |
| disabled || medication.dosage_instruction[0]?.as_needed_boolean | |
| } | |
| value={ | |
| medication.dosage_instruction[0]?.timing?.repeat?.bounds_duration | |
| ?.value ?? "" | |
| } | |
| onChange={(e) => | |
| handleUpdateDosageInstruction({ | |
| timing: { | |
| ...dosageInstruction?.timing, | |
| repeat: { | |
| frequency: | |
| dosageInstruction?.timing?.repeat?.frequency ?? 1, | |
| period: dosageInstruction?.timing?.repeat?.period ?? 1, | |
| period_unit: | |
| dosageInstruction?.timing?.repeat?.period_unit ?? "d", | |
| bounds_duration: { | |
| value: e.target.value | |
| ? parseInt(e.target.value) | |
| : undefined, | |
| unit: "d", | |
| }, | |
| }, | |
| }, | |
| }) | |
| } | |
| /> | |
| <Input | |
| type="number" | |
| min="1" | |
| max="365" | |
| disabled={ | |
| disabled || medication.dosage_instruction[0]?.as_needed_boolean | |
| } | |
| value={ | |
| medication.dosage_instruction[0]?.timing?.repeat?.bounds_duration | |
| ?.value ?? "" | |
| } | |
| onChange={(e) => { | |
| const value = e.target.value ? Math.max(1, Math.min(365, parseInt(e.target.value))) : undefined; | |
| handleUpdateDosageInstruction({ | |
| timing: { | |
| ...dosageInstruction?.timing, | |
| repeat: { | |
| frequency: | |
| dosageInstruction?.timing?.repeat?.frequency ?? 1, | |
| period: dosageInstruction?.timing?.repeat?.period ?? 1, | |
| period_unit: | |
| dosageInstruction?.timing?.repeat?.period_unit ?? "d", | |
| bounds_duration: { | |
| value, | |
| unit: "d", | |
| }, | |
| }, | |
| }, | |
| }) | |
| }} | |
| /> |
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (1)
Line range hint
444-457: Handle custom timing patterns in reverseFrequencyOption.The function might return undefined for custom timing patterns not present in FREQUENCY_OPTIONS. Consider adding a fallback or custom option.
const reverseFrequencyOption = ( option: MedicationRequest["dosage_instruction"][0]["timing"], ) => { - return Object.entries(FREQUENCY_OPTIONS).find( + const found = 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; + )?.[0]; + return found as keyof typeof FREQUENCY_OPTIONS || 'custom'; };
🧹 Nitpick comments (5)
src/components/Common/QuantityInput.tsx (1)
Line range hint
1-74: Consider adding unit tests for the new functionalityWhile the changes look good, consider adding unit tests to verify:
- autoFocus behavior
- unitLabels rendering with both provided and fallback cases
Would you like me to help generate test cases for these scenarios?
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (4)
108-108: Consider using CSS Grid'sauto-fitorminmaxfor responsive column widths.The hardcoded grid template columns might cause layout issues with different content lengths or translations. Consider using more flexible grid options.
-<div className="hidden lg:grid grid-cols-[280px,180px,170px,160px,300px,230px,180px,250px,180px,160px,48px] bg-gray-50 border-b text-sm font-medium text-gray-500"> +<div className="hidden lg:grid" style={{ + gridTemplateColumns: 'minmax(280px, auto) minmax(180px, auto) minmax(170px, auto) minmax(160px, auto) minmax(300px, auto) minmax(230px, auto) minmax(180px, auto) minmax(250px, auto) minmax(180px, auto) minmax(160px, auto) 48px', +}} className="bg-gray-50 border-b text-sm font-medium text-gray-500">
139-139: Consider optimizing shadow performance.The shadow implementation using
shadow-[-12px_0_15px_-4px_rgba(0,0,0,0.15)]on scroll might cause performance issues. Consider usingtransform: translateZ(0)orwill-change: transformto optimize rendering.-<div className="font-semibold text-gray-600 p-3 sticky right-0 bg-gray-50 shadow-[-12px_0_15px_-4px_rgba(0,0,0,0.15)] w-12" /> +<div className="font-semibold text-gray-600 p-3 sticky right-0 bg-gray-50 shadow-[-12px_0_15px_-4px_rgba(0,0,0,0.15)] w-12 transform-gpu" />Also applies to: 430-430
471-482: Clarify the difference between AM, PM, and QD options.The AM, PM, and QD options all use the same timing configuration (frequency: 1, period: 1, period_unit: "d"). Consider:
- Adding specific time-of-day information
- Using a more specific timing structure
AM: { display: "Every morning", - timing: { repeat: { frequency: 1, period: 1, period_unit: "d" } }, + timing: { + repeat: { frequency: 1, period: 1, period_unit: "d" }, + code: { coding: [{ code: "morning" }] } + }, },
102-169: Enhance accessibility with ARIA attributes.Consider adding the following accessibility improvements:
- Add
role="grid"to the main grid container- Add
aria-labelto describe the medication list- Add
aria-sortto sortable columns if applicable-<div className="space-y-4"> +<div className="space-y-4" role="region" aria-label={t("medication_list")}> <div className="md:overflow-x-auto w-auto"> {medications.length > 0 && ( <div className="min-w-fit"> - <div className="max-w-[2144px] relative border rounded-md"> + <div className="max-w-[2144px] relative border rounded-md" role="grid">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/Common/QuantityInput.tsx(4 hunks)src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx(5 hunks)src/types/emr/medicationRequest.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/types/emr/medicationRequest.ts
🔇 Additional comments (5)
src/components/Common/QuantityInput.tsx (4)
21-22: LGTM! Well-typed prop additionsThe new optional props
autoFocusandunitLabelsare properly typed and provide valuable functionality for accessibility and internationalization.
31-32: LGTM! Clean parameter destructuringThe new props are properly destructured in the component parameters.
65-65: LGTM! Safe unit label renderingThe nullish coalescing operator provides a clean fallback to the unit name when no label is provided.
51-51: Verify autoFocus support in the Input componentPlease ensure that the imported Input component from "@/components/ui/input" supports the autoFocus prop.
✅ Verification successful
autoFocus prop is properly supported ✓
The Input component extends native input props and correctly forwards the autoFocus prop to the underlying HTML input element.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if Input component supports autoFocus prop ast-grep --pattern 'interface $InputProps { $$$ autoFocus?: boolean; $$$ }'Length of output: 83
Script:
#!/bin/bash # Find Input component file fd -e tsx -e ts "input.tsx" -i # If found, check its implementation fd -e tsx -e ts "input.tsx" -i -x cat {}Length of output: 26957
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (1)
277-319: Improve days input validation and handling.The days input needs additional validation and better type handling:
- No minimum value check
- No maximum value constraint
- Potential NaN issues with parseInt
| const FREQUENCY_OPTIONS = { | ||
| BD: { | ||
| display: "Twice daily", | ||
| BID: { | ||
| display: "Two times a day", | ||
| timing: { repeat: { frequency: 2, period: 1, period_unit: "d" } }, | ||
| }, | ||
| HS: { | ||
| display: "Night only", | ||
| TID: { | ||
| display: "Three times a day", | ||
| timing: { repeat: { frequency: 3, period: 1, period_unit: "d" } }, | ||
| }, | ||
| QID: { | ||
| display: "Four times a day", | ||
| timing: { repeat: { frequency: 4, period: 1, period_unit: "d" } }, | ||
| }, | ||
| AM: { | ||
| display: "Every morning", | ||
| timing: { repeat: { frequency: 1, period: 1, period_unit: "d" } }, | ||
| }, | ||
| OD: { | ||
| display: "Once daily", | ||
| PM: { | ||
| display: "Every afternoon", | ||
| timing: { repeat: { frequency: 1, period: 1, period_unit: "d" } }, | ||
| }, | ||
| QD: { | ||
| display: "Every day", | ||
| timing: { repeat: { frequency: 1, period: 1, period_unit: "d" } }, | ||
| }, | ||
| QOD: { | ||
| display: "Every other day", | ||
| timing: { repeat: { frequency: 1, period: 2, period_unit: "d" } }, | ||
| }, | ||
| Q1H: { | ||
| display: "Every hour", | ||
| timing: { repeat: { frequency: 24, period: 1, period_unit: "d" } }, | ||
| }, | ||
| Q2H: { | ||
| display: "Every 2 hours", | ||
| timing: { repeat: { frequency: 12, period: 1, period_unit: "d" } }, | ||
| }, | ||
| Q3H: { | ||
| display: "Every 3 hours", | ||
| timing: { repeat: { frequency: 8, period: 1, period_unit: "d" } }, | ||
| }, | ||
| Q4H: { | ||
| display: "4th hourly", | ||
| timing: { repeat: { frequency: 4, period: 1, period_unit: "h" } }, | ||
| display: "Every 4 hours", | ||
| timing: { repeat: { frequency: 6, period: 1, period_unit: "d" } }, | ||
| }, | ||
| QID: { | ||
| display: "6th hourly", | ||
| timing: { repeat: { frequency: 6, period: 1, period_unit: "h" } }, | ||
| Q6H: { | ||
| display: "Every 6 hours", | ||
| timing: { repeat: { frequency: 4, period: 1, period_unit: "d" } }, | ||
| }, | ||
| QOD: { | ||
| display: "Alternate day", | ||
| timing: { repeat: { frequency: 2, period: 1, period_unit: "d" } }, | ||
| Q8H: { | ||
| display: "Every 8 hours", | ||
| timing: { repeat: { frequency: 3, period: 1, period_unit: "d" } }, | ||
| }, | ||
| BED: { | ||
| display: "At bedtime", | ||
| timing: { repeat: { frequency: 1, period: 1, period_unit: "d" } }, | ||
| }, | ||
| QWK: { | ||
| display: "Once a week", | ||
| WK: { | ||
| display: "Weekly", | ||
| timing: { repeat: { frequency: 1, period: 1, period_unit: "wk" } }, | ||
| }, | ||
| STAT: { | ||
| display: "Imediately", | ||
| timing: { repeat: { frequency: 1, period: 1, period_unit: "s" } }, | ||
| MO: { | ||
| display: "Monthly", | ||
| timing: { repeat: { frequency: 1, period: 1, period_unit: "mo" } }, | ||
| }, | ||
| TID: { | ||
| display: "8th hourly", | ||
| timing: { repeat: { frequency: 8, period: 1, period_unit: "h" } }, | ||
| STAT: { | ||
| display: "Immediately", | ||
| timing: { repeat: { frequency: 1, period: 1, period_unit: "s" } }, // One-time | ||
| }, |
There was a problem hiding this comment.
💡 Codebase verification
Safety measures needed for medication frequency handling
The codebase lacks critical safety measures for medication frequency handling:
- No validation constraints exist for frequency values, allowing potentially unsafe values like 24 times per day (Q1H)
- STAT medication timing uses seconds as the period unit, which might not be the most appropriate representation for one-time immediate administration
- No safety checks are implemented for high-frequency medications
🔗 Analysis chain
Verify medication frequency options with medical professionals.
The frequency options and their corresponding timing calculations need verification:
- Some frequencies like Q1H (24 times per day) might need additional safety checks
- The STAT timing using seconds as period_unit might need review
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any existing validation or safety checks for medication frequency
rg -l "frequency|period_unit" --type ts
Length of output: 390
Script:
#!/bin/bash
# Check type definitions and validation logic
echo "=== Medication Request Types ==="
cat src/types/emr/medicationRequest.ts
echo -e "\n=== Validation Logic in MedicationRequestQuestion ==="
rg "validate|check|safe|restrict" src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx -A 5
echo -e "\n=== STAT Timing Implementation ==="
rg "STAT|immediately|one.?time" src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx -A 5
echo -e "\n=== Medicine Administration Safety Checks ==="
rg "validate|check|safe|restrict" src/components/Medicine/MedicineAdministrationSheet/index.tsx -A 5
Length of output: 4563
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx
Show resolved
Hide resolved
|
Reminder: To add the "needs testing" label, comment "ready for testing" on this PR. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/Questionnaire/QuestionRenderer.tsx (2)
81-83: Consider using more semantic class names and improving responsive design.The current implementation could be more explicit:
- "md:w-auto" is not descriptive of its purpose
- Missing mobile-first approach
- className={cn( - shouldBeFullWidth(question) ? "md:w-auto" : "max-w-4xl", - )} + className={cn( + "w-full", + shouldBeFullWidth(question) + ? "md:w-full" + : "md:max-w-4xl", + )}
80-80: Address static analysis warning about assignment in expression.The current ref callback uses assignment in an expression, which can be confusing. Consider making it more explicit.
- ref={(el) => (questionRefs.current[question.id] = el)} + ref={(el) => { + questionRefs.current[question.id] = el; + }}🧰 Tools
🪛 Biome (1.9.4)
[error] 80-80: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Questionnaire/QuestionRenderer.tsx(2 hunks)src/types/emr/medicationRequest.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/types/emr/medicationRequest.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Questionnaire/QuestionRenderer.tsx
[error] 80-80: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (3)
src/components/Questionnaire/QuestionRenderer.tsx (3)
3-10: LGTM! Imports are properly organized.The new imports for
cnutility andStructuredQuestionTypeare correctly added and necessary for the new functionality.
14-17: Consider making the full-width configuration more flexible.Based on the previous review comment, more question types might need full-width rendering in the future. Consider:
- Moving this configuration to a separate config file
- Making it configurable through props or context
-// Questions that should be rendered full width -const FULL_WIDTH_QUESTION_TYPES: StructuredQuestionType[] = [ - "medication_request", -]; +import { FULL_WIDTH_QUESTION_TYPES } from "@/config/questionnaire";
68-73: LGTM! Well-implemented utility function.The
shouldBeFullWidthfunction is well-structured with proper type checking and null safety.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (2)
108-140: Consider extracting grid configuration and improving accessibility.The grid layout could be improved by:
- Extracting the column configuration to avoid repetition
- Adding ARIA attributes for better screen reader support
+ const GRID_COLUMNS = [ + { width: '280px', key: 'medicine' }, + { width: '180px', key: 'dosage' }, + // ... other columns + ]; - <div className="hidden lg:grid grid-cols-[280px,180px,170px,160px,300px,230px,180px,250px,180px,160px,48px] bg-gray-50 border-b text-sm font-medium text-gray-500"> + <div + className="hidden lg:grid bg-gray-50 border-b text-sm font-medium text-gray-500" + style={{ gridTemplateColumns: GRID_COLUMNS.map(col => col.width).join(' ') }} + role="grid" + aria-label="Medication request form"> {GRID_COLUMNS.map(({ key, width }) => ( <div key={key} className="font-semibold text-gray-600 p-3 border-r" role="columnheader" aria-label={t(key)}> {t(key)} </div> ))}
102-169: Well-structured implementation with room for safety improvements.The grid-based redesign improves the UI/UX significantly. However, consider adding the following safety features:
- Implement dosage range validation
- Add drug interaction checks
- Include weight-based dosing support for applicable medications
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Test
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
- GitHub Check: CodeQL-Build
- GitHub Check: OSSAR-Scan
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (2)
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (2)
277-321: Duration input still lacks validation.The duration input needs validation constraints as previously suggested.
461-472: Consider extending frequency display format.As suggested in the previous review comment, extend the display format to include the timing pattern.
| <Select | ||
| value={ | ||
| medication.dosage_instruction[0]?.as_needed_boolean | ||
| ? "PRN" | ||
| : reverseFrequencyOption(dosageInstruction?.timing) | ||
| } | ||
| onValueChange={(value) => { | ||
| if (value === "PRN") { | ||
| handleUpdateDosageInstruction({ | ||
| as_needed_boolean: true, | ||
| timing: undefined, | ||
| }); | ||
| } else { | ||
| handleUpdateDosageInstruction({ | ||
| dose_and_rate: { type: "ordered", dose_quantity: value }, | ||
| }) | ||
| as_needed_boolean: false, | ||
| timing: | ||
| FREQUENCY_OPTIONS[value as keyof typeof FREQUENCY_OPTIONS] | ||
| .timing, | ||
| }); | ||
| } | ||
| disabled={disabled} | ||
| /> | ||
| </div> | ||
| <div className="flex-[2]"> | ||
| <Label className="mb-1 block text-sm font-medium">Route</Label> | ||
| <ValueSetSelect | ||
| system="system-route" | ||
| value={medication.dosage_instruction[0]?.route} | ||
| onSelect={(route) => handleUpdateDosageInstruction({ route })} | ||
| placeholder="Select route" | ||
| disabled={disabled} | ||
| /> | ||
| </div> | ||
| <div className="flex-1"> | ||
| <Label className="mb-1 block text-sm font-medium pr-4"> | ||
| Method | ||
| </Label> | ||
| <ValueSetSelect | ||
| system="system-administration-method" | ||
| value={medication.dosage_instruction[0]?.method} | ||
| onSelect={(method) => handleUpdateDosageInstruction({ method })} | ||
| placeholder="Select method" | ||
| disabled={disabled} | ||
| count={20} | ||
| /> | ||
| </div> | ||
| }} | ||
| disabled={disabled} | ||
| > | ||
| <SelectTrigger className="h-8 text-sm"> | ||
| <SelectValue placeholder={t("select_frequency")} /> | ||
| </SelectTrigger> | ||
| <SelectContent> | ||
| <SelectItem value="PRN">{t("as_needed_prn")}</SelectItem> | ||
| {Object.entries(FREQUENCY_OPTIONS).map(([key, option]) => ( | ||
| <SelectItem key={key} value={key}> | ||
| {option.display} | ||
| </SelectItem> | ||
| ))} | ||
| </SelectContent> | ||
| </Select> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance PRN medication handling.
The PRN (as needed) medication handling needs additional safety measures:
- Consider adding a required reason when PRN is selected
- Add a maximum daily limit field for PRN medications
<Select
value={
medication.dosage_instruction[0]?.as_needed_boolean
? "PRN"
: reverseFrequencyOption(dosageInstruction?.timing)
}
onValueChange={(value) => {
if (value === "PRN") {
handleUpdateDosageInstruction({
as_needed_boolean: true,
timing: undefined,
+ max_dose_per_day: { value: undefined, unit: "1/d" },
});
} else {
handleUpdateDosageInstruction({
as_needed_boolean: false,
+ max_dose_per_day: undefined,
timing:
FREQUENCY_OPTIONS[value as keyof typeof FREQUENCY_OPTIONS]
.timing,
});
}
}}
disabled={disabled}
>Committable suggestion skipped: line range outside the PR's diff.
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx
Outdated
Show resolved
Hide resolved
|
👋 Hi, @amjithtitus09, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
…medication-request-figma
This is the mobile view of the Medication Advice form. When the user adds a new form, any existing entries should automatically collapse. Users can manually expand or collapse entries to edit the medication details. Only one form can remain expanded at a time on the mobile screen. |
Shouldn't we remove the medicines that have already been entered from the search results? |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (1)
102-169: Enhance accessibility of the medication grid.The grid layout is well-structured, but could benefit from improved accessibility.
Consider these improvements:
- Add
aria-labelattributes to the grid container- Add
role="grid"androle="row"to appropriate elements- Add keyboard navigation support for the grid cells
-<div className="space-y-4"> +<div className="space-y-4" role="grid" aria-label={t("medication_request_grid")}> - <div className="md:overflow-x-auto w-auto"> + <div className="md:overflow-x-auto w-auto" role="presentation"> {medications.length > 0 && ( - <div className="min-w-fit"> + <div className="min-w-fit" role="presentation"> - <div className="max-w-[2144px] relative border rounded-md"> + <div className="max-w-[2144px] relative border rounded-md" role="presentation"> {/* Header */} - <div className="hidden lg:grid grid-cols-[280px,180px,170px,160px,300px,230px,180px,250px,180px,160px,48px] bg-gray-50 border-b text-sm font-medium text-gray-500"> + <div className="hidden lg:grid grid-cols-[280px,180px,170px,160px,300px,230px,180px,250px,180px,160px,48px] bg-gray-50 border-b text-sm font-medium text-gray-500" role="row">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
public/locale/en.json(9 hunks)src/components/Common/QuantityInput.tsx(5 hunks)src/components/Medicine/MedicineAdministrationSheet/index.tsx(2 hunks)src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx(5 hunks)src/components/Questionnaire/QuestionTypes/QuestionInput.tsx(2 hunks)src/components/Questionnaire/QuestionnaireForm.tsx(4 hunks)src/types/emr/medicationRequest.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/Medicine/MedicineAdministrationSheet/index.tsx
- src/components/Common/QuantityInput.tsx
- src/components/Questionnaire/QuestionTypes/QuestionInput.tsx
- src/components/Questionnaire/QuestionnaireForm.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (4)
src/types/emr/medicationRequest.ts (1)
93-96: LGTM! Type safety improvements.The changes correctly make
frequencyrequired and integrate the newBoundsDurationtype, improving type safety and alignment with backend requirements.src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (2)
Line range hint
173-428: [Duplicate] Enhance PRN medication handling.The PRN medication handling still needs additional safety measures as previously suggested.
This is a duplicate of a previous review comment that suggested:
- Adding a required reason when PRN is selected
- Adding a maximum daily limit field for PRN medications
445-513: [Duplicate] Critical: Review high-frequency medication safety and STAT timing.The frequency options still need safety measures for high-frequency medications and STAT timing.
This is a duplicate of a previous review comment that suggested:
- Adding validation/warnings for high-frequency medications
- Using a dedicated timing type for STAT medications
public/locale/en.json (1)
343-343: LGTM! Comprehensive internationalization coverage.The new translation keys provide complete coverage for the medication request UI elements.
Also applies to: 402-402, 1074-1074, 1081-1081, 1226-1226, 1681-1681, 1690-1690, 1699-1699, 1701-1701, 1706-1706, 1712-1712, 1715-1715, 1717-1717, 1862-1877
src/types/emr/medicationRequest.ts
Outdated
| export interface BoundsDuration { | ||
| value?: number; | ||
| unit: (typeof BOUNDS_DURATION_UNITS)[number]; | ||
| } |
There was a problem hiding this comment.
Make value required as per backend specification.
According to the backend specification shared in past review comments, the value field should not be nullable.
Apply this change:
export interface BoundsDuration {
- value?: number;
+ value: number;
- unit: (typeof BOUNDS_DURATION_UNITS)[number];
+ unit: keyof typeof BOUNDS_DURATION_UNITS;
}📝 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 interface BoundsDuration { | |
| value?: number; | |
| unit: (typeof BOUNDS_DURATION_UNITS)[number]; | |
| } | |
| export interface BoundsDuration { | |
| value: number; | |
| unit: keyof typeof BOUNDS_DURATION_UNITS; | |
| } |
…medication-request-figma
@vinutv Not enforcing this now as duplicates can occur. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (1)
613-616:⚠️ Potential issueCritical: Review high-frequency medication safety and STAT timing.
Two critical safety concerns need addressing:
- Q1H frequency (24 times per day) needs additional safety measures
- STAT timing using seconds as period_unit needs review
Consider:
- Adding validation/warnings for high-frequency medications
- Using a dedicated timing type for STAT medications
Q1H: { display: "Every hour", - timing: { repeat: { frequency: 24, period: 1, period_unit: "d" } }, + timing: { + repeat: { frequency: 24, period: 1, period_unit: "d" }, + maxDosePerDay: 24, + requiresPharmacistReview: true, + }, }, STAT: { display: "Immediately", - timing: { repeat: { frequency: 1, period: 1, period_unit: "s" } }, // One-time + timing: { + type: "once", + when: "immediate", + }, },Also applies to: 649-652
🧹 Nitpick comments (2)
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (2)
72-74: Consider auto-collapsing other items when expanding a new one on mobile.When a user expands a medication item on mobile, consider automatically collapsing any previously expanded items to maintain a clean interface and prevent excessive scrolling.
setExpandedMedicationIndex( - expandedMedicationIndex === index ? null : index, + expandedMedicationIndex === index ? null : index ); +// Auto-collapse previously expanded items when expanding a new one +if (expandedMedicationIndex !== null && expandedMedicationIndex !== index) { + setExpandedMedicationIndex(null); +}Also applies to: 216-218
368-370: Remove autoFocus from QuantityInput.The autoFocus prop on QuantityInput could lead to unexpected behavior, especially when multiple medication rows are present.
<QuantityInput units={DOSAGE_UNITS} quantity={dosageInstruction?.dose_and_rate?.dose_quantity} onChange={(value) => handleUpdateDosageInstruction({ dose_and_rate: { type: "ordered", dose_quantity: value }, }) } disabled={disabled} - autoFocus={true} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
public/locale/en.json(10 hunks)src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx(7 hunks)src/components/Questionnaire/QuestionnaireForm.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/Questionnaire/QuestionnaireForm.tsx
- public/locale/en.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (1)
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (1)
584-584: Address the TODO comment about period_unit verification.The comment indicates uncertainty about the correctness of period_unit implementations.
Let's verify the period units against the FHIR specification:
Would you like me to:
- Research and verify the correct FHIR/UCUM units for each frequency?
- Open a GitHub issue to track this verification task?
✅ Verification successful
Remove TODO comment - period_unit implementation is correct
The period_unit implementations in FREQUENCY_OPTIONS are verified to be correct and consistent with:
- FHIR timing patterns
- Type definitions in medicationRequest.ts
- Standard medical frequency notations (BID, TID, QID, etc.)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for FHIR timing definitions and usage rg -l "period_unit|UCUM" --type tsLength of output: 268
Script:
#!/bin/bash # Check type definitions and implementations echo "=== Medication Request Types ===" cat src/types/emr/medicationRequest.ts echo -e "\n=== Period Unit Usage in MedicationRequestQuestion ===" rg "period_unit" -C 2 src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx echo -e "\n=== Utils Period Unit Handling ===" rg "period_unit" -C 2 src/Utils/utils.tsLength of output: 6417
| <QuantityInput | ||
| units={BOUNDS_DURATION_UNITS} | ||
| quantity={{ | ||
| value: dosageInstruction?.timing?.repeat?.bounds_duration?.value, | ||
| unit: | ||
| dosageInstruction?.timing?.repeat?.bounds_duration?.unit ?? "d", | ||
| }} | ||
| onChange={(value) => { | ||
| if (dosageInstruction?.timing?.repeat) { | ||
| handleUpdateDosageInstruction({ | ||
| timing: { | ||
| ...dosageInstruction.timing, | ||
| repeat: { | ||
| ...dosageInstruction.timing.repeat, | ||
| bounds_duration: { | ||
| value: value?.value, | ||
| unit: value?.unit as (typeof BOUNDS_DURATION_UNITS)[number], | ||
| }, | ||
| }, | ||
| }, | ||
| }); | ||
| } | ||
| }} | ||
| disabled={disabled || !isFrequencySet || as_needed_boolean} | ||
| /> |
There was a problem hiding this comment.
Add validation for duration input.
The duration input needs validation to prevent invalid values:
- Negative values should not be allowed
- Maximum duration should be constrained
- Different units might need different validation rules
<QuantityInput
units={BOUNDS_DURATION_UNITS}
quantity={{
value: dosageInstruction?.timing?.repeat?.bounds_duration?.value,
unit:
dosageInstruction?.timing?.repeat?.bounds_duration?.unit ?? "d",
}}
+ min={1}
+ max={unit === 'd' ? 365 : unit === 'wk' ? 52 : unit === 'mo' ? 12 : undefined}
onChange={(value) => {
if (dosageInstruction?.timing?.repeat) {
handleUpdateDosageInstruction({
timing: {
...dosageInstruction.timing,
repeat: {
...dosageInstruction.timing.repeat,
bounds_duration: {
- value: value?.value,
+ value: value?.value ? Math.max(1, value.value) : undefined,
unit: value?.unit as (typeof BOUNDS_DURATION_UNITS)[number],
},
},
},
});
}
}}
disabled={disabled || !isFrequencySet || as_needed_boolean}
/>Committable suggestion skipped: line range outside the PR's diff.
| <ValueSetSelect | ||
| system="system-medication" | ||
| placeholder={t("search_medications")} | ||
| onSelect={handleAddMedication} | ||
| disabled={disabled} | ||
| searchPostFix=" clinical drug" | ||
| /> |
There was a problem hiding this comment.
💡 Codebase verification
🛠️ Refactor suggestion
Filter out already selected medications from search results.
The verification confirms that the ValueSetSelect component currently has no mechanism to filter out already selected medications. The suggested change to add an excludeValues prop is valid and necessary to prevent duplicate selections.
- Add
excludeValues?: string[]to the Props interface insrc/components/Questionnaire/ValueSetSelect.tsx - Either filter the results client-side or pass the exclusion list to the API query
🔗 Analysis chain
Filter out already selected medications from search results.
The medication search should exclude medications that have already been added to prevent duplicate entries.
Let's verify if there's any existing filtering logic:
Consider passing the list of selected medications to filter them out:
<ValueSetSelect
system="system-medication"
placeholder={t("search_medications")}
onSelect={handleAddMedication}
disabled={disabled}
searchPostFix=" clinical drug"
+ excludeValues={medications.map(m => m.medication?.code)}
/>🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for filtering logic in ValueSetSelect component
rg -A 5 "ValueSetSelect.*system-medication"
Length of output: 45
Script:
#!/bin/bash
# Find ValueSetSelect component definition
fd ValueSetSelect
# Search for the component's implementation
ast-grep --pattern 'interface ValueSetSelectProps {
$$$
}'
# Look for medication-related state management
rg -B 2 -A 5 'medications.*\['
Length of output: 1083
Script:
#!/bin/bash
# Check ValueSetSelect implementation
cat src/components/Questionnaire/ValueSetSelect.tsx
# Look for any existing filtering logic in the component
rg -B 2 -A 5 'filter|exclude' src/components/Questionnaire/ValueSetSelect.tsx
Length of output: 3739
|
Reminder: To add the "needs testing" label, comment "ready for testing" on this PR. |
Co-authored-by: Rithvik Nishad <[email protected]>
|
@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! 🙌 |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Documentation