-
Notifications
You must be signed in to change notification settings - Fork 44
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
(refactor) Generalizing Application Logic - Decoupling from UgandaEMR in Lab Workflows #65
Conversation
LGTM ... |
@pirupius @jabahum @ojwanganto WDYT? |
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.
Thanks @samuelmale LGTM!
import { InlineLoading } from "@carbon/react"; | ||
import styles from "./loader.scss"; | ||
|
||
const Loader: React.FC = () => { |
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.
At some point, I hope we'll have such in a common/core lib where we just reuse.
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.
Plus 100
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.
Thanks @samuelmale for the amazing work. I've left some comments around some quick fixes we can add to this PR. And i'll start a conversation on the slack channel around the workflows
_default: false, | ||
_description: "This enables sending results to patient via email", | ||
}, | ||
enableSpecimenIdAutoGeneration: { |
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 is something i believe should be part of the default because it was a requirement for multiple implementations. With Ampath and KenyaEMR interested
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.
@pirupius Do we have a clear ID generation strategy? I checked the KenyaEMR instance and this feature doesn't exist and I believed it's one of the reasons they forked
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.
@jabahum @slubwama please shade more light on this. @samuelmale you can see the demo on the UgandaEMR staging instance
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.
@pirupius I'm familiar with how it works for the UgandaEMR and it seems to works perfectly. The plan is to port this feature in their custom repo so they won't be affected.
If we are to support the auto ID generation feature we have to either adopt UgandEMR's backend solution or figure out an ID gen strategy which is beyond the scope of this PR.
Co-authored-by: Pius Rubangakene <[email protected]>
…nmrs-esm-laboratory into chore/generalize-app
The "Tests ordered" tab is like a bucket of all tests ordered with different statuses. It's equipped with a filter that lets you filter the list by status. However there was no way for say "view new orders; orders without a fulfiller status and with a new action". To solve this problem, I've introduced a filter option called "NEW". This is important for UgandEMR's workflow. |
@samuelmale I don't understand the rationale for the new filter especially for statuses already covered in other tabs. We need to discuss the transition process and the corresponding tabs that various fulfiller actions should map to. Am certain lab processes are common across different countries/implementations |
@ojwanganto The "Tests ordered" is like a bucket of orders with different fulfiller statuses. However this varies across implementations, eg. For UgandaEMR's use case, this tab list only the freshly published orders (without a fulfiller status). This ensures that the lab attendants focus on the incoming fresh orders to take action.
Imagine having a very long mixed (orders of disparate statuses) list of orders but without a way of applying a filter that will let you focus on the recently published orders? The main rationale for this filter option (a derived status) is to bring in the ability of viewing those new orders. However, if this seems unnecessary I can revert; just let me know |
@samuelmale @ojwanganto this takes us back to the conversation i started on slack about having that bucket of statuses (aka NEW) being the only ones shown in the first tab. It's the landing point and the primary users for this are the lab attendants who want to know what lab orders need to be worked on. |
Thanks @samuelmale, @pirupius - my guess is that the implementation of the first tab should be common across :). It is basically what is fresh from a clinician to the lab. |
@pirupius @ojwanganto I'm on board with dropping the filter in the "Tests Ordered" table and default to loading "NEW" orders. However this idea of having the filter was something the DIGI team was interested in and I believe we had arrived at a consensus of us having it that way? Should we drop the filter? cc: @gitcliff @caseyi @slubwama @jabahum |
LGTM unless there's any concerns from others |
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.
LGTM
Merging this in to unblock other tasks. Any other enhancements will come in follow up PR's |
Requirements
Summary
This PR primarily removes logic that is specific to UgandaEMR's custom Lab workflows. I'm preparing a follow-up PR to migrate this logic to a dedicated UgandaEMR-specific repository. Additionally, the PR includes bug fixes, general code enhancements. However, there is room for improvement in code quality, and follow-up tasks will be created to address this.
Screenshots
https://www.loom.com/share/2aa94526398f4114bd96e06d64d56c44
Related Issue
https://openmrs.atlassian.net/browse/O3-2640