OBPIH-5806 Update with PENDING_APPROVAL status on approval submit and REQUESTED on reagular outbound #4317
Conversation
There was a problem hiding this comment.
answering your question - it was probably missed because we forgot that those methods are also used for outbounds, and we focused on conditions (if a location does/does not support approvals).
It is probably my fault that I have caused this regression, so I'm sorry that you had to fix that 🙈
| } else { | ||
| transitionRequisitionStatus(requisition, RequisitionStatus.VERIFYING, EventCode.SUBMITTED, currentUser) | ||
| requisition.approvalRequired = false | ||
| } |
There was a problem hiding this comment.
why have you got rid of those lines?
If you changed on frontend to send PENDING_APPROVAL no matter of location supported activities, if a location doesn't support the approvals it would set the status for PENDING_APPROVAL not for VERIFYING, wouldn't it?
There was a problem hiding this comment.
yes, good point., this is what I wanted to hear, because I had a feeling I was missing something.
I'll bring back those lines and leave the VERYFING case too.
| @@ -1229,7 +1229,7 @@ class AddItemsPage extends Component { | |||
| submitRequest(lineItems) { | |||
| const nonEmptyLineItems = _.filter(lineItems, val => !_.isEmpty(val) && val.product); | |||
| this.saveRequisitionItems(nonEmptyLineItems) | |||
| .then(() => this.transitionToNextStep('REQUESTED')); | |||
| .then(() => this.transitionToNextStep('PENDING_APPROVAL')); | |||
There was a problem hiding this comment.
see the comment above - either you bring back those lines in the PENDING_APPROVAL case, or we should should check (or actually we might do it in both places) here, if a location supports approval workflow - if it does, send PENDING_APPROVAL, otherwise after your refactor, I guess we should send VERIFYING directly, instead of REQUESTED.
| } else { | ||
| transitionRequisitionStatus(requisition, RequisitionStatus.VERIFYING, EventCode.SUBMITTED, currentUser) | ||
| requisition.approvalRequired = false | ||
| break |
There was a problem hiding this comment.
now - don't we want to send an event at all if we set it for VERIFYING? (when we send a request when origin doesn't support approvals)?
There was a problem hiding this comment.
what do you mean?
the break is inside of an if() and if requisition.origin.approvalRequired is false it will go to next case which is case RequisitionStatus.VERIFYING:.
I wanted to avoid duplicate code with this approach but if you feel it is too confusing then let me know, I'll either add a comment or duplicate it for the sake of clarity.
There was a problem hiding this comment.
yes, it will go there, but from the frontend you still always send PENDING_APPROVAL and if a location doesn't support the approvals, it won't run the transitionRequisitionStatus method at all. Or am I missing something?
There was a problem hiding this comment.
it will run triggerRequisitionStatusTransition and set the status to VERIFYING.
Anyways, these state transitions are getting a bit too confusing, I think I need to rethink my approach to which status to send and how to react to it.
| .then(() => this.transitionToNextStep('REQUESTED')); | ||
| .then(() => { | ||
| if (supports(this.state.values.origin?.supportedActivities, ActivityCode.APPROVE_REQUEST)) { | ||
| this.transitionToNextStep('PENDING_APPROVAL'); |
There was a problem hiding this comment.
let's use the enum values defines in the requisitonStatus.js
| if (supports(this.state.values.origin?.supportedActivities, ActivityCode.APPROVE_REQUEST)) { | ||
| this.transitionToNextStep('PENDING_APPROVAL'); | ||
| } else { | ||
| this.transitionToNextStep('REQUESTED'); |
… REQUESTED on reagular outbound
…ported activity on submit request
drodzewicz
force-pushed
the
OBPIH-5806-approval-and-reject-action-approvals-end-to-end-workflow
branch
from
1d0607d
to
0512892
Compare
awalkowiak
deleted the
OBPIH-5806-approval-and-reject-action-approvals-end-to-end-workflow
branch
… REQUESTED on reagular outbound (#4317) * OBPIH-5806 Update with PENDING_APPROVAL status on approval submit and REQUESTED on reagular outbound * OBPIH-5806 check if origin requires approval on PENDING_APPROVAL status * OBPIH-5806 Send propper status update based on fulfiling location supported activity on submit request * OBPIH-5806 Use constant status values for RequisitionStatus
lets discuss...
The issue came up when trying to create an outbound with a location which has the supported activity for request approval.
The expected behaviour would be that on Edit step an outbound should have status
VERIFYINGbut instead it wasWAITING FOR APPROVAL.The reason for that was that in both cases (Request and Outbound) we update status with
{ status: 'REQUESTED' }which is mapped toPENDING_APPROVAL(change which was introduced in here)Was there a particular reason why we didn't separate VERIFYING and PEWNDING_APPROVAL as individual cases?