Skip to content

Commit

Permalink
Improve error message for users encountering duplicate email / same U…
Browse files Browse the repository at this point in the history
…RN issue
  • Loading branch information
andymantell committed Feb 26, 2025
1 parent c2de990 commit 1d91e2a
Show file tree
Hide file tree
Showing 19 changed files with 151 additions and 107 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ public async Task OnPost_NullOrganisation_ReturnsPage()
public static IEnumerable<object[]> Get_OnPost_AddsModelError_TestData()
{
yield return new object[] { ErrorCodes.ORGANISATION_ALREADY_EXISTS, ErrorMessagesList.DuplicateOrganisationName, StatusCodes.Status400BadRequest };
yield return new object[] { ErrorCodes.ARGUMENT_NULL, ErrorMessagesList.PayLoadIssueOrNullAurgument, StatusCodes.Status400BadRequest };
yield return new object[] { ErrorCodes.ARGUMENT_NULL, ErrorMessagesList.PayLoadIssueOrNullArgument, StatusCodes.Status400BadRequest };
yield return new object[] { ErrorCodes.INVALID_OPERATION, ErrorMessagesList.OrganisationCreationFailed, StatusCodes.Status400BadRequest };
yield return new object[] { ErrorCodes.PERSON_DOES_NOT_EXIST, ErrorMessagesList.PersonNotFound, StatusCodes.Status404NotFound };
yield return new object[] { ErrorCodes.UNPROCESSABLE_ENTITY, ErrorMessagesList.UnprocessableEntity, StatusCodes.Status422UnprocessableEntity };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,9 @@ public async Task OnPost_WhenErrorInRegisteringPerson_ShouldReturnPageWithError(
}

[Fact]
public async Task OnPost_DuplicatePersonName_AddsModelError()
public async Task OnPost_DuplicatePersonEmail_AddsSystemError()
{
var problemDetails = GivenProblemDetails(statusCode: 400, code: ErrorCodes.PERSON_ALREADY_EXISTS);
var problemDetails = GivenProblemDetails(statusCode: 400, code: ErrorCodes.PERSON_EMAIL_ALREADY_EXISTS);
var aex = GivenApiException(statusCode: 400, problemDetails: problemDetails);

sessionMock.Setup(s => s.Get<UserDetails>(Session.UserDetailsKey))
Expand All @@ -222,11 +222,16 @@ public async Task OnPost_DuplicatePersonName_AddsModelError()
personClientMock.Setup(o => o.CreatePersonAsync(It.IsAny<NewPerson>()))
.ThrowsAsync(aex);

var httpContextMock = new Mock<HttpContext>();
httpContextMock.Setup(c => c.TraceIdentifier).Returns("trace-123");

var model = GivenYourDetailsModel();

model.PageContext.HttpContext = httpContextMock.Object;

await model.OnPost();
model.ModelState[string.Empty].As<ModelStateEntry>().Errors
.Should().Contain(e => e.ErrorMessage == ErrorMessagesList.DuplicatePersonName);

model.SystemError?.ToString().Should().Be(string.Format(ErrorMessagesList.FirstAccessDuplicatePersonEmail, "trace-123"));
}

[Fact]
Expand All @@ -245,7 +250,7 @@ public async Task OnPost_ArgumentNull_AddsModelError()

await model.OnPost();
model.ModelState[string.Empty].As<ModelStateEntry>().Errors
.Should().Contain(e => e.ErrorMessage == ErrorMessagesList.PayLoadIssueOrNullAurgument);
.Should().Contain(e => e.ErrorMessage == ErrorMessagesList.PayLoadIssueOrNullArgument);
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ private static string GetErrorMessageByCode(string errorCode)
ErrorCodes.ORGANISATION_ALREADY_EXISTS => ErrorMessagesList.DuplicateOrganisationName,
ErrorCodes.EMAIL_ALREADY_EXISTS_WITHIN_ORGANISATION => ErrorMessagesList.DuplicatePersonEmail,
ErrorCodes.INVITE_EMAIL_ALREADY_EXISTS_FOR_ORGANISATION => ErrorMessagesList.DuplicateInviteEmail,
ErrorCodes.ARGUMENT_NULL => ErrorMessagesList.PayLoadIssueOrNullAurgument,
ErrorCodes.ARGUMENT_NULL => ErrorMessagesList.PayLoadIssueOrNullArgument,
ErrorCodes.INVALID_OPERATION => ErrorMessagesList.OrganisationCreationFailed,
ErrorCodes.PERSON_DOES_NOT_EXIST => ErrorMessagesList.PersonNotFound,
ErrorCodes.MOU_DOES_NOT_EXIST=> ErrorMessagesList.MouNotFound,
Expand Down
3 changes: 2 additions & 1 deletion Frontend/CO.CDP.OrganisationApp/Constants/ErrorCodes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ public static class ErrorCodes
public const string ORGANISATION_ALREADY_EXISTS = "ORGANISATION_ALREADY_EXISTS";
public const string INVALID_OPERATION = "INVALID_OPERATION";
public const string ARGUMENT_NULL = "ARGUMENT_NULL";
public const string PERSON_ALREADY_EXISTS = "PERSON_ALREADY_EXISTS";
public const string PERSON_GUID_ALREADY_EXISTS = "PERSON_GUID_ALREADY_EXISTS";
public const string PERSON_EMAIL_ALREADY_EXISTS = "PERSON_EMAIL_ALREADY_EXISTS";

public const string PERSON_DOES_NOT_EXIST = "PERSON_DOES_NOT_EXIST";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ public static class ErrorMessagesList
{
public static readonly string DuplicatePersonName = StaticTextResource.ErrorMessageList_DuplicatePersonName;
public static readonly string DuplicatePersonEmail = StaticTextResource.ErrorMessageList_DuplicatePersonEmail;
public static readonly string FirstAccessDuplicatePersonEmail = StaticTextResource.ErrorMessageList_FirstAccessDuplicatePersonEmail;
public static readonly string DuplicateInviteEmail = StaticTextResource.ErrorMessageList_DuplicateInviteEmail;
public static readonly string PersonNotFound = StaticTextResource.ErrorMessageList_PersonNotFound;
public static readonly string MouNotFound = StaticTextResource.ErrorMessageList_MouNotFound;
Expand All @@ -16,7 +17,7 @@ public static class ErrorMessagesList
public static readonly string OrganisationNotFound = StaticTextResource.ErrorMessageList_OrganisationNotFound;
public static readonly string DuplicateOrganisationName = StaticTextResource.ErrorMessageList_DuplicateOrganisationName;

public static readonly string PayLoadIssueOrNullAurgument = StaticTextResource.ErrorMessageList_PayLoadIssueOrNullAurgument;
public static readonly string PayLoadIssueOrNullArgument = StaticTextResource.ErrorMessageList_PayLoadIssueOrNullArgument;

public static readonly string UnprocessableEntity = StaticTextResource.ErrorMessageList_UnprocessableEntity;
public static readonly string UnexpectedError = StaticTextResource.ErrorMessageList_UnexpectedError;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public async Task<IActionResult> OnPost()

if (csc == null)
{
ModelState.AddModelError(string.Empty, ErrorMessagesList.PayLoadIssueOrNullAurgument);
ModelState.AddModelError(string.Empty, ErrorMessagesList.PayLoadIssueOrNullArgument);
return Page();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public async Task<IActionResult> OnPost()

if (payload == null)
{
ModelState.AddModelError(string.Empty, ErrorMessagesList.PayLoadIssueOrNullAurgument);
ModelState.AddModelError(string.Empty, ErrorMessagesList.PayLoadIssueOrNullArgument);
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public async Task<IActionResult> OnPost()

if (payload == null)
{
ModelState.AddModelError(string.Empty, ErrorMessagesList.PayLoadIssueOrNullAurgument);
ModelState.AddModelError(string.Empty, ErrorMessagesList.PayLoadIssueOrNullArgument);
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public async Task<IActionResult> OnPost()

if (updatePayload == null)
{
ModelState.AddModelError(string.Empty, ErrorMessagesList.PayLoadIssueOrNullAurgument);
ModelState.AddModelError(string.Empty, ErrorMessagesList.PayLoadIssueOrNullArgument);
return Page();
}

Expand All @@ -108,7 +108,7 @@ public async Task<IActionResult> OnPost()

if (payload == null)
{
ModelState.AddModelError(string.Empty, ErrorMessagesList.PayLoadIssueOrNullAurgument);
ModelState.AddModelError(string.Empty, ErrorMessagesList.PayLoadIssueOrNullArgument);
return Page();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public async Task<IActionResult> OnPost()

if (updatePayload == null)
{
ModelState.AddModelError(string.Empty, ErrorMessagesList.PayLoadIssueOrNullAurgument);
ModelState.AddModelError(string.Empty, ErrorMessagesList.PayLoadIssueOrNullArgument);
return Page();
}

Expand All @@ -105,7 +105,7 @@ public async Task<IActionResult> OnPost()

if (payload == null)
{
ModelState.AddModelError(string.Empty, ErrorMessagesList.PayLoadIssueOrNullAurgument);
ModelState.AddModelError(string.Empty, ErrorMessagesList.PayLoadIssueOrNullArgument);
return Page();
}

Expand Down
115 changes: 67 additions & 48 deletions Frontend/CO.CDP.OrganisationApp/Pages/YourDetails.cshtml
Original file line number Diff line number Diff line change
Expand Up @@ -9,52 +9,71 @@
var lastNameHasError = ((TagBuilder)Html.ValidationMessageFor(m => m.LastName)).HasInnerHtml;
}

<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds">
<partial name="_ErrorSummary" model="@ModelState" />

<h1 class="govuk-heading-l">
@ViewData["Title"]
</h1>

<form method="post">
<fieldset class="govuk-fieldset">
<div class="govuk-form-group @(firstNameHasError ? "govuk-form-group--error" : "")">
<label class="govuk-label" for="@nameof(Model.FirstName)">
@Html.DisplayNameFor(m => m.FirstName)
</label>
@if (firstNameHasError)
{
<p id="first-name-error" class="govuk-error-message">
<span class="govuk-visually-hidden">Error:</span>
@Html.ValidationMessageFor(m => m.FirstName)
</p>
}
<input class="govuk-input @(firstNameHasError ? "govuk-input--error" : "")" id="@nameof(Model.FirstName)"
value="@Model.FirstName" name="@nameof(Model.FirstName)" type="text"
spellcheck="false" @(firstNameHasError ? "aria-describedby=first-name-error" : "")>
</div>

<div class="govuk-form-group @(lastNameHasError ? "govuk-form-group--error" : "")">
<label class="govuk-label" for="@nameof(Model.LastName)">
@Html.DisplayNameFor(m => m.LastName)
</label>
@if (lastNameHasError)
{
<p id="last-name-error" class="govuk-error-message">
<span class="govuk-visually-hidden">Error:</span>
@Html.ValidationMessageFor(m => m.LastName)
</p>
}
<input class="govuk-input @(lastNameHasError ? "govuk-input--error" : "")" id="@nameof(Model.LastName)"
value="@Model.LastName" name="@nameof(Model.LastName)" type="text"
spellcheck="false" @(lastNameHasError ? "aria-describedby=last-name-error" : "")>
</div>
</fieldset>

<govuk-button>
@StaticTextResource.Global_Continue
</govuk-button>
</form>
</div>
<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds">
<partial name="_ErrorSummary" model="@ModelState" />

@if(Model.SystemError != null)
{
<div class="govuk-notification-banner app-notification-banner--failure" role="region" aria-labelledby="govuk-notification-banner-title" data-module="govuk-notification-banner">
<div class="govuk-notification-banner__header">
<h2 class="govuk-notification-banner__title" id="govuk-notification-banner-title">
Important
</h2>
</div>
<div class="govuk-notification-banner__content">
<p class="govuk-notification-banner__heading">
There is a problem
</p>
<p class="govuk-body">
@Model.SystemError
</p>
</div>
</div>
}

<h1 class="govuk-heading-l">
@ViewData["Title"]
</h1>

<form method="post">
<fieldset class="govuk-fieldset">
<div class="govuk-form-group @(firstNameHasError ? "govuk-form-group--error" : "")">
<label class="govuk-label" for="@nameof(Model.FirstName)">
@Html.DisplayNameFor(m => m.FirstName)
</label>
@if (firstNameHasError)
{
<p id="first-name-error" class="govuk-error-message">
<span class="govuk-visually-hidden">Error:</span>
@Html.ValidationMessageFor(m => m.FirstName)
</p>
}
<input class="govuk-input @(firstNameHasError ? "govuk-input--error" : "")" id="@nameof(Model.FirstName)"
value="@Model.FirstName" name="@nameof(Model.FirstName)" type="text"
spellcheck="false" @(firstNameHasError ? "aria-describedby=first-name-error" : "")>
</div>

<div class="govuk-form-group @(lastNameHasError ? "govuk-form-group--error" : "")">
<label class="govuk-label" for="@nameof(Model.LastName)">
@Html.DisplayNameFor(m => m.LastName)
</label>
@if (lastNameHasError)
{
<p id="last-name-error" class="govuk-error-message">
<span class="govuk-visually-hidden">Error:</span>
@Html.ValidationMessageFor(m => m.LastName)
</p>
}
<input class="govuk-input @(lastNameHasError ? "govuk-input--error" : "")" id="@nameof(Model.LastName)"
value="@Model.LastName" name="@nameof(Model.LastName)" type="text"
spellcheck="false" @(lastNameHasError ? "aria-describedby=last-name-error" : "")>
</div>
</fieldset>

<govuk-button>
@StaticTextResource.Global_Continue
</govuk-button>
</form>
</div>
</div>
21 changes: 16 additions & 5 deletions Frontend/CO.CDP.OrganisationApp/Pages/YourDetails.cshtml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.ComponentModel;
using System.ComponentModel.DataAnnotations;
using CO.CDP.Localization;
using Microsoft.AspNetCore.Html;

namespace CO.CDP.OrganisationApp.Pages;

Expand All @@ -22,7 +23,7 @@ public class YourDetailsModel(
[Required(ErrorMessageResourceName = nameof(StaticTextResource.Users_LastName_Validation), ErrorMessageResourceType = typeof(StaticTextResource))]
public string? LastName { get; set; }

public string? Error { get; set; }
public IHtmlContent? SystemError { get; set; }

public IActionResult OnGet()
{
Expand Down Expand Up @@ -95,14 +96,24 @@ private void MapApiException(ApiException<Person.WebApiClient.ProblemDetails> ae

if (!string.IsNullOrEmpty(code))
{
ModelState.AddModelError(string.Empty, code switch
var error = code switch
{
ErrorCodes.PERSON_ALREADY_EXISTS => ErrorMessagesList.DuplicatePersonName,
ErrorCodes.ARGUMENT_NULL => ErrorMessagesList.PayLoadIssueOrNullAurgument,
ErrorCodes.PERSON_GUID_ALREADY_EXISTS => ErrorMessagesList.DuplicatePersonName,
ErrorCodes.PERSON_EMAIL_ALREADY_EXISTS => string.Format(ErrorMessagesList.FirstAccessDuplicatePersonEmail, HttpContext.TraceIdentifier),
ErrorCodes.ARGUMENT_NULL => ErrorMessagesList.PayLoadIssueOrNullArgument,
ErrorCodes.INVALID_OPERATION => ErrorMessagesList.PersonCreationFailed,
ErrorCodes.UNPROCESSABLE_ENTITY => ErrorMessagesList.UnprocessableEntity,
_ => ErrorMessagesList.UnexpectedError
});
};

if (code == ErrorCodes.PERSON_EMAIL_ALREADY_EXISTS)
{
SystemError = new HtmlString(error);
}
else
{
ModelState.AddModelError(string.Empty, error);
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions Services/CO.CDP.Localization/StaticTextResource.cy.resx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<root>
<!--
Microsoft ResX Schema
Expand Down Expand Up @@ -2997,7 +2997,7 @@
<data name="ErrorMessageList_DuplicatePersonName" xml:space="preserve">
<value>Mae person gyda’r enw hwn eisoes yn bodoli. Rhowch gynnig arall arni.</value>
</data>
<data name="ErrorMessageList_PayLoadIssueOrNullAurgument" xml:space="preserve">
<data name="ErrorMessageList_PayLoadIssueOrNullArgument" xml:space="preserve">
<value>Rhowch yr holl wybodaeth ofynnol yn gywir.</value>
</data>
<data name="ErrorMessageList_SharecodeNotFound" xml:space="preserve">
Expand Down
Loading

0 comments on commit 1d91e2a

Please sign in to comment.