Merge pull request #826 from cabinetoffice/bugfix/DP-765-check-person…

…-not-already-part-of-org Bugfix/dp 765 check person not already part of org
cabinetoffice · Oct 25, 2024 · ff7e247 · ff7e247
2 parents 016b649 + f6c7c25
commit ff7e247
Show file tree

Hide file tree

Showing 8 changed files with 70 additions and 9 deletions.
diff --git a/Frontend/CO.CDP.OrganisationApp.Tests/Pages/Registration/JoinOrganisationTests.cs b/Frontend/CO.CDP.OrganisationApp.Tests/Pages/Registration/JoinOrganisationTests.cs
@@ -1,9 +1,11 @@
 using CO.CDP.Organisation.WebApiClient;
+using CO.CDP.OrganisationApp.Constants;
 using CO.CDP.OrganisationApp.Models;
 using CO.CDP.OrganisationApp.Pages.Registration;
 using FluentAssertions;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.AspNetCore.Mvc.RazorPages;
+using OrganisationWebApiClient = CO.CDP.Organisation.WebApiClient;
 using Moq;
 
 namespace CO.CDP.OrganisationApp.Tests.Pages.Registration;
@@ -12,6 +14,7 @@ public class JoinOrganisationModelTests
 {
     private readonly Mock<IOrganisationClient> _organisationClientMock;
     private readonly Mock<ISession> _sessionMock;
+    private readonly Mock<ITempDataService> _tempDataMock;
     private readonly JoinOrganisationModel _joinOrganisationModel;
     private readonly Guid _organisationId = Guid.NewGuid();
     private readonly string _identifier = "GB-COH:123456789";
@@ -22,9 +25,10 @@ public JoinOrganisationModelTests()
     {
         _organisationClientMock = new Mock<IOrganisationClient>();
         _sessionMock = new Mock<ISession>();
+        _tempDataMock = new Mock<ITempDataService>();
         _sessionMock.Setup(s => s.Get<UserDetails>(Session.UserDetailsKey))
             .Returns(new UserDetails() { UserUrn = "testUserUrn", PersonId = _personId});
-        _joinOrganisationModel = new JoinOrganisationModel(_organisationClientMock.Object, _sessionMock.Object);
+        _joinOrganisationModel = new JoinOrganisationModel(_organisationClientMock.Object, _sessionMock.Object, _tempDataMock.Object);
         _organisation = new CO.CDP.Organisation.WebApiClient.Organisation(null, null, null, null, _organisationId, null, "Test Org", []);
     }
 
@@ -119,4 +123,28 @@ public async Task OnPost_UserHasNoPersonId_RedirectsToHomePage()
 
         _organisationClientMock.Verify(client => client.CreateJoinRequestAsync(It.IsAny<Guid>(), It.IsAny<CreateOrganisationJoinRequest>()), Times.Never);
     }
+
+    [Fact]
+    public async Task OnPost_CreateJoinRequestThrowsApiException_SetsFlashMessageAndReturnsPage()
+    {
+        _organisationClientMock.Setup(client => client.LookupOrganisationAsync(string.Empty, _identifier))
+            .ReturnsAsync(_organisation);
+        _joinOrganisationModel.Join = true;
+
+        _organisationClientMock.Setup(client => client.CreateJoinRequestAsync(
+                _organisationId,
+                It.Is<CreateOrganisationJoinRequest>(r => r.PersonId == _joinOrganisationModel.UserDetails.PersonId)))
+            .ThrowsAsync(new ApiException<OrganisationWebApiClient.ProblemDetails>("Error", 400, "Error", null!, null!, null!));
+
+        var result = await _joinOrganisationModel.OnPost(_identifier);
+
+        _tempDataMock.Verify(tempData => tempData.Put(
+                FlashMessageTypes.Important,
+                It.IsAny<FlashMessage>()),
+            Times.Once);
+
+        result.Should().BeOfType<PageResult>();
+    }
+
+
 }
diff --git a/Frontend/CO.CDP.OrganisationApp/Constants/ApiExceptionMapper.cs b/Frontend/CO.CDP.OrganisationApp/Constants/ApiExceptionMapper.cs
@@ -19,7 +19,7 @@ public static void MapApiExceptions(
 
         var errorMessage = GetErrorMessageByCode(code);
         modelState.AddModelError(string.Empty, errorMessage);
-       
+
     }
 
     private static string? ExtractErrorCode(ApiException<ProblemDetails> aex)
@@ -42,6 +42,7 @@ private static string GetErrorMessageByCode(string errorCode)
             ErrorCodes.UNKNOWN_ORGANISATION => ErrorMessagesList.UnknownOrganisation,
             ErrorCodes.BUYER_INFO_NOT_EXISTS => ErrorMessagesList.BuyerInfoNotExists,
             ErrorCodes.UNKNOWN_BUYER_INFORMATION_UPDATE_TYPE => ErrorMessagesList.UnknownBuyerInformationUpdateType,
+            ErrorCodes.PERSON_ALREADY_ADDED_TO_ORGANISATION => ErrorMessagesList.AlreadyMemberOfOrganisation,
             _ => ErrorMessagesList.UnexpectedError
         };
     }

diff --git a/Frontend/CO.CDP.OrganisationApp/Constants/ErrorCodes.cs b/Frontend/CO.CDP.OrganisationApp/Constants/ErrorCodes.cs
@@ -20,4 +20,5 @@ public static class ErrorCodes
     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";
+    public const string PERSON_ALREADY_ADDED_TO_ORGANISATION = "PERSON_ALREADY_ADDED_TO_ORGANISATION";
 }
