Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 69 additions & 0 deletions src/common/Permissions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,22 @@ export const PERMISSION_READ_ACCOUNT = "can_read_account";
export const PERMISSION_MANAGE_LOCKED_INVOICE =
"can_manage_locked_invoice_in_facility";

// Service Request Permissions
export const PERMISSION_READ_SERVICE_REQUEST = "can_read_service_request";

// Pharmacy Permissions
export const PERMISSION_VIEW_AS_PHARMACIST = "is_pharmacist";

// Inventory Permissions
export const PERMISSION_READ_INVENTORY = "can_read_inventory_item";

// Supply Delivery Permissions
export const PERMISSION_READ_SUPPLY_DELIVERY = "can_read_supply_delivery";
export const PERMISSION_WRITE_SUPPLY_DELIVERY = "can_write_supply_delivery";

// Supply Request Permissions
export const PERMISSION_READ_SUPPLY_REQUEST = "can_read_supply_request";
export const PERMISSION_WRITE_SUPPLY_REQUEST = "can_write_supply_request";
export interface Permissions {
// Patient Permissions
/** Permission slug: "can_create_patient" */
Expand Down Expand Up @@ -279,6 +295,25 @@ export interface Permissions {

/** Permission slug: "can_manage_locked_invoice_in_facility" */
canManageLockedInvoice: boolean;

/** Permission slug: "can_read_service_request" */
canReadServiceRequest: boolean;

/** Permission slug: "is_pharmacist" */
canViewAsPharmacist: boolean;

/** Permission slug: "can_read_inventory_item" */
canReadInventory: boolean;

/** Permission slug: "can_read_supply_delivery" */
canReadSupplyDelivery: boolean;
/** Permission slug: "can_write_supply_delivery" */
canWriteSupplyDelivery: boolean;

/** Permission slug: "can_read_supply_request" */
canReadSupplyRequest: boolean;
/** Permission slug: "can_write_supply_request" */
canWriteSupplyRequest: boolean;
}

export type HasPermissionFn = (
Expand Down Expand Up @@ -508,5 +543,39 @@ export function getPermissions(
PERMISSION_MANAGE_LOCKED_INVOICE,
permissions,
),

// Service Request
canReadServiceRequest: hasPermission(
PERMISSION_READ_SERVICE_REQUEST,
permissions,
),

// Medication Request
canViewAsPharmacist: hasPermission(
PERMISSION_VIEW_AS_PHARMACIST,
permissions,
),

// Inventory
canReadInventory: hasPermission(PERMISSION_READ_INVENTORY, permissions),

// Supply Delivery
canReadSupplyDelivery: hasPermission(
PERMISSION_READ_SUPPLY_DELIVERY,
permissions,
),
canWriteSupplyDelivery: hasPermission(
PERMISSION_WRITE_SUPPLY_DELIVERY,
permissions,
),
// Supply Request
canReadSupplyRequest: hasPermission(
PERMISSION_READ_SUPPLY_REQUEST,
permissions,
),
canWriteSupplyRequest: hasPermission(
PERMISSION_WRITE_SUPPLY_REQUEST,
permissions,
),
};
}
37 changes: 35 additions & 2 deletions src/components/ui/sidebar/facility/location/location-nav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,32 @@ import CareIcon from "@/CAREUI/icons/CareIcon";

import { NavMain } from "@/components/ui/sidebar/nav-main";

import { getPermissions } from "@/common/Permissions";
import { usePermissions } from "@/context/PermissionContext";
import useCurrentLocation from "@/pages/Facility/locations/utils/useCurrentLocation";
import useCurrentFacility from "@/pages/Facility/utils/useCurrentFacility";
import { CalendarIcon, Logs } from "lucide-react";

export function LocationNav() {
const { t } = useTranslation();

const { facilityId } = useCurrentFacility();
const { locationId } = useCurrentLocation();
const { facilityId, facility } = useCurrentFacility();
const { locationId, location } = useCurrentLocation();
const { hasPermission } = usePermissions();
const {
canListFacilityLocations,
canReadServiceRequest,
canReadSupplyDelivery,
canReadInventory,
canReadSupplyRequest,
canViewSchedule,
canViewAppointments,
canListTokens,
} = getPermissions(hasPermission, location?.permissions ?? []);
const { canViewAsPharmacist } = getPermissions(
hasPermission,
facility?.permissions ?? [],
);
Comment on lines +29 to +32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Facility permission field usage across repo =="
rg -nP 'facility\?\.(permissions|root_org_permissions)|facility\.(permissions|root_org_permissions)' --type ts --type tsx src

echo
echo "== Facility model/type declarations =="
rg -nP 'interface\s+\w*Facility|type\s+\w*Facility' --type ts --type tsx src/types src/pages src/components

echo
echo "== Direct declarations of permission fields in type/model files =="
rg -nP '\b(permissions|root_org_permissions)\b' --type ts --type tsx src/types

Repository: ohcnetwork/care_fe

Length of output: 156


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Facility permission field usage across repo =="
rg -nP 'facility\?\.(permissions|root_org_permissions)|facility\.(permissions|root_org_permissions)' src --glob '*.ts' --glob '*.tsx'

echo
echo "== Facility model/type declarations =="
rg -nP 'interface\s+\w*Facility|type\s+\w*Facility' src/types src/pages src/components --glob '*.ts' --glob '*.tsx'

echo
echo "== Check location-nav.tsx file =="
cat -n src/components/ui/sidebar/facility/location/location-nav.tsx | head -40

echo
echo "== Check HealthcareServiceShow.tsx file =="
cat -n src/pages/Facility/services/HealthcareServiceShow.tsx | sed -n '125,145p'

echo
echo "== Facility type definition =="
rg -nP 'permissions|root_org_permissions' src/types --glob '*.ts' --glob '*.tsx' -B 2 -A 2

Repository: ohcnetwork/care_fe

Length of output: 10215


Use consistent facility.permissions across navigation components for uniform permission checking.

Line 31 uses facility?.permissions, but src/pages/Facility/services/HealthcareServiceShow.tsx Line 134 and src/components/ui/sidebar/facility/service/service-nav.tsx Line 29 use facility?.root_org_permissions. All other usages in the codebase (22+ locations) follow the facility?.permissions pattern. This inconsistency can cause permission-based navigation visibility to diverge if only one permission field is populated.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/ui/sidebar/facility/location/location-nav.tsx` around lines 29
- 32, The navigation uses inconsistent permission fields: update the call to
getPermissions in location-nav (where canViewAsPharmacist is derived) to use the
same permission source as other nav components—prefer facility?.permissions—so
replace any use of facility?.root_org_permissions with facility?.permissions (or
ensure a canonical selector) when calling getPermissions in the location-nav
component (referencing getPermissions and canViewAsPharmacist) so all
sidebar/service/location/nav components check the same permission array.


const baseUrl = `/facility/${facilityId}/locations/${locationId}`;

Expand All @@ -23,11 +40,13 @@ export function LocationNav() {
name: t("beds"),
url: `${baseUrl}/beds`,
icon: <CareIcon icon="l-bed" />,
visibility: canListFacilityLocations,
},
{
name: t("laboratory"),
url: `${baseUrl}/laboratory`,
icon: <CareIcon icon="l-microscope" />,
visibility: canReadServiceRequest,
children: [
{
name: t("service_requests"),
Expand All @@ -39,64 +58,78 @@ export function LocationNav() {
name: t("pharmacy"),
url: `${baseUrl}/pharmacy`,
icon: <CareIcon icon="l-medical-drip" />,
visibility: canViewAsPharmacist || canReadSupplyDelivery,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Tighten pharmacy workflow visibility to pharmacy-specific permission.

Lines 61, 71, and 76 allow canReadSupplyDelivery to expose pharmacy workflows. This broadens access beyond canViewAsPharmacist and can reveal medication-facing routes to non-pharmacy roles.

🔒 Proposed fix
-          visibility: canViewAsPharmacist || canReadSupplyDelivery,
+          visibility: canViewAsPharmacist,
@@
-              visibility: canViewAsPharmacist || canReadSupplyDelivery,
+              visibility: canViewAsPharmacist,
@@
-              visibility: canReadSupplyDelivery,
+              visibility: canViewAsPharmacist,

Also applies to: 71-71, 76-76

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/ui/sidebar/facility/location/location-nav.tsx` at line 61, The
visibility flags currently allow pharmacy workflows to be shown when either
canViewAsPharmacist or canReadSupplyDelivery is true; tighten this by changing
those visibility conditions to depend only on canViewAsPharmacist. Locate the
visibility properties in the LocationNav sidebar items (where visibility:
canViewAsPharmacist || canReadSupplyDelivery is used) and remove the
canReadSupplyDelivery operand so the expression is just canViewAsPharmacist;
ensure the same change is applied to the other two occurrences that reference
canReadSupplyDelivery alongside canViewAsPharmacist.

children: [
{
name: t("prescription_queue"),
url: `${baseUrl}/medication_requests`,
visibility: canViewAsPharmacist,
},
{
name: "℞ " + t("dispense"),
url: `${baseUrl}/medication_dispense`,
visibility: canViewAsPharmacist || canReadSupplyDelivery,
},
{
name: t("medication_return"),
url: `${baseUrl}/medication_return`,
visibility: canReadSupplyDelivery,
},
],
},
{
name: t("inventory"),
url: `${baseUrl}/inventory/summary`,
icon: <CareIcon icon="l-shop" />,
visibility:
canReadInventory || canReadSupplyRequest || canReadSupplyDelivery,
children: [
{
name: t("items"),
url: `${baseUrl}/inventory/summary`,
visibility: canReadInventory,
},
{
header: t("internal_transfers"),
name: t("to_receive"),
url: `${baseUrl}/inventory/internal/receive/`,
visibility: canReadSupplyRequest || canReadSupplyDelivery,
},
{
name: t("to_dispatch"),
url: `${baseUrl}/inventory/internal/dispatch/`,
visibility: canReadSupplyRequest || canReadSupplyDelivery,
},
{
header: t("external_supply"),
name: t("purchase_orders"),
url: `${baseUrl}/inventory/external/orders/outgoing`,
visibility: canReadSupplyRequest,
},
{
name: t("purchase_deliveries"),
url: `${baseUrl}/inventory/external/deliveries/incoming`,
visibility: canReadSupplyDelivery,
},
],
},
{
name: t("schedule"),
url: `${baseUrl}/schedule`,
icon: <CalendarIcon />,
visibility: canViewSchedule,
},
{
name: t("appointments"),
url: `${baseUrl}/appointments`,
icon: <CareIcon icon="d-calendar" />,
visibility: canViewAppointments,
},
{
name: t("queues"),
url: `${baseUrl}/queues`,
icon: <Logs />,
visibility: canListTokens,
},
]}
/>
Expand Down
27 changes: 27 additions & 0 deletions src/components/ui/sidebar/facility/service/service-nav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,30 @@ import CareIcon from "@/CAREUI/icons/CareIcon";

import { NavMain } from "@/components/ui/sidebar/nav-main";

import { getPermissions } from "@/common/Permissions";
import { usePermissions } from "@/context/PermissionContext";
import useCurrentService from "@/pages/Facility/services/utils/useCurrentService";
import useCurrentFacility from "@/pages/Facility/utils/useCurrentFacility";
import { Logs } from "lucide-react";

export function ServiceNav() {
const { t } = useTranslation();

const { service, facilityId } = useCurrentService();
const { facility } = useCurrentFacility();
const { hasPermission } = usePermissions();
const { canViewSchedule, canViewAppointments, canListTokens } =
getPermissions(hasPermission, service?.permissions ?? []);
const { canReadHealthcareService } = getPermissions(
hasPermission,
facility?.permissions ?? [],
);
const {
canViewSchedule: canViewScheduleForFacility,
canViewAppointments: canViewAppointmentsForFacility,
canListTokens: canListTokensForFacility,
} = getPermissions(hasPermission, facility?.root_org_permissions ?? []);
const hasManagingOrganization = !!service?.managing_organization?.id;

const baseUrl = `/facility/${facilityId}/services/${service?.id}`;

Expand All @@ -21,21 +38,31 @@ export function ServiceNav() {
name: t("locations"),
url: `${baseUrl}/locations`,
icon: <CareIcon icon="l-map-pin" />,
visibility: canReadHealthcareService,
},
{
name: t("schedule"),
url: `${baseUrl}/schedule`,
icon: <CareIcon icon="l-calender" />,
visibility: hasManagingOrganization
? canViewSchedule
: canViewScheduleForFacility,
},
{
name: t("appointments"),
url: `${baseUrl}/appointments`,
icon: <CareIcon icon="d-calendar" />,
visibility: hasManagingOrganization
? canViewAppointments
: canViewAppointmentsForFacility,
},
{
name: t("queues"),
url: `${baseUrl}/queues`,
icon: <Logs />,
visibility: hasManagingOrganization
? canListTokens
: canListTokensForFacility,
},
]}
/>
Expand Down
31 changes: 18 additions & 13 deletions src/components/ui/sidebar/nav-main.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,17 @@ export function NavMain({ links }: { links: NavigationLink[] }) {
[fullPath],
);

const isAnyChildVisible = (link: NavigationLink) =>
link.children?.some((child) => child.visibility !== false);

return (
<SidebarGroup>
<SidebarMenu>
{links
.filter((link) => link.visibility !== false)
.map((link) => (
<Fragment key={link.name}>
{link.children ? (
{link.children && isAnyChildVisible(link) ? (
isCollapsed ? (
<PopoverMenu link={link} />
) : (
Expand Down Expand Up @@ -228,18 +231,20 @@ function PopoverMenu({ link }: { link: NavigationLink }) {
onCloseAutoFocus={(e) => e.preventDefault()}
>
<div className="flex flex-col gap-1">
{link.children?.map((subItem) => (
<ActiveLink
key={subItem.name}
href={subItem.url}
onClick={() => setOpen(false)}
className="w-full rounded-md px-2 py-1.5 text-sm outline-none transition-colors hover:bg-gray-100 focus:bg-gray-100"
activeClass="bg-gray-100 text-green-700"
exactActiveClass="bg-gray-100 text-green-700"
>
{subItem.name}
</ActiveLink>
))}
{link.children
?.filter((subItem) => subItem.visibility !== false)
.map((subItem) => (
<ActiveLink
key={subItem.name}
href={subItem.url}
onClick={() => setOpen(false)}
className="w-full rounded-md px-2 py-1.5 text-sm outline-none transition-colors hover:bg-gray-100 focus:bg-gray-100"
activeClass="bg-gray-100 text-green-700"
exactActiveClass="bg-gray-100 text-green-700"
>
{subItem.name}
</ActiveLink>
))}
</div>
</PopoverContent>
</Popover>
Expand Down
Loading
Loading