-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat(new): warn about potential duplicate patient (#2187) #2240
Changes from 10 commits
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,90 @@ | ||
import { Modal } from '@hospitalrun/components' | ||
import { act } from '@testing-library/react' | ||
import { mount, ReactWrapper } 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]) | ||
|
||
const setupOnClick = (onClose: any, onContinue: any, prop: string) => { | ||
const store = mockStore({ | ||
patient: { | ||
patient: { | ||
id: '1234', | ||
}, | ||
}, | ||
} as any) | ||
|
||
const wrapper = mount( | ||
<Provider store={store}> | ||
<DuplicateNewPatientModal | ||
show | ||
toggle={jest.fn()} | ||
onCloseButtonClick={onClose} | ||
onContinueButtonClick={onContinue} | ||
/> | ||
</Provider>, | ||
) | ||
wrapper.update() | ||
|
||
act(() => { | ||
const modal = wrapper.find(Modal) | ||
const { onClick } = modal.prop(prop) as any | ||
onClick() | ||
}) | ||
|
||
return { wrapper: wrapper as ReactWrapper } | ||
} | ||
|
||
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('patients.newPatient') | ||
expect(modal.prop('closeButton')?.children).toEqual('actions.cancel') | ||
expect(modal.prop('closeButton')?.color).toEqual('danger') | ||
expect(modal.prop('successButton')?.children).toEqual('actions.save') | ||
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 closeButtonProp = 'closeButton' | ||
setupOnClick(onCloseButtonClickSpy, jest.fn(), closeButtonProp) | ||
expect(onCloseButtonClickSpy).toHaveBeenCalledTimes(1) | ||
}) | ||
}) | ||
|
||
describe('on save', () => { | ||
it('should call the onContinueButtonClick function when the continue button is clicked', () => { | ||
const onContinueButtonClickSpy = jest.fn() | ||
const continueButtonProp = 'successButton' | ||
setupOnClick(jest.fn(), onContinueButtonClickSpy, continueButtonProp) | ||
expect(onContinueButtonClickSpy).toHaveBeenCalledTimes(1) | ||
}) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
import { Modal, Alert } 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 alertMessage = | ||
'Patient with matching information found in database. Are you sure you want to create this patient?' | ||
|
||
const body = ( | ||
<> | ||
<Alert color="danger" title={t('Warning!')} message={t(alertMessage)} /> | ||
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. Thoughts on having this as a warning color rather than danger? 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 sense. It's more of a warning message than an error message, and it still stands out. |
||
<div className="row"> | ||
<div className="col-md-12"> | ||
{`${t('Possible duplicate 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. The value passed to be a key that is defined in the |
||
{duplicatePatient !== undefined && | ||
Object.entries(duplicatePatient).length === 1 && | ||
Object.entries(duplicatePatient).map(([key, patient]) => ( | ||
<Link key={key.toString()} to={`/patients/${patient.id}`}> | ||
{patient.fullName} | ||
</Link> | ||
))} | ||
{duplicatePatient !== undefined && | ||
Object.entries(duplicatePatient).length > 1 && | ||
Object.entries(duplicatePatient).map(([key, patient]) => ( | ||
<li key={key.toString()}> | ||
<Link to={`/patients/${patient.id}`}>{patient.fullName}</Link> | ||
</li> | ||
))} | ||
</div> | ||
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 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. |
||
</div> | ||
</> | ||
) | ||
|
||
return ( | ||
<Modal | ||
show={show} | ||
toggle={toggle} | ||
title={t('patients.newPatient')} | ||
body={body} | ||
closeButton={{ | ||
children: t('actions.cancel'), | ||
color: 'danger', | ||
onClick: onCloseButtonClick, | ||
}} | ||
successButton={{ | ||
children: t('actions.save'), | ||
color: 'success', | ||
onClick: onContinueButtonClick, | ||
}} | ||
/> | ||
) | ||
} | ||
|
||
export default DuplicateNewPatientModal |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,8 @@ import Patient from '../../shared/model/Patient' | |
import { RootState } from '../../shared/store' | ||
import GeneralInformation from '../GeneralInformation' | ||
import { createPatient } from '../patient-slice' | ||
import { isPossibleDuplicatePatient } from '../util/is-possible-duplicate-patient' | ||
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 +23,18 @@ 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<Patient | undefined>(undefined) | ||
const [showDuplicateNewPatientModal, setShowDuplicateNewPatientModal] = useState<boolean>(false) | ||
|
||
const testPatient = { | ||
givenName: 'Kelly', | ||
familyName: 'Clark', | ||
sex: 'female', | ||
dateOfBirth: '1963-01-09T05:00:00.000Z', | ||
} as Patient | ||
|
||
useTitle(t('patients.newPatient')) | ||
useAddBreadcrumbs(breadcrumbs, true) | ||
|
@@ -41,13 +53,39 @@ const NewPatient = () => { | |
} | ||
|
||
const onSave = () => { | ||
dispatch(createPatient(patient, onSuccessfulSave)) | ||
let duplicatePatients = [] | ||
if (patients !== undefined) { | ||
duplicatePatients = patients.filter((existingPatient: any) => | ||
isPossibleDuplicatePatient(patient, existingPatient), | ||
) | ||
} | ||
|
||
if (duplicatePatients.length > 0) { | ||
setShowDuplicateNewPatientModal(true) | ||
setDuplicatePatient(duplicatePatients as Patient) | ||
} else { | ||
dispatch(createPatient(patient, onSuccessfulSave)) | ||
} | ||
|
||
const testCase = [isPossibleDuplicatePatient(patient, testPatient)] | ||
if (testCase.length > 0) { | ||
return true | ||
} | ||
return false | ||
} | ||
|
||
const onPatientChange = (newPatient: Partial<Patient>) => { | ||
setPatient(newPatient as Patient) | ||
} | ||
|
||
const createDuplicateNewPatient = () => { | ||
dispatch(createPatient(patient, onSuccessfulSave)) | ||
} | ||
|
||
const closeDuplicateNewPatientModal = () => { | ||
setShowDuplicateNewPatientModal(false) | ||
} | ||
|
||
return ( | ||
<div> | ||
<GeneralInformation | ||
|
@@ -66,6 +104,14 @@ const NewPatient = () => { | |
</Button> | ||
</div> | ||
</div> | ||
|
||
<DuplicateNewPatientModal | ||
duplicatePatient={duplicatePatient} | ||
show={showDuplicateNewPatientModal} | ||
toggle={closeDuplicateNewPatientModal} | ||
onContinueButtonClick={createDuplicateNewPatient} | ||
onCloseButtonClick={closeDuplicateNewPatientModal} | ||
/> | ||
</div> | ||
) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import Patient from '../../shared/model/Patient' | ||
|
||
export function isPossibleDuplicatePatient(newPatient: Patient, existingPatient: 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. as trivial as it is, we should have a test around this functionality. 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 would expect a function named 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
} |
||
if ( | ||
newPatient.givenName === existingPatient.givenName && | ||
newPatient.familyName === existingPatient.familyName && | ||
newPatient.sex === existingPatient.sex && | ||
newPatient.dateOfBirth === existingPatient.dateOfBirth | ||
) { | ||
return newPatient | ||
} | ||
return null | ||
} |
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 alert message needs to have the string internationalized.