(fix) O3-1594 BUG: Patient chart close button has inconsistent behavi…

…or (#1581)
openmrs · Jan 15, 2024 · 87d6161 · 87d6161
1 parent 6bac380
commit 87d6161
Show file tree

Hide file tree

Showing 7 changed files with 366 additions and 174 deletions.
diff --git a/packages/esm-patient-chart-app/src/visit-header/close-button.component.tsx b/packages/esm-patient-chart-app/src/visit-header/close-button.component.tsx
@@ -0,0 +1,40 @@
+import React from 'react';
+import { HeaderGlobalAction } from '@carbon/react';
+import { CloseFilled } from '@carbon/react/icons';
+import { getHistory, goBackInHistory, navigate, usePatient } from '@openmrs/esm-framework';
+import { useCallback } from 'react';
+import { useTranslation } from 'react-i18next';
+import styles from './close-button.scss';
+
+export function CloseButton() {
+  const { t } = useTranslation();
+  const { patientUuid } = usePatient();
+
+  const onClosePatientChart = useCallback(() => {
+    const history = getHistory();
+    // Get the last page the user was on before opening the patient chart by going backward
+    // through the history until a URL does not include patientUuid
+    let onCloseTarget = '';
+    for (let i = history.length - 1; i >= 0; i--) {
+      if (!history[i].includes(patientUuid)) {
+        onCloseTarget = history[i];
+        break;
+      }
+    }
+    if (onCloseTarget) {
+      goBackInHistory({ toUrl: onCloseTarget });
+    } else {
+      navigate({ to: '${openmrsSpaBase}/home' });
+    }
+  }, [patientUuid]);
+
+  return (
+    <HeaderGlobalAction
+      className={styles.headerGlobalBarCloseButton}
+      aria-label={t('close', 'Close')}
+      onClick={onClosePatientChart}
+    >
+      <CloseFilled size={20} />
+    </HeaderGlobalAction>
+  );
+}
diff --git a/packages/esm-patient-chart-app/src/visit-header/close-button.scss b/packages/esm-patient-chart-app/src/visit-header/close-button.scss
@@ -0,0 +1,5 @@
+@import '~@openmrs/esm-styleguide/src/vars';
+
+.headerGlobalBarCloseButton {
+  @include brand-02(background-color);
+}
diff --git a/packages/esm-patient-chart-app/src/visit-header/visit-header.component.tsx b/packages/esm-patient-chart-app/src/visit-header/visit-header.component.tsx
@@ -1,15 +1,13 @@
 import React, { useCallback, useState } from 'react';
 import { useTranslation } from 'react-i18next';
-import { Button, Header, HeaderGlobalAction, HeaderGlobalBar, HeaderMenuButton, Tag, Tooltip } from '@carbon/react';
-import { CloseFilled } from '@carbon/react/icons';
+import { Button, Header, HeaderGlobalBar, HeaderMenuButton, Tag, Tooltip } from '@carbon/react';
 import {
   age,
   ConfigurableLink,
   useAssignedExtensions,
   useLayoutType,
   usePatient,
   useVisit,
-  navigate,
   useConfig,
   showModal,
   ExtensionSlot,
@@ -21,6 +19,7 @@ import { EditQueueEntry } from '../visit/queue-entry/edit-queue-entry.component'
 import VisitHeaderSideMenu from './visit-header-side-menu.component';
 import styles from './visit-header.scss';
 import RetrospectiveVisitLabel from './retrospective-visit-label.component';
+import { CloseButton } from './close-button.component';
 
 interface PatientInfoProps {
   patient: fhir.Patient;
@@ -139,10 +138,6 @@ const VisitHeader: React.FC = () => {
 
   const toggleSideMenu = useCallback(() => setIsSideMenuExpanded((prevState) => !prevState), []);
 
-  const onClosePatientChart = useCallback(() => {
-    document.referrer === '' ? navigate({ to: `${window.spaBase}/home` }) : window.history.back();
-  }, []);
-
   const openModal = useCallback((patientUuid) => {
     const dispose = showModal('end-visit-dialog', {
       closeModal: () => dispose(),
@@ -198,13 +193,7 @@ const VisitHeader: React.FC = () => {
             )}
           </>
         )}
-        <HeaderGlobalAction
-          className={styles.headerGlobalBarCloseButton}
-          aria-label={t('close', 'Close')}
-          onClick={onClosePatientChart}
-        >
-          <CloseFilled size={20} />
-        </HeaderGlobalAction>
+        <CloseButton />
       </HeaderGlobalBar>
       <VisitHeaderSideMenu isExpanded={isSideMenuExpanded} toggleSideMenu={toggleSideMenu} />
     </Header>

diff --git a/packages/esm-patient-chart-app/src/visit-header/visit-header.scss b/packages/esm-patient-chart-app/src/visit-header/visit-header.scss
@@ -8,10 +8,6 @@
   @include brand-02(background-color);
 }
 
-.headerGlobalBarCloseButton {
-  @include brand-02(background-color);
-}
-
 .headerName {
   display: flex;
 }

diff --git a/packages/esm-patient-chart-app/src/visit-header/visit-header.test.tsx b/packages/esm-patient-chart-app/src/visit-header/visit-header.test.tsx
@@ -9,6 +9,9 @@ import {
   useVisit,
   navigate,
   showModal,
+  getHistory,
+  goBackInHistory,
+  age,
 } from '@openmrs/esm-framework';
 import { registerWorkspace, launchPatientWorkspace } from '@openmrs/esm-patient-common-lib';
 import { mockPatient, mockPatientWithLongName, getByTextWithMarkup } from 'tools';
@@ -19,26 +22,21 @@ const mockUseAssignedExtensions = useAssignedExtensions as jest.Mock;
 const mockUsePatient = usePatient as jest.Mock;
 const mockUseVisit = useVisit as jest.Mock;
 const mockUseLayoutType = useLayoutType as jest.Mock;
-const mockExtensionRegistry = {};
 const mockShowModal = showModal as jest.Mock;
-const mockNavigateBack = jest.fn();
+const mockGetHistory = getHistory as jest.Mock;
+const mockGoBackInHistory = goBackInHistory as jest.Mock;
 
 jest.mock('@openmrs/esm-framework', () => {
   const originalModule = jest.requireActual('@openmrs/esm-framework');
   return {
     ...originalModule,
-    usePatient: jest.fn(),
-    useAssignedExtensions: jest.fn(),
-    age: jest.fn(() => 20),
+    age: jest.fn().mockReturnValue('20'),
+    getHistory: jest.fn(() => []),
+    goBackInHistory: jest.fn(),
     LeftNavMenu: jest.fn().mockImplementation(() => <div>Left Nav Menu</div>),
-    useVisit: jest.fn(),
-    registerExtension: (ext) => {
-      mockExtensionRegistry[ext.name] = ext;
-    },
-    getExtensionRegistration: (name) => mockExtensionRegistry[name],
     translateFrom: (module, key, defaultValue, options) => defaultValue,
+    useAssignedExtensions: jest.fn(),
     useOnClickOutside: jest.fn(),
-    showModal: jest.fn(),
   };
 });
 
@@ -47,20 +45,19 @@ jest.mock('@openmrs/esm-patient-common-lib', () => {
   return {
     ...originalModule,
     launchPatientWorkspace: jest.fn(),
-    navigate: jest.fn(),
   };
 });
 
 describe('Visit Header', () => {
   beforeEach(() => {
-    window.history.back = mockNavigateBack;
+    mockUseAssignedExtensions.mockReturnValue([{ id: 'someId' }]);
+    mockGoBackInHistory.mockClear();
   });
 
   test('should display visit header and left nav bar hamburger icon', async () => {
     const user = userEvent.setup();
 
     registerWorkspace({ name: 'start-visit-workspace-form', title: 'Start visit', load: jest.fn() });
-    mockUseAssignedExtensions.mockReturnValue([{ id: 'someId' }]);
     mockUsePatient.mockReturnValue({
       patient: mockPatient,
       isLoading: false,
@@ -70,7 +67,7 @@ describe('Visit Header', () => {
     mockUseVisit.mockReturnValue({ isValidating: null, currentVisit: null });
     mockUseLayoutType.mockReturnValue('tablet');
 
-    renderVisitHeader();
+    render(<VisitHeader />);
 
     const headerBanner = screen.getByRole('banner', { name: /OpenMRS/i });
     expect(headerBanner).toBeInTheDocument();
@@ -96,25 +93,10 @@ describe('Visit Header', () => {
     expect(startVisitButton).toBeInTheDocument();
 
     await user.click(startVisitButton);
-    expect(launchPatientWorkspace).toHaveBeenCalled();
     expect(launchPatientWorkspace).toHaveBeenCalledWith('start-visit-workspace-form');
-
-    const closeButton = screen.getByRole('button', { name: 'Close' });
-    expect(closeButton).toBeInTheDocument();
-
-    // Should close the visit-header
-    await user.click(closeButton);
-    expect(navigate).toHaveBeenCalledWith({ to: '/spa/home' });
-    expect(window.history.back).not.toHaveBeenCalled();
-
-    Object.defineProperty(document, 'referrer', { value: 'some-uuid', configurable: true });
-    await user.click(closeButton);
-    expect(window.history.back).toHaveBeenCalled();
-    expect(mockNavigateBack).toHaveBeenCalled();
   });
 
   test('should display a truncated name when the patient name is very long', async () => {
-    mockUseAssignedExtensions.mockReturnValue([{ id: 'someId' }]);
     mockUsePatient.mockReturnValue({
       patient: mockPatientWithLongName,
       isLoading: false,
@@ -124,7 +106,7 @@ describe('Visit Header', () => {
     mockUseVisit.mockReturnValue({ isValidating: null, currentVisit: null });
     mockUseLayoutType.mockReturnValue('desktop');
 
-    renderVisitHeader();
+    render(<VisitHeader />);
 
     const longNameText = screen.getByText(/^Some very long given name...$/i);
     expect(longNameText).toBeInTheDocument();
@@ -133,7 +115,6 @@ describe('Visit Header', () => {
 
   it('should be able to show configurable stop visit button and modal to stop current visit', async () => {
     const user = userEvent.setup();
-    mockUseAssignedExtensions.mockReturnValue([{ id: 'someId' }]);
     mockUsePatient.mockReturnValue({
       patient: mockPatientWithLongName,
       isLoading: false,
@@ -143,7 +124,7 @@ describe('Visit Header', () => {
     mockUseVisit.mockReturnValue({ isValidating: false, currentVisit: mockCurrentVisit });
     mockUseLayoutType.mockReturnValue('desktop');
 
-    renderVisitHeader();
+    render(<VisitHeader />);
 
     // Should be able to end a visit
     const endVisitButton = screen.getByRole('button', { name: /End visit/i });
@@ -154,14 +135,33 @@ describe('Visit Header', () => {
     expect(mockShowModal).toHaveBeenCalledTimes(1);
 
     const closeButton = screen.getByRole('button', { name: 'Close' });
-    expect(closeButton).toBeInTheDocument();
+    await user.click(closeButton);
+    expect(navigate).toHaveBeenCalled();
+  });
 
-    // Should close the visit-header
+  test('close button should navigate back to before the patient chart', async () => {
+    const user = userEvent.setup();
+    mockGetHistory.mockReturnValue([
+      'https://o3.openmrs.org/openmrs/spa/home',
+      'https://o3.openmrs.org/openmrs/spa/patient/1234/chart',
+      `https://o3.openmrs.org/openmrs/spa/patient/${mockPatient.id}/chart`,
+      `https://o3.openmrs.org/openmrs/spa/patient/${mockPatient.id}/chart/labs`,
+    ]);
+    render(<VisitHeader />);
+    const closeButton = screen.getByRole('button', { name: 'Close' });
     await user.click(closeButton);
-    expect(navigate).toHaveBeenCalledWith({ to: '/spa/home' });
+    expect(goBackInHistory).toHaveBeenCalledWith({ toUrl: 'https://o3.openmrs.org/openmrs/spa/patient/1234/chart' });
   });
-});
 
-function renderVisitHeader() {
-  render(<VisitHeader />);
-}
+  test('close button should navigate to home if no such URL exists in history', async () => {
+    const user = userEvent.setup();
+    render(<VisitHeader />);
+    mockGetHistory.mockReturnValue([
+      `https://o3.openmrs.org/openmrs/spa/patient/${mockPatient.id}/chart`,
+      `https://o3.openmrs.org/openmrs/spa/patient/${mockPatient.id}/chart/labs`,
+    ]);
+    const closeButton = screen.getByRole('button', { name: 'Close' });
+    await user.click(closeButton);
+    expect(navigate).toHaveBeenCalledWith({ to: '${openmrsSpaBase}/home' });
+  });
+});
diff --git a/packages/esm-patient-chart-app/src/workspace/workspace-window.test.tsx b/packages/esm-patient-chart-app/src/workspace/workspace-window.test.tsx
@@ -19,10 +19,6 @@ jest.mock('@openmrs/esm-framework', () => {
   return {
     ...originalModule,
     isDesktop: jest.fn(),
-    getExtensionRegistration: (name) => mockExtensionRegistry[name],
-    registerExtension: (ext) => {
-      mockExtensionRegistry[ext.name] = ext;
-    },
     translateFrom: (module, key, defaultValue, options) => defaultValue,
     useBodyScrollLock: jest.fn(),
   };