From de0d45721d49d6664c6deb9946ed08174463625e Mon Sep 17 00:00:00 2001 From: maciej-goaco Date: Thu, 17 Oct 2024 12:35:31 +0100 Subject: [PATCH 1/2] DP-578 - Basic implementation --- .../CO.CDP.OrganisationApp/Constants/ApiExceptionMapper.cs | 1 + Frontend/CO.CDP.OrganisationApp/Constants/ErrorCodes.cs | 1 + .../CO.CDP.OrganisationApp/Constants/ErrorMessagesList.cs | 1 + .../Pages/Users/UserCheckAnswers.cshtml | 2 +- .../Pages/Users/UserCheckAnswers.cshtml.cs | 5 +++-- .../Extensions/ServiceCollectionExtensions.cs | 1 + Services/CO.CDP.Organisation.WebApi/Model/Exceptions.cs | 3 ++- .../UseCase/InvitePersonToOrganisationUseCase.cs | 6 ++++++ .../DatabaseOrganisationRepository.cs | 7 +++++++ .../IOrganisationRepository.cs | 2 ++ 10 files changed, 25 insertions(+), 4 deletions(-) diff --git a/Frontend/CO.CDP.OrganisationApp/Constants/ApiExceptionMapper.cs b/Frontend/CO.CDP.OrganisationApp/Constants/ApiExceptionMapper.cs index 53d61b86f..f97853b97 100644 --- a/Frontend/CO.CDP.OrganisationApp/Constants/ApiExceptionMapper.cs +++ b/Frontend/CO.CDP.OrganisationApp/Constants/ApiExceptionMapper.cs @@ -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, diff --git a/Frontend/CO.CDP.OrganisationApp/Constants/ErrorCodes.cs b/Frontend/CO.CDP.OrganisationApp/Constants/ErrorCodes.cs index 87578aa3f..019e696c7 100644 --- a/Frontend/CO.CDP.OrganisationApp/Constants/ErrorCodes.cs +++ b/Frontend/CO.CDP.OrganisationApp/Constants/ErrorCodes.cs @@ -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"; } \ No newline at end of file diff --git a/Frontend/CO.CDP.OrganisationApp/Constants/ErrorMessagesList.cs b/Frontend/CO.CDP.OrganisationApp/Constants/ErrorMessagesList.cs index c06ac2d2c..cf9b13eee 100644 --- a/Frontend/CO.CDP.OrganisationApp/Constants/ErrorMessagesList.cs +++ b/Frontend/CO.CDP.OrganisationApp/Constants/ErrorMessagesList.cs @@ -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."; diff --git a/Frontend/CO.CDP.OrganisationApp/Pages/Users/UserCheckAnswers.cshtml b/Frontend/CO.CDP.OrganisationApp/Pages/Users/UserCheckAnswers.cshtml index 445b64d6b..27e7cb25d 100644 --- a/Frontend/CO.CDP.OrganisationApp/Pages/Users/UserCheckAnswers.cshtml +++ b/Frontend/CO.CDP.OrganisationApp/Pages/Users/UserCheckAnswers.cshtml @@ -14,7 +14,7 @@
- +

Check your answers

