OBPIH-5816 approval and reject actions on list page approvals lists pages #4319
Conversation
drodzewicz
force-pushed
the
OBPIH-5816-approval-and-reject-actions-on-list-page-approvals-lists-pages
branch
2 times, most recently
from
39505b8
to
45df840
Compare
drodzewicz
force-pushed
the
OBPIH-5816-approval-and-reject-actions-on-list-page-approvals-lists-pages
branch
from
45df840
to
3549248
Compare
| }; | ||
| actions.push(approveAction); | ||
| } | ||
| if (statusCode === RequisitionStatus.PENDING_APPROVAL) { |
There was a problem hiding this comment.
you can move the content of that if into the if above, since those are the same "ifs"
| }; | ||
|
|
||
| const rollbackRequest = () => { | ||
| Alert.error('Not implemented'); |
There was a problem hiding this comment.
will it be handled in another ticket or are you waiting for it to be merged?
There was a problem hiding this comment.
I am waiting for the changes to be merged.
I can either wait for it or add it in the following PR.
| @@ -90,6 +91,7 @@ export default function (state = initialState, action) { | |||
| isSuperuser: _.get(action, 'payload.data.data.isSuperuser'), | |||
| isUserAdmin: _.get(action, 'payload.data.data.isUserAdmin'), | |||
| isUserApprover: _.get(action, 'payload.data.data.isUserApprover', false), | |||
| isUserRequestApprover: _.get(action, 'payload.data.data.isUserRequestApprover', false), | |||
There was a problem hiding this comment.
whenever I see this ugly _.get I wish we refactored it in the future 🙏
to something like:
const { isUserRequestApprover, ... } = payload.data.data
...
isUserApprover: isUserApprover ?? false
| try { | ||
| await stockMovementApi.updateStatus(id, RequisitionStatus.REJECTED); | ||
| const successMessage = `You have successfully Rejected the request ${identifier}`; | ||
| Alert.success(successMessage); |
There was a problem hiding this comment.
and a question why don't you use our new notifications?
notification(NotificationType.SUCCESS)({
message: <message>
})There was a problem hiding this comment.
I simply used the same Alert as we have in this file
There was a problem hiding this comment.
because notification was implemented later this is why in this file Alert was used. I think we should use notification, since it lets us do more and this should become a convention. (I know in this case it would evaluate to the same, but just not to mix Alert vs notification.
drodzewicz
force-pushed
the
OBPIH-5816-approval-and-reject-actions-on-list-page-approvals-lists-pages
branch
from
8a972f6
to
14052ed
Compare
awalkowiak
deleted the
OBPIH-5816-approval-and-reject-actions-on-list-page-approvals-lists-pages
branch
…ages (#4319) * OBPIH-5816 Return isUserRequestApprover from config endpoint * OBPIH-5816 Add approval actions in the request list * OBPIH-5816 Call status update for approved and rejected actions * OBPIH-5816 Fix tests * OBPIH-5816 use notification util function to render notifications * OBPIH-5816 Fix API context test
No description provided.