diff --git a/Frontend/CO.CDP.OrganisationApp/Constants/ErrorMessagesList.cs b/Frontend/CO.CDP.OrganisationApp/Constants/ErrorMessagesList.cs
@@ -22,5 +22,6 @@ public static class ErrorMessagesList
     public const string UnknownBuyerInformationUpdateType = "The requested buyer information update type is unknown.";
 
     public const string DuplicateApiKeyName = "An API key with this name already exists. Please try again.";
+    public const string AlreadyMemberOfOrganisation = "You are already a member of this organisation.";
 
 }
diff --git a/Frontend/CO.CDP.OrganisationApp/Pages/Registration/JoinOrganisation.cshtml.cs b/Frontend/CO.CDP.OrganisationApp/Pages/Registration/JoinOrganisation.cshtml.cs
@@ -1,20 +1,25 @@
 using System.ComponentModel.DataAnnotations;
 using CO.CDP.Organisation.WebApiClient;
+using CO.CDP.OrganisationApp.Constants;
+using CO.CDP.OrganisationApp.Models;
 using Microsoft.AspNetCore.Mvc;
 using OrganisationWebApiClient = CO.CDP.Organisation.WebApiClient;
 
 namespace CO.CDP.OrganisationApp.Pages.Registration;
 
 public class JoinOrganisationModel(
     IOrganisationClient organisationClient,
-    ISession session) : LoggedInUserAwareModel(session)
+    ISession session,
+    ITempDataService tempDataService) : LoggedInUserAwareModel(session)
 {
     public OrganisationWebApiClient.Organisation? OrganisationDetails { get; set; }
 
     [BindProperty]
     [Required(ErrorMessage = "Select an option")]
     public bool Join { get; set; }
 
+    public FlashMessage NotificationBannerAlreadyMemberOfOrganisation { get { return new FlashMessage(ErrorMessagesList.AlreadyMemberOfOrganisation + " <a class='govuk-notification-banner__link' href='/organisation/" + OrganisationDetails?.Id + "'>View Organisation</a>"); } }
+
     public async Task<IActionResult> OnGet(string identifier)
     {
         try
@@ -39,11 +44,20 @@ public async Task<IActionResult> OnPost(string identifier)
 
         if (UserDetails.PersonId != null && OrganisationDetails != null)
         {
-            if (Join == true)
+            if (Join)
             {
-                await organisationClient.CreateJoinRequestAsync(OrganisationDetails.Id, new CreateOrganisationJoinRequest(
-                    personId: UserDetails.PersonId.Value
-                ));
+                try
+                {
+                    await organisationClient.CreateJoinRequestAsync(OrganisationDetails.Id,
+                        new CreateOrganisationJoinRequest(
+                            personId: UserDetails.PersonId.Value
+                        ));
+                }
+                catch (ApiException<OrganisationWebApiClient.ProblemDetails>)
+                {
+                    tempDataService.Put(FlashMessageTypes.Important, NotificationBannerAlreadyMemberOfOrganisation);
+                    return Page();
+                }
 
                 return Redirect("/registration/" + identifier + "/join-organisation/success");
             }

diff --git a/Services/CO.CDP.Organisation.WebApi/Extensions/ServiceCollectionExtensions.cs b/Services/CO.CDP.Organisation.WebApi/Extensions/ServiceCollectionExtensions.cs
@@ -21,7 +21,8 @@ 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") }
+        { typeof(DuplicateEmailWithinOrganisationException), (StatusCodes.Status400BadRequest, "EMAIL_ALREADY_EXISTS_WITHIN_ORGANISATION") },
+        { typeof(PersonAlreadyAddedToOrganisationException), (StatusCodes.Status400BadRequest, "PERSON_ALREADY_ADDED_TO_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
@@ -19,4 +19,6 @@ 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 DuplicateEmailWithinOrganisationException(string message, Exception? cause = null) : Exception(message, cause);
+public class DuplicateEmailWithinOrganisationException(string message, Exception? cause = null) : Exception(message, cause);
+public class PersonAlreadyAddedToOrganisationException(string message, Exception? cause = null) : Exception(message, cause);
+public class UnknownOrganisationJoinRequestException(string message, Exception? cause = null) : Exception(message, cause);
diff --git a/Services/CO.CDP.Organisation.WebApi/UseCase/CreateOrganisationJoinRequestUseCase.cs b/Services/CO.CDP.Organisation.WebApi/UseCase/CreateOrganisationJoinRequestUseCase.cs
@@ -42,6 +42,8 @@ public async Task<OrganisationJoinRequest> Execute((Guid organisationId, CreateO
         var person = await personRepository.Find(command.createOrganisationJoinRequestCommand.PersonId)
                            ?? throw new UnknownPersonException($"Unknown person {command.createOrganisationJoinRequestCommand.PersonId}.");
 
+        await GuardPersonIsNotAlreadyAdded(organisation, person);
+
         var organisationJoinRequest = CreateOrganisationJoinRequest(organisation, person);
 
         organisationJoinRequestRepository.Save(organisationJoinRequest);
@@ -157,4 +159,15 @@ private async Task<List<Person>> GetOrganisationAdminUsers(Persistence.Organisat
         var organisationPersons = await organisationRepository.FindOrganisationPersons(organisation.Guid);
         return organisationPersons.Where(op => op.Scopes.Contains(Constants.OrganisationPersonScope.Admin)).Select(op => op.Person).ToList();
     }
+
+    private async Task GuardPersonIsNotAlreadyAdded(Persistence.Organisation organisation, Person person)
+    {
+        var matchingOrganisationPerson = await organisationRepository.FindOrganisationPerson(organisation.Guid, person.Guid);
+
+        if (matchingOrganisationPerson != null)
+        {
+            throw new PersonAlreadyAddedToOrganisationException(
+                $"Person {person.Guid} already added to organisation {organisation.Guid}.");
+        }
+    }
 }