(feat)O3-4206-Add ability to view patient diagnoses and active conditions #123

Omoshlawi · 2024-11-18T14:05:12Z

Requirements

This PR has a title that briefly describes the work done, including the ticket number if there is a ticket.
My work conforms to the OpenMRS 3.0 Styleguide and design documentation.
My work includes tests or is validated by existing tests.

Summary

As discused in slack and based

Added Configurable patient active conditions (Disabled by default)
Made patient Visit Diagnoses configurable (disabled by default)
Added extra patient information tab in vertical tab menu whose content is a slot allowing different implementors to plug in any extra information required
The tab is also configurable and disabled by default

Screenshots

patient-diagnoses-conditions-cmpressed.mp4

Related Issue

https://openmrs.atlassian.net/browse/O3-4206

Other

mogoodrich

Thanks so much @Omoshlawi .... will generally defer to others on this, with some thoughts:

Looks like this only displays the diagnoses records in the visit, which likely makes sense, but would be good for others to weigh in
I don't love the way the diagnoses are displayed in the screen capture, but will defer to the UX team if we are following display standards correctly
In my opinion, we should hide the diagnoses display if no diagnoses are found (instead of displaying "no diagnoses found")

I will add some others to the reviewers.

mogoodrich · 2024-11-18T17:13:52Z

src/diagnoses/diagnoses.component.tsx

+          ))}
+        </>
+      ) : (
+        <>{t('noDiagnoses', 'No diagnoses found')}</>


I would go with just displaying nothing at all if no diagnoses are found? This is would make it more somewhat more compatible with implementations that are not yet storing diagnoses using the "encounter diagnosis" module?

Thanks @mogoodrich , fixed to display nothing if there are no diagnoses for the visit, for the UI I'll wait for the UX team output.

mogoodrich · 2024-11-18T17:22:38Z

Are Paul and/or Ciaran on Github and can they be flagged here to review the UI? Otherwise, @Omoshlawi can you add the screen capture you collected to the linked ticket and flag them there for review? thanks!

denniskigen · 2024-11-18T18:57:07Z

@mogoodrich Ping @paulsonder @ciaranduffy

Omoshlawi · 2024-11-19T06:13:28Z

Are Paul and/or Ciaran on Github and can they be flagged here to review the UI? Otherwise, @Omoshlawi can you add the screen capture you collected to the linked ticket and flag them there for review? thanks!

Okay, sure

paulsonder · 2024-11-19T09:47:23Z

@Omoshlawi I left some comments in the Jira ticket here.

This PR fixes warnings for `exhaustive-deps` and `rules-of-hooks` in the codebase flagged by the ESLint react hooks plugin.

…ses to the slot

…onditions

mogoodrich

Thanks so much, this is looking really good!

So I added one question/comment. Beyond that:

eventually we want to be able to pull in the conditions and diagnoses via a standard resource provided by core or the patient chart instead of having to recreate a resource in each app that we want to display conditions/diagnoses in, but I don't want to be a blocker in getting this merged in if we aren't there yet. Thoughts @ibacher @denniskigen ?
Also, overall, it would be good for someone else to get eyes on this with more O3 experience that me

mogoodrich · 2024-11-25T20:59:38Z

src/config-schema.ts

+  showExtraPatientInformationSlot: {
+    _type: Type.Boolean,
+    _description: 'Enable or disable viewing of patient extra information slot diagnoses',
+    _default: false,


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.

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.

ibacher

Some more technical comments that what I've put on Slack.

ibacher · 2024-11-25T21:17:50Z

src/components/empty-state.component.tsx

+import { Report } from '@carbon/react/icons';
+import styles from './empty-state.scss';
+
+type EmptyState = {


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.

ibacher · 2024-11-25T21:18:20Z

src/components/empty-state.scss

+  display: flex;
+  align-items: center;
+  justify-content: space-between;
+  row-gap: 1.5rem;


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

ibacher · 2024-11-25T21:18:34Z

src/components/empty-state.scss

+    width: 2rem;
+    border-bottom: 0.375rem solid var(--brand-03);
+    position: absolute;
+    bottom: -0.75rem;


This looks like a hack. Why do we need this here?

ibacher · 2024-11-25T21:19:18Z

src/conditions/conditions.resource.ts

+
+export const usePatientConditions = (patientUuid: string) => {
+  const { showPatientConditions } = useConfig<PharmacyConfig>();
+  const url = `${fhirBaseUrl}/Condition?patient=${patientUuid}&_count=100&_summary=data`;


What happens if there are more than 100? Shouldn't we have some sort of paging here?

ibacher · 2024-11-25T21:20:00Z

src/config-schema.ts

+    _description: 'Enable or disable viewing of patient extra information slot diagnoses',
+    _default: false,
+  },
+  showPatientConditions: {


We also don't need configuration options where things are exposed as extensions. Please remove these.

ibacher · 2024-11-25T21:20:33Z

src/diagnoses/diagnoses.component.tsx

+        </StructuredListHead>
+        <StructuredListBody>
+          {diagnoses.map(({ certainty, text }) => (
+            <StructuredListRow>


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

ibacher · 2024-11-25T21:20:59Z

src/diagnoses/diagnoses.component.tsx

+        <StructuredListHead>
+          <StructuredListRow head>
+            <StructuredListCell head>
+              {t('diagnoses', 'Diagnoses')} {`(${diagnoses.length})`}


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

ibacher · 2024-11-25T21:21:36Z

src/routes.json

@@ -19,6 +19,20 @@
      "online": true,
      "offline": true
    },
+    {
+      "name": "patient-diagnoses",
+      "slot": "extra-patient-information-slot",


These extensions should not be bound to any slot by default. That way, they are already able to be enabled or disabled by configuration.

ibacher · 2024-11-25T21:21:48Z

src/routes.json

+      "slot": "extra-patient-information-slot",
+      "component": "patientDiagnoses",
+      "online": true,
+      "offline": true


Also, I'd probably just leave offline alone.

Omoshlawi added 2 commits November 18, 2024 08:12

added patient diagnoses to dispensing app

01c7fec

Added free texts to to translations

1b50388

ojwanganto requested review from denniskigen, mogoodrich and makombe November 18, 2024 14:38

mogoodrich reviewed Nov 18, 2024

View reviewed changes

mogoodrich requested a review from ibacher November 18, 2024 17:20

Displaying nothing if no diagnoses are recorded for the visit

5815f00

denniskigen and others added 5 commits November 21, 2024 17:27

(fix) Fix exhaustive-deps and rules-of-hooks warnings (openmrs#124)

1d930fc

This PR fixes warnings for `exhaustive-deps` and `rules-of-hooks` in the codebase flagged by the ESLint react hooks plugin.

Made viewing of visit diagnoses configurable

0e92207

Added configurable extra-patient info slot, plugged in patient diagno…

982dbc1

…ses to the slot

Added empty state component and handle empty state in diagnoses and c…

bf5c2e6

…onditions

Handle loading and error in conditions component

9c2e293

Omoshlawi changed the title ~~(feat)O3-4206-Add ability to view patient diagnoses alongside prescribed drugs and allergies~~ (feat)O3-4206-Add ability to view patient diagnoses and active conditions Nov 25, 2024

mogoodrich reviewed Nov 25, 2024

View reviewed changes

ibacher requested changes Nov 25, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(feat)O3-4206-Add ability to view patient diagnoses and active conditions #123

(feat)O3-4206-Add ability to view patient diagnoses and active conditions #123

Omoshlawi commented Nov 18, 2024 •

edited

Loading

mogoodrich left a comment •

edited

Loading

mogoodrich Nov 18, 2024

Omoshlawi Nov 19, 2024

mogoodrich commented Nov 18, 2024

denniskigen commented Nov 18, 2024

Omoshlawi commented Nov 19, 2024

paulsonder commented Nov 19, 2024

mogoodrich left a comment

mogoodrich Nov 25, 2024

ibacher Nov 25, 2024

ibacher left a comment

ibacher Nov 25, 2024

ibacher Nov 25, 2024

ibacher Nov 25, 2024

ibacher Nov 25, 2024

ibacher Nov 25, 2024

ibacher Nov 25, 2024

ibacher Nov 25, 2024

ibacher Nov 25, 2024

ibacher Nov 25, 2024

(feat)O3-4206-Add ability to view patient diagnoses and active conditions #123

Are you sure you want to change the base?

(feat)O3-4206-Add ability to view patient diagnoses and active conditions #123

Conversation

Omoshlawi commented Nov 18, 2024 • edited Loading

Requirements

Summary

Screenshots

Related Issue

Other

mogoodrich left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mogoodrich commented Nov 18, 2024

denniskigen commented Nov 18, 2024

Omoshlawi commented Nov 19, 2024

paulsonder commented Nov 19, 2024

mogoodrich left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibacher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Omoshlawi commented Nov 18, 2024 •

edited

Loading

mogoodrich left a comment •

edited

Loading