-
-
Notifications
You must be signed in to change notification settings - Fork 392
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OBPIH-5809 Approval rollback - Approvals end to end workflow (fix after QA) #4335
Conversation
@@ -90,7 +90,11 @@ | |||
<g:isUserInAllRoles location="${stockMovement?.origin?.id}" roles="${[RoleType.ROLE_REQUISITION_APPROVER]}"> | |||
<g:set var="userHasRequestApproverRole" value="${true}"/> | |||
</g:isUserInAllRoles> | |||
<g:set var="isApprovalRequired" value="${stockMovement?.requisition?.approvalRequired && stockMovement?.origin?.isApprovalRequired()}" /> | |||
<g:set var="isApprovalRequired" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also could be moved to the StockMovement
<g:set var="isApprovalRequired" | ||
value="${stockMovement?.requisition?.approvalRequired && | ||
stockMovement?.origin?.isApprovalRequired() && | ||
stockMovement.status < RequisitionStatus.PICKING}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this is dangerous - won't it compare the statuses chronologically?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is compared by sortOrder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure? it's compare
method in the RequisitionStatus
which is not the same as compareTo
method available in Comparator
interface. Those are not the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alannadolny yup please make sure about that one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This topic was previously answered by Justin: #4133 (comment)
<g:set var="isApprovalRequired" value="${stockMovement?.requisition?.approvalRequired && stockMovement?.origin?.isApprovalRequired()}" /> | ||
<g:set var="isApprovalRequired" | ||
value="${stockMovement?.requisition?.approvalRequired && | ||
stockMovement?.origin?.isApprovalRequired() && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stockMovement?.origin?.isApprovalRequired() && | |
stockMovement?.origin?.approvalRequired && |
…er QA) (#4335) * OBPIH-5809 Fix throwing error with missing required argument * OBPIH-5809 Show buttons when stock movement status is higher than approved / rejected * OBPIH-5809 Fixes after review * OBPIH-5809 Fixes after review * OBPIH-5809 Add comments
No description provided.