Disabled view files and assign volunteer for discharged patients #9278
Conversation
WalkthroughThe changes in this pull request focus on the Changes
Assessment against linked issues
Possibly related issues
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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
src/components/Patient/FileUploadPage.tsx (1)
20-22: Consider adding a notification before redirect.The navigation logic correctly prevents inactive patients from accessing the file upload page. However, consider adding a notification to inform users why they're being redirected.
if (!patient?.is_active) { + Notification.Error({ + msg: t("cannot_access_files_for_discharged_patient") + }); navigate(`/facility/${facilityId}/patient/${patientId}`); }src/components/Patient/PatientHome.tsx (2)
492-492: Consider adding a tooltip for disabled state.The button is correctly disabled for inactive patients. However, consider adding a tooltip to explain why the button is disabled.
disabled={!patientData.is_active} + title={!patientData.is_active ? t("button_disabled_discharged_patient") : ""}
511-511: Consider adding a tooltip for disabled state.The button is correctly disabled for inactive patients. However, consider adding a tooltip to explain why the button is disabled.
disabled={!patientData.is_active} + title={!patientData.is_active ? t("button_disabled_discharged_patient") : ""}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/components/Patient/FileUploadPage.tsx(2 hunks)src/components/Patient/PatientHome.tsx(2 hunks)
Jacobjeevan
added
changes required
and removed
needs testing
reviewed
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
src/components/Patient/PatientHome.tsx (1)
92-98: MoveisPatientDischargedto a shared utility fileBased on previous discussions, this logic should be moved to a shared utility file for reuse across components. This would:
- Ensure consistent discharge status checks across the application
- Follow DRY principles
- Make it easier to maintain and update the logic in one place
Consider creating a new utility file (e.g.,
src/Utils/patient.ts):export const isPatientDischarged = ( patientData: PatientModel, facilityId?: string ): boolean => { return ( !patientData.is_active || facilityId !== patientData.facility_object?.id?.toString() ); };Then import and use it in this component:
import { isPatientDischarged } from '@/Utils/patient'; // ... disabled={isPatientDischarged(patientData, facilityId)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/components/Patient/PatientHome.tsx(3 hunks)
🔇 Additional comments (1)
src/components/Patient/PatientHome.tsx (1)
511-511: LGTM: Button disabled states are correctly implemented
The isPatientDischarged function is properly used to disable both the file upload and volunteer assignment buttons for discharged patients.
Also applies to: 530-530
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
src/components/Patient/Utils.ts (1)
20-29: Add JSDoc documentation for the functionAdding documentation would improve maintainability and help other developers understand the function's purpose and behavior.
Consider adding JSDoc documentation:
+/** + * Determines if a patient is considered discharged based on their active status + * and facility assignment. + * @param patientData - The patient data object + * @param facilityId - The current facility ID to check against + * @returns true if the patient is discharged or not in the current facility + */ export const isPatientDischarged = ( patientData: PatientModel, facilityId: string | undefined, ) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/components/Patient/PatientHome.tsx(3 hunks)src/components/Patient/Utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Patient/PatientHome.tsx
🔇 Additional comments (2)
src/components/Patient/Utils.ts (2)
20-23: 🛠️ Refactor suggestion
Consider handling undefined facilityId explicitly
The function signature allows undefined for facilityId, but the implementation doesn't explicitly handle this case.
Consider this safer implementation:
export const isPatientDischarged = (
patientData: PatientModel,
facilityId: string | undefined,
) => {Likely invalid or redundant comment.
20-29: Verify integration with patient discharge workflow
The function correctly implements the discharge check based on active status and facility ID, but let's verify its integration with the broader patient discharge workflow.
✅ Verification successful
Function is correctly integrated with patient discharge workflow
The verification shows that isPatientDischarged is well-integrated with the patient discharge workflow:
- It's used in
PatientHome.tsxto disable actions when a patient is discharged - The function's logic aligns with the broader discharge handling in the codebase:
- Checks for
is_activestatus matches how it's used throughout the application - Facility ID comparison is consistent with other facility checks in the codebase
- Checks for
- The implementation correctly supports the UI states in
PatientHome.tsxand integrates with the discharge-related features
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other discharge-related logic to ensure consistency
echo "Searching for related discharge logic..."
rg -t typescript -t javascript "is_active|discharge" --glob "!{test,__tests__}/*"
# Search for other facility ID comparisons
echo "Searching for similar facility ID checks..."
rg -t typescript -t javascript "facility.*id.*toString" --glob "!{test,__tests__}/*"
Length of output: 428
Script:
#!/bin/bash
# Search for discharge-related logic with correct file extensions
echo "Searching for related discharge logic..."
rg "is_active|discharge" --type-add 'ts:*.{ts,tsx}' -t ts
# Search for facility ID comparisons
echo "Searching for similar facility ID checks..."
rg "facility.*id.*toString" --type-add 'ts:*.{ts,tsx}' -t ts
# Look for usages of isPatientDischarged function
echo "Searching for usages of isPatientDischarged..."
rg "isPatientDischarged" --type-add 'ts:*.{ts,tsx}' -t ts
Length of output: 29722
| const patientFacilityId = patientData.facility_object?.id; | ||
| if (!patientFacilityId || !facilityId) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
- why do we have to check if facility ID matches?
- Shouldn't we instead check if the
last_consultation.discharge_datehas a value or not?
Well, for a patient who's readmitted, is_active check alone won't suffice. For example:
I just saw Nihal's reply here: #9289 (comment). So depends on how we address this. |
|
expected behavior has been modified, closing the PR as it's not solving the core issue |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Bug Fixes