-
Notifications
You must be signed in to change notification settings - Fork 39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(feat)O3-4206-Add ability to view patient diagnoses and active conditions #123
base: main
Are you sure you want to change the base?
Changes from 2 commits
01c7fec
1b50388
5815f00
1d930fc
0e92207
982dbc1
bf5c2e6
9c2e293
68876e6
e0a915c
1f3419d
637242d
54e491e
0ed2af6
bdfad34
7c88475
954e710
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,23 @@ | ||
import React from 'react'; | ||
import { Layer } from '@carbon/react'; | ||
import { Report } from '@carbon/react/icons'; | ||
import styles from './empty-state.scss'; | ||
|
||
type EmptyState = { | ||
title: string; | ||
message: string; | ||
}; | ||
|
||
const EmptyState: React.FC<EmptyState> = ({ title, message }) => { | ||
return ( | ||
<Layer className={styles.container}> | ||
<p className={styles.title}>{title}</p> | ||
<div className={styles.messageContainer}> | ||
<Report className={styles.emptyIcon} size={50} /> | ||
<p className={styles.message}>{message}</p> | ||
</div> | ||
</Layer> | ||
); | ||
}; | ||
|
||
export default EmptyState; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
@use '@carbon/layout'; | ||
@use '@carbon/type'; | ||
@use '@carbon/colors'; | ||
|
||
.container { | ||
padding: layout.$spacing-03; | ||
background-color: colors.$white; | ||
margin: layout.$spacing-03 0 0 0; | ||
} | ||
|
||
.title { | ||
@include type.type-style('heading-01'); | ||
display: flex; | ||
align-items: center; | ||
justify-content: space-between; | ||
row-gap: 1.5rem; | ||
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. Please use Carbon spacing tokens for spacing in SCSS |
||
position: relative; | ||
color: colors.$gray-50; | ||
|
||
&::after { | ||
content: ''; | ||
display: block; | ||
width: 2rem; | ||
border-bottom: 0.375rem solid var(--brand-03); | ||
position: absolute; | ||
bottom: -0.75rem; | ||
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 looks like a hack. Why do we need this here? |
||
left: 0; | ||
} | ||
|
||
& > span { | ||
@include type.type-style('body-01'); | ||
} | ||
} | ||
|
||
.message { | ||
color: colors.$gray-50; | ||
} | ||
|
||
.messageContainer { | ||
display: flex; | ||
flex-grow: 1; | ||
flex-direction: column; | ||
justify-content: center; | ||
align-items: center; | ||
gap: layout.$spacing-03; | ||
} | ||
|
||
.emptyIcon { | ||
color: colors.$gray-50; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
import React from 'react'; | ||
import { | ||
StructuredListWrapper, | ||
StructuredListHead, | ||
StructuredListRow, | ||
StructuredListCell, | ||
StructuredListBody, | ||
Layer, | ||
} from '@carbon/react'; | ||
import styles from './conditions.scss'; | ||
import { useTranslation } from 'react-i18next'; | ||
import { usePatientConditions } from './conditions.resource'; | ||
import { formatDate, parseDate } from '@openmrs/esm-framework'; | ||
import EmptyState from '../components/empty-state.component'; | ||
|
||
type PatientConditionsProps = { | ||
patientUuid: string; | ||
encounterUuid: string; | ||
}; | ||
|
||
const PatientConditions: React.FC<PatientConditionsProps> = ({ encounterUuid, patientUuid }) => { | ||
const { t } = useTranslation(); | ||
const { conditions, error, isLoading, mutate, showPatientConditions } = usePatientConditions(patientUuid); | ||
|
||
if (!showPatientConditions) return null; | ||
|
||
if (!conditions.length) | ||
return ( | ||
<EmptyState title={t('conditions', 'Conditions')} message={t('noConditions', 'No conditions for this patient')} /> | ||
); | ||
|
||
return ( | ||
<Layer className={styles.conditionsContainer}> | ||
<StructuredListWrapper> | ||
<StructuredListHead> | ||
<StructuredListRow head> | ||
<StructuredListCell head> | ||
{t('activeConditions', 'Active Conditions')} | ||
{`(${conditions.length})`} | ||
</StructuredListCell> | ||
<StructuredListCell head>{t('onSetDate', 'Onset Date')}</StructuredListCell> | ||
</StructuredListRow> | ||
</StructuredListHead> | ||
<StructuredListBody> | ||
{conditions.map(({ display, onsetDateTime, status }) => ( | ||
<StructuredListRow> | ||
<StructuredListCell noWrap>{display}</StructuredListCell> | ||
<StructuredListCell>{formatDate(parseDate(onsetDateTime))}</StructuredListCell> | ||
</StructuredListRow> | ||
))} | ||
</StructuredListBody> | ||
</StructuredListWrapper> | ||
</Layer> | ||
); | ||
}; | ||
|
||
export default PatientConditions; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,133 @@ | ||
import { type FetchResponse, fhirBaseUrl, openmrsFetch, useConfig } from '@openmrs/esm-framework'; | ||
import useSWR from 'swr'; | ||
import { type PharmacyConfig } from '../config-schema'; | ||
import { useMemo } from 'react'; | ||
|
||
export interface ConditionsBundle { | ||
resourceType: string; | ||
id: string; | ||
meta: Meta; | ||
type: string; | ||
total: number; | ||
link: Array<Link>; | ||
entry: Array<Entry>; | ||
} | ||
|
||
export interface Meta { | ||
lastUpdated: string; | ||
tag: Array<Tag>; | ||
} | ||
|
||
export interface Tag { | ||
system: string; | ||
code: string; | ||
display: string; | ||
} | ||
|
||
export interface Link { | ||
relation: string; | ||
url: string; | ||
} | ||
|
||
export interface Entry { | ||
fullUrl: string; | ||
resource: Resource; | ||
} | ||
|
||
export interface Resource { | ||
resourceType: string; | ||
id: string; | ||
meta: ResourceMeta; | ||
clinicalStatus: ClinicalStatus; | ||
code: Code; | ||
subject: Subject; | ||
onsetDateTime: string; | ||
recordedDate: string; | ||
recorder: Recorder; | ||
} | ||
|
||
export interface ResourceMeta { | ||
versionId: string; | ||
lastUpdated: string; | ||
tag: Array<ResourceMetaTag>; | ||
} | ||
|
||
export interface ResourceMetaTag { | ||
system: string; | ||
code: string; | ||
display: string; | ||
} | ||
|
||
export interface ClinicalStatus { | ||
coding: Array<Coding>; | ||
} | ||
|
||
export interface Coding { | ||
system: string; | ||
code: string; | ||
} | ||
|
||
export interface Code { | ||
coding: Array<ConditionCoding>; | ||
text: string; | ||
} | ||
|
||
export interface ConditionCoding { | ||
code: string; | ||
display?: string; | ||
system?: string; | ||
} | ||
|
||
export interface Subject { | ||
reference: string; | ||
type: string; | ||
display: string; | ||
} | ||
|
||
export interface Recorder { | ||
reference: string; | ||
type: string; | ||
display: string; | ||
} | ||
|
||
export interface Condition { | ||
status?: 'active' | 'inactive'; | ||
display: string; | ||
patient: string; | ||
onsetDateTime: string; | ||
recordedDate: string; | ||
recorder: string; | ||
} | ||
|
||
export const usePatientConditions = (patientUuid: string) => { | ||
const { showPatientConditions } = useConfig<PharmacyConfig>(); | ||
const url = `${fhirBaseUrl}/Condition?patient=${patientUuid}&_count=100&_summary=data`; | ||
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. What happens if there are more than 100? Shouldn't we have some sort of paging here? 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. Ok, the endpoint seem not to support pagination fully as shown in bellow endpoint.So I though it's extremely rare for a patient to have more that 100 conditions.Also saw the same From the screenshort, I espected one entry item since the totalMatched Records is 3, the count is 2, leaving page 1 to 2 entries and page 1 to one entry. Also looking at the links (next and previous), its unespected, and if i query either of them, i find same same results, i have also played around with the values for query params bt i get same results, tried using 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. So, |
||
const { data, isLoading, mutate, error } = useSWR<FetchResponse<ConditionsBundle>>( | ||
showPatientConditions ? url : null, | ||
openmrsFetch, | ||
); | ||
|
||
const conditions = useMemo(() => { | ||
return data?.data?.entry?.reduce<Array<Condition>>((prev, entry) => { | ||
if (entry?.resource?.resourceType === 'Condition') { | ||
const condition: Condition = { | ||
display: entry?.resource?.code?.text, | ||
onsetDateTime: entry?.resource?.onsetDateTime, | ||
patient: entry?.resource?.subject?.display, | ||
recordedDate: entry?.resource?.recordedDate, | ||
recorder: entry?.resource?.recorder?.display, | ||
status: entry?.resource?.clinicalStatus?.coding[0]?.code as any, | ||
}; | ||
return [...prev, condition]; | ||
} | ||
return prev; | ||
}, []); | ||
}, [data]); | ||
return { | ||
conditions: (conditions ?? []).filter((condition) => condition.status === 'active'), | ||
isLoading, | ||
error, | ||
mutate, | ||
showPatientConditions, | ||
}; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
@use '@carbon/layout'; | ||
@use '@carbon/colors'; | ||
|
||
.conditionsContainer { | ||
background-color: colors.$white; | ||
margin: layout.$spacing-03 0 0 0; | ||
} | ||
|
||
.emptyText { | ||
color: colors.$gray-50; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -138,6 +138,16 @@ export const configSchema = { | |
_description: 'Enable or disable viewing of patient visit diagnoses', | ||
_default: false, | ||
}, | ||
showExtraPatientInformationSlot: { | ||
_type: Type.Boolean, | ||
_description: 'Enable or disable viewing of patient extra information slot diagnoses', | ||
_default: false, | ||
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. Would it make sense that rather than having a separate parameter for this we determine whether to display this based on "showDiagnosesFromVisit || showPatientConditions"? I guess the downside would be that someone couldn't separately enable this slot to just to insert there own extensions? If we do keep it like this, we should make note in the "_descriptions" of the "showDiagnosesFromVisit" and "showPatientConditions" that this slot needs to be enabled in order for diagnoses/conditions to be visible. 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. In all honesty, I don't like hiding extension slots behind some sort of configuration. Extension slots should just be rendered on the screen and content added to them according to the implementation config. I don't see any value (and I think it just makes the configuration more confusing) to to things like this. Please remove this option. 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. Ok, Removed |
||
}, | ||
showPatientConditions: { | ||
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. We also don't need configuration options where things are exposed as extensions. Please remove these. 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. Yes, this makes much sense, did just that, the extensions will be added from the config-app of (distro/implentor), for now to demo just added them from implementor tool configure-diagnoses.mp4 |
||
_type: Type.Boolean, | ||
_description: 'Enable or disable viewing of patient conditions', | ||
_default: false, | ||
}, | ||
}; | ||
|
||
export interface PharmacyConfig { | ||
|
@@ -182,4 +192,6 @@ export interface PharmacyConfig { | |
}; | ||
enableStockDispense: boolean; | ||
showDiagnosesFromVisit: boolean; | ||
showPatientConditions: boolean; | ||
showExtraPatientInformationSlot: boolean; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,17 +1,30 @@ | ||
import { InlineLoading, InlineNotification, Tag, Tile } from '@carbon/react'; | ||
import { InlineLoading, InlineNotification } from '@carbon/react'; | ||
import React from 'react'; | ||
import { useTranslation } from 'react-i18next'; | ||
import { usePatientDiagnosis } from './diagnoses.resource'; | ||
import { | ||
StructuredListWrapper, | ||
StructuredListHead, | ||
StructuredListRow, | ||
StructuredListCell, | ||
StructuredListBody, | ||
Layer, | ||
} from '@carbon/react'; | ||
import styles from './diagnoses.scss'; | ||
import EmptyState from '../components/empty-state.component'; | ||
|
||
type PatientDiagnosesProps = { | ||
patientUuid: string; | ||
encounterUuid: string; | ||
}; | ||
|
||
const PatientDiagnoses: React.FC<PatientDiagnosesProps> = ({ encounterUuid, patientUuid }) => { | ||
const { diagnoses, isLoading, error } = usePatientDiagnosis(encounterUuid); | ||
const { diagnoses, isLoading, error, showDiagnosesFromVisit } = usePatientDiagnosis(encounterUuid); | ||
|
||
const { t } = useTranslation(); | ||
|
||
if (!showDiagnosesFromVisit) return null; | ||
|
||
if (isLoading) | ||
return ( | ||
<InlineLoading | ||
|
@@ -24,18 +37,35 @@ const PatientDiagnoses: React.FC<PatientDiagnosesProps> = ({ encounterUuid, pati | |
if (error) | ||
return <InlineNotification kind="error" subtitle={t('diagnosesError', 'Error loading diagnoses')} lowContrast />; | ||
|
||
if (!diagnoses?.length) return null; | ||
if (!diagnoses.length) | ||
return ( | ||
<EmptyState | ||
title={t('diagnoses', 'Diagnoses')} | ||
message={t('noDiagnoses', "No diagnoses for this patient's visit")} | ||
/> | ||
); | ||
|
||
return ( | ||
<Tile> | ||
<strong> | ||
{t('diagnoses', 'Diagnoses')} {diagnoses.length ? `(${diagnoses.length})` : ''} | ||
</strong> | ||
<br /> | ||
{diagnoses.map(({ text }, index) => ( | ||
<Tag key={index}>{text}</Tag> | ||
))} | ||
</Tile> | ||
<Layer className={styles.diagnosesContainer}> | ||
<StructuredListWrapper> | ||
<StructuredListHead> | ||
<StructuredListRow head> | ||
<StructuredListCell head> | ||
{t('diagnoses', 'Diagnoses')} {`(${diagnoses.length})`} | ||
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. Is 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 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. Thanks, @Omoshlawi for the good work. I do think that for pharmacists, a more compressed presentation is all they need. I would suggest that you revert to the initial implementation that looked like the below:
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. @Omoshlawi could you kindly provide a screenshot that reverts this change? 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 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. Please add screenshots which are a bit visible 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. |
||
</StructuredListCell> | ||
<StructuredListCell head>{t('status', 'Status')}</StructuredListCell> | ||
</StructuredListRow> | ||
</StructuredListHead> | ||
<StructuredListBody> | ||
{diagnoses.map(({ certainty, text }) => ( | ||
<StructuredListRow> | ||
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. Any time you call |
||
<StructuredListCell noWrap>{text}</StructuredListCell> | ||
<StructuredListCell>{certainty}</StructuredListCell> | ||
</StructuredListRow> | ||
))} | ||
</StructuredListBody> | ||
</StructuredListWrapper> | ||
</Layer> | ||
); | ||
}; | ||
|
||
|
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.
We don't need a whole component for this. Empty states should match the design of the thing that they are the empty state for. Generic empty states are only useful in contexts where we have many similar widgets.
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.
i didn't quite understand this, kindly explain further, do you mean something like this for consistent desgin in despensing app

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.
And what do you think of empty state in screenshot bellow that uses Data table??