From 127f1fd34d65c1ab5a60f0cd8c7d09e8d8021d15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Tue, 10 Dec 2024 11:14:34 +0000 Subject: [PATCH] =?UTF-8?q?[PM-10338]=C2=A0Update=20the=20Organization=20'?= =?UTF-8?q?Leave'=20endpoint=20to=20log=20EventType.OrganizationUser=5FLef?= =?UTF-8?q?t=20(#4908)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Implement UserLeaveAsync in IRemoveOrganizationUserCommand and refactor OrganizationsController to use it * Edit summary message for IRemoveOrganizationUserCommand.UserLeaveAsync * Refactor RemoveOrganizationUserCommand.RemoveUsersAsync to log in bulk --------- Co-authored-by: Matt Bishop --- .../Controllers/OrganizationsController.cs | 2 +- .../IRemoveOrganizationUserCommand.cs | 7 + .../RemoveOrganizationUserCommand.cs | 12 +- .../OrganizationsControllerTests.cs | 4 +- .../RemoveOrganizationUserCommandTests.cs | 136 +++++++++++++++++- 5 files changed, 153 insertions(+), 8 deletions(-) diff --git a/src/Api/AdminConsole/Controllers/OrganizationsController.cs b/src/Api/AdminConsole/Controllers/OrganizationsController.cs index 4421af3a9a73..226fe83c730e 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationsController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationsController.cs @@ -259,7 +259,7 @@ public async Task Leave(Guid id) throw new BadRequestException("Managed user account cannot leave managing organization. Contact your organization administrator for additional details."); } - await _removeOrganizationUserCommand.RemoveUserAsync(id, user.Id); + await _removeOrganizationUserCommand.UserLeaveAsync(id, user.Id); } [HttpDelete("{id}")] diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IRemoveOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IRemoveOrganizationUserCommand.cs index 7c1cdf05f8ea..605a5f5aeed1 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IRemoveOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IRemoveOrganizationUserCommand.cs @@ -50,4 +50,11 @@ public interface IRemoveOrganizationUserCommand /// Task> RemoveUsersAsync( Guid organizationId, IEnumerable organizationUserIds, EventSystemUser eventSystemUser); + + /// + /// Removes a user from an organization when they have left voluntarily. This should only be called by the same user who is being removed. + /// + /// Organization to leave. + /// User to leave. + Task UserLeaveAsync(Guid organizationId, Guid userId); } diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommand.cs index fa027f8e470a..e45f109df14d 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommand.cs @@ -114,6 +114,16 @@ await _eventService.LogOrganizationUserEventsAsync( return result.Select(r => (r.OrganizationUser.Id, r.ErrorMessage)); } + public async Task UserLeaveAsync(Guid organizationId, Guid userId) + { + var organizationUser = await _organizationUserRepository.GetByOrganizationAsync(organizationId, userId); + ValidateRemoveUser(organizationId, organizationUser); + + await RepositoryRemoveUserAsync(organizationUser, deletingUserId: null, eventSystemUser: null); + + await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Left); + } + private void ValidateRemoveUser(Guid organizationId, OrganizationUser orgUser) { if (orgUser == null || orgUser.OrganizationId != organizationId) @@ -234,7 +244,7 @@ await _pushRegistrationService.DeleteUserRegistrationOrganizationAsync(devices, await _organizationUserRepository.DeleteManyAsync(organizationUsersToRemove.Select(ou => ou.Id)); foreach (var orgUser in organizationUsersToRemove.Where(ou => ou.UserId.HasValue)) { - await DeleteAndPushUserRegistrationAsync(organizationId, orgUser.UserId.Value); + await DeleteAndPushUserRegistrationAsync(organizationId, orgUser.UserId!.Value); } } diff --git a/test/Api.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs b/test/Api.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs index e58cd05b9dff..f35dbaa5cfb8 100644 --- a/test/Api.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs +++ b/test/Api.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs @@ -130,7 +130,7 @@ public async Task OrganizationsController_UserCannotLeaveOrganizationThatProvide Assert.Contains("Your organization's Single Sign-On settings prevent you from leaving.", exception.Message); - await _removeOrganizationUserCommand.DidNotReceiveWithAnyArgs().RemoveUserAsync(default, default); + await _removeOrganizationUserCommand.DidNotReceiveWithAnyArgs().UserLeaveAsync(default, default); } [Theory, AutoData] @@ -193,7 +193,7 @@ public async Task OrganizationsController_UserCanLeaveOrganizationThatDoesntProv await _sut.Leave(orgId); - await _removeOrganizationUserCommand.Received(1).RemoveUserAsync(orgId, user.Id); + await _removeOrganizationUserCommand.Received(1).UserLeaveAsync(orgId, user.Id); } [Theory, AutoData] diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommandTests.cs index 61371b756e8c..6ab8236b8e23 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommandTests.cs @@ -202,6 +202,14 @@ await sutProvider.GetDependency() .HasConfirmedOwnersExceptAsync( organizationUser.OrganizationId, Arg.Is>(i => i.Contains(organizationUser.Id)), true); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .DeleteAsync(default); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .LogOrganizationUserEventAsync((OrganizationUser)default, default); } [Theory, BitAutoData] @@ -346,9 +354,7 @@ public async Task RemoveUser_ByUserId_Success( [OrganizationUser(type: OrganizationUserType.User)] OrganizationUser organizationUser, SutProvider sutProvider) { - var organizationUserRepository = sutProvider.GetDependency(); - - organizationUserRepository + sutProvider.GetDependency() .GetByOrganizationAsync(organizationUser.OrganizationId, organizationUser.UserId!.Value) .Returns(organizationUser); @@ -361,7 +367,13 @@ public async Task RemoveUser_ByUserId_Success( await sutProvider.Sut.RemoveUserAsync(organizationUser.OrganizationId, organizationUser.UserId.Value); - await sutProvider.GetDependency().Received(1).LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Removed); + await sutProvider.GetDependency() + .Received(1) + .DeleteAsync(organizationUser); + + await sutProvider.GetDependency() + .Received(1) + .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Removed); } [Theory, BitAutoData] @@ -370,6 +382,14 @@ public async Task RemoveUser_ByUserId_NotFound_ThrowsException( { // Act & Assert await Assert.ThrowsAsync(async () => await sutProvider.Sut.RemoveUserAsync(organizationId, userId)); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .DeleteAsync(default); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .LogOrganizationUserEventAsync((OrganizationUser)default, default); } [Theory, BitAutoData] @@ -413,6 +433,14 @@ await sutProvider.GetDependency() organizationUser.OrganizationId, Arg.Is>(i => i.Contains(organizationUser.Id)), Arg.Any()); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .DeleteAsync(default); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .LogOrganizationUserEventAsync((OrganizationUser)default, default); } [Theory, BitAutoData] @@ -424,6 +452,7 @@ public async Task RemoveUsers_WithDeletingUserId_Success( var sutProvider = SutProviderFactory(); var eventDate = sutProvider.GetDependency().GetUtcNow().UtcDateTime; orgUser1.OrganizationId = orgUser2.OrganizationId = deletingUser.OrganizationId; + var organizationUsers = new[] { orgUser1, orgUser2 }; var organizationUserIds = organizationUsers.Select(u => u.Id); @@ -774,6 +803,105 @@ public async Task RemoveUsers_WithEventSystemUser_LastOwner_ThrowsException( Assert.Contains(RemoveOrganizationUserCommand.RemoveLastConfirmedOwnerErrorMessage, exception.Message); } + [Theory, BitAutoData] + public async Task UserLeave_Success( + [OrganizationUser(type: OrganizationUserType.User)] OrganizationUser organizationUser, + SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetByOrganizationAsync(organizationUser.OrganizationId, organizationUser.UserId!.Value) + .Returns(organizationUser); + + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync( + organizationUser.OrganizationId, + Arg.Is>(i => i.Contains(organizationUser.Id)), + Arg.Any()) + .Returns(true); + + await sutProvider.Sut.UserLeaveAsync(organizationUser.OrganizationId, organizationUser.UserId.Value); + + await sutProvider.GetDependency() + .Received(1) + .DeleteAsync(organizationUser); + + await sutProvider.GetDependency() + .Received(1) + .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Left); + } + + [Theory, BitAutoData] + public async Task UserLeave_NotFound_ThrowsException(SutProvider sutProvider, + Guid organizationId, Guid userId) + { + await Assert.ThrowsAsync(async () => await sutProvider.Sut.UserLeaveAsync(organizationId, userId)); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .DeleteAsync(default); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .LogOrganizationUserEventAsync((OrganizationUser)default, default); + } + + [Theory, BitAutoData] + public async Task UserLeave_InvalidUser_ThrowsException(OrganizationUser organizationUser, + SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetByOrganizationAsync(organizationUser.OrganizationId, organizationUser.UserId!.Value) + .Returns(organizationUser); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.UserLeaveAsync(Guid.NewGuid(), organizationUser.UserId.Value)); + + Assert.Contains(RemoveOrganizationUserCommand.UserNotFoundErrorMessage, exception.Message); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .DeleteAsync(default); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .LogOrganizationUserEventAsync((OrganizationUser)default, default); + } + + [Theory, BitAutoData] + public async Task UserLeave_RemovingLastOwner_ThrowsException( + [OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser organizationUser, + SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetByOrganizationAsync(organizationUser.OrganizationId, organizationUser.UserId!.Value) + .Returns(organizationUser); + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync( + organizationUser.OrganizationId, + Arg.Is>(i => i.Contains(organizationUser.Id)), + Arg.Any()) + .Returns(false); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.UserLeaveAsync(organizationUser.OrganizationId, organizationUser.UserId.Value)); + + Assert.Contains(RemoveOrganizationUserCommand.RemoveLastConfirmedOwnerErrorMessage, exception.Message); + _ = sutProvider.GetDependency() + .Received(1) + .HasConfirmedOwnersExceptAsync( + organizationUser.OrganizationId, + Arg.Is>(i => i.Contains(organizationUser.Id)), + Arg.Any()); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .DeleteAsync(default); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .LogOrganizationUserEventAsync((OrganizationUser)default, default); + } + /// /// Returns a new SutProvider with a FakeTimeProvider registered in the Sut. ///