-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat(new): warn about potential duplicate patient (#2187) #2240
Changes from 1 commit
4a800af
c340f7a
2f5db58
9c601fb
13ff647
5c9ebff
13954ae
9d33ea8
e56a6f0
8ad58a0
8e503da
73fb4c3
58b069c
732bdb4
97a5dac
21a3f94
d09c90d
924cf9d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
import { Modal } from '@hospitalrun/components' | ||
import { act } from '@testing-library/react' | ||
import { mount } from 'enzyme' | ||
import React from 'react' | ||
import { Provider } from 'react-redux' | ||
import createMockStore from 'redux-mock-store' | ||
import thunk from 'redux-thunk' | ||
|
||
import DuplicateNewPatientModal from '../../../patients/new/DuplicateNewPatientModal' | ||
import { RootState } from '../../../shared/store' | ||
|
||
const mockStore = createMockStore<RootState, any>([thunk]) | ||
|
||
describe('Duplicate New Patient Modal', () => { | ||
it('should render a modal with the correct labels', () => { | ||
const store = mockStore({ | ||
patient: { | ||
patient: { | ||
id: '1234', | ||
}, | ||
}, | ||
} as any) | ||
const wrapper = mount( | ||
<Provider store={store}> | ||
<DuplicateNewPatientModal | ||
show | ||
toggle={jest.fn()} | ||
onCloseButtonClick={jest.fn()} | ||
onContinueButtonClick={jest.fn()} | ||
/> | ||
</Provider>, | ||
) | ||
wrapper.update() | ||
const modal = wrapper.find(Modal) | ||
expect(modal).toHaveLength(1) | ||
expect(modal.prop('title')).toEqual('Warning') | ||
expect(modal.prop('closeButton')?.children).toEqual('actions.cancel') | ||
expect(modal.prop('closeButton')?.color).toEqual('danger') | ||
expect(modal.prop('successButton')?.children).toEqual('Continue') | ||
expect(modal.prop('successButton')?.color).toEqual('success') | ||
}) | ||
|
||
describe('cancel', () => { | ||
it('should call the onCloseButtonClick function when the close button is clicked', () => { | ||
const onCloseButtonClickSpy = jest.fn() | ||
const store = mockStore({ | ||
patient: { | ||
patient: { | ||
id: '1234', | ||
}, | ||
}, | ||
} as any) | ||
const wrapper = mount( | ||
<Provider store={store}> | ||
<DuplicateNewPatientModal | ||
show | ||
toggle={jest.fn()} | ||
onCloseButtonClick={onCloseButtonClickSpy} | ||
onContinueButtonClick={jest.fn()} | ||
/> | ||
</Provider>, | ||
) | ||
wrapper.update() | ||
|
||
act(() => { | ||
const modal = wrapper.find(Modal) | ||
const { onClick } = modal.prop('closeButton') as any | ||
onClick() | ||
}) | ||
|
||
expect(onCloseButtonClickSpy).toHaveBeenCalledTimes(1) | ||
}) | ||
}) | ||
|
||
describe('on save', () => { | ||
it('should call the onContinueButtonClick function when the continue button is clicked', () => { | ||
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() | ||
|
||
act(() => { | ||
const modal = wrapper.find(Modal) | ||
const { onClick } = modal.prop('successButton') as any | ||
onClick() | ||
}) | ||
|
||
expect(onContinueButtonClickSpy).toHaveBeenCalledTimes(1) | ||
}) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
import { Modal } from '@hospitalrun/components' | ||
import React from 'react' | ||
import { Link } from 'react-router-dom' | ||
|
||
import useTranslator from '../../shared/hooks/useTranslator' | ||
import Patient from '../../shared/model/Patient' | ||
|
||
interface Props { | ||
duplicatePatient?: Patient | ||
show: boolean | ||
toggle: () => void | ||
onCloseButtonClick: () => void | ||
onContinueButtonClick: () => void | ||
} | ||
|
||
const DuplicateNewPatientModal = (props: Props) => { | ||
const { t } = useTranslator() | ||
const { duplicatePatient, show, toggle, onCloseButtonClick, onContinueButtonClick } = props | ||
|
||
const body = ( | ||
<div className="row"> | ||
<div className="col-md-12"> | ||
<p> | ||
Possible duplicate of:{' '} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needs to be internationalized. |
||
{duplicatePatient !== undefined && ( | ||
<Link to={`/patients/${duplicatePatient.id}`}>{duplicatePatient.fullName}</Link> | ||
)} | ||
</p> | ||
<p>Are you sure you want to create this patient?</p> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think maybe this could go in a warning alert to bring more attention to the warning we are providing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All text should be provided through the internationalization process. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just for clarification, do you mean using the translator? For example:
|
||
</div> | ||
</div> | ||
) | ||
|
||
return ( | ||
<Modal | ||
show={show} | ||
toggle={toggle} | ||
title={t('Warning')} | ||
body={body} | ||
closeButton={{ | ||
children: t('actions.cancel'), | ||
color: 'danger', | ||
onClick: onCloseButtonClick, | ||
}} | ||
successButton={{ | ||
children: t('Continue'), | ||
color: 'success', | ||
onClick: onContinueButtonClick, | ||
}} | ||
/> | ||
) | ||
} | ||
|
||
export default DuplicateNewPatientModal |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ import Patient from '../../shared/model/Patient' | |
import { RootState } from '../../shared/store' | ||
import GeneralInformation from '../GeneralInformation' | ||
import { createPatient } from '../patient-slice' | ||
import DuplicateNewPatientModal from './DuplicateNewPatientModal' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
||
const breadcrumbs = [ | ||
{ i18nKey: 'patients.label', location: '/patients' }, | ||
|
@@ -21,8 +22,11 @@ const NewPatient = () => { | |
const history = useHistory() | ||
const dispatch = useDispatch() | ||
const { createError } = useSelector((state: RootState) => state.patient) | ||
const { patients } = Object(useSelector((state: RootState) => state.patients)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line can be simplified to:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I originally had that line, but the tests would always fail. The message is:
The only solution I found was adding the Object(...) at the beginning. I can't seem to get it working without it. |
||
|
||
const [patient, setPatient] = useState({} as Patient) | ||
const [duplicatePatient, setDuplicatePatient] = useState({} as Patient) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it makes more sense to represent this state as
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
const [showDuplicateNewPatientModal, setShowDuplicateNewPatientModal] = useState<boolean>(false) | ||
|
||
useTitle(t('patients.newPatient')) | ||
useAddBreadcrumbs(breadcrumbs, true) | ||
|
@@ -41,13 +45,39 @@ const NewPatient = () => { | |
} | ||
|
||
const onSave = () => { | ||
dispatch(createPatient(patient, onSuccessfulSave)) | ||
let isDuplicatePatient = false | ||
const patientsObj = {} | ||
Object.assign(patientsObj, patients) | ||
Object.keys(patientsObj).forEach((patientInfo: any) => { | ||
const loggedPatient = patients[patientInfo] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the variable name |
||
if ( | ||
loggedPatient.givenName === patient.givenName && | ||
loggedPatient.familyName === patient.familyName && | ||
loggedPatient.sex === patient.sex && | ||
loggedPatient.dateOfBirth === patient.dateOfBirth | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that we should extra this to a helper util file called With this, we can then unit test that capability independently, plus reuse if necessary. |
||
) { | ||
setShowDuplicateNewPatientModal(true) | ||
setDuplicatePatient(loggedPatient as Patient) | ||
isDuplicatePatient = true | ||
} | ||
}) | ||
if (!isDuplicatePatient) { | ||
dispatch(createPatient(patient, onSuccessfulSave)) | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After adding support for finding all possible duplicate patients, rather than just the first, this function would be a good candidate for const onSave = () => {
const duplicatePatients = patients.filter((existingPatient) => isPossibleDuplicatePatient(patient, existingPatient));
if(duplicatePatients.length > 0) {
setShowDuplicateNewPatientModal(true)
setDuplicatePatient(existingPatient)
} else {
dispatch(createPatient(patient, onSuccessfulSave))
}
} |
||
|
||
const onPatientChange = (newPatient: Partial<Patient>) => { | ||
setPatient(newPatient as Patient) | ||
} | ||
|
||
const createDuplicateNewPatient = () => { | ||
dispatch(createPatient(patient, onSuccessfulSave)) | ||
} | ||
|
||
const closeDuplicateNewPatientModal = () => { | ||
setShowDuplicateNewPatientModal(false) | ||
} | ||
|
||
return ( | ||
<div> | ||
<GeneralInformation | ||
|
@@ -66,6 +96,14 @@ const NewPatient = () => { | |
</Button> | ||
</div> | ||
</div> | ||
|
||
<DuplicateNewPatientModal | ||
duplicatePatient={duplicatePatient} | ||
show={showDuplicateNewPatientModal} | ||
toggle={closeDuplicateNewPatientModal} | ||
onContinueButtonClick={createDuplicateNewPatient} | ||
onCloseButtonClick={closeDuplicateNewPatientModal} | ||
/> | ||
</div> | ||
) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code block is duplicated several times in this file. I think we can extract it to a setup function that looks like: