Examiner UI - Application Details - Assign and Unassign#669
Examiner UI - Application Details - Assign and Unassign#669shaangill025 merged 5 commits intobcgov:mainfrom
Conversation
|
Temporary Url for review: https://strr-examiner-dev--pr-669-lxwb50p1.web.app |
|
@andyyanggov This is ready for UXA. |
|
@shaangill025 A few discrepancies I noticed:
|
dimak1
left a comment
dimak1
left a comment
There was a problem hiding this comment.
Nice work, thanks for the tests! Added few comments, and maybe we can connect to go over some of the code. Thank you
| } | ||
|
|
||
| const props = withDefaults(defineProps<Props>(), { | ||
| confirmButtonText: 'Confirm', |
There was a problem hiding this comment.
Should use translation here
| /** | ||
| * Composable for managing action modals | ||
| */ | ||
| export const useConfirmationModal = () => { |
There was a problem hiding this comment.
Not sure if we need this composable... Nothing extraordinary is happening here. We can pass the labels and actions directly to ConfirmationModal component.
There was a problem hiding this comment.
This is needed to avoid code duplication, otherwise this will be replicated in both application and registration details along with AssignmentActions component.
There was a problem hiding this comment.
Has been reworked based on our discussion.
| if (examinerActions && examinerActions.length > 0) { | ||
| const leftButtons: ConnectBtnControlItem[] = [] | ||
| const rightButtons: ConnectBtnControlItem[] = [] | ||
| const currentButtons = mergeWithExisting ? getButtonControl() : { leftButtons: [], rightButtons: [] } |
There was a problem hiding this comment.
Maybe you can walk me thru this piece.
There was a problem hiding this comment.
Sure, this has been updated in commit and cleaned up.
|
/gcbrun |
This comment was marked as outdated.
This comment was marked as outdated.
@andyyanggov clarified that assign or unassign operation should be allowed on all application statuses including approved (end states)
Done, email body is hidden and action buttons disabled in application and registration details.
Done, this should be fully functional once the follow-up API PR (above) is merged. Also includes a confirmation modal for cancel action. |
Signed-off-by: Shaanjot Gill <gill.shaanjots@gmail.com>
Signed-off-by: Shaanjot Gill <gill.shaanjots@gmail.com>
… app statuses Signed-off-by: Shaanjot Gill <gill.shaanjots@gmail.com>
Signed-off-by: Shaanjot Gill <gill.shaanjots@gmail.com>
This comment was marked as outdated.
This comment was marked as outdated.
Signed-off-by: Shaanjot Gill <gill.shaanjots@gmail.com>
|
|
Temporary Url for review: https://strr-examiner-dev--pr-669-lxwb50p1.web.app |
dimak1
left a comment
dimak1
left a comment
There was a problem hiding this comment.
We can make few improvements after. Thanks for addressing earlier comments. 👍🏻
| watch( | ||
| () => [activeHeader.value?.applicationNumber, activeHeader.value?.reviewer?.username], | ||
| async ([applicationNumber, _]) => { | ||
| try { |
There was a problem hiding this comment.
No need of the try/catch block here because isCurrentUserAssignee is already handling errors and returning false so isCurrentUserAssignee could be called and assigned right away.
| <AssignmentActions :is-registration-page="true" @refresh="refresh" /> | ||
| <ConfirmationModal | ||
| ref="confirmErrorModal" | ||
| :is-open="false" |
There was a problem hiding this comment.
This is-open should be conditional.
| approve: { | ||
| action: (id: string) => handleApplicationAction(id, ApplicationActionsE.APPROVE, 'right', 1), | ||
| label: t('btn.approve') | ||
| action: (id: string) => handleAssigneeAction(id, ApplicationActionsE.APPROVE, 'right', 1), |
There was a problem hiding this comment.
Nested functions could be reduced to just checking isAssignedToUser.value and calling original handleApplicationAction() or open confirm modal.
There was a problem hiding this comment.
I will address these comments during next sprint.
|
@shaangill025 Looks good to me. Good work! |
4 participants
Issue:
Description of changes:
#663 needs to be merged first.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the BC Registry and Digital Services BSD 3-Clause License