-
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 | Convert dosage instruction to array #9665
Conversation
WalkthroughThe pull request introduces a significant change in how dosage instructions are structured across multiple components. The primary modification is transforming the Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 #4140
Run Properties:
|
Project |
CARE
|
Branch Review |
medication-request-dosage-array
|
Run status |
Passed #4140
|
Run duration | 01m 09s |
Commit |
5b3fa6f90f: Medication Request | Convert dosage instruction to array
|
Committer | Amjith Titus |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
3
|
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: 6
🧹 Nitpick comments (11)
src/components/Medicine/PrescriptionDetailCard.tsx (2)
166-167
: Consistent null checks for dose_quantity
The code checksdose_quantity
in multiple places. Ensure thatdosage_instruction
,dose_and_rate
, anddose_quantity
are never null or undefined. If they can be, handle these cases gracefully (e.g., show default UI or handle errors).Also applies to: 173-173
183-183
: Display fallback or loading state for route
Ifprescription.dosage_instruction[0].route
might be undefined or not loaded yet, consider showing a fallback or “N/A” value to avoid potential runtime errors or blank UI states.src/components/Medicine/AdministerMedicine.tsx (1)
157-160
: Guard against null properties in dosage object
When constructing thedosage
object, ensuredosage_instruction[0]
is valid. If it’s missing or partial, the submission might fail.src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (8)
60-64
: Clarify repetitive initialization
Here,dosage_instruction
is reset to an empty array again. If you intend to set default dosage instructions, consider factoring out into a single constant or function for better reusability and maintenance.
148-148
: Split logic for clarity
When updating dosage instructions, consider splitting logic for single vs. multiple dosage instructions. This helps future-proof the code if multiple instructions are eventually supported.
151-151
: Deep copy objects
When merging updates into[...dosageInstruction, ...updates]
, ensure references are not unintentionally shared. If potential side effects exist, consider using structured clone or similar.
220-220
: Rename or clarify route prop
For clarity, consider usingselectedRoute
or something more descriptive thanvalue={...}
to reduce confusion with other potential props.
232-232
: Validate method selection
Ensure a default “unselected” state or placeholder is viable. If so, handle it gracefully in subsequent code (like the “Administer Medicine” form).
246-246
: Consider site validations
Selecting a body site should be validated (e.g., if certain medications require specific routes). Provide a fallback or handle missing site.
275-275
: Refine “as needed” instructions
Whenas_needed_boolean
is true, ensure theas_needed_for
dropdown content is relevant. Possibly provide custom instructions or restrict to an appropriate subset.Also applies to: 280-280
330-340
: Array-based update for additional_instruction
Since you’re updating the first item inadditional_instruction
, confirm future increments can handle multiple instructions gracefully. If you foresee multiple instructions, consider a loop or map.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/components/Medicine/AdministerMedicine.tsx
(2 hunks)src/components/Medicine/MedicineAdministrationSheet/AdministrationTable.tsx
(1 hunks)src/components/Medicine/MedicineAdministrationSheet/AdministrationTableRow.tsx
(2 hunks)src/components/Medicine/PrescriptionDetailCard.tsx
(3 hunks)src/components/Medicine/PrescriptionsTable.tsx
(1 hunks)src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx
(13 hunks)src/types/emr/medicationRequest.ts
(1 hunks)
🔇 Additional comments (11)
src/components/Medicine/PrescriptionDetailCard.tsx (3)
78-78
: Ensure dosage_instruction is non-empty before indexing
Although referencing the first element ofdosage_instruction
works, confirm thatdosage_instruction
is guaranteed to have at least one element. Otherwise, this line may throw an error in edge cases.
149-159
: Handle potential null values in dose_range
When referencingdose_range?.low
anddose_range?.high
, ensure the object initialization is always valid. Consider using optional chaining more extensively or adding fallback values to prevent runtime errors.
195-195
: Confirm timing is defined
Accessingprescription.dosage_instruction[0].timing
could fail if the array is empty or the object uninitialized. Ensure safe checks fortiming
or provide a default.src/components/Medicine/AdministerMedicine.tsx (1)
57-57
: Check for empty dosage_instruction array
You’re dereferencing[0]
fromprescription.dosage_instruction
. Confirm thatdosage_instruction
always contains at least one element. Otherwise, guard against potential runtime errors or empty arrays.src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (7)
23-23
: Ensure consistent usage of new type
The newMedicationRequestDosageInstruction
type is introduced. Make sure other existing references todosage_instruction
use this type to maintain consistency and prevent casting or type errors.
45-45
: Initialization as an empty array
Changingdosage_instruction
to an empty array is appropriate. Confirm that any code relying on a non-emptydosage_instruction
is adapted accordingly.
146-146
: Guard array access
Before indexingmedication.dosage_instruction[0]
, make suredosage_instruction
is never empty.
206-206
: Optional chaining for dose_quantity
medication.dosage_instruction[0]?.dose_and_rate?.dose_quantity
is properly guarded. Consider verifying existence ofdose_and_rate
if it can be null.
257-259
: Consolidate PRN checks
You’re togglingas_needed_boolean
and resettingas_needed_for
. Confirm your logic ensuresas_needed_for
is cleared or retained correctly across the UI.Also applies to: 264-264
357-357
: Validate timing match
ThereverseFrequencyOption
function matches frequency, period, andperiod_unit
. If the function fails to find a match (e.g., an unrecognized frequency), handle the fallback gracefully.
409-409
: Ensure type-safety in FREQUENCY_OPTIONS
By setting the type fortiming
, any new frequency definitions must matchMedicationRequest["dosage_instruction"][0]["timing"]
exactly. That’s good for consistency but be cautious about partial property additions.
@@ -124,7 +124,7 @@ export interface MedicationRequest { | |||
patient?: string; // UUID | |||
encounter?: string; // UUID | |||
authored_on: string; | |||
dosage_instruction: MedicationRequestDosageInstruction; | |||
dosage_instruction: MedicationRequestDosageInstruction[]; |
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.
💡 Codebase verification
Array access needs defensive checks in multiple components
Based on the type definition and usage analysis, there are potential issues:
dosage_instruction
is required (not optional) in theMedicationRequest
interface, but its elements liketiming
,as_needed_boolean
, and other properties are optional- Multiple components directly access
dosage_instruction[0]
without checking if:- The array has elements
- The accessed optional properties exist
Locations requiring defensive programming:
src/components/Medicine/PrescriptionDetailCard.tsx
: Add checks before accessing optional properties likeas_needed_boolean
,dose_and_rate
, etc.src/components/Medicine/PrescriptionsTable.tsx
: Add null checks forroute
,timing
, and other optional fieldssrc/components/Medicine/MedicineAdministrationSheet/AdministrationTableRow.tsx
: Add array length check before accessing first elementsrc/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx
: Add checks for optional fields in form handlingsrc/components/Medicine/AdministerMedicine.tsx
: Add null check fordose_and_rate
which is optional
Example pattern to follow:
const dosageInstruction = prescription.dosage_instruction[0];
if (dosageInstruction?.dose_and_rate?.dose_quantity) {
// Safe to access
}
🔗 Analysis chain
Change from single dosage instruction to an array
Converting dosage_instruction
to an array is a good improvement for handling multiple dosage instructions. However, please ensure that all referencing components defensively check against empty arrays, especially when accessing [0]
.
Direct array access without defensive checks found in multiple components
Several components directly access dosage_instruction[0]
without checking if the array is empty, which could lead to runtime errors. Here are the specific locations that need attention:
src/components/Medicine/PrescriptionDetailCard.tsx
: Multiple instances of direct[0]
access for displaying prescription detailssrc/components/Medicine/PrescriptionsTable.tsx
: Direct access in table data transformationsrc/components/Medicine/MedicineAdministrationSheet/AdministrationTableRow.tsx
: Direct access indosageInstruction
assignmentsrc/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx
: Multiple instances in form handlingsrc/components/Medicine/AdministerMedicine.tsx
: Direct access when getting dose information
Let's verify if there's any defensive programming in place before these accesses.
Let me try a different approach to check for defensive programming patterns.
Let me check one more thing to see the complete type definition of MedicationRequest.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify all references to `dosage_instruction` and ensure they properly handle arrays that may be empty.
rg -A 5 $'dosage_instruction\\[0\\]'
Length of output: 17632
Script:
#!/bin/bash
# Check for any null checks or length checks before accessing dosage_instruction[0]
rg -B 3 "dosage_instruction(\?.length|\.length|&&)"
Length of output: 53
Script:
#!/bin/bash
# Look for any conditional checks around dosage_instruction
rg -B 2 "(\?\.|\&\&|if.*?).*?dosage_instruction"
# Also check the type definition to understand if it can be undefined/null
rg -A 2 "interface.*?MedicationRequest"
Length of output: 1194
Script:
#!/bin/bash
# Get the complete MedicationRequest interface definition
cat src/types/emr/medicationRequest.ts
Length of output: 3193
@@ -37,7 +37,7 @@ export default function MedicineAdministrationTable({ | |||
<span className="hidden px-2 text-center text-xs leading-none lg:block"> | |||
<p>{t("dosage")} &</p> | |||
<p> | |||
{!prescriptions[0]?.dosage_instruction.as_needed_boolean | |||
{!prescriptions[0]?.dosage_instruction[0].as_needed_boolean |
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
Check for potential index error when accessing the first dosage instruction
Referencing dosage_instruction[0]
could lead to runtime errors if the array is empty. A simple guard check is recommended:
- {!prescriptions[0]?.dosage_instruction[0].as_needed_boolean
+ {!prescriptions[0]?.dosage_instruction?.length &&
+ !prescriptions[0].dosage_instruction[0].as_needed_boolean
Committable suggestion skipped: line range outside the PR's diff.
obj.dosage_instruction[0].route && | ||
displayCode(obj.dosage_instruction[0].route), | ||
frequency__pretty: | ||
obj.dosage_instruction.timing && | ||
displayTiming(obj.dosage_instruction.timing), | ||
obj.dosage_instruction[0].timing && | ||
displayTiming(obj.dosage_instruction[0].timing), | ||
max_dose_per_period__pretty: | ||
obj.dosage_instruction.max_dose_per_period && | ||
obj.dosage_instruction[0].max_dose_per_period && | ||
displayDoseRange( | ||
obj.dosage_instruction.max_dose_per_period, | ||
obj.dosage_instruction[0].max_dose_per_period, | ||
), | ||
indicator: | ||
obj.dosage_instruction.as_needed_for && | ||
displayCode(obj.dosage_instruction.as_needed_for), | ||
obj.dosage_instruction[0].as_needed_for && | ||
displayCode(obj.dosage_instruction[0].as_needed_for), |
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
Guard against empty dosage_instruction arrays
All these references to obj.dosage_instruction[0]
assume at least one element is present. It’s advisable to gracefully handle or fallback if the array is empty. For instance:
- route__pretty:
- obj.dosage_instruction[0].route &&
- displayCode(obj.dosage_instruction[0].route),
+ route__pretty:
+ obj.dosage_instruction?.length &&
+ obj.dosage_instruction[0].route &&
+ displayCode(obj.dosage_instruction[0].route),
Apply a similar pattern for the other properties (timing
, max_dose_per_period
, as_needed_for
) to avoid runtime errors.
Committable suggestion skipped: line range outside the PR's diff.
{prescription.dosage_instruction[0].route && ( | ||
<span className="hidden rounded-full border border-blue-500 bg-blue-100 px-1.5 text-xs font-medium text-blue-700 lg:block"> | ||
{displayCode(prescription.dosage_instruction.route)} | ||
{displayCode(prescription.dosage_instruction[0].route)} |
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
Safely access prescription.dosage_instruction[0].route
As with other components, ensure that the dosage_instruction
array is not empty before accessing [0]
.
- {prescription.dosage_instruction[0].route && (
+ {prescription.dosage_instruction?.length &&
+ prescription.dosage_instruction[0].route && (
<span> ... </span>
📝 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.
{prescription.dosage_instruction[0].route && ( | |
<span className="hidden rounded-full border border-blue-500 bg-blue-100 px-1.5 text-xs font-medium text-blue-700 lg:block"> | |
{displayCode(prescription.dosage_instruction.route)} | |
{displayCode(prescription.dosage_instruction[0].route)} | |
{prescription.dosage_instruction?.length && | |
prescription.dosage_instruction[0].route && ( | |
<span className="hidden rounded-full border border-blue-500 bg-blue-100 px-1.5 text-xs font-medium text-blue-700 lg:block"> | |
{displayCode(prescription.dosage_instruction[0].route)} |
@@ -217,7 +217,7 @@ | |||
export function DosageFrequencyInfo({ | |||
prescription, | |||
}: DosageFrequencyInfoProps) { | |||
const dosageInstruction = prescription.dosage_instruction; | |||
const dosageInstruction = prescription.dosage_instruction[0]; |
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
Add null-check for dosage_instruction array
In DosageFrequencyInfo
, referencing prescription.dosage_instruction[0]
directly can cause errors if the array is empty. Consider a guard to avoid potential runtime issues.
- const dosageInstruction = prescription.dosage_instruction[0];
+ const dosageInstruction = prescription.dosage_instruction?.[0];
+ if (!dosageInstruction) {
+ return null; // or a placeholder
+ }
Committable suggestion skipped: line range outside the PR's diff.
</Detail> | ||
|
||
{prescription.dosage_instruction.as_needed_boolean ? ( | ||
{prescription.dosage_instruction[0].as_needed_boolean ? ( |
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
Validate PRN fields together
When as_needed_boolean
is true, as_needed_for
should be required. Consider adding validation in the parent form or data model to ensure correct usage of these fields.
Also applies to: 191-191
Deploying care-fe with Cloudflare Pages
|
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: 2
🧹 Nitpick comments (2)
src/pages/Encounters/PrintPrescription.tsx (2)
44-46
: Consider a more user-friendly empty state.When no medications are found, the user sees a simple message. You could enhance this with a "Back" button or recommended next steps (e.g., "Add Medication").
128-165
: Add test coverage for “PrescriptionsTable” and “PrescriptionEntry.”Unit tests or integration tests would ensure that different medication data scenarios (empty array, multiple instructions, PRN vs. normal medications) render as expected without regressions.
Want help writing a test file covering these components?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Routers/routes/ConsultationRoutes.tsx
(1 hunks)src/pages/Encounters/PrintPrescription.tsx
(1 hunks)
🔇 Additional comments (3)
src/Routers/routes/ConsultationRoutes.tsx (2)
8-8
: Good import practice.Importing
PrintPrescription
from"@/pages/Encounters/PrintPrescription"
aligns with the file structure and naming conventions seen elsewhere. No issues here.
11-14
: Verify consistency with existing route patterns.Ensure that this new route
"/facility/:facilityId/encounter/:encounterId/prescriptions"
is consistently documented and tested alongside other encounter-related routes. In particular, confirm that the trailing/prescriptions
segment is the desired pattern (instead of a potentialprescription
orprint-prescription
) and that this route integrates well with the rest of the application’s routing structure.src/pages/Encounters/PrintPrescription.tsx (1)
218-258
: Check optional fields for null values.Route, method, site, and frequency assumptions rely on nested properties. Although optional chaining is partially used, ensure each field gracefully handles missing data to avoid rendering blank or unwanted text.
const normalMedications = medications?.results?.filter( | ||
(m) => !m.dosage_instruction[0]?.as_needed_boolean, | ||
); | ||
const prnMedications = medications?.results?.filter( | ||
(m) => m.dosage_instruction[0]?.as_needed_boolean, | ||
); |
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
Potentially omit [0]
indexing if multiple dosage instructions will be supported.
Currently, medication classification for normal vs. PRN references only the first dosage instruction (dosage_instruction[0]
). If future requirements include multiple dosage instructions, you might need to handle or iterate through the entire array.
const instruction = medication.dosage_instruction[0]; | ||
|
||
if (!instruction) return null; |
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
Handle multiple instructions if required.
Similar to the classification logic, referencing only [0]
might not capture multiple dosage instructions. If supporting an array of instructions is a PR objective, a loop or map might be needed to render all dosage instructions.
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 (1)
src/components/Medicine/MedicineAdministrationSheet/index.tsx (1)
162-174
: Consider internationalization for “Print Prescription”
Everything looks great functionally. However, the text “Print Prescription” might need to be moved into I18n files if your application is localized.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Medicine/MedicineAdministrationSheet/index.tsx
(3 hunks)src/pages/Encounters/tabs/EncounterMedicinesTab.tsx
(1 hunks)
🔇 Additional comments (4)
src/pages/Encounters/tabs/EncounterMedicinesTab.tsx (2)
11-15
: Ensure consistent prop usage for readability
It’s good to seefacilityId
being passed down to maintain context for theMedicineAdministrationSheet
. Just ensure that anywhere else in the codebase wherefacilityId
is used, it aligns with the same naming and type to avoid confusion.
16-20
: Confirm PRN logic
PassingisPrn={true}
here is straightforward, but please verify that the downstream logic properly distinguishes PRN vs. non-PRN usage in conjunction withfacilityId
.src/components/Medicine/MedicineAdministrationSheet/index.tsx (2)
26-26
: Validate required prop
facilityId
is introduced as a mandatory string. Make sure the caller always has a valid value forfacilityId
to prevent routes with undefined facility IDs.
31-35
: Destructure new prop for clarity
DestructuringfacilityId
is clear and concise. Good job adding the new prop here.
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 (4)
src/pages/Encounters/PrintPrescription.tsx (4)
28-35
: Provide a fallback or default handling for unknown frequency keys.Currently, the function returns
undefined
if the key is not found inFREQUENCY_DISPLAY
. Consider returning a default frequency or gracefully handling the unknown case (e.g., logging a warning or using a placeholder) to avoid potential rendering issues.
87-110
: Evaluate merging or parallelizing queries.Fetching the encounter and medications separately is clear, but depending on usage patterns, you might consider merging these queries or ensuring an optimized parallel fetch. This could simplify error and loading states when the data is always needed together.
129-135
: Offer a retry action or navigation path on empty results.The message “No medications found...” is helpful feedback. You may want to include a “Retry” or “Go Back” button to guide users in the absence of data, enhancing their overall experience.
275-292
: Refactor skeleton loader into a shared component.The placeholder pulse loader for missing patient details effectively communicates loading state. If similar placeholders are used elsewhere in the codebase, consider extracting it into a reusable Skeleton component for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/Encounters/PrintPrescription.tsx
(1 hunks)
🔇 Additional comments (6)
src/pages/Encounters/PrintPrescription.tsx (6)
16-26
: Good approach for frequency mapping.This dictionary-based approach for mapping frequency codes is clear and easy to maintain.
36-50
: Ensure robustness when referencingdose_range
properties.Within the
formatDosage
function, consider verifyingdose_range.low
anddose_range.high
before accessing their nested fields. This step prevents runtime errors if the data is partially missing or malformed.
52-85
: Consider accommodating multiple instructions.If future requirements involve multiple dosage instructions, a single call to
formatSig
per medication instruction may no longer be sufficient. Iterating over each instruction could ensure consistency in the printed prescription details.
111-116
: Revisit indexing only the first dosage instruction.Filtering medications by
dosage_instruction[0]?.as_needed_boolean
addresses simple cases but may miss scenarios with multiple instructions. If your requirements expand, iterating over all dosage instructions can better reflect the medication's full PRN or non-PRN status.
118-127
: Iterate over the entire dosage instruction array for frequencies.Right now, you're collecting frequencies from
dosage_instruction[0]
only. If multiple instructions can have different frequencies, consider extending the logic to parse all instructions for a truly comprehensive frequency set.
302-308
: Map over all dosage instructions to handle multiple instructions.Your current implementation references only
dosage_instruction[0]
, meaning additional instructions (if present) remain unhandled. If support for multiple instructions is needed, consider mapping over the entire array and rendering each instruction’s dosage and details.
src/components/Medicine/MedicineAdministrationSheet/AdministrationTable.tsx
Outdated
Show resolved
Hide resolved
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 (4)
src/components/Medicine/MedicineAdministrationSheet/index.tsx (4)
27-37
: Consider enhancing frequency mapping maintainability.The
FREQUENCY_DISPLAY
constant provides a good mapping of frequency codes, but could benefit from being moved to a separate constants file for better maintainability.Consider moving this to a dedicated constants file:
- const FREQUENCY_DISPLAY: Record<string, { code: string; meaning: string }> = { + // src/constants/medicationFrequencies.ts + export const FREQUENCY_DISPLAY: Record<string, { code: string; meaning: string }> = {
63-79
: Consider memoizing filtered results.The medication filtering logic could benefit from memoization to prevent unnecessary recalculations.
+ import { useMemo } from "react"; - const filteredMedications = medications?.results?.filter( + const filteredMedications = useMemo(() => medications?.results?.filter( (med: MedicationRequest) => { if (!searchQuery.trim()) return true; const searchTerm = searchQuery.toLowerCase().trim(); const medicationName = med.medication?.display?.toLowerCase() || ""; return medicationName.includes(searchTerm); }, - ); + ), [medications?.results, searchQuery]); - const activeMedications = filteredMedications?.filter( + const activeMedications = useMemo(() => filteredMedications?.filter( (med: MedicationRequest) => ["active", "on_hold"].includes(med.status || ""), - ); + ), [filteredMedications]);
202-217
: Extract variant utility functions.The status and priority variant functions should be moved to a utility file for reusability.
Create a new utility file:
// src/utils/medicationUtils.ts export const getStatusVariant = ( status: string = "", ): "default" | "destructive" | "secondary" | "outline" | "primary" => { // ... existing implementation }; export const getPriorityVariant = ( priority: string = "", ): "default" | "destructive" | "secondary" | "outline" | "primary" => { // ... existing implementation };Also applies to: 220-232
284-300
: Consider extracting dose display logic.The dose display logic is complex and could benefit from being extracted into a separate component.
type DoseDisplayProps = { doseAndRate: MedicationRequest["dosage_instruction"][0]["dose_and_rate"]; }; const DoseDisplay = ({ doseAndRate }: DoseDisplayProps) => { if (!doseAndRate) return null; return doseAndRate.type === "calculated" ? ( <span> {doseAndRate.dose_range?.low.value} {doseAndRate.dose_range?.low.unit} →{" "} {doseAndRate.dose_range?.high.value} {doseAndRate.dose_range?.high.unit} </span> ) : ( <span> {doseAndRate.dose_quantity?.value} {doseAndRate.dose_quantity?.unit} </span> ); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Routers/routes/ConsultationRoutes.tsx
(1 hunks)src/components/Medicine/MedicineAdministrationSheet/index.tsx
(1 hunks)src/pages/Encounters/tabs/EncounterMedicinesTab.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Routers/routes/ConsultationRoutes.tsx
🔇 Additional comments (2)
src/pages/Encounters/tabs/EncounterMedicinesTab.tsx (1)
8-8
: LGTM! Verify prop removal impact.The simplified component interface looks good, passing only the essential
facilityId
prop.Let's verify that the removed props were not essential:
✅ Verification successful
Props removal is safe and aligned with component's interface
Based on the search results, I can confirm that:
- The
MedicineAdministrationSheet
component insrc/components/Medicine/MedicineAdministrationSheet/index.tsx
only expectsfacilityId
as a prop in its interface- There are no other usages of the component that pass
isDischarged
orreadonly
props- The component is properly imported and used consistently across the codebase
The removal of these props appears to be intentional and aligns with the component's current implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining usage of readonly prop in MedicineAdministrationSheet rg "readonly.*MedicineAdministrationSheet" # Search for any references to isDischarged in the context of medicines rg "isDischarged.*medicine" -A 5Length of output: 79
Script:
#!/bin/bash # Let's try a broader search for the component usage and its props ast-grep --pattern 'MedicineAdministrationSheet $$$' # Also search for the component definition to understand its props interface ast-grep --pattern 'interface MedicineAdministrationSheetProps { $$$ }' # Backup search for any isDischarged prop usage rg "isDischarged" -A 3Length of output: 153
Script:
#!/bin/bash # Let's try finding the component file first fd "MedicineAdministrationSheet" --type f # Then let's look for TypeScript/JavaScript files containing MedicineAdministrationSheet rg "MedicineAdministrationSheet" --type ts --type tsx --type js --type jsx -B 2 -A 2Length of output: 154
Script:
#!/bin/bash # Search for files without type restriction rg "MedicineAdministrationSheet" -B 2 -A 2 # Also try finding the file directly find . -type f -name "*MedicineAdministrationSheet*"Length of output: 4028
src/components/Medicine/MedicineAdministrationSheet/index.tsx (1)
39-45
: LGTM! Well-structured helper function.The function correctly handles the new array structure of
dosage_instruction
and includes proper null checks.
const instruction = medication.dosage_instruction[0]; | ||
if (!instruction) return null; |
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
Add type guard for dosage instruction access.
The access to the first element of dosage_instruction
array could be more type-safe.
- const instruction = medication.dosage_instruction[0];
- if (!instruction) return null;
+ const [instruction] = medication.dosage_instruction;
+ if (!instruction || !medication.medication?.display) {
+ console.warn(`Invalid medication request data: ${medication.id}`);
+ return null;
+ }
📝 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 instruction = medication.dosage_instruction[0]; | |
if (!instruction) return null; | |
const [instruction] = medication.dosage_instruction; | |
if (!instruction || !medication.medication?.display) { | |
console.warn(`Invalid medication request data: ${medication.id}`); | |
return null; | |
} |
import { formatPatientAge } from "@/Utils/utils"; | ||
import { MedicationRequest } from "@/types/emr/medicationRequest"; | ||
|
||
const FREQUENCY_DISPLAY: Record<string, { code: string; meaning: string }> = { |
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.
@coderabbitai create an issue to clean up PrintPrescription implementaiton
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.
@bodhish The issue has been successfully created at #9670. Let me know if there's anything else you'd like to address.
🔗 Analysis chain
@bodhish Acknowledged! Creating a GitHub issue as requested.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
gh issue create \
--title "Clean up PrintPrescription implementation" \
--body $'User @bodhish requested to clean up the PrintPrescription implementation.\n\nPR link: https://github.com/ohcnetwork/care_fe/pull/9665\nComment link: https://github.com/ohcnetwork/care_fe/pull/9665#discussion_r1901047429\n\nRequested by bodhish.' \
--assignee bodhish
Length of output: 387
@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! 🙌 |
Medication Request | Convert dosage instruction to array with necessary refactor
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
MedicineAdministrationSheet
to allow printing prescriptions based on facility context.Refactor
MedicineAdministrationSheet
by removing unnecessary variables.Documentation