Skip to content
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

Merged
merged 7 commits into from
Jan 2, 2025

Conversation

amjithtitus09
Copy link
Contributor

@amjithtitus09 amjithtitus09 commented Jan 2, 2025

Medication Request | Convert dosage instruction to array with necessary refactor

Proposed Changes

  • Converted Dosage Instruction inside Medication Request to array

@ohcnetwork/care-fe-code-reviewers

Merge Checklist

  • Add specs that demonstrate bug / test a new feature.
  • Update product documentation.
  • Ensure that UI text is kept in I18n files.
  • Prep screenshot or demo video for changelog entry, and attach it to issue.
  • Request for Peer Reviews
  • Completion of QA

Summary by CodeRabbit

  • New Features

    • Introduced a new route for printing prescriptions based on facility and encounter IDs.
    • Added a component for rendering a prescription print preview, including patient and medication details.
    • Enhanced MedicineAdministrationSheet to allow printing prescriptions based on facility context.
    • Implemented an empty state component for better user feedback in the medication list.
  • Refactor

    • Updated medication request data structure to support multiple dosage instructions.
    • Modified type definitions and component logic to access dosage instructions as an array.
    • Updated data access patterns in multiple medicine-related components to use array indexing.
    • Simplified props passed to MedicineAdministrationSheet by removing unnecessary variables.
  • Documentation

    • Implicitly documented changes in data structure for medication requests.

@amjithtitus09 amjithtitus09 requested a review from a team as a code owner January 2, 2025 15:12
Copy link
Contributor

coderabbitai bot commented Jan 2, 2025

Walkthrough

The pull request introduces a significant change in how dosage instructions are structured across multiple components. The primary modification is transforming the dosage_instruction from a single object to an array of objects. This change affects various components in the medicine and questionnaire sections, requiring updates to access dosage instruction properties through the first array element. The modifications ensure consistent handling of dosage instructions across the application, allowing for more flexible representation of medication instructions.

Changes

File Change Summary
src/components/Medicine/AdministerMedicine.tsx Updated doseAndRate and dosage instruction property access to use first array element.
src/components/Medicine/MedicineAdministrationSheet/AdministrationTable.tsx Modified as_needed_boolean access to use first array element.
src/components/Medicine/MedicineAdministrationSheet/AdministrationTableRow.tsx Changed dosage_instruction property access to use first array element.
src/components/Medicine/PrescriptionDetailCard.tsx Updated multiple dosage_instruction property accesses to use first array element.
src/components/Medicine/PrescriptionsTable.tsx Modified dosage_instruction property access in table rendering.
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx Introduced new type and updated dosage instruction handling to use array.
src/types/emr/medicationRequest.ts Changed dosage_instruction from single object to array of objects.
src/Routers/routes/ConsultationRoutes.tsx Added new route for printing prescriptions.
src/pages/Encounters/PrintPrescription.tsx Introduced new component for rendering prescription print preview.
src/components/Medicine/MedicineAdministrationSheet/index.tsx Updated component to include new facilityId prop and conditional rendering for print prescriptions.
src/pages/Encounters/tabs/EncounterMedicinesTab.tsx Passed facilityId prop to MedicineAdministrationSheet component.

Possibly related PRs

  • Dosage heading is misaligned in the prescription module #8645 #8751: The changes in the main PR regarding the handling of dosage instructions as an array are related to the modifications in AdministrationTable.tsx, which also updates how dosage information is accessed, indicating a similar shift in data structure.
  • Improve code splitting #8979: The main PR's changes to dosage instruction handling are relevant to the updates in PrescriptionsTable.tsx, which also modifies how dosage instructions are accessed as an array, reflecting a consistent approach across components.
  • New Cypress Test for HCX Workflow in the platform #9007: The changes in the main PR regarding dosage instructions are related to the modifications in PrescriptionDetailCard.tsx, which also updates the access pattern for dosage instructions to accommodate the new array structure.
  • Fix wrong terminology used in place of Log Update #9089: The changes in the main PR regarding dosage instructions are connected to the updates in ConsultationRoutes.tsx, which also reflects a shift in terminology and structure related to dosage instructions.
  • New Patient Search and Register UI #9400: The changes in the main PR regarding dosage instructions are relevant to the updates in FacilityList.tsx, which also enhances the search functionality and data handling related to patient management.

Suggested Labels

needs review, tested

Suggested Reviewers

  • rithviknishad
  • Jacobjeevan
  • nihal467

Poem

🐰 Dosage Instructions Dance
From single object to array's embrace,
Medication data finds its new space,
Flexible, nimble, with room to grow,
A rabbit's code leap, smooth and slow! 🏥
Hop, hop, hooray for structure so bright! 🌈


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 486a660 and 5b3fa6f.

📒 Files selected for processing (1)
  • src/components/Medicine/MedicineAdministrationSheet/AdministrationTable.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/Medicine/MedicineAdministrationSheet/AdministrationTable.tsx

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

netlify bot commented Jan 2, 2025

Deploy Preview for care-ohc ready!

Name Link
🔨 Latest commit 5b3fa6f
🔍 Latest deploy log https://app.netlify.com/sites/care-ohc/deploys/6776bdb30a013b00089c6027
😎 Deploy Preview https://deploy-preview-9665--care-ohc.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

cypress bot commented Jan 2, 2025

CARE    Run #4140

Run Properties:  status check passed Passed #4140  •  git commit 5b3fa6f90f: Medication Request | Convert dosage instruction to array
Project CARE
Branch Review medication-request-dosage-array
Run status status check passed Passed #4140
Run duration 01m 09s
Commit git commit 5b3fa6f90f: Medication Request | Convert dosage instruction to array
Committer Amjith Titus
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 3
View all changes introduced in this branch ↗︎

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 checks dose_quantity in multiple places. Ensure that dosage_instruction, dose_and_rate, and dose_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
If prescription.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 the dosage object, ensure dosage_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 using selectedRoute or something more descriptive than value={...} 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
When as_needed_boolean is true, ensure the as_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 in additional_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

📥 Commits

Reviewing files that changed from the base of the PR and between b58db6a and bf83af8.

📒 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 of dosage_instruction works, confirm that dosage_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 referencing dose_range?.low and dose_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
Accessing prescription.dosage_instruction[0].timing could fail if the array is empty or the object uninitialized. Ensure safe checks for timing or provide a default.

src/components/Medicine/AdministerMedicine.tsx (1)

57-57: Check for empty dosage_instruction array
You’re dereferencing [0] from prescription.dosage_instruction. Confirm that dosage_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 new MedicationRequestDosageInstruction type is introduced. Make sure other existing references to dosage_instruction use this type to maintain consistency and prevent casting or type errors.


45-45: Initialization as an empty array
Changing dosage_instruction to an empty array is appropriate. Confirm that any code relying on a non-empty dosage_instruction is adapted accordingly.


146-146: Guard array access
Before indexing medication.dosage_instruction[0], make sure dosage_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 of dose_and_rate if it can be null.


257-259: Consolidate PRN checks
You’re toggling as_needed_boolean and resetting as_needed_for. Confirm your logic ensures as_needed_for is cleared or retained correctly across the UI.

Also applies to: 264-264


357-357: Validate timing match
The reverseFrequencyOption function matches frequency, period, and period_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 for timing, any new frequency definitions must match MedicationRequest["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[];
Copy link
Contributor

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:

  1. dosage_instruction is required (not optional) in the MedicationRequest interface, but its elements like timing, as_needed_boolean, and other properties are optional
  2. 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 like as_needed_boolean, dose_and_rate, etc.
  • src/components/Medicine/PrescriptionsTable.tsx: Add null checks for route, timing, and other optional fields
  • src/components/Medicine/MedicineAdministrationSheet/AdministrationTableRow.tsx: Add array length check before accessing first element
  • src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx: Add checks for optional fields in form handling
  • src/components/Medicine/AdministerMedicine.tsx: Add null check for dose_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 details
  • src/components/Medicine/PrescriptionsTable.tsx: Direct access in table data transformation
  • src/components/Medicine/MedicineAdministrationSheet/AdministrationTableRow.tsx: Direct access in dosageInstruction assignment
  • src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx: Multiple instances in form handling
  • src/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
Copy link
Contributor

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.

Comment on lines +94 to +106
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),
Copy link
Contributor

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.

Comment on lines +148 to +150
{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)}
Copy link
Contributor

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.

Suggested change
{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];
Copy link
Contributor

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 ? (
Copy link
Contributor

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

Copy link

cloudflare-workers-and-pages bot commented Jan 2, 2025

Deploying care-fe with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5b3fa6f
Status: ✅  Deploy successful!
Preview URL: https://25bc8558.care-fe.pages.dev
Branch Preview URL: https://medication-request-dosage-ar.care-fe.pages.dev

View logs

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between bf83af8 and d396a76.

📒 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 potential prescription or print-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.

Comment on lines +37 to +42
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,
);
Copy link
Contributor

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.

Comment on lines +172 to +174
const instruction = medication.dosage_instruction[0];

if (!instruction) return null;
Copy link
Contributor

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between d396a76 and b70cd3a.

📒 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 see facilityId being passed down to maintain context for the MedicineAdministrationSheet. Just ensure that anywhere else in the codebase where facilityId is used, it aligns with the same naming and type to avoid confusion.


16-20: Confirm PRN logic
Passing isPrn={true} here is straightforward, but please verify that the downstream logic properly distinguishes PRN vs. non-PRN usage in conjunction with facilityId.

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 for facilityId to prevent routes with undefined facility IDs.


31-35: Destructure new prop for clarity
Destructuring facilityId is clear and concise. Good job adding the new prop here.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 in FREQUENCY_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

📥 Commits

Reviewing files that changed from the base of the PR and between b70cd3a and bdc50ab.

📒 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 referencing dose_range properties.

Within the formatDosage function, consider verifying dose_range.low and dose_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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between bdc50ab and 486a660.

📒 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:

  1. The MedicineAdministrationSheet component in src/components/Medicine/MedicineAdministrationSheet/index.tsx only expects facilityId as a prop in its interface
  2. There are no other usages of the component that pass isDischarged or readonly props
  3. 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 5

Length 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 3

Length 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 2

Length 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.

Comment on lines +194 to +195
const instruction = medication.dosage_instruction[0];
if (!instruction) return null;
Copy link
Contributor

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.

Suggested change
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 }> = {
Copy link
Member

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

Copy link
Contributor

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

@bodhish bodhish merged commit ec54284 into develop Jan 2, 2025
23 checks passed
@bodhish bodhish deleted the medication-request-dosage-array branch January 2, 2025 16:27
Copy link

github-actions bot commented Jan 2, 2025

@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! 🙌

@coderabbitai coderabbitai bot mentioned this pull request Jan 7, 2025
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants