(feat) O3-4078 - Ward App - Simplify patient card configurations #1339
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chibongho
requested review from
mseaton,
brandones,
mogoodrich,
denniskigen,
vasharma05,
ibacher and
usamaidrsk
|
Size Change: -119 kB (-1.8%) Total Size: 6.46 MB
ℹ️ View Unchanged
|
mogoodrich
approved these changes
Oct 11, 2024
There was a problem hiding this comment.
Thanks for slogging through this, @chibongho ! Yeah, I didn't make it 100% through this, but generally looks good and I agreed with the strategy based on our discussion earlier this week about this.
| identifierElementDefinitions: Array<IdentifierElementDefinition>; | ||
| addressElementDefinitions: Array<AddressElementDefinition>; | ||
| cardDefinitions: Array<WardPatientCardDefinition>; | ||
| patientCardElements: { |
There was a problem hiding this comment.
Potential rabbit-hole... I guess I was thinking that each WardView would have it's own config object? When we get to the point where a custom WardView can be defined outside the Ward App ESM, is there a way for the developers of new Ward Views to define new config parameters without adding to the Ward View config? (For discussion, let's not let this be a blocker for merging)
There was a problem hiding this comment.
For a custom WardView outside this ESM, it can have patient cards with a mixture of elements defined in this ESM and outside it. I think ideally we'd have the elements configurations be tied to whichever ESM it's defined in. We'll want to be careful as what useConfig() returns is dependent on which ESM / extension the component is in. I haven't done something like this before, but it feels doable.
usamaidrsk
reviewed
Oct 15, 2024
| obsElementDefinitions: { | ||
| patientCardElements: { | ||
| _description: | ||
| 'Configuration of various patient card elements. Each configured element must have an unique id, defined in the ward React component being used.', |
There was a problem hiding this comment.
Fixing some small grammar:
Configuration of various patient card elements. Each configured element must have a unique id, defined in the ward React component being used.
There was a problem hiding this comment.
If we could have a way to automatically generate the keys, it would significantly reduce the risk of human error.
There was a problem hiding this comment.
If we could have a way to automatically generate the keys, it would significantly reduce the risk of human error.
This might be difficult, with how the ward view can be an extension outside the ward app.
usamaidrsk
reviewed
Oct 15, 2024
| _type: Type.Array, | ||
| _description: `Defines patient identifier elements that can be included in the card header or footer. The default element 'patient-identifier' displays the preferred identifier.`, | ||
| _description: `Configures patient identifier to display. An unconfigured element displays the preferred identifier.`, | ||
| _default: [ |
There was a problem hiding this comment.
looks like the _default should be an empty array, provided the default object passed is missing some keys i.e identifierTypeUuid
usamaidrsk
reviewed
Oct 15, 2024
| } | ||
|
|
||
| export interface PendingItemsDefinition { | ||
| export interface PendingItemsElementConfig { |
There was a problem hiding this comment.
I think you missed the id here.
usamaidrsk
reviewed
Oct 15, 2024
| id: string; | ||
| fields: Array<AddressField>; | ||
| } | ||
|
|
||
| export interface WardPatientCardDefinition { | ||
| export interface AdmissionRequestNoteElementConfig { |
|
Thanks @chibongho , I have a few comments. |
Thanks for the comments. I was getting a bit sloppy with the config documentations. I'm going to merge this in. Post-hoc comments welcome. |
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requirements
Summary
See O3-4078 and relevant slack thread.
This refactoring touches a lot of files (sorry), but there is almost no functional change besides how we configure the patient cards.
DefaultWardViewandMaternalWardViewboth define their own data fetching and card rendering. What elements appear in those cards are hard-coded (thus not configurable). If another implementation wants a different looking ward, they will need to make their own WardView. (Currently, they can only make that within the esm ward app).DefaultWardVieworMaternalWardView, and the element configuration requires specifying the id. This means whoever is configuring the elements will need to know what id's are hard-coded into the WardView they want to use.Most relevant files to review:
With this change, the configuration for maternal wards in MCOE is simpler:
Screenshots
Related Issue
Other
I introduced a bug with the baby row appearing twice within a mother's patient card sometimes (highlighted in screenshot). I will address that in this ticket.