Enhancement of Immunisation Records Tab in patient details page #9441
Conversation
WalkthroughThe pull request modifies the Changes
Assessment against linked issues
Possibly related PRs
Poem
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. |
|
Hi @bodhish Currently we have only covid vaccine details. So, for now i just displayed covid vaccine details in pateint details immunisation tab. This is how its look like now Will we get immunisation record form as you mentioned earlier that we should have structure form to capture Immunisation records in encounter form. is this fine for now or should i hold this PR till we get encounter form ready. |
|
👋 Hi, @Rishith25, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/components/Patient/PatientDetailsTab/ImmunisationRecords.tsx (5)
54-58: Simplify the conditional expression using optional chainingYou can simplify the conditional check by using optional chaining (
?.) to handle cases wherepatientData.vaccine_namemight beundefinedornull. This makes the code more concise and readable.Apply this diff to simplify the expression:
const filteredData = !searchTerm || - (patientData.vaccine_name && - patientData.vaccine_name.toLowerCase().includes(searchTerm)); + patientData.vaccine_name?.toLowerCase().includes(searchTerm);🧰 Tools
🪛 Biome (1.9.4)
[error] 56-57: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
54-58: RenamefilteredDatato reflect its boolean natureThe variable
filteredDataholds a boolean value indicating whether the data matches the search criteria. Renaming it to something likematchesSearchTermwould improve code clarity.Apply this diff to rename the variable:
-const filteredData = +const matchesSearchTerm =Remember to update all instances where
filteredDatais used accordingly.🧰 Tools
🪛 Biome (1.9.4)
[error] 56-57: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
87-95: AddonSelecthandler toDropdownMenuItemTo make the filter operational, add an
onSelecthandler to eachDropdownMenuItemto update the selected status in the state.Apply this diff to add the handler:
{vaccineStatuses.map((status) => ( <DropdownMenuItem key={status} className="hover:font-bold cursor-pointer" + onSelect={() => setSelectedStatus(status)} > {status} </DropdownMenuItem> ))}Ensure you define
selectedStatususinguseStateand adjust the data filtering logic accordingly.
144-185: Avoid redundant checks in conditional renderingSince
patientData.is_vaccinatedis already checked in the parent conditional at line 144, you can remove redundantpatientData.is_vaccinatedchecks within the child elements to simplify the code.Apply this diff to remove redundant checks:
{patientData.is_vaccinated && filteredData ? ( <tr> <td className="..."> - {patientData.is_vaccinated && patientData.covin_id + {patientData.covin_id ? patientData.covin_id : "-"} </td> <!-- Repeat for other fields --> </tr> ) : ( <!-- No records available row --> )}
189-189: Consider handling multiple immunisation recordsCurrently, the component is set up to display only COVID-19 vaccine details. If you plan to expand this to handle multiple immunisation records, consider iterating over an array of records.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Patient/PatientDetailsTab/ImmunisationRecords.tsx(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Patient/PatientDetailsTab/ImmunisationRecords.tsx
[error] 56-57: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (3)
src/components/Patient/PatientDetailsTab/ImmunisationRecords.tsx (3)
48-49: Initialize state variables and constants effectively
The initialization of searchTerm and vaccineStatuses is appropriate and correctly implemented.
51-53: Ensure consistent handling of search input
The handleSearchChange function correctly updates the searchTerm state with a lowercase value for case-insensitive searching.
98-115: Confirm edit permission logic and notification
The edit button correctly checks for patient activity and user permissions before allowing edits. The notification for permission denial is appropriate.
| <div className="flex flex-col sm:flex-row sm:justify-between sm:items-center mb-4"> | ||
| <SearchInput | ||
| className="w-72 sm:w-108 mb-4 sm:mb-0" | ||
| value={searchTerm} | ||
| onChange={handleSearchChange} | ||
| placeholder="Search by Vaccine Name" | ||
| name="vaccineNameSearch" | ||
| /> | ||
| <div className="flex sm:ml-4 sm:space-x-4 w-full sm:w-auto mb-4 sm:mb-0"> | ||
| <DropdownMenu modal={false}> | ||
| <DropdownMenuTrigger asChild> | ||
| <Button | ||
| variant="outline" | ||
| className="flex items-center w-full sm:w-auto" | ||
| > | ||
| <CareIcon icon="l-filter" className="mr-2" /> | ||
| {t("filter")} | ||
| </Button> | ||
| </DropdownMenuTrigger> | ||
| <DropdownMenuContent align="end" className="w-full font-medium"> | ||
| {vaccineStatuses.map((status) => ( | ||
| <DropdownMenuItem | ||
| key={status} | ||
| className="hover:font-bold cursor-pointer" | ||
| > | ||
| {status} | ||
| </DropdownMenuItem> | ||
| ))} | ||
| </DropdownMenuContent> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implement state management for filter functionality
Currently, the filter dropdown for vaccine statuses is rendered, but there's no state variable to store the selected status or logic to filter the data based on the selected status. Implementing this will make the filter functional.
Would you like assistance in implementing the state management and filtering logic for vaccine statuses?
| text="Completed" | ||
| className="text-green rounded-full text-sm font-semibold" | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make the immunisation status dynamic
The text prop for the Chip component is hardcoded to "Completed". To reflect the actual status of the immunisation record, consider making this value dynamic based on patient data.
Apply this diff to use dynamic status:
<Chip
- text="Completed"
+ text={patientData.immunisation_status || "Status Unknown"}
className="text-green rounded-full text-sm font-semibold"
/>Ensure that patientData includes an immunisation_status field or adjust accordingly.
Committable suggestion skipped: line range outside the PR's diff.
| {patientData.is_vaccinated && patientData.number_of_doses | ||
| ? patientData.number_of_doses + "/3" | ||
| : "-"} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid hardcoding total doses in Series Progress
The total number of vaccine doses ("/3") is hardcoded. To make the component adaptable to different vaccines with varying dose requirements, consider making the total number of doses dynamic.
Apply this diff to make total doses dynamic:
{patientData.number_of_doses
- ? patientData.number_of_doses + "/3"
+ ? `${patientData.number_of_doses}/${patientData.total_doses || "N/A"}`
: "-"}Ensure that patientData includes a total_doses field representing the required doses for the vaccine.
Committable suggestion skipped: line range outside the PR's diff.
Jacobjeevan
added
work-in-progress
and removed
question
|
@Rishith25 Are you waiting on the backend/input from Vignesh? 🤔 |
Yes |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Bug Fixes