(feat) O3-2117: Save orders on a new encounter if there is no encounter #1727

usamaidrsk · 2024-03-10T22:27:01Z

Requirements

This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.
My work conforms to the OpenMRS 3.0 Styleguide and design documentation.
My work includes tests or is validated by existing tests.

Summary

This PR adds the functionality to save orders along with creating a new encounter if one didn't exist. Before two transactions existed, i.e one to create encounter first and then create orders.

Screenshots

add-orders-on-creating-encounters.mp4

Related Issue

O3-2117

Other

packages/esm-patient-orders-app/src/api/api.ts

denniskigen

Thanks for working on this, @usamaidrsk. I've left a few suggestions

packages/esm-patient-orders-app/src/order-basket/order-basket.workspace.tsx

denniskigen · 2024-03-19T07:20:50Z

packages/esm-patient-orders-app/src/order-basket/order-basket.workspace.tsx

@@ -116,21 +124,27 @@ const OrderBasket: React.FC<DefaultWorkspaceProps> = ({
            <InlineNotification
              kind="error"
              title={t('errorCreatingAnEncounter', 'Error when creating an encounter')}
-              subtitle={t('tryReopeningTheWorkspaceAgain', 'Please try launching the workspace again')}


I think it's best to show the original message here instead of the error message from the backend, which we're already logging to the console on line 67.

I think that's right.
Just what I thought that given on creating the encounter we're adding orders too, then if the error was from the orders, showing the exact message might be helpful to the user what exactly is the issue.

Dennis may be the leading authority on UI copy; I would defer to him @usamaidrsk .

packages/esm-patient-orders-app/src/order-basket/order-basket.workspace.tsx

vasharma05

A few changes

packages/esm-patient-orders-app/src/api/api.ts

samuelmale

LGTM

samuelmale · 2024-05-28T12:10:40Z

packages/esm-patient-orders-app/src/api/api.ts

+import { type OrderPost, useSystemVisitSetting, useVisitOrOfflineVisit } from '@openmrs/esm-patient-common-lib';
+import { orderBasketStore, type OrderBasketStore } from '@openmrs/esm-patient-common-lib/src/orders/store';


Is it possible to use a single import statement?

You'll likely need to make sure that those exports are re-exported from https://github.com/openmrs/openmrs-esm-patient-chart/blob/main/packages/esm-patient-common-lib/src/orders/index.ts first. That might need to happen in a separate PR.

This needs to be exported from the @openmrs/esm-patient-common-lib or else we will very likely end up with multiple copies of the orderBasketStore, since this import means that each app will load it's own copy.

To explain: code from things listed as peerDependencies like @openmrs/esm-patient-common-lib is treated as an external by Webpack, meaning this will compile to code that ensures @openmrs/esm-patient-common-lib is loaded and load the exports from the named module. Code imported from @openmrs/esm-patient-common-lib/src/..., however, does not get converted into an external, so the code ends up imported here like in a normal dependency, meaning you're working with an app-local copy of the orderBasketStore.

It's technically ok to do this with type-only imports, because those are not part of the runtime code, so none of that code makes it into the final bundle.

As discussed with @brandones , moving the implementation of saving the encounter and orders to the common-lib seemed like a better idea. Therefore, I have created the postOrdersOnNewEncounter function in the postOrders.ts file.

cc @ibacher @denniskigen @samuelmale

packages/esm-patient-orders-app/src/api/api.ts

denniskigen · 2024-05-28T12:42:07Z

packages/esm-patient-orders-app/src/api/api.ts

+import { type OrderPost, useSystemVisitSetting, useVisitOrOfflineVisit } from '@openmrs/esm-patient-common-lib';
+import { orderBasketStore, type OrderBasketStore } from '@openmrs/esm-patient-common-lib/src/orders/store';


You'll likely need to make sure that those exports are re-exported from https://github.com/openmrs/openmrs-esm-patient-chart/blob/main/packages/esm-patient-common-lib/src/orders/index.ts first. That might need to happen in a separate PR.

…ter' into ft-send-orders-on-creating-enconter

packages/esm-patient-common-lib/src/orders/postOrders.ts

brandones · 2024-05-28T20:42:41Z

Please also modify here and here with the optional encounterUuid type.

brandones · 2024-05-28T20:41:26Z

packages/esm-patient-orders-app/src/order-basket/order-basket.workspace.tsx

@@ -116,21 +124,27 @@ const OrderBasket: React.FC<DefaultWorkspaceProps> = ({
            <InlineNotification
              kind="error"
              title={t('errorCreatingAnEncounter', 'Error when creating an encounter')}
-              subtitle={t('tryReopeningTheWorkspaceAgain', 'Please try launching the workspace again')}


Dennis may be the leading authority on UI copy; I would defer to him @usamaidrsk .

packages/esm-patient-orders-app/src/api/api.ts

…nOrderPostData

brandones

LGTM

brandones · 2024-05-29T02:58:11Z

@jayasanka-sack The E2E tests kept getting canceled? Any idea what that's about? Can we go ahead and merge this or would we risk breaking the E2E tests?

jayasanka-sack · 2024-05-29T10:45:31Z

Hey, I checked out the recordings, and it looks like the UI isn't doing anything after clicking the save button. Since other PRs and the main branch don't have this issue, it seems like this problem was introduced in this PR.

Recordings:

lab order
drug order

brandones · 2024-05-29T11:51:49Z

Thanks @jayasanka-sack ! Ok @usamaidrsk we need to fix the E2E tests.

jayasanka-sack · 2024-05-30T06:56:46Z

I ran the test locally but couldn't reproduce this behavior. The key difference when running E2E tests on GitHub compared to running them locally is that we don't use the dev server on GitHub. Instead, we build the frontend and serve it, which is somewhat similar to the dev3 environment.

However, I reviewed the trace from the report and found these console errors:

TypeError: (0 , c.postOrdersOnNewEncounter) is not a function
    at http://localhost:8080/openmrs/spa/openmrs-esm-patient-orders-app-8.0.1/287.js:2:14249
    at http://localhost:8080/openmrs/spa/openmrs-esm-patient-orders-app-8.0.1/287.js:2:13958
    at Object.next (http://localhost:8080/openmrs/spa/openmrs-esm-patient-orders-app-8.0.1/287.js:2:14063)
    at S (http://localhost:8080/openmrs/spa/openmrs-esm-patient-orders-app-8.0.1/287.js:2:9123)
    at a (http://localhost:8080/openmrs/spa/openmrs-esm-patient-orders-app-8.0.1/287.js:2:9326)
    at http://localhost:8080/openmrs/spa/openmrs-esm-patient-orders-app-8.0.1/287.js:2:9385
    at new Promise (<anonymous>)
    at http://localhost:8080/openmrs/spa/openmrs-esm-patient-orders-app-8.0.1/287.js:2:9266
    at Object.Re (http://localhost:8080/openmrs/spa/openmrs.167c04e490dfd1aa.js:2:954551)
    at Ve (http://localhost:8080/openmrs/spa/openmrs.167c04e490dfd1aa.js:2:954705)
Unhandled rejection:  TypeError: Cannot read properties of undefined (reading 'error')
    at http://localhost:8080/openmrs/spa/openmrs-esm-patient-orders-app-8.0.1/287.js:2:14503
    at http://localhost:8080/openmrs/spa/openmrs-esm-patient-orders-app-8.0.1/287.js:2:13958
    at Object.next (http://localhost:8080/openmrs/spa/openmrs-esm-patient-orders-app-8.0.1/287.js:2:14063)
    at S (http://localhost:8080/openmrs/spa/openmrs-esm-patient-orders-app-8.0.1/287.js:2:9123)
    at a (http://localhost:8080/openmrs/spa/openmrs-esm-patient-orders-app-8.0.1/287.js:2:9326)
    at http://localhost:8080/openmrs/spa/openmrs-esm-patient-orders-app-8.0.1/287.js:2:9385
    at new Promise (<anonymous>)
    at http://localhost:8080/openmrs/spa/openmrs-esm-patient-orders-app-8.0.1/287.js:2:9266
    at Object.Re (http://localhost:8080/openmrs/spa/openmrs.167c04e490dfd1aa.js:2:954551)
    at Ve (http://localhost:8080/openmrs/spa/openmrs.167c04e490dfd1aa.js:2:954705)
showToast must be called with an object having a 'description' property that is a non-empty string or object
Cannot read properties of undefined (reading 'error')
TypeError: Cannot read properties of undefined (reading 'error')
    at http://localhost:8080/openmrs/spa/openmrs-esm-patient-orders-app-8.0.1/287.js:2:14503
    at http://localhost:8080/openmrs/spa/openmrs-esm-patient-orders-app-8.0.1/287.js:2:13958
    at Object.next (http://localhost:8080/openmrs/spa/openmrs-esm-patient-orders-app-8.0.1/287.js:2:14063)
    at S (http://localhost:8080/openmrs/spa/openmrs-esm-patient-orders-app-8.0.1/287.js:2:9123)
    at a (http://localhost:8080/openmrs/spa/openmrs-esm-patient-orders-app-8.0.1/287.js:2:9326)
    at http://localhost:8080/openmrs/spa/openmrs-esm-patient-orders-app-8.0.1/287.js:2:9385
    at new Promise (<anonymous>)
    at http://localhost:8080/openmrs/spa/openmrs-esm-patient-orders-app-8.0.1/287.js:2:9266
    at Object.Re (http://localhost:8080/openmrs/spa/openmrs.167c04e490dfd1aa.js:2:954551)
    at Ve (http://localhost:8080/openmrs/spa/openmrs.167c04e490dfd1aa.js:2:954705)

jayasanka-sack · 2024-05-30T07:05:47Z

Ah, I understand now. We don't package esm-patient-common-lib, which means the postOrdersOnNewEncounter function isn't available. This is why the UI doesn't proceed once we hit the save button. The PR is safe to merge. But we should update the setup to reflect changes in the esm-patient-common-lib.

Meanwhile, @usamaidrsk I wonder why the UI silenced the error. We should display an error message to the user if something goes wrong.

usamaidrsk · 2024-05-30T07:12:50Z

Meanwhile, @usamaidrsk I wonder why the UI silenced the error. We should display an error message to the user if something goes wrong.

Thanks for the follow up @jayasanka-sack , however all errors are displayed by an inline notification on the UI. Not sure why it is skipping that on the tests

packages/esm-patient-orders-app/src/order-basket/order-basket.workspace.tsx

…ter' into ft-send-orders-on-creating-enconter

brandones · 2024-05-31T14:18:46Z

Ok, since this is an E2E bug I'm going to go ahead and merge.

denniskigen · 2024-06-04T22:15:48Z

Would you expect E2E tests to pass once this got merged in, @brandones? Still failing for new PRs surprisingly.

ibacher · 2024-06-04T23:03:53Z

They should, but unfortunately the patient-common-lib is embedded in lots of places that are not the chart, so we may not be using the latest version in tests.

raghavawasthi24 reviewed Mar 11, 2024

View reviewed changes

packages/esm-patient-orders-app/src/api/api.ts Outdated Show resolved Hide resolved

denniskigen requested review from mogoodrich, mseaton and ibacher March 19, 2024 07:08

denniskigen changed the title ~~(feat) O3-2117: Save oders on creating encounter~~ (feat) O3-2117: Save orders and encounters in the same transaction Mar 19, 2024

denniskigen requested a review from brandones March 19, 2024 07:13

denniskigen reviewed Mar 19, 2024

View reviewed changes

usamaidrsk requested a review from denniskigen March 26, 2024 12:22

usamaidrsk force-pushed the ft-send-orders-on-creating-enconter branch from 329e58a to 856641d Compare April 15, 2024 11:24

denniskigen requested a review from samuelmale May 9, 2024 07:34

vasharma05 previously requested changes May 13, 2024

View reviewed changes

packages/esm-patient-orders-app/src/api/api.ts Outdated Show resolved Hide resolved

usamaidrsk added 4 commits May 14, 2024 15:11

feat: send oders on creating encounter

c8a2491

refactor: undo unrelated changes

5c48954

refactor: remove unregister prop inline on inlineNotification

1508540

refactor: rename createEmptyEncounter

641f32f

usamaidrsk force-pushed the ft-send-orders-on-creating-enconter branch from 08f3f41 to 641f32f Compare May 14, 2024 12:27

usamaidrsk requested a review from vasharma05 May 15, 2024 12:40

Merge branch 'main' into ft-send-orders-on-creating-enconter

325079b

samuelmale approved these changes May 28, 2024

View reviewed changes

Merge branch 'main' into ft-send-orders-on-creating-enconter

9ccfab7

denniskigen reviewed May 28, 2024

View reviewed changes

usamaidrsk and others added 4 commits May 28, 2024 21:04

refactor: code cleanup

a9530a7

Merge remote-tracking branch 'origin/ft-send-orders-on-creating-encon…

e6cc766

…ter' into ft-send-orders-on-creating-enconter

refactor: move post orders on new encounter in common lib

f5e5969

Merge branch 'main' into ft-send-orders-on-creating-enconter

43a9b7b

usamaidrsk requested review from samuelmale and denniskigen May 28, 2024 20:06

brandones reviewed May 28, 2024

View reviewed changes

packages/esm-patient-common-lib/src/orders/postOrders.ts Outdated Show resolved Hide resolved

refactor: use postOrdersOnNewEncounter on the console.warn

165eaf3

brandones requested changes May 28, 2024

View reviewed changes

usamaidrsk added 2 commits May 29, 2024 00:08

refactor: remove unused function postOrders in api.ts

804f450

fix: fix encounterUuid type on prepLabOrderPostData and prepMedicatio…

8fe0c3a

…nOrderPostData

usamaidrsk requested a review from brandones May 28, 2024 21:12

usamaidrsk changed the title ~~(feat) O3-2117: Save orders and encounters in the same transaction~~ (feat) O3-2117: Save orders on a new encounter if there is no encounter May 28, 2024

brandones approved these changes May 28, 2024

View reviewed changes

Merge branch 'main' into ft-send-orders-on-creating-enconter

1e57db3

jayasanka-sack reviewed May 30, 2024

View reviewed changes

packages/esm-patient-orders-app/src/order-basket/order-basket.workspace.tsx Outdated Show resolved Hide resolved

jayasanka-sack mentioned this pull request May 30, 2024

Use local esm-patient-common-lib in E2E tests #1855

Draft

3 tasks

usamaidrsk added 2 commits May 30, 2024 14:56

fix: add conditions on error

987baff

Merge remote-tracking branch 'origin/ft-send-orders-on-creating-encon…

e8f2b3d

…ter' into ft-send-orders-on-creating-enconter

Merge branch 'main' into ft-send-orders-on-creating-enconter

10222dc

brandones merged commit c5cb4cc into openmrs:main May 31, 2024
5 of 6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(feat) O3-2117: Save orders on a new encounter if there is no encounter #1727

(feat) O3-2117: Save orders on a new encounter if there is no encounter #1727

usamaidrsk commented Mar 10, 2024 •

edited by jira bot

Loading

denniskigen left a comment

denniskigen Mar 19, 2024

usamaidrsk Mar 19, 2024

brandones May 28, 2024

vasharma05 left a comment

samuelmale left a comment

samuelmale May 28, 2024

denniskigen May 28, 2024

ibacher May 28, 2024

usamaidrsk May 28, 2024

denniskigen May 28, 2024

brandones commented May 28, 2024

brandones May 28, 2024

brandones left a comment

brandones commented May 29, 2024

jayasanka-sack commented May 29, 2024

brandones commented May 29, 2024

jayasanka-sack commented May 30, 2024

jayasanka-sack commented May 30, 2024

usamaidrsk commented May 30, 2024

brandones commented May 31, 2024

denniskigen commented Jun 4, 2024

ibacher commented Jun 4, 2024

		import { type OrderPost, useSystemVisitSetting, useVisitOrOfflineVisit } from '@openmrs/esm-patient-common-lib';
		import { orderBasketStore, type OrderBasketStore } from '@openmrs/esm-patient-common-lib/src/orders/store';

(feat) O3-2117: Save orders on a new encounter if there is no encounter #1727

(feat) O3-2117: Save orders on a new encounter if there is no encounter #1727

Conversation

usamaidrsk commented Mar 10, 2024 • edited by jira bot Loading

Requirements

Summary

Screenshots

Related Issue

Other

denniskigen 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

vasharma05 left a comment

Choose a reason for hiding this comment

samuelmale 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

brandones commented May 28, 2024

Choose a reason for hiding this comment

brandones left a comment

Choose a reason for hiding this comment

brandones commented May 29, 2024

jayasanka-sack commented May 29, 2024

brandones commented May 29, 2024

jayasanka-sack commented May 30, 2024

jayasanka-sack commented May 30, 2024

usamaidrsk commented May 30, 2024

brandones commented May 31, 2024

denniskigen commented Jun 4, 2024

ibacher commented Jun 4, 2024

usamaidrsk commented Mar 10, 2024 •

edited by jira bot

Loading