Perms based Rendering #10526
Perms based Rendering #10526
Conversation
WalkthroughThis pull request introduces extensive permission and localization updates across the application. It adds several new keys to the English localization file while removing a legacy permission check. Many UI components, pages, and type definitions now incorporate permission-related props and hooks to conditionally render content, execute queries, and handle user feedback via toast notifications and redirection. Overall, the changes modernize permission handling and streamline localization for improved clarity and control flow. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as Component
participant P as usePermissions Hook
participant G as getPermissions Function
participant Q as Query/Action Handler
U->>C: Access component (e.g. appointment or encounter)
C->>P: Retrieve current user permissions
P-->>C: Return hasPermission flag
C->>G: Evaluate permissions (e.g. canAccess, canWrite)
G-->>C: Return permission flags
alt Has Required Permission
C->>Q: Execute queries / Render UI elements
Q-->>C: Return data / Display content
else Insufficient Permission
C->>C: Display error toast
C->>U: Redirect to previous page
end
Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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. |
|
Conflicts have been detected against the base branch. Please merge the base branch into your branch.
|
|
Conflicts have been detected against the base branch. Please merge the base branch into your branch.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/components/Patient/PatientDetailsTab/patientUpdates.tsx (1)
71-79:⚠️ Potential issueAdd permission check for Add Updates button.
The "Add Patient Updates" button should be conditionally rendered based on permissions.
- <Button asChild variant="outline_primary"> + {canCreatePatientQuestionnaireResponses && ( + <Button asChild variant="outline_primary"> <Link href={`/facility/${facilityId}/patient/${patientId}/questionnaire`} > <CareIcon icon="l-plus" className="mr-2" /> {t("add_patient_updates")} </Link> - </Button> + </Button> + )}
🧹 Nitpick comments (28)
src/components/Files/ArchivedFileDialog.tsx (1)
38-42: LGTM! Good implementation of accessible tooltip.The implementation effectively handles long filenames with truncation and tooltip, while maintaining good accessibility practices.
Consider adding
titleattribute to the span for native browser tooltip support as a fallback:- <span className="max-w-[200px] truncate">{fileName}</span> + <span className="max-w-[200px] truncate" title={fileName}>{fileName}</span>src/types/emr/encounter.ts (1)
169-173: Add type safety and documentation to the new constant.While the implementation is correct, consider these improvements:
- Add JSDoc documentation explaining the purpose and usage of this constant
- Add type safety using
as constandEncounterStatus+/** + * Represents encounter statuses that indicate the encounter is no longer active. + * This includes completed, discharged, cancelled, and erroneously entered encounters. + */ -export const inactiveEncounterStatus = [ +export const inactiveEncounterStatus = [ ...completedEncounterStatus, "cancelled", "entered_in_error", -]; +] as const satisfies readonly EncounterStatus[];src/pages/Encounters/tabs/EncounterPlotsTab.tsx (1)
47-53: Enhance the error message for better user experience.While the error handling logic is correct, consider providing more context in the error message to help users understand why they can't access the encounter.
- toast.error("You do not have permission to view this encounter"); + toast.error(t("errors.encounter_access_denied", { + reason: "Insufficient permissions to view clinical data or encounter details" + }));src/components/ui/sidebar/facility-nav.tsx (1)
8-12: Remove or guard console logs in production code.Logging user permissions (
console.log(authUser.permissions)) may leak information in production. Consider removing or gating this behind a development-only flag for better security and cleanliness.- console.log(authUser.permissions); + if (process.env.NODE_ENV !== "production") { + console.log(authUser.permissions); + }src/components/Patient/PatientDetailsTab/index.tsx (2)
33-62: BASE_PATIENT_TABS usage note.Defining the base tabs in a single constant is tidy. Over time, ensure that new patient-facing tabs also integrate permission gating or a suitable default, matching the pattern established here.
99-144: “getTabs” function elegantly filters based on route-specific checks.Using a switch statement in
getTabVisibilityis fine, though consider a data-driven map if the number of routes grows substantially. The filtering approach is efficient and maintains clarity.src/components/Patient/diagnosis/list.tsx (1)
156-156: Hover styling and text checks.Using the
underlinedlink style for “edit” is consistent with other interactive elements. This is minor but contributes to consistent UI.src/components/Medicine/MedicationRequestTable/index.tsx (1)
114-122: User feedback on insufficient permissions.
Displaying a toast and navigating back is good for user experience. Consider a fallback view if partial access is desired, but otherwise the approach is fine.src/components/Facility/ConsultationDetails/OverviewSideBar.tsx (1)
10-13: Consider adding type validation for the canAccess prop.The
canAccessprop is added to the Props interface but lacks runtime validation. Consider using a prop-types library or zod for runtime type checking.src/components/Patient/PatientDetailsTab/HealthProfileSummary.tsx (1)
29-39: Consider refactoring the useEffect dependencies.The ESLint disable comment suggests potential issues with the effect dependencies. Consider including all required dependencies or extracting the navigation logic to a separate effect.
- // eslint-disable-next-line react-hooks/exhaustive-deps - }, [canViewClinicalData]); + }, [canViewClinicalData, goBack, facilityId, patientId]);src/pages/Encounters/tabs/EncounterUpdatesTab.tsx (1)
32-38: Consider refactoring the useEffect dependencies.The ESLint disable comment suggests potential issues with the effect dependencies. Consider including all required dependencies.
- // eslint-disable-next-line react-hooks/exhaustive-deps - }, [canAccess]); + }, [canAccess, goBack]);src/pages/Facility/overview.tsx (1)
31-46: Add type safety for shortcuts array.Consider defining an interface for the shortcuts to ensure type safety and better maintainability.
+interface Shortcut { + title: string; + description: string; + icon: LucideIcon; + href: string; + visible: boolean; +} -const shortcuts = [ +const shortcuts: Shortcut[] = [src/components/Patient/PatientDetailsTab/EncounterHistory.tsx (1)
53-59: Consider adding missing dependencies to useEffect.While the permission check and redirection logic are well implemented, the useEffect dependency array is incomplete. Consider adding the following dependencies:
t(translation function)goBackfacilityIdpatientId- // eslint-disable-next-line react-hooks/exhaustive-deps - }, [canViewPatients]); + }, [canViewPatients, t, goBack, facilityId, patientId]);src/components/Users/UserHome.tsx (1)
46-49: Consider making the permission check more explicit.While the implementation is correct, the permission check in the TABS object could be more explicit for better readability.
- hidden: !props.facilityId || !canViewSchedule, + hidden: !props.facilityId || !canViewSchedule, // Hide if no facility or no schedule permissionAlso applies to: 79-79
src/components/Patient/symptoms/list.tsx (1)
51-58: Consider adding error handling for unauthorized access.While the query is correctly disabled based on permissions, there's no user feedback when access is denied.
Consider adding an error toast and redirect similar to other components:
+ useEffect(() => { + if (!canAccess) { + toast.error(t("no_permission_to_view_page")); + goBack(); + } + }, [canAccess]);src/components/Patient/PatientDetailsTab/Appointments.tsx (1)
90-100: Consider extracting button component.The schedule appointment button logic could be reused across the application.
Consider creating a reusable component:
+const ScheduleAppointmentButton = ({ facilityId, patientId }: { facilityId: string; patientId: string }) => ( + <Button variant="outline_primary" asChild> + <Link + href={`/facility/${facilityId}/patient/${patientId}/book-appointment`} + className="flex items-center justify-center w-full sm:w-auto" + > + <CareIcon icon="l-plus" className="mr-2" /> + {t("schedule_appointment")} + </Link> + </Button> +);src/components/Patient/MedicationStatementList.tsx (1)
179-187: Remove unnecessary fragment.The fragment wrapper is unnecessary since there's only one child element.
- <> <MedicationRow key={statement.id} statement={statement} isEnteredInError={isEnteredInError} index={index} /> - </>src/components/Users/UserSummary.tsx (1)
45-58: Consider enhancing error handling in the deleteUser mutation.The error handling could be more specific to help users understand and resolve issues.
onError: () => { setShowDeleteDialog(false); - toast.error(t("user_delete_error")); + toast.error((error) => { + if (error instanceof HTTPError) { + return t("user_delete_error_with_reason", { reason: error.message }); + } + return t("user_delete_error"); + }); },src/pages/Facility/settings/organizations/FacilityOrganizationUsers.tsx (1)
54-59: Consider extracting permission checks to a custom hook.The permission check logic could be encapsulated in a custom hook to improve reusability and reduce duplication.
// src/hooks/useFacilityPermissions.ts export const useFacilityPermissions = () => { const authUser = useAuthUser(); const { hasPermission } = usePermissions(); return getPermissions(hasPermission, authUser.permissions); };src/pages/Encounters/EncounterShow.tsx (2)
57-62: Consider using a custom hook for encounter permissions.Similar to the previous suggestion, the permission check logic could be encapsulated in a custom hook.
// src/hooks/useEncounterPermissions.ts export const useEncounterPermissions = () => { const authUser = useAuthUser(); const { hasPermission } = usePermissions(); return getPermissions(hasPermission, authUser.permissions); };
35-45: Consider using an enum for tab identifiers.Using string literals for tab identifiers could lead to typos. Consider using an enum for type safety.
enum EncounterTab { Updates = 'updates', Plots = 'plots', Medicines = 'medicines', Files = 'files', Notes = 'notes', } const defaultTabs: Record<EncounterTab, React.FC<EncounterTabProps>> = { [EncounterTab.Updates]: EncounterUpdatesTab, [EncounterTab.Plots]: EncounterPlotsTab, [EncounterTab.Medicines]: EncounterMedicinesTab, [EncounterTab.Files]: EncounterFilesTab, [EncounterTab.Notes]: EncounterNotesTab, };src/pages/Patients/VerifyPatient.tsx (1)
86-91: Consider adding loading state before redirection.The immediate redirection on permission check might cause a flash of content. Consider adding a loading state while checking permissions.
useEffect(() => { + const [isChecking, setIsChecking] = useState(true); if (!canPerformActions) { toast.error(t("no_permission_to_view_page")); goBack("/"); + } else { + setIsChecking(false); } }, [canPerformActions]); +if (isChecking) { + return <Loading />; +}src/common/Permissions.tsx (2)
78-184: Consider using TypeScript enums for permission slugs.The interface is well-documented with JSDoc comments, but consider using enums for permission slugs to prevent typos and enable better type checking.
+export enum PermissionSlug { + CREATE_PATIENT = "can_create_patient", + WRITE_PATIENT = "can_write_patient", + // ... other permissions +} export interface Permissions { /** Permission slug: "can_create_patient" */ - canCreatePatient: boolean; + [PermissionSlug.CREATE_PATIENT]: boolean; // ... other permissions }
186-350: Consider memoizing the getPermissions result.The
getPermissionsfunction creates a new object on every call. Consider memoizing the result to prevent unnecessary re-renders.+import { useMemo } from 'react'; -export function getPermissions( +export function usePermissions( hasPermission: HasPermissionFn, permissions: string[], ): Permissions { + return useMemo( + () => ({ // ... existing implementation + }), + [hasPermission, permissions] + ); }src/components/Facility/FacilityHome.tsx (1)
260-317: Remove TODO comment about backend permissions.The code contains a TODO comment about getting permissions from the backend, but the permissions system is already implemented. This comment should be removed or updated.
- {/* TODO: get permissions from backend */} - {/* {hasPermissionToDeleteFacility && ( - <DropdownMenuItem - className="text-destructive" - onClick={() => setOpenDeleteDialog(true)} - > - <Trash2 className="mr-2 h-4 w-4" /> - {t("delete_facility")} - </DropdownMenuItem> - )} */}src/pages/Appointments/AppointmentsPage.tsx (1)
345-350: Consider adding loading state during redirection.The permission-based redirection is implemented correctly, but it might benefit from a loading state to prevent flash of unauthorized content.
+ const [isLoading, setIsLoading] = useState(true); useEffect(() => { if (!canViewAppointments) { toast.error(t("no_permission_to_view_page")); goBack("/"); + } else { + setIsLoading(false); } }, [canViewAppointments]); + + if (isLoading) { + return <Loading />; + }src/components/Files/FilesTab.tsx (2)
232-239: Simplify the editPermission function.The function can be simplified using a switch statement or object literal for better maintainability.
Consider this refactor:
const editPermission = () => { - if (type === "encounter") { - return canWriteCurrentEncounter; - } else if (type === "patient") { - return canWritePatient; - } - return false; + const permissions = { + encounter: canWriteCurrentEncounter, + patient: canWritePatient, + }; + return permissions[type] ?? false; };
507-518: Consider using CSS Grid for table header widths.Using fixed percentage widths might cause layout issues with different content lengths. CSS Grid would provide more flexibility.
Consider using CSS Grid with
minmax()for more responsive column widths:-<TableHead className="w-[20%] bg-white rounded-l"> +<TableHead className="min-w-[150px] bg-white rounded-l">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (47)
public/locale/en.json(4 hunks)src/Utils/permissions.ts(0 hunks)src/common/Permissions.tsx(1 hunks)src/components/Common/Charts/ObservationChart.tsx(3 hunks)src/components/Facility/ConsultationDetails/ObservationsList.tsx(3 hunks)src/components/Facility/ConsultationDetails/OverviewSideBar.tsx(2 hunks)src/components/Facility/ConsultationDetails/QuestionnaireResponsesList.tsx(5 hunks)src/components/Facility/EncounterCard.tsx(3 hunks)src/components/Facility/FacilityHome.tsx(5 hunks)src/components/Files/ArchivedFileDialog.tsx(2 hunks)src/components/Files/FilesTab.tsx(7 hunks)src/components/Medicine/MedicationAdministration/AdministrationTab.tsx(12 hunks)src/components/Medicine/MedicationRequestTable/index.tsx(7 hunks)src/components/Patient/MedicationStatementList.tsx(3 hunks)src/components/Patient/PatientDetailsTab/Appointments.tsx(3 hunks)src/components/Patient/PatientDetailsTab/Demography.tsx(4 hunks)src/components/Patient/PatientDetailsTab/EncounterHistory.tsx(3 hunks)src/components/Patient/PatientDetailsTab/HealthProfileSummary.tsx(2 hunks)src/components/Patient/PatientDetailsTab/PatientFiles.tsx(0 hunks)src/components/Patient/PatientDetailsTab/PatientUsers.tsx(4 hunks)src/components/Patient/PatientDetailsTab/index.tsx(4 hunks)src/components/Patient/PatientDetailsTab/patientUpdates.tsx(3 hunks)src/components/Patient/PatientHome.tsx(4 hunks)src/components/Patient/PatientIndex.tsx(3 hunks)src/components/Patient/allergy/list.tsx(7 hunks)src/components/Patient/diagnosis/list.tsx(4 hunks)src/components/Patient/symptoms/list.tsx(5 hunks)src/components/Questionnaire/QuestionnaireForm.tsx(2 hunks)src/components/Users/UserAvailabilityTab.tsx(5 hunks)src/components/Users/UserHome.tsx(3 hunks)src/components/Users/UserSummary.tsx(5 hunks)src/components/ui/sidebar/facility-nav.tsx(4 hunks)src/components/ui/sidebar/nav-main.tsx(1 hunks)src/pages/Appointments/AppointmentDetail.tsx(9 hunks)src/pages/Appointments/AppointmentsPage.tsx(11 hunks)src/pages/Encounters/EncounterShow.tsx(3 hunks)src/pages/Encounters/tabs/EncounterMedicinesTab.tsx(1 hunks)src/pages/Encounters/tabs/EncounterNotesTab.tsx(7 hunks)src/pages/Encounters/tabs/EncounterPlotsTab.tsx(4 hunks)src/pages/Encounters/tabs/EncounterUpdatesTab.tsx(4 hunks)src/pages/Facility/overview.tsx(2 hunks)src/pages/Facility/settings/organizations/FacilityOrganizationUsers.tsx(4 hunks)src/pages/Facility/settings/organizations/components/EditFacilityUserRoleSheet.tsx(4 hunks)src/pages/Organization/OrganizationUsers.tsx(5 hunks)src/pages/Organization/components/EditUserRoleSheet.tsx(3 hunks)src/pages/Patients/VerifyPatient.tsx(4 hunks)src/types/emr/encounter.ts(1 hunks)
💤 Files with no reviewable changes (2)
- src/Utils/permissions.ts
- src/components/Patient/PatientDetailsTab/PatientFiles.tsx
🔇 Additional comments (141)
src/components/Files/ArchivedFileDialog.tsx (1)
10-10: LGTM! Good refactoring of tooltip imports.The consolidation of multiple tooltip components into a single
TooltipComponentimproves code maintainability and reusability.src/types/emr/encounter.ts (1)
169-173: Verify the usage of this new constant.Let's ensure this constant is being used consistently across the codebase.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Find all usages of inactiveEncounterStatus and related encounter status checks # Search for direct usage of inactiveEncounterStatus echo "Direct usage of inactiveEncounterStatus:" rg "inactiveEncounterStatus" # Search for potential places where encounter status is being checked echo -e "\nPotential encounter status checks:" ast-grep --pattern 'encounter.status === $_' -l ast-grep --pattern 'status === $_' -lLength of output: 2149
Consistent Usage Verified for inactiveEncounterStatus
The search confirms that the
inactiveEncounterStatusconstant is imported and used consistently across multiple relevant files (e.g., in EncounterNotesTab, FilesTab, MedicationRequestTable, MedicationAdministration, Patient symptoms, allergy, and diagnosis lists). There are no discrepancies or outliers in how this constant is utilized.src/pages/Encounters/tabs/EncounterPlotsTab.tsx (3)
4-4: LGTM! Well-structured permission management setup.The new imports and hook declarations are well-organized and follow React best practices for implementing permission-based access control.
Also applies to: 6-6, 16-17, 20-20, 22-22, 32-38
34-39: Verify the permission logic requirements.The current implementation allows access if either
canViewClinicalDataORcanViewEncounteris true. Please confirm if this level of access control aligns with the security requirements.Let's check other encounter-related components to verify consistency in permission handling:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for similar permission checks in encounter components rg -A 3 "canViewClinicalData|canViewEncounter" "src/pages/Encounters"Length of output: 1946
Permission logic is consistently applied across encounter components.
Our search confirms that in multiple encounter-related tabs (i.e., EncounterNotesTab.tsx, EncounterPlotsTab.tsx, and EncounterUpdatesTab.tsx), the same logic—granting access if either
canViewClinicalDataORcanViewEncounteris true—is used. This consistency suggests that the intended design is to allow access when at least one permission is granted.
84-90: Verify ObservationVisualizer's handling of canAccess prop.The
canAccessprop is passed correctly, but we should verify its implementation in the ObservationVisualizer component.Let's check the ObservationVisualizer implementation:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for the ObservationVisualizer component definition ast-grep --pattern 'interface ObservationVisualizerProps { $$$ }'Length of output: 667
ObservationVisualizer's canAccess Prop Handling Verified
We confirmed that the
canAccessprop is correctly defined in the component’s props interface insrc/components/Common/Charts/ObservationChart.tsx:
- The
ObservationVisualizerPropsinterface (lines 50–57) declarescanAccess: boolean.- This indicates that the component accepts and is set up to use the
canAccessprop.Based on the search, there is no evidence of incorrect implementation regarding this prop. If further conditional behavior is expected (e.g., to restrict rendering when access is not permitted), please ensure this logic is implemented in the component’s render function, but as currently implemented, the prop is correctly passed and typed.
src/components/ui/sidebar/facility-nav.tsx (6)
18-18: Interface property appropriately reflects optional display logic.Adding
visibility?: boolean;is a logical way to allow or hide navigation links based on permissions. This is aligned with best practices for dynamic elements.
28-35: Additional parameter for permissions is consistent and extensible.Passing permissions explicitly to
generateFacilityLinksclarifies the function's dependencies and keeps the code flexible for future enhancements.
45-45: Effective use of permission-based visibility.Conditionally displaying the "appointments" link is a good approach to ensure that only users with the correct permission can view this section.
51-55: Re-evaluate combined visibility logic.You're combining three different permissions (
canCreateAppointment,canListEncounters, andcanCreateEncounter) to control thesearch_patientslink. Verify that all usage scenarios are correct and that authorized users can see the link without unintended exposure for unauthorized users.
56-60: Correct gating of "encounters" link.Visibility of the "encounters" link is solely based on
canListEncounters. This is consistent with standard read permission gating.
80-100: Unused permission property.
canViewEncounteris destructured into thepermissionsobject but not actually used in generating links within this component. Verify that this is intentional or remove it to minimize confusion.src/components/Patient/PatientDetailsTab/index.tsx (5)
7-7: Centralizing permission logic.Importing
getPermissionshere promotes consistency across the app. Ensure any upstream changes to permission constants or function parameters won't break usage.
22-26: Interface “Tab” looks good for typed tab definitions.Optional
visibleproperty provides a clean approach for gating tab displays.
28-31: Separate “Tabs” interface aids maintainability.Providing a clear structure for separated patient vs. facility tabs is beneficial for clarity and future extension.
64-97: BASE_FACILITY_TABS maintains parallel structure.Mirroring the same tab approach for facility contexts provides consistency across user flows.
146-148: Aliases for router usage.Re-exporting
BASE_PATIENT_TABSandBASE_FACILITY_TABSaspatientTabsandfacilityPatientTabsensures existing imports remain valid. This is a smooth transitional approach.src/components/Patient/diagnosis/list.tsx (8)
12-15: Use of new hooks ensures consistent authentication flow.Fetching
authUserandhasPermissionaligns with the broader permission-based rendering strategy.
17-19: Encounter status usage.Importing
encounterStatusdefinitions from a central source is best practice, ensuring consistent usage across multiple components.
27-27: Adding “encounterStatus” prop.Allows the component to handle or block editing actions for inactive encounters, which is a common scenario in medical workflows.
34-44: Comprehensive permission checks.
canAccessandcanEditadequately differentiate between read and write flow. The logic is clear and ties in well with theencounterStatus.
45-45: Conditional data fetching.
enabled: canAccessensures queries only run if the user has permission to view data, reducing unnecessary calls and preventing unauthorized requests.
90-90: Passing “canEdit” into the layout.Propagating the can-edit state ensures consistent logic for child components or displayed links.
104-105: Conditional rendering the edit link, especially for inactive encounters.You correctly check
encounterStatusand gating logic. This is a common EMR requirement to prevent workflows on closed or inactive encounters.
144-145: Optional canEdit prop.Defaulting to false prevents accidental enabling of the edit logic if the parent forgets to pass this prop.
src/components/Medicine/MedicationRequestTable/index.tsx (19)
5-6: Imports look good.
No issues with addinguseEffect,useState, andtoast. They are used appropriately in the file.
18-19: New hook imports.
ImportinguseAppHistoryanduseAuthUseris appropriate to handle navigation and user data.
21-21: Permissions import.
Bringing ingetPermissionsaligns with the updated permission checks.
24-25: Context and types import.
UsingusePermissionsand referencingEncounteris consistent with the new approach.
69-69: Prop change fromencounterIdtoencounter.
Switching to the fullencounterobject allows for better data handling.
74-74: Auth hook usage.
TheuseAuthUser()call is a clean way to retrieve the logged-in user data.
75-80: Permission checks.
ExtractingcanViewClinicalData,canViewEncounter, andcanSubmitEncounterQuestionnairefromgetPermissionsis clear and maintainable.
81-81: Access variable.
canAccess = canViewClinicalData || canViewEncounterhelps unify the permission check into one boolean.
82-82: Navigation hook usage.
goBackis a straightforward approach for returning the user to the previous context when permissions are lacking.
84-87:canWriteCurrentEncounterlogic.
Ensuring the encounter is active before enabling the write action is well-handled here.
93-93: Query parameter updated.
ReplacingencounterIdwithencounter.idis consistent with the new prop.
98-98: Query enable condition.
enabled: !!patientId && canAccesscorrectly gates data fetching behind permission and valid patient ID.
103-103: Stopped medications query param.
Same pattern used withencounter.id—consistency maintained.
106-106: Same enable condition for stopped meds.
Ensures both queries are uniformly permission-gated.
147-147: Tabs list stylistic change.
Applyingbg-gray-200andpy-0is consistent with existing UI patterns.
163-163: Responsive layout update.
Addingflex-wrapmanages small screens effectively.
185-199: Conditional edit button.
Restricting the “Edit” button to users who have write access on the current encounter is a solid approach.
207-207: Print link updated.
Switching toencounter.idensures consistency with newly passed props.
253-257: Passing IDs and status toAdministrationTab.
UsingencounterId={encounter.id}andencounterStatus={encounter.status}is consistent with the new design.src/components/Facility/ConsultationDetails/QuestionnaireResponsesList.tsx (13)
4-4: Effect import.
useEffectsupports side effects, matching the new permission-based redirect logic.
6-6: Toast dependency.
Usingtoastto notify the user about permission issues is consistent with the global notification pattern.
17-18: History and auth hooks.
useAppHistoryanduseAuthUserremain consistent with the rest of the app for navigation and user data.
20-20: ImportinggetPermissions.
Keeps the permission logic encapsulated in a shared utility.
26-26:usePermissionsimport.
Matches the overall approach to consistent permission context usage.
35-35: New optionalfacilityIdprop.
Allows for facility-specific redirection behavior. Good improvement for route management.
297-298: Retrieving user permissions.
authUserandhasPermissionare essential for the subsequentgetPermissionscall.
299-303: Extracting relevant permissions.
canViewPatientQuestionnaireResponsesdifferentiates between encounter-level and patient-level access.
304-304:goBackfromuseAppHistory.
Used to navigate away when permission is denied.
306-308: ConditionalcanAccess.
Considers both encounter-level permissions and fallback to patient-level permissions.
320-320: Enable condition for questionnaire queries.
Ensuring queries only run whencanAccessistrueprevents unauthorized data fetching.
322-324: Comment clarifying redirect logic.
Provides a note about redirection behavior for patient tabs.
326-336: Redirecting without encounter if access fails.
TheuseEffectblock properly informs the user and navigates them away when permissions are invalid.src/pages/Encounters/tabs/EncounterMedicinesTab.tsx (1)
11-11: Prop update to pass the fullencounter.
ReplacingencounterIdwithencounterharmonizes with the new data flow inMedicationRequestTable.src/components/Facility/ConsultationDetails/OverviewSideBar.tsx (1)
35-38: LGTM! Proper prop drilling implementation.The
canAccessprop is correctly passed down to the ObservationsList component, maintaining the permission check chain.src/components/ui/sidebar/nav-main.tsx (3)
20-25: LGTM! Well-structured type definition.The optional visibility property is properly typed and integrated into the links interface.
32-34: LGTM! Efficient filtering implementation.The filter implementation correctly handles the visibility check, only excluding links where visibility is explicitly false.
42-46: LGTM! Proper mobile sidebar handling.The mobile sidebar state is correctly managed using the isMobile check and setOpenMobile callback.
src/components/Patient/PatientDetailsTab/HealthProfileSummary.tsx (2)
21-26: LGTM! Comprehensive permission check implementation.The permission check is properly implemented using the authUser and hasPermission hooks.
53-56: LGTM! Proper prop passing to MedicationStatementList.The canAccess prop is correctly passed to control access to medication data.
src/pages/Encounters/tabs/EncounterUpdatesTab.tsx (2)
23-30: LGTM! Well-structured permission management.The permission checks are properly implemented using a combination of canViewClinicalData and canViewEncounter.
87-87: LGTM! Proper prop passing to SideOverview.The canAccess prop is correctly passed to control access to the overview data.
src/pages/Facility/overview.tsx (1)
14-16: Well-implemented permission checks!The permission management is cleanly implemented using the new hooks and utilities. The code properly destructures the required permissions and uses them effectively.
Also applies to: 25-29
src/components/Facility/EncounterCard.tsx (1)
26-31: Clean implementation of permission-based rendering!The permission check and conditional rendering are well implemented. The code properly prevents unauthorized access to encounter details.
Also applies to: 105-117
src/components/Patient/PatientDetailsTab/EncounterHistory.tsx (1)
41-51: Well-implemented query permission check!The query's enabled property is properly tied to the permission check, preventing unnecessary data fetching for unauthorized users.
src/components/Users/UserHome.tsx (1)
72-81: Excellent type-safe implementation with satisfies operator!The TABS object implementation with the satisfies operator ensures type safety while maintaining flexibility.
src/components/Patient/symptoms/list.tsx (2)
38-50: LGTM! Well-structured permission checks.The permission logic effectively combines user permissions with encounter status to determine access and edit capabilities.
154-162: Verify edit link visibility logic.The edit link visibility depends on multiple conditions. Let's verify this implementation across the codebase.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for similar permission-based edit link patterns rg -A 5 'canEdit.*\&\&.*Link'Length of output: 31
Edit Link Visibility Condition Verified
After re-running our search with a multiline-capable regex, we confirmed that the edit link insrc/components/Patient/symptoms/list.tsxis consistently rendered only when all three conditions—facilityId,encounterId, andcanEdit—are truthy. There are no conflicting patterns elsewhere in the codebase that suggest alternative handling of this logic.src/components/Patient/PatientDetailsTab/Appointments.tsx (1)
53-59: LGTM! Proper error handling for unauthorized access.The useEffect hook correctly handles unauthorized access by showing an error message and redirecting the user.
src/components/Patient/MedicationStatementList.tsx (1)
27-30: LGTM! Clear prop interface.The canAccess prop is well-defined and mandatory, ensuring proper permission checks.
src/components/Patient/PatientDetailsTab/patientUpdates.tsx (1)
56-66: LGTM! Comprehensive permission check handling.The useEffect hook properly handles unauthorized access with appropriate error message and dynamic redirection based on facilityId.
src/components/Users/UserSummary.tsx (1)
42-43: LGTM! Clean permission management implementation.The use of
usePermissionshook andgetPermissionsfunction provides a clear and maintainable way to handle user permissions.src/components/Facility/ConsultationDetails/ObservationsList.tsx (2)
19-22: LGTM! Clear and type-safe props interface.The addition of the
canAccessboolean prop provides a clean way to control data fetching permissions.
83-105: LGTM! Well-implemented conditional data fetching.The
enabledcondition in useInfiniteQuery effectively prevents unnecessary data fetching when access is not permitted.src/pages/Facility/settings/organizations/FacilityOrganizationUsers.tsx (1)
84-105: LGTM! Clean conditional rendering implementation.The permission-based conditional rendering of user management components is well-implemented and maintainable.
src/pages/Encounters/EncounterShow.tsx (1)
69-78: LGTM! Well-implemented conditional data fetching.The query's
enabledcondition effectively combines both the ID check and permission check.src/pages/Organization/OrganizationUsers.tsx (4)
14-20: LGTM!The permission-related imports are well-organized and necessary for implementing permission-based rendering.
40-62: LGTM!The permission hooks are correctly initialized, and the permission checks are properly derived using
getPermissions.
81-92: LGTM!The conditional rendering of
AddUserSheetbased oncanCreateUserpermission is correctly implemented.
93-102: LGTM!The conditional rendering of
LinkUserSheetandEditUserRoleSheetbased oncanManageOrganizationUserspermission is correctly implemented.Also applies to: 175-195
src/pages/Organization/components/EditUserRoleSheet.tsx (2)
40-47: LGTM!The permission-related imports are well-organized and necessary for implementing permission-based rendering.
128-130: LGTM!The permission hooks are correctly initialized, and the conditional rendering of the edit user button based on
canCreateUserpermission is properly implemented.Also applies to: 254-263
src/pages/Facility/settings/organizations/components/EditFacilityUserRoleSheet.tsx (3)
38-46: LGTM!The permission-related imports are well-organized and necessary for implementing permission-based rendering.
135-137: LGTM!The permission hooks are correctly initialized, and the permission check is properly derived using
getPermissions.
259-268: LGTM!The conditional rendering of the edit user button based on
canCreateUserpermission is correctly implemented.src/components/Patient/PatientIndex.tsx (3)
35-42: LGTM!The permission-related imports are well-organized and necessary for implementing permission-based rendering.
53-58: LGTM!The permission hooks are correctly initialized, and the permission check is properly derived using
getPermissions.
145-149: LGTM!The conditional rendering of
AddPatientButtonand text based oncanCreatePatientpermission is correctly implemented, with proper localization using the translation function.Also applies to: 155-159
src/components/Patient/PatientDetailsTab/Demography.tsx (4)
12-19: LGTM! Clean import organization.The imports are well-organized, with related hooks and utilities grouped together.
29-34: LGTM! Proper permission setup.The permission setup follows best practices:
- Uses hooks to get auth user and permissions
- Destructures only the needed permission from
getPermissions
137-137: LGTM! Dynamic permission check for edit access.The
allowEditproperty is now properly derived from user permissions instead of being hardcoded.
262-262: Verify edit functionality with disabled state.The edit button's
onClickhandler is simplified, but ensure it works correctly with thedisabledstate whenpatientData.death_datetimeis set.❌ Verification inconclusive
🏁 Script executed:
#!/bin/bash # Search for tests covering the edit button's disabled state rg -A 5 "death_datetime.*disabled" --type=testLength of output: 77
Edit button disabled state requires manual verification
Our initial search (usingrg -A 5 "death_datetime.*disabled" --type=test) did not yield any automated tests or context confirming that the edit button is actually disabled whenpatientData.death_datetimeis set. Please verify manually—either by reviewing the rendered component behavior in the UI or by adding automated tests—to ensure that clicking the edit button is indeed prevented when the patient is deceased.
- Location to review:
src/components/Patient/PatientDetailsTab/Demography.tsx(line 262, where theonClick={() => handleEditClick(subtab.id)}is defined).- Next step: Verify that if
patientData.death_datetimeis present, the edit action is disabled (e.g., the button should not be clickable or trigger the edit function).src/components/Patient/PatientHome.tsx (4)
20-32: LGTM! Clean import organization.The imports are well-organized, with related hooks and utilities grouped together.
43-52: LGTM! Comprehensive permission setup.The permission setup is well-structured:
- Uses hooks to get auth user and permissions
- Destructures needed permissions from
getPermissions- Properly handles both facility and patient-level permissions
68-68: LGTM! Dynamic tab rendering.The tabs are now dynamically determined based on facility context and permissions.
81-81: LGTM! Proper permission check for appointment scheduling.The appointment scheduling button is correctly gated behind both facility context and permission check.
src/components/Patient/allergy/list.tsx (4)
34-40: LGTM! Clean import organization.The imports are well-organized, with related hooks and utilities grouped together.
82-94: LGTM! Comprehensive permission setup.The permission setup is well-structured:
- Uses hooks to get auth user and permissions
- Properly handles both encounter and clinical data permissions
- Correctly checks encounter status for edit permissions
106-106: LGTM! Permission-gated query execution.The query is correctly enabled only when the user has access permission.
318-319: LGTM! Clean prop interface update.The
AllergyListLayoutcomponent's props are properly updated:
- Added optional
canEditprop with default value- Maintains backward compatibility
Also applies to: 324-325
src/components/Patient/PatientDetailsTab/PatientUsers.tsx (4)
41-49: LGTM! Clean import organization.The imports are well-organized, with related hooks and utilities grouped together.
216-221: LGTM! Proper permission setup.The permission setup follows best practices:
- Uses hooks to get auth user and permissions
- Destructures only the needed permission from
getPermissions
279-313: LGTM! Well-structured user management UI.The user management UI is properly gated behind permissions:
- Remove user button is only shown to users with write permission
- Uses AlertDialog for confirmation
- Proper error handling in place
339-339: LGTM! Permission-gated add user functionality.The add user functionality is correctly gated behind write permission.
src/pages/Patients/VerifyPatient.tsx (3)
25-48: LGTM! Well-structured permission setup.The permission setup is clean and follows best practices:
- Hooks are properly imported and used
- Permission checks are aggregated into a single
canPerformActionsflag- Clear separation of concerns between auth, permissions, and rendering logic
137-224: LGTM! Excellent conditional rendering of quick actions.The implementation:
- Properly checks for required permissions
- Uses clear, descriptive translations
- Has good responsive design with proper spacing
- Includes proper accessibility attributes
226-259: LGTM! Well-structured encounters section.The encounters section:
- Properly checks for list permission
- Has good empty state handling
- Uses proper responsive design
- Includes proper accessibility attributes
src/common/Permissions.tsx (1)
1-76: LGTM! Well-organized permission constants.The permission constants are:
- Logically grouped by domain
- Follow consistent naming conventions
- Use clear, descriptive names
src/components/Common/Charts/ObservationChart.tsx (1)
56-56: LGTM! Clean implementation of permission-based data fetching.The changes:
- Add a clear
canAccessprop to control data fetching- Properly type the new prop in the interface
- Correctly use the prop to enable/disable the query
Also applies to: 107-107, 128-128
src/components/Facility/FacilityHome.tsx (1)
106-111: LGTM! Clean permission hooks setup.The permission hooks are properly set up and used to derive the
canUpdateFacilitypermission.src/components/Users/UserAvailabilityTab.tsx (4)
3-5: LGTM! Import statements are well-organized.The imports are logically grouped and include all necessary hooks and utilities for permission handling.
Also applies to: 27-29, 31-31, 39-39
74-80: LGTM! Permission setup is comprehensive.The permission setup correctly uses the
getPermissionsfunction to check if the user has the necessary permissions to view schedules.
84-91: LGTM! Queries are properly guarded.The
enabledproperty in both queries ensures they only execute when the user has permission to view schedules and the facility ID is available.Also applies to: 93-100
102-112: LGTM! Permission-based redirection is well implemented.The effect hook correctly handles unauthorized access by showing an error toast and redirecting the user. The ESLint disable comment is appropriate as the dependency array is intentionally limited.
src/components/Questionnaire/QuestionnaireForm.tsx (2)
228-232: LGTM! Enhanced validation logic for array values.The validation logic now correctly handles array values by checking their length, in addition to the existing checks for undefined, null, and empty string values.
300-300: LGTM! Improved resource ID assignment.The resource ID assignment now correctly falls back to
patientIdwhenencounterIdis not available.src/pages/Appointments/AppointmentDetail.tsx (5)
50-54: LGTM! Import statements are well-organized.The imports are logically grouped and include all necessary hooks and utilities for permission handling.
Also applies to: 64-64
85-91: LGTM! Permission setup is comprehensive.The permission setup correctly uses the
getPermissionsfunction to check if the user has the necessary permissions to view and update appointments.
102-111: LGTM! Query is properly guarded.The
enabledproperty ensures the query only executes when the user has permission to view appointments.
123-129: LGTM! Permission-based redirection is well implemented.The effect hook correctly handles unauthorized access by showing an error toast and redirecting the user. The ESLint disable comment is appropriate as the dependency array is intentionally limited.
210-222: LGTM! Conditional rendering is properly implemented.The
AppointmentActionscomponent is only rendered when the user has permission to update appointments.src/pages/Encounters/tabs/EncounterNotesTab.tsx (4)
52-54: LGTM! Import statements are well-organized.The imports are logically grouped and include all necessary hooks and utilities for permission handling.
Also applies to: 64-64
315-323: LGTM! Permission setup is comprehensive.The permission setup correctly uses the
getPermissionsfunction to check if the user has the necessary permissions. ThecanWriteCurrentEncountercheck also considers the encounter status.
326-333: LGTM! Queries are properly guarded.The
enabledproperty in both queries ensures they only execute when the user has the necessary permissions.Also applies to: 369-370
453-463: LGTM! UI elements are properly guarded with informative feedback.The UI elements are conditionally rendered based on permissions, and helpful feedback is provided when users lack permission or when the encounter status prevents actions.
Also applies to: 611-646, 667-673
src/components/Medicine/MedicationAdministration/AdministrationTab.tsx (4)
24-32: LGTM! Well-structured permission management implementation.The permission setup is comprehensive and follows best practices:
- Imports required hooks and utilities
- Properly extracts permissions using
getPermissions- Correctly combines encounter status with write permissions
Also applies to: 367-373
99-99: LGTM! Clear interface updates.The interface updates properly type the new properties:
encounterStatusinAdministrationTabPropscanWriteinMedicationRowPropsAlso applies to: 127-127
414-414: LGTM! Proper query access control.Queries are correctly gated with the
canAccesspermission check in theenabledproperty.Also applies to: 427-427, 455-455
313-322: LGTM! Proper conditional rendering based on permissions.The UI elements are correctly controlled based on permissions:
- Administer button visibility depends on
canWrite- Discontinue popover visibility depends on
canWrite- Medicine administration button visibility depends on
canWriteAlso applies to: 332-356, 786-795
src/pages/Appointments/AppointmentsPage.tsx (3)
257-262: LGTM! Well-structured permission setup.The permission setup follows best practices:
- Uses
usePermissionshook for permission checks- Uses
useAppHistoryfor navigation- Properly extracts
canViewAppointmentspermission
702-702: LGTM! Proper query access control.Queries are correctly gated with the
canViewAppointmentspermission check in theenabledproperty.Also applies to: 828-828
648-648: LGTM! Consistent permission prop passing.The
canViewAppointmentspermission is properly passed down to child components:
- AppointmentColumn
- AppointmentRow
Also applies to: 662-662, 677-677, 802-802
public/locale/en.json (4)
873-873: LGTM! Well-structured encounter notes message.The message is clear and uses proper template variable syntax for dynamic status insertion.
715-716: LGTM! Clear and concise quick action descriptions.The descriptions effectively communicate the purpose of each action to users.
2308-2308: LGTM! Simple and clear action text.The translation follows the established pattern for action-related keys.
1909-1909: LGTM! Clear search instruction text.The message effectively guides users on how to search for existing patients.
src/components/Files/FilesTab.tsx (2)
72-87: LGTM! Well-structured permission checks.The permission system is well-implemented with clear separation of concerns:
- Destructured permissions for better readability
- Separate checks for clinical data and encounter-specific permissions
- Proper handling of encounter status for write permissions
543-547: LGTM! Improved tooltip implementation.The tooltip implementation is cleaner and more maintainable with the new
TooltipComponent.
src/components/Files/FilesTab.tsx
Outdated
| export interface FilesTabProps { | ||
| type: "encounter" | "patient"; | ||
| facilityId: string; | ||
| patientId?: string; | ||
| encounter?: Encounter; | ||
| patient?: Patient; | ||
| } |
There was a problem hiding this comment.
Props interface could be more strict.
The patient prop is marked as optional but is used without proper null checks in the code. This could lead to runtime errors.
Apply this diff to fix the type definition:
export interface FilesTabProps {
type: "encounter" | "patient";
facilityId: string;
encounter?: Encounter;
- patient?: Patient;
+ patient: Patient;
}📝 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.
| export interface FilesTabProps { | |
| type: "encounter" | "patient"; | |
| facilityId: string; | |
| patientId?: string; | |
| encounter?: Encounter; | |
| patient?: Patient; | |
| } | |
| export interface FilesTabProps { | |
| type: "encounter" | "patient"; | |
| facilityId: string; | |
| encounter?: Encounter; | |
| patient: Patient; | |
| } |
| <TooltipComponent | ||
| content={dayjs(file.created_date).format( | ||
| "DD MMM YYYY, hh:mm A", | ||
| )} | ||
| > | ||
| <span> | ||
| {dayjs(file.created_date).format("DD MMM YYYY ")} | ||
| </span> | ||
| </TooltipComponent> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider user timezone for date display.
The date formatting doesn't account for user timezone, which could lead to confusion.
Add timezone support:
-dayjs(file.created_date).format("DD MMM YYYY ")
+dayjs(file.created_date).tz(userTimezone).format("DD MMM YYYY ")✅ Verification successful
🌐 Web query:
What is the recommended way to handle timezones in dayjs?
💡 Result:
To handle timezones effectively in Day.js, follow these recommended practices:
Core Setup
-
Install required plugins
npm install dayjs dayjs/plugin/utc dayjs/plugin/timezone
import dayjs from 'dayjs'; import utc from 'dayjs/plugin/utc'; import timezone from 'dayjs/plugin/timezone'; dayjs.extend(utc); dayjs.extend(timezone);
-
Store dates in UTC
Always persist timestamps in UTC format (e.g.,2023-11-18T03:55:00.000Z) in databases[1][6].
Timezone Operations
Convert between zones
// Parse input in specific zone
const nyTime = dayjs.tz("2023-11-18 11:55", "America/New_York");
// Convert existing date to different zone
const londonTime = dayjs("2023-11-18T12:00Z").tz("Europe/London");Set default timezone
dayjs.tz.setDefault("Asia/Tokyo");
const defaultZoneDate = dayjs.tz("2023-11-18 09:00"); // Uses TokyoHandle daylight saving automatically
const dstDate = dayjs.tz("2023-03-12 02:30", "America/Chicago");
// Automatically adjusts for DST transitionBest Practices
-
User-facing conversions
Only convert to local timezones when displaying dates:const userTime = storedUTCDate.tz(dayjs.tz.guess());
-
Explicit conversions
Avoid ambiguous handling:// ❌ Avoid (uses system TZ) dayjs("2023-11-18 12:00"); // ✅ Explicit dayjs.utc("2023-11-18 12:00").tz("Target/Zone");
-
Validation
Handle invalid zones gracefully:try { dayjs().tz("Invalid/Zone"); } catch (e) { console.error("Invalid timezone"); }
Common Pitfalls & Solutions
| Issue | Solution |
|---|---|
| Missing UTC plugin | Ensure utc plugin loads before timezone[2][6] |
| Default TZ not applying | Use dayjs.tz() without parameters[3] |
| DST edge cases | Test transitions with isDST() method[1] |
| Invalid zone errors | Use IANA-compliant zone names (e.g., "America/New_York")[10] |
For production systems, combine these techniques with proper error boundaries and consider adding logging for TZ conversion operations[9]. The modular design keeps bundle size minimal (~2kb core + ~20kb plugins)[4], making this approach suitable for both Node.js and browser environments.
Citations:
- 1: https://tillitsdone.com/blogs/day-js-timezone-guide-for-node-js/
- 2: https://day.js.org/docs/en/plugin/timezone
- 3: https://day.js.org/docs/en/timezone/set-default-timezone
- 4: https://www.freecodecamp.org/news/javascript-date-time-dayjs/
- 5: https://day.js.org/docs/en/timezone/converting-to-zone
- 6: https://blog.openreplay.com/working-with-dates-and-times-with-day-js/
- 7:
dayjs.tz()is building dates with incorrect timezone for many cases iamkun/dayjs#1827 - 8: https://dev.to/shameel/guide-to-learn-dayjs-36l9
- 9: https://laracasts.com/discuss/channels/general-discussion/datetime-issues-in-production?reply=886032
- 10: https://day.js.org/docs/en/timezone/timezone
Action: Update date formatting to include user timezone handling.
The review comment is valid. Day.js does not adjust dates to the user's timezone by default. As confirmed by the web query, the recommended approach is to extend Day.js with the UTC and Timezone plugins and then use the .tz() method to convert dates appropriately (e.g., using a variable like userTimezone or dayjs.tz.guess()). Ensure these plugins have been set up before invoking tz().
- Location:
src/components/Files/FilesTab.tsx(lines 564-572) - Suggestion: Modify the date formatting to use
.tz(userTimezone)(or a similar mechanism withdayjs.tz.guess()), ensuring the user’s local timezone is applied.
📝 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.
| <TooltipComponent | |
| content={dayjs(file.created_date).format( | |
| "DD MMM YYYY, hh:mm A", | |
| )} | |
| > | |
| <span> | |
| {dayjs(file.created_date).format("DD MMM YYYY ")} | |
| </span> | |
| </TooltipComponent> | |
| <TooltipComponent | |
| content={dayjs(file.created_date).format( | |
| "DD MMM YYYY, hh:mm A", | |
| )} | |
| > | |
| <span> | |
| {dayjs(file.created_date).tz(userTimezone).format("DD MMM YYYY ")} | |
| </span> | |
| </TooltipComponent> |
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
src/components/Facility/ConsultationDetails/OverviewSideBar.tsx (1)
69-78: 🛠️ Refactor suggestionAdd error handling for unauthorized access.
The query is disabled based on permissions, but there's no explicit error handling for unauthorized access attempts.
const { data: encounterData, isLoading } = useQuery({ queryKey: ["encounter", encounterId], queryFn: query(routes.encounter.get, { pathParams: { id: encounterId }, queryParams: { facility: facilityId, }, }), enabled: !!encounterId && canListEncounters, + onError: (error) => { + if (error.status === 403) { + toast.error(t("errors.unauthorized_access")); + goBack(); + } + }, });src/pages/Appointments/AppointmentDetail.tsx (1)
415-428: 🛠️ Refactor suggestionAdd error handling for appointment cancellation.
The cancel appointment mutation lacks error handling, which could lead to a poor user experience if the cancellation fails.
Add error handling:
const { mutate: cancelAppointment } = useMutation({ mutationFn: mutate(scheduleApis.appointments.cancel, { pathParams: { facility_id: facilityId, id: appointment.id, }, }), onSuccess: () => { toast.success(t("appointment_cancelled")); queryClient.invalidateQueries({ queryKey: ["appointment", appointment.id], }); }, + onError: (error) => { + toast.error(t("failed_to_cancel_appointment")); + console.error("Failed to cancel appointment:", error); + }, });
🧹 Nitpick comments (16)
src/components/Medicine/MedicationRequestTable/index.tsx (2)
74-82: Consider adding missing dependencies to the effect hook.The effect hook's dependency array only includes
canAccess, but it also usesgoBackwhich should be included to prevent stale closures.- // eslint-disable-next-line react-hooks/exhaustive-deps - }, [canAccess]); + }, [canAccess, goBack]);Also applies to: 116-122
84-87: LGTM! Consider extracting permission check to a custom hook.The permission-based rendering is well implemented. However, the permission check logic could be extracted to a custom hook for reusability.
// Example custom hook function useEncounterPermissions(encounter: Encounter) { const { hasPermission } = usePermissions(); const authUser = useAuthUser(); const { canSubmitEncounterQuestionnaire } = getPermissions( hasPermission, authUser.permissions ); return { canWrite: canSubmitEncounterQuestionnaire && encounter && !inactiveEncounterStatus.includes(encounter.status) }; }Also applies to: 185-199
src/components/Patient/diagnosis/list.tsx (1)
43-56: Consider abstracting shared permission logic.The permission logic is identical to SymptomsList. Consider creating a custom hook to reduce code duplication:
function useEncounterPermissions(encounterId?: string, encounterStatus?: Encounter["status"]) { const authUser = useAuthUser(); const { hasPermission } = usePermissions(); const { canViewClinicalData, canViewEncounter, canSubmitEncounterQuestionnaire, canSubmitPatientQuestionnaireResponses, } = getPermissions(hasPermission, authUser.permissions); return { canAccess: encounterId ? canViewEncounter : canViewClinicalData, canEdit: encounterId ? canSubmitEncounterQuestionnaire && !inactiveEncounterStatus.includes(encounterStatus ?? "") : canSubmitPatientQuestionnaireResponses, }; }src/components/Facility/ConsultationDetails/OverviewSideBar.tsx (5)
27-29: Consider decomposing the permission check for better clarity.The
canWritevariable combines multiple conditions into a single boolean. Consider separating the status check from the permission check for better maintainability and debugging.- const canWrite = - canSubmitEncounterQuestionnaire && - !inactiveEncounterStatus.includes(props.encounter.status); + const isEncounterActive = !inactiveEncounterStatus.includes(props.encounter.status); + const canWrite = canSubmitEncounterQuestionnaire && isEncounterActive;
33-35: Consider memoizing the default tab value.The default tab value depends on permissions, which could cause UI flicker if permissions change. Consider memoizing this value.
+ const defaultTab = React.useMemo( + () => (canWrite ? "quick_access" : "observations"), + [canWrite] + ); <Tabs - defaultValue={canWrite ? "quick_access" : "observations"} + defaultValue={defaultTab} className="w-full" >
23-30: Consider implementing an error boundary.Instead of directly calling
goBack(), consider implementing an error boundary to handle permission-related errors more gracefully.
76-79: Consider using type-safe permission checks.The tab visibility condition could benefit from type safety to prevent potential runtime errors.
+ type TabVisibility = { + [K in keyof typeof TABS]: boolean; + }; + + const tabVisibility: TabVisibility = { + PROFILE: true, + AVAILABILITY: Boolean(props.facilityId && canViewSchedule), + }; AVAILABILITY: { body: UserAvailabilityTab, - hidden: !props.facilityId || !canViewSchedule, + hidden: !tabVisibility.AVAILABILITY, },
80-82: Consider using a custom hook for permission checks.The permission check logic is repeated across components. Consider extracting it into a custom hook for reusability.
// useEncounterPermissions.ts export const useEncounterPermissions = (encounterStatus: string) => { const authUser = useAuthUser(); const { hasPermission } = usePermissions(); const { canWriteEncounter } = getPermissions(hasPermission, authUser.permissions); return { canWrite: canWriteEncounter && !inactiveEncounterStatus.includes(encounterStatus), }; };src/components/Patient/PatientInfoCard.tsx (1)
417-448: Simplify conditional rendering logic.The nested conditions could be simplified for better readability.
Consider this refactoring:
- {canWrite && ( - <Button - variant="outline" - className="border-gray-400 w-full" - > - <Link - href={`/facility/${props.encounter.facility.id}/patient/${props.patient.id}/encounter/${props.encounter.id}/questionnaire/location_association`} - className="text-sm text-gray-950 font-semibold" - > - {t("update_location")} - </Link> - </Button> - )} + {canWrite && ( + <Link + href={`/facility/${props.encounter.facility.id}/patient/${props.patient.id}/encounter/${props.encounter.id}/questionnaire/location_association`} + className={cn( + buttonVariants({ variant: "outline" }), + "border-gray-400 w-full text-sm text-gray-950 font-semibold" + )} + > + {t("update_location")} + </Link> + )}And for the location add badge:
- ) : canWrite ? ( - <Badge variant="outline"> - <Link - href={`/facility/${props.encounter.facility.id}/patient/${props.patient.id}/encounter/${props.encounter.id}/questionnaire/location_association`} - className="flex items-center gap-1 text-gray-950 py-0.5" - > - <CareIcon - icon="l-location-point" - className="h-4 w-4 text-green-600" - /> - {t("add_location")} - </Link> - </Badge> - ) : ( - <></> + ) : canWrite ? ( + <Link + href={`/facility/${props.encounter.facility.id}/patient/${props.patient.id}/encounter/${props.encounter.id}/questionnaire/location_association`} + className="flex items-center gap-1 text-gray-950 py-0.5" + > + <Badge variant="outline"> + <CareIcon + icon="l-location-point" + className="h-4 w-4 text-green-600" + /> + {t("add_location")} + </Badge> + </Link> + ) : nullsrc/pages/Appointments/AppointmentDetail.tsx (1)
462-508: Reset sheet state when permissions change.The reschedule sheet's open state should be reset when permissions change to prevent UI inconsistencies.
Add an effect to handle permission changes:
+useEffect(() => { + if (!canCreateAppointment && isRescheduleOpen) { + setIsRescheduleOpen(false); + setSelectedSlotId(undefined); + } +}, [canCreateAppointment, isRescheduleOpen]); {canCreateAppointment && ( <Sheet open={isRescheduleOpen} onOpenChange={setIsRescheduleOpen}> // ... existing code </Sheet> )}src/pages/Encounters/tabs/EncounterNotesTab.tsx (2)
333-333: Consider more specific permission checks.The
canAccessvariable combines two permissions with OR logic, which might be too permissive. Consider using more specific permission checks for different operations.- const canAccess = canViewClinicalData || canViewEncounter; + const canViewNotes = canViewClinicalData && canViewEncounter;
728-734: Enhance error message for inactive encounters.The error message could be more informative by explaining why discussions are disabled for inactive encounters and what actions users can take.
- {t("encounter_notes__cannot_start_discussion_old_encounter", { - encounterStatus: encounter.status, - })} + {t("encounter_notes__cannot_start_discussion_old_encounter", { + encounterStatus: encounter.status, + reason: "Discussions are disabled for completed or inactive encounters to maintain accurate medical records.", + action: "Please create a new encounter if you need to continue the discussion." + })}public/locale/en.json (1)
877-877: Enhance localization key with additional context.The error message for inactive encounters could benefit from additional placeholders to provide more context and guidance to users.
- "encounter_notes__cannot_start_discussion_old_encounter": "You cannot start a new discussion for this encounter as it is {{encounterStatus}}.", + "encounter_notes__cannot_start_discussion_old_encounter": "You cannot start a new discussion for this encounter as it is {{encounterStatus}}. {{reason}} {{action}}",src/pages/Encounters/tabs/EncounterUpdatesTab.tsx (1)
32-38: Consider adding a loading state during redirect.While the permission check and redirect logic is correct, the user might see a flash of content before being redirected.
useEffect(() => { if (!canAccess) { toast.error("You do not have permission to view this encounter"); - goBack(); + // Add a small delay to prevent flash of content + setTimeout(goBack, 100); } // eslint-disable-next-line react-hooks/exhaustive-deps }, [canAccess]);src/components/Users/UserHome.tsx (1)
78-78: Consider extracting the tab visibility logic.The condition
!props.facilityId || !canViewSchedulecould be moved to a separate function for better maintainability, especially if more tabs need similar permission checks in the future.+const isTabHidden = (tab: keyof typeof TABS, props: UserHomeProps, permissions: ReturnType<typeof getPermissions>) => { + switch (tab) { + case 'AVAILABILITY': + return !props.facilityId || !permissions.canViewSchedule; + default: + return false; + } +}; const TABS = { PROFILE: { body: UserSummaryTab, hidden: false, }, AVAILABILITY: { body: UserAvailabilityTab, - hidden: !props.facilityId || !canViewSchedule, + hidden: isTabHidden('AVAILABILITY', props, { canViewSchedule }), }, } satisfies Record<string, TabChildProp>;src/pages/Encounters/EncounterShow.tsx (1)
80-82: Consider extracting the canWrite logic to a utility function.The write permission check combines multiple conditions and could be reused across components.
+const canWriteToEncounter = ( + permissions: ReturnType<typeof getPermissions>, + encounterStatus?: string +) => { + return ( + permissions.canWriteEncounter && + !inactiveEncounterStatus.includes(encounterStatus ?? "") + ); +}; -const canWrite = - canWriteEncounter && - !inactiveEncounterStatus.includes(encounterData?.status ?? ""); +const canWrite = canWriteToEncounter( + { canWriteEncounter }, + encounterData?.status +);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
public/locale/en.json(5 hunks)src/components/Facility/ConsultationDetails/OverviewSideBar.tsx(1 hunks)src/components/Facility/ConsultationDetails/QuestionnaireResponsesList.tsx(5 hunks)src/components/Facility/ConsultationDetails/QuickAccess.tsx(1 hunks)src/components/Medicine/MedicationRequestTable/index.tsx(6 hunks)src/components/Patient/MedicationStatementList.tsx(3 hunks)src/components/Patient/PatientInfoCard.tsx(5 hunks)src/components/Patient/TreatmentSummary.tsx(3 hunks)src/components/Patient/allergy/list.tsx(8 hunks)src/components/Patient/diagnosis/list.tsx(6 hunks)src/components/Patient/symptoms/list.tsx(7 hunks)src/components/Users/UserHome.tsx(3 hunks)src/pages/Appointments/AppointmentDetail.tsx(9 hunks)src/pages/Appointments/AppointmentsPage.tsx(11 hunks)src/pages/Encounters/EncounterShow.tsx(4 hunks)src/pages/Encounters/tabs/EncounterNotesTab.tsx(7 hunks)src/pages/Encounters/tabs/EncounterUpdatesTab.tsx(4 hunks)src/types/emr/encounter.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/types/emr/encounter.ts
- src/components/Facility/ConsultationDetails/QuestionnaireResponsesList.tsx
- src/components/Patient/MedicationStatementList.tsx
- src/components/Patient/allergy/list.tsx
- src/pages/Appointments/AppointmentsPage.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (19)
src/components/Medicine/MedicationRequestTable/index.tsx (2)
5-6: LGTM! Type changes enhance type safety.The changes to use the
Encountertype instead of a string ID and the addition of permission-related imports improve type safety and align with the permission-based rendering objectives.Also applies to: 18-21, 24-25, 64-64
93-93: LGTM! Query modifications enhance security.The queries are now properly gated by permissions and use the structured encounter data consistently.
Also applies to: 98-98, 106-106, 113-113
src/components/Patient/symptoms/list.tsx (4)
14-20: LGTM! Well-structured permission management setup.The imports are well-organized, and the interface is properly typed with the optional encounterStatus.
Also applies to: 29-29
44-64: LGTM! Robust permission management implementation.The implementation:
- Properly separates encounter and clinical data permissions
- Correctly handles inactive encounter states
- Prevents unauthorized data fetching via query enablement
95-95: LGTM! Consistent permission-based rendering.The canEdit prop is consistently passed to SymptomListLayout across all render paths.
Also applies to: 109-109
151-151: LGTM! Secure edit link rendering.The edit functionality is properly gated behind the canEdit permission check.
Also applies to: 159-159, 173-174
src/components/Facility/ConsultationDetails/OverviewSideBar.tsx (1)
32-38:⚠️ Potential issueAdd missing dependencies to useEffect.
The
goBackfunction is used in the effect but not included in the dependency array. This could lead to stale closures.useEffect(() => { if (!canAccess) { toast.error("You do not have permission to view this encounter"); goBack(); } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [canAccess]); + }, [canAccess, goBack]);Likely an incorrect or invalid review comment.
src/components/Patient/TreatmentSummary.tsx (3)
14-21: LGTM! Clean permission setup.The permission system is well-integrated using appropriate hooks and follows React best practices.
Also applies to: 34-39
238-243: LGTM! Proper permission-based rendering.The
canAccessprop is correctly passed to control access to medication statements based on user permissions.
1-265: Add test coverage for permission-based rendering.Consider adding test cases to verify:
- Rendering with and without view permissions
- Proper propagation of permissions to child components
Would you like me to help generate test cases for the permission-based rendering?
src/components/Patient/PatientInfoCard.tsx (3)
58-63: LGTM! Clear prop interface.The
canWriteprop is well-named and properly typed.
98-126: LGTM! Well-structured organization badges.The function effectively:
- Handles both write and read-only states
- Provides empty state feedback
- Uses proper TypeScript and React patterns
358-368: LGTM! Clean conditional rendering.The component effectively switches between editable and read-only views based on user permissions.
public/locale/en.json (1)
729-730: LGTM! Clear and descriptive quick action messages.The quick action description keys are clear and follow consistent naming patterns.
src/pages/Encounters/tabs/EncounterUpdatesTab.tsx (2)
23-30: LGTM! Permission checks are well implemented.The permission checks using
getPermissionsand combiningcanViewClinicalDataandcanViewEncounterinto a singlecanAccessflag is a clean approach.
87-87: LGTM! Consistent permission handling.The
canAccessprop is correctly passed toSideOverview, maintaining consistent permission-based rendering.src/components/Users/UserHome.tsx (1)
45-48: LGTM! Clean permission check implementation.The
getPermissionscall is well-placed and correctly uses bothhasPermissionandauthUser.permissions.src/pages/Encounters/EncounterShow.tsx (2)
77-77: LGTM! Query execution is properly guarded.The
enabledcondition correctly combines the existence check with permission check.
157-157: Verify PatientInfoCard's handling of canWrite prop.Ensure that the
PatientInfoCardcomponent correctly uses thecanWriteprop for conditional rendering.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Check how canWrite is used in PatientInfoCard ast-grep --pattern 'interface $interface { $$$ canWrite: $_ $$$ }'Length of output: 74
I'm not seeing evidence that the
PatientInfoCardcomponent's definition uses an interface with acanWriteprop. It appears likely that its props are either defined inline (via destructuring) or managed differently. To clarify howcanWriteis used, please run this script to locate thePatientInfoCardfile and inspect its handling of thecanWriteprop:Once we have the output from this script, we can verify whether
canWriteis used in conditional rendering (e.g., conditionally showing editing options). Please run the above script and provide its output if further investigation is needed.
🏁 Script executed:
#!/bin/bash # Locate the PatientInfoCard component file (searching .tsx files for "PatientInfoCard") file=$(fd -t f "PatientInfoCard.tsx") if [ -n "$file" ]; then echo "Found PatientInfoCard file: $file" echo "Searching for usage of 'canWrite' inside the file:" rg -n "canWrite" "$file" else echo "PatientInfoCard file not found" fiLength of output: 983
Verified: PatientInfoCard correctly handles the canWrite prop.
- The component defines
canWriteas a boolean in its interface (line 62).- It destructures
canWritefrom props (line 66) and uses it in multiple conditional renderings.- Conditional logic is applied to adjust CSS classes (e.g., line 106) and rendering behavior (e.g., lines 115, 358, 417).
| useEffect(() => { | ||
| if (!canViewAppointments) { | ||
| toast.error(t("no_permission_to_view_page")); | ||
| goBack(`/facility/${props.facilityId}/overview`); | ||
| } | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [canViewAppointments]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and prevent race conditions.
The current implementation has two potential issues:
- Race condition: The effect might run after the query has already started
- Generic error message: The toast message could be more specific
Consider this implementation:
-useEffect(() => {
- if (!canViewAppointments) {
- toast.error(t("no_permission_to_view_page"));
- goBack(`/facility/${props.facilityId}/overview`);
- }
- // eslint-disable-next-line react-hooks/exhaustive-deps
-}, [canViewAppointments]);
+useEffect(() => {
+ if (!canViewAppointments) {
+ toast.error(t("no_permission_to_view_appointments"));
+ goBack(`/facility/${props.facilityId}/overview`);
+ return;
+ }
+}, [canViewAppointments, props.facilityId, goBack, t]);📝 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.
| useEffect(() => { | |
| if (!canViewAppointments) { | |
| toast.error(t("no_permission_to_view_page")); | |
| goBack(`/facility/${props.facilityId}/overview`); | |
| } | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, [canViewAppointments]); | |
| useEffect(() => { | |
| if (!canViewAppointments) { | |
| toast.error(t("no_permission_to_view_appointments")); | |
| goBack(`/facility/${props.facilityId}/overview`); | |
| return; | |
| } | |
| }, [canViewAppointments, props.facilityId, goBack, t]); |
| <section className="space-y-2 p-2"> | ||
| <h3 className="text-lg font-semibold mb-3">{t("questionnaire")}</h3> | ||
| <div className="space-y-3 p-2 font-semibold"> | ||
| {questionnaireOptions.map((option) => ( | ||
| <Link | ||
| key={option.slug} | ||
| href={`/facility/${encounter.facility.id}/patient/${encounter.patient.id}/encounter/${encounter.id}/questionnaire/${option.slug}`} | ||
| className="flex items-center gap-2 text-sm hover:text-gray-500 text-gray-900" | ||
| data-cy="update-encounter-option" | ||
| > | ||
| <CareIcon icon="l-file-alt" className="h-4 w-4 text-gray-950" /> | ||
| {t(option.title)} | ||
| </Link> | ||
| ))} | ||
| </div> | ||
| <div className="w-full border-t border-dashed border-gray-300" /> | ||
| </section> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add permission checks for questionnaire section.
The questionnaire section is rendered unconditionally after removing the encounter.status check. Given this PR's focus on permissions-based rendering, consider adding appropriate permission checks to ensure only authorized users can access this section.
+ const { hasPermission } = usePermissions();
+ const { canViewQuestionnaire } = getPermissions(hasPermission, authUser.permissions);
return (
<div className="flex flex-col gap-6">
{/* Questionnaire Section */}
+ {canViewQuestionnaire && (
<section className="space-y-2 p-2">
// ... existing code ...
</section>
+ )}📝 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.
| <section className="space-y-2 p-2"> | |
| <h3 className="text-lg font-semibold mb-3">{t("questionnaire")}</h3> | |
| <div className="space-y-3 p-2 font-semibold"> | |
| {questionnaireOptions.map((option) => ( | |
| <Link | |
| key={option.slug} | |
| href={`/facility/${encounter.facility.id}/patient/${encounter.patient.id}/encounter/${encounter.id}/questionnaire/${option.slug}`} | |
| className="flex items-center gap-2 text-sm hover:text-gray-500 text-gray-900" | |
| data-cy="update-encounter-option" | |
| > | |
| <CareIcon icon="l-file-alt" className="h-4 w-4 text-gray-950" /> | |
| {t(option.title)} | |
| </Link> | |
| ))} | |
| </div> | |
| <div className="w-full border-t border-dashed border-gray-300" /> | |
| </section> | |
| const { hasPermission } = usePermissions(); | |
| const { canViewQuestionnaire } = getPermissions(hasPermission, authUser.permissions); | |
| return ( | |
| <div className="flex flex-col gap-6"> | |
| {/* Questionnaire Section */} | |
| {canViewQuestionnaire && ( | |
| <section className="space-y-2 p-2"> | |
| <h3 className="text-lg font-semibold mb-3">{t("questionnaire")}</h3> | |
| <div className="space-y-3 p-2 font-semibold"> | |
| {questionnaireOptions.map((option) => ( | |
| <Link | |
| key={option.slug} | |
| href={`/facility/${encounter.facility.id}/patient/${encounter.patient.id}/encounter/${encounter.id}/questionnaire/${option.slug}`} | |
| className="flex items-center gap-2 text-sm hover:text-gray-500 text-gray-900" | |
| data-cy="update-encounter-option" | |
| > | |
| <CareIcon icon="l-file-alt" className="h-4 w-4 text-gray-950" /> | |
| {t(option.title)} | |
| </Link> | |
| ))} | |
| </div> | |
| <div className="w-full border-t border-dashed border-gray-300" /> | |
| </section> | |
| )} | |
| </div> | |
| ); |
| @@ -33,6 +59,7 @@ export const EncounterUpdatesTab = ({ | |||
| patientId={patient.id} | |||
| encounterId={encounter.id} | |||
| facilityId={facilityId} | |||
| encounterStatus={encounter.status} | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the impact of passing encounterStatus.
The encounterStatus prop has been added to both SymptomsList and DiagnosisList components. Please ensure these components handle the status correctly.
Also applies to: 72-72
🏁 Script executed:
#!/bin/bash
# Check how encounterStatus is used in these components
ast-grep --pattern 'interface $interface {
$$$
encounterStatus: $_
$$$
}'Length of output: 81
Let's collect more details on how these two components consume the new encounterStatus prop. I'll run a script that searches for occurrences of encounterStatus within any files whose names indicate they belong to the SymptomsList and DiagnosisList components.
Once we have the output of this script, we can verify if these components are reading or reacting to the encounterStatus prop as expected.
🏁 Script executed:
#!/bin/bash
echo "----- Searching in SymptomsList components -----"
find . -type f -iname "*SymptomsList*.tsx" -exec grep -Hn "encounterStatus" {} \;
echo "----- Searching in DiagnosisList components -----"
find . -type f -iname "*DiagnosisList*.tsx" -exec grep -Hn "encounterStatus" {} \;Length of output: 386
EncounterStatus Prop Not Utilized in SymptomsList and DiagnosisList Components
Our investigation shows that neither the SymptomsList nor DiagnosisList components reference or otherwise use the encounterStatus prop. This indicates that while the prop is passed from the EncounterUpdatesTab, there's currently no handling logic in these components to take any action based on its value. Please verify if this is intended; otherwise, consider implementing the necessary logic in these components.
|
Conflicts have been detected against the base branch. Please merge the base branch into your branch.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (13)
src/components/Facility/ConsultationDetails/QuestionnaireResponsesList.tsx (2)
303-316: Simplify permission management logic.While the permission checks are comprehensive, the code could be more concise and readable.
Consider this improvement:
- const { - canViewClinicalData, - canViewEncounter, - canViewPatientQuestionnaireResponses, - } = getPermissions( - hasPermission, - encounter?.permissions ?? patientData?.permissions ?? [], - ); - - const canAccess = encounter - ? canViewClinicalData || canViewEncounter - : canViewPatientQuestionnaireResponses; + const permissions = getPermissions( + hasPermission, + encounter?.permissions ?? patientData?.permissions ?? [] + ); + + const canAccess = encounter + ? permissions.canViewClinicalData || permissions.canViewEncounter + : permissions.canViewPatientQuestionnaireResponses;
335-349: Improve redirection logic and error messaging.The redirection logic works correctly but could benefit from some improvements:
- The comment could be more descriptive
- The error message could provide more context
Consider these improvements:
- // redirect only if encounter is not present and canAccess is false - // This is ensure redirection is only active when this component is rendered as a patient tab + // Only redirect when viewing questionnaire responses outside of an encounter context + // This ensures users are redirected to the patient page when they lack permissions const enableRedirect = !encounter && !canAccess; useEffect(() => { if (enableRedirect) { - toast.error(t("no_permission_to_view_page")); + toast.error(t("no_permission_to_view_questionnaire_responses")); goBack( facilityId ? `/facility/${facilityId}/patient/${patientId}` : `/patient/${patientId}`, ); } // eslint-disable-next-line react-hooks/exhaustive-deps }, [enableRedirect]);src/types/emr/newPatient.ts (1)
37-37:❓ Verification inconclusive
Consider using a more type-safe permission definition.
While
string[]provides flexibility, it offers no compile-time validation of permission values. Consider using a union type or enum to ensure type safety and maintainability.Example improvement:
export type PatientPermission = | "VIEW_PATIENT" | "EDIT_PATIENT" | "DELETE_PATIENT" | "MANAGE_ENCOUNTERS" // Add other valid permissions export interface Patient { // ... other properties permissions: PatientPermission[]; }Let's check how permissions are typed across the codebase:
🏁 Script executed:
#!/bin/bash # Search for permission type definitions and usage echo "Searching for permission type definitions..." rg -t typescript "type.*Permission" -A 5 echo "Searching for permission array declarations..." rg -t typescript "permissions:\s*(string\[\]|.*Permission\[\])" -A 2Length of output: 385
I'll re-run the search using a revised command that targets TypeScript files by using the
-t tsflag. This should help us verify if there’s an existing, more type-safe permission definition in the codebase. For example:Please run this updated script and share the output, so we can better verify the consistency of permission types across the codebase before finalizing the review comment.
🏁 Script executed:
[run_scripts] #!/bin/bash echo "Searching for permission type definitions in .ts files..." rg -t ts "type.*Permission" -A 5 echo "Searching for permission array declarations in .ts files..." rg -t ts "permissions:\s*(string\[\]|.*Permission\[\])" -A 2Length of output: 1747
Refine Permission Type Definition for Enhanced Safety
The use of
string[]for thepermissionsproperty is consistent with similar declarations elsewhere in the codebase (e.g., insrc/types/emr/encounter.ts,src/types/organization/organization.ts, and within functions implemented insrc/common/Permissions.tsx). While the current implementation works as intended, you might consider a more type-safe approach if you plan to enforce strict permission values. For example:export type PatientPermission = | "VIEW_PATIENT" | "EDIT_PATIENT" | "DELETE_PATIENT" | "MANAGE_ENCOUNTERS" // Add other valid permissions as needed export interface Patient { // ... other properties permissions: PatientPermission[]; }Adopting this change would enhance compile-time validation and reduce the risk of assigning unintended permission strings. However, note that this refactoring would also require updating similar definitions across the codebase to maintain consistency. Please verify if a project-wide change in the permission type strategy aligns with the overall design goals before proceeding.
src/components/Patient/PatientDetailsTab/HealthProfileSummary.tsx (1)
51-54: Consider adding error boundary for MedicationStatementList.While the component correctly passes the
canAccessprop, consider adding an error boundary to gracefully handle any permission-related errors during data fetching.src/components/ui/sidebar/facility-nav.tsx (1)
51-55: Consider extracting complex permission logic.The visibility condition for search_patients combines multiple permissions. Consider extracting this logic into a separate function for better maintainability.
+const canAccessPatientSearch = (permissions: FacilityPermissions) => + permissions.canCreateAppointment || + permissions.canListEncounters || + permissions.canCreateEncounter; { name: t("search_patients"), url: `${baseUrl}/patients`, icon: "d-patient", - visibility: - permissions.canCreateAppointment || - permissions.canListEncounters || - permissions.canCreateEncounter, + visibility: canAccessPatientSearch(permissions), }src/pages/Encounters/tabs/EncounterPlotsTab.tsx (1)
32-37: Consider memoizing permission checks.The permission checks are recalculated on every render. Consider using useMemo to optimize performance.
-const { canViewClinicalData, canViewEncounter } = getPermissions( - hasPermission, - props.encounter.permissions, -); +const permissions = useMemo( + () => getPermissions(hasPermission, props.encounter.permissions), + [hasPermission, props.encounter.permissions] +); +const { canViewClinicalData, canViewEncounter } = permissions;src/pages/Encounters/tabs/EncounterUpdatesTab.tsx (1)
30-36: Consider adding a loading state during redirection.While the redirection logic is correct, users might see a flash of content before being redirected.
useEffect(() => { + const [isRedirecting, setIsRedirecting] = useState(false); if (!canAccess) { + setIsRedirecting(true); toast.error("You do not have permission to view this encounter"); goBack(); } // eslint-disable-next-line react-hooks/exhaustive-deps }, [canAccess]); + +if (isRedirecting) { + return <LoadingSpinner />; +}src/components/Facility/EncounterCard.tsx (1)
102-114: Consider adding a tooltip for disabled state.When the encounter hasn't started (
!encounter.period.start), the link is disabled but there's no visual feedback explaining why.{canViewEncounter && ( <Link href={`/facility/${encounter.facility.id}/encounter/${encounter.id}/updates`} className={cn( buttonVariants({ variant: "secondary" }), "mt-2 shadow-none border border-secondary-300", !encounter.period.start && "pointer-events-none opacity-50", )} + title={!encounter.period.start ? t("encounter_not_started") : ""} > <CareIcon icon="l-plus-circle" /> {t("view_encounter")} </Link> )}src/components/Patient/PatientDetailsTab/EncounterHistory.tsx (1)
39-49: Consider adding error handling for query failures.While the query is correctly enabled based on permissions, there's no error handling for query failures.
const { data: encounterData, isLoading } = useQuery({ queryKey: ["encounterHistory", patientId, qParams], queryFn: query(routes.encounter.list, { queryParams: { patient: patientId, limit: 5, offset: ((qParams.page ?? 1) - 1) * 5, }, }), enabled: canViewPatients, + onError: (error) => { + toast.error(t("error_loading_encounter_history")); + }, });src/components/Patient/PatientDetailsTab/patientUpdates.tsx (1)
54-64: Simplify the redirection logic.The redirection logic can be simplified by using template literals.
- goBack( - facilityId - ? `/facility/${facilityId}/patient/${patientId}` - : `/patient/${patientId}`, - ); + goBack(`${facilityId ? `/facility/${facilityId}` : ''}/patient/${patientId}`);src/pages/Patients/VerifyPatient.tsx (1)
138-225: Simplify the nested ternary operator in CardDescription.The nested ternary operator could be simplified for better readability.
- <CardDescription> - {canCreateAppointment && canCreateEncounter - ? t("quick_actions_description") - : canCreateAppointment - ? t("quick_actions_description_create_appointment") - : canCreateEncounter - ? t("quick_actions_description_create_encounter") - : ""} - </CardDescription> + <CardDescription> + {(() => { + if (canCreateAppointment && canCreateEncounter) { + return t("quick_actions_description"); + } + if (canCreateAppointment) { + return t("quick_actions_description_create_appointment"); + } + if (canCreateEncounter) { + return t("quick_actions_description_create_encounter"); + } + return ""; + })()} + </CardDescription>Also applies to: 227-260
src/pages/Encounters/tabs/EncounterNotesTab.tsx (2)
497-507: LGTM! Comprehensive UI permission handling.The implementation:
- Conditionally renders the "New Thread" button
- Controls message input access
- Provides clear feedback when users can't start discussions
- Properly handles inactive encounter states
However, consider adding a tooltip to explain why the "New Thread" button is disabled when
!canWriteCurrentEncounter.<Button variant="outline" size="sm" onClick={() => setShowNewThreadDialog(true)} className="h-8" + title={!canWriteCurrentEncounter ? t("encounter_notes__cannot_start_discussion_old_encounter", { encounterStatus: encounter.status }) : undefined} > <Plus className="h-4 w-4" /> {t("encounter_notes__new")} </Button>Also applies to: 671-706, 722-733
679-685: Consider usinge.preventDefault()conditionally.The current implementation prevents the default behavior even when the message is empty.
onKeyDown={(e) => { if (e.key === "Enter" && !e.shiftKey) { - e.preventDefault(); - if (newMessage.trim()) { + if (newMessage.trim()) { + e.preventDefault(); handleSendMessage(e); } } }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
src/components/Facility/ConsultationDetails/QuestionnaireResponsesList.tsx(4 hunks)src/components/Facility/EncounterCard.tsx(3 hunks)src/components/Files/FilesTab.tsx(8 hunks)src/components/Medicine/MedicationAdministration/AdministrationTab.tsx(12 hunks)src/components/Medicine/MedicationRequestTable/index.tsx(6 hunks)src/components/Patient/PatientDetailsTab/Appointments.tsx(3 hunks)src/components/Patient/PatientDetailsTab/Demography.tsx(4 hunks)src/components/Patient/PatientDetailsTab/EncounterHistory.tsx(3 hunks)src/components/Patient/PatientDetailsTab/HealthProfileSummary.tsx(2 hunks)src/components/Patient/PatientDetailsTab/PatientUsers.tsx(4 hunks)src/components/Patient/PatientDetailsTab/patientUpdates.tsx(3 hunks)src/components/ui/sidebar/facility-nav.tsx(4 hunks)src/context/PermissionContext.tsx(1 hunks)src/pages/Encounters/tabs/EncounterNotesTab.tsx(7 hunks)src/pages/Encounters/tabs/EncounterPlotsTab.tsx(4 hunks)src/pages/Encounters/tabs/EncounterUpdatesTab.tsx(4 hunks)src/pages/Patients/VerifyPatient.tsx(4 hunks)src/types/emr/encounter.ts(2 hunks)src/types/emr/newPatient.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/types/emr/encounter.ts
🔇 Additional comments (39)
src/components/Files/FilesTab.tsx (5)
50-55: Props interface needs to be stricter.The
patientprop is marked as optional but is used without proper null checks (e.g.,patient?.idon line 91). This could lead to runtime errors.Apply this diff to fix the type definition:
export interface FilesTabProps { type: "encounter" | "patient"; facilityId: string; encounter?: Encounter; - patient?: Patient; + patient: Patient; }
70-88: Well-structured permission management!The permission handling is well-implemented with:
- Clear separation of concerns
- Comprehensive checks for both encounter and patient permissions
- Proper handling of encounter status for write operations
565-573: Consider user timezone for date display.The date formatting doesn't account for user timezone, which could lead to confusion.
Add timezone support:
-dayjs(file.created_date).format("DD MMM YYYY ") +dayjs(file.created_date).tz(userTimezone).format("DD MMM YYYY ")
508-518: Improved table layout!The adjusted column widths provide better content distribution and visual balance.
544-548: Enhanced tooltip implementation!The new
TooltipComponentprovides a consistent and reusable way to display tooltips for truncated content.src/components/Facility/ConsultationDetails/QuestionnaireResponsesList.tsx (2)
32-39: LGTM! Props interface is well-defined.The new optional props
facilityIdandpatientDataare correctly typed and their optionality aligns with their usage in the component.
318-334:✅ Verification successful
Verify query behavior when access is denied.
The query is correctly disabled when
canAccessis false, but we should verify that no data is leaked and the loading state is handled appropriately.
🏁 Script executed:
#!/bin/bash # Description: Search for other instances of useQuery to ensure consistent permission handling # Search for useQuery usage patterns ast-grep --pattern 'useQuery({ $$$ enabled: $condition, $$$ })' # Search for permission-based query enabling rg 'enabled:\s*\w+Access' -A 2Length of output: 1818
Verify query behavior when access is denied.
The query in
QuestionnaireResponsesList.tsxis consistently guarded by thecanAccessflag, just like in similar components (e.g.,ObservationsList,ObservationChart, etc.). This ensures that when access is denied (canAccessis false), no network request is issued and no unintended data leakage occurs. Please double-check that the loading state is handled appropriately in the UI when access is disabled.src/components/Medicine/MedicationRequestTable/index.tsx (4)
73-80: LGTM! Well-structured permission management.The implementation effectively uses permission hooks and variables to control access to the component, with a clear separation of concerns between permission checks and navigation.
96-96: LGTM! Efficient query management.The queries are correctly gated by the
canAccesspermission check, preventing unnecessary API calls when the user lacks access.Also applies to: 111-111
82-85: LGTM! Well-implemented permission-based rendering.The edit button's visibility is correctly controlled by both user permissions and encounter status, ensuring that only authorized users can modify active encounters.
Also applies to: 183-197
63-63:✅ Verification successful
Verify the impact of the Props interface change.
The change from
encounterId: stringtoencounter: Encounteris a breaking change that requires updates in all components that useMedicationRequestTable.Run the following script to verify all usages of the component:
🏁 Script executed:
#!/bin/bash # Description: Find all files that import and use MedicationRequestTable. # Search for files importing MedicationRequestTable rg -l "MedicationRequestTable" | while read -r file; do # Show the context of each usage echo "=== $file ===" rg -A 10 "MedicationRequestTable" "$file" doneLength of output: 2075
Breaking Change Impact Verified: Use of New
encounterProp ConfirmedThe update from
encounterId: stringtoencounter: Encounterhas been correctly adopted in the codebase. Our search results show that:
src/pages/Encounters/tabs/EncounterMedicinesTab.tsx– The component is now passed theencounterprop and aligns with the updated interface.src/components/Medicine/MedicationAdministration/AdministrationTab.tsx– Only imports a sub-export (EmptyState) fromMedicationRequestTableand is not directly affected by the interface change.No instances of the legacy
encounterIdprop were found. The breaking change appears to be fully integrated without impacting consuming components.src/components/Medicine/MedicationAdministration/AdministrationTab.tsx (4)
24-31: LGTM! Consistent permission management.The permission-related imports and props follow the same pattern as MedicationRequestTable, maintaining consistency across components.
Also applies to: 97-98
367-372: LGTM! Comprehensive permission checks.The implementation effectively combines user permissions with encounter status to determine both access and write capabilities.
413-413: LGTM! Efficient query management.All queries are correctly gated by the
canAccesspermission check, preventing unnecessary API calls when the user lacks access.Also applies to: 426-426, 454-454
312-312: LGTM! Well-implemented permission-based rendering.All medication administration actions (administer, discontinue) are correctly gated by permission checks, ensuring that only authorized users can perform these operations.
Also applies to: 331-355, 785-794
src/context/PermissionContext.tsx (1)
39-42:❓ Verification inconclusive
Verify the impact of permission checking logic change.
The new implementation prioritizes
objectPermissionsoveruserPermissionswhen present. This is a significant change in behavior that could affect existing code relying on the previous implementation where both permission sets were checked.
🏁 Script executed:
#!/bin/bash # Search for all usages of hasPermission with objectPermissions parameter ast-grep --pattern 'hasPermission($_, $_)'Length of output: 9274
Review on Updated Permission Logic in PermissionContext.tsx
The updated
hasPermissionfunction now exclusively prioritizesobjectPermissionswhen provided, bypassing the fallback touserPermissions. This behavior is a significant deviation from the previous combined check and merits close examination:
- In src/context/PermissionContext.tsx (lines 39-42), if
objectPermissionsexists, the function only checks that array, which may lead to unintended denials when broader user permissions (found inuserPermissions) would have granted access.- A repository-wide search shows that most usages—in src/common/Permissions.tsx and src/pages/Organization/components/OrganizationLayout.tsx—pass a single permissions source. Verify that the distinct handling in PermissionContext is intentional and that the change doesn’t disrupt application flows expecting a merged permission set.
- Please double-check any related tests and documentation to ensure this behavioral change won’t adversely affect existing permission-based logic.
src/components/Patient/PatientDetailsTab/HealthProfileSummary.tsx (1)
27-37: LGTM! Good error handling and user redirection.The implementation properly handles unauthorized access by showing an error toast and redirecting users, providing a good user experience.
src/pages/Encounters/tabs/EncounterPlotsTab.tsx (1)
45-51: LGTM! Good error handling with user feedback.The implementation properly handles unauthorized access by showing an error message and redirecting users.
src/pages/Encounters/tabs/EncounterUpdatesTab.tsx (2)
22-28: LGTM! Permission check implementation looks good.The permission check is well-implemented, combining both clinical data and encounter view permissions.
60-60:❓ Verification inconclusive
Verify encounterStatus handling in child components.
The
encounterStatusprop has been added to bothSymptomsListandDiagnosisListcomponents. Please ensure these components handle the status correctly.Also applies to: 70-70
🏁 Script executed:
#!/bin/bash # Check how encounterStatus is used in these components ast-grep --pattern 'interface $interface { $$$ encounterStatus: $_ $$$ }'Length of output: 81
Below is an updated review comment that reflects the current verification and requests further manual validation:
Encounter Status Verification for Child Components
The update passes the
encounterStatusprop into both theSymptomsListandDiagnosisListcomponents, but our initial automated search (using an AST grep for interface definitions containingencounterStatus) did not return any results. This could mean that these components may not define or explicitly use the prop in an interface—or it might simply be a limitation of our search pattern.Action Items:
Confirm Interface/Prop Definitions: Please verify that both
SymptomsListandDiagnosisListcorrectly define and document the expected type forencounterStatus.Manual Verification: Manually review the usage of
encounterStatusinside these components to ensure they process the status correctly.Automated Check (Optional): You can run the following script to search for
encounterStatusoccurrences in files likely related to these components:#!/bin/bash for comp in SymptomsList DiagnosisList; do echo "Searching in ${comp} component files:" rg -l "$comp" --type tsx | while read file; do echo "File: $file" rg "encounterStatus" "$file" done donePlease verify that the components are handling the prop as intended.
Action: Verify Child Components' Usage of
encounterStatusPropThe initial automated check (using the
ast-grepcommand) did not yield any results, which might be due to limitations in our search pattern. To further validate that both theSymptomsListandDiagnosisListcomponents correctly define and utilize theencounterStatusprop, please run the following shell script to search for its occurrence within these component files:Once the script completes, review the output to ensure that the
encounterStatusprop is properly declared, typed, and handled within both components. This manual verification will confirm that the recent changes are appropriately integrated.
🏁 Script executed:
[run_scripts] #!/bin/bash echo "Searching for 'encounterStatus' in SymptomsList component files:" rg -l "SymptomsList" --type tsx | xargs -r grep "encounterStatus" echo "---------------------" echo "Searching for 'encounterStatus' in DiagnosisList component files:" rg -l "DiagnosisList" --type tsx | xargs -r grep "encounterStatus"Length of output: 649
Below is a revised shell script to search for the usage of
encounterStatusin the TSX files for both theSymptomsListandDiagnosisListcomponents. This script uses the-g "*.tsx"flag instead of relying on the unrecognized--type tsxoption:Next Steps:
- Run the above script to verify that
encounterStatusis correctly declared, typed, and used within theSymptomsListandDiagnosisListcomponents.- Manually inspect the output to ensure that the prop handling aligns with the intended functionality.
🏁 Script executed:
[run_scripts] #!/bin/bash echo "Searching for 'encounterStatus' in SymptomsList component TSX files:" rg -l "SymptomsList" -g "*.tsx" | xargs -r grep "encounterStatus" echo "---------------------" echo "Searching for 'encounterStatus' in DiagnosisList component TSX files:" rg -l "DiagnosisList" -g "*.tsx" | xargs -r grep "encounterStatus"Length of output: 1698
Child Component
encounterStatusProp VerificationThe
encounterStatusprop is now being passed fromsrc/pages/Encounters/tabs/EncounterUpdatesTab.tsx(asencounterStatus={encounter.status}) to bothSymptomsListandDiagnosisList. Automated searches confirm that in both component files the prop is declared as follows:
SymptomsList (src/components/Patient/symptoms/list.tsx):
- Declaration:
encounterStatus?: Encounter["status"];- Usage: it is passed down and used in conditional logic (e.g., checking with
!inactiveEncounterStatus.includes(encounterStatus ?? "")).DiagnosisList (src/components/Patient/diagnosis/list.tsx):
- Declaration:
encounterStatus?: Encounter["status"];- Usage: similar handling with conditional checks.
Action Items:
- Manual Confirmation: Please verify that the fallback (
encounterStatus ?? "") and the conditional check againstinactiveEncounterStatuscorrectly handle all possible values ofencounterStatus(includingundefined).- Type Consistency: Ensure that the definition
Encounter["status"]matches the expectations throughout the codebase.src/components/Facility/EncounterCard.tsx (1)
24-28: LGTM! Permission check implementation looks good.The permission check is well-implemented, using the encounter's permissions.
src/components/Patient/PatientDetailsTab/EncounterHistory.tsx (2)
32-36: LGTM! Permission check implementation looks good.The permission check is well-implemented, using the patient's permissions.
51-57: Consider adding a loading state during redirection.Similar to EncounterUpdatesTab, users might see a flash of content before being redirected.
src/components/Patient/PatientDetailsTab/Appointments.tsx (4)
35-40: LGTM! Permission check implementation looks good.The permission check is well-implemented, using the patient's permissions.
42-49: Consider adding error handling for query failures.While the query is correctly enabled based on permissions, there's no error handling for query failures.
51-57: Consider adding a loading state during redirection.Similar to EncounterUpdatesTab, users might see a flash of content before being redirected.
88-98: LGTM! Conditional rendering looks good.The "Schedule Appointment" button is correctly rendered based on permissions.
src/components/Patient/PatientDetailsTab/patientUpdates.tsx (2)
3-5: LGTM!The imports are well-organized and necessary for implementing the permission-based functionality.
Also applies to: 17-25
31-38: LGTM!The permission check is well-implemented, and the query is correctly configured to execute only when the user has the required permissions. This optimization prevents unnecessary API calls.
Also applies to: 51-51
src/components/Patient/PatientDetailsTab/Demography.tsx (2)
12-17: LGTM!The imports are well-organized and necessary for implementing the permission-based functionality.
27-31: LGTM!The permission check is well-implemented, and the allowEdit flag is correctly updated based on the user's permissions.
Also applies to: 134-134
src/components/Patient/PatientDetailsTab/PatientUsers.tsx (3)
41-47: LGTM!The imports are well-organized and necessary for implementing the permission-based functionality.
212-218: LGTM!The permission check is well-implemented with clean destructuring of the required permissions.
276-310: LGTM!The conditional rendering is well-implemented, ensuring that UI elements are only shown to users with appropriate permissions.
Also applies to: 336-336
src/pages/Patients/VerifyPatient.tsx (3)
25-34: LGTM!The imports are well-organized and necessary for implementing the permission-based functionality.
43-48: LGTM!The permission checks are well-implemented with a logical computation of canPerformActions.
86-92: LGTM!The useEffect hook correctly handles unauthorized access with appropriate error feedback and redirection.
src/pages/Encounters/tabs/EncounterNotesTab.tsx (2)
55-65: LGTM! Well-structured permission management implementation.The implementation correctly:
- Imports and uses the permission hooks
- Derives granular permissions for viewing and writing
- Considers both user permissions and encounter status for write access
Also applies to: 329-337
346-346: LGTM! Proper query access control.Queries are correctly gated behind the
canAccesspermission check, preventing unauthorized data access.Also applies to: 377-377
Attached BE PR, feel free to test locally. Everything except for perms on facility settings page (departments/users) should be good to go 🤔 But again 😅 |
|
Conflicts have been detected against the base branch. Please merge the base branch into your branch.
|
|
Testing on going based on the Gsheet
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/components/Files/FilesTab.tsx (2)
523-523: Check custom rounded-y class usage.
Ifrounded-yis a custom utility, please ensure consistency and clarity across styles.
529-529: Reconfirm therounded-yutility.
Same styling note: confirm its presence in your tailwind/theme definitions for consistency.src/components/Patient/PatientInfoCard.tsx (1)
450-468: Consider simplifying the conditional rendering.The nested ternary operators make the code slightly harder to follow. Consider using a more explicit approach for better readability.
- ) : canWrite ? ( - <Badge variant="outline"> - <LocationSheet - facilityId={props.encounter.facility.id} - encounterId={props.encounter.id} - trigger={ - <div className="flex items-center gap-1 text-gray-950 py-0.5 cursor-pointer hover:bg-secondary-100"> - <CareIcon - icon="l-location-point" - className="h-4 w-4 text-green-600" - /> - {t("add_location")} - </div> - } - history={encounter.location_history} - /> - </Badge> - ) : ( - <></> - )} + ) : canWrite ? ( + <Badge variant="outline"> + <LocationSheet + facilityId={props.encounter.facility.id} + encounterId={props.encounter.id} + trigger={ + <div className="flex items-center gap-1 text-gray-950 py-0.5 cursor-pointer hover:bg-secondary-100"> + <CareIcon + icon="l-location-point" + className="h-4 w-4 text-green-600" + /> + {t("add_location")} + </div> + } + history={encounter.location_history} + /> + </Badge> + ) : null}src/components/Patient/diagnosis/list.tsx (1)
169-177: Consider using early return pattern for edit link rendering.The current conditional rendering approach works, but for improved readability, consider extracting the edit link rendering into a separate function with an early return pattern.
- {facilityId && encounterId && canEdit && ( - <Link - href={`/facility/${facilityId}/patient/${patientId}/encounter/${encounterId}/questionnaire/diagnosis`} - className="flex items-center gap-1 text-sm hover:text-gray-500 text-gray-950" - > - <CareIcon icon="l-pen" className="w-4 h-4" /> - {t("edit")} - </Link> - )} + {renderEditLink()} // Add this function before return statement: // function renderEditLink() { // if (!facilityId || !encounterId || !canEdit) return null; // // return ( // <Link // href={`/facility/${facilityId}/patient/${patientId}/encounter/${encounterId}/questionnaire/diagnosis`} // className="flex items-center gap-1 text-sm hover:text-gray-500 text-gray-950" // > // <CareIcon icon="l-pen" className="w-4 h-4" /> // {t("edit")} // </Link> // ); // }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
public/locale/en.json(4 hunks)src/components/Facility/FacilityHome.tsx(6 hunks)src/components/Files/FilesTab.tsx(9 hunks)src/components/Medicine/MedicationAdministration/AdministrationTab.tsx(12 hunks)src/components/Patient/EncounterQuestionnaire.tsx(3 hunks)src/components/Patient/PatientHome.tsx(4 hunks)src/components/Patient/PatientInfoCard.tsx(4 hunks)src/components/Patient/allergy/list.tsx(8 hunks)src/components/Patient/diagnosis/list.tsx(6 hunks)src/components/Patient/symptoms/list.tsx(5 hunks)src/pages/Encounters/tabs/EncounterNotesTab.tsx(7 hunks)src/pages/Facility/overview.tsx(3 hunks)src/pages/UserDashboard.tsx(3 hunks)src/types/emr/encounter.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- src/types/emr/encounter.ts
- src/pages/Facility/overview.tsx
- src/components/Facility/FacilityHome.tsx
- src/components/Patient/symptoms/list.tsx
- src/components/Medicine/MedicationAdministration/AdministrationTab.tsx
- public/locale/en.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (58)
src/pages/UserDashboard.tsx (5)
12-13: Good implementation of permission-based access control.The import of
getPermissionsfunction is an appropriate step towards implementing the permission-based rendering approach mentioned in the PR objectives.
15-15: Good choice using context for permissions management.Using a context-based approach with
usePermissionsis a clean way to access permission data throughout the component tree without prop drilling.
25-29: Well structured permission check implementation.The code properly extracts the
hasPermissionfunction from the context and uses it along with the user's permissions to determine specific access rights. This approach enables granular control over feature access.
70-70: Access control improvement: role-based to permission-based.The change from checking
user.is_superuserto usingcanWriteQuestionnairealigns perfectly with the PR objectives by implementing permission-based rendering rather than role-based checks. This allows for more flexible access control where users can be granted specific permissions without needing full superuser privileges.This change supports the PR testing item regarding admin and facility admin roles' access to questionnaires, providing a mechanism to control that access based on permissions rather than roles.
12-70:❓ Verification inconclusive
Verify permission propagation across components.
To ensure consistent behavior, it's important to verify that the permission system is properly implemented across all relevant components that interact with questionnaires.
🏁 Script executed:
#!/bin/bash # Check for other components that might need similar permission updates for questionnaires # Look for components that reference questionnaires echo "Searching for components that reference questionnaires..." rg -i "questionnaire" --type tsx --type ts --glob "!*.test.*" -l # Check for any remaining superuser checks that might need to be converted to permission-based echo "\nSearching for remaining superuser checks..." rg -i "is_superuser" --type tsx --type ts --glob "!*.test.*" -A 2 -B 2Length of output: 417
Below is a revised review comment that reflects our investigation so far. Since our initial automated query encountered errors recognizing TSX files (due to missing file type registration), we recommend a manual/adjusted re-check to ensure that all questionnaire-related components use the proper permission logic.
Action Required: Manually verify permission propagation for questionnaires
- The code in
src/pages/UserDashboard.tsxnow uses the updated permission retrieval (getPermissionsandhasPermission) correctly.- However, our automated search for components referencing “questionnaire” (and for legacy
is_superuserchecks) failed because TSX files were not recognized by default. Please re-run your search with TSX file types explicitly added (for example, usingrg --type-add 'tsx:*.tsx' ...) or verify manually.- Confirm that all components interacting with questionnaires have updated permission checks (i.e. no remaining hard-coded superuser privileges).
src/components/Files/FilesTab.tsx (14)
31-31: New Tooltip import looks good.
No issues identified with switching to the newTooltipComponent.
43-43: ImportinggetPermissionsis appropriate.
This aligns well with the new permission checks in the file.
51-51: New import ofinactiveEncounterStatusis verified.
It’s used correctly to gate write permissions against inactive Encounters.
56-56: Facility ID prop addition is fine, but there's an inconsistency in the summary.
The AI-generated summary states thatpatientwas changed from optional to required, but the code still showspatient?: Patient;.Likely an incorrect or invalid review comment.
63-63: Destructuring props with a default subPage is clear.
UsingsubPage = "all"prevents undefined checks later on.
75-83: New permission variables initialized fromgetPermissions.
This pattern helps keep permission checks consistent across the codebase.
84-88: ConditionalcanAccesslogic is appropriate.
Restricting file viewing tocanViewClinicalDataorcanViewEncounterlooks correct.
89-92: Write permissions are blocked for inactive encounters.
Short-circuiting with!inactiveEncounterStatus.includes(...)is a clean approach.
139-139:enabled: canAccessin the query is good for performance.
This prevents unnecessary network calls if the user lacks permission.
249-249: ReturningcanWriteCurrentEncounterrationalizes editing checks.
Adhering to the global permission logic keeps it consistent.
251-251: ReturningcanWritePatientfor patient edits is consistent.
Good reuse of the permission variable.
520-520: Table column width adjustment to 20% seems intentional.
No further action required if the layout matches design specs.
556-560: Tooltip surrounding truncated file name is good for usability.
This ensures the user can see the full file name on hover.
577-585: Time-zone handling for displayed dates remains unaddressed.
This mirrors a prior recommendation to usedayjs.tz(...)to avoid confusion in different user locales.src/components/Patient/PatientInfoCard.tsx (5)
59-64: Good implementation of permission control for PatientInfoCard component.Adding the
canWriteproperty to the interface is a clean approach to control permission-based rendering.
67-68: Implementation aligns with interface changes.The destructuring of props correctly includes the new
canWriteparameter, maintaining consistency with the interface changes.
352-381: Effective conditional rendering based on permissions.The implementation properly separates the editable and view-only states for organization management. When users have write permissions, they see the full
LinkDepartmentsSheetcomponent; otherwise, they only see the organization badges without editing capabilities.
432-446: Consistent permission pattern for location updates.The conditional rendering for the update location button follows the same pattern, ensuring consistency in the permission model throughout the component.
479-548: Well-implemented update/completion actions based on permissions.The conditional rendering for the update actions, including the "Mark as Complete" functionality, correctly depends on the
canWritepermission and maintains consistency with the permission model implemented in the component.src/components/Patient/PatientHome.tsx (6)
24-25: Good integration of permission utilities.The import of
getPermissionsfrom the common module indicates proper architectural organization of permission-related code.
30-30: Correct use of permission hooks.Using the
usePermissionshook aligns with React best practices for accessing global context values.
41-42: Proper initialization of permission hooks.The hook is correctly destructured to access the
hasPermissionfunction.
53-60: Well-structured permission and tab handling logic.The code effectively integrates permission checks with dynamic tab generation, ensuring users only see tabs they have permission to access.
66-66: Concise tab assignment based on context.The ternary operator effectively selects the appropriate tabs based on whether this is a facility context or not.
79-87: Proper permission-based rendering of appointment scheduling.The appointment scheduling button is now correctly gated behind both the facility context check and the permission check.
src/components/Patient/EncounterQuestionnaire.tsx (4)
13-14: Good import of permission utilities.The import of
getPermissionsfrom the common module indicates proper architectural organization of permission-related code.
18-19: Effective use of permission hooks and encounter status constants.The component correctly imports both the permission hooks and the encounter status constants needed for determining edit capabilities.
46-56: Comprehensive permission logic for encounter interaction.The permission checks are thorough and consider both user permissions and encounter state:
- Using the permission hook to get the base permission function
- Getting specific permission capabilities for the encounter
- Computing a clear
canWriteflag based on all relevant factorsThis approach ensures robust permission enforcement.
62-68: Correctly passing permission state to child components.The
canWriteflag is properly passed to thePatientInfoCardcomponent, ensuring that UI elements in child components respect user permissions.src/components/Patient/diagnosis/list.tsx (8)
14-15: Good import of permission utilities.The import of
getPermissionsaligns with the pattern seen in other components.
17-20: Effective import of permission hooks and encounter status constants.The component correctly imports both the permission hooks and the encounter status constants needed for determining access and edit capabilities.
23-30: Well-defined interface with appropriate permission properties.The interface has been updated to include:
encounterStatusto check if the encounter is active/editablepermissionsarray to determine user capabilitiesThis provides a clean contract for the component.
32-52: Comprehensive permission logic with distinct access levels.The component defines two separate permission flags:
canAccess- Controls whether the user can view diagnosis datacanEdit- Controls whether the user can modify diagnosesThis separation of concerns is excellent for fine-grained permission control, especially considering both user permissions and encounter status.
60-61: Query enabled conditionally based on permissions.The addition of the
enabled: canAccessproperty to the query is an important performance and security improvement, ensuring data is only fetched when the user has proper permissions.
90-101: Passing canEdit to layout component for empty state.The component correctly passes the
canEditpermission to the layout component even when there are no diagnoses to display, ensuring consistent permission enforcement.
105-146: Consistent permission handling in populated state.The component maintains the same permission checks when there are diagnoses to display, ensuring a consistent user experience regardless of data state.
148-182: Clean implementation of conditional edit link.The
DiagnosisListLayoutcomponent now properly gates the edit link behind the combined condition of having a facility ID, encounter ID, and edit permissions. This ensures that only users with appropriate permissions can see and use the edit functionality.src/pages/Encounters/tabs/EncounterNotesTab.tsx (8)
60-61: Imports look good.These added imports from
"@common/Permissions","@context/PermissionContext", and"@types/emr/encounter"properly support the new permission-based logic.Also applies to: 67-67, 69-69
334-341: Ensure consistent handling of missing permissions.While this permission-based logic is sound, consider verifying that
encounter.permissionsis always defined to avoid unexpected issues if null or undefined. Otherwise, this is a good and clear use of permission checks.
351-351: Query enablement logic confirmed.Restricting the threads query to
canAccessproperly prevents accidental data fetching for unauthorized users.
382-382: Additional query guard is valid.Requiring both
selectedThreadandcanAccessto be true for the messages query is a safe approach to protect data and avoid unnecessary requests.
502-512: Conditional creation button is well-implemented.Hiding the “New Thread” button for users who lack
canWriteCurrentEncounteris a clean and direct enforcement of permissions.
680-715: Protected message input.Conditionally rendering the message form for writable encounters properly prevents unintended data submission. This approach cleanly separates read access from write access.
731-731: Good disabled state control.Disabling the button when
canWriteCurrentEncounteris false ensures the UI reflects permission restrictions in a consistent manner.
736-742: Clear feedback for restricted encounters.Displaying a helpful message for inactive or non-writable encounters improves user experience by indicating why the discussion start is disabled.
src/components/Patient/allergy/list.tsx (8)
36-37: New imports are aligned with permission logic.These additions facilitate the usage of permission functions and hooks without introducing any conflicts.
Also applies to: 40-40
49-53: Encounter-related imports are valid.Bringing in
Encounter,completedEncounterStatus, andinactiveEncounterStatushere supports the extended logic for filtering and editing.
62-62: Extra prop for permissions is appropriate.Defining the
permissionsproperty in theAllergyListPropsinterface clarifies the contract for permission checks within this component.
84-84: Robust permission checks.Splitting out
canAccessandcanEditis a clear approach. Ensure that edge cases (e.g., missingencounterStatus) are handled gracefully, especially whenencounterorpermissionsmight be absent.Also applies to: 86-97
111-111: Conditional query fetch is correct.Using
enabled: canAccessprevents loading allergy data for users who lack the appropriate clinical or encounter permissions.
142-142: Passing canEdit to layout is transparent.Exposing
canEditvia props cleanly separates the presentation logic from permission checks, improving maintainability.
241-241: Consistent prop usage.Continuing to pass
canEditwhen rendering the component ensures uniform permission logic in all relevant UI sections.
327-327: Edit link guarded by permissions.Conditionally displaying the edit link with
canEditprevents unauthorized users from editing allergy data, further aligning with secure design principles.
This will probably happen later on, since we currently don't have FE interface for creating organizations.
Backend currently allows volunteer users to see other volunteer users - hmm 🤔 Product decision.
They can't directly access, but can through patients tab under organization. Though I'm wondering if geo-admin should even have access to patients. Another product question.
Product question since BE currently doesn't support this for volunteer role. |
|
Conflicts have been detected against the base branch. Please merge the base branch into your branch.
|
|
@Jacobjeevan status here? |
nihal467
added
needs testing
needs review
P1
|
|
|
Conditionally approving this due to its priority nature.
CC: @gigincg |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/types/facility/facility.ts (1)
20-20: Consider enhancing type safety for permissions.Adding the permissions property is appropriate for implementing permission-based rendering. However, using a simple string array offers flexibility at the cost of type safety.
Consider using a more type-safe approach such as string literals or an enum to restrict possible permission values:
- permissions: string[]; + permissions: Array<'view_facility' | 'edit_facility' | 'delete_facility' | 'manage_users' | /* other specific permissions */>;Alternatively, you could create a dedicated permissions type:
export type FacilityPermission = 'view_facility' | 'edit_facility' | /* other permissions */;And then reference it:
permissions: FacilityPermission[];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/Facility/FacilityHome.tsx(6 hunks)src/components/Patient/PatientInfoCard.tsx(4 hunks)src/types/facility/facility.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/Patient/PatientInfoCard.tsx
- src/components/Facility/FacilityHome.tsx
🔇 Additional comments (2)
src/types/facility/facility.ts (2)
23-23: Good practice for permission management.Excluding permissions from the creation type is a good security practice. This ensures that permissions are assigned through proper authorization channels rather than directly during facility creation.
39-41:❓ Verification inconclusive
Consider migration strategy for existing facilities.
The addition of these required permission fields might impact existing facilities in the database that don't have these fields populated yet.
Is there a migration plan in place? Let's check if there are corresponding backend changes:
🏁 Script executed:
#!/bin/bash # Look for migration files or backend changes related to permissions echo "Searching for migration files..." fd -t f migration -i # Look for backend models related to facilities echo "Searching for facility models in the backend..." fd -t f facility -i modelLength of output: 323
Action Required: Confirm Migration Handling for Permission Fields
The current verification did not reveal any migration scripts or backend updates addressing the new required permission fields (
permissions,root_org_permissions,child_org_permissions). Since these fields are now mandatory, existing facilities in the database may lack appropriate values, potentially causing runtime issues.
- Migration Scripts: Ensure that a migration strategy is in place—either by adding new migration scripts or updating existing ones—to set default values for these fields in existing records.
- Backend Model Updates: Verify that the backend facility models have been updated accordingly to handle these fields, ensuring consistency between the database and the application.
Please verify that the necessary migration plan and backend changes are implemented and aligned with these new requirements.
| permissions: string[]; | ||
| root_org_permissions: string[]; | ||
| child_org_permissions: string[]; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Document the purpose of different permission levels.
The addition of three distinct permission arrays (permissions, root_org_permissions, child_org_permissions) suggests a hierarchical permission model, but their specific purposes and relationships aren't documented.
Consider adding documentation comments to clarify:
+ /** Permissions that apply directly to this facility */
permissions: string[];
+ /** Permissions inherited from or applicable to the root organization */
root_org_permissions: string[];
+ /** Permissions that apply to child organizations of this facility */
child_org_permissions: string[];Does the codebase have a central place where these permission types are defined or documented? Let's check:
🏁 Script executed:
#!/bin/bash
# Look for permission-related constants or documentation
echo "Searching for permission-related files..."
fd -t f permission -i
echo "Searching for permission constants..."
rg -i "permission.*constants|PERMISSION" --type tsLength of output: 48373
Action: Add inline documentation for facility permission arrays
After reviewing the repository, it’s clear that while permission constants and their usage are documented in files like src/common/Permissions.tsx, there isn’t a dedicated explanation for the hierarchical permission arrays in src/types/facility/facility.ts. In this file, the three arrays—permissions, root_org_permissions, and child_org_permissions—are only typed as string arrays, making their specific roles unclear.
To improve clarity and future maintainability, please add inline documentation comments that explain:
permissions: Permissions that apply directly to this facility.root_org_permissions: Permissions inherited from or applicable to the root organization.child_org_permissions: Permissions that apply to child organizations of this facility.
For example:
+ /** Permissions that apply directly to this facility */
permissions: string[];
+ /** Permissions inherited from or applicable to the root organization */
root_org_permissions: string[];
+ /** Permissions that apply to child organizations of this facility */
child_org_permissions: string[];📝 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.
| permissions: string[]; | |
| root_org_permissions: string[]; | |
| child_org_permissions: string[]; | |
| /** Permissions that apply directly to this facility */ | |
| permissions: string[]; | |
| /** Permissions inherited from or applicable to the root organization */ | |
| root_org_permissions: string[]; | |
| /** Permissions that apply to child organizations of this facility */ | |
| child_org_permissions: string[]; |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/pages/Patients/VerifyPatient.tsx (2)
91-97: Early return to avoid rendering
After navigating away due to lack of permissions, consider returningnullto prevent UI flicker before redirect.useEffect(() => { if (!canPerformActions) { toast.error(t("no_permission_to_view_page")); goBack("/"); + return; } }, [canPerformActions]);
143-232: Consider splitting conditional sections
Rendering "quick actions" and "active encounters" based on different permission flags is good. For readability, extracting these blocks into subcomponents might help maintainability.src/components/Medicine/MedicationRequestTable/index.tsx (1)
115-122: Return after navigation
When you show a toast and navigate for insufficient permissions, returningnullprevents leftover UI from rendering.if (!canAccess) { toast.error("You do not have permission to view this encounter"); goBack(); + return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
public/locale/en.json(4 hunks)src/components/Medicine/MedicationRequestTable/index.tsx(5 hunks)src/components/Patient/EncounterQuestionnaire.tsx(3 hunks)src/components/Users/UserAvailabilityTab.tsx(4 hunks)src/pages/Encounters/tabs/EncounterNotesTab.tsx(7 hunks)src/pages/Patients/VerifyPatient.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/Users/UserAvailabilityTab.tsx
- src/components/Patient/EncounterQuestionnaire.tsx
- public/locale/en.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (26)
src/pages/Patients/VerifyPatient.tsx (6)
26-27: No issues with the new import statements
ImportinggetPermissionsfrom "@/common/Permissions" aligns well with your existing permission-check approach.
33-33: Consistent use of PermissionContext
The import forusePermissionsneatly integrates permission checks at the top of the file.
41-42: Minimal destructuring
UsingusePermissions()and extractinghasPermissionhelps keep the permission logic focused.
43-49: Add robust error handling
While usinguseQueryfor facility data is solid, consider adding an error-handling path to gracefully handle 4xx/5xx responses.
50-53: Straightforward permission extraction
DerivingcanCreateAppointment,canCreateEncounter, andcanListEncountersis a clear way to centralize your role-based logic. Ensure stale data is refreshed if the facility ID changes.
78-78: Clean conditional enabling
enabled: !!patientData?.id && canListEncounterssuccinctly avoids fetching encounters if permissions or patient data are invalid.src/components/Medicine/MedicationRequestTable/index.tsx (13)
5-6: Relevant imports
Adding React hooks (useEffect,useState) andtoastfrom Sonner solidifies user feedback capabilities and state management.
19-19: Navigational consistency
ImportinguseAppHistorymatches your typical navigation pattern. Verify possible edge cases when going back from this component.
21-21: Centralized permission logic
getPermissionsis used across your application, ensuring consistent role-based checks.
24-25: Clear domain imports
Pulling inusePermissions,Encounter, andinactiveEncounterStatuskeeps the domain logic neatly organized.
74-74: DestructuringhasPermission
UtilizingusePermissions()for a straightforward boolean is a clean approach to permission logic.
75-79: Granular role-based checks
canViewClinicalData,canViewEncounter, andcanSubmitEncounterQuestionnaireprovide precise toggles for which features to expose.
80-80: Simple composite condition
const canAccess = canViewClinicalData || canViewEncounter;neatly consolidates read access logic.
83-86: Write-limiting condition
canWriteCurrentEncountercorrectly restricts edits for inactive encounters. Great for preventing unintended modifications.
92-92: Encounter parameter usage
Includingencounter: encounter.idclarifies the scope of your medication queries, reducing potential confusion.
97-97: Conditional query
enabled: !!patientId && canAccessensures resource fetching only happens if permissions are valid and the ID is valid.
105-105: Consistency in stopped queries
Settingencounter: encounter.idfor the stopped medications query mirrors the active meds query for consistent data retrieval.
112-112: Aligned gating logic
ReusingcanAccessensures the stopped medications query doesn't run if the user lacks permissions, maintaining uniformity.
258-260: Structured prop passing
ForwardingencounterId,encounterStatus, andencounterPermissionstoAdministrationTabhelps keep the logic modular and maintainable.src/pages/Encounters/tabs/EncounterNotesTab.tsx (7)
60-60: Central permission import
UsinggetPermissionsfrom "@/common/Permissions" adds consistent role-based checks for the encounters module.
67-67: Tapping into PermissionContext
ImportingusePermissionsmirrors the pattern across the rest of your code, ensuring uniform permission checks.
345-351: Comprehensive permission checks
Destructuring and combiningcanViewClinicalData,canViewEncounter, andcanWriteEncounterintocanAccessandcanWriteCurrentEncounterkeeps the logic readable and flexible.
507-517: Conditional new discussion
Showing the "New" button exclusively forcanWriteCurrentEncounterprevents unauthorized thread creation.
694-728: Guarded message input
Conditionally rendering the message form based oncanWriteCurrentEncountercorrectly restricts updates to allowed users only.
744-744: Disabling the start discussion button
disabled={!canWriteCurrentEncounter}gracefully blocks certain roles from initiating threads.
750-755: Clear feedback on restricted encounters
Providing a message to indicate why a discussion cannot be started enhances user clarity and usability.
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Refactor