(feat) O3-3259 Add ability to deduct stock items while performing dispensing medication #107

donaldkibet · 2024-05-22T05:21:35Z

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

See https://openmrs.atlassian.net/browse/O3-3259

Screenshots

Related Issue

For the feature to work, the pharmacist has to be given the privileges required to access stock and inventory rest endpoints.

Other

…pensing

jabahum

Please share a gif

jabahum · 2024-05-22T08:28:39Z

src/routes.json

@@ -1,7 +1,8 @@
 {
  "backendDependencies": {
    "fhir2": ">=1.2",
-    "webservices.rest": "^2.2.0"
+    "webservices.rest": "^2.2.0",
+    "stockmanagement": "^1.0.0"


change stockmanagement version to ^1.4.1 >

jabahum · 2024-05-22T08:36:37Z

src/forms/stock-dispense/stock-dispense.component.tsx

+        batchNumber: inventoryItems.batchNumber,
+        quantity: Math.floor(inventoryItems.quantity),
+        quantityUoM: inventoryItems.quantityUoM,
+        expiration: formatDate(new Date(inventoryItems.expiration)),


format this to MM/YYYY

Any specific reason why it should marked that way?

not really but it looks better when shorter

Date formatting should remain consistent within the application unless there's a strong reason not to, so I think formatDate() is more likely to be correct here.

Tend to agree with @ibacher

jabahum · 2024-05-22T08:39:27Z

src/forms/stock-dispense/useDispenseStock.tsx

+ * Interface for the stock dispense request object.
+ */
+export type StockDispenseRequest = {
+  locationUuid: string;


this too changes to dispenseLocationUuid

Updated with the requested changes, according to @slubwama PR this should be dispenseLocation

as per the request body and not the response ?

jabahum · 2024-05-22T08:40:55Z

src/forms/stock-dispense/useDispenseStock.tsx

+ */
+export type StockDispenseRequest = {
+  locationUuid: string;
+  patientUuid: string;


this is just patient,order,encounter,stockItem,stockBatch as per new version of stockmanagement 1.4.0

pirupius · 2024-05-22T09:11:26Z

src/routes.json

@@ -1,7 +1,8 @@
 {
  "backendDependencies": {
    "fhir2": ">=1.2",
-    "webservices.rest": "^2.2.0"
+    "webservices.rest": "^2.2.0",
+    "stockmanagement": "^1.4.1 "


@donaldkibet won't this mean implementations that don't use stock management will keep getting dependency errors? Is there a middle ground for this?

Agree with @pirupius , thanks for flagging this.

Hmmm... We probably do need a way of indicating optional dependencies, though, right, i.e., if this only works with stockmanagement >= 1.4.1, then we do want some way of flagging that for implementations that do use stockmanagement.

Another thing for the backlog: https://openmrs.atlassian.net/browse/O3-3266

Tried to relax this with

"stockmanagement": ">=1.4.1"

Either way, for now, we can't have it in backend dependencies, because it will create an annoying pop-up for everyone who's not using stock management, hence the ticket.

should it then be removed entirely? If yes, how best can we still have this requirement, at least somewhere in the application?

Yes. For now, I don't think we can. We need this feature to properly support that.

Yes, this needs to be removed for now.

mogoodrich · 2024-05-22T12:36:59Z

Thanks @donaldkibet ! Looking forward to adding this functionality... I should be able to review this today and will follow up with questions... would it be possible to add a little more details to the ticket about the approach we are taking for this?

mogoodrich

Generally looks good @donaldkibet though I added some requests for changes.

Would be great to see some screen shots of this as well when you get a chance.

mogoodrich · 2024-05-22T20:03:47Z

src/config-schema.ts

+  enableStockDispense: {
+    _type: Type.Boolean,
+    _description: 'To enable or disable the deduction of stock during the dispensing process',
+    _default: true,


To maintain the same behavior by default, this default value should be false.

mogoodrich · 2024-05-22T20:05:06Z

src/routes.json

@@ -1,7 +1,8 @@
 {
  "backendDependencies": {
    "fhir2": ">=1.2",
-    "webservices.rest": "^2.2.0"
+    "webservices.rest": "^2.2.0",
+    "stockmanagement": "^1.4.1 "


Agree with @pirupius , thanks for flagging this.

mogoodrich · 2024-05-22T20:16:18Z

src/forms/stock-dispense/useDispenseStock.tsx

@@ -0,0 +1,79 @@
+import useSWR from 'swr';


I know we've started a pattern of calling something "useDispenseStock" but then including multiple different hooks in it, but I think it would be better to get away from this and call this something like stock.resource.tsx or stock-management.resource.tsx. Do you agree @denniskigen @ibacher ?

Indeed, that's substantially more consistent with every other app in the ecosystem.

Thanks @ibacher ! So @donaldkibet yes, let's please change the name of this file.

mogoodrich · 2024-05-22T20:17:00Z

src/forms/stock-dispense/useDispenseStock.tsx

+/**
+ * Interface for the stock dispense request object.
+ */
+export type StockDispenseRequest = {


Is there a reason to include this here instead of in types?

This was left by mistake, moved to types file

mogoodrich · 2024-05-22T20:18:49Z

src/forms/stock-dispense/useDispenseStock.tsx

+ * @param {string} drugUuid - The UUID of the drug.
+ * @returns {Array} - The inventory items.
+ */
+export const useDispenseStock = (drugUuid: string) => {


I see you are setting limit=10, will there be a problem if there are more than 10 matching inventory items?

@slubwama can give us a clear explanation why we need to add this and if its really necessary we find how to pass it from the UI.

mogoodrich · 2024-05-22T20:27:00Z

src/forms/dispense-form.component.tsx

@@ -67,6 +77,30 @@ const DispenseForm: React.FC<DispenseFormProps> = ({
              medicationRequestBundle,
              config.dispenseBehavior.restrictTotalQuantityDispensed,
            );
+
+            if (config.enableStockDispense) {


This is breaking up the logic around fulfiller status, can we break this up into a separate "then" block, or at least move it up to line 73, before the "const newFulfillerStatus..." line?

Also, defer to you about the order operations... ie, would you want to move the dispense request before the "saveMedicationRequest" so that you only update the medication request if the stock update is successful? Of course, then you run into the opposite risk: that the stock could be updated and then the dispense fails. Would be great to make this a little more transactional, but, admittedly, that is likely difficult.

donaldkibet · 2024-05-24T06:31:56Z

@mogoodrich @ibacher made the requested changes. Attached is an image of the changes

Kapture.2024-05-24.at.09.22.27.mp4

mogoodrich

@donaldkibet everything looks good to me, with the exception of the fact that it looks like we are still including the stock management dependency?

donaldkibet · 2024-05-27T13:22:01Z

@mogoodrich removed the stock management requirement from the backend dependencies until this PR is merged in. I have also update comments on the config to make it explicit.

mogoodrich

Thanks @donaldkibet , LGTM!

Feel free to merge.

ojwanganto · 2024-05-28T13:21:38Z

Thanks, @mogoodrich

…pensing medication (#107) (fix bug with stock dispense conditional)

mogoodrich · 2024-05-30T14:41:04Z

src/forms/stock-dispense/stock.resource.tsx

+ */
+export const useDispenseStock = (drugUuid: string) => {
+  const session = useSession();
+  const url = `/ws/rest/v1/stockmanagement/stockiteminventory?v=default&totalCount=true&drugUuid=${drugUuid}&includeBatchNo=true&groupBy=LocationStockItemBatchNo&dispenseLocationUuid=${session?.sessionLocation?.uuid}&includeStrength=1&includeConceptRefIds=1&emptyBatch=1&emptyBatchLocationUuid=${session?.sessionLocation?.uuid}&dispenseAtLocation=1`;


Not to weigh into this after the fact, but, thinking about the TAC call we are on right now, it would be great if, long-term, the stock management module support an FHIR endpoint so that it would hopefully be more standardized.

donaldkibet added 2 commits May 21, 2024 14:46

(chore) upgrade @openmrs/esm-framework version

1511795

(feat) O3-3259 Add ability to deduct stock items while performing dis…

cfb6d33

…pensing

donaldkibet requested review from denniskigen, mogoodrich, slubwama and ojwanganto and removed request for denniskigen May 22, 2024 05:21

jabahum reviewed May 22, 2024

View reviewed changes

code reviews changes updated

a6d86fc

pirupius reviewed May 22, 2024

View reviewed changes

mogoodrich requested changes May 22, 2024

View reviewed changes

donaldkibet force-pushed the O3-3259 branch 2 times, most recently from 713e06e to 008f671 Compare May 24, 2024 06:40

mogoodrich requested changes May 24, 2024

View reviewed changes

code reviews changes

9af75a9

donaldkibet force-pushed the O3-3259 branch from 008f671 to 9af75a9 Compare May 27, 2024 13:13

donaldkibet requested a review from mogoodrich May 27, 2024 13:28

mogoodrich approved these changes May 28, 2024

View reviewed changes

ojwanganto merged commit 347959b into main May 28, 2024
4 checks passed

ojwanganto deleted the O3-3259 branch May 28, 2024 13:21

mogoodrich added a commit that referenced this pull request May 28, 2024

(feat) O3-3259 Add ability to deduct stock items while performing dis…

b9a32f6

…pensing medication (#107) (fix bug with stock dispense conditional)

mogoodrich reviewed May 30, 2024

View reviewed changes

(feat) O3-3259 Add ability to deduct stock items while performing dispensing medication #107

(feat) O3-3259 Add ability to deduct stock items while performing dispensing medication #107

Conversation

donaldkibet commented May 22, 2024 • edited by denniskigen Loading

Requirements

Summary

Screenshots

Related Issue

Other

jabahum 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

Choose a reason for hiding this comment

pirupius May 22, 2024 • edited Loading

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

mogoodrich commented May 22, 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

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

donaldkibet commented May 24, 2024

mogoodrich left a comment

Choose a reason for hiding this comment

donaldkibet commented May 27, 2024

mogoodrich left a comment • edited Loading

Choose a reason for hiding this comment

ojwanganto commented May 28, 2024

Choose a reason for hiding this comment

donaldkibet commented May 22, 2024 •

edited by denniskigen

Loading

pirupius May 22, 2024 •

edited

Loading

mogoodrich left a comment •

edited

Loading