Skip to content
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

DP-578 - User Management - New User Validation Check #774

Merged
merged 2 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ private static string GetErrorMessageByCode(string errorCode)
return errorCode switch
{
ErrorCodes.ORGANISATION_ALREADY_EXISTS => ErrorMessagesList.DuplicateOgranisationName,
ErrorCodes.EMAIL_ALREADY_EXISTS_WITHIN_ORGANISATION => ErrorMessagesList.DuplicatePersonEmail,
ErrorCodes.ARGUMENT_NULL => ErrorMessagesList.PayLoadIssueOrNullAurgument,
ErrorCodes.INVALID_OPERATION => ErrorMessagesList.OrganisationCreationFailed,
ErrorCodes.PERSON_DOES_NOT_EXIST => ErrorMessagesList.PersonNotFound,
Expand Down
1 change: 1 addition & 0 deletions Frontend/CO.CDP.OrganisationApp/Constants/ErrorCodes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,5 @@ public static class ErrorCodes
public const string UNKNOWN_BUYER_INFORMATION_UPDATE_TYPE = "UNKNOWN_BUYER_INFORMATION_UPDATE_TYPE";
public const string PERSON_INVITE_ALREADY_CLAIMED = "PERSON_INVITE_ALREADY_CLAIMED";
public const string APIKEY_NAME_ALREADY_EXISTS = "APIKEY_NAME_ALREADY_EXISTS";
public const string EMAIL_ALREADY_EXISTS_WITHIN_ORGANISATION = "EMAIL_ALREADY_EXISTS_WITHIN_ORGANISATION";
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ namespace CO.CDP.OrganisationApp.Constants;
public static class ErrorMessagesList
{
public const string DuplicatePersonName = "A person with this name already exists. Please try again.";
public const string DuplicatePersonEmail = "A user with this email address already exists for your organisation.";
public const string PersonNotFound = "The requested person was not found.";
public const string PersonCreationFailed = "Adding a person failed, have you provided the correct person details.";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<main class="govuk-main-wrapper" id="main-content">
<div class="govuk-grid-row">
<div class="govuk-grid-column-three-quarters">

<partial name="_ErrorSummary" model="@Model.ModelState" />
<form class="form" method="post">

<h1 class="govuk-heading-l">Check your answers</h1>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,10 @@ public async Task<IActionResult> OnPost()

return RedirectToPage("UserSummary", new { Id });
}
catch
catch (ApiException<CO.CDP.Organisation.WebApiClient.ProblemDetails> exc)
{
throw;
ApiExceptionMapper.MapApiExceptions(exc, ModelState);
return Page();
}
finally
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,12 @@ public async Task Execute_ValidOrganisationAndInvite_SuccessfullySendsInvite()

var command = (organisationId, invitePersonData);


_organisationRepository.Setup(repo => repo.Find(organisationId))
.ReturnsAsync(organisation);

_organisationRepository.Setup(repo => repo.IsEmailUniqueWithinOrganisation(organisationId, invitePersonData.Email))
.ReturnsAsync(true);

_mockGovUKNotifyApiClient.Setup(client => client.SendEmail(It.IsAny<EmailNotificationRequest>()));

var result = await _useCase.Execute(command);
Expand All @@ -72,6 +74,39 @@ public async Task Execute_ValidOrganisationAndInvite_SuccessfullySendsInvite()
_mockGovUKNotifyApiClient.Verify(client => client.SendEmail(It.IsAny<EmailNotificationRequest>()), Times.Once);
}

[Fact]
public async Task Execute_ValidOrganisationInviteWithDuplicateEmail_ThrowsException()
{
var organisationId = Guid.NewGuid();
InvitePersonToOrganisation invitePersonData = CreateDummyInviteToPerson();

var organisation = new Persistence.Organisation
{
Guid = organisationId,
Name = "Test Organisation",
Tenant = It.IsAny<Tenant>()
};

var command = (organisationId, invitePersonData);

_organisationRepository.Setup(repo => repo.Find(organisationId))
.ReturnsAsync(organisation);

_organisationRepository.Setup(repo => repo.IsEmailUniqueWithinOrganisation(organisationId, invitePersonData.Email))
.ReturnsAsync(false);

_mockGovUKNotifyApiClient.Setup(client => client.SendEmail(It.IsAny<EmailNotificationRequest>()));

var result = async () => await _useCase.Execute(command);

await result.Should().ThrowAsync<DuplicateEmailWithinOrganisationException>();

_organisationRepository.Verify(repo => repo.Find(organisationId), Times.Once);
_organisationRepository.Verify(repo => repo.IsEmailUniqueWithinOrganisation(organisationId, invitePersonData.Email), Times.Once);
_personsInviteRepository.Verify(repo => repo.Save(It.IsAny<PersonInvite>()), Times.Never);
_mockGovUKNotifyApiClient.Verify(client => client.SendEmail(It.IsAny<EmailNotificationRequest>()), Times.Never);
}

[Fact]
public async Task Execute_UnknownOrganisation_ThrowsUnknownOrganisationException()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ public static class ServiceCollectionExtensions
{ typeof(InvalidUpdateSupplierInformationCommand), (StatusCodes.Status400BadRequest, "INVALID_SUPPLIER_INFORMATION_UPDATE_ENTITY") },
{ typeof(InvalidQueryException), (StatusCodes.Status400BadRequest, "ISSUE_WITH_QUERY_PARAMETERS") },
{ typeof(DuplicateAuthenticationKeyNameException), (StatusCodes.Status400BadRequest, "APIKEY_NAME_ALREADY_EXISTS") },
{ typeof(DuplicateEmailWithinOrganisationException), (StatusCodes.Status400BadRequest, "EMAIL_ALREADY_EXISTS_WITHIN_ORGANISATION") }
};

public static IServiceCollection AddOrganisationProblemDetails(this IServiceCollection services)
Expand Down
3 changes: 2 additions & 1 deletion Services/CO.CDP.Organisation.WebApi/Model/Exceptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,5 @@ public class MissingOrganisationIdException(string message, Exception? cause = n

public class EmptyAuthenticationKeyNameException(string message, Exception? cause = null) : Exception(message, cause);
public class UnknownAuthenticationKeyException(string message, Exception? cause = null) : Exception(message, cause);
public class InvalidSupportUpdateOrganisationCommand(string message, Exception? cause = null) : Exception(message, cause);
public class InvalidSupportUpdateOrganisationCommand(string message, Exception? cause = null) : Exception(message, cause);
public class DuplicateEmailWithinOrganisationException(string message, Exception? cause = null) : Exception(message, cause);
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ public async Task<PersonInvite> Execute((Guid organisationId, InvitePersonToOrga
var organisation = await organisationRepository.Find(command.organisationId)
?? throw new UnknownOrganisationException($"Unknown organisation {command.organisationId}.");

var isEmailUnique = await organisationRepository.IsEmailUniqueWithinOrganisation(command.organisationId, command.invitePersonData.Email);
if (!isEmailUnique)
{
throw new DuplicateEmailWithinOrganisationException($"A user with this email address already exists for your organisation.");
}

var personInvite = CreatePersonInvite(command.invitePersonData, organisation);

personInviteRepository.Save(personInvite);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,53 @@ public async Task GetOperationTypes_WhenOperationTypeExists_ReturnsEmptyList()
result.Should().Contain(OperationType.SmallOrMediumSized);
}

[Fact]
public async Task IsEmailUniqueWithinOrganisation_WhenDoesNotExist_ReturnsTrue()
{
using var repository = OrganisationRepository();

var organisation = GivenOrganisation();
organisation.OrganisationPersons = [];
var personEmail = "john.doe@example.com";

await using var context = postgreSql.OrganisationInformationContext();
await context.Organisations.AddAsync(organisation);
await context.SaveChangesAsync();

var result = await repository.IsEmailUniqueWithinOrganisation(organisation.Guid, personEmail);

result.Should().Be(true);
}

[Fact]
public async Task IsEmailUniqueWithinOrganisation_WhenDoesExist_ReturnsFalse()
{
using var repository = OrganisationRepository();

var organisation = GivenOrganisation();

var organisationPerson = new OrganisationPerson()
{
Organisation = organisation,
Person = new Person
{
FirstName = "John",
LastName = "Doe",
Email = "john.doe@example.com",
Guid = Guid.NewGuid()
}
};
organisation.OrganisationPersons.Add(organisationPerson);

await using var context = postgreSql.OrganisationInformationContext();
await context.Organisations.AddAsync(organisation);
await context.SaveChangesAsync();

var result = await repository.IsEmailUniqueWithinOrganisation(organisation.Guid, organisationPerson.Person.Email);

result.Should().Be(false);
}

private IOrganisationRepository OrganisationRepository(OrganisationInformationContext? context = null)
{
return new DatabaseOrganisationRepository(context ?? postgreSql.OrganisationInformationContext());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,13 @@ public async Task<IList<OperationType>> GetOperationTypes(int organisationId)
return organisation?.SupplierInfo?.OperationTypes ?? [];
}

public async Task<bool> IsEmailUniqueWithinOrganisation(Guid organisationId, string email)
{
return await context.Organisations
.Where(x => x.Guid == organisationId)
.AnyAsync(x => !x.Persons.Any(y => y.Email == email));
}

public void Save(Organisation organisation)
{
try
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,6 @@ public class DuplicateOrganisationException(string message, Exception? cause = n
public Task<Organisation.LegalForm?> GetLegalForm(int organisationId);

public Task<IList<OperationType>> GetOperationTypes(int organisationId);

public Task<bool> IsEmailUniqueWithinOrganisation(Guid organisationId, string email);
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public class Organisation : IEntityDate
public ICollection<ContactPoint> ContactPoints { get; set; } = [];
public List<PartyRole> Roles { get; set; } = [];
public List<Person> Persons => OrganisationPersons.Select(p => p.Person).ToList();
public List<OrganisationPerson> OrganisationPersons { get; init; } = [];
public List<OrganisationPerson> OrganisationPersons { get; set; } = [];
public SupplierInformation? SupplierInfo { get; set; }
public BuyerInformation? BuyerInfo { get; set; }
public DateTimeOffset CreatedOn { get; set; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ namespace CO.CDP.OrganisationInformation.Persistence;
public class OrganisationPerson : IEntityDate
{
public int PersonId { get; set; }
public required Person Person { get; init; }
public required Person Person { get; set; }
public int OrganisationId { get; set; }
public required Organisation Organisation { get; init; }
public List<string> Scopes { get; set; } = [];
Expand Down