(feat) O3-3759: Refactor usePatient hook to leverage useSWR

packages/apps/esm-offline-tools-app/src/hooks/offline-actions.ts

@@ -1,11 +1,6 @@
-import type { SyncItem } from '@openmrs/esm-framework/src/internal';
-import {
-  fetchCurrentPatient,
-  getFullSynchronizationItems,
-  getSynchronizationItems,
-} from '@openmrs/esm-framework/src/internal';
 import uniq from 'lodash-es/uniq';
 import useSWR from 'swr';
+import { fetchCurrentPatient, getFullSynchronizationItems, type SyncItem } from '@openmrs/esm-framework/src/internal';
 
 export function usePendingSyncItems() {
   return useSWR('offlineActions/pending', () => getFullSynchronizationItems());

packages/apps/esm-offline-tools-app/src/hooks/offline-patient-data-hooks.ts

@@ -1,8 +1,7 @@
+import { useMemo } from 'react';
+import useSWR, { type SWRResponse } from 'swr';
 import { fetchCurrentPatient, getSynchronizationItems, getDynamicOfflineDataEntries } from '@openmrs/esm-framework';
 import merge from 'lodash-es/merge';
-import { useMemo } from 'react';
-import type { SWRResponse } from 'swr';
-import useSWR from 'swr';
 
 function useDynamicOfflineDataEntries(type: string) {
   return useSWR(`dynamicOfflineData/entries/${type}`, () => getDynamicOfflineDataEntries(type));

packages/framework/esm-api/src/shared-api-objects/current-patient.test.ts

@@ -1,50 +1,52 @@
-import { fhirBaseUrl, openmrsFetch } from '../openmrs-fetch';
+import { getSynchronizationItems } from '@openmrs/esm-offline';
+import { type FetchResponse } from '../types';
 import { fetchCurrentPatient } from './current-patient';
+import { openmrsFetch } from '../openmrs-fetch';
 
-const mockOpenmrsFetch = openmrsFetch as jest.MockedFunction<any>;
+const mockOpenmrsFetch = jest.mocked(openmrsFetch);
+const mockGetSynchronizationItems = jest.mocked(getSynchronizationItems);
 
 jest.mock('../openmrs-fetch', () => ({
   openmrsFetch: jest.fn(),
+  fhirBaseUrl: '/ws/fhir2/R4',
 }));
 
-describe('current patient', () => {
+jest.mock('@openmrs/esm-offline', () => ({
+  getSynchronizationItems: jest.fn(),
+}));
+
+describe('fetchPatientData', () => {
   beforeEach(() => {
-    mockOpenmrsFetch.mockReset();
+    jest.clearAllMocks();
+    mockGetSynchronizationItems.mockResolvedValue([]);
+  });
+
+  it('should return null when patientUuid is falsy', async () => {
+    const result = await fetchCurrentPatient('');
+    expect(result).toBeNull();
   });
 
-  it('fetches the correct patient from a patient chart URL', () => {
-    mockOpenmrsFetch.mockReturnValueOnce(
-      Promise.resolve({
-        data: {},
-      }),
-    );
+  it('should return online patient data when available', async () => {
+    const mockPatient = { id: '123', name: [{ given: ['John'], family: 'Doe' }] };
+    mockOpenmrsFetch.mockResolvedValue({ data: mockPatient, ok: true } as Partial<FetchResponse> as FetchResponse);
 
-    return fetchCurrentPatient('12', undefined, false).then(() => {
-      expect(mockOpenmrsFetch).toHaveBeenCalledWith(`${fhirBaseUrl}/Patient/12`, undefined);
-    });
+    const result = await fetchCurrentPatient('123');
+    expect(result).toEqual(mockPatient);
   });
 
-  it('fetches the correct patient from the patient home URL', () => {
-    mockOpenmrsFetch.mockReturnValueOnce(
-      Promise.resolve({
-        data: {},
-      }),
-    );
+  it('should return offline patient data when online fetch fails', async () => {
+    const mockOfflinePatient = { id: '123', name: [{ given: ['Jane'], family: 'Doe' }] };
+    mockOpenmrsFetch.mockRejectedValue(new Error('Network error'));
+    mockGetSynchronizationItems.mockResolvedValue([{ fhirPatient: mockOfflinePatient }]);
 
-    return fetchCurrentPatient('34', undefined, false).then(() => {
-      expect(mockOpenmrsFetch).toHaveBeenCalledWith(`${fhirBaseUrl}/Patient/34`, undefined);
-    });
+    const result = await fetchCurrentPatient('123');
+    expect(result).toEqual(mockOfflinePatient);
   });
 
-  it('can handle dashes and alphanumeric characters in the patient uuid', () => {
-    mockOpenmrsFetch.mockReturnValueOnce(
-      Promise.resolve({
-        data: {},
-      }),
-    );
+  it('should throw an error when both online and offline fetches fail', async () => {
+    mockOpenmrsFetch.mockRejectedValue(new Error('Network error'));
+    mockGetSynchronizationItems.mockResolvedValue([]);
 
-    return fetchCurrentPatient('34-asdsd-234243h342', undefined, false).then(() => {
-      expect(mockOpenmrsFetch).toHaveBeenCalledWith(`${fhirBaseUrl}/Patient/34-asdsd-234243h342`, undefined);
-    });
+    await expect(fetchCurrentPatient('123')).rejects.toThrow('Network error');
   });
 });

packages/framework/esm-api/src/shared-api-objects/current-patient.ts

@@ -1,10 +1,10 @@
 /** @module @category API */
 import { getSynchronizationItems } from '@openmrs/esm-offline';
-import type { FetchConfig } from '../openmrs-fetch';
-import { fhirBaseUrl, openmrsFetch } from '../openmrs-fetch';
-import type { FetchResponse } from '../types';
+import { fhirBaseUrl, openmrsFetch, type FetchConfig } from '../openmrs-fetch';
+import { type FetchResponse } from '../types';
 
 export type CurrentPatient = fhir.Patient | FetchResponse<fhir.Patient>;
+
 export interface CurrentPatientOptions {
   includeConfig?: boolean;
 }

packages/framework/esm-framework/docs/API.md

@@ -271,7 +271,7 @@
 
 #### Defined in
 
-[packages/framework/esm-api/src/shared-api-objects/current-patient.ts:7](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-api/src/shared-api-objects/current-patient.ts#L7)
+[packages/framework/esm-api/src/shared-api-objects/current-patient.ts:6](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-api/src/shared-api-objects/current-patient.ts#L6)
 
 ___
 
@@ -2873,14 +2873,14 @@ a route listener is set up to update the patient whenever the route changes.
 
 | Name | Type |
 | :------ | :------ |
-| `error` | ``null`` \| `Error` |
+| `error` | `undefined` \| ``null`` \| `Error` |
 | `isLoading` | `boolean` |
-| `patient` | [`NullablePatient`](API.md#nullablepatient) |
+| `patient` | `undefined` \| [`NullablePatient`](API.md#nullablepatient) |
 | `patientUuid` | ``null`` \| `string` |
 
 #### Defined in
 
-[packages/framework/esm-react-utils/src/usePatient.ts:90](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-react-utils/src/usePatient.ts#L90)
+[packages/framework/esm-react-utils/src/usePatient.ts:19](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-react-utils/src/usePatient.ts#L19)
 
 ___
 

packages/framework/esm-framework/jest.config.js

@@ -17,5 +17,5 @@ module.exports = {
   testEnvironmentOptions: {
     url: 'http://localhost/',
   },
-  testTimeout: 10000,
+  testTimeout: 20000,
 };

packages/framework/esm-react-utils/src/usePatient.ts

@@ -1,153 +1,51 @@
 /** @module @category API */
-import { useEffect, useReducer } from 'react';
-import type { PatientUuid } from '@openmrs/esm-api';
+import { useEffect, useMemo, useState } from 'react';
+import useSWR from 'swr';
 import { fetchCurrentPatient } from '@openmrs/esm-api';
 
 export type NullablePatient = fhir.Patient | null;
 
-export interface CurrentPatientState {
-  patientUuid: string | null;
-  patient: NullablePatient;
-  isPendingUuid: boolean;
-  isLoadingPatient: boolean;
-  err: Error | null;
-}
-
-interface LoadPatient {
-  type: ActionTypes.loadPatient;
-  patientUuid: string | null;
-}
-
-interface NewPatient {
-  type: ActionTypes.newPatient;
-  patient: NullablePatient;
-}
-
-interface PatientLoadError {
-  type: ActionTypes.loadError;
-  err: Error | null;
-}
-
-type Action = LoadPatient | NewPatient | PatientLoadError;
-
-enum ActionTypes {
-  loadPatient = 'loadPatient',
-  newPatient = 'newPatient',
-  loadError = 'patientLoadError',
-}
-
-const initialState: CurrentPatientState = {
-  patientUuid: null,
-  patient: null,
-  isPendingUuid: true,
-  isLoadingPatient: false,
-  err: null,
-};
-
-function getPatientUuidFromUrl(): PatientUuid {
+function getPatientUuidFromUrl() {
   const match = /\/patient\/([a-zA-Z0-9\-]+)\/?/.exec(location.pathname);
   return match && match[1];
 }
 
-function reducer(state: CurrentPatientState, action: Action): CurrentPatientState {
-  switch (action.type) {
-    case ActionTypes.loadPatient:
-      return {
-        ...state,
-        patientUuid: action.patientUuid,
-        patient: null,
-        isPendingUuid: false,
-        isLoadingPatient: true,
-        err: null,
-      };
-    case ActionTypes.newPatient:
-      return {
-        ...state,
-        patient: action.patient,
-        isPendingUuid: false,
-        isLoadingPatient: false,
-        err: null,
-      };
-    case ActionTypes.loadError:
-      return {
-        ...state,
-        patient: null,
-        isPendingUuid: false,
-        isLoadingPatient: false,
-        err: action.err,
-      };
-    default:
-      return state;
-  }
-}
-
 /**
  * This React hook returns a patient object. If the `patientUuid` is provided
  * as a parameter, then the patient for that UUID is returned. If the parameter
  * is not provided, the patient UUID is obtained from the current route, and
  * a route listener is set up to update the patient whenever the route changes.
  */
 export function usePatient(patientUuid?: string) {
-  const [state, dispatch] = useReducer(reducer, {
-    ...initialState,
-    patientUuid: patientUuid ?? null,
-    isPendingUuid: !patientUuid,
-    isLoadingPatient: !!patientUuid,
-  });
-
-  useEffect(() => {
-    if (state.isPendingUuid) {
-      const patientUuidFromUrl = getPatientUuidFromUrl();
-      if (patientUuidFromUrl) {
-        dispatch({
-          type: ActionTypes.loadPatient,
-          patientUuid: patientUuidFromUrl,
-        });
-      } else {
-        dispatch({ type: ActionTypes.newPatient, patient: null });
-      }
-    }
+  const [currentPatientUuid, setCurrentPatientUuid] = useState(patientUuid ?? getPatientUuidFromUrl());
 
-    let active = true;
-    if (state.isLoadingPatient && state.patientUuid) {
-      fetchCurrentPatient(state.patientUuid).then(
-        (patient) =>
-          active &&
-          dispatch({
-            patient: patient,
-            type: ActionTypes.newPatient,
-          }),
-        (err) =>
-          active &&
-          dispatch({
-            err,
-            type: ActionTypes.loadError,
-          }),
-      );
-    }
-    return () => {
-      active = false;
-    };
-  }, [state.isPendingUuid, state.isLoadingPatient, state.patientUuid]);
+  const {
+    data: patient,
+    error,
+    isValidating,
+  } = useSWR<NullablePatient, Error | null>(currentPatientUuid ? ['patient', currentPatientUuid] : null, () =>
+    fetchCurrentPatient(currentPatientUuid!, {}),
+  );
 
   useEffect(() => {
-    const handleRouteUpdate = (evt) => {
+    const handleRouteUpdate = () => {
       const newPatientUuid = getPatientUuidFromUrl();
-      if (newPatientUuid != state.patientUuid) {
-        dispatch({
-          type: ActionTypes.loadPatient,
-          patientUuid: newPatientUuid,
-        });
+      if (newPatientUuid !== currentPatientUuid) {
+        setCurrentPatientUuid(newPatientUuid);
       }
     };
+
     window.addEventListener('single-spa:routing-event', handleRouteUpdate);
     return () => window.removeEventListener('single-spa:routing-event', handleRouteUpdate);
-  }, [state.patientUuid]);
-
-  return {
-    isLoading: state.isPendingUuid || state.isLoadingPatient,
-    patient: state.patient,
-    patientUuid: patientUuid ?? state.patientUuid,
-    error: state.err,
-  };
+  }, [currentPatientUuid]);
+
+  return useMemo(
+    () => ({
+      isLoading: isValidating && !error && !patient,
+      patient,
+      patientUuid: currentPatientUuid,
+      error,
+    }),
+    [isValidating, error, patient, currentPatientUuid],
+  );
 }