5139 delete response requisition lines #5226

fergie-nz · 2024-10-24T21:17:23Z

👩🏻‍💻 What does this PR do?

Adds delete response requisition lines & line back end service and graphql layers.

Adds front end api for batch delete requisition lines.

💌 Any notes for the reviewer?

Have added delete singular response requisition lines but doesn't expose this in API (similar to request requisition lines).
We have the singular delete, but all deletes are handled through the batch delete.

Adds tests, but graphql delete lines are a little limited.

🧪 Testing

Go to distribution -> requisitions
select some lines
delete via drop down
(You might need to create lines in the DB as no create lines method currently exists. Or you can reassign lines from another (request) requisition for data to delete.

📃 Documentation

Part of an epic: documentation will be completed for the feature as a whole

minus graphql tests

Limited to success and not found. Quite limited and unsure of testing comprehensively here

Rename draft response requisition to new Restructure error handling to prioritise not request requisition

fergie-nz · 2024-10-24T21:18:44Z

client/packages/requisitions/src/ResponseRequisition/DetailView/Toolbar/ToolbarDropDown.tsx

+import React from 'react';
+import {
+  DropdownMenu,
+  DropdownMenuItem,
+  DeleteIcon,
+  useTranslation,
+} from '@openmsupply-client/common';
+import { useResponse } from '../../api';
+
+interface ToolbarDropDownProps {
+  isDisabled: boolean;
+}
+
+export const ToolbarDropDown = ({ isDisabled }: ToolbarDropDownProps) => {
+  const t = useTranslation();
+  const onDelete = useResponse.line.delete();
+
+  return (
+    <DropdownMenu label={t('label.actions')}>
+      <DropdownMenuItem
+        IconComponent={DeleteIcon}
+        onClick={onDelete}
+        disabled={isDisabled}
+      >
+        {t('button.delete-lines', { ns: 'distribution' })}
+      </DropdownMenuItem>
+    </DropdownMenu>
+  );
+};


Moved into its own file to duplicate RequestRequisition pattern

fergie-nz · 2024-10-24T21:19:25Z

client/packages/requisitions/src/ResponseRequisition/api/api.ts

+  deleteLine: async (responseLineId: string) => {
+    const ids = [{ id: responseLineId }];
+    const result = await sdk.deleteResponseLines({ ids, storeId });
+
+    if (result.batchResponseRequisition.deleteResponseRequisitionLines) {
+      const failedLines =
+        result.batchResponseRequisition.deleteResponseRequisitionLines.filter(
+          line =>
+            line.response.__typename === 'DeleteResponseRequisitionLineError'
+        );
+      if (failedLines.length === 0) {
+        return result.batchResponseRequisition.deleteResponseRequisitionLines;
+      }
+    }


This uses the batch delete api even for singular.

Not currently used, but will be used for edit single line in future

We don't need this. It's fine to use the deleteLines batch deletion to delete singular lines as well. Can you remove it?

server/graphql/requisition_line/src/lib.rs

fergie-nz · 2024-10-24T21:22:48Z

server/service/src/requisition/request_requisition/delete.rs

-    if requisition_row.status != RequisitionStatus::Draft {
-        return Err(OutError::CannotEditRequisition);
-    }
-
    if requisition_row.r#type != RequisitionType::Request {
        return Err(OutError::NotARequestRequisition);
    }
+
+    if requisition_row.status != RequisitionStatus::Draft {
+        return Err(OutError::CannotEditRequisition);
+    }


I've rearranged error handling for mutating requisitions throughout.

To me this makes more sense returning the wrong type of requisition error before returning a status error.

server/service/src/requisition/request_requisition/update/test.rs

github-actions · 2024-10-24T21:29:09Z

Bundle size difference

Comparing this PR to main

Old size	New size	Diff
4.87 MB	4.87 MB	1.46 KB (0.03%)

roxy-dao

Thanks Ferg (: Excellent job on the deletes~

I just have a few comments below that need to be addressed before the PR can be approved (:

RE::StandUP: Create carryover issue to not allow deletion of lines if they are in a linked shipment (requires extra logic)

client/packages/requisitions/src/ResponseRequisition/DetailView/Toolbar/ToolbarDropDown.tsx

roxy-dao · 2024-10-24T21:49:46Z

client/packages/requisitions/src/ResponseRequisition/api/api.ts

+  deleteLine: async (responseLineId: string) => {
+    const ids = [{ id: responseLineId }];
+    const result = await sdk.deleteResponseLines({ ids, storeId });
+
+    if (result.batchResponseRequisition.deleteResponseRequisitionLines) {
+      const failedLines =
+        result.batchResponseRequisition.deleteResponseRequisitionLines.filter(
+          line =>
+            line.response.__typename === 'DeleteResponseRequisitionLineError'
+        );
+      if (failedLines.length === 0) {
+        return result.batchResponseRequisition.deleteResponseRequisitionLines;
+      }
+    }


We don't need this. It's fine to use the deleteLines batch deletion to delete singular lines as well. Can you remove it?

client/packages/requisitions/src/ResponseRequisition/api/hooks/line/useDeleteResponseLines.ts

server/graphql/requisition_line/src/lib.rs

roxy-dao · 2024-10-24T22:06:40Z

server/service/src/requisition_line/response_requisition_line/delete.rs

+    NotThisStoreRequisition,
+    NotAResponseRequisition,
+    RequisitionDoesNotExist,
+    CannotEditRequisition,


Can you add a CannotDeleteTransferredRequisition er or some better naming... This needs to check that the requisition is not created from an Internal Order

server/service/src/requisition_line/response_requisition_line/delete.rs

roxy-dao · 2024-10-24T22:16:29Z

Oh! And the postgres tests are failing, can you fix this (:

And can you change the deletion message when you cannot delete #5128 should be a small fix

…delete.rs Co-authored-by: roxy-dao <[email protected]>

roxy-dao

Thanks for the changes Ferg! I just have a few more comments and have put them in this example branch (:

roxy-dao · 2024-10-28T19:19:35Z

client/packages/requisitions/src/ResponseRequisition/DetailView/Toolbar/Toolbar.tsx

-            {t('button.delete-lines', { ns: 'distribution' })}
-          </DropdownMenuItem>
-        </DropdownMenu>
+        <ToolbarDropDown isDisabled={isDisabled || !!theirReference} />


The user can enter in theirReference, so we shouldn't be using this for the check. Their is a linkedRequisition node that gets returned which will tell us if the Requisition was created as a result of an Internal Order.

roxy-dao · 2024-10-28T19:20:55Z

client/packages/requisitions/src/ResponseRequisition/DetailView/Toolbar/ToolbarDropDown.tsx

+import { useResponse } from '../../api';
+
+interface ToolbarDropDownProps {
+  isDisabled: boolean;


I would pass in the linkedRequisition here as a check just in case we add other actions to Requisition and having it this way will help differentiate between the two

roxy-dao · 2024-10-28T19:21:13Z

client/packages/requisitions/src/ResponseRequisition/DetailView/Toolbar/ToolbarDropDown.tsx

+        onClick={onDelete}
+        disabled={isDisabled}
+      >
+        {t('button.delete-lines', { ns: 'distribution' })}


We don't need to use namespace anymore since all the translations are in one common.json file

client/packages/requisitions/src/ResponseRequisition/api/hooks/line/useDeleteResponseLines.ts

fergie-nz · 2024-10-29T01:03:20Z

Thanks for the changes Ferg! I just have a few more comments and have put them in this example branch (:

Awesome thank you Roxy! I have added those.

roxy-dao

Awesome! Thanks for all the changes Ferg /o/

fergie-nz added 13 commits October 24, 2024 12:57

Add delete response requisition service layer

949d692

Add service layer tests

18a745b

Add graphql layer delete response requisition line

a9bb801

Add check for status as per update response requisition line

88953a6

Add tests to graphql layer

32c14fb

Add front end deleteResponseLines

fcad1b2

Add batch mutations service layer

f743641

Add batch mutations graphql layer

5772374

minus graphql tests

Add individual line delete (using batch mutation on back end)

ccb8f4b

Add graphql tests

e6ec2a2

Limited to success and not found. Quite limited and unsure of testing comprehensively here

Merge branch 'develop' into 5139-delete-response-requisition

b5d434e

Regenerate schema

e6406a7

Fix tests

322715b

Rename draft response requisition to new Restructure error handling to prioritise not request requisition

github-actions bot added enhancement New feature or request Team Ruru 🦉 Roxy, Ferg, Noel feature: manual requisition labels Oct 24, 2024

fergie-nz commented Oct 24, 2024

View reviewed changes

roxy-dao self-assigned this Oct 24, 2024

fergie-nz commented Oct 24, 2024

View reviewed changes

server/graphql/requisition_line/src/lib.rs Outdated Show resolved Hide resolved

fergie-nz commented Oct 24, 2024

View reviewed changes

server/service/src/requisition/request_requisition/update/test.rs Show resolved Hide resolved

Restructure order of errors

1d668b0

roxy-dao requested changes Oct 24, 2024

View reviewed changes

roxy-dao linked an issue Oct 24, 2024 that may be closed by this pull request

Cannot delete lines - error message could be clearer #5128

Closed

roxy-dao changed the title ~~5139 delete response requisition~~ 5139 delete response requisition lines Oct 24, 2024

github-actions bot added this to the V2.4.0 milestone Oct 24, 2024

github-actions bot added bug Something is borken Severity: Low Bugs that don't block workflows. UX like wrong fonts, bad layout labels Oct 24, 2024

fergie-nz and others added 9 commits October 25, 2024 11:27

Remove redundant delete line from api

07df943

Add check for request requisition reference

c5f479b

Update server/service/src/requisition_line/response_requisition_line/…

d933c3b

…delete.rs Co-authored-by: roxy-dao <[email protected]>

Add check for transferred requisition

49c32da

Don't make map_response public

206d377

Passing in wrong type for isResponseDisabled check

8e8b2bb

Expose linkedRequisition node

be7022a

Use linkedRequisition for disabled check

010e33c

Disambiguiating map_errors

e1c3e4d

roxy-dao requested changes Oct 28, 2024

View reviewed changes

roxy-dao and others added 2 commits October 29, 2024 10:51

Fix test

83d98e2

Merge branch '5139-example' into 5139-delete-response-requisition

b685bb2

fergie-nz requested a review from roxy-dao October 29, 2024 01:03

roxy-dao approved these changes Oct 29, 2024

View reviewed changes

fergie-nz merged commit 8e94318 into develop Oct 29, 2024
6 checks passed

fergie-nz deleted the 5139-delete-response-requisition branch October 29, 2024 03:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

5139 delete response requisition lines #5226

5139 delete response requisition lines #5226

fergie-nz commented Oct 24, 2024

fergie-nz Oct 24, 2024

fergie-nz Oct 24, 2024

roxy-dao Oct 24, 2024

fergie-nz Oct 24, 2024

github-actions bot commented Oct 24, 2024 •

edited

Loading

roxy-dao left a comment •

edited

Loading

roxy-dao Oct 24, 2024

roxy-dao Oct 24, 2024

roxy-dao commented Oct 24, 2024 •

edited

Loading

roxy-dao left a comment

roxy-dao Oct 28, 2024

roxy-dao Oct 28, 2024

roxy-dao Oct 28, 2024

fergie-nz commented Oct 29, 2024

roxy-dao left a comment

5139 delete response requisition lines #5226

5139 delete response requisition lines #5226

Conversation

fergie-nz commented Oct 24, 2024

👩🏻‍💻 What does this PR do?

💌 Any notes for the reviewer?

🧪 Testing

📃 Documentation

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

github-actions bot commented Oct 24, 2024 • edited Loading

Bundle size difference

roxy-dao left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roxy-dao commented Oct 24, 2024 • edited Loading

roxy-dao 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

fergie-nz commented Oct 29, 2024

roxy-dao left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 24, 2024 •

edited

Loading

roxy-dao left a comment •

edited

Loading

roxy-dao commented Oct 24, 2024 •

edited

Loading