feat(patients): add no patients exist #2235

Fibii · 2020-07-13T21:51:14Z

Fixes #2231

Changes proposed in this pull request:

add "no patients exists" state when no patients exist in /patient as discussed in Implement "no patients" state on the /patients route #2231
add test for said changes
add few locales called noPatients for languages: eng/de/fr/ar/ru

jsf-clabot · 2020-07-13T21:51:17Z

All committers have signed the CLA.

gitpod-io · 2020-07-13T21:51:18Z

add missing useEffect dependency in ViewPatients.tsx

vercel · 2020-07-13T22:47:50Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/hospitalrun/hospitalrun-frontend/rdymsc2nf
✅ Preview: https://hospitalrun-fr-git-fork-fibii-featpatients-implement-nop-eaebe0.hospitalrun.vercel.app

Fibii · 2020-07-14T13:32:11Z

so for some reason i added a patient to the vercel build to check if the "add new patient" shows up when patients exist (even tho the tests passed) and i couldn't find a way to remove the patient, which means my commit won't be displayed (because there's a patient) so whomever reviews the code in vercel have to remove that test patient.

jackcmeyer · 2020-07-15T00:07:56Z

src/patients/list/ViewPatients.tsx

+      <div style={{ display: 'flex', justifyContent: 'center' }}>
+        <div style={{ display: 'flex', flexDirection: 'column' }}>
+          <div style={{ display: 'flex', justifyContent: 'center' }}>
+            <Icon icon="patient" outline={false} size="6x" />


lets use the patients icon rather than patient.

jackcmeyer · 2020-07-15T00:09:37Z

src/patients/list/ViewPatients.tsx

+              outlined
+              color="success"


rather than using a success and outlined button, I think a primary button makes more sense here since it is the call to action on the page.

jackcmeyer · 2020-07-15T00:13:10Z

src/patients/list/ViewPatients.tsx

-      </Button>,
-    ])
-
+    if (patients && patients.length > 0) {


this patients variable represents the current patients in state.

This could be the default page of patients or a search result. When searching for patients and my search results return 0, I would not expect the new icon to appear.

I think that we should implement a new function called count in Repository that returns the number of documents that are available for a type.

Then here, we use that count function to determine if the new icon should display or not.

i get it, what i don't get is why do we need to implement count?

it seems like PatientRepository.findAll() returns all the existing patients in the db, we can use it as:

implement a state called count

at initial load, setCount(PatientRepository.findAll().length)

if (count == 0) show addNewPations component, else show the table

but it seems that implementing a count in Repository seems like a better solution here, since it makes Repository more flexible for future issues/enhancements

jackcmeyer · 2020-07-15T00:14:14Z

src/patients/list/ViewPatients.tsx

+    return (
+      <div style={{ display: 'flex', justifyContent: 'center' }}>
+        <div style={{ display: 'flex', flexDirection: 'column' }}>
+          <div style={{ display: 'flex', justifyContent: 'center' }}>
+            <Icon icon="patient" outline={false} size="6x" />
+          </div>
+          <div style={{ display: 'flex', justifyContent: 'center' }}>
+            <div style={{ textAlign: 'center', width: '60%', margin: 16 }}>
+              <Typography variant="h5">{t('patients.noPatients')}</Typography>
+            </div>
+          </div>
+          <div style={{ display: 'flex', justifyContent: 'center' }}>
+            <Button
+              key="newPatientButton"
+              outlined
+              color="success"
+              icon="patient-add"
+              onClick={() => history.push('/patients/new')}
+            >
+              {t('patients.newPatient')}
+            </Button>
+          </div>
+        </div>
+      </div>


I think that we should move this to its own component to:

keep components smaller and easier to understand

make it easier to test

jackcmeyer · 2020-07-21T22:39:07Z

@Fibii please let me know if you have any questions or need help addressing the code review feedback.

Fibii · 2020-07-22T19:00:47Z

@jackcmeyer how would i add count to patient-slice, should i even do that ?

i already added count to Repository as

async count(): Promise<number> {
    const result = await this.findAll()
    return result.length
  }

what i want to do now is use it in ViewPatients.tsx as

  const { patients, isLoading, count } = useSelector((state: RootState) => state.patients)

but i couldn't figure it out. I updated the interface in patients-slice.ts to be

interface PatientsState {
  isLoading: boolean
  patients: Patient[]
  count: number
}

and called Repository#count in fetchPatients

export const fetchPatients = (sortRequest: SortRequest = Unsorted): AppThunk => async (
  dispatch,
) => {
  dispatch(fetchPatientsStart())
  const patients = await PatientRepository.findAll(sortRequest)
  const count = await PatientRepository.count()
  // dispatch(fetchPatientsSuccess(... ? ))
}

but i couldn't figure what to dispatch there. I need a little bit of help here.

matteovivona · 2020-07-24T07:25:29Z

@fox1t some advice?

jackcmeyer

Few more code related comments.

One other thing:
The AddNewPatient component should not display because a search result has no results.

It should show up when there are no patients in the database.

jackcmeyer · 2020-07-28T03:33:29Z

src/__tests__/patients/list/ViewPatients.test.tsx

+    it('should render no patients exists when no patients exist', () => {
+      const wrapper = setup(false, [], 0)
+
+      const addNewPatient = wrapper.find(AddNewPatient)
+      expect(addNewPatient).toHaveLength(1)
+
+      const icon = wrapper.find(Icon).first()
+      const typography = wrapper.find(Typography)
+      const button = wrapper.find(Button)
+      const iconType = icon.prop('icon')
+      const iconSize = icon.prop('size')
+      const typographyText = typography.prop('children')
+      const typographyVariant = typography.prop('variant')
+      const buttonIcon = button.prop('icon')
+      const buttonText = button.prop('children')
+
+      expect(iconType).toEqual('patients')
+      expect(iconSize).toEqual('6x')
+      expect(typographyText).toEqual('patients.noPatients')
+      expect(typographyVariant).toEqual('h5')
+      expect(buttonIcon).toEqual('patient-add')
+      expect(buttonText).toEqual('patients.newPatient')
+    })


This test is testing behavior inside the AddNewPatient component. I think instead, this test should test that the AddNewPatient component gets rendered and then this current test should be moved to an AddNewPatient test.

jackcmeyer · 2020-07-28T03:34:28Z

src/patients/view/AddNewPatient.tsx

+
+import useTranslator from '../../shared/hooks/useTranslator'
+
+const AddNewPatient = () => {


Thoughts on NoPatientsExist for this component name? AddNewPatient could potentially be confusing with NewPatient component.

feat(patients): add no patients exist

9ada8f0

add missing useEffect dependency

0b5701c

add missing useEffect dependency in ViewPatients.tsx

vercel bot deployed to Preview July 13, 2020 22:47 View deployment

matteovivona requested a review from jackcmeyer July 14, 2020 05:46

matteovivona added the in progress indicates that issue/pull request is currently being worked on label Jul 14, 2020

matteovivona added this to In progress in Version 2.0 via automation Jul 14, 2020

matteovivona added this to the v2.0 milestone Jul 14, 2020

jackcmeyer suggested changes Jul 15, 2020

View reviewed changes

Merge branch 'master' into feat(patients)-implement-noPatients

86ca0f9

Merge branch 'master' into feat(patients)-implement-noPatients

a65f44c

vercel bot deployed to Preview July 24, 2020 07:25 View deployment

feat(patients): add proposed changes in PR #2235

09a9066

Fibii requested a review from jackcmeyer July 25, 2020 01:47

vercel bot deployed to Preview July 26, 2020 04:33 View deployment

jackcmeyer approved these changes Jul 28, 2020

View reviewed changes

jackcmeyer suggested changes Jul 28, 2020

View reviewed changes

Merge branch 'master' into feat(patients)-implement-noPatients

69e689b

vercel bot deployed to Preview July 28, 2020 09:03 View deployment

matteovivona added the 🚀enhancement an issue/pull request that adds a feature to the application label Jul 30, 2020

feat(patients): add proposed changes in PR

3aff81e

Fibii requested a review from jackcmeyer July 31, 2020 11:43

Merge branch 'master' into feat(patients)-implement-noPatients

fbd94b4

vercel bot deployed to Preview July 31, 2020 12:50 View deployment

jackcmeyer approved these changes Aug 5, 2020

View reviewed changes

jackcmeyer merged commit 886163a into HospitalRun:master Aug 5, 2020

Version 2.0 automation moved this from In progress to Done Aug 5, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(patients): add no patients exist #2235

feat(patients): add no patients exist #2235

Fibii commented Jul 13, 2020

jsf-clabot commented Jul 13, 2020 •

edited

gitpod-io bot commented Jul 13, 2020 •

edited

vercel bot commented Jul 13, 2020 •

edited

Fibii commented Jul 14, 2020

jackcmeyer Jul 15, 2020

jackcmeyer Jul 15, 2020

jackcmeyer Jul 15, 2020

Fibii Jul 22, 2020

jackcmeyer Jul 15, 2020

jackcmeyer commented Jul 21, 2020

Fibii commented Jul 22, 2020

matteovivona commented Jul 24, 2020

jackcmeyer left a comment

jackcmeyer Jul 28, 2020

jackcmeyer Jul 28, 2020


		import useTranslator from '../../shared/hooks/useTranslator'

		const AddNewPatient = () => {

feat(patients): add no patients exist #2235

feat(patients): add no patients exist #2235

Conversation

Fibii commented Jul 13, 2020

jsf-clabot commented Jul 13, 2020 • edited

gitpod-io bot commented Jul 13, 2020 • edited

vercel bot commented Jul 13, 2020 • edited

Fibii commented Jul 14, 2020

jackcmeyer Jul 15, 2020

Choose a reason for hiding this comment

jackcmeyer Jul 15, 2020

Choose a reason for hiding this comment

jackcmeyer Jul 15, 2020

Choose a reason for hiding this comment

Fibii Jul 22, 2020

Choose a reason for hiding this comment

jackcmeyer Jul 15, 2020

Choose a reason for hiding this comment

jackcmeyer commented Jul 21, 2020

Fibii commented Jul 22, 2020

matteovivona commented Jul 24, 2020

jackcmeyer left a comment

Choose a reason for hiding this comment

jackcmeyer Jul 28, 2020

Choose a reason for hiding this comment

jackcmeyer Jul 28, 2020

Choose a reason for hiding this comment

jsf-clabot commented Jul 13, 2020 •

edited

gitpod-io bot commented Jul 13, 2020 •

edited

vercel bot commented Jul 13, 2020 •

edited