diff --git a/Frontend/CO.CDP.OrganisationApp/Pages/Users/UserCheckAnswers.cshtml.cs b/Frontend/CO.CDP.OrganisationApp/Pages/Users/UserCheckAnswers.cshtml.cs index 4d785de54..fe9bfde70 100644 --- a/Frontend/CO.CDP.OrganisationApp/Pages/Users/UserCheckAnswers.cshtml.cs +++ b/Frontend/CO.CDP.OrganisationApp/Pages/Users/UserCheckAnswers.cshtml.cs @@ -55,9 +55,10 @@ public async Task OnPost() return RedirectToPage("UserSummary", new { Id }); } - catch + catch (ApiException exc) { - throw; + ApiExceptionMapper.MapApiExceptions(exc, ModelState); + return Page(); } finally { diff --git a/Services/CO.CDP.Organisation.WebApi/Extensions/ServiceCollectionExtensions.cs b/Services/CO.CDP.Organisation.WebApi/Extensions/ServiceCollectionExtensions.cs index b91a82638..3ed48a701 100644 --- a/Services/CO.CDP.Organisation.WebApi/Extensions/ServiceCollectionExtensions.cs +++ b/Services/CO.CDP.Organisation.WebApi/Extensions/ServiceCollectionExtensions.cs @@ -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) diff --git a/Services/CO.CDP.Organisation.WebApi/Model/Exceptions.cs b/Services/CO.CDP.Organisation.WebApi/Model/Exceptions.cs index 773ebb5cd..d216cb0b0 100644 --- a/Services/CO.CDP.Organisation.WebApi/Model/Exceptions.cs +++ b/Services/CO.CDP.Organisation.WebApi/Model/Exceptions.cs @@ -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); \ No newline at end of file +public class InvalidSupportUpdateOrganisationCommand(string message, Exception? cause = null) : Exception(message, cause); +public class DuplicateEmailWithinOrganisationException(string message, Exception? cause = null) : Exception(message, cause); \ No newline at end of file diff --git a/Services/CO.CDP.Organisation.WebApi/UseCase/InvitePersonToOrganisationUseCase.cs b/Services/CO.CDP.Organisation.WebApi/UseCase/InvitePersonToOrganisationUseCase.cs index cd0226f91..4cb4bb26b 100644 --- a/Services/CO.CDP.Organisation.WebApi/UseCase/InvitePersonToOrganisationUseCase.cs +++ b/Services/CO.CDP.Organisation.WebApi/UseCase/InvitePersonToOrganisationUseCase.cs @@ -27,6 +27,12 @@ public async Task 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); diff --git a/Services/CO.CDP.OrganisationInformation.Persistence/DatabaseOrganisationRepository.cs b/Services/CO.CDP.OrganisationInformation.Persistence/DatabaseOrganisationRepository.cs index b5085ea65..5765dec18 100644 --- a/Services/CO.CDP.OrganisationInformation.Persistence/DatabaseOrganisationRepository.cs +++ b/Services/CO.CDP.OrganisationInformation.Persistence/DatabaseOrganisationRepository.cs @@ -139,6 +139,13 @@ public async Task> GetOperationTypes(int organisationId) return organisation?.SupplierInfo?.OperationTypes ?? []; } + public async Task 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 diff --git a/Services/CO.CDP.OrganisationInformation.Persistence/IOrganisationRepository.cs b/Services/CO.CDP.OrganisationInformation.Persistence/IOrganisationRepository.cs index b830327f2..2ac23d0dd 100644 --- a/Services/CO.CDP.OrganisationInformation.Persistence/IOrganisationRepository.cs +++ b/Services/CO.CDP.OrganisationInformation.Persistence/IOrganisationRepository.cs @@ -36,4 +36,6 @@ public class DuplicateOrganisationException(string message, Exception? cause = n public Task GetLegalForm(int organisationId); public Task> GetOperationTypes(int organisationId); + + public Task IsEmailUniqueWithinOrganisation(Guid organisationId, string email); } \ No newline at end of file From c4f30b026cbb444bdb53ea2b53ac599e292a3957 Mon Sep 17 00:00:00 2001 From: maciej-goaco Date: Thu, 17 Oct 2024 14:45:14 +0100 Subject: [PATCH 2/2] DP-578 - Tests added --- .../InvitePersonToOrganisationUseCaseTest.cs | 37 ++++++++++++++- .../DatabaseOrganisationRepositoryTest.cs | 47 +++++++++++++++++++ .../Organisation.cs | 2 +- .../OrganisationPerson.cs | 2 +- 4 files changed, 85 insertions(+), 3 deletions(-) diff --git a/Services/CO.CDP.Organisation.WebApi.Tests/UseCase/InvitePersonToOrganisationUseCaseTest.cs b/Services/CO.CDP.Organisation.WebApi.Tests/UseCase/InvitePersonToOrganisationUseCaseTest.cs index 60cafd9ea..3b6589f61 100644 --- a/Services/CO.CDP.Organisation.WebApi.Tests/UseCase/InvitePersonToOrganisationUseCaseTest.cs +++ b/Services/CO.CDP.Organisation.WebApi.Tests/UseCase/InvitePersonToOrganisationUseCaseTest.cs @@ -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())); var result = await _useCase.Execute(command); @@ -72,6 +74,39 @@ public async Task Execute_ValidOrganisationAndInvite_SuccessfullySendsInvite() _mockGovUKNotifyApiClient.Verify(client => client.SendEmail(It.IsAny()), 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() + }; + + 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())); + + var result = async () => await _useCase.Execute(command); + + await result.Should().ThrowAsync(); + + _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()), Times.Never); + _mockGovUKNotifyApiClient.Verify(client => client.SendEmail(It.IsAny()), Times.Never); + } + [Fact] public async Task Execute_UnknownOrganisation_ThrowsUnknownOrganisationException() { diff --git a/Services/CO.CDP.OrganisationInformation.Persistence.Tests/DatabaseOrganisationRepositoryTest.cs b/Services/CO.CDP.OrganisationInformation.Persistence.Tests/DatabaseOrganisationRepositoryTest.cs index e6c2d3968..0a67480d1 100644 --- a/Services/CO.CDP.OrganisationInformation.Persistence.Tests/DatabaseOrganisationRepositoryTest.cs +++ b/Services/CO.CDP.OrganisationInformation.Persistence.Tests/DatabaseOrganisationRepositoryTest.cs @@ -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()); diff --git a/Services/CO.CDP.OrganisationInformation.Persistence/Organisation.cs b/Services/CO.CDP.OrganisationInformation.Persistence/Organisation.cs index 40d3fcc89..ac51ff71f 100644 --- a/Services/CO.CDP.OrganisationInformation.Persistence/Organisation.cs +++ b/Services/CO.CDP.OrganisationInformation.Persistence/Organisation.cs @@ -17,7 +17,7 @@ public class Organisation : IEntityDate public ICollection ContactPoints { get; set; } = []; public List Roles { get; set; } = []; public List Persons => OrganisationPersons.Select(p => p.Person).ToList(); - public List OrganisationPersons { get; init; } = []; + public List OrganisationPersons { get; set; } = []; public SupplierInformation? SupplierInfo { get; set; } public BuyerInformation? BuyerInfo { get; set; } public DateTimeOffset CreatedOn { get; set; } diff --git a/Services/CO.CDP.OrganisationInformation.Persistence/OrganisationPerson.cs b/Services/CO.CDP.OrganisationInformation.Persistence/OrganisationPerson.cs index 90f20003c..2d852a98e 100644 --- a/Services/CO.CDP.OrganisationInformation.Persistence/OrganisationPerson.cs +++ b/Services/CO.CDP.OrganisationInformation.Persistence/OrganisationPerson.cs @@ -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 Scopes { get; set; } = [];