Skip to content
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

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
Changes from 2 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
01c7fec
added patient diagnoses to dispensing app
Omoshlawi Nov 18, 2024
1b50388
Added free texts to to translations
Omoshlawi Nov 18, 2024
5815f00
Displaying nothing if no diagnoses are recorded for the visit
Omoshlawi Nov 19, 2024
1d930fc
(fix) Fix exhaustive-deps and rules-of-hooks warnings (#124)
denniskigen Nov 20, 2024
0e92207
Made viewing of visit diagnoses configurable
Omoshlawi Nov 21, 2024
982dbc1
Added configurable extra-patient info slot, plugged in patient diagno…
Omoshlawi Nov 25, 2024
bf5c2e6
Added empty state component and handle empty state in diagnoses and c…
Omoshlawi Nov 25, 2024
9c2e293
Handle loading and error in conditions component
Omoshlawi Nov 25, 2024
68876e6
Merge branch 'main' into O3-4206-add-ability-to-view-patient-diagnose…
Omoshlawi Jan 10, 2025
e0a915c
(refactor) Remove conditional rendering for patient conditions and di…
Omoshlawi Jan 10, 2025
1f3419d
(refactor) Enhance patient diagnoses component with DataTable and rem…
Omoshlawi Jan 10, 2025
637242d
Merge branch 'main' into O3-4206-add-ability-to-view-patient-diagnose…
Omoshlawi Jan 24, 2025
54e491e
(refactor) Simplify conditions and diagnoses components by removing D…
Omoshlawi Jan 26, 2025
0ed2af6
Merge branch 'main' into O3-4206-add-ability-to-view-patient-diagnose…
Omoshlawi Feb 25, 2025
bdfad34
(refactor) using useFhirFetchAll to fetch all conditions aggressively
Omoshlawi Feb 25, 2025
7c88475
Merge branch 'main' into O3-4206-add-ability-to-view-patient-diagnose…
Omoshlawi Mar 13, 2025
954e710
(refactor) Update conditions and diagnoses display to adopt the pill …
Omoshlawi Mar 13, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions src/components/empty-state.component.tsx
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 = {
Copy link
Member

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.

Copy link
Contributor Author

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
Screenshot 2025-01-10 at 12 14 21

Copy link
Contributor Author

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??

Screenshot 2025-01-10 at 19 07 54

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;
50 changes: 50 additions & 0 deletions src/components/empty-state.scss
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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use Carbon spacing tokens for spacing in SCSS row-gap, width, etc.

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;
Copy link
Member

Choose a reason for hiding this comment

The 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;
}
57 changes: 57 additions & 0 deletions src/conditions/conditions.component.tsx
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;
133 changes: 133 additions & 0 deletions src/conditions/conditions.resource.ts
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`;
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

@Omoshlawi Omoshlawi Jan 10, 2025

Choose a reason for hiding this comment

The 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 _count=100 used in patient chart so I thought it could work.If you have a walk around or if my query ain't right kindly guide further

Screenshot 2025-01-10 at 16 17 34

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 _page, _getpageoffset, all do not avail

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, _offset is a non-standard FHIR parameter and should not be sent in the initial request (as it's not in the actual code). With that parameter, the next and previous links are weird, without it they should work.

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,
};
};
11 changes: 11 additions & 0 deletions src/conditions/conditions.scss
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;
}
12 changes: 12 additions & 0 deletions src/config-schema.ts
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,
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, Removed

},
showPatientConditions: {
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

@Omoshlawi Omoshlawi Jan 10, 2025

Choose a reason for hiding this comment

The 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;
}
54 changes: 42 additions & 12 deletions src/diagnoses/diagnoses.component.tsx
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})`}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is diagnoses.length something that anyone asked for here? Why aren't we just using a data table or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed and replaced by data tables, bellow is a screenshot of how it looks now
Screenshot 2025-01-10 at 19 15 18

Copy link

@ojwanganto ojwanganto Jan 10, 2025

Choose a reason for hiding this comment

The 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:

image
@ibacher and I are aligned on the same. We don't need to show the count of diagnoses

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Omoshlawi could you kindly provide a screenshot that reverts this change?

Copy link
Contributor Author

@Omoshlawi Omoshlawi Jan 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it is.I have reserved the configurability through implementor config-app

Screenshot 2025-01-26 at 11 00 15

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add screenshots which are a bit visible

Copy link
Contributor Author

@Omoshlawi Omoshlawi Jan 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, hope this is clearer

Screenshot 2025-01-27 at 18 45 01

</StructuredListCell>
<StructuredListCell head>{t('status', 'Status')}</StructuredListCell>
</StructuredListRow>
</StructuredListHead>
<StructuredListBody>
{diagnoses.map(({ certainty, text }) => (
<StructuredListRow>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any time you call map() in a React component, the component must have a key.

<StructuredListCell noWrap>{text}</StructuredListCell>
<StructuredListCell>{certainty}</StructuredListCell>
</StructuredListRow>
))}
</StructuredListBody>
</StructuredListWrapper>
</Layer>
);
};

Loading
Oops, something went wrong.