Encounter Info Card #10792
Encounter Info Card #10792
Conversation
WalkthroughThe changes update two patient-related components. In the 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 (2)
🔇 Additional comments (6)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Deploying care-fe with
|
| Latest commit: |
95a277d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://59c4a7f0.care-fe.pages.dev |
| Branch Preview URL: | https://encounter-info-card.care-fe.pages.dev |
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/pages/Encounters/EncounterInfoCard.tsx (3)
24-31: Consider adding error handling for the query.While the loading state is handled, there's no explicit error handling for the query. Consider adding error state handling to improve user experience.
const { data: encounter, isLoading } = useQuery({ + const { data: encounter, isLoading, error } = useQuery({ queryKey: ["encounter", encounterId], queryFn: query(routes.encounter.get, { pathParams: { id: encounterId }, queryParams: { facility: facilityId }, }), enabled: !!encounterId, }); +if (error) { + return <div className="text-red-500">Error loading encounter information</div>; +}
63-78: Consider extracting Badge components to reduce duplication.The date badges share similar structure and styling. Consider extracting them into a reusable component to improve maintainability.
+interface DateBadgeProps { + label: string; + date: string | null; + fallback: string; +} + +function DateBadge({ label, date, fallback }: DateBadgeProps) { + const formattedDate = date ? formatDateTime(date) : fallback; + return ( + <Badge + className="gap-1 rounded-sm py-1 px-2" + variant="primary" + title={`${label}: ${formattedDate}`} + > + {label}: + <span className="text-xs">{formattedDate}</span> + </Badge> + ); +} -<Badge className="gap-1 rounded-sm py-1 px-2" variant="primary" title={...}> - {t("start_date")}: - <span className="text-xs"> - {encounter.period.start ? formatDateTime(encounter.period.start) : t("not_started")} - </span> -</Badge> +<DateBadge + label={t("start_date")} + date={encounter.period.start} + fallback={t("not_started")} +/>Also applies to: 79-94
108-114: Consider using a constant for status icon mapping.The status icon logic could be moved to a constant mapping to make it more maintainable and reusable.
+const STATUS_ICONS = { + completed: <CircleCheck className="w-4 h-4 text-green-300" fill="green" />, + default: <CircleDashed className="w-4 h-4 text-yellow-500" /> +}; -{completedEncounterStatus.includes(encounter.status) ? ( - <CircleCheck className="w-4 h-4 text-green-300" fill="green" /> -) : ( - <CircleDashed className="w-4 h-4 text-yellow-500" /> -)} +{STATUS_ICONS[completedEncounterStatus.includes(encounter.status) ? 'completed' : 'default']}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
public/locale/en.json(1 hunks)src/components/Patient/EncounterQuestionnaire.tsx(2 hunks)src/pages/Encounters/EncounterInfoCard.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Pages changed - care-ohc
- GitHub Check: Test
- GitHub Check: OSSAR-Scan
- GitHub Check: cypress-run (1)
🔇 Additional comments (3)
public/locale/en.json (1)
1540-1540: LGTM! New localization key added.The new key "op_number" follows the existing naming pattern and provides a clear translation.
src/components/Patient/EncounterQuestionnaire.tsx (1)
11-12: LGTM! Clean integration of EncounterInfoCard.The component is properly imported and conditionally rendered with the required props. The implementation follows React best practices.
Also applies to: 31-33
src/pages/Encounters/EncounterInfoCard.tsx (1)
15-19: LGTM! Well-defined TypeScript interface.The interface clearly defines the required props with appropriate types.
| {patient.blood_group && ( | ||
| <Badge | ||
| className="capitalize gap-1 py-1 px-2" | ||
| variant="outline" | ||
| title={`Blood Group: ${patient.blood_group?.replace("_", " ")}`} | ||
| > | ||
| <Droplet className="w-4 h-4 text-red-300" fill="red" /> | ||
| {patient.blood_group?.replace("_", " ")} | ||
| </Badge> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider sanitizing blood group value before display.
The blood group value is directly rendered after a simple underscore replacement. Consider proper sanitization and validation.
+const sanitizeBloodGroup = (bloodGroup: string) => {
+ const validGroups = ['A+', 'A-', 'B+', 'B-', 'AB+', 'AB-', 'O+', 'O-'];
+ const formatted = bloodGroup.replace(/_/g, ' ').toUpperCase();
+ return validGroups.includes(formatted) ? formatted : 'Unknown';
+};
-{patient.blood_group?.replace("_", " ")}
+{sanitizeBloodGroup(patient.blood_group || '')}📝 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.
| {patient.blood_group && ( | |
| <Badge | |
| className="capitalize gap-1 py-1 px-2" | |
| variant="outline" | |
| title={`Blood Group: ${patient.blood_group?.replace("_", " ")}`} | |
| > | |
| <Droplet className="w-4 h-4 text-red-300" fill="red" /> | |
| {patient.blood_group?.replace("_", " ")} | |
| </Badge> | |
| )} | |
| // Add the following helper function (adjust the location as needed, e.g. above the component definition) | |
| const sanitizeBloodGroup = (bloodGroup: string) => { | |
| const validGroups = ['A+', 'A-', 'B+', 'B-', 'AB+', 'AB-', 'O+', 'O-']; | |
| const formatted = bloodGroup.replace(/_/g, ' ').toUpperCase(); | |
| return validGroups.includes(formatted) ? formatted : 'Unknown'; | |
| }; | |
| {patient.blood_group && ( | |
| <Badge | |
| className="capitalize gap-1 py-1 px-2" | |
| variant="outline" | |
| title={`Blood Group: ${sanitizeBloodGroup(patient.blood_group || '')}`} | |
| > | |
| <Droplet className="w-4 h-4 text-red-300" fill="red" /> | |
| {sanitizeBloodGroup(patient.blood_group || '')} | |
| </Badge> | |
| )} |
CARE
|
||||||||||||||||||||||||||||
| Project |
CARE
|
| Branch Review |
encounter-info-card
|
| Run status |
|
| Run duration | 07m 01s |
| Commit |
|
| Committer | Amjith Titus |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
11
|
| View all changes introduced in this branch ↗︎ | |
|
|
|
@nihal467 Done |
|
LGTM in cloudflare pages |
|
@nihal467 Switched to existing Patient Info Card but in disabled state |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/Patient/EncounterQuestionnaire.tsx (1)
59-61: Consider handling loading stateThe code doesn't handle the loading state of the encounter data fetch, which could result in layout shifts.
Consider adding a loading state indicator:
<div className="flex flex-col justify-between gap-2 px-4 py-1 md:flex-row"> <div className="font-base flex flex-col text-xs leading-relaxed text-secondary-700 md:text-right"> <div className="flex items-center"> <span className="text-secondary-900"> {t("last_modified")}:{" "} </span> - {formatDateTime(encounter.modified_date)} + {encounter.modified_date ? formatDateTime(encounter.modified_date) : t("loading")} </div> </div> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Patient/EncounterQuestionnaire.tsx(2 hunks)src/components/Patient/PatientInfoCard.tsx(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Pages changed - care-ohc
- GitHub Check: cypress-run (1)
🔇 Additional comments (11)
src/components/Patient/EncounterQuestionnaire.tsx (4)
1-15: Import structure looks goodThe new imports are well organized and properly support the functionality being added to fetch and display encounter data.
33-40: Good implementation of conditional data fetchingThe useQuery hook is properly implemented with appropriate query key, function, and conditional enabling based on encounter ID availability.
43-65: PatientInfoCard implementation aligns with PR objectivesThis implementation properly addresses the "Encounter Info Card" requirement from the PR objectives. The disabled state prevents interaction while displaying relevant encounter data.
66-87: Layout structure looks goodThe nested Card component with the QuestionnaireForm is properly structured within the flex container.
src/components/Patient/PatientInfoCard.tsx (7)
56-63: Good type import and interface updateThe
FacilityOrganizationtype import and adding the optionaldisabledproperty to the interface are appropriate changes.
66-66: Appropriate default value for disabled propSetting the default value of
disabledtofalseensures backward compatibility with existing code.
328-357: Implements hospital identifier badge hiding requirementThis conditional rendering implementation fulfills the PR objective of hiding empty hospital identifier badges, as part of the broader implementation of a disabled state.
405-420: Good conditional rendering of interactive elementsThe location update button is properly conditionally rendered based on the disabled state.
447-519: Proper implementation of update actions conditional renderingPreventing update actions when the component is in disabled state is consistent with the PR requirements.
525-540: Good extraction of organization badge rendering logicExtracting the
organizationBadgefunction improves code organization and maintainability.
532-532: Good cursor styling based on disabled stateThe cursor styling is appropriately conditional based on the disabled state.
…encounter-info-card
|
|
|
@nihal467 Removed the Add Location button |
|
LGTM |
|
@amjithtitus09 Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌 |
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
disableButtonsproperty in the patient info card.