Patient Home Page redesign #9203
Conversation
…g on the sticky tab
WalkthroughThe pull request introduces extensive modifications across various components related to patient management, including enhancements to Cypress tests for patient registration, improvements in UI components, and the addition of new React components for displaying patient details. Key changes include improved assertions in tests, refined layouts for patient details, and new localization keys for better internationalization. The updates aim to streamline user interactions and ensure accurate display of patient information throughout the application. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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. |
Deploying care-fe with
|
| Latest commit: |
c9d20eb
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://000275fe.care-fe.pages.dev |
| Branch Preview URL: | https://new-patient-home.care-fe.pages.dev |
|
Todo:
|
CARE
|
||||||||||||||||||||||||||||
| Project |
CARE
|
| Branch Review |
new-patient-home
|
| Run status |
|
| Run duration | 05m 25s |
| Commit |
|
| Committer | Shivank Kacker |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
135
|
| View all changes introduced in this branch ↗︎ | |
|
Moving 4. to a new issue as it would require a lot of refactor |
There was a problem hiding this comment.
Actionable comments posted: 29
🧹 Outside diff range and nitpick comments (68)
src/components/Patient/PatientDetailsTab/index.tsx (4)
1-8: Consider standardizing import style for better maintainabilityThe file mixes default imports (
EncounterHistory,PatientNotes,ShiftingHistory) with named imports. Consider using consistent named imports throughout for better maintainability.-import EncounterHistory from "./EncounterHistory"; -import PatientNotes from "./Notes"; -import ShiftingHistory from "./ShiftingHistory"; +import { EncounterHistory } from "./EncounterHistory"; +import { PatientNotes } from "./Notes"; +import { ShiftingHistory } from "./ShiftingHistory";
10-14: Add JSDoc documentation for the interfaceConsider adding JSDoc documentation to describe the purpose of the interface and its properties for better maintainability.
+/** + * Props interface for patient-related components + * @property facilityId - ID of the healthcare facility + * @property id - Unique identifier for the patient + * @property patientData - Complete patient information model + */ export interface PatientProps { facilityId: string; id: string; patientData: PatientModel; }
16-45: Add TypeScript type definition for the tabs arrayConsider adding a type definition for better type safety and documentation.
+interface PatientTab { + route: string; + component: React.ComponentType<PatientProps>; +} + -export const patientTabs = [ +export const patientTabs: PatientTab[] = [
34-36: Consider using more descriptive route nameThe route name "shift" could be more descriptive to better reflect its purpose (e.g., "shifting-history" to match component name).
{ - route: "shift", + route: "shifting-history", component: ShiftingHistory, },src/components/Patient/InsuranceDetails.tsx (2)
Line range hint
18-18: Fix typo in variable nameThe variable name
insuranceDetialscontains a typo and should beinsuranceDetailsfor better code readability and consistency.Apply this change throughout the file:
-const { data: insuranceDetials, loading } = useQuery( +const { data: insuranceDetails, loading } = useQuery(
Line range hint
38-48: Remove redundant loading checkThe loading state is checked twice - once before rendering the Page component and again inside it. This is redundant and can be simplified.
Consider removing the outer loading check since the inner one already handles this case:
-if (loading) { - return <Loading />; -} return ( <Page title="Insurance Details" crumbsReplacements={{ [facilityId]: { - name: insuranceDetials?.results[0]?.patient_object?.facility_object?.name, + name: insuranceDetails?.results[0]?.patient_object?.facility_object?.name, }, [id]: { - name: insuranceDetials?.results[0]?.patient_object?.name, + name: insuranceDetails?.results[0]?.patient_object?.name, }, }} className="w-full overflow-x-hidden" >src/components/Common/Page.tsx (1)
42-42: Consider adding ARIA landmarks for better accessibility.The layout structure improvements look good, but consider adding appropriate ARIA landmarks to enhance accessibility. The flex container could benefit from a semantic role.
-<div className="flex flex-col justify-between gap-2 px-3 md:flex-row md:items-center md:gap-6 md:px-0"> +<div + role="banner" + aria-label="Page header with title and actions" + className="flex flex-col justify-between gap-2 px-3 md:flex-row md:items-center md:gap-6 md:px-0" +>src/components/Patient/InsuranceDetailsCard.tsx (3)
1-7: Consider handling null/undefined insurance dataWhile making the
dataprop required improves type safety, consider handling cases where insurance data might be null/undefined at runtime.interface InsuranceDetails { - data: HCXPolicyModel; + data: HCXPolicyModel | null; }
18-19: Remove empty className stringsRemove the empty className strings as they don't serve any purpose.
-<div className=" "> +<div>Also applies to: 26-27
16-50: Enhance code maintainability with CSS class extraction and null handlingConsider the following improvements:
- Extract repeated CSS classes into constants or Tailwind components
- Use nullish coalescing operator (??) instead of OR (||) for fallbacks
+ const labelClass = "text-sm font-semibold leading-5 text-zinc-400"; + const valueClass = "mt-1 whitespace-normal break-words text-sm font-medium leading-5"; <div className="h-full space-y-2 pr-5"> <div className="grid grid-cols-1 gap-2 rounded-md border border-secondary-400 bg-gray-50 p-5 sm:grid-cols-2 md:grid-cols-2"> <div> - <div className="text-sm font-semibold leading-5 text-zinc-400"> + <div className={labelClass}> {t("insurance__member_id")} </div> - <div className="mt-1 whitespace-normal break-words text-sm font-medium leading-5"> + <div className={valueClass}> - {data.subscriber_id || ""} + {data.subscriber_id ?? ""} </div> </div> // Apply similar changes to other sectionssrc/Routers/routes/PatientRoutes.tsx (1)
15-15: Consider extracting the default page value as a constantThe string "demography" is repeated across routes. Consider extracting it to a constant for better maintainability:
+const DEFAULT_PATIENT_PAGE = "demography"; const PatientRoutes: AppRoutes = { - "/patient/:id": ({ id }) => <PatientHome id={id} page="demography" />, + "/patient/:id": ({ id }) => <PatientHome id={id} page={DEFAULT_PATIENT_PAGE} />, // ... - "/facility/:facilityId/patient/:id": ({ facilityId, id }) => ( - <PatientHome facilityId={facilityId} id={id} page="demography" /> + "/facility/:facilityId/patient/:id": ({ facilityId, id }) => ( + <PatientHome facilityId={facilityId} id={id} page={DEFAULT_PATIENT_PAGE} />Also applies to: 23-23
src/components/Patient/PatientDetailsTab/ShiftingHistory.tsx (3)
26-31: Consider improving type safety and simplifying the condition.The function could benefit from explicit type checking and a simpler condition.
- const isPatientInactive = (patientData: PatientModel, facilityId: string) => { - return ( - !patientData.is_active || - !(patientData?.last_consultation?.facility === facilityId) - ); + const isPatientInactive = (patientData: PatientModel, facilityId: string): boolean => { + const isInactiveFacility = patientData.last_consultation?.facility !== facilityId; + return !patientData.is_active || isInactiveFacility; };
44-70: Clean up unnecessary props and improve accessibility.A few improvements could enhance the code quality:
- Remove empty className prop
- Remove stray whitespace character
- Add ARIA labels for better accessibility
- <ButtonV2 - className="" + <ButtonV2 disabled={isPatientInactive(patientData, facilityId)} size="default" onClick={() => navigate(`/facility/${facilityId}/patient/${id}/shift/new`) } authorizeFor={NonReadOnlyUsers} + aria-label={t("create_new_shift")} > - </div>{" "} + </div>
1-73: Consider implementing data caching and optimistic updates.Given that this component is part of a tab-based interface:
- Consider implementing React Query or similar caching solution to persist shift history data across tab switches
- Implement optimistic updates when creating new shifts to improve perceived performance
- Consider implementing virtual scrolling for large datasets instead of traditional pagination
This would improve the user experience, especially when dealing with frequent tab switches or large datasets.
src/components/Patient/PatientDetailsTab/EncounterHistory.tsx (4)
1-19: Consider organizing imports for better maintainabilityConsider grouping imports into these categories with a blank line between each:
- External dependencies (React, i18n)
- UI Components
- Models/Types
- Hooks
- Utils/Constants
import React, { useEffect, useState } from "react"; import { useTranslation } from "react-i18next"; import PaginatedList from "@/CAREUI/misc/PaginatedList"; - import CircularProgress from "@/components/Common/CircularProgress"; import Loading from "@/components/Common/Loading"; import { ConsultationCard } from "@/components/Facility/ConsultationCard"; -import { ConsultationModel } from "@/components/Facility/models"; - -import useAuthUser from "@/hooks/useAuthUser"; +import { ConsultationModel } from "@/components/Facility/models"; +import { PatientModel } from "../models"; + +import useAuthUser from "@/hooks/useAuthUser"; + import { triggerGoal } from "@/Integrations/Plausible"; import routes from "@/Utils/request/api"; import useQuery from "@/Utils/request/useQuery"; - -import { PatientProps } from "."; -import { PatientModel } from "../models"; +import { PatientProps } from ".";
20-24: Add prop types documentationConsider adding JSDoc comments to document the component's props for better maintainability and type clarity.
+/** + * Displays a patient's consultation history in a paginated list + * @param {Object} props + * @param {PatientModel} props.patientData - Initial patient data + * @param {string} props.facilityId - ID of the facility + * @param {string} props.id - Patient ID + */ const EncounterHistory = (props: PatientProps) => {
26-29: Consider removing redundant state managementThe current implementation maintains a local state that mirrors the props. Since the state is only updated when props change, consider using the props directly unless there's a specific need for local state management.
- const [patientData, setPatientData] = - useState<PatientModel>(initialPatientData); - - useEffect(() => { - setPatientData(initialPatientData); - }, [initialPatientData]); + const patientData = initialPatientData;
51-88: Enhance component configuration and accessibility
- Consider making the page size configurable through props instead of hardcoding.
- Add appropriate ARIA labels for better accessibility.
+interface EncounterHistoryProps extends PatientProps { + pageSize?: number; +} + -const EncounterHistory = (props: PatientProps) => { +const EncounterHistory = ({ pageSize = 5, ...props }: EncounterHistoryProps) => { // ... existing code ... return ( <PaginatedList route={routes.getConsultationList} query={{ patient: id }} - perPage={5} + perPage={pageSize} > {(_) => ( - <div className="mt-8"> + <div className="mt-8" role="region" aria-label={t("consultation_history")}> <PaginatedList.WhenLoading> <CircularProgress /> </PaginatedList.WhenLoading> - <PaginatedList.WhenEmpty className="py-2"> + <PaginatedList.WhenEmpty className="py-2" role="status"> <div className="h-full space-y-2 rounded-lg bg-white p-7 border border-secondary-300"> <div className="flex w-full items-center justify-center text-xl text-secondary-600"> {t("no_consultation_history")}cypress/pageobject/Sample/SampleTestCreate.ts (2)
Line range hint
64-68: Extract common navigation logic and improve test reliability
The "Service Request" click action is duplicated from
visitSampleRequestPage. Consider:
- Extracting navigation to a separate method
- Reusing it in both places
Add explicit waiting/loading checks after navigation to prevent flaky tests
+ private navigateToServiceRequest(): void { + cy.get('[data-testid="service-request-link"]').click(); + // Add loading check + cy.get('[data-testid="content-loader"]').should('not.exist'); + } + checkRequestHistory( sampleTestStatus: string, sampleTestType: string, fastTrack: string, sampleTestResult: string, ): void { - cy.get("a").contains("Service Request").click(); + this.navigateToServiceRequest(); cy.verifyContentPresence("#sample-test-status", [sampleTestStatus]); cy.verifyContentPresence("#sample-test-type", [sampleTestType]); cy.verifyContentPresence("#sample-test-fast-track", [fastTrack]); cy.verifyContentPresence("#sample-test-result", [sampleTestResult]); }Also update
visitSampleRequestPageto use the new method:visitSampleRequestPage(): void { - cy.get("a").contains("Service Request").click(); + this.navigateToServiceRequest(); cy.verifyAndClickElement("#sample-request-btn", "Request Sample Test"); cy.url().should("include", "/sample-test"); }
Line range hint
1-116: Consider architectural improvements for maintainability
Centralize selector management:
- Move selectors to a constant object
- Reduce duplication and make updates easier
Add TypeScript interfaces for test data:
- Define interfaces for input parameters
- Improve type safety and maintainability
Example implementation:
interface SampleTestSelectors { serviceRequestLink: string; sampleRequestBtn: string; statusElement: string; // ... other selectors } interface PatientTestDetails { patientName: string; fastTrackReason: string; doctorName: string; diagnosis: string; differentialDiagnosis: string; etiologyIdentified: string; } const SELECTORS: SampleTestSelectors = { serviceRequestLink: '[data-testid="service-request-link"]', sampleRequestBtn: '#sample-request-btn', statusElement: '#sample-test-status', // ... other selectors };Would you like me to create a new issue to track these architectural improvements?
src/components/Shifting/ShiftingList.tsx (3)
Line range hint
24-43: Add error handling for data fetching operations.Consider handling error states from the useQuery hook to improve user experience when data fetching fails.
const { data: shiftData, loading, + error, refetch: fetchData, } = useQuery(routes.listShiftRequests, { query: formatFilter({ ...qParams, offset: (qParams.page ? qParams.page - 1 : 0) * resultsPerPage, }), }); + if (error) { + return <div className="text-red-500">{t("error_fetching_shifts")}</div>; + }
Line range hint
45-90: Enhance search input accessibility.The search functionality looks good, but consider adding ARIA labels and role attributes for better accessibility.
<SearchInput name="patient_name" value={qParams.patient_name} onChange={(e) => updateQuery({ [e.name]: e.value })} placeholder={t("search_patient")} + aria-label={t("search_patient")} + role="searchbox" />
Line range hint
98-109: Add loading state to refresh button.Consider disabling the refresh button while data is being fetched to prevent multiple simultaneous requests.
<button className="text-sm hover:text-blue-800" onClick={() => fetchData()} + disabled={loading} > <CareIcon - icon="l-refresh" + icon={loading ? "l-spinner" : "l-refresh"} + className={`mr-1 ${loading ? "animate-spin" : ""}`} aria-hidden="true" /> {t("refresh_list")} </button>src/components/Patient/PatientDetailsTab/SampleTestHistory.tsx (2)
43-68: Enhance accessibility for better user experience.While the layout is well-structured, consider adding ARIA labels and roles for better accessibility.
-<div className="mt-4 px-3 md:px-0"> +<div className="mt-4 px-3 md:px-0" role="region" aria-label={t("sample_test_history")}> - <div> + <div role="heading" aria-level={1}> <div className="flex justify-between items-center"> <h2 className="my-4 ml-0 text-2xl font-semibold leading-tight"> {t("sample_test_history")} </h2> <ButtonV2 + aria-label={t("request_sample_test")} className="bg-green-600 px-3 py-2 font-semibold text-white hover:bg-green-500"
70-103: Consider adding loading state feedback.The PaginatedList component handles loading states, but consider adding more user feedback during data fetching.
<PaginatedList route={routes.sampleTestList} pathParams={{ patientId: id }} perPage={5} + aria-live="polite" > {(_, query) => ( <div> <PaginatedList.WhenLoading> - <CircularProgress /> + <CircularProgress aria-label={t("loading_sample_tests")} /> </PaginatedList.WhenLoading> <PaginatedList.WhenEmpty className="py-2"> - <div className="h-full space-y-2 rounded-lg bg-white p-7 shadow"> + <div className="h-full space-y-2 rounded-lg bg-white p-7 shadow" role="alert">src/components/Common/Breadcrumbs.tsx (2)
104-105: Consider maintaining consistent icon sizesThe back arrow icon (
h-5) is larger than other icons in the breadcrumb (h-4). While this might be intentional for emphasis, consider maintaining consistent icon sizes for visual harmony.- <CareIcon icon="l-arrow-left" className="h-5 text-gray-700" /> + <CareIcon icon="l-arrow-left" className="h-4 text-gray-700" />
75-82: Consider enhancing accessibilityWhile the styling improvements are good, consider these accessibility enhancements:
- Add
aria-current="page"to the current breadcrumb item- Ensure hover states have both color and texture changes for better accessibility
- <span className="text-gray-600">{crumb.name}</span> + <span className="text-gray-600" aria-current="page">{crumb.name}</span>public/locale/mr.json (2)
71-72: Consider consolidating duplicate emergency contact translationsThere are two separate sections for emergency contact information:
- General emergency contact (lines 71-72)
- Volunteer emergency contact (lines 89-91)
Consider using a more structured approach to avoid duplication and maintain consistency.
- "emergency_contact": "आपत्कालीन संपर्क", - "emergency_contact_person_name": "आपत्कालीन संपर्क व्यक्तीचे नाव", - "emergency_contact_volunteer": "आपत्कालीन संपर्क (स्वयंसेवक)", - "emergency_contact_person_name_volunteer": "आपत्कालीन संपर्क व्यक्तीचे नाव (स्वयंसेवक)", + "emergency_contact": { + "title": "आपत्कालीन संपर्क", + "person_name": "आपत्कालीन संपर्क व्यक्तीचे नाव", + "volunteer": { + "title": "आपत्कालीन संपर्क (स्वयंसेवक)", + "person_name": "आपत्कालीन संपर्क व्यक्तीचे नाव (स्वयंसेवक)" + } + }Also applies to: 89-91
Line range hint
1-97: Missing translations for new tabs mentioned in PR objectivesBased on the PR objectives, translations are missing for the following tabs:
- Consultation History
- Test Sample History (or Service Request as per comments)
- Shifting History
- Visit History
Please add translations for these new sections to ensure complete localization coverage.
Add the following translations:
+ "consultation_history": "सल्ला इतिहास", + "service_request_history": "सेवा विनंती इतिहास", + "shifting_history": "शिफ्टिंग इतिहास", + "visit_history": "भेट इतिहास"src/components/Patient/PatientDetailsTab/ImmunisationRecords.tsx (4)
1-15: Consider reorganizing imports for better maintainabilityThe imports could be better organized by grouping them into:
- External dependencies
- UI components/icons
- Models/types
- Hooks
- Utils
Also, consider using absolute imports instead of deep relative paths (e.g., "../../../Utils/Notifications").
import { navigate } from "raviger"; import { useTranslation } from "react-i18next"; - import CareIcon from "@/CAREUI/icons/CareIcon"; - import { UserModel } from "@/components/Users/models"; - import useAuthUser from "@/hooks/useAuthUser"; - import { formatDateTime } from "@/Utils/utils"; - import { PatientProps } from "."; -import * as Notification from "../../../Utils/Notifications"; +import * as Notification from "@/Utils/Notifications"; import { PatientModel } from "../models";
16-21: Add component documentation and prop validationConsider adding:
- JSDoc documentation for the component describing its purpose and usage
- Prop-types validation for runtime type checking
- Default values for optional props
+/** + * ImmunisationRecords displays a patient's COVID-19 vaccination and travel history. + * It includes editing capabilities based on user permissions. + * + * @param {PatientProps} props - Component props + * @param {PatientModel} props.patientData - Patient information + * @param {string} props.facilityId - ID of the medical facility + * @param {string} props.id - Patient ID + */ export const ImmunisationRecords = (props: PatientProps) => {
22-37: Extract permission logic to a custom hookConsider moving the permission check logic to a reusable custom hook for better code organization and reusability.
// usePatientPermissions.ts export const usePatientPermissions = (patientData: PatientModel) => { const authUser = useAuthUser(); const canEdit = useCallback(() => { return canEditPatient(authUser, patientData); }, [authUser, patientData]); return { canEdit }; };
45-61: Improve button accessibility and visibilityThe edit button has some potential issues:
- Hidden by default (
hiddenclass) which might affect accessibility- Missing aria-label
- Disabled state should be more apparent to users
<button - className="hidden rounded border border-secondary-400 bg-white px-1 py-1 text-sm font-semibold text-green-800 hover:bg-secondary-200 group-hover:flex" + className="rounded border border-secondary-400 bg-white px-1 py-1 text-sm font-semibold text-green-800 hover:bg-secondary-200 disabled:opacity-50 disabled:cursor-not-allowed" disabled={!patientData.is_active} + aria-label={t("edit_immunisation_records")} onClick={() => {src/Routers/AppRouter.tsx (1)
159-159: Consider layout implications of overflow changesThe change from
overflow-y-scrolltooverflow-y-autoand addition of responsive padding (md:pb-2 md:pr-2) aligns with the PR's responsive design goals. However:Consider these improvements:
-className="flex-1 overflow-y-auto bg-gray-100 focus:outline-none md:pb-2 md:pr-2" +className="flex-1 overflow-y-auto bg-gray-100 focus:outline-none md:pb-2 md:pr-2 print:overflow-visible"Adding
print:overflow-visiblewould maintain consistency with the print styles already applied to parent containers.src/components/Patient/PatientDetailsTab/HealthProfileSummary.tsx (4)
1-13: Consider reorganizing imports for better maintainabilityConsider grouping imports into these categories with newline separators:
- External dependencies (raviger, react-i18next)
- Internal components/icons
- Hooks
- Types/models
- Utils
Also, consider using absolute imports instead of relative paths for better maintainability.
-import { navigate } from "raviger"; -import { useTranslation } from "react-i18next"; - -import CareIcon from "@/CAREUI/icons/CareIcon"; - -import { UserModel } from "@/components/Users/models"; - -import useAuthUser from "@/hooks/useAuthUser"; - -import { PatientProps } from "."; -import * as Notification from "../../../Utils/Notifications"; -import { PatientModel } from "../models"; +// External dependencies +import { navigate } from "raviger"; +import { useTranslation } from "react-i18next"; + +// Components +import CareIcon from "@/CAREUI/icons/CareIcon"; + +// Hooks +import useAuthUser from "@/hooks/useAuthUser"; + +// Types +import { PatientProps } from "@/components/Patient/PatientDetailsTab"; +import { UserModel } from "@/components/Users/models"; +import { PatientModel } from "@/components/Patient/models"; + +// Utils +import * as Notification from "@/Utils/Notifications";
20-25: Consider adding type safety and URL construction utilitiesThe edit handler could benefit from:
- Type safety for the
sectionIdparameter- Using a URL construction utility to prevent potential issues with malformed URLs
-const handleEditClick = (sectionId: string) => { +type SectionId = 'medical-history' | 'other-section'; // Add all valid sections +const handleEditClick = (sectionId: SectionId) => { navigate( `/facility/${facilityId}/patient/${id}/update?section=${sectionId}`, ); };
44-52: Improve type safety and move constants
- Consider moving
ADMIN_USER_TYPESto a constants file for reusability- Add type guard for safer user type checking
-const ADMIN_USER_TYPES = ["DistrictAdmin", "StateAdmin"] as const; +// Move to constants.ts +export const ADMIN_USER_TYPES = ["DistrictAdmin", "StateAdmin"] as const; +type AdminUserType = typeof ADMIN_USER_TYPES[number]; + +// Add type guard +const isAdminUserType = (type: string): type is AdminUserType => + ADMIN_USER_TYPES.includes(type as AdminUserType); const canEditPatient = (authUser: UserModel, patientData: PatientModel) => { return ( - ADMIN_USER_TYPES.includes( - authUser.user_type as (typeof ADMIN_USER_TYPES)[number], - ) || authUser.home_facility_object?.id === patientData.facility + (authUser.user_type && isAdminUserType(authUser.user_type)) || + authUser.home_facility_object?.id === patientData.facility ); };
54-134: Enhance accessibility and maintainabilityConsider the following improvements:
- Add aria-labels for better screen reader support
- Extract repeated styles to CSS classes
- Add loading state handling for better UX
+// Add to your CSS file +.health-profile-label { + @apply text-sm font-semibold leading-5 text-zinc-400; +} +.health-profile-value { + @apply mt-1 overflow-x-scroll whitespace-normal break-words text-sm font-medium leading-5; +} return ( <div className="mt-4 px-4 md:px-0" data-test-id="patient-health-profile"> + {patientData === undefined ? ( + <div role="status" aria-label="Loading health profile">Loading...</div> + ) : ( <div className="group my-2 w-full rounded bg-white p-4 shadow"> {/* ... */} - <div className="text-sm font-semibold leading-5 text-zinc-400"> + <div className="health-profile-label" aria-label="Present Health Label"> {t("present_health")} </div> - <div className="mt-1 overflow-x-scroll whitespace-normal break-words text-sm font-medium leading-5"> + <div className="health-profile-value" aria-label="Present Health Value"> {patientData.present_health || "-"} </div> {/* Apply similar changes to other sections */} </div> + )} </div> );src/components/Patient/PatientDetailsTab/Notes.tsx (4)
27-30: Add JSDoc documentation for the interface.Consider adding JSDoc documentation to describe the purpose of the interface and its properties.
+/** + * Props for the PatientNotes component + * @property {string} id - The patient's unique identifier + * @property {string} facilityId - The facility's unique identifier + */ interface PatientNotesProps { id: string; facilityId: string; }
35-54: Consider consolidating state management.The component uses multiple
useStatehooks that could be consolidated usinguseReducerfor better state management. Additionally, the thread selection logic could be moved to a custom hook.+interface NotesState { + thread: string; + patientActive: boolean; + noteField: string; + reload: boolean; + replyTo?: PatientNotesModel; + notes: PatientNoteStateType; +} +const useInitialThread = (userType: string) => { + return userType === "Nurse" + ? PATIENT_NOTES_THREADS.Nurses + : PATIENT_NOTES_THREADS.Doctors; +}; const PatientNotes = (props: PatientNotesProps) => { const authUser = useAuthUser(); - const [thread, setThread] = useState(/* ... */); - const [patientActive, setPatientActive] = useState(true); - // ... other state variables + const initialThread = useInitialThread(authUser.user_type); + const [state, dispatch] = useReducer(notesReducer, { + thread: initialThread, + patientActive: true, + noteField: "", + reload: false, + notes: { + notes: [], + cPage: 1, + totalPages: 1, + } + });
87-117: Enhance side effects handling.
- Add loading state for patient status fetch.
- Simplify message listener conditions.
+const PATIENT_NOTE_MESSAGES = ['patient/doctor_notes/create', 'patient/doctor_notes/edit']; useEffect(() => { + const [isLoading, setIsLoading] = useState(true); async function fetchPatientName() { if (patientId) { try { + setIsLoading(true); const { data } = await request(routes.getPatient, { pathParams: { id: patientId }, }); if (data) { setPatientActive(data.is_active ?? true); } } catch (error) { Notification.Error({ msg: "Failed to fetch patient status", }); + } finally { + setIsLoading(false); } } } fetchPatientName(); }, [patientId]); useMessageListener((data) => { const message = data?.message; if ( - (message?.from == "patient/doctor_notes/create" || - message?.from == "patient/doctor_notes/edit") && + PATIENT_NOTE_MESSAGES.includes(message?.from) && message?.facility_id == facilityId && message?.patient_id == patientId ) { setReload(true); } });
119-182: Improve UI accessibility and maintainability.
- Extract common Tailwind classes into reusable components or apply classes.
- Enhance button accessibility with ARIA labels.
- Consider extracting tab component for reusability.
+const TAB_BUTTON_BASE_CLASSES = "flex flex-1 justify-center border-b-2 py-2"; +const TAB_BUTTON_ACTIVE_CLASSES = "border-primary-500 font-bold text-secondary-800"; +const TAB_BUTTON_INACTIVE_CLASSES = "border-secondary-300 text-secondary-800"; <button id={`patient-note-tab-${current}`} key={current} className={classNames( - "flex flex-1 justify-center border-b-2 py-2", + TAB_BUTTON_BASE_CLASSES, thread === PATIENT_NOTES_THREADS[current] - ? "border-primary-500 font-bold text-secondary-800" - : "border-secondary-300 text-secondary-800", + ? TAB_BUTTON_ACTIVE_CLASSES + : TAB_BUTTON_INACTIVE_CLASSES, )} + aria-selected={thread === PATIENT_NOTES_THREADS[current]} + role="tab" onClick={() => setThread(PATIENT_NOTES_THREADS[current])} >Also, consider extracting the tabs into a reusable component:
interface Tab { id: string; label: string; value: string; } interface TabsProps { tabs: Tab[]; activeTab: string; onTabChange: (value: string) => void; } const Tabs: React.FC<TabsProps> = ({ tabs, activeTab, onTabChange }) => { // ... implementation };src/components/Patient/SampleTestCard.tsx (3)
Line range hint
35-57: Add error handling and loading statesThe
handleApproval1function needs improvements in error handling and user feedback:
- Missing error handling for API failures
- No loading state during the API call
- Using
anytype forsampleDataConsider applying these improvements:
const handleApproval1 = async ( sample: SampleTestModel, status: number, result: number, ) => { + const [isLoading, setIsLoading] = useState(false); + try { + setIsLoading(true); - const sampleData: any = { + const sampleData: Partial<SampleTestModel> = { id: sample.id, status, consultation: sample.consultation, }; if (status === 7) { sampleData.result = result; sampleData.date_of_result = new Date().toISOString(); } const statusName = SAMPLE_TEST_STATUS.find((i) => i.id === status)?.desc; await request(routes.patchSample, { pathParams: { id: sample.id!, }, body: sampleData, onResponse: ({ res }) => { if (res?.ok) { refetch(); Notification.Success({ msg: `Success - ${statusName}`, }); } dismissUpdateStatus(); }, }); + } catch (error) { + Notification.Error({ + msg: 'Failed to update sample status', + }); + } finally { + setIsLoading(false); + } };
Line range hint
38-42: Optimize status lookup performanceThe status lookup in
handleApproval1can be optimized by using a Map for O(1) lookup instead of usingfind.Consider this optimization:
+const STATUS_MAP = new Map( + SAMPLE_TEST_STATUS.map(status => [status.id, status.desc]) +); const handleApproval1 = async (...) => { - const statusName = SAMPLE_TEST_STATUS.find((i) => i.id === status)?.desc; + const statusName = STATUS_MAP.get(status); // ... rest of the function };
Line range hint
182-204: Align action buttons with PR requirementsAccording to the PR objectives, action buttons should be moved to the right side of the page. The current implementation has them in a flex column layout that stacks on mobile.
Consider these UI improvements:
-<div className="mx-2 mt-4 flex flex-col justify-between gap-2 md:flex-row"> +<div className="mx-2 mt-4 flex flex-col justify-end gap-2 md:flex-row"> {itemData.status === "APPROVED" && ( <ButtonV2 onClick={(e) => { e.stopPropagation(); handleApproval(4, itemData); }} - className="border border-secondary-500 bg-white text-black hover:bg-secondary-300" + className="border border-secondary-500 bg-white text-black hover:bg-secondary-300 md:ml-auto" > Send to Collection Centre </ButtonV2> )} <ButtonV2 onClick={() => showUpdateStatus(itemData)} className="border border-secondary-500 bg-white text-black hover:bg-secondary-300" authorizeFor={NonReadOnlyUsers} > Update Sample Test Status </ButtonV2> <ButtonV2 onClick={(_e) => navigate(`/sample/${itemData.id}`)} className="border border-secondary-500 bg-white text-black hover:bg-secondary-300" > Sample Report </ButtonV2> </div>cypress/e2e/patient_spec/PatientRegistration.cy.ts (1)
Based on the findings, I can now generate the final response since I have confirmed:
- The presence of multiple arbitrary cy.wait() statements in the test file
- The existence of proper intercept methods in the page object that can be used instead
- The pattern of network requests that need to be waited for
Replace arbitrary waits with proper network request assertions
The test file contains several arbitrary waits (2000-5000ms) that should be replaced with proper network request intercepts. The PatientPage class already has several intercept methods that can be utilized:
- Use
interceptCreatePatientAPI()andverifyCreatedPatientResponse()instead ofcy.wait(3000)after patient creation- Use
interceptFacilities()andverifyStatusCode()for facility loading- Add new intercepts for patient updates and form data loading
Example refactor:
// Instead of: patientPage.clickCreatePatient(); cy.wait(3000); // Use: patientPage.interceptCreatePatientAPI(); patientPage.clickCreatePatient(); patientPage.verifyCreatedPatientResponse();🔗 Analysis chain
Line range hint
1-277: Consider these test suite improvements for better maintainability and reliability.
- Replace all
cy.wait()calls with proper assertions or intercepts- Move test data to fixtures for better maintainability
- Add negative test cases for form validation
Example for moving test data to a fixture:
// cypress/fixtures/patient-test-data.json { "patientOne": { "name": "Patient With No Consultation", "gender": "Male", "updatedGender": "Female", "address": "Test Patient Address", "pincode": "682001", // ... other patient data } }Then in the test:
let testData: any; before(() => { cy.fixture('patient-test-data').then((data) => { testData = data; }); }); // Use testData.patientOne.name instead of hard-coded valuesLet's check for other instances of hard-coded waits:
The output confirms the presence of numerous arbitrary wait statements across test files, including several in the
PatientRegistration.cy.tsfile under review. Let me check if there are any existing intercepts defined in the test file that could be used instead of these waits.
Let me check the PatientPage class to understand what network requests are being intercepted and if we can leverage more intercepts to replace the waits.
Let me try a different approach to find the interceptFacilities method and other potential intercepts in the page objects.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find all cy.wait() calls in test files rg "cy\.wait\(" cypress/e2eLength of output: 2352
Script:
#!/bin/bash # Search for intercept definitions in the patient registration test rg "intercept" cypress/e2e/patient_spec/PatientRegistration.cy.tsLength of output: 104
Script:
#!/bin/bash # Search for the interceptFacilities method definition ast-grep --pattern 'interceptFacilities() { $$$ }'Length of output: 54
Script:
#!/bin/bash # Find the PatientPage class and its intercept methods fd "PatientCreation.ts" cypress/pageobject -x cat {} | grep -A 10 "intercept"Length of output: 2293
src/components/Shifting/ShiftingTable.tsx (4)
1-25: Consider enhancing type definitions and prop interfaceThe props interface could be more explicit about the shape of data:
export default function ShiftingTable(props: { - data?: ShiftingModel[]; + data?: Array<ShiftingModel>; loading?: boolean; hidePatient?: boolean; + className?: string; // Add for better component customization }) {
30-36: Improve type safety for modal stateConsider extracting the modal state interface and initial state:
+interface ModalState { + externalId: string | undefined; + loading: boolean; +} + +const INITIAL_MODAL_STATE: ModalState = { + externalId: undefined, + loading: false, +}; - const [modalFor, setModalFor] = useState<{ - externalId: string | undefined; - loading: boolean; - }>({ - externalId: undefined, - loading: false, - }); + const [modalFor, setModalFor] = useState<ModalState>(INITIAL_MODAL_STATE);
52-61: Enhance empty and loading statesConsider providing more context in the empty state and a more specific loading indicator:
if (loading) { - return <Loading />; + return ( + <div className="flex flex-col items-center justify-center p-4"> + <Loading /> + <p className="mt-2 text-secondary-600">{t("loading_shifts")}</p> + </div> + ); } if (data && !data.length) { return ( - <div className="mt-64 flex flex-1 justify-center text-secondary-600"> - {t("no_results_found")} + <div className="mt-64 flex flex-1 flex-col items-center justify-center text-secondary-600"> + <CareIcon icon="l-truck" className="mb-2 h-8 w-8" /> + <p>{t("no_shifts_found")}</p> + <p className="mt-1 text-sm">{t("shifts_empty_state_description")}</p> </div> ); }
66-69: Consolidate repeated grid class patternsExtract common grid classes to avoid duplication:
+ const getGridClasses = (hidePatient: boolean) => cn( + "grid w-full gap-2", + hidePatient ? "lg:grid-cols-4" : "lg:grid-cols-5", + "sm:grid-cols-1 md:grid-cols-1" + ); - className={cn( - "mt-5 grid w-full gap-2 border-b-2 border-gray-100 p-4 text-sm font-medium sm:grid-cols-1 md:grid-cols-1", - hidePatient ? "lg:grid-cols-4" : "lg:grid-cols-5", - )} + className={cn( + getGridClasses(hidePatient), + "mt-5 border-b-2 border-gray-100 p-4 text-sm font-medium" + )}Also applies to: 96-99
src/components/Facility/ConsultationDetails/index.tsx (2)
Line range hint
283-399: Fix duplicate tab rendering and improve tab accessibility
- The
SelectedTabcomponent is rendered twice (also at lines 400-402)- Tab navigation needs accessibility improvements
Apply these changes:
<div className="mt-4 w-full border-b-2 border-secondary-200"> <div className="overflow-x-auto sm:flex sm:items-baseline"> <div className="mt-4 sm:mt-0"> <nav className="flex space-x-6 overflow-x-auto pb-2 pl-2" id="consultation_tab_nav" + role="tablist" + aria-label="Consultation sections" > {keysOf(tabs).map((p) => { if (p === "FEED") { if ( isCameraAttached === false || consultationData?.discharge_date || !consultationData?.current_bed?.bed_object?.id || !CameraFeedPermittedUserTypes.includes(authUser.user_type) ) return null; } return ( <Link key={p} className={tabButtonClasses(tab === p)} href={`/facility/${facilityId}/patient/${patientId}/consultation/${consultationId}/${p.toLocaleLowerCase()}`} + role="tab" + aria-selected={tab === p} + aria-controls={`panel-${p.toLowerCase()}`} > {t(`CONSULTATION_TAB__${p}`)} </Link> ); })} </nav> </div> </div> </div> -<SelectedTab {...consultationTabProps} />
405-418: Add error handling and cleanup for slideover componentsThe slideover components need proper error handling and cleanup.
Consider implementing these improvements:
+const [error, setError] = useState<Error | null>(null); + +useEffect(() => { + return () => { + // Cleanup on unmount + setShowDoctors(false); + setShowPatientNotesPopup(false); + }; +}, []); <DoctorVideoSlideover facilityId={facilityId} show={showDoctors} - setShow={setShowDoctors} + setShow={(show) => { + try { + setShowDoctors(show); + } catch (err) { + setError(err as Error); + console.error('Failed to toggle doctor video:', err); + } + }} + onError={(err) => setError(err)} /> {showPatientNotesPopup && ( <PatientNotesSlideover patientId={patientId} facilityId={facilityId} consultationId={consultationId} - setShowPatientNotesPopup={setShowPatientNotesPopup} + setShowPatientNotesPopup={(show) => { + try { + setShowPatientNotesPopup(show); + } catch (err) { + setError(err as Error); + console.error('Failed to toggle patient notes:', err); + } + }} + onError={(err) => setError(err)} /> )} +{error && ( + <div role="alert" className="text-red-500"> + {error.message} + </div> +)}src/components/Users/ManageUsers.tsx (3)
Line range hint
253-275: Enhance accessibility for interactive elementsThe user interface elements in this section need proper accessibility attributes:
- The online status indicator div should have a more descriptive aria-label
- The delete button should have an aria-label describing its action
{user.last_login && cur_online ? ( <div - className="h-4 w-4 rounded-full bg-primary-500" - aria-label="Online" + className="h-4 w-4 rounded-full bg-primary-500" + aria-label="User is currently online" + role="status" /> ) : null} {showUserDelete(authUser, user) && ( <div className="w-8 cursor-pointer rounded-lg bg-red-50 text-xl text-red-600 hover:bg-red-50 hover:text-red-700" onClick={() => handleDelete(user)} + role="button" + aria-label={`Delete user ${formatName(user)}`} + tabIndex={0} > <CareIcon icon="l-trash" className="ml-[5px]" /> </div> )}
Line range hint
73-96: Improve error handling for working hours updateThe current error handling in
handleWorkingHourSubmitcould be enhanced:
- Add input validation for non-numeric values
- Sanitize error messages from the API
- Handle network errors explicitly
const handleWorkingHourSubmit = async () => { const username = selectedUser; - if (!username || !weeklyHours || +weeklyHours < 0 || +weeklyHours > 168) { + if (!username) { + Notification.Error({ msg: "User not selected" }); + return; + } + + const hours = Number(weeklyHours); + if (isNaN(hours)) { setWeeklyHoursError("Value should be between 0 and 168"); return; } - const { res, data, error } = await request(routes.partialUpdateUser, { - pathParams: { username }, - body: { weekly_working_hours: weeklyHours }, - }); - if (res && res.status === 200 && data) { - Notification.Success({ - msg: "Working hours updated successfully", - }); - setExpandWorkingHours(false); - setSelectedUser(null); - } else { - Notification.Error({ - msg: "Error while updating working hours: " + (error || ""), - }); + + if (hours < 0 || hours > 168) { + setWeeklyHoursError("Value should be between 0 and 168"); + return; } - setWeeklyHours("0"); - setWeeklyHoursError(""); - await refetchUserList(); + + try { + const { res, data, error } = await request(routes.partialUpdateUser, { + pathParams: { username }, + body: { weekly_working_hours: hours }, + }); + + if (res && res.status === 200 && data) { + Notification.Success({ + msg: "Working hours updated successfully", + }); + setExpandWorkingHours(false); + setSelectedUser(null); + } else { + Notification.Error({ + msg: "Failed to update working hours. Please try again.", + }); + console.error("API Error:", error); + } + } catch (error) { + Notification.Error({ + msg: "Network error occurred. Please check your connection.", + }); + console.error("Network Error:", error); + } finally { + setWeeklyHours("0"); + setWeeklyHoursError(""); + await refetchUserList(); + } };
Line range hint
644-691: Standardize facility management error handlingThe facility management functions need more consistent error handling and state management:
- Wrap state updates in try-catch blocks
- Standardize success/error notification messages
- Handle loading states more predictably
const addFacility = async (username: string, facility: any) => { - setIsLoading(true); - const { res } = await request(routes.addUserFacility, { - pathParams: { username }, - body: { facility: facility.id.toString() }, - }); - - if (!res?.ok) { - Notification.Error({ - msg: "Error while linking facility", - }); - } else { - Notification.Success({ - msg: "Facility linked successfully", - }); - } - await refetchUserFacilities(); - setIsLoading(false); - setFacility(null); + try { + setIsLoading(true); + const { res, error } = await request(routes.addUserFacility, { + pathParams: { username }, + body: { facility: facility.id.toString() }, + }); + + if (!res?.ok) { + throw new Error(error || 'Failed to link facility'); + } + + Notification.Success({ + msg: `Successfully linked facility: ${facility.name}`, + }); + + await refetchUserFacilities(); + } catch (error) { + Notification.Error({ + msg: error instanceof Error ? error.message : "Error while linking facility", + }); + console.error("Facility linking error:", error); + } finally { + setIsLoading(false); + setFacility(null); + } };src/components/Patient/PatientRegister.tsx (1)
1420-1420: Well-structured section organization with semantic IDs.The addition of semantic IDs for different sections (
social-profile,covid-details,medical-history,insurance-details) aligns well with the PR objectives for implementing a tab-based navigation system. The consistent styling and structure across sections improves maintainability.Consider documenting these section IDs in the component's documentation or in a constants file to maintain a single source of truth for URL-based navigation.
Also applies to: 1498-1498, 1619-1622, 1682-1685
public/locale/en.json (2)
Line range hint
286-350: Consider grouping related medical history keys together.While the translations are accurate, consider reorganizing these keys to group related medical history items together (e.g., all consultation related keys, all insurance related keys, etc.) for better maintainability.
Example organization:
# Consultation related add_consultation add_consultation_update consultation_history # Insurance related add_insurance_details insurance__insurer_id insurance__insurer_name
1404-1419: Consider consistent casing in view related keys.While most keys use snake_case, there are some inconsistencies in casing:
view_faciliy(typo: should beview_facility)view_cns(inconsistent: should beview_central_nursing_stationfor clarity)- "view_faciliy": "View Facility", + "view_facility": "View Facility", - "view_cns": "View CNS", + "view_central_nursing_station": "View Central Nursing Station",src/components/Facility/ConsultationCard.tsx (3)
29-32: Refactor nested ternary operator for better readabilityConsider refactoring the nested ternary operator used in assigning
bedDialogTitleto improve code readability and maintainability.Here's a suggested refactor:
-const bedDialogTitle = itemData.discharge_date - ? t("bed_history") - : !itemData.current_bed - ? t("assign_bed") - : t("switch_bed"); +const bedDialogTitle = (() => { + if (itemData.discharge_date) { + return t("bed_history"); + } else if (!itemData.current_bed) { + return t("assign_bed"); + } else { + return t("switch_bed"); + } +})();
58-60: Avoid magic strings by using constants foritemData.suggestionConsider defining constants or enums for
itemData.suggestionvalues (e.g.,'A') to improve code maintainability and reduce potential errors from typos.Example:
+const SUGGESTION_TYPES = { + ADMISSION: "A", + /* other types */ +}; ... -variant={itemData.suggestion === "A" ? "alert" : "primary"} +variant={itemData.suggestion === SUGGESTION_TYPES.ADMISSION ? "alert" : "primary"} ... -text={ - itemData.suggestion === "A" ? t("ip_encounter") : t("op_encounter") -} +text={ + itemData.suggestion === SUGGESTION_TYPES.ADMISSION ? t("ip_encounter") : t("op_encounter") +}
65-66: Use consistent date formatting withformatDateTimeutilityFor consistency across the application, consider using the
formatDateTimeutility function instead of directly usingdayjs.format.Here's the suggested change:
-{dayjs(itemData.created_date).format("DD/MM/YYYY")} +{formatDateTime(itemData.created_date)}src/components/Patient/PatientDetailsTab/Demography.tsx (2)
185-186: Localize hardcoded string "Chat on WhatsApp"Replace the hardcoded string with a localized version using the
tfunction for internationalization.<CareIcon icon="l-whatsapp" /> - Chat on WhatsApp + {t("chat_on_whatsapp")}
100-143: MoveEmergencyContactcomponent definition outside ofDemographycomponentDefining components within another component causes them to be redefined on every render, which can impact performance. Consider moving
EmergencyContactoutside ofDemographyto avoid unnecessary re-renders.+// Move EmergencyContact component outside of Demography +const EmergencyContact = (props: { number?: string; name?: string }) => ( + <div className="sm:col-span-2"> + {/* Component code remains the same */} + </div> +); export const Demography = (props: PatientProps) => { // Demography component code - const EmergencyContact = (props: { number?: string; name?: string }) => ( - <div className="sm:col-span-2"> - {/* Component code */} - </div> - ); // Rest of the Demography component };src/components/Patient/PatientHome.tsx (5)
49-53: Consider defining a specific type for thepageprop for better type safetyThe
pageprop is currently typed as(typeof patientTabs)[0]["route"], which may not provide optimal type safety or readability. Defining an explicit union type or enum for the possible routes can enhance type checking and developer experience.Here's a suggestion:
+ type PatientTabRoute = 'demography' | 'consultation-history' | 'sample-test-history' | 'shifting-history' | 'visit-history'; export const PatientHome = (props: { facilityId?: string; id: string; - page: (typeof patientTabs)[0]["route"]; + page: PatientTabRoute; }) => {
64-71: Avoid redundant state by usingpatientData.assigned_to_objectdirectlyMaintaining
assignedVolunteeras a separate state duplicates the data frompatientData.assigned_to_object. This can lead to unnecessary re-renders and potential inconsistencies. Consider usingpatientData.assigned_to_objectdirectly throughout the component.
629-630: SimplifyclassNameby removing redundant classesThe
classNameattribute contains duplicate classes:"tooltip-text tooltip". You can simplify it by removing the redundant class.Here’s a suggestion:
- <span className={`tooltip-text tooltip`}> + <span className="tooltip-text">
505-513: Remove unnecessarydisabledprop set to falseThe
disabledprop is explicitly set tofalse, which is unnecessary since the default behavior is enabled. Removing it simplifies the code.Here’s a suggestion:
<ButtonV2 id="assign-volunteer" onClick={() => setOpenAssignVolunteerDialog(true)} - disabled={false} authorizeFor={NonReadOnlyUsers} className="w-full bg-white font-semibold text-green-800 hover:bg-secondary-200" size="large" >
Line range hint
608-619: Refactor nested ternary operators for better readabilityThe nested ternary operators in the conditional rendering of the discharge reason reduce readability. Consider refactoring to a more readable format, such as using if-else statements or extracting the logic into a separate function.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (37)
cypress/e2e/patient_spec/PatientRegistration.cy.ts(2 hunks)cypress/pageobject/Patient/PatientConsultation.ts(1 hunks)cypress/pageobject/Patient/PatientCreation.ts(2 hunks)cypress/pageobject/Patient/PatientMedicalHistory.ts(1 hunks)cypress/pageobject/Sample/SampleTestCreate.ts(2 hunks)public/locale/en.json(39 hunks)public/locale/hi.json(1 hunks)public/locale/kn.json(1 hunks)public/locale/ml.json(1 hunks)public/locale/mr.json(2 hunks)src/CAREUI/display/Chip.tsx(1 hunks)src/Routers/AppRouter.tsx(3 hunks)src/Routers/routes/PatientRoutes.tsx(2 hunks)src/components/Common/Breadcrumbs.tsx(2 hunks)src/components/Common/Menu.tsx(1 hunks)src/components/Common/Page.tsx(1 hunks)src/components/Facility/ConsultationCard.tsx(4 hunks)src/components/Facility/ConsultationDetails/index.tsx(2 hunks)src/components/Patient/InsuranceDetails.tsx(2 hunks)src/components/Patient/InsuranceDetailsCard.tsx(1 hunks)src/components/Patient/PatientDetailsTab/Demography.tsx(1 hunks)src/components/Patient/PatientDetailsTab/EncounterHistory.tsx(1 hunks)src/components/Patient/PatientDetailsTab/HealthProfileSummary.tsx(1 hunks)src/components/Patient/PatientDetailsTab/ImmunisationRecords.tsx(1 hunks)src/components/Patient/PatientDetailsTab/Notes.tsx(1 hunks)src/components/Patient/PatientDetailsTab/SampleTestHistory.tsx(1 hunks)src/components/Patient/PatientDetailsTab/ShiftingHistory.tsx(1 hunks)src/components/Patient/PatientDetailsTab/index.tsx(1 hunks)src/components/Patient/PatientHome.tsx(9 hunks)src/components/Patient/PatientInfoCard.tsx(1 hunks)src/components/Patient/PatientRegister.tsx(6 hunks)src/components/Patient/SampleTestCard.tsx(1 hunks)src/components/Patient/models.tsx(1 hunks)src/components/Shifting/ShiftingBlock.tsx(2 hunks)src/components/Shifting/ShiftingList.tsx(3 hunks)src/components/Shifting/ShiftingTable.tsx(1 hunks)src/components/Users/ManageUsers.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/CAREUI/display/Chip.tsx
- src/components/Patient/PatientInfoCard.tsx
🧰 Additional context used
📓 Learnings (1)
cypress/e2e/patient_spec/PatientRegistration.cy.ts (1)
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9145
File: cypress/e2e/patient_spec/PatientConsultationCreation.cy.ts:93-94
Timestamp: 2024-11-18T10:48:08.501Z
Learning: In `cypress/e2e/patient_spec/PatientConsultationCreation.cy.ts`, bed capacity verification assertions after patient admission are already being performed elsewhere, so adding them here is unnecessary.
🪛 Biome (1.9.4)
src/components/Patient/InsuranceDetails.tsx
[error] 56-56: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
src/components/Patient/PatientDetailsTab/Demography.tsx
[error] 203-206: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 285-292: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 298-298: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
src/components/Shifting/ShiftingBlock.tsx
[error] 129-146: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (57)
src/components/Patient/PatientDetailsTab/index.tsx (1)
16-45: Verify tab order matches design requirements
The PR objectives mention a specific order for tabs: "Patient Profile, Consultation History, Test Sample History, Shifting History, and Visit History". The current implementation has a different order. Please verify if this ordering is intentional or needs to be adjusted to match the requirements.
src/components/Common/Page.tsx (1)
36-38: LGTM! Verify responsive behavior across screen sizes.
The padding logic has been simplified and provides better consistency across screen sizes. The new values align well with the page redesign objectives.
Let's verify the responsive behavior:
✅ Verification successful
Padding changes are safe and consistent with design system
The padding changes in Page.tsx align with the project's design system as evidenced by:
- The
md:p-6pattern is consistently used across multiple components for medium-sized screens (e.g., FileUpload.tsx, FacilityHome.tsx, FilePreviewDialog.tsx) - The
py-3pattern is a common vertical padding value used throughout the codebase for consistent spacing - The changes maintain responsive behavior similar to other core components
The new padding values maintain consistency with the established design patterns across the application.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other components that might be affected by these padding changes
rg "md:p-6|md:px-6|py-3" --type tsx
Length of output: 65
Script:
#!/bin/bash
# Search for other components that might be affected by these padding changes, using correct file extension
rg "md:p-6|md:px-6|py-3" --type ts --type tsx --type jsx --type js
Length of output: 96
Script:
#!/bin/bash
# Search for other components that might be affected by these padding changes without file type restrictions
rg "md:p-6|md:px-6|py-3"
Length of output: 10590
cypress/pageobject/Patient/PatientMedicalHistory.ts (1)
38-38: 🛠️ Refactor suggestion
Maintain consistent data attribute naming
The selector uses data-test-id while other selectors in the file use data-testid. This inconsistency could lead to maintenance issues.
Apply this change to maintain consistency:
-cy.get("[data-test-id=patient-health-profile]").then(($dashboard) => {
+cy.get("[data-testid=patient-health-profile]").then(($dashboard) => {Also, ensure the component uses the consistent attribute name.
Let's verify the attribute naming convention across the codebase:
src/components/Patient/InsuranceDetailsCard.tsx (2)
9-12: LGTM! Clean component setup
The component declaration, props destructuring, and translation hook initialization follow React best practices.
16-50: Verify translations are properly configured
The component uses several translation keys. Let's verify they are properly configured in the localization files.
✅ Verification successful
Let me check the actual content of the translation file to verify if all the required keys are properly defined.
Translation keys are properly configured
All required translation keys (insurance__member_id, insurance__policy_name, insurance__insurer_id, and insurance__insurer_name) are present and properly defined in the English locale file with appropriate translations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if all translation keys are defined
rg -l "insurance__member_id|insurance__policy_name|insurance__insurer_id|insurance__insurer_name" public/locale/
Length of output: 136
Script:
#!/bin/bash
# Check the content of the translation file
cat public/locale/en.json | jq '. | {insurance__member_id, insurance__policy_name, insurance__insurer_id, insurance__insurer_name}'
Length of output: 316
src/Routers/routes/PatientRoutes.tsx (3)
6-6: LGTM! Import aligns with tab-based navigation requirements
The import of patientTabs supports the new tabbed interface design mentioned in the PR objectives.
6-6: Verify alignment with required tab sections
Please ensure that patientTabs includes all the sections mentioned in the PR objectives: Patient Profile, Consultation History, Test Sample History, Shifting History, and Visit History.
Also applies to: 25-31
✅ Verification successful
All required sections are present with additional useful tabs
The patientTabs implementation includes all the sections mentioned in the PR objectives and more:
- Patient Profile (via "demography" and "health-profile")
- Consultation History (via "encounters")
- Test Sample History (via "request-sample-test")
- Shifting History (via "shift")
- Visit History (covered under "encounters")
Plus additional useful sections:
- Immunisation Records
- Patient Notes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify patientTabs implementation includes all required sections
ast-grep --pattern 'export const patientTabs = [
$$$
]'
Length of output: 2198
25-31: 🛠️ Refactor suggestion
Enhance dynamic route generation with validation and completeness
While the dynamic route generation is a good approach, consider these improvements:
-
Verify route uniqueness:
-
Add type safety and validation:
...patientTabs.reduce((acc: AppRoutes, tab) => {
+ if (!tab.route || typeof tab.route !== 'string') {
+ throw new Error(`Invalid tab route: ${JSON.stringify(tab)}`);
+ }
acc["/facility/:facilityId/patient/:id/" + tab.route] = ({
facilityId,
id,
}) => <PatientHome facilityId={facilityId} id={id} page={tab.route} />;
+
+ // Mirror the route for non-facility path
+ acc["/patient/:id/" + tab.route] = ({
+ id,
+ }) => <PatientHome id={id} page={tab.route} />;
return acc;
}, {}),This ensures:
- Type safety for tab routes
- Consistent routing patterns for both facility and non-facility paths
- Runtime validation of tab configuration
src/components/Patient/PatientDetailsTab/ShiftingHistory.tsx (2)
1-18: LGTM! Well-organized imports.
The imports are logically grouped and all are being utilized within the component.
19-24: Verify the cacheBlacklist configuration and props interface.
The cacheBlacklist includes "patient_name" but its purpose isn't immediately clear. Also, ensure that the imported PatientProps interface includes all required properties (patientData, facilityId, id).
✅ Verification successful
The shell script results provide clear evidence about both concerns. Let me generate the final response:
Props interface and cacheBlacklist configuration are correctly implemented
The PatientProps interface in src/components/Patient/PatientDetailsTab/index.tsx correctly defines all the required properties:
facilityId: stringid: stringpatientData: PatientModel
The cacheBlacklist: ["patient_name"] configuration is consistently used across multiple shifting-related components (ShiftingList, ShiftingBoard, ShiftingHistory), indicating this is an intentional pattern for handling patient name filtering in the shifting functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify PatientProps interface definition and its usage
# Search for PatientProps interface definition
ast-grep --pattern 'interface PatientProps {
$$$
}'
# Search for other usages of cacheBlacklist with "patient_name"
rg -A 2 'cacheBlacklist.*patient_name'
Length of output: 1310
src/components/Patient/PatientDetailsTab/EncounterHistory.tsx (2)
47-50: LGTM!
The loading state is handled appropriately with a dedicated Loading component.
1-90: Implementation aligns with PR objectives
The component successfully implements the consultation history tab as part of the patient details page redesign mentioned in PR #9203. The implementation follows the tab-based structure and provides a clean interface for viewing consultation history.
Let's verify the integration with other tabs mentioned in the PR:
✅ Verification successful
Implementation aligns with PR objectives and integrates correctly with other tabs
The verification confirms that:
- The component is properly integrated into the tab structure in
PatientDetailsTab/index.tsx - All related tab components are present:
- EncounterHistory (consultation history)
- HealthProfileSummary
- ImmunisationRecords
- PatientNotes
- SampleTestHistory
- ShiftingHistory
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the implementation of other tabs mentioned in the PR
# Check for other tab components
echo "Checking for other tab components..."
fd -e tsx -e ts "Profile|TestSample|Shifting|Visit" src/components/Patient/
# Verify tab integration in parent component
echo "Checking tab integration..."
rg -A 5 "EncounterHistory" src/components/Patient/
Length of output: 2165
cypress/pageobject/Sample/SampleTestCreate.ts (1)
3-5: 🛠️ Refactor suggestion
Improve selector reliability and maintain consistent naming
-
The generic
cy.get("a")selector is fragile and could match multiple links. Consider:- Adding a specific data-testid attribute
- Using a more specific selector
-
There's inconsistent naming across UI elements:
- Link shows "Service Request"
- Button shows "Request Sample Test"
- URL contains "sample-test"
- cy.get("a").contains("Service Request").click();
+ cy.get('[data-testid="service-request-link"]').click();
cy.verifyAndClickElement("#sample-request-btn", "Request Sample Test");
cy.url().should("include", "/sample-test");src/components/Common/Menu.tsx (1)
58-58: Verify z-index change across different pages
The z-index increase from 10 to 50 looks reasonable for ensuring dropdown visibility in the redesigned patient page. However, since this is a shared component, we should verify that this change doesn't cause any unintended overlay issues in other pages using this dropdown.
Let's verify other usages of this component:
✅ Verification successful
The previous script failed because the file type wasn't recognized. Let's try with broader patterns and multiple approaches to find the component usage across the codebase.
Z-index change is safe to proceed
The z-index increase from 10 to 50 in the Menu component is safe based on the following findings:
- The component is used in critical areas like PatientInfoCard, FacilityHome, Export, SortDropdown, and ConditionVerificationStatus
- Only two other z-index definitions were found in related files:
- z-20 in PatientInfoCard (lower than menu's z-50)
- z-30 in FacilityHome (lower than menu's z-50)
- The new z-50 value ensures the dropdown menu will appear above these existing elements while maintaining a consistent stacking order
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all files that import the Menu component to verify impact
rg -l "import.*Menu.*from.*Common/Menu" --type tsx
Length of output: 80
Script:
#!/bin/bash
# Search for Menu component imports with broader patterns and file types
rg -l "import.*Menu.*from.*Common/Menu" --type ts --type jsx --type tsx
# Fallback: Search for any imports from Common/Menu
rg -l "from ['\"].*Common/Menu['\"]"
# Additional search for Menu component usage
rg -l "<Menu\W"
Length of output: 410
Script:
#!/bin/bash
# Let's examine the context of Menu usage in these files to understand the impact
rg -C 5 "from ['\"].*Common/Menu['\"]" src/components/Patient/PatientInfoCard.tsx src/components/Facility/FacilityHome.tsx src/components/Common/Export.tsx src/components/Common/SortDropdown.tsx src/components/Diagnosis/ConditionVerificationStatusMenu.tsx
# Also check if there are any other z-index definitions that might conflict
rg "z-\d+" src/components/Patient/PatientInfoCard.tsx src/components/Facility/FacilityHome.tsx src/components/Common/Export.tsx src/components/Common/SortDropdown.tsx src/components/Diagnosis/ConditionVerificationStatusMenu.tsx
Length of output: 5289
src/components/Shifting/ShiftingList.tsx (2)
1-22: LGTM! Clean import structure aligned with architectural changes.
The imports reflect the shift from card-based to table-based view while maintaining essential UI components.
Line range hint 110-114: Verify ShiftingTable integration with the new design.
Please ensure that the ShiftingTable component properly implements the new design requirements for the patient details page redesign.
✅ Verification successful
Based on the shell script results, I can now generate the final response:
The ShiftingTable component appears to be properly integrated with the new design
The component shows a well-structured design implementation with:
- Responsive grid layouts that adapt based on screen sizes
- Proper styling using Tailwind classes for spacing, colors, and typography
- Consistent use of design tokens for colors (e.g., text-secondary-600, bg-sky-200)
- Clear information hierarchy with appropriate font weights and sizes
- Icon integration with the CareIcon component
- Proper handling of patient details, contact info, status, and facility information
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for ShiftingTable component implementation
ast-grep --pattern 'export default function ShiftingTable'
# Look for design-related props and styling
rg -A 10 'className=' src/components/Shifting/ShiftingTable.tsx
Length of output: 7584
cypress/pageobject/Patient/PatientConsultation.ts (2)
125-125: LGTM: Navigation to Encounters tab added
The addition of clicking the "Encounters" link aligns with the new tabbed interface mentioned in the PR objectives.
126-129: Consider updating the selector name for consistency
The selector #view_consultation_and_log_updates no longer matches the updated button text "View Updates". Consider updating the selector name to maintain consistency with the UI changes.
- "#view_consultation_and_log_updates",
+ "#view_updates",src/components/Patient/PatientDetailsTab/SampleTestHistory.tsx (2)
1-17: LGTM! Well-organized imports with clear separation of concerns.
The imports are properly structured with clear grouping of:
- Core React and routing dependencies
- UI components and icons
- Utility functions and types
1-106: Verify integration with the new tab-based layout.
The component implementation aligns with the PR objectives for the patient details page redesign. However, ensure that the component integrates properly with the new tab-based layout and maintains consistency with other tabs.
✅ Verification successful
Based on the results, I can now provide a final verification response. The search results show multiple tab components under PatientDetailsTab directory and their implementations, which confirms proper integration with the tab-based layout.
Component properly integrates with the new tab-based layout
The implementation aligns well with the tab-based redesign:
- Component is correctly placed in the
PatientDetailsTabdirectory alongside other tab components (Demography, EncounterHistory, etc.) - Follows consistent styling patterns with other tabs (mt-4 spacing, similar header structures)
- Shares the same
PatientPropsinterface used by other tab components - Maintains consistent layout patterns for content organization
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for tab-related components and verify integration
rg -l "PatientDetailsTab" --type tsx
# Check for consistent styling patterns across tabs
rg -l "className=\"mt-4 px-3 md:px-0\"" --type tsx
Length of output: 143
Script:
#!/bin/bash
# Search for tab-related components and verify integration
rg -l "PatientDetailsTab"
# Check for consistent styling patterns across tabs
rg -l "className=\"mt-4 px-3 md:px-0\""
# Look for other tab components in the patient details section
rg -l "PatientDetails.*Tab"
# Search for tab-based layout implementation
ast-grep --pattern 'const $_ = ($_: PatientProps) => {
$$$
}'
Length of output: 84872
src/components/Common/Breadcrumbs.tsx (4)
75-75: LGTM! Improved text contrast
The darker gray (600) for the current breadcrumb item improves readability and creates better visual hierarchy.
94-97: LGTM! Enhanced back button visibility
The styling changes improve the back button's visibility and clickable area through:
- Better spacing with increased margin
- Clear visual feedback with background and hover states
- Consistent text contrast
113-113: LGTM! Consistent link styling
The home button styling maintains consistency with other link buttons while improving readability.
80-80: Verify padding consistency with other link buttons
While the styling improvements enhance readability, verify that the p-1 padding aligns with other link buttons in the application for consistent spacing.
✅ Verification successful
Padding is consistent with application patterns
The p-1 padding on the breadcrumb link is consistent with the application's button and link patterns. The codebase shows that p-1 to p-4 padding values are commonly used across interactive elements, with p-1 and p-2 being the most frequent choices for compact link-style buttons. The current padding maintains visual harmony with the rest of the interface.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other link button implementations to verify padding consistency
rg -t tsx -t jsx 'variant="link".*className=".*p-\d.*"' --no-heading
Length of output: 98
Script:
#!/bin/bash
# Search for className patterns with padding in tsx/jsx files
rg --type-add 'react:*.{tsx,jsx}' -t react 'className="[^"]*p-\d[^"]*"' -A 2 -B 2
# Also search for styled buttons or links to understand padding patterns
rg --type-add 'react:*.{tsx,jsx}' -t react 'button.*className|Link.*className' -A 2 -B 2
Length of output: 100945
public/locale/mr.json (4)
52-52: LGTM: Patient Profile & General Info translations look good
The translations for basic patient information are clear and consistent with existing terminology.
Also applies to: 67-70
73-78: LGTM: COVID-related translations are comprehensive
The COVID-related translations cover all necessary information including vaccination details and travel history.
79-88: LGTM: Location and social profile translations are complete
The translations for location details and social profile information are thorough and consistent with existing terminology.
92-97: LGTM: Medical information translations are clear
The medical-related translations effectively cover present health, medications, and specific conditions.
src/components/Patient/PatientDetailsTab/ImmunisationRecords.tsx (1)
16-124: Verify component integration with patient details page
Let's verify that this component is properly integrated into the patient details page redesign as mentioned in the PR objectives.
✅ Verification successful
Component is properly integrated into patient details page
The verification shows that:
- The ImmunisationRecords component is properly imported in
index.tsx - It's correctly integrated as one of the tab components in the patient details page
- The component receives the required props (facilityId, id, patientData) through the PatientProps interface
- It's part of a structured tab system alongside other patient-related components like ShiftingHistory and SampleTestHistory
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify component integration and usage
# Check if the component is imported in the patient details page
echo "Checking component integration..."
rg -l "ImmunisationRecords" "src/components/Patient/PatientDetailsTab/"
# Check if the component is used within tabs structure
echo "Checking tabs implementation..."
ast-grep --pattern 'const tabs = [
$$$
{
$$$
component: ImmunisationRecords,
$$$
},
$$$
]'
Length of output: 448
Script:
#!/bin/bash
# Let's check the actual implementation in index.tsx to verify integration
echo "Checking index.tsx implementation..."
rg -A 10 "ImmunisationRecords" "src/components/Patient/PatientDetailsTab/index.tsx"
# Also check if there are any other tab components to understand the structure
echo "Checking tab structure..."
ast-grep --pattern 'import { $_ } from "./$_"' "src/components/Patient/PatientDetailsTab/index.tsx"
Length of output: 815
src/Routers/AppRouter.tsx (3)
124-124: LGTM: Improved layout structure with relative positioning
Adding relative positioning to the flex container is a good improvement as it:
- Creates a new stacking context for the patient page tabs
- Maintains proper containment for fixed/absolute positioned elements
- Helps with the responsive layout requirements mentioned in the PR
Line range hint 162-166: Verify spacing for patient details tabs
The container for page content uses fixed spacing (mt-4 p-3) which might affect the tab layout mentioned in the PR objectives.
Let's check the tab component implementation:
✅ Verification successful
Based on the search results, I can now provide a conclusive response about the spacing concern for patient details tabs.
Fixed spacing in page container won't affect tab layout
The tab layout in patient details is implemented with its own spacing system:
- The tabs container uses
flexlayout with proper spacing classes - Individual tabs have their own padding (
px-4 py-2) and styling - The page container's
mt-4 p-3spacing provides a consistent frame without interfering with the tab component's internal layout
The tab implementation in PatientHome.tsx shows that:
- Tabs are rendered in a flex container with proper spacing
- Each tab has its own padding and hover states
- The active tab uses a bottom border highlight that's not affected by the container spacing
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Look for tab components that might need spacing adjustments
ast-grep --pattern 'const $_ = $$$; export function $_($$$) { $$$ tabs $$$}'
Length of output: 78
Script:
#!/bin/bash
# Let's search for tab-related components and their spacing implementation
rg -l "Tab" --type tsx
# Then look for specific tab implementations related to patient details
rg "patient.*tab" -i --type tsx -A 5
Length of output: 117
Script:
#!/bin/bash
# Search for tab-related components without file type restriction
rg -l "Tab"
# Look for patient-related tabs in the codebase
rg "patient.*tab" -i -A 5
# Also search for any tab-related styling classes
rg "className.*tab" -A 2
Length of output: 25374
114-114: Verify layout stability after removing 'absolute' positioning
The removal of the absolute class from the main container could affect the layout stability, especially for the patient details page tabs mentioned in the PR objectives.
Let's check for any layout dependencies:
✅ Verification successful
Let me also check for specific layout dependencies in the AppRouter and related components:
Based on the search results, I can now generate the final response:
Layout changes in AppRouter appear safe and aligned with patient home page redesign
The removal of absolute positioning from the main container is safe because:
- The layout hierarchy uses a combination of
relative,flex, andoverflowproperties that maintain structural integrity - Patient-related components (
PatientHome.tsx,PatientDetailsTab/*) already use flex-based layouts - The change to
overflow-hiddenon the main container withoverflow-y-autoon child elements creates a proper scrolling context
The modifications align well with modern responsive design patterns and won't negatively impact the patient details page tabs or other components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for layout-dependent components that might be affected
rg -l 'absolute|fixed|relative|sticky' --type tsx --type jsx src/
Length of output: 95
Script:
#!/bin/bash
# Search for layout-dependent components that might be affected, using correct file extensions
rg -l 'absolute|fixed|relative|sticky' --type ts --type js --type jsx --type tsx src/
Length of output: 115
Script:
#!/bin/bash
# Check AppRouter and its direct imports for layout-related classes
rg -A 5 'className=.*(?:absolute|fixed|relative|sticky|overflow)' src/Routers/AppRouter.tsx
# Look for components rendered within AppRouter
ast-grep --pattern 'const AppRouter = () => {
$$$
return (
$$$
)
}' src/Routers/AppRouter.tsx
Length of output: 1244
Script:
#!/bin/bash
# Let's try a more specific search using ripgrep without file type restrictions
rg -l 'className.*(?:absolute|fixed|relative|sticky)' src/
# Also check for any components that might be affected by overflow changes
rg -l 'className.*overflow' src/
# Look for the patient details page components specifically
fd -e ts -e tsx -e js -e jsx . src/ --exec grep -l "patient.*details"
Length of output: 7955
src/components/Patient/PatientDetailsTab/HealthProfileSummary.tsx (2)
14-16: LGTM!
The component definition and props destructuring follow React best practices.
17-19: LGTM!
Hooks are properly initialized following React hooks rules.
src/components/Shifting/ShiftingBlock.tsx (1)
16-16: LGTM! Props interface change improves component flexibility.
Making onTransfer optional allows the component to be used in contexts where transfer functionality isn't needed, which aligns well with the page redesign objectives.
src/components/Patient/PatientDetailsTab/Notes.tsx (1)
1-184: Verify integration with patient home page redesign.
Please ensure that:
- The component integrates correctly with the new tab structure.
- The notes functionality works seamlessly for both doctors and nurses.
- Real-time updates are properly synchronized across different tabs.
✅ Verification successful
Let me try with corrected file type flags for the search commands.
Let me check the tab structure integration with one final verification.
Component integration is properly implemented
Based on the verification results:
- The Notes component is correctly integrated into the tab structure through
PatientDetailsTab/index.tsx - Real-time updates are properly synchronized using
useMessageListeneracross all note-related components - Message handling for doctor notes is consistently implemented in all relevant components
- The component handles both doctor and nurse notes through the
PATIENT_NOTES_THREADSconstant
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify component integration and dependencies
# Check for component imports in parent components
echo "Checking component usage..."
rg -l "PatientNotes" --type tsx
# Verify message handling implementation
echo "Checking message handling..."
rg -A 5 "patient/doctor_notes" --type tsx
# Check for related test files
echo "Checking test coverage..."
fd "Notes.test.tsx|Notes.spec.tsx" --type f
Length of output: 368
Script:
#!/bin/bash
# Description: Verify component integration and dependencies
# Check for component imports in parent components
echo "Checking component usage..."
rg -l "PatientNotes"
# Verify message handling implementation
echo "Checking message handling..."
rg -A 5 "patient/doctor_notes"
# Check for tab structure implementation
echo "Checking tab structure..."
rg -l "PatientDetailsTab"
# Check real-time updates implementation
echo "Checking real-time updates..."
rg -A 5 "useMessageListener"
Length of output: 9040
Script:
#!/bin/bash
# Check tab integration in PatientHome
echo "Checking PatientHome integration..."
rg -A 10 "PatientDetailsTab" src/components/Patient/PatientHome.tsx
# Check tab structure implementation
echo "Checking tab implementation..."
rg -A 10 "Notes" src/components/Patient/PatientDetailsTab/index.tsx
Length of output: 1004
src/components/Patient/SampleTestCard.tsx (1)
19-20: Verify type changes across the codebase
The change from number to string for facilityId and patientId could impact API compatibility and route handling. These IDs are used in navigation paths and potentially in API requests.
Let's verify the consistency of these types across the codebase:
✅ Verification successful
Type changes are consistent with the codebase
The search results confirm that facilityId and patientId are consistently typed as string across multiple components in the codebase, including:
- Treatment summary components
- Consultation-related components
- Patient notes components
- Investigation components
- Critical care components
The only exception is in PatientNotes.tsx where both fields are typed as any, which is a separate concern not related to this change.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for facilityId and patientId type definitions
ast-grep --pattern 'interface $_ {
$$$
facilityId: number;
$$$
}'
# Check for direct usage in API routes and components
rg -A 2 'facilityId:|patientId:' --type typescript
Length of output: 349
Script:
#!/bin/bash
# Search for type definitions and usages with proper file type
rg -t ts -t tsx 'facilityId:|patientId:' -B 2 -A 2
# Search for interfaces containing these fields using a more specific pattern
ast-grep --pattern 'interface $_ {
$$$
patientId: $_
$$$
}'
# Also search for type definitions using type keyword
ast-grep --pattern 'type $_ = {
$$$
facilityId: $_
$$$
}'
Length of output: 6629
cypress/e2e/patient_spec/PatientRegistration.cy.ts (1)
133-133: LGTM! Comprehensive medical history verification.
The added verification step thoroughly checks all medical conditions and patient health details, ensuring data persistence after patient creation.
src/components/Shifting/ShiftingTable.tsx (2)
186-196: Verify wartime shifting feature flag usage
The wartime shifting feature flag should be consistently applied across the application:
✅ Verification successful
Let me check the actual usage of the wartimeShifting flag in these files to ensure consistency.
Feature flag wartimeShifting is consistently implemented across shifting-related components
The wartimeShifting feature flag is properly and consistently used across all shifting-related components:
- Conditionally renders approving facility UI in
ShiftingTable.tsxandShiftingBlock.tsx - Controls shifting choices (wartime vs peacetime) in
ShiftDetails.tsxandShiftingBoard.tsx - Manages form fields and validation in
ShiftDetailsUpdate.tsxandShiftCreate.tsx - Handles filtering options in
ShiftingFilters.tsx - Properly configured in
care.config.tsbased on environment variable
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for consistent usage of wartimeShifting feature flag
rg -l "wartimeShifting"
# Check for related components that might need the same flag
ast-grep --pattern 'careConfig.$_Shifting'
Length of output: 380
Script:
#!/bin/bash
# Check how wartimeShifting is used in the identified files
rg "wartimeShifting" -B 2 -A 2
# Check the feature flag definition in config
cat care.config.ts
Length of output: 10358
90-258: 🛠️ Refactor suggestion
Consider extracting shift row into a separate component
The shift row rendering logic is complex and would benefit from being extracted into its own component:
interface ShiftRowProps {
shift: ShiftingModel;
hidePatient: boolean;
onTransferComplete: (shift: ShiftingModel) => Promise<void>;
}
const ShiftRow: React.FC<ShiftRowProps> = ({ shift, hidePatient, onTransferComplete }) => {
// Extract row rendering logic here
};This would:
- Improve code organization and maintainability
- Make the main component more focused
- Allow for better testing of individual row logic
✅ Verification successful
Let me analyze the results to verify if there are similar patterns or existing row components in the codebase that could inform our recommendation.
This will help us understand the existing patterns of table component organization in the codebase and verify if there are other shifting-related components that could inform our recommendation.
Looking at the codebase context, I can now generate the final response based on the patterns observed in the table components.
Extracting shift row into a separate component would improve code organization
- The codebase already follows a pattern of separating row components in other tables (e.g.,
MedicineAdministrationTableRow,TestRow, etc.) - This refactoring would align with the existing codebase patterns and improve maintainability
- The row component should be placed in the same directory as
ShiftingTable.tsx
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for similar patterns in other table components
ast-grep --pattern 'const $_ = ({ $_ }: { $_ }) => {
return (
<div className="w-full border-b-2">
$$$
</div>
);
}'
Length of output: 134
Script:
#!/bin/bash
# Look for similar table row patterns with different class names
ast-grep --pattern 'const $_ = ({ $_ }: { $_ }) => {
return (
<div $$$>
<div className="$$$grid$$$">
$$$
</div>
</div>
);
}'
# Also check for existing row components
fd -e tsx -e ts -e jsx -e js "Row.tsx" -X cat
Length of output: 10641
Script:
#!/bin/bash
# Let's check for other table components to see the pattern of component organization
fd -e tsx "Table.tsx" -X cat
# Also check if there are any other similar shifting components
rg -l "ShiftingModel" --type ts --type tsx
Length of output: 46318
src/components/Patient/models.tsx (1)
139-141: Approve type change but verify migration of existing code
The change to assigned_to?: number | null is a good improvement as it:
- Follows REST API best practices by using ID references
- Provides better type safety
- Maintains data normalization with
assigned_to_objectfor detailed information
However, this is a breaking change that requires verification.
Let's verify the migration of existing code:
✅ Verification successful
Let me correct the file type flags and try again with a modified script.
Let me try one more time with the correct ripgrep type flags.
Breaking change verified - migration appears complete
The type change from object to number | null appears safe to proceed with, as the verification shows:
- No direct access to
assigned_to.first_name,assigned_to.username, orassigned_to.last_namewas found in the codebase - All existing usages of
assigned_totreat it as an ID (string or number) or null value - The pattern of having both
assigned_toandassigned_to_objectis consistently used across components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining usage of the old structure (accessing properties directly from assigned_to)
# This would help identify code that needs to be updated to use assigned_to_object instead
# Search for direct property access on assigned_to
echo "Checking for direct property access on assigned_to that needs migration..."
rg -l "assigned_to\.(first_name|username|last_name)" --type ts --type tsx
# Search for proper usage of assigned_to_object
echo "Checking current usage of assigned_to_object..."
rg -l "assigned_to_object\.(first_name|last_name)" --type ts --type tsx
# Search for assigned_to type declarations to ensure consistency
echo "Checking other assigned_to type declarations..."
ast-grep --pattern 'assigned_to?: $_'
Length of output: 614
Script:
#!/bin/bash
# Description: Check for any remaining usage of the old structure with correct file type flags
# Search for direct property access on assigned_to
echo "Checking for direct property access on assigned_to that needs migration..."
rg -l "assigned_to\.(first_name|username|last_name)" -t typescript -t tsx
# Search for proper usage of assigned_to_object
echo "Checking current usage of assigned_to_object..."
rg -l "assigned_to_object\.(first_name|last_name)" -t typescript -t tsx
# Let's also check for any assignments or comparisons with assigned_to
echo "Checking assigned_to usage patterns..."
rg "assigned_to\s*[=:]" -t typescript -t tsx -A 1
Length of output: 657
Script:
#!/bin/bash
# Description: Check for any remaining usage of the old structure with correct file type flags
# Search for direct property access on assigned_to
echo "Checking for direct property access on assigned_to that needs migration..."
rg -l "assigned_to\.(first_name|username|last_name)" --type-add 'ts:*.{ts,tsx}'
# Search for proper usage of assigned_to_object
echo "Checking current usage of assigned_to_object..."
rg -l "assigned_to_object\.(first_name|last_name)" --type-add 'ts:*.{ts,tsx}'
# Let's also check for any assignments or comparisons with assigned_to
echo "Checking assigned_to usage patterns..."
rg "assigned_to\s*[=:]" --type-add 'ts:*.{ts,tsx}' -A 1
Length of output: 3301
src/components/Users/ManageUsers.tsx (2)
253-253: LGTM: Simplified className for name container
The removal of items-center class is appropriate as the div primarily contains text content and doesn't require vertical centering.
Line range hint 1-1000: Enhance security with consistent role-based access control
Several operations need additional security checks:
- Working hours updates
- Facility management operations
- User deletion
Consider implementing a centralized permission system:
// permissions.ts
export enum UserPermission {
UPDATE_WORKING_HOURS = 'UPDATE_WORKING_HOURS',
MANAGE_FACILITIES = 'MANAGE_FACILITIES',
DELETE_USER = 'DELETE_USER',
}
export function hasPermission(userType: string, permission: UserPermission): boolean {
const permissionMap = {
[UserPermission.UPDATE_WORKING_HOURS]: ['DistrictAdmin', 'StateAdmin'],
[UserPermission.MANAGE_FACILITIES]: ['DistrictAdmin', 'StateAdmin'],
[UserPermission.DELETE_USER]: ['StateAdmin'],
};
return permissionMap[permission]?.includes(userType) || false;
}Then use this system consistently throughout the component:
const handleWorkingHourSubmit = async () => {
+ if (!hasPermission(authUser.user_type, UserPermission.UPDATE_WORKING_HOURS)) {
+ Notification.Error({ msg: "Unauthorized operation" });
+ return;
+ }
// ... rest of the function
};public/locale/hi.json (1)
570-570: LGTM! Translation addition follows conventions.
The new translation key-value pair is properly formatted and uses correct Hindi characters for "Patient Profile".
public/locale/kn.json (1)
571-571: LGTM! Verify the translation key usage.
The added translation for "patient_profile" is properly formatted and consistent with the localization file structure.
Let's verify that this translation key is being used in the codebase:
✅ Verification successful
Translation key is actively used in the codebase
The translation key "patient_profile" is properly used in the codebase:
- It's consistently defined across all language files (en.json, hi.json, kn.json, ml.json, mr.json)
- The key is actively used in
src/components/Patient/PatientHome.tsxwhere it's referenced in the page title
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for usage of the "patient_profile" translation key in the codebase
# Expected: Find references to this key in React components, particularly in the Patient Home Page
# Search for direct key usage
rg "patient_profile" --type ts --type tsx --type js --type jsx
# Search for dynamic key usage patterns that might include this key
rg "t\(['\"]patient_profile['\"]" --type ts --type tsx --type js --type jsx
Length of output: 200
Script:
#!/bin/bash
# Let's try a different approach to find usage of the translation key
# Search for the key in all files, showing context
rg -C 2 "patient_profile"
# Also search for potential dynamic translation patterns
rg -C 2 't\(['\''"]\w*patient\w*['\''"]\)'
Length of output: 27676
public/locale/ml.json (1)
570-570: LGTM! Translation added for patient profile section.
The new Malayalam translation for "patient_profile" is correctly added and aligns with the PR objective of implementing the Patient Home Page redesign. The translation maintains consistency with the existing entries in terms of both key naming convention and language style.
public/locale/en.json (3)
277-285: LGTM! Well-structured action and consultation related keys.
The keys follow consistent naming conventions and provide clear, concise translations.
1029-1033: LGTM! Well-structured patient profile section keys.
The keys are logically grouped and follow consistent naming patterns using double underscores for namespacing.
1323-1326: LGTM! Clear and consistent transfer status keys.
The transfer status related keys are well-organized and provide clear translations.
src/components/Facility/ConsultationCard.tsx (8)
1-2: Imports added are necessary
The imports of careConfig and dayjs are appropriate for the added functionality in the component.
8-8: Importing CareIcon for icon usage
Including CareIcon allows for consistent icon usage across the application.
52-52: Proper error handling with localization
Using {t("invalid_patient_data")} ensures the error message is localized, enhancing user experience for different languages.
55-63: Enhanced UI with Chip component
The introduction of the Chip component improves the visual distinction between encounter types, making the UI more intuitive.
73-98: Dynamic rendering of consultation details improves maintainability
Using an array of field objects to render consultation details dynamically simplifies the component and makes it easier to maintain.
Line range hint 100-117: Conditional display of admission and discharge information
The logic for conditionally displaying admission and discharge dates is correctly implemented, ensuring accurate information is shown to the user.
138-160: Navigation buttons for updates and files
The buttons for viewing updates and files are properly configured, enhancing user navigation within the consultation details.
163-185: Handling 'Add Consultation Update' action with appropriate checks
The conditional logic ensures that users cannot add consultation updates without assigning a bed when required, which maintains data integrity.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
public/locale/en.json (2)
655-659: Consider adding error state messages for emergency contact operations.While the emergency contact related keys are well-structured, consider adding corresponding error state messages (e.g.,
emergency_contact_update_error,emergency_contact_save_error) for better error handling in the UI.+ "emergency_contact_update_error": "Failed to update emergency contact. Please try again.", + "emergency_contact_save_error": "Failed to save emergency contact. Please try again."
799-802: Standardize insurance-related key prefixes.Some insurance-related keys use
insurance__prefix while others don't. Consider standardizing all insurance-related keys to use the same prefix pattern for better maintainability.- "insurer_id": "Insurer ID", - "insurer_name": "Insurer Name", - "member_id": "Member ID", + "insurance__insurer_id": "Insurer ID", + "insurance__insurer_name": "Insurer Name", + "insurance__member_id": "Member ID"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
public/locale/en.json(39 hunks)src/components/Patient/PatientRegister.tsx(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Patient/PatientRegister.tsx
🔇 Additional comments (3)
public/locale/en.json (3)
277-285: LGTM! Clear and consistent naming pattern for action-related keys.
The new keys follow the established naming conventions and provide clear, user-friendly messages.
Line range hint 952-976: LGTM! Comprehensive empty state messages for medical history.
The empty state messages are clear and follow a consistent pattern.
1324-1327: LGTM! Clear transfer status messages.
The transfer status related keys provide clear feedback to users about the transfer state.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
src/components/Patient/PatientDetailsTab/ImmunisationRecords.tsx (3)
1-17: Consider improving import organization and pathsThe imports could be better organized and the relative path on line 15 could be improved:
-import * as Notification from "../../../Utils/Notifications"; +import * as Notification from "@/Utils/Notifications"; // Group imports by type: // 1. External dependencies // 2. Components/Icons // 3. Models/Types // 4. Hooks // 5. Utils/Constants
18-23: Add prop types validationConsider adding runtime prop types validation using PropTypes or Zod to catch potential issues early:
import { z } from "zod"; const patientPropsSchema = z.object({ patientData: z.object({ is_active: z.boolean(), is_vaccinated: z.boolean().optional(), number_of_doses: z.number().optional(), // ... other fields }), facilityId: z.string(), id: z.string() }); // Validate in component useEffect(() => { const result = patientPropsSchema.safeParse(props); if (!result.success) { console.error('Invalid props:', result.error); } }, [props]);
24-29: Add error handling for navigationConsider adding error handling for the navigation operation:
const handleEditClick = (sectionId: string) => { + try { navigate( `/facility/${facilityId}/patient/${id}/update?section=${sectionId}`, ); + } catch (error) { + Notification.Error({ + msg: t("navigation_failed"), + }); + } };src/common/constants.tsx (1)
1721-1721: Consider deriving ADMIN_USER_TYPES from USER_TYPE_OPTIONSWhile the implementation is correct, we can improve type safety and maintainability by deriving the admin user types from the existing
USER_TYPE_OPTIONSconstant to avoid potential inconsistencies.Consider this alternative implementation:
-export const ADMIN_USER_TYPES = ["DistrictAdmin", "StateAdmin"] as const; +export const ADMIN_USER_TYPES = USER_TYPE_OPTIONS + .filter((type) => type.id === "DistrictAdmin" || type.id === "StateAdmin") + .map((type) => type.id) as readonly ["DistrictAdmin", "StateAdmin"];This approach:
- Ensures consistency with
USER_TYPE_OPTIONS- Maintains type safety through the explicit type assertion
- Makes maintenance easier as changes to admin types in
USER_TYPE_OPTIONSwill be automatically reflected
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
src/common/constants.tsx(1 hunks)src/components/Patient/PatientDetailsTab/HealthProfileSummary.tsx(1 hunks)src/components/Patient/PatientDetailsTab/ImmunisationRecords.tsx(1 hunks)src/components/Patient/PatientDetailsTab/SampleTestHistory.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/Patient/PatientDetailsTab/HealthProfileSummary.tsx
- src/components/Patient/PatientDetailsTab/SampleTestHistory.tsx
🔇 Additional comments (1)
src/components/Patient/PatientDetailsTab/ImmunisationRecords.tsx (1)
63-119: 🛠️ Refactor suggestion
Add semantic HTML and improve accessibility
The data display section needs semantic HTML elements and ARIA attributes:
-<div className="mb-8 mt-2 grid grid-cols-1 gap-x-4 gap-y-2 md:grid-cols-2 md:gap-y-8 lg:grid-cols-2">
+<dl className="mb-8 mt-2 grid grid-cols-1 gap-x-4 gap-y-2 md:grid-cols-2 md:gap-y-8 lg:grid-cols-2" role="list">
{/* For each data field */}
- <div className="sm:col-span-1">
- <div className="text-sm font-semibold leading-5 text-zinc-400">
+ <div className="sm:col-span-1" role="listitem">
+ <dt className="text-sm font-semibold leading-5 text-zinc-400">
{t("number_of_covid_vaccine_doses")}
- </div>
- <div className="mt-1 text-sm font-medium leading-5">
+ </dt>
+ <dd className="mt-1 text-sm font-medium leading-5">
{patientData.is_vaccinated && patientData.number_of_doses
? patientData.number_of_doses
: "-"}
- </div>
+ </dd>
</div>Additionally, I notice that the past review comment about extracting repeated data display patterns is still valid and would help reduce code duplication.
| @@ -136,7 +136,7 @@ export interface PatientModel { | |||
| is_declared_positive?: boolean; | |||
| last_edited?: UserBareMinimum; | |||
| created_by?: UserBareMinimum; | |||
| assigned_to?: { first_name?: string; username?: string; last_name?: string }; | |||
| assigned_to?: number | null; | |||
There was a problem hiding this comment.
Why do we get a number for ID from the backend 🤔
|
@shivankacker @shivankacker 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! 🙌 |
Co-authored-by: mahendar <mahendarchikkolla@gmail.com> Co-authored-by: vinu tv <vinutv91@gmail.com>
Proposed Changes
Proposed:
Modified page:
Need further suggestion on implementing ABHA card and Hospital ID Card
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Release Notes
New Features
ShiftingTablecomponent for displaying patient shift data.Improvements
PatientHomecomponent with a new tabbed interface for better organization of patient details.ShiftingBlockcomponent for optional transfer handling.PatientInfoCardandInsuranceDetailscomponents.Chipcomponent's styling for better visual consistency.Bug Fixes