feature: Added services history section in device details page (10828) #10934
Conversation
WalkthroughThe pull request introduces new localization keys in the English JSON for service record-related UI elements. It adds a new component, Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant D as DeviceDetail
participant DHS as DeviceServiceHistory
participant SHEET as ServiceHistorySheet
participant API as deviceApi
U->>D: Open Device Detail Page
D->>DHS: Render service history component with facilityId & deviceId
DHS->>API: Fetch service history list
API-->>DHS: Return service history data
U->>DHS: Click on add/edit service record
DHS->>SHEET: Open service history sheet/modal
SHEET->>API: Submit create/update request
API-->>SHEET: Confirm mutation
SHEET-->>DHS: Trigger refresh of service history list
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (3)
🔇 Additional comments (11)
✨ 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (3)
src/types/device/device.ts (1)
68-72: Consider making the meta field more type-safe.While using
Record<string, any>provides flexibility, it could lead to type safety issues. Consider defining a more specific structure for the meta field if the structure is known.- meta?: Record<string, any>; + meta?: Record<string, string | number | boolean | null>;src/pages/Facility/settings/devices/components/ServiceHistorySheet.tsx (1)
193-196: Add character limit and validation for note field.The note textarea should have a maximum length to prevent excessively long entries.
<Textarea placeholder={t("service_notes_enter")} {...field} rows={5} + maxLength={500} />Also, update the form schema:
const formSchema = z.object({ - note: z.string().min(1, { message: "Notes are required" }), + note: z.string().min(1, { message: "Notes are required" }).max(500, { message: "Notes cannot exceed 500 characters" }), serviced_on: z.date({ required_error: "Service date is required" }), });src/types/device/deviceApi.ts (1)
56-82: Consider using type annotations instead of type assertions for better type safety.The service history API methods use type assertions with empty objects (e.g.,
{} as ServiceHistoryResponse). While this works, it's generally better practice to use type annotations for better type safety.- TRes: {} as ServiceHistoryListResponse, + TRes: Type<ServiceHistoryListResponse>(), - TReq: {}, + TReq: Type<void>(), - TRes: {} as ServiceHistoryResponse, + TRes: Type<ServiceHistoryResponse>(), - TReq: {}, + TReq: Type<void>(), - TRes: {} as ServiceHistoryResponse, + TRes: Type<ServiceHistoryResponse>(), - TReq: {} as ServiceHistoryCreateUpdateRequest, + TReq: Type<ServiceHistoryCreateUpdateRequest>(), - TRes: {} as ServiceHistoryResponse, + TRes: Type<ServiceHistoryResponse>(), - TReq: {} as ServiceHistoryCreateUpdateRequest, + TReq: Type<ServiceHistoryCreateUpdateRequest>(),This matches the pattern used in other API methods in this file and provides more consistent type handling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
public/locale/en.json(2 hunks)src/pages/Facility/settings/devices/DeviceDetail.tsx(2 hunks)src/pages/Facility/settings/devices/components/DeviceServiceHistory.tsx(1 hunks)src/pages/Facility/settings/devices/components/ServiceHistorySheet.tsx(1 hunks)src/types/device/device.ts(1 hunks)src/types/device/deviceApi.ts(2 hunks)
🔇 Additional comments (9)
src/pages/Facility/settings/devices/DeviceDetail.tsx (2)
41-41: Good addition of the DeviceServiceHistory component import.The import is correctly placed with other component imports.
372-372: Properly integrated the DeviceServiceHistory component.The DeviceServiceHistory component is correctly added to the UI with the required props (facilityId and deviceId).
src/types/device/device.ts (2)
53-59: Type definition for ServiceHistoryResponse looks good.The interface properly defines the structure for a service history record with appropriate fields.
61-66: Well-structured pagination interface for service history.The ServiceHistoryListResponse interface follows the standard pagination pattern with count, next, previous, and results fields.
src/pages/Facility/settings/devices/components/DeviceServiceHistory.tsx (1)
118-118: Good use of fallback for translation.Providing a fallback text in case the translation key is missing is a good practice.
src/types/device/deviceApi.ts (2)
4-11: Good addition of new service history types to import statement.The import statement has been updated to include the new types needed for the service history API functionality. This is a clean approach to organizing the imports.
85-85: Export statement updated correctly.The export statement has been properly updated to export the deviceApi constant, making the API accessible to other modules.
public/locale/en.json (2)
505-509: Good addition of reusable button labels.These button labels ("Cancel", "Save", "Saving...", etc.) are well-structured and can be reused across the application, promoting consistency in the UI.
2191-2198: Service history labels added correctly.The service history-related labels are descriptive and follow the established naming conventions of the project. They adequately cover all the UI needs for the new service history functionality.
src/pages/Facility/settings/devices/components/DeviceServiceHistory.tsx
Outdated
Show resolved
Hide resolved
src/pages/Facility/settings/devices/components/DeviceServiceHistory.tsx
Outdated
Show resolved
Hide resolved
src/pages/Facility/settings/devices/components/DeviceServiceHistory.tsx
Outdated
Show resolved
Hide resolved
src/pages/Facility/settings/devices/components/DeviceServiceHistory.tsx
Outdated
Show resolved
Hide resolved
src/pages/Facility/settings/devices/components/ServiceHistorySheet.tsx
Outdated
Show resolved
Hide resolved
src/pages/Facility/settings/devices/components/ServiceHistorySheet.tsx
Outdated
Show resolved
Hide resolved
src/pages/Facility/settings/devices/components/ServiceHistorySheet.tsx
Outdated
Show resolved
Hide resolved
src/pages/Facility/settings/devices/components/ServiceHistorySheet.tsx
Outdated
Show resolved
Hide resolved
src/pages/Facility/settings/devices/components/ServiceHistorySheet.tsx
Outdated
Show resolved
Hide resolved
src/pages/Facility/settings/devices/components/ServiceHistorySheet.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/pages/Facility/settings/devices/components/ServiceHistorySheet.tsx (3)
48-51: Localize validation error messages.The form schema contains hardcoded error messages which should be internationalized for consistency with the rest of the application.
const formSchema = z.object({ - note: z.string().min(1, { message: "Notes are required" }), - serviced_on: z.date({ required_error: "Service date is required" }), + note: z.string().min(1, { message: t("error_notes_required") }), + serviced_on: z.date({ required_error: t("error_service_date_required") }), });Don't forget to add these new translation keys to your localization files.
180-185: Add date constraints to the Calendar component.The Calendar component currently allows selection of any date, including future dates. Consider adding constraints appropriate for service records (e.g., preventing future dates).
<Calendar mode="single" selected={field.value} onSelect={field.onChange} initialFocus + disabled={(date) => date > new Date()} + fromDate={new Date(2000, 0, 1)} // Set a reasonable minimum date />
140-140: Extract loading state to a more descriptive variable name.The variable name
isPendingis correct but could be more descriptive about what operation is pending.-const isPending = createMutation.isPending || updateMutation.isPending; +const isSubmitting = createMutation.isPending || updateMutation.isPending; // Then update usage: -<Button type="submit" disabled={isPending}> - {isPending +<Button type="submit" disabled={isSubmitting}> + {isSubmitting
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/pages/Facility/settings/devices/components/DeviceServiceHistory.tsx(1 hunks)src/pages/Facility/settings/devices/components/ServiceHistorySheet.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/Facility/settings/devices/components/DeviceServiceHistory.tsx
🔇 Additional comments (5)
src/pages/Facility/settings/devices/components/ServiceHistorySheet.tsx (5)
114-116: Improve error handling with user feedback.Similar to the create mutation, the update mutation should provide user feedback on errors.
onError: (error) => { console.error("Failed to update service record:", error); + // Add user-facing error notification + toast({ + title: t("error_title"), + description: t("error_update_service_record"), + variant: "destructive", + }); },
36-38: LGTM - Good use of type imports and API structure.The component properly imports and uses the correct types from device APIs, making it type-safe and maintainable.
73-85: LGTM - Well-implemented form reset logic.The useEffect hook properly handles form reset logic for both new and existing service records, ensuring the form is in the correct state.
126-138: LGTM - Effective submission handler with metadata preservation.The onSubmit function correctly handles both creation and update cases while preserving metadata.
209-226: LGTM - Well-implemented button states with internationalization.The form buttons correctly handle loading states and use translation keys for text, making the UI responsive and properly localized.
src/pages/Facility/settings/devices/components/ServiceHistorySheet.tsx
Outdated
Show resolved
Hide resolved
| path: "/api/v1/facility/{facility_external_id}/device/{device_external_id}/service_history/", | ||
| method: HttpMethod.GET, |
There was a problem hiding this comment.
this is javascript; follow naming conventions;
| path: "/api/v1/facility/{facility_external_id}/device/{device_external_id}/service_history/", | |
| method: HttpMethod.GET, | |
| path: "/api/v1/facility/{facilityId}/device/{deviceId}/service_history/", | |
| method: HttpMethod.GET, |
There was a problem hiding this comment.
but i am passsing also like these and in api documentation these names facility_external_id are there
const { data: serviceHistory, isLoading } = useQuery({
queryKey: ["device", facilityId, deviceId, "serviceHistory"],
queryFn: query(deviceApi.serviceHistory.list, {
pathParams: {
facility_external_id: facilityId,
device_external_id: deviceId,
},
}),
src/pages/Facility/settings/devices/components/DeviceServiceHistory.tsx
Outdated
Show resolved
Hide resolved
| <TableRow> | ||
| <TableHead>{t("service_date")}</TableHead> | ||
| <TableHead>{t("service_notes")}</TableHead> | ||
| <TableHead className="text-right">{t("actions")}</TableHead> | ||
| </TableRow> |
src/pages/Facility/settings/devices/components/DeviceServiceHistory.tsx
Outdated
Show resolved
Hide resolved
Proposed Changes
Screen.Recording.2025-03-01.222211.mp4
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
DeviceServiceHistory,ServiceHistorySheet, andServiceHistoryForm, enhancing user interaction with service records.