Repository: HospitalRun/hospitalrun-frontend (ts)

Issue:
Focus of menu item "Visits" disappears when I view the details
## 🐛 Bug Report

A clear and concise description of what the bug is.

## To Reproduce

When I view a visit detail tab, the focus of the "Visits" menu item disappears preventing me from understanding in which section I'm in.

Right behavior

<img width="1570" alt="Screenshot 2021-02-09 at 12 38 19" src="https://user-images.githubusercontent.com/6388707/107358559-d3439f00-6ad3-11eb-9248-0cdc9fb7a622.png">

Wrong behavior

<img width="1558" alt="Screenshot 2021-02-09 at 12 38 25" src="https://user-images.githubusercontent.com/6388707/107358566-d63e8f80-6ad3-11eb-8569-f561a5e64739.png">


Pull request:
fix(viewpatient): keep tabs active for nested routes
Fixes #2563 

**Note:** The bug was initially reported for the patient **visits tab**, but I realized that the same bug was also happening in the tabs: **allergies, notes, care-plans, care-goals**. Fixed all of them.

**Why the bug happened?**
When someone visits one of the mentioned tabs and, for example, clicks an element to view it, and it opens in a nested route, then the tab loses the active status because the route path changed.

**Changes proposed in this pull request:**

- fix: Keep corresponding tabs as active for nested routes
- Add missing warning alert in visits table (attached screenshots)


Warning alert before (shows empty table):
![image](https://user-images.githubusercontent.com/6878389/107455029-df0f7f80-6b2c-11eb-86fe-04586d2c4b36.png)

Warning alert after:
![image](https://user-images.githubusercontent.com/6878389/107454927-abccf080-6b2c-11eb-9026-7721a8f6eb70.png)


Patch:
diff --git a/src/patients/hooks/usePatientVisits.tsx b/src/patients/hooks/usePatientVisits.tsx
index 57a17d29..d202ea34 100644
--- a/src/patients/hooks/usePatientVisits.tsx
+++ b/src/patients/hooks/usePatientVisits.tsx
@@ -3,10 +3,11 @@ import { useQuery } from 'react-query'
 import PatientRepository from '../../shared/db/PatientRepository'
 import Visit from '../../shared/model/Visit'
 
-async function fetchPatientVisits(_: string, id: string): Promise<Visit[]> {
-  return (await PatientRepository.find(id)).visits
+async function fetchPatientVisits(_: string, patientId: string): Promise<Visit[]> {
+  const patient = await PatientRepository.find(patientId)
+  return patient.visits || []
 }
 
-export default function usePatientVisits(id: string) {
-  return useQuery(['visits', id], fetchPatientVisits)
+export default function usePatientVisits(patientId: string) {
+  return useQuery(['visits', patientId], fetchPatientVisits)
 }
diff --git a/src/patients/view/ViewPatient.tsx b/src/patients/view/ViewPatient.tsx
index 00355a8a..d16b9031 100644
--- a/src/patients/view/ViewPatient.tsx
+++ b/src/patients/view/ViewPatient.tsx
@@ -104,7 +104,7 @@ const ViewPatient = () => {
             onClick={() => history.push(`/patients/${patient.id}/appointments`)}
           />
           <Tab
-            active={location.pathname === `/patients/${patient.id}/allergies`}
+            active={location.pathname.startsWith(`/patients/${patient.id}/allergies`)}
             label={t('patient.allergies.label')}
             onClick={() => history.push(`/patients/${patient.id}/allergies`)}
           />
@@ -114,7 +114,7 @@ const ViewPatient = () => {
             onClick={() => history.push(`/patients/${patient.id}/diagnoses`)}
           />
           <Tab
-            active={location.pathname === `/patients/${patient.id}/notes`}
+            active={location.pathname.startsWith(`/patients/${patient.id}/notes`)}
             label={t('patient.notes.label')}
             onClick={() => history.push(`/patients/${patient.id}/notes`)}
           />
@@ -129,17 +129,17 @@ const ViewPatient = () => {
             onClick={() => history.push(`/patients/${patient.id}/labs`)}
           />
           <Tab
-            active={location.pathname === `/patients/${patient.id}/care-plans`}
+            active={location.pathname.startsWith(`/patients/${patient.id}/care-plans`)}
             label={t('patient.carePlan.label')}
             onClick={() => history.push(`/patients/${patient.id}/care-plans`)}
           />
           <Tab
-            active={location.pathname === `/patients/${patient.id}/care-goals`}
+            active={location.pathname.startsWith(`/patients/${patient.id}/care-goals`)}
             label={t('patient.careGoal.label')}
             onClick={() => history.push(`/patients/${patient.id}/care-goals`)}
           />
           <Tab
-            active={location.pathname === `/patients/${patient.id}/visits`}
+            active={location.pathname.startsWith(`/patients/${patient.id}/visits`)}
             label={t('patient.visits.label')}
             onClick={() => history.push(`/patients/${patient.id}/visits`)}
           />
diff --git a/src/patients/visits/VisitTable.tsx b/src/patients/visits/VisitTable.tsx
index 6a0c326e..faeb5b59 100644
--- a/src/patients/visits/VisitTable.tsx
+++ b/src/patients/visits/VisitTable.tsx
@@ -1,4 +1,4 @@
-import { Table } from '@hospitalrun/components'
+import { Alert, Table } from '@hospitalrun/components'
 import format from 'date-fns/format'
 import React from 'react'
 import { useHistory } from 'react-router-dom'
@@ -16,14 +16,24 @@ const VisitTable = ({ patientId }: Props) => {
 
   const { data: patientVisits, status } = usePatientVisits(patientId)
 
-  if (patientVisits === undefined && status === 'loading') {
+  if (patientVisits === undefined || status === 'loading') {
     return <Loading />
   }
 
+  if (patientVisits.length === 0) {
+    return (
+      <Alert
+        color="warning"
+        title={t('patient.visits.warning.noVisits')}
+        message={t('patient.visits.warning.addVisitAbove')}
+      />
+    )
+  }
+
   return (
     <Table
       getID={(row) => row.id}
-      data={patientVisits || []}
+      data={patientVisits}
       columns={[
         {
           label: t('patient.visits.startDateTime'),
diff --git a/src/shared/locales/enUs/translations/patient/index.ts b/src/shared/locales/enUs/translations/patient/index.ts
index 9eafb44d..d3227438 100644
--- a/src/shared/locales/enUs/translations/patient/index.ts
+++ b/src/shared/locales/enUs/translations/patient/index.ts
@@ -209,6 +209,10 @@ export default {
         reasonRequired: 'Reason is required.',
         locationRequired: 'Location is required.',
       },
+      warning: {
+        noVisits: 'No Visits',
+        addVisitAbove: 'Add a visit using the button above.',
+      },
     },
     types: {
       charity: 'Charity',


Test patch:
diff --git a/src/__tests__/patients/view/ViewPatient.test.tsx b/src/__tests__/patients/view/ViewPatient.test.tsx
index 5c8f160a..ab46713b 100644
--- a/src/__tests__/patients/view/ViewPatient.test.tsx
+++ b/src/__tests__/patients/view/ViewPatient.test.tsx
@@ -196,6 +196,16 @@ describe('ViewPatient', () => {
     })
   })
 
+  it('should render the allergies tab as active when route starts with /patients/:id/allergies', async () => {
+    setup({ startPath: `/patients/${testPatient.id}/allergies/nested-route` })
+
+    await waitFor(() => {
+      expect(screen.getByRole('button', { name: /patient\.allergies\.label/i })).toHaveClass(
+        'active',
+      )
+    })
+  })
+
   it('should mark the diagnoses tab as active when it is clicked and render the diagnoses component when route is /patients/:id/diagnoses', async () => {
     const { history } = setup()
 
@@ -228,6 +238,14 @@ describe('ViewPatient', () => {
     })
   })
 
+  it('should render the notes tab as active when route starts with /patients/:id/notes', async () => {
+    setup({ startPath: `/patients/${testPatient.id}/notes/nested-route` })
+
+    await waitFor(() => {
+      expect(screen.getByRole('button', { name: /patient\.notes\.label/i })).toHaveClass('active')
+    })
+  })
+
   it('should mark the medications tab as active when it is clicked and render the medication component when route is /patients/:id/medications', async () => {
     const { history } = setup()
 
@@ -278,6 +296,16 @@ describe('ViewPatient', () => {
     })
   })
 
+  it('should render the care plans tab as active when route starts with /patients/:id/care-plans', async () => {
+    setup({ startPath: `/patients/${testPatient.id}/care-plans/nested-route` })
+
+    await waitFor(() => {
+      expect(screen.getByRole('button', { name: /patient\.carePlan\.label/i })).toHaveClass(
+        'active',
+      )
+    })
+  })
+
   it('should mark the care goals tab as active when it is clicked and render the care goal tab component when route is /patients/:id/care-goals', async () => {
     const { history } = setup()
 
@@ -293,4 +321,38 @@ describe('ViewPatient', () => {
       expect(screen.getByText(/patient\.careGoals\.warning\.noCareGoals/i)).toBeInTheDocument()
     })
   })
+
+  it('should render the care goals tab as active when route starts with /patients/:id/care-goals', async () => {
+    setup({ startPath: `/patients/${testPatient.id}/care-goals/nested-route` })
+
+    await waitFor(() => {
+      expect(screen.getByRole('button', { name: /patient\.careGoal\.label/i })).toHaveClass(
+        'active',
+      )
+    })
+  })
+
+  it('should mark the visits tab as active when it is clicked and render the visit tab component when route is /patients/:id/visits', async () => {
+    const { history } = setup()
+
+    await waitFor(() => {
+      userEvent.click(screen.getByRole('button', { name: /patient\.visits\.label/i }))
+    })
+
+    await waitFor(() => {
+      expect(history.location.pathname).toEqual(`/patients/${testPatient.id}/visits`)
+    })
+    expect(screen.getByRole('button', { name: /patient\.visits\.label/i })).toHaveClass('active')
+    await waitFor(() => {
+      expect(screen.getByText(/patient\.visits\.warning\.noVisits/i)).toBeInTheDocument()
+    })
+  })
+
+  it('should render the visits tab as active when route starts with /patients/:id/visits', async () => {
+    setup({ startPath: `/patients/${testPatient.id}/visits/nested-route` })
+
+    await waitFor(() => {
+      expect(screen.getByRole('button', { name: /patient\.visits\.label/i })).toHaveClass('active')
+    })
+  })
 })
diff --git a/src/__tests__/patients/visits/VisitTable.test.tsx b/src/__tests__/patients/visits/VisitTable.test.tsx
index fb5c1644..2effd9c9 100644
--- a/src/__tests__/patients/visits/VisitTable.test.tsx
+++ b/src/__tests__/patients/visits/VisitTable.test.tsx
@@ -25,8 +25,8 @@ const patient = {
   visits: [visit],
 } as Patient
 
-const setup = () => {
-  jest.spyOn(PatientRepository, 'find').mockResolvedValue(patient)
+const setup = (expectedPatient = patient) => {
+  jest.spyOn(PatientRepository, 'find').mockResolvedValue(expectedPatient)
   const history = createMemoryHistory()
   history.push(`/patients/${patient.id}/visits/${patient.visits[0].id}`)
 
@@ -34,7 +34,7 @@ const setup = () => {
     history,
     ...render(
       <Router history={history}>
-        <VisitTable patientId={patient.id} />
+        <VisitTable patientId={expectedPatient.id} />
       </Router>,
     ),
   }
@@ -85,4 +85,13 @@ describe('Visit Table', () => {
 
     expect(history.location.pathname).toEqual(`/patients/${patient.id}/visits/${visit.id}`)
   })
+
+  it('should display a warning if there are no visits', async () => {
+    setup({ ...patient, visits: [] })
+    const alert = await screen.findByRole('alert')
+    expect(alert).toBeInTheDocument()
+    expect(alert).toHaveClass('alert-warning')
+    expect(screen.getByText(/patient.visits.warning.noVisits/i)).toBeInTheDocument()
+    expect(screen.getByText(/patient.visits.warning.addVisitAbove/i)).toBeInTheDocument()
+  })
 })