-
Notifications
You must be signed in to change notification settings - Fork 1
Heedls 629 group delegates small UI amendment on remove delegate page #692
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
Heedls 629 group delegates small UI amendment on remove delegate page #692
Conversation
...rningSolutions.Web/Views/TrackingSystem/Delegates/DelegateGroups/GroupDelegatesRemove.cshtml
Outdated
Show resolved
Hide resolved
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.
Couple of minor changes on the page styling, and a potentially more significant change for the data service method.
Also you are going to want to merge master into this branch so that the accessibility tests pass on Jenkins.
...rningSolutions.Web/Views/TrackingSystem/Delegates/DelegateGroups/GroupDelegatesRemove.cshtml
Outdated
Show resolved
Hide resolved
...rningSolutions.Web/Views/TrackingSystem/Delegates/DelegateGroups/GroupDelegatesRemove.cshtml
Outdated
Show resolved
Hide resolved
|
Actually, I've realised the controller needs changes as well. Judging from the wording of the ticket, and the wording on the page, we should be calling that dataservice method when the second checkbox is unchecked as well. It is just when the checkbox is checked, then we should no longer care about the login count. |
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.
Looking pretty good, just a few minor things.
DigitalLearningSolutions.Data/DataServices/GroupsDataService.cs
Outdated
Show resolved
Hide resolved
DigitalLearningSolutions.Data/DataServices/GroupsDataService.cs
Outdated
Show resolved
Hide resolved
...ingSolutions.Web.Tests/Controllers/TrackingSystem/Delegates/DelegateGroupsControllerTests.cs
Outdated
Show resolved
Hide resolved
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.
Some quick renaming that I've spotted, but it should be good after that.
...ingSolutions.Web.Tests/Controllers/TrackingSystem/Delegates/DelegateGroupsControllerTests.cs
Show resolved
Hide resolved
DigitalLearningSolutions.Data.Tests/DataServices/GroupsDataServiceTests.cs
Outdated
Show resolved
Hide resolved
...rningSolutions.Web/Views/TrackingSystem/Delegates/DelegateGroups/GroupDelegatesRemove.cshtml
Outdated
Show resolved
Hide resolved
DigitalLearningSolutions.Web/Controllers/TrackingSystem/Delegates/DelegateGroupsController.cs
Outdated
Show resolved
Hide resolved
...arningSolutions.Data.Tests/Services/GroupServiceTests/GroupsServiceSynchroniseGroupsTests.cs
Outdated
Show resolved
Hide resolved
...arningSolutions.Data.Tests/Services/GroupServiceTests/GroupsServiceSynchroniseGroupsTests.cs
Outdated
Show resolved
Hide resolved
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.
Looks good! Feel free to merge.
JIRA link
https://softwiretech.atlassian.net/browse/HEEDLS-629
Description
Added new text as per wireframe,
Updated exiting text
Removed LoginCount=0 when removing progress records since the progress records can be deleted where course is started but not competed.
Screenshots
Developer checks
(Leave tasks unticked if they haven't been appropriate for your ticket.)
I have: