Add Support for Location Association History of Devices #10950
Conversation
WalkthroughThis pull request integrates a new device location history feature. It adds a localization entry, updates the DeviceDetail component to include a location history button, and introduces new components for displaying device location history. Additionally, a new route is established for navigation, and corresponding type definitions and API endpoints are added for proper data handling. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant DD as DeviceDetail
participant DL as DeviceLocationHistory
participant API as deviceApi
participant DCard as DeviceLocationCard
U->>DD: Click "Location History" button
DD->>DL: Navigate to DeviceLocationHistory route
DL->>API: Request device location history data
API-->>DL: Return paginated location data
DL->>DCard: Render each device location card
note right of DL: Pagination is managed via query parameters
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| <div className="w-full mx-3 sm:w-auto"> | ||
| <div className="text-gray-600 text-sm"> | ||
| {t("association_start_date")} | ||
| </div> |
There was a problem hiding this comment.
translations for these keys are defined in encounter history PR #10904
|
should we create tabs for overview, location history and encounter history, rather than buttons |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/types/device/device.ts (1)
53-59: Well-structured interface for device location historyThe interface provides all necessary fields to track when a device was associated with a location, who created the association, and the duration of the association.
Consider adding optional documentation comments to clarify that:
startandendare timestampsendwould be null/empty for the current active locationexport interface DeviceLocationHistory { id: string; created_by: UserBase; location: LocationList; + /** ISO timestamp when this location association started */ start: string; + /** ISO timestamp when this location association ended, empty for current active location */ end: string; }src/pages/Facility/settings/devices/components/DeviceLocationCard.tsx (2)
50-77: Consider adding a tooltip for dates that need explanationFor dates like association start and end, users might benefit from a tooltip explaining what these dates represent, especially for the case when end date is missing (displayed as "-").
<div className="font-semibold text-base"> - {end ? formatDateTime(end) : "-"} + <span title={!end ? t("currently_associated") : ""}> + {end ? formatDateTime(end) : "-"} + </span> </div>
19-81: Improve responsive design for better mobile experienceThe current layout may not provide optimal spacing on very small screens. Consider adjusting the spacing and grid layout for better mobile experience.
-<Card className="flex-1 p-4"> +<Card className="flex-1 p-3 sm:p-4"> - <CardContent className="p-4 sm:p-3 space-y-4"> + <CardContent className="p-3 sm:p-4 space-y-3 sm:space-y-4"> {/* Rest of the content */} - <div className="grid sm:flex sm:flex-wrap gap-7"> + <div className="grid grid-cols-1 sm:flex sm:flex-wrap gap-4 sm:gap-7">src/pages/Facility/settings/devices/DeviceLocationHistory.tsx (1)
65-73: Consider adding a separator or timestamp between location entriesTo improve readability and distinguish between different location history entries, consider adding a visual separator or timestamp indicator between entries. This can help users better understand the chronological sequence of location changes.
<ul className="grid gap-4 my-5"> {locationsData?.results?.map((locationData) => ( - <li key={locationData.id} className="w-full"> + <li key={locationData.id} className="w-full relative"> + {locationData.start && ( + <div className="absolute -left-3 h-full w-1 bg-gray-200 rounded-full"></div> + )} <DeviceLocationCard key={locationData.id} locationData={locationData} /> </li> ))}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
public/locale/en.json(1 hunks)src/pages/Facility/settings/devices/DeviceDetail.tsx(2 hunks)src/pages/Facility/settings/devices/DeviceLocationHistory.tsx(1 hunks)src/pages/Facility/settings/devices/components/AssociateLocationSheet.tsx(1 hunks)src/pages/Facility/settings/devices/components/DeviceLocationCard.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 (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (18)
public/locale/en.json (1)
785-785: Added localization for Device Location History featureGood addition of the localization key for the new device location history feature.
src/pages/Facility/settings/devices/components/AssociateLocationSheet.tsx (1)
49-51: Properly invalidating location history queries after mutationGood implementation of cache invalidation for the device location history after associating a new location. This ensures that the UI will display the most up-to-date information.
src/types/device/deviceApi.ts (1)
4-9: Well-organized imports for device typesClean organization of imports to include the new
DeviceLocationHistorytype.src/pages/Facility/settings/devices/components/DeviceLocationCard.tsx (5)
1-15: Component structure and props look well-definedThe
DeviceLocationCardcomponent has a clean interface with properly typed props. The use of theDeviceLocationHistorytype ensures type safety for the component's data requirements.
16-35: Good implementation of location link with proper accessibilityThe component correctly extracts data from the
locationDataprop and provides a proper link to navigate to the location details. The addition of the external link icon is a good UI pattern to indicate navigation.
38-48: Appropriate use of badges for visual status indicatorsThe badges for location form and status provide clear visual indicators with proper styling. The variant selection based on active status is a good practice.
58-61: Confirm translations are availableBased on the past review comment, these translation keys are defined in the encounter history PR. Please ensure that these translations are merged or available when this PR is deployed.
63-74: Proper date handling with fallbackGood implementation of date formatting with appropriate fallback for null values. The usage of the
formatDateTimeutility ensures consistent date formatting across the application.src/pages/Facility/settings/devices/DeviceDetail.tsx (2)
10-11: Appropriate import additionGood addition of the CareIcon import which is needed for the new location history button.
149-155: Well-implemented location history buttonThe location history button is correctly implemented with appropriate styling, icon and translation key. The positioning next to the edit button is logical from a UX perspective.
src/pages/Facility/settings/layout.tsx (3)
11-11: Appropriate component importThe import for the DeviceLocationHistory component is correctly added.
47-49: Well-defined route configurationThe route for device location history is properly defined with the correct props being passed to the component.
47-49:✅ Verification successful
Verify URL consistency with the Link in DeviceDetail component
The route pattern here is
/devices/:id/locationHistorybut in DeviceDetail.tsx, the href is structured as/devices/${deviceId}/locationHistory. Make sure these are consistent in the entire application context (especially considering the facility prefix).
🏁 Script executed:
#!/bin/bash # Description: Check for consistency between route definitions and link hrefs for device location history # Find route definition for device location history echo "Route definition in layout.tsx:" rg -A 1 "devices/:id/locationHistory" --glob "*.tsx" # Find links to device location history echo "Links to device location history:" rg -A 1 "devices/.*locationHistory" --glob "*.tsx" | grep -E "href|to"Length of output: 928
URL Consistency Verified
The device location history route insrc/pages/Facility/settings/layout.tsx("/devices/:id/locationHistory") is used consistently with the link inDeviceDetail.tsx(<Link href={/devices/${deviceId}/locationHistory}>). Both paths follow the same structure. Please ensure that any facility prefixing intended by the application's routing architecture is correctly handled at the parent level.src/pages/Facility/settings/devices/DeviceLocationHistory.tsx (5)
1-16: Proper imports and interface definitionAll necessary imports are included and the Props interface is well-defined with appropriate types.
27-39: Well-implemented data fetching with React QueryThe component correctly uses the useQuery hook with appropriate queryKey dependencies including pagination parameters. The query function is properly structured with the necessary path and query parameters.
44-49: Good loading state implementationThe component properly handles the loading state by showing a skeleton loader, which provides a better user experience than a blank screen or loading spinner.
52-63: Appropriate empty state handlingThe component correctly handles the case when no location data is found by displaying a user-friendly message.
74-90: Well-implemented paginationThe pagination component is properly implemented with conditional visibility based on result count, and it correctly updates the query parameters when page changes.
|
@rajku-dev Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌 |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit