(fix)O3-2197: Unable to remove a patient from a list #757

jwnasambu · 2023-07-12T13:03:19Z

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.
I checked for feature overlap with existing widgets.

Summary

Created a overflow-menu to enable a user delete a patient from a given patient list

Screenshots

This is the output using post method

Uploading screencast 2023-08-23 4 PM-50-35.mp4…

This is the output using delete method

screencast.2023-08-07.10.PM-57-20.mp4

Related Issue

https://issues.openmrs.org/browse/O3-2197

Other

None.

ibacher · 2023-08-01T16:27:32Z

packages/esm-patient-list-app/src/patient-table/overflow-menu.component.tsx

+  const desktopLayout = isDesktop(layoutType);
+
+  // eslint-disable-next-line no-console
+  console.log(cohortMembershipUuid);


If you're logging for debugging purposes, it's probably better to put this in the callback than here.

ibacher · 2023-08-01T16:27:35Z

packages/esm-patient-list-app/src/api/api-remote.ts

+  return deleteData(
+    `${cohortUrl}/cohortmember/${cohortMembershipUuid}`,
+    {
+      voidReason: '',


We're not voiding the cohortMember, just adding an endDate to the record.

Call the deleteData instead of postData @jwnasambu .

@vasharma05 Deleting isn't the correct thing in this instance. Removing a patient from a cohort makes them no longer a cohort member, so their membership expires. It doesn't void the membership (that's a different operation).

@jwnasambu POST is better than DELETE; however, we need to post something more like the full existing record with an endDate (and remove the voidReason property)

Hi @ibacher and @dkayiwa !
I tried both the DELETE and POST methods, but POST request didn't make any change.
Do we need to use the DELETE or the POST method?
Thanks!

POST Request:

curl --location 'https://dev3.openmrs.org/openmrs/ws/rest/v1/cohortm/cohortmember/90fec1c6-8799-4c65-98ea-9053f1568457' \ --header 'Content-Type: application/json' \ --header 'Cookie: JSESSIONID=375DCF7704E139C04853E6277331448F' \ --data '{ "endDate": "2023-08-07T12:11:12.558Z" }'

P.S. DELETE method is deleting the cohortmember as expected.

@dkayiwa , any update here ☝️

https://issues.openmrs.org/browse/COH-54

ibacher · 2023-08-01T16:28:35Z

packages/esm-patient-list-app/src/patient-table/overflow-menu.component.tsx

+  cohortMembershipUuid: string;
+}
+
+const OverflowMenuComponent: React.FC<OverflowMenuComponentProps> = ({ cohortMembershipUuid }) => {


We should have a better name for this... overflow-menu.component sounds like its actually the overflow menu, not the object containing the overflow menu. Maybe overflow-menu-cell?

ibacher · 2023-08-01T16:29:25Z

packages/esm-patient-list-app/src/patient-table/overflow-menu.component.tsx

+  }, [cohortMembershipUuid, t]);
+
+  return (
+    <TableCell className="cds--table-column-menu">


You don't need a TableCell here since the component that uses this component already wraps it in one.

packages/esm-patient-list-app/src/patient-table/patient-table.component.tsx

vasharma05

Small corrections. otherwise looks good!

vasharma05 · 2023-08-02T11:41:04Z

packages/esm-patient-list-app/src/api/api-remote.ts

+  return deleteData(
+    `${cohortUrl}/cohortmember/${cohortMembershipUuid}`,
+    {
+      voidReason: '',


Call the deleteData instead of postData @jwnasambu .

vasharma05 · 2023-08-02T11:46:28Z

packages/esm-patient-list-app/src/patient-table/overflow-menu.component.tsx

+        ariaLabel="Remove patient from the list"
+        size={desktopLayout ? 'sm' : 'lg'}
+        flipped>
+        <OverflowMenuItem


Add an ExtensionSlot in here, with slot name = "cohort-member-action-items"

@vasharma05 Was the intention here to have the "Remove Patient" stuff as an extension?

ibacher · 2023-08-02T18:48:37Z

packages/esm-patient-list-app/src/api/api-remote.ts

+  return deleteData(
+    `${cohortUrl}/cohortmember/${cohortMembershipUuid}`,
+    {
+      voidReason: '',


@vasharma05 Deleting isn't the correct thing in this instance. Removing a patient from a cohort makes them no longer a cohort member, so their membership expires. It doesn't void the membership (that's a different operation).

@jwnasambu POST is better than DELETE; however, we need to post something more like the full existing record with an endDate (and remove the voidReason property)

ibacher · 2023-08-02T18:48:45Z

packages/esm-patient-list-app/src/api/api-remote.ts

+}
+
+export async function createPatientList(cohort: NewCohortDataPayload, ac = new AbortController()) {
+  return deleteData(


Suggested change

return deleteData(

return postData(

@ibacher and @vasharma05 thanks for the review let share the proposed changes shortly!

vasharma05 · 2023-08-07T12:17:03Z

packages/esm-patient-list-app/src/api/api-remote.ts

+  return deleteData(
+    `${cohortUrl}/cohortmember/${cohortMembershipUuid}`,
+    {
+      voidReason: '',


P.S. DELETE method is deleting the cohortmember as expected.

packages/esm-patient-list-app/src/patient-table/patient-table.component.tsx

jwnasambu · 2023-08-10T19:47:57Z

@ibacher Kindly free to review the proposed changes on this PR please!

packages/esm-patient-list-app/src/api/api-remote.ts

ibacher · 2023-09-20T12:42:31Z

packages/esm-patient-list-app/src/patient-table/overflow-menu.component.tsx

+        ariaLabel={`Remove patient from the ${cohortName} list`}
+        size={desktopLayout ? 'sm' : 'lg'}
+        flipped>
+        <ExtensionSlot name={'cohort-member-action-items'} />


Suggested change

<ExtensionSlot name={'cohort-member-action-items'} />

<ExtensionSlot name="cohort-member-action-items" />

ibacher · 2023-09-20T12:43:31Z

packages/esm-patient-list-app/src/patient-table/overflow-menu.component.tsx

+        ariaLabel="Remove patient from the list"
+        size={desktopLayout ? 'sm' : 'lg'}
+        flipped>
+        <OverflowMenuItem


@vasharma05 Was the intention here to have the "Remove Patient" stuff as an extension?

ibacher · 2023-09-20T12:44:01Z

packages/esm-patient-list-app/src/patient-table/patient-table.component.tsx

-          id: index,
+          id: patient.membershipUuid,


Why this change?

jwnasambu · 2023-09-20T16:38:56Z

@ibacher thanks for the review and your time! I will fix it shortly am in the middle of another ticket.

jwnasambu marked this pull request as draft July 12, 2023 13:30

ibacher reviewed Aug 1, 2023

View reviewed changes

jwnasambu force-pushed the (fix)O3-2197 branch 2 times, most recently from 6732c01 to 7bb76dd Compare August 1, 2023 21:37

vasharma05 reviewed Aug 2, 2023

View reviewed changes

jwnasambu force-pushed the (fix)O3-2197 branch from d340c64 to ad3bc72 Compare August 2, 2023 18:30

ibacher reviewed Aug 2, 2023

View reviewed changes

vasharma05 requested changes Aug 7, 2023

View reviewed changes

jwnasambu marked this pull request as ready for review August 8, 2023 11:33

jwnasambu force-pushed the (fix)O3-2197 branch from bb49cf2 to a450115 Compare August 10, 2023 19:34

jwnasambu marked this pull request as draft August 16, 2023 14:09

jwnasambu marked this pull request as ready for review August 16, 2023 16:08

Jexsie reviewed Aug 17, 2023

View reviewed changes

packages/esm-patient-list-app/src/api/api-remote.ts Show resolved Hide resolved

jwnasambu force-pushed the (fix)O3-2197 branch from e1c2d84 to e4eab9b Compare August 23, 2023 13:36

denniskigen requested review from ibacher, dkayiwa and vasharma05 September 20, 2023 12:02

ibacher reviewed Sep 20, 2023

View reviewed changes

denniskigen changed the title ~~(fix)O3-2197:Unable to remove a patient from a list~~ (fix)O3-2197: Unable to remove a patient from a list Sep 21, 2023

jwnasambu force-pushed the (fix)O3-2197 branch from fe42776 to 2fe3cb5 Compare October 3, 2023 13:26

jwnasambu marked this pull request as draft October 3, 2023 13:41

jwnasambu force-pushed the (fix)O3-2197 branch 2 times, most recently from a5cf005 to 4ad40f2 Compare October 3, 2023 13:54

(fix)O3-2197:Unable to remove a patient from a list

c8f859d

jwnasambu force-pushed the (fix)O3-2197 branch from 4ad40f2 to c8f859d Compare October 3, 2023 14:49

denniskigen mentioned this pull request Oct 3, 2023

(feat) Support for translations in the Home dashboard #820

Merged

3 tasks

jwnasambu closed this Oct 3, 2023

jwnasambu deleted the (fix)O3-2197 branch October 3, 2023 21:56

jwnasambu mentioned this pull request Oct 4, 2023

(fix) O3-2197: Add the ability to remove a patient from a list #832

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(fix)O3-2197: Unable to remove a patient from a list #757

(fix)O3-2197: Unable to remove a patient from a list #757

jwnasambu commented Jul 12, 2023 •

edited

Loading

ibacher Aug 1, 2023

ibacher Aug 1, 2023

vasharma05 Aug 2, 2023

ibacher Aug 2, 2023

vasharma05 Aug 7, 2023

vasharma05 Aug 7, 2023

vasharma05 Aug 10, 2023

dkayiwa Aug 24, 2023

ibacher Aug 1, 2023

ibacher Aug 1, 2023

vasharma05 left a comment

vasharma05 Aug 2, 2023

vasharma05 Aug 2, 2023

ibacher Sep 20, 2023

ibacher Aug 2, 2023

ibacher Aug 2, 2023

jwnasambu Aug 2, 2023

vasharma05 Aug 7, 2023

jwnasambu commented Aug 10, 2023

ibacher Sep 20, 2023

ibacher Sep 20, 2023

ibacher Sep 20, 2023

jwnasambu commented Sep 20, 2023

	<ExtensionSlot name={'cohort-member-action-items'} />
	<ExtensionSlot name="cohort-member-action-items" />

(fix)O3-2197: Unable to remove a patient from a list #757

(fix)O3-2197: Unable to remove a patient from a list #757

Conversation

jwnasambu commented Jul 12, 2023 • edited Loading

Requirements

Summary

Screenshots

Related Issue

Other

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

vasharma05 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

jwnasambu commented Aug 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwnasambu commented Sep 20, 2023

jwnasambu commented Jul 12, 2023 •

edited

Loading