feat(new): warn about potential duplicate patient (#2187)

[janmarkusmilan]

Fixes #2187

Changes proposed in this pull request:

• A modal appears when a patient with matching name, sex and date of birth is created
• The modal outputs the message "Potential duplicate of: [Patient Name](link to patient record). Are you sure you want to create this patient?"
• Clicking on the patient name will redirect to the patient view
• Clicking on the cancel button will close the modal
• Clicking on the continue button will dispatch create patient, redirect to the new patient view and output a success message
• Added a new test file for the modal

No new dependency added

[jsf-clabot]

All committers have signed the CLA.

[gitpod-io]

[vercel]

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/dqbxrat2q
✅ Preview: https://hospitalrun-fro-git-fork-janmarkusmilan-duplicate-patien-bed13f.hospitalrun.vercel.app

[jackcmeyer]

I think that we should extra this to a helper util file called is-possible-duplicate-patient.ts. The file can then export one function isPossibleDuplicatePatient(newPatient, existingPatient).

With this, we can then unit test that capability independently, plus reuse if necessary.

[jackcmeyer]

I think maybe this could go in a warning alert to bring more attention to the warning we are providing.

[jackcmeyer]

All text should be provided through the internationalization process.

[janmarkusmilan]

Just for clarification, do you mean using the translator? For example:

<Alert color="danger" message={t('addTheTextToBeInternationalizedHere')} />

[jackcmeyer]

Needs to be internationalized.

[jackcmeyer]

I think it makes more sense to represent this state as

useState<Patient | undefiend>(undefined). I think more clearly represents the presence/absence of a duplicate patient.

[jackcmeyer]

This line can be simplified to:

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

[janmarkusmilan]

I originally had that line, but the tests would always fail. The message is:

TypeError: Cannot destructure property 'patients' of '(0 , _reactRedux.useSelector)(...)' as it is undefined.

The only solution I found was adding the Object(...) at the beginning. I can't seem to get it working without it.

[jackcmeyer]

After thinking about this functionality some more and seeing it action. I think it make sense to support possibly finding multiple duplicates?

If there were already two other patients named "Test User" born on 7/16/2000 and are male. I think all of the should appear in the warning.

[jackcmeyer]

After adding support for finding all possible duplicate patients, rather than just the first, this function would be a good candidate for Arrays.filter https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/filter.

const onSave = () => {
	 const duplicatePatients = patients.filter((existingPatient) => isPossibleDuplicatePatient(patient, existingPatient));
	
	if(duplicatePatients.length > 0) {
       setShowDuplicateNewPatientModal(true)
       setDuplicatePatient(existingPatient)
	} else {
      dispatch(createPatient(patient, onSuccessfulSave))
	}
}

[jackcmeyer]

There should be some tests to test this new workflow.

Basically, have an already existing patient in the redux store, try to create a new patient with the same information, and test to make sure that the new modal shows.

[jackcmeyer]

This code block is duplicated several times in this file. I think we can extract it to a setup function that looks like:

const setup = () => {
 const onContinueButtonClickSpy = jest.fn()
      const store = mockStore({
        patient: {
          patient: {
            id: '1234',
          },
        },
      } as any)

      const wrapper = mount(
        <Provider store={store}>
          <DuplicateNewPatientModal
            show
            toggle={jest.fn()}
            onCloseButtonClick={jest.fn()}
            onContinueButtonClick={onContinueButtonClickSpy}
          />
        </Provider>,
      )
	wrapper.update()

   return { wrapper: wrapper as ReactWrapper }
}

[gitpod-io]

[janmarkusmilan]

Sorry, I accidentally requested a merge to master (did not make the changes yet). I am making the changes now.

[janmarkusmilan]

Recently Pushed Requested Changes

1. Helper util file (is-possible-duplicate-patient.ts)

Created helper util file to allow for reusability if necessary.

---

2. Support for finding multiple duplicate patients

NewPatient.tsx

DuplicateNewPatientModal.tsx

Used the Arrays.filter method and the newly created helper util file is-possible-duplicate-patient.ts to loop through the existing patients and determine if there are zero, one or multiple patients with the same information as the one being created. Also styled the look of the modal based on if there is one or multiple patients.

---

3. New alert and internationalized text

DuplicateNewPatientModal.tsx

Added an alert to bring more attention to the warning being provided. Also, all the text is internationalized (still missing english translation key in console).

Modal (Multiple Patients)

Modal (Single Patient)

---

4. Setup function in test file

DuplicateNewPatientModal.test.tsx

Using the setup function

Since this code block was duplicated multiple times, it was extracted to a setup function for easy reusability.

---

5. Test case for new modal

NewPatient.test.tsx

NewPatient.tsx

A test needed to be created in order to test the new modal feature. Given some existing patient in the redux store, if a newly created patient had the same information as the existing one, then the modal should render (or in this case return true).

---

Other Things

1. Changed const [duplicatePatient, setDuplicatePatient] = useState({} as Patient) to const [duplicatePatient, setDuplicatePatient] = useState<Patient | undefined>(undefined)
2. Had to keep const { patients } = Object(useSelector((state: RootState) => state.patients)) as it is, since removing the Object(...) results in errors when running the test cases for NewPatient.tsx

[jackcmeyer]

Few comments on the code! I really like the new functionality for the multiple duplicate patients!

[jackcmeyer]

this alert message needs to have the string internationalized.

[jackcmeyer]

The value passed to be a key that is defined in the locales folder.

[jackcmeyer]

Thoughts on having this as a warning color rather than danger?

[janmarkusmilan]

I think it makes sense. It's more of a warning message than an error message, and it still stands out.

[jackcmeyer]

as trivial as it is, we should have a test around this functionality.

[jackcmeyer]

I would expect a function named isPossibleDuplicatePatient to return a boolean rather than a object.

I think this function can be simplified as:

export function isPossibleDuplicatePatient(newPatient: Patient, existingPatient: Patient) {
	return newPatient.givenName === existingPatient.givenName &&
    newPatient.familyName === existingPatient.familyName &&
    newPatient.sex === existingPatient.sex &&
    newPatient.dateOfBirth === existingPatient.dateOfBirth
}

[jackcmeyer]

I think that it is ok if it always displays a list no matter the number of duplicate patients returned. If we do that, then we can simplify this code.

[janmarkusmilan]

Newly Pushed Changes

1. Changed alert color and internationalized text

I changed the color of the alert to "warning" instead of danger, and internationalized the text by adding them in the locales folder.

---

2. Changed is-possible-duplicate-patient.ts to return a boolean instead of an object, and created test file

I simplified the util file to return a boolean instead of an object, and created a test file for the function.

---

3. Set modal to display a list regardless of the amount of duplicate patients

I simplified the code by always rendering a list of duplicate patients in the modal.

---