Add support for associating devices to an encounter #10904
Conversation
WalkthroughThis update adds new API endpoints for associating encounters and retrieving device encounter history. Several new React components have been introduced to manage device associations, search functionality, and encounter history display. The changes also update localization files with additional encounter and device association strings and include modifications to existing components for dynamic behavior and enhanced UI, including debounced queries and responsive layouts. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant ADS as AssociateDeviceSheet
participant DS as DeviceSearch
participant API as Device API
participant UI as UI Components
U->>ADS: Opens Associate Device Sheet
ADS->>DS: Renders device search
U->>DS: Searches and selects a device
DS-->>ADS: Returns selected device
ADS->>API: Calls associateEncounter (POST)
API-->>ADS: Responds with DeviceDetail
ADS->>UI: Invalidates query and shows success toast
sequenceDiagram
participant UD as User
participant DD as DeviceDetail
participant DEH as DeviceEncounterHistory
participant API as Device API
participant UI as Encounter List UI
UD->>DD: Clicks encounter history button
DD->>DEH: Renders Encounter History component
DEH->>API: Requests encounterHistory (GET)
API-->>DEH: Returns paginated encounter data
DEH->>UI: Displays encounter history list
UI->>DEH: Triggers query update on pagination change
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/types/device/deviceApi.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test
- GitHub Check: OSSAR-Scan
- GitHub Check: cypress-run (1)
🔇 Additional comments (3)
src/types/device/deviceApi.ts (3)
3-3: Good addition of the Encounter import.The import of the Encounter type is necessary for the new encounter-related endpoints, in line with the PR's objective of associating devices with encounters.
48-53: The associateEncounter endpoint implementation looks good.The endpoint correctly implements a POST method to associate an encounter with a device, taking an encounter ID in the request body and returning the updated DeviceDetail.
48-63: Missing query parameters for filtering.The
encounterHistoryandencounterDevicesendpoints will likely need query parameters for pagination and filtering. Consider whether you need to define these parameters (likepage,limit, orencounter_idfor filtering) in the API definition.Can you confirm whether these endpoints support additional query parameters for filtering and pagination? If so, they should be documented in the API definition.
|
@rithviknishad ready for review! |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/types/device/deviceApi.ts (1)
52-62:⚠️ Potential issuePath parameter naming inconsistency.
There's an inconsistency in the path parameter naming convention. Both new endpoints use
{facilityId}while other endpoints in this file use{facility_id}. This inconsistency could cause issues when building the request URL.Apply this diff to fix the inconsistency:
associateEncounter: { - path: "/api/v1/facility/{facilityId}/device/{id}/associate_encounter/", + path: "/api/v1/facility/{facility_id}/device/{id}/associate_encounter/", method: HttpMethod.POST, TRes: Type<DeviceDetail>(), TBody: Type<{ encounter: string }>(), }, encounterHistory: { - path: "/api/v1/facility/{facilityId}/device/{id}/encounter_history/", + path: "/api/v1/facility/{facility_id}/device/{id}/encounter_history/", method: HttpMethod.GET, TRes: Type<PaginatedResponse<DeviceEncounters>>(), },
🧹 Nitpick comments (9)
src/types/device/device.ts (1)
53-59: Interface looks good but consider documenting date format.The
DeviceEncountersinterface is well-structured and aligns with existing type patterns. However, for thestartandendstring fields, consider adding JSDoc comments to specify the expected date format for better maintainability.export interface DeviceEncounters { id: string; encounter: Encounter; created_by: UserBase; + /** Date string in ISO format (YYYY-MM-DDTHH:mm:ss.sssZ) */ start: string; + /** Date string in ISO format (YYYY-MM-DDTHH:mm:ss.sssZ) */ end: string; }src/pages/Encounters/tabs/EncounterDevicesTab.tsx (1)
74-77: Unused parameter in onChange callback.The second parameter in the
onChangecallback is not being used, as indicated by the underscore. Consider removing it for cleaner code.<Pagination data={{ totalCount: data.count }} - onChange={(page, _) => setPage(page)} + onChange={(page) => setPage(page)} defaultPerPage={limit} cPage={page} />src/pages/Encounters/AssociateDeviceSheet.tsx (1)
52-55: Add validation for the selected device before submission.While you check if
selectedDeviceexists, it's better to also validate that the device ID is available before proceeding with the API call.const handleSubmit = () => { - if (!selectedDevice) return; + if (!selectedDevice || !selectedDevice.id) return; associateDevice({ encounter: encounterId }); };src/pages/Facility/settings/devices/DeviceSearch.tsx (2)
48-57: Improve accessibility and add loading state indicator.The component could benefit from improved accessibility and loading state indication.
<Popover open={open} onOpenChange={setOpen}> <PopoverTrigger asChild disabled={disabled}> <div className="w-full h-9 px-3 rounded-md border text-sm flex items-center justify-between cursor-pointer" role="combobox" aria-expanded={open} + aria-controls="device-search-popover" > - {value?.registered_name || t("select_device")} + <span>{value?.registered_name || t("select_device")}</span> + {devices === undefined && !disabled && open && ( + <span className="ml-2 animate-spin">...</span> + )} </div> </PopoverTrigger>
67-80: Enhance device display with additional information.Consider enhancing the device display to include more information like device type or status for better identification.
<CommandGroup> {devices?.results.map((device) => ( <CommandItem key={device.id} value={device.id} onSelect={() => { onSelect(device); setOpen(false); }} > - {device.registered_name} + <div className="flex flex-col"> + <span className="font-medium">{device.registered_name}</span> + {device.kind && ( + <span className="text-xs text-muted-foreground">{device.kind}</span> + )} + </div> </CommandItem> ))} </CommandGroup>src/pages/Facility/settings/devices/components/DeviceEncounterCard.tsx (2)
57-86: Consider adding null checks for associated data.While the code effectively displays various encounter details, it doesn't handle potential missing data. If properties like
created_byor its nested properties are undefined, this could lead to runtime errors.- {`${created_by.first_name} ${created_by.last_name}`} + {created_by ? `${created_by.first_name || ''} ${created_by.last_name || ''}` : '-'}
87-97: Consider validating encounter and patient data for the link.The link to the encounter details assumes that
encounter.patient.idandencounter.idboth exist. Consider adding validation to prevent broken links if these values are missing.<Link - href={`/../patient/${encounter.patient.id}/encounter/${encounter.id}/updates`} + href={encounter && encounter.patient && encounter.patient.id && encounter.id ? + `/../patient/${encounter.patient.id}/encounter/${encounter.id}/updates` : '#'} className="flex items-center gap-2" >src/pages/Facility/settings/devices/DeviceEncounterHistory.tsx (2)
24-36: Extract pagination constants for reusability.The limit value (5) is hardcoded in multiple places. Consider extracting this to a constant to maintain DRY principles and simplify future changes.
+ const ITEMS_PER_PAGE = 5; const { data: encountersData, isLoading } = useQuery({ queryKey: ["deviceEncounterHistory", facilityId, id], queryFn: query(deviceApi.encounterHistory, { queryParams: { - limit: 5, + limit: ITEMS_PER_PAGE, - offset: ((qParams.page ?? 1) - 1) * 5, + offset: ((qParams.page ?? 1) - 1) * ITEMS_PER_PAGE, }, pathParams: { facilityId, id, }, }), });
73-86: Consider adding error handling.The component doesn't explicitly handle API request errors. Consider adding error state handling to provide feedback to users when data fetching fails.
- const { data: encountersData, isLoading } = useQuery({ + const { data: encountersData, isLoading, error } = useQuery({ // existing code }); // Then add in the render section: + {error && ( + <div className="p-2"> + <div className="h-full space-y-2 rounded-lg bg-white px-7 py-12 border border-red-300"> + <div className="flex w-full items-center justify-center text-lg text-red-600"> + <div className="h-full flex w-full items-center justify-center"> + <span className="text-sm"> + {t("error_fetching_encounter_history")} + </span> + </div> + </div> + </div> + </div> + )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
public/locale/en.json(5 hunks)src/pages/Encounters/AssociateDeviceSheet.tsx(1 hunks)src/pages/Encounters/EncounterShow.tsx(2 hunks)src/pages/Encounters/tabs/EncounterDevicesTab.tsx(1 hunks)src/pages/Facility/settings/devices/DeviceDetail.tsx(2 hunks)src/pages/Facility/settings/devices/DeviceEncounterHistory.tsx(1 hunks)src/pages/Facility/settings/devices/DeviceSearch.tsx(1 hunks)src/pages/Facility/settings/devices/components/DeviceEncounterCard.tsx(1 hunks)src/pages/Facility/settings/layout.tsx(2 hunks)src/types/device/device.ts(1 hunks)src/types/device/deviceApi.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Pages changed - care-ohc
- GitHub Check: Test
- GitHub Check: cypress-run (1)
🔇 Additional comments (21)
src/pages/Encounters/EncounterShow.tsx (2)
16-16: New import looks good.The import of the
EncounterDevicesTabcomponent follows the established pattern.
40-40: Tab registration for devices is consistent with existing pattern.The addition of the devices tab to the
defaultTabsobject follows the existing pattern for tab registration.src/pages/Facility/settings/devices/DeviceDetail.tsx (3)
10-10: Icon import is appropriate.Adding the CareIcon import for enhancing the UI with icons follows good practices.
153-156: Icon addition enhances UI clarity.Adding icons to the edit button improves the user experience by providing visual cues.
161-164: Icon addition enhances UI clarity.Adding icons to the delete button improves the user experience by providing visual cues.
src/pages/Facility/settings/layout.tsx (2)
11-11: New component import is appropriate.The import of the DeviceEncounterHistory component follows the established pattern.
47-49: New route for device encounter history follows existing patterns.The addition of the route for the device encounter history is consistent with other routes in the file. Both facilityId and device id are correctly passed to the component.
src/pages/Encounters/tabs/EncounterDevicesTab.tsx (1)
82-87: LGTM! Great implementation of the AssociateDeviceSheet.The implementation correctly passes all required props to the AssociateDeviceSheet component.
src/pages/Encounters/AssociateDeviceSheet.tsx (1)
74-79: LGTM! Good user feedback during pending state.The button correctly shows different text while the association is in progress and is disabled appropriately.
src/pages/Facility/settings/devices/components/DeviceEncounterCard.tsx (3)
17-19: Well-defined interface structure.The interface clearly defines what data the component requires through the
encounterDataprop.
21-23: Clean object destructuring pattern.Using destructuring to extract properties from the encounterData object makes the code more readable and concise.
32-47: Good status visualization with dynamic styling.The conditional styling based on encounter status provides clear visual differentiation, making it intuitive for users to understand the status at a glance.
src/pages/Facility/settings/devices/DeviceEncounterHistory.tsx (3)
14-17: Clear type definition for component props.The Props interface clearly defines the required properties for the component, making it easy to understand what data is needed.
41-61: Well-structured loading state handling.The component appropriately handles loading states by displaying a skeleton loading component while data is being fetched.
80-85: Consistent pagination component usage.The pagination implementation correctly uses the same page size (5) as the query limit and properly handles page changes.
public/locale/en.json (6)
52-52: Appropriate tab label for device section.The addition of "ENCOUNTER_TAB__devices" provides proper internationalization for the devices tab in encounters.
430-432: Clear action and description text for associating devices.The added translations for device association actions are clear and descriptive, helping users understand the purpose of this functionality.
434-437: Appropriate labels for display fields.The added translation keys for association details (by, start date, end date) provide clear field labels for the UI.
784-784: Success message for device association.The addition of a clear success message helps provide feedback to users when a device is successfully associated.
961-961: Descriptive encounter history label.The "encounter_history" label provides a clear title for the encounter history section.
2133-2133: Device selection prompt added.The "select_device" string provides appropriate internationalization for device selection prompts.
….com/rajku-dev/care_fe into issue/10822/link-device-to-encounter
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/pages/Facility/settings/devices/DeviceSearch.tsx (5)
38-44: Add loading state handling for improved user experience.The component is missing loading state handling, which could lead to a poor user experience when waiting for results. Consider adding a loading indicator to show users when data is being fetched.
- const { data: devices } = useQuery({ + const { data: devices, isLoading } = useQuery({ queryKey: ["devices", facilityId, search], queryFn: query(deviceApi.list, { pathParams: { facility_id: facilityId }, queryParams: { search_text: search }, }), });Then use this loading state in the UI, for example:
{isLoading ? ( <CommandItem>Loading devices...</CommandItem> ) : ( devices?.results.map((device) => ( // existing code )) )}
38-44: Add error handling for API requests.The component doesn't handle error states from the API request, which could result in errors going unnoticed and a confusing user experience.
- const { data: devices } = useQuery({ + const { data: devices, error, isError } = useQuery({ queryKey: ["devices", facilityId, search], queryFn: query(deviceApi.list, { pathParams: { facility_id: facilityId }, queryParams: { search_text: search }, }), });Then handle the error in the UI:
{isError && ( <CommandItem className="text-red-500"> {t("error_loading_devices")} </CommandItem> )}
38-44: Consider adding theenabledoption to prevent unnecessary queries.The
useQueryhook should have anenabledoption to prevent queries when not needed (e.g., when facilityId is invalid).const { data: devices } = useQuery({ queryKey: ["devices", facilityId, search], queryFn: query(deviceApi.list, { pathParams: { facility_id: facilityId }, queryParams: { search_text: search }, }), + enabled: !!facilityId && facilityId !== "preview", });
46-55: Improve accessibility with additional ARIA attributes.The component uses
role="combobox"but could benefit from additional ARIA attributes for better accessibility.<div className="w-full h-9 px-3 rounded-md border text-sm flex items-center justify-between cursor-pointer" role="combobox" aria-expanded={open} + aria-haspopup="listbox" + aria-controls="device-search-popup" + aria-label={t("search_devices")} > {value?.registered_name || t("select_device")} </div>And add a corresponding ID to the PopoverContent:
-<PopoverContent className="w-[400px] p-0"> +<PopoverContent className="w-[400px] p-0" id="device-search-popup">
38-44: Add debouncing to search for better performance.To prevent excessive API calls as the user types, consider implementing debouncing for the search functionality.
import { useState } from "react"; +import { useDebounce } from "@/hooks/useDebounce"; // Within the component: const [search, setSearch] = useState(""); +const debouncedSearch = useDebounce(search, 300); const { data: devices } = useQuery({ - queryKey: ["devices", facilityId, search], + queryKey: ["devices", facilityId, debouncedSearch], queryFn: query(deviceApi.list, { pathParams: { facility_id: facilityId }, - queryParams: { search_text: search }, + queryParams: { search_text: debouncedSearch }, }), });This assumes you have a
useDebouncehook. If not, you'll need to implement one or use a library that provides this functionality.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/pages/Encounters/tabs/EncounterDevicesTab.tsx(1 hunks)src/pages/Facility/settings/devices/DeviceSearch.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/Encounters/tabs/EncounterDevicesTab.tsx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test
- GitHub Check: OSSAR-Scan
- GitHub Check: cypress-run (1)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/pages/Facility/settings/devices/DeviceSearch.tsx (4)
69-70: Use device.id instead of device.registered_name for CommandItem value.Using
registered_nameas the value may lead to issues if multiple devices have the same name. Theidis a more reliable unique identifier.<CommandItem key={device.id} - value={device.registered_name} + value={device.id} onSelect={() => { onSelect(device); setOpen(false); }} >
58-64: Consider debouncing the search input to prevent excessive API calls.The current implementation triggers a new API call on every character typed, which could lead to performance issues and unnecessary server load.
You could implement debouncing using a utility like
lodash.debounceor a custom hook:import { useState, useEffect } from "react"; + import debounce from "lodash.debounce"; // In your component: const [inputValue, setInputValue] = useState(""); const [search, setSearch] = useState(""); + // Debounce search updates + useEffect(() => { + const handler = debounce(() => { + setSearch(inputValue); + }, 300); + + handler(); + return () => { + handler.cancel(); + }; + }, [inputValue]); // Then use inputValue for the input and search for the API call <CommandInput placeholder={t("search_devices")} - value={search} + value={inputValue} className="outline-none border-none ring-0 shadow-none" - onValueChange={setSearch} + onValueChange={setInputValue} />
38-45: Add error handling for the query.The component doesn't handle error states, which could lead to a poor user experience if the API request fails.
- const { data: devices } = useQuery({ + const { data: devices, error, isError } = useQuery({ queryKey: ["devices", facilityId, search], queryFn: query(deviceApi.list, { pathParams: { facility_id: facilityId }, queryParams: { search_text: search }, }), enabled: facilityId !== "preview", }); + // Add error handling in your JSX + {isError && ( + <div className="text-red-500 p-2"> + {t("failed_to_load_devices")} + </div> + )}
54-55: Add fallback text for unavailable devices.If the
valueprop is null or undefined andvalue?.registered_nameis accessed, it could lead to rendering issues.- {value?.registered_name || t("select_device")} + {value?.registered_name || t("select_device")}This is already correctly handled with the OR operator, but consider adding a comment for clarity about the fallback.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/Facility/settings/devices/DeviceSearch.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test
- GitHub Check: cypress-run (1)
🔇 Additional comments (2)
src/pages/Facility/settings/devices/DeviceSearch.tsx (2)
38-45: Use consistent search parameter naming and add loading state handling.The query implementation uses
search_textas the parameter name while the past review comment suggested usingsearch. This inconsistency could lead to confusion. Additionally, there's no loading state handling, which could result in a poor user experience.Suggested changes:
const { data: devices, isLoading } = useQuery({ queryKey: ["devices", facilityId, search], queryFn: query(deviceApi.list, { pathParams: { facility_id: facilityId }, queryParams: { search_text: search }, }), enabled: facilityId !== "preview", });Consider adding a loading indicator when
isLoadingis true to improve the user experience.
67-78: Add null check for devices?.results.The component doesn't handle the case when
devices?.resultsis undefined, which could occur during loading or error states.- {devices?.results.map((device) => ( + {devices?.results?.map((device) => ( <CommandItem key={device.id} value={device.registered_name} onSelect={() => { onSelect(device); setOpen(false); }} > {device.registered_name} </CommandItem> ))}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/pages/Facility/settings/devices/DeviceEncounterHistory.tsx (4)
25-37: Add error handling for the query.While the loading state is properly handled, there's no error handling for the query. If the API request fails, the user won't see any feedback.
- const { data: encountersData, isLoading } = useQuery({ + const { data: encountersData, isLoading, error } = useQuery({ queryKey: ["deviceEncounterHistory", facilityId, id], queryFn: query(deviceApi.encounterHistory, {And then add error handling in the UI:
) : ( + error ? ( + <div className="p-2"> + <div className="h-full space-y-2 rounded-lg bg-white px-7 py-12 border border-secondary-300"> + <div className="flex w-full items-center justify-center text-lg text-secondary-600"> + <span className="text-sm text-gray-500"> + {t("error_loading_encounters")} + </span> + </div> + </div> + </div> + ) : ( <div>
29-30: Extract pagination limit to a constant.The page size (5) is hardcoded in multiple places. Consider extracting it to a constant for better maintainability.
+const ITEMS_PER_PAGE = 5; + const DeviceEncounterHistory = ({ facilityId, id }: Props) => { const { t } = useTranslation(); const [qParams, setQueryParams] = useQueryParams<{ page?: number }>(); const { data: encountersData, isLoading } = useQuery({ queryKey: ["deviceEncounterHistory", facilityId, id], queryFn: query(deviceApi.encounterHistory, { queryParams: { - limit: 5, - offset: ((qParams.page ?? 1) - 1) * 5, + limit: ITEMS_PER_PAGE, + offset: ((qParams.page ?? 1) - 1) * ITEMS_PER_PAGE,And update other instances:
<PaginationComponent cPage={qParams.page ?? 1} - defaultPerPage={5} + defaultPerPage={ITEMS_PER_PAGE}Also update the visibility check:
className={cn( "flex w-full justify-center", - (encountersData?.count ?? 0) > 5 + (encountersData?.count ?? 0) > ITEMS_PER_PAGE ? "visible" : "invisible",Also applies to: 83-84
45-45: Align skeleton count with actual page size.Ensure the skeleton loading indicator shows the same number of items as the actual page size for visual consistency.
- <CardListSkeleton count={5} /> + <CardListSkeleton count={ITEMS_PER_PAGE} />
63-89: Consider adding ARIA landmarks for better accessibility.The list of encounters would benefit from proper ARIA landmarks for better screen reader support.
- <ul className="grid gap-4 my-5"> + <ul className="grid gap-4 my-5" aria-label={t("device_encounters_list")}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
public/locale/en.json(5 hunks)src/pages/Facility/settings/devices/DeviceDetail.tsx(2 hunks)src/pages/Facility/settings/devices/DeviceEncounterHistory.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/Facility/settings/devices/DeviceDetail.tsx
- public/locale/en.json
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Pages changed - care-ohc
- GitHub Check: Test
- GitHub Check: cypress-run (1)
🔇 Additional comments (1)
src/pages/Facility/settings/devices/DeviceEncounterHistory.tsx (1)
1-98: Overall structure is well-designed and follows React best practices.This component is well-structured, with appropriate separation of concerns between data fetching and UI rendering. The loading state is handled correctly, and pagination is implemented properly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/pages/Facility/settings/devices/components/DeviceEncounterCard.tsx (2)
24-26: Add null/undefined check for encounterData.The component immediately destructures
encounterDatawithout verifying its existence. Consider adding a safety check to handle cases where the data might be undefined.export const DeviceEncounterCard = ({ encounterData }: EncounterCardProps) => { + if (!encounterData) { + return null; // or a placeholder/loading state + } const { start, end, encounter, created_by } = encounterData;
119-129: Button styling might be redundant.The button has both
variant="outline"and a customborder border-blackclass. This might be redundant as the outline variant likely already has a border style. Consider using either the variant or the custom border class.- <Button variant="outline" className="p-2 border border-black"> + <Button variant="outline" className="p-2">Alternatively, if you need the specific black border:
- <Button variant="outline" className="p-2 border border-black"> + <Button className="p-2 border border-black bg-white">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/Facility/settings/devices/components/DeviceEncounterCard.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (6)
src/pages/Facility/settings/devices/components/DeviceEncounterCard.tsx (6)
20-22: Props interface looks good.The
EncounterCardPropsinterface is well-defined with a clear type for theencounterDataprop.
67-72: Good use of conditional styling.Nice implementation of conditional styling based on encounter status. The code clearly differentiates between completed and ongoing encounters with appropriate visual indicators.
90-117: Well-structured responsive layout.The responsive grid layout for the association details is well-implemented. It displays properly on small screens as a stacked grid and as a flex row on larger screens.
1-19: Well-organized imports.The imports are well-organized with clear grouping: external libraries first, followed by internal utilities, components, constants, and type definitions.
29-132: Well-structured component with good i18n support.The component demonstrates excellent use of internationalization with the
t()function for all user-facing text. The layout is clean and follows design patterns consistent with the rest of the application.
37-48:❓ Verification inconclusive
Verify the path pattern for patient links.
The link to patient details uses a path with
/../patient/${encounter.patient.id}. The/../prefix is unusual and might indicate relative path navigation. Confirm this is the intended routing approach.
🏁 Script executed:
#!/bin/bash # Check for other instances of this path pattern rg -g "*.tsx" -g "*.ts" "/../patient/" --no-heading -C 1Length of output: 8739
Action Required: Verify Patient Link Routing
The link in
src/pages/Facility/settings/devices/components/DeviceEncounterCard.tsxcurrently uses the URL pattern/../patient/${encounter.patient.id}(and similarly in the nested encounter update link). Our repository search confirms that this pattern is unique to these links and isn’t used elsewhere. While browsers normalize/../patient/to/patient/, this unconventional relative path can be confusing and may not be the best practice for clarity and consistency.
- Review the intended routing behavior: Confirm if the use of
/../is deliberate or if an absolute path (/patient/${encounter.patient.id}) should be used instead.- Check related routing configurations: Ensure that downstream routing or any middleware isn’t adversely affected by this unusual pattern.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/pages/Facility/settings/devices/DeviceEncounterHistory.tsx (4)
25-37: Add error handling for API requests.The component currently handles loading states but doesn't account for potential API errors. Consider adding error handling logic to provide feedback when the API request fails.
const { data: encountersData, isLoading } = useQuery({ + const { data: encountersData, isLoading, isError, error } = useQuery({ queryKey: ["deviceEncounterHistory", facilityId, id, qParams], queryFn: query(deviceApi.encounterHistory, { queryParams: { limit: 5, offset: ((qParams.page ?? 1) - 1) * 5, }, pathParams: { facilityId, id, }, }), });Then display an error message when
isErroris true:{isError && ( <div className="p-2"> <div className="h-full space-y-2 rounded-lg bg-white px-7 py-12 border border-red-300"> <div className="flex w-full items-center justify-center text-lg text-red-600"> <span className="text-sm"> {t("error_loading_encounters")} </span> </div> </div> </div> )}
50-62: Improve accessibility for the "no data" state.The current "no encounters found" message could benefit from better semantic structure and accessibility attributes for screen readers.
<div className="p-2"> <div className="h-full space-y-2 rounded-lg bg-white px-7 py-12 border border-secondary-300"> - <div className="flex w-full items-center justify-center text-lg text-secondary-600"> - <div className="h-full flex w-full items-center justify-center"> - <span className="text-sm text-gray-500"> + <div className="flex w-full items-center justify-center text-lg text-secondary-600" role="status"> + <div className="h-full flex w-full items-center justify-center"> + <span className="text-sm text-gray-500" aria-live="polite"> {t("no_encounters_found")} </span> </div> </div> </div> </div>
63-71: Consider adding type safety for the encounter data.The component assumes that
encounterData.idis available, but it would be beneficial to define a proper type for the encounter data structure to ensure type safety.Consider defining an interface for the encounter data structure at the top of the file:
interface DeviceEncounter { id: string; // other properties that are used in DeviceEncounterCard }Then update the component to use this interface:
- {encountersData?.results?.map((encounterData) => ( + {encountersData?.results?.map((encounterData: DeviceEncounter) => (
76-79: Simplify conditional rendering for pagination visibility.The conditional rendering expression for hiding/showing pagination could be simplified for better readability.
- (encountersData?.count ?? 0) > 5 - ? "visible" - : "invisible", + (encountersData?.count ?? 0) <= 5 ? "invisible" : "visible",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/Facility/settings/devices/DeviceEncounterHistory.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Pages changed - care-ohc
- GitHub Check: CodeQL-Build
- GitHub Check: Test
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (1)
src/pages/Facility/settings/devices/DeviceEncounterHistory.tsx (1)
1-98: Overall component implementation looks solid.The component is well-structured with proper separation of concerns, handles pagination appropriately, and manages loading states correctly. The use of React Query for data fetching with appropriate query keys is a good practice.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/pages/Encounters/AssociateDeviceSheet.tsx (1)
54-67: 🛠️ Refactor suggestionMissing error handling in the mutation.
The mutation successfully handles the success case but lacks error handling, which is important for providing user feedback when operations fail.
const { mutate: associateDevice, isPending: isPendingAssociation } = useMutation({ mutationFn: mutate(deviceApi.associateEncounter, { pathParams: { facilityId, id: selectedDevice?.id }, }), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ["devices", facilityId], }); toast.success(t("device_associated_successfully")); onOpenChange(false); setSelectedDevice(null); }, + onError: (error) => { + toast.error(t("failed_to_associate_device")); + console.error("Error associating device:", error); + }, });
🧹 Nitpick comments (2)
src/pages/Encounters/AssociateDeviceSheet.tsx (2)
99-132: Add loading indicator while confirming association.The AlertDialog's action button shows 'associating' when in progress, but there's no visual indication that the submission is in progress after clicking the proceed button.
<AlertDialogAction onClick={handleSubmit} className={cn(buttonVariants({ variant: "primary" }))} disabled={isPendingDevice || isPendingAssociation} > <CareIcon icon="l-link-add" className="h-4" /> - {isPendingAssociation ? t("associating") : t("proceed")} + {isPendingAssociation ? ( + <> + <CareIcon icon="l-spinner-5" className="h-4 mr-2 animate-spin" /> + {t("associating")} + </> + ) : ( + t("proceed") + )} </AlertDialogAction>
133-142: Add loading indicator in association button.For consistency with other parts of the UI and to provide better feedback, add a spinner icon when the association is in progress.
<Button onClick={handleSubmit} disabled={ !selectedDevice || isPendingAssociation || isPendingDevice } > - <CareIcon icon="l-link-add" className="h-4" /> - {isPendingAssociation ? t("associating") : t("associate")} + {isPendingAssociation ? ( + <> + <CareIcon icon="l-spinner-5" className="h-4 mr-2 animate-spin" /> + {t("associating")} + </> + ) : ( + <> + <CareIcon icon="l-link-add" className="h-4" /> + {t("associate")} + </> + )} </Button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
public/locale/en.json(5 hunks)src/pages/Encounters/AssociateDeviceSheet.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- public/locale/en.json
🧰 Additional context used
🧠 Learnings (1)
src/pages/Encounters/AssociateDeviceSheet.tsx (2)
Learnt from: rajku-dev
PR: ohcnetwork/care_fe#10904
File: src/pages/Encounters/AssociateDeviceSheet.tsx:38-50
Timestamp: 2025-03-01T20:06:39.727Z
Learning: The device API endpoints in the codebase use camelCase parameter naming convention, with parameters like `facilityId` instead of snake_case `facility_id`.
Learnt from: rajku-dev
PR: ohcnetwork/care_fe#10904
File: src/pages/Encounters/AssociateDeviceSheet.tsx:38-50
Timestamp: 2025-03-01T20:06:39.727Z
Learning: The device API endpoints in the codebase use `{facilityId}` (camelCase) in path templates, not `{facility_id}` (snake_case).
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Pages changed - care-ohc
- GitHub Check: Test
- GitHub Check: cypress-run (1)
🔇 Additional comments (3)
src/pages/Encounters/AssociateDeviceSheet.tsx (3)
69-75: Parameter naming inconsistency with API convention.The device retrieve API call uses
facility_id(snake_case) which is inconsistent with the API convention that uses camelCase for parameters, as seen in the associateEncounter endpoint.const { data: selectedDeviceDetail, isPending: isPendingDevice } = useQuery({ queryKey: ["device", facilityId, selectedDevice?.id], queryFn: query(deviceApi.retrieve, { - pathParams: { facility_id: facilityId, id: selectedDevice?.id }, + pathParams: { facilityId, id: selectedDevice?.id }, }), enabled: !!selectedDevice, });
77-80: The handleSubmit logic handles validation correctly.Good validation to prevent submission if no device is selected, and the encounterId is passed correctly to the API.
37-42: Props interface is well-defined and clear.The component's props interface is concise and clearly defines all the necessary inputs for the component.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/pages/Facility/settings/devices/DeviceDetail.tsx (1)
147-153:⚠️ Potential issueFix incorrect URL path for device encounter history.
The URL path used here (
/devices/${deviceId}/encounterHistory) doesn't match the route pattern that would be expected for facility-scoped resources (/facility/${facilityId}/settings/devices/${deviceId}/encounterHistory). This will likely lead to routing errors.-<Link href={`/devices/${deviceId}/encounterHistory`}> +<Link href={`/facility/${facilityId}/settings/devices/${deviceId}/encounterHistory`}> <Button variant="outline_primary"> <CareIcon icon="l-medkit" className="h-4 w-4" /> {t("encounter_history")} </Button> </Link>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/pages/Encounters/EncounterShow.tsx(2 hunks)src/pages/Encounters/tabs/EncounterDevicesTab.tsx(1 hunks)src/pages/Facility/settings/devices/DeviceDetail.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/Encounters/EncounterShow.tsx
- src/pages/Encounters/tabs/EncounterDevicesTab.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (5)
src/pages/Facility/settings/devices/DeviceDetail.tsx (5)
144-146: Improved responsive layout for device detail page headerThe updated layout with flex-wrap and gap properties provides better responsive behavior, allowing the title and action buttons to stack properly on smaller screens.
147-161: Improved organization of history buttonsThe grouping of encounter and location history buttons in a separate div with proper responsive styling helps maintain consistency in the UI. The use of flex-col and sm:flex-row ensures proper stacking on mobile screens.
162-194: Enhanced action buttons with iconsAdding icons to the edit and delete buttons improves the user experience by making the actions more visually recognizable. The grouped layout also helps with better organization of the UI.
237-245: Simplified AssociateLocationSheet implementationThe AssociateLocationSheet component is now used with a cleaner implementation, passing required props directly while avoiding state management in the parent component. The Button is now properly nested as a child, which is a good improvement.
247-267:✅ Verification successful
New encounter information section
The newly added encounter information section provides valuable context about the current encounter associated with the device. This enhances the usefulness of the device detail page by showing the relationship between devices and patients.
However, you should ensure that the
current_encounterproperty is properly defined in the device type definition, as this appears to be a new property being accessed.
🏁 Script executed:
#!/bin/bash # Check if the current_encounter property is properly defined in the device type grep -r "current_encounter" --include="*.ts" --include="*.tsx" src/typesLength of output: 207
Encounter Information Section Verified
The new encounter information section enhances the device detail page by effectively linking devices with their associated encounter and patient details. The conditional rendering properly checks for
current_encounter, showing a link with encounter details when available and a fallback message otherwise. We verified that the device type definescurrent_encounterasEncounter | undefined(insrc/types/device/device.ts), ensuring the type is correctly set up.No further fixes are required at this time.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/pages/Facility/settings/devices/components/DeviceCard.tsx (1)
52-56: Add null check to prevent potential runtime errors.The conditional check for
encounteris good, but there's no safeguard againstencounter.facilitybeing undefined. Consider using optional chaining to prevent potential runtime errors.<Link href={`/devices/${device.id}`} - basePath={encounter ? `/facility/${encounter.facility.id}/settings` : ""} + basePath={encounter?.facility?.id ? `/facility/${encounter.facility.id}/settings` : ""} className="block h-[160px]" >public/locale/en.json (1)
811-811: New Error Message for Existing Device AssociationThe key
"device_association_exist": "Device association exist!"appears to communicate that an association is already present. Consider rephrasing it for grammatical accuracy (for example, "Device association already exists") to improve clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
public/locale/en.json(7 hunks)src/pages/Encounters/tabs/EncounterDevicesTab.tsx(1 hunks)src/pages/Facility/settings/devices/components/DeviceCard.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/Encounters/tabs/EncounterDevicesTab.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (12)
src/pages/Facility/settings/devices/components/DeviceCard.tsx (3)
14-14: Good addition of the Encounter import.This import is required to support the new optional
encounterprop being added to the component.
18-18: Properly typed optional prop.The
encounterprop is correctly marked as optional with the?operator, which aligns with how it's used conditionally in the component.
21-21: Function signature updated correctly.The component signature has been properly updated to destructure both the
deviceand the new optionalencounterprop from the Props interface.public/locale/en.json (9)
52-52: New Localization Key for Encounter Devices TabThe new key
"ENCOUNTER_TAB__devices": "Devices"has been added to support the devices tab in the encounter section. Please verify that the UI components reference this key correctly and that it follows your established naming and tone guidelines.
438-438: New Localization Key: Associate DeviceThe key
"associate_device": "Associate device"has been added as a label for device association. The text is concise and clear. Ensure that this label is used uniformly across the application.
439-439: New Localization Key: Associate Device ConfirmationThe new key
"associate_device_confirmation": "This device is currently linked to another encounter. Proceeding with this association will unlink it from the previous encounter."provides a clear warning message. Please double-check for consistency in punctuation and tone with other confirmation messages across the app.
440-440: New Localization Key: Associate Device DescriptionThe key
"associate_device_description": "Select a device to associate with this encounter"offers a helpful prompt. Verify that its phrasing aligns with similar instructional texts elsewhere in the project.
810-810: New Success Message for Device AssociationThe key
"device_associated_successfully": "Device associated successfully"has been introduced as a success notification. Its brevity is good; please confirm that it follows the project’s style for success messages.
1599-1599: New No Encounter IndicatorThe key
"no_encounter": "No associated encounter currently"provides a clear message when no encounter is linked. Verify that its tone and punctuation are consistent with other similar “no data” messages.
2195-2195: New Prompt for Device SelectionThe added key
"select_device": "Select device..."serves as a prompt in device-related dialogs. Its use of ellipsis is appropriate, but ensure that it is in line with other prompt strings used throughout the application.
2608-2608: New UI Label: View Associated EncounterThe key
"view_associated_encounter": "View associated encounter"is introduced to facilitate navigating to the encounter linked with a device. Confirm that this label is correctly integrated into the UI and that its capitalization and phrasing match the design guidelines.
992-992: New Localization Key: Encounter HistoryAlthough not marked with an explicit diff marker in the snippet, the PR summary indicates that
"encounter_history": "Encounter History"has been added. This key is intended to label the encounter history page for devices. Please ensure it is placed in the appropriate section and referenced correctly in the UI 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 (2)
public/locale/en.json (2)
440-442: New Device Association Localization Keys
The added keys:
"associate_device": "Associate device""associate_device_confirmation": "This device is currently linked to another encounter. Proceeding with this association will unlink it from the previous encounter.""associate_device_description": "Select a device to associate with this encounter"
provide clear guidance for the device association feature. They are well-named and follow the existing naming conventions. Consider a quick review of the wording for consistency with the rest of the UI, particularly ensuring clarity and uniform tone.
813-814: Device Association Feedback Messages
The new keys:
"device_associated_successfully": "Device associated successfully""device_association_exist": "Device association exist!"
offer necessary user feedback. Note that the message for an existing association might benefit from slight grammatical adjustment (e.g. “Device association already exists” or “Device already associated”) to improve readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
public/locale/en.json(7 hunks)src/pages/Encounters/EncounterShow.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/Encounters/EncounterShow.tsx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (3)
public/locale/en.json (3)
52-52: New Encounter Tab Label Added
The new key"ENCOUNTER_TAB__devices": "Devices"is consistent with other encounter tab labels and clearly indicates the new devices tab in the encounter section.
2215-2215: Select Device Placeholder Updated
The key"select_device": "Select device..."fits well as a prompt for users to choose a device, maintaining consistency with similar placeholder texts across the application.
2628-2628: View Associated Encounter Label Added
The key"view_associated_encounter": "View associated encounter"provides a clear and concise label for the UI element that allows users to view an encounter linked with a device. This aligns with the overall user interface language.
|
@rajku-dev what is the status of this PR |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
src/pages/Encounters/EncountersOverview.tsx (1)
22-56: Effective tab implementation with proper navigationThe component effectively implements a tabbed interface with proper navigation between encounters views. The use of the Tabs component from the UI library provides a consistent user experience.
One minor suggestion would be to consider adding data-testid attributes to facilitate testing of tab switching and content display.
<TabsList className="bg-transparent p-0 h-8"> <TabsTrigger value="patients" + data-testid="patients-tab" className="data-[state=active]:bg-primary/10 data-[state=active]:text-primary" > {t("patients")} </TabsTrigger> <TabsTrigger value="locations" + data-testid="locations-tab" className="data-[state=active]:bg-primary/10 data-[state=active]:text-primary" > {t("locations")} </TabsTrigger> </TabsList>src/components/Encounter/EncounterInfoCard.tsx (1)
1-120: Well-structured component with clear separation of concerns.The EncounterInfoCard component is well implemented with good organization of imports, utility functions, and component structure. The code effectively displays encounter information with appropriate styling based on status and priority.
Consider extracting the utility functions
getStatusColorandgetPriorityColorto a separate utility file if they might be reused elsewhere in the application. This would improve code reusability and maintainability.src/pages/Facility/locations/LocationNavbar.tsx (1)
17-112: Well-implemented recursive component for location tree nodes.The LocationTreeNode component effectively handles the rendering of hierarchical location data with proper expansion/collapse functionality. The loading states and icons are appropriately managed.
The recursive querying of child locations could potentially lead to many network requests if users expand multiple branches. Consider implementing prefetching of immediate children or batching requests to improve performance in more complex location hierarchies.
src/Routers/routes/PatientRoutes.tsx (1)
26-28: Add fallback for unknown tabs.While this route correctly renders
EncountersOverview, consider handling unexpectedtabparameters within the component to ensure graceful error or fallback behavior.src/pages/Facility/locations/LocationContent.tsx (4)
1-19: Avoid mixingtimport from bothi18nextandreact-i18next.Currently, this file imports
tfrom"i18next"(line 2) and also usesuseTranslationfrom"react-i18next"(line 5). Consider standardizing onuseTranslationonly to ensure consistency and clarity.
32-91:BedCardimplementation.
- Using
EncounterInfoCardto display the occupant’s current encounter makes the UI more informative.- The
isOccupied ? ... : ...structure is straightforward, but watch out for alignment with future bed-state definitions (e.g., “reserved” or “unavailable”).
98-144:LocationCardfunctionality.
- The component layout with an icon, title, and status fits typical location usage.
- The “onClick” pattern for navigation is intuitive; ensure it is accessible for keyboard users (e.g., consider adding onKeyPress if needed).
210-366:LocationContentcomponent.
- Clear separation of bed vs. non-bed sub-locations.
- Good use of React Query to fetch child locations.
- Consider adding an error boundary or fallback if the API call fails, ensuring a graceful user experience.
Could you confirm whether the API error state is handled in a parent component or a global handler?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (48)
public/locale/en.json(4 hunks)public/locale/en.json(2 hunks)public/locale/en.json(0 hunks)public/locale/en.json(0 hunks)public/locale/en.json(1 hunks)public/locale/en.json(2 hunks)public/locale/en.json(1 hunks)public/locale/en.json(2 hunks)public/locale/en.json(4 hunks)public/locale/en.json(8 hunks)src/Routers/AppRouter.tsx(1 hunks)public/locale/en.json(1 hunks)public/locale/en.json(1 hunks)public/locale/en.json(2 hunks)public/locale/en.json(14 hunks)src/Routers/AppRouter.tsx(2 hunks)src/Routers/routes/PatientRoutes.tsx(2 hunks)public/locale/en.json(1 hunks)src/components/Resource/ResourceForm.tsx(2 hunks)public/locale/en.json(17 hunks)src/Routers/AppRouter.tsx(2 hunks)src/Routers/routes/PatientRoutes.tsx(2 hunks)src/components/Resource/ResourceForm.tsx(2 hunks)public/locale/en.json(14 hunks)src/Routers/AppRouter.tsx(1 hunks)src/pages/Appointments/AppointmentsPage.tsx(20 hunks)src/pages/PublicAppointments/PatientRegistration.tsx(7 hunks)public/locale/en.json(16 hunks)src/Routers/routes/PatientRoutes.tsx(3 hunks).github/workflows/cypress.yaml(1 hunks)public/locale/en.json(11 hunks)src/App.tsx(1 hunks)src/Routers/AppRouter.tsx(1 hunks)src/Routers/routes/PatientRoutes.tsx(3 hunks)src/components/Encounter/EncounterInfoCard.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/AppointmentQuestion.tsx(2 hunks)src/components/Resource/ResourceForm.tsx(1 hunks)src/components/ui/sidebar/facility-nav.tsx(1 hunks)src/pages/Appointments/AppointmentsPage.tsx(5 hunks)src/pages/Appointments/BookAppointment.tsx(3 hunks)src/pages/Appointments/components/PractitionerSelector.tsx(1 hunks)src/pages/Encounters/EncounterList.tsx(5 hunks)src/pages/Encounters/EncountersOverview.tsx(1 hunks)src/pages/Facility/locations/LocationContent.tsx(1 hunks)src/pages/Facility/locations/LocationList.tsx(1 hunks)src/pages/Facility/locations/LocationNavbar.tsx(1 hunks)src/pages/Organization/components/OrganizationLayout.tsx(1 hunks)src/pages/PublicAppointments/PatientRegistration.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/components/ui/sidebar/facility-nav.tsx
🚧 Files skipped from review as they are similar to previous changes (19)
- public/locale/en.json
- public/locale/en.json
- public/locale/en.json
- public/locale/en.json
- public/locale/en.json
- public/locale/en.json
- public/locale/en.json
- public/locale/en.json
- public/locale/en.json
- public/locale/en.json
- public/locale/en.json
- public/locale/en.json
- public/locale/en.json
- public/locale/en.json
- public/locale/en.json
- public/locale/en.json
- public/locale/en.json
- public/locale/en.json
- public/locale/en.json
🧰 Additional context used
🧠 Learnings (2)
src/pages/Appointments/AppointmentsPage.tsx (2)
Learnt from: rithviknishad
PR: ohcnetwork/care_fe#9799
File: src/pages/Appointments/AppointmentsPage.tsx:254-283
Timestamp: 2025-01-10T13:27:15.826Z
Learning: In the AppointmentsPage component, the useEffect for initializing query parameters (practitioner and date range) is intentionally dependent only on schedulableUsersQuery.isLoading to ensure it runs only on first load, preventing unwanted resets of user-set filters.
Learnt from: rithviknishad
PR: ohcnetwork/care_fe#9799
File: src/components/Schedule/Appointments/AppointmentsPage.tsx:233-262
Timestamp: 2025-01-10T12:50:08.308Z
Learning: In AppointmentsPage component, the useEffect hook for initializing filters (practitioner and date range) intentionally uses a minimal dependency array with only `shedulableUsersQuery.isLoading` to ensure the initialization happens only on first load, avoiding unnecessary re-runs and infinite loops.
src/pages/PublicAppointments/PatientRegistration.tsx (1)
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#10260
File: src/components/Patient/PatientRegistration.tsx:286-289
Timestamp: 2025-01-28T15:50:07.442Z
Learning: For patient registration in the frontend, either year_of_birth or date_of_birth is required for successful registration. If date_of_birth is not available, year_of_birth must be present.
🔇 Additional comments (54)
.github/workflows/cypress.yaml (1)
33-33: Node.js Version Update: Compatibility Check RequiredThe Node.js version has been updated to
"22.14.0-13265982013". Please verify that this specific version is compatible with all dependencies—especially the Cypress testing environment—to ensure it does not introduce any unexpected behavior in CI.src/pages/Organization/components/OrganizationLayout.tsx (1)
111-111: Layout improvement: Added flex and min-width control to the navigation containerThe addition of
flexandmin-w-0classes to this container is a good improvement for layout stability. Theflexclass establishes a flex container, whilemin-w-0prevents flex child elements from expanding beyond their container's boundaries, which is particularly helpful for handling long text or content within the navigation items.src/Routers/AppRouter.tsx (2)
89-89: Logic reversed for sidebar type determinationThe condition for determining sidebar type has been inverted. Previously, sidebar was set to FACILITY if
appPagesexisted, now it's set to ADMIN ifadminPagesexists. This change might affect which sidebar is displayed in different contexts throughout the application.Please verify that this change in logic was intentional and doesn't cause unexpected sidebar behavior in various navigation scenarios.
108-108: Added overflow control for improved scrollingAdding
overflow-y-autoto the main content container is a good usability improvement. This ensures that content exceeding the viewport height will be scrollable rather than causing layout issues.src/App.tsx (1)
51-82: Restructured component hierarchyThe component hierarchy has been restructured to wrap everything in a fragment and move the Sentry integration outside of the QueryClientProvider. While this restructuring maintains the same components, moving Sentry outside could affect error reporting behavior.
Verify that error handling and reporting through Sentry still works as expected with this new structure, especially for errors that might occur within the QueryClientProvider scope.
src/pages/Encounters/EncountersOverview.tsx (2)
9-13: Well-structured props interfaceThe interface for EncountersOverview is well-defined with clear types and appropriate optional parameters.
15-20: Good default tab implementationSetting a default tab value of "patients" is a good practice to ensure the component always has a valid state, even when the tab prop isn't provided.
src/pages/Facility/locations/LocationList.tsx (3)
21-31: Robust implementation of parent chain tracking.The
getParentChainfunction correctly handles traversing parent relationships to build a set of parent IDs, which is essential for proper tree navigation.
33-114: Well-designed custom hook with comprehensive state management.The
useLocationStatehook effectively encapsulates all the location state management logic, providing a clean API with well-named callback functions for location selection, expansion toggling, search, and pagination.
116-178: Clean component implementation with proper data fetching and transformation.The LocationList component correctly uses the custom hook and handles data fetching with proper conditional loading. The transform from LocationDetail to LocationList is handled appropriately in the useEffect.
Verify that the
handleLocationSelectcallback doesn't cause unnecessary re-renders when location details are fetched. Since it's wrapped in useCallback with facilityId as a dependency, it should be stable, but it's worth confirming in real usage.src/components/Resource/ResourceForm.tsx (2)
87-89: Good conditional validation based on component state.The schema validation for
assigned_tocorrectly implements conditional validation based on whether an ID is present, which aligns with the business logic requiring assignment only for existing resources.
415-434: Proper conditional rendering aligned with validation rules.The conditional rendering of the
assigned_toFormField matches the schema validation logic, ensuring the UI reflects the same business rules as the validation. This provides a consistent user experience.src/pages/Facility/locations/LocationNavbar.tsx (1)
114-177: Clean implementation of the location navigation sidebar.The LocationNavbar component properly fetches top-level locations and renders them using the LocationTreeNode component. The loading states and empty state handling are well implemented.
The component is hidden on small screens with
hidden md:block(line 150). Verify that there's an alternative navigation method for mobile users to ensure proper accessibility across all device sizes.src/Routers/routes/PatientRoutes.tsx (4)
1-1: Imports look good.The addition of
Redirectfrom "raviger" is consistent with the existing routing approach.
12-12: New import forEncountersOverview.Switching from
EncounterListtoEncountersOverviewaligns with the proposed routing changes.
24-25: Redirect logic is appropriate.Redirecting
"/facility/:facilityId/encounters"to"/facility/${facilityId}/encounters/patients"helps consolidate encounter routes under a single entry point.
29-37: Ensure graceful handling for invalid location IDs.This route is adding support for location-based encounters. Verify that
EncountersOverviewgracefully handles non-existent or invalidlocationIdvalues, avoiding runtime errors or blank screens.src/pages/Encounters/EncounterList.tsx (5)
11-11: Import usage looks fine.Importing
Cardfrom"@/components/ui/card"is consistent with the component usage in this file.
30-30: Use ofEncounterInfoCard.The addition of
EncounterInfoCardis a clean way to encapsulate and display encounter details.
41-41:facilityIdnow required.Making
facilityIdmandatory clarifies usage and prevents accidental omission. Confirm that all callers ofEncounterListnow provide thefacilityIdprop.
129-129: Query parameter construction order.
buildQueryParamscorrectly referencesfacilityIdfirst, matching the updated function signature.
659-663: Proper usage ofEncounterInfoCard.Passing
encounter={encounter}andfacilityId={facilityId}ensures accurate rendering of each encounter’s details.src/pages/Facility/locations/LocationContent.tsx (7)
27-31:BedCardPropsinterface.Declaring explicit props for bed-related content keeps the code self-documenting and maintainable.
93-97:LocationCardPropsinterface.The interface is concise and clearly conveys the required fields for location rendering.
146-150:ChildLocationCardPropsinterface.Combining bed and non-bed handling under one interface is a neat approach for location children.
152-159:ChildLocationCardlogic.Branching on
location.form === "bd"clarifies bed vs. non-bed. This avoids duplicating the rendering logic in multiple places.
161-165:BreadcrumbsPropsinterface.Straightforward property definitions that keep breadcrumb references typed and explicit.
166-197:Breadcrumbscomponent.
- Recursive parent traversal is a nice way to build location ancestry.
- The use of
<BreadcrumbSeparator>ensures a consistent UI.
199-208:LocationContentPropsinterface.Well-defined interface enumerates all the interactions required for location-based navigation and pagination.
src/pages/Appointments/BookAppointment.tsx (3)
19-19: Good addition of the PractitionerSelector component import.This import aligns with the refactoring to use a dedicated component for practitioner selection.
116-122: Good refactoring to use the dedicated PractitionerSelector component.The implementation correctly passes all required props: facility ID, selected resource, selection handler, and the "show all" label. This improves component reusability and consistency across the application.
39-39:✅ Verification successful
Verify consistent use of "practitioners" query key across the application.
The query key was changed from "availableResources" to "practitioners". While this naming is more semantic, ensure all consumers of this data are updated.
🏁 Script executed:
#!/bin/bash # Check for other uses of the old query key "availableResources" rg -A 3 -B 3 '"availableResources"' --type tsLength of output: 47
🏁 Script executed:
#!/bin/bash # Perform a comprehensive search for any occurrences of the old query key "availableResources" across the entire repository rg -a "availableResources"Length of output: 26
Consistency Check Result for Query Key Update
The search for the old query key
"availableResources"returned no results, indicating that all consumers have been updated to use the new and more semantic"practitioners"key. No further action is required.src/components/Questionnaire/QuestionTypes/AppointmentQuestion.tsx (2)
20-21: Good organization of imports.The imports are properly structured with the dateQueryString utility and the new PractitionerSelector component.
106-111: Excellent refactoring to use the dedicated PractitionerSelector component.The implementation correctly passes all required props and maintains the same functionality while improving code consistency. This matches the pattern used in BookAppointment.tsx for a unified approach to practitioner selection.
src/pages/Appointments/components/PractitionerSelector.tsx (5)
1-28: Well-organized imports following project conventions.The imports are properly grouped by categories: third-party libraries, UI components, common components, utilities, and types.
29-34: Well-defined props interface with appropriate types.The PractitionerSelectorProps interface clearly defines the required and optional props with proper TypeScript types, making the component easy to use.
36-53: Well-implemented component with proper query handling.The component correctly:
- Destructures and uses props
- Uses translations for internationalization
- Sets up the query with appropriate key and parameters
- Handles loading and fetching states
54-77: Well-implemented popover trigger with accessibility features.The component:
- Uses a button as trigger for better accessibility
- Shows proper visual feedback (avatar and name) for the selected practitioner
- Disables the trigger when loading data
- Has proper ARIA roles and styling
78-126: Well-implemented command menu for practitioner selection.This implementation:
- Provides a search input for filtering practitioners
- Shows appropriate loading and empty states
- Includes a "clear selection" option when needed
- Displays practitioners with avatar, name, and user type
- Indicates the currently selected item with a check icon
- Closes the popover after selection for a smooth user experience
src/pages/PublicAppointments/PatientRegistration.tsx (6)
72-72: Good schema update for age handling.The schema and validation logic have been updated to use
ageinstead ofyear_of_birth, maintaining the required validation while improving the field naming to match user expectations.Note that I'm seeing from the retrieved learnings that either
year_of_birthordate_of_birthis required for successful registration, and this change preserves that requirement.Also applies to: 88-107
119-119: Consistent form defaults update.The default values have been properly updated to use
ageinstead ofyear_of_birth.
337-344: Form field correctly updated to use age.The form field name and registration have been properly updated to use the new
agefield.
351-365: Great UX improvement showing calculated year of birth.This provides immediate feedback to users about how their entered age maps to a birth year, helping validate their input. Good error handling for invalid age inputs (≤ 0).
185-185: Submit handler correctly updated for the age field.The data formatting logic has been properly updated to use
agein place ofyear_of_birth.
164-166: Improved query invalidation after patient creation.The invalidation of the "patients" query after creating a patient will ensure UI consistency across the application.
src/pages/Appointments/AppointmentsPage.tsx (9)
65-65: Nice upgrade to useFilters hookThe new
useFiltersimport is a good replacement for the removeduseQueryParamshook, which will provide more consistent state management with the added benefit of pagination features.
238-240: Good implementation of useFiltersThe hook is properly initialized with a default limit of 15 results per page, and it provides the necessary utilities for filtering and pagination in a clean, reusable way.
272-290: Direct qParams assignment is more straightforwardThe direct assignment to qParams properties followed by a single updateQuery call is cleaner than creating an intermediate updates object.
294-299: Well-structured query update logicGood practice to check if there are any keys in qParams before triggering an update, preventing unnecessary re-renders.
299-299: Maintained correct dependency array for useEffectKeeping only
schedulableUsersQuery.isLoadingin the dependency array maintains the intended behavior of initializing filters only on first load, preventing unwanted resets of user-set filters, which aligns with the existing patterns.
620-627: Properly added pagination props to AppointmentRowThe AppointmentRow component now correctly receives the necessary props for pagination functionality including page, resultsPerPage, and the Pagination component.
758-805: Good update to AppointmentRow component signatureThe component signature has been expanded to include pagination and filtering props. The queryKey and queryParams have been updated accordingly to support these new features.
815-819: Improved Tab state managementThe Tabs component now properly uses status from props and calls updateQuery when the value changes, which keeps the UI state in sync with the filtering state.
872-872: Effectively utilized Pagination componentThe Pagination component is correctly implemented to show the total count of results, enhancing the user experience by providing better navigation through appointment lists.
…-device-to-encounter
….com/rajku-dev/care_fe into issue/10822/link-device-to-encounter
….com/rajku-dev/care_fe into issue/10822/link-device-to-encounter
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
public/locale/en.json (1)
815-816: Suggestion: Grammar Adjustment for Device Association Message
The key"device_association_exist"currently has the message "Device association exist!" which would read more naturally as "Device association exists!" or "Device association already exists!" Consider updating the text for grammatical correctness.Below is a suggested diff:
- "device_association_exist": "Device association exist!" + "device_association_exist": "Device association exists!"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
public/locale/en.json(7 hunks)src/pages/Encounters/EncounterShow.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/Encounters/EncounterShow.tsx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test
- GitHub Check: OSSAR-Scan
- GitHub Check: cypress-run (1)
🔇 Additional comments (3)
public/locale/en.json (3)
52-52: New Localization Key: "ENCOUNTER_TAB__devices"
A new key has been added for the devices tab in the encounter section with the label "Devices". This clearly supports the new functionality for associating and managing devices.
440-442: Localization for Device Association Feature
The new entries:
"associate_device": "Associate device""associate_device_confirmation": "This device is currently linked to another encounter. Proceeding with this association will unlink it from the previous encounter.""associate_device_description": "Select a device to associate with this encounter"These additions provide clear instructions and messages for the device association workflow. They are descriptive and consistent with the other localization keys.
2643-2643: New Localization Key: "view_associated_encounter"
The key"view_associated_encounter": "View associated encounter"has been added to enable users to view the encounter linked with a device. This key is concise and aligns well with the new device association functionality.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/pages/Facility/settings/devices/DeviceDetail.tsx (1)
159-164:⚠️ Potential issueFix incorrect URL path for location history.
The URL path should be updated to match the facility-scoped pattern for consistency with the route structure. Currently using a direct device ID reference without the facility context.
-<Link href={`/devices/${deviceId}/locationHistory`}> +<Link href={`/facility/${facilityId}/settings/devices/${deviceId}/locationHistory`}> <Button variant="outline_primary" className="sm:mr-3"> <CareIcon icon="l-location-point" className="h-4 w-4" /> {t("location_history")} </Button> </Link>
🧹 Nitpick comments (4)
src/pages/Facility/settings/devices/components/DeviceEncounterCard.tsx (4)
1-4: Consider adding an explicit import order patternYour import structure is clean with logical grouping, but consider adopting a more standard import order pattern such as:
- External packages
- Internal components
- Types/interfaces
- Utils/helpers
This makes it easier to scan imports and maintain consistency across files.
60-62: Consider using localized date formattingThe date format is hardcoded. Consider using a localized date format based on the user's preferences.
- {format(new Date(start), "MMM d, yyyy h:mm a")} + {format(new Date(start), t("date_time_format", "MMM d, yyyy h:mm a"))}
74-86: Extract conditional logic for better readabilityThe
!endconditional check is repeated several times in this component. Consider extracting this to a descriptive variable to improve readability and maintainability.export const DeviceEncounterCard = ({ encounterData }: EncounterCardProps) => { const { start, end, encounter, created_by } = encounterData; + const isOngoing = !end; return ( <div className={`relative flex gap-8 pl-12 pt-0.5`}> <div className="absolute left-0 top-0 bottom-0 flex flex-col items-center"> <div - className={`absolute w-px bg-gray-200 h-full ${!end ? "top-3" : "-top-3"}`} + className={`absolute w-px bg-gray-200 h-full ${isOngoing ? "top-3" : "-top-3"}`} /> <div - className={`h-6 w-6 rounded-full ${!end ? "bg-green-100" : "bg-gray-100"} flex items-center justify-center z-10`} + className={`h-6 w-6 rounded-full ${isOngoing ? "bg-green-100" : "bg-gray-100"} flex items-center justify-center z-10`} > <CareIcon - icon={!end ? "l-location-point" : "l-check"} - className={`h-4 w-4 ${!end ? "text-green-600" : "text-gray-600"}`} + icon={isOngoing ? "l-location-point" : "l-check"} + className={`h-4 w-4 ${isOngoing ? "text-green-600" : "text-gray-600"}`} /> </div> - {!end && <div className="flex-1 w-px bg-gray-200" />} + {isOngoing && <div className="flex-1 w-px bg-gray-200" />} </div>
34-46: Add aria-label for better link accessibilityThe link to the patient's encounter details lacks descriptive aria attributes. Consider adding an aria-label to improve accessibility.
<Link href={`/patient/${encounter.patient.id}/encounter/${encounter.id}/updates`} basePath={`/facility/${encounter.facility.id}`} className="flex gap-1" + aria-label={t("view_encounter_details_for", { patient_name: encounter.patient.name })} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
public/locale/en.json(7 hunks)src/pages/Facility/settings/devices/DeviceDetail.tsx(3 hunks)src/pages/Facility/settings/devices/DeviceEncounterHistory.tsx(1 hunks)src/pages/Facility/settings/devices/components/DeviceEncounterCard.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/Facility/settings/devices/DeviceEncounterHistory.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (14)
src/pages/Facility/settings/devices/DeviceDetail.tsx (5)
41-41: New import for DeviceEncounterHistory component.Good addition of the new component import which supports the device-to-encounter association functionality.
143-148: Improved responsive layout structure.The layout structure has been improved with proper flex-wrap and responsive design. The use of
max-w-4xllimits the content width for better readability on larger screens.
149-158: Well-implemented DeviceEncounterHistory component.Good implementation of the DeviceEncounterHistory component with a trigger button. The button styling and icon choice are consistent with the application design.
165-199: Improved UI for device management actions.Good reorganization of the edit and delete buttons with appropriate icons, providing a consistent UI experience. The responsive design with flex-wrap ensures proper display on different screen sizes.
252-271: Good addition of device encounter information.Excellent addition of the encounter section that shows the current encounter information when available, with a link to the encounter updates. The conditional rendering with a fallback message for when no encounter exists provides a good user experience.
public/locale/en.json (6)
52-52: Added ENCOUNTER_TAB__devices label.Good addition of the "Devices" tab label for the encounter section.
440-443: Added associate device localization strings.Good addition of the localization strings for the device association functionality, including confirmation message for when a device is already linked to another encounter.
815-816: Added device association success messages.Properly added localization strings for success and existing association messages.
1003-1003: Added encounter history localization key.Good addition of the encounter history label used in the device detail view.
1624-1624: Added no_encounter message.Good addition of the message shown when a device has no associated encounter.
2644-2644: Added view_associated_encounter message.Good addition of the label for viewing an associated encounter.
src/pages/Facility/settings/devices/components/DeviceEncounterCard.tsx (3)
35-39: Good implementation of basePathI see you've properly implemented the
basePathprop as suggested in the previous review comments, avoiding relative paths. This is a better approach for URL construction.
51-54: Excellent use of internationalizationYou've correctly implemented the i18next translation function with named parameters, which is the preferred approach for handling user names in translated text.
68-97: Good component implementation for encounter historyBased on the previous review comments and your clarification, I see this component is specifically designed for the encounter history page. The implementation appears to be well-structured with appropriate visualization of the encounter timeline.
Proposed Changes
Fixes Add support for associating devices to an encounter #10822
Fixes User unable to scroll through all Locations when linking to a Device at once #10925
Fixes Make the facility devices page mobile device responsive #11138
Add support for associating devices to encounter
Show encounter history page for devices
This page has been filed for a design update 👇
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
UI Enhancements