-
Notifications
You must be signed in to change notification settings - Fork 1
Develop/fixes/dlsv2 687 page 500 error on choose working group members screen when given user email not associated with dls account #1431
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
Develop/fixes/dlsv2 687 page 500 error on choose working group members screen when given user email not associated with dls account #1431
Conversation
|
So I decided not to use the view Component specifically created for displaying error such as this because the field doesn't belong to a strongly typed model where error messages and validation messages could be added. |
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.
See changes requested in code.
| } | ||
| @if (Model.Error) | ||
| { | ||
| @*<vc:error-summary order-of-property-names="@(new string[] { "userEmail" })" />*@ |
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.
We should be using the error summary view component here. It contains all of the markup included by hand below.
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.
fixed
| else if (collaboratorId == -4) | ||
| { | ||
| TempData["FrameworkError"] = $"The email address {userEmail} must match registered DLS Admin account"; | ||
| TempData["FrameworkError"] = $"The email address must match registered DLS Admin account"; |
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.
Tweak to wording:
"The email address must match registered a DLS Admin account"
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.
fixed
| else if (collaboratorId == -2) | ||
| { | ||
| TempData["FrameworkError"] = $"User with the email address {userEmail} has been previously added"; | ||
| TempData["FrameworkError"] = $"User with the email address has been previously added"; |
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.
Tweak to wording:
"A user with the email address has been previously added"
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.
fixed
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. Please avoid committing fixes for different tickets in the same PR in future, though.
JIRA link
DLSV2-687
Description
Error not properly displayed to conform with all other error across the system.
Screenshots
Developer checks
(Leave tasks unticked if they haven't been appropriate for your ticket.)
I have: