-
Notifications
You must be signed in to change notification settings - Fork 1
TD_2987_Code_Changes for Developing the Change Role Feature on the self assessment interface within the Supervisor module for the users. #3415
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
base: DLS-Release-v1.2.4
Are you sure you want to change the base?
Conversation
…lf assessment interface for the user within the supervisor module.
| public List<int> GetCandidateAssessmentSupervisorVerifications(int supervisorDelegateId) | ||
| { | ||
| var verificationIds = new List<int>(); | ||
| verificationIds = [.. connection.Query<int>( |
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.
is this a typo here?
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's not a typo, [..] is a list shorthand operator. Hope you were referring to this.
| } | ||
| if (model.SupervisorRoleOptions.SelectedValue == "RemoveAsSupervisor") | ||
| { | ||
| var removed = supervisorService.RemoveCandidateAssessmentSupervisor(model.CandidateAssessmentID, model.SupervisorDelegateID); |
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.
variable assigned but never used?
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 have removed it.
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.
In the first screenshot, for step2 we see Role as supervising but still see an action for Supervise?
Please see a couple of other inline comments as well.
rshrirohit
left a comment
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.
Auldrin-Possa
left a comment
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.
-
Supervisor.cs - Code logic to display role seems to be incorrect. @kevwhitt-hee Please have a look.
-
'Change role' action link show/hide - logic incomplete and not based on Ireland release. This code may not have been tested in the Ireland release branch. Further testing was not possible locally as the Change Role link was not visible.
… comments to the code, changing the selfassessmentsupervisorRoleIds on change role feature.
|
I have tried to make changes in order to accommodate the comments received from @rshrirohit and @Auldrin-Possa. There seems to be an existing bug with the supervisor role and yet displaying the 'Supervise' action link for that profile assessment which unfortunately, I couldn't undertake. |

JIRA link
TD-2987
Description
_Using this feature, user will be able to change their respective roles available for a specific self assessment, they would be able to choose whether they are 'Educator/Managers' or 'Assessors' or 'Remove Them As Supervisors from the Self Assessment'.
Screenshots
Developer checks
(Leave tasks unticked if they haven't been appropriate for your ticket.)
I have:
Either:
Or: