5242 add delete response requisitions #5269
Conversation
github-actions
bot
added
enhancement
|
|
||
| // response requisition errors | ||
|
|
||
| pub struct FinalisedRequisition; | ||
| #[Object] | ||
| impl FinalisedRequisition { | ||
| pub async fn description(&self) -> &str { | ||
| "Response requisition has already been finalised" | ||
| } | ||
| } | ||
|
|
||
| pub struct TransferRequisition; | ||
| #[Object] | ||
| impl TransferRequisition { | ||
| pub async fn description(&self) -> &str { | ||
| "Cannot delete a response requisition transferred from a request requisition" | ||
| } | ||
| } | ||
|
|
||
| pub struct RequisitionWithShipment; | ||
| #[Object] | ||
| impl RequisitionWithShipment { | ||
| pub async fn description(&self) -> &str { | ||
| "Cannot delete a response requisition once a shipment has been generated" | ||
| } | ||
| } |
There was a problem hiding this comment.
These errors won't propagate to UI currently. We need error handling for both response and request requisitions
Bundle size differenceComparing this PR to
|
client/packages/requisitions/src/ResponseRequisition/ListView/ListView.tsx
Outdated
Show resolved
Hide resolved
client/packages/requisitions/src/ResponseRequisition/api/api.ts
Outdated
Show resolved
Hide resolved
server/graphql/requisition/src/mutations/response_requisition/delete.rs
Outdated
Show resolved
Hide resolved
…ListView.tsx Co-authored-by: roxy-dao <[email protected]>
…b.com/msupply-foundation/open-msupply into 5242-add-delete-response-requisitions
| result.forEach(line => { | ||
| if (line.response.__typename == 'DeleteResponseRequisitionError') { | ||
| info(line.response.error.description)(); | ||
| errorMessages.push(line.response.error.description); | ||
| } | ||
| }); | ||
| if (errorMessages.length > 0) { | ||
| throw new Error(); | ||
| } |
There was a problem hiding this comment.
I've added a check here for errors on delete action to throw errors for each failed delete with structure rather than one error with the generic hook.
However it does leave an extra snack when error handling. But figure that is OK rather than more extended refactoring
There was a problem hiding this comment.
I'm getting double toast on error
client/packages/common/src/hooks/useDeleteConfirmation/useDeleteConfirmation.ts
Outdated
Show resolved
Hide resolved
client/packages/requisitions/src/ResponseRequisition/ListView/ListView.tsx
Outdated
Show resolved
Hide resolved
client/packages/requisitions/src/ResponseRequisition/api/api.ts
Outdated
Show resolved
Hide resolved
| result.forEach(line => { | ||
| if (line.response.__typename == 'DeleteResponseRequisitionError') { | ||
| info(line.response.error.description)(); | ||
| errorMessages.push(line.response.error.description); | ||
| } | ||
| }); | ||
| if (errorMessages.length > 0) { | ||
| throw new Error(); | ||
| } |
There was a problem hiding this comment.
I'm getting double toast on error
…ListView.tsx Co-authored-by: roxy-dao <[email protected]>
Nice yeah I had added that. I thought that might have been better than only showing one toast when there were multiple errors. But have removed. I think it might be helpful in future to allow useDeleteConfirmation hook to accept an array of errors to propagate them all, rather than the first one which is caught so that the user has to iterate through a trial and error process if there are more than one error across multiple lines |
| switch (line.response.error.__typename) { | ||
| case 'FinalisedRequisition': | ||
| throw Error(t('messages.cannot-delete-finalised-requisition')); | ||
| case 'RecordNotFound': | ||
| throw Error(t('messages.record-not-found')); | ||
| case 'RequisitionWithShipment': | ||
| throw Error(t('messages.cannot-delete-requisition-with-shipment')); | ||
| case 'TransferRequisition': | ||
| throw Error(t('messages.cannot-delete-transfer-requisition')); |
There was a problem hiding this comment.
these don't actually get propped...
| pub enum DeleteErrorInterface { | ||
| RecordNotFound(RecordNotFound), | ||
| FinalisedRequisition(FinalisedRequisition), | ||
| TransferRequisition(TransferRequisition), |
| Some(service_provider(test_service, &connection_manager)) | ||
| ); | ||
|
|
||
| // line delete error ? |
Fixes #5242
👩🏻💻 What does this PR do?
Adds delete for response requisitions
Matches pattern of request requisition with slightly different validation.
Note the front end error handling for both response and request requisitions isn't quite right and will return all is OK when structured errors are returned.
I've made a separate issue for structured error handling as changes are required for both request and response error handling. (#5270)
💌 Any notes for the reviewer?
I think also missing is a check for linked shipments before being able to select and delete.
I wonder should requisitions be disabled once a shipment has been created? Currently this doesn't include checks for this.
🧪 Testing
📃 Documentation