Skip to content

Commit

Permalink
[PM-10338] Update the Organization 'Leave' endpoint to log EventType.…
Browse files Browse the repository at this point in the history 
…OrganizationUser_Left (#4908)

* 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 <[email protected]>
  • Loading branch information
@r-tome @withinfocus
r-tome and withinfocus authored Dec 10, 2024
1 parent 2212f55 commit 127f1fd
Show file tree
Hide file tree
Showing 5 changed files with 153 additions and 8 deletions.
2 changes: 1 addition & 1 deletion src/Api/AdminConsole/Controllers/OrganizationsController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}")]
Expand Down
7 changes: 7 additions & 0 deletions ...nsole/OrganizationFeatures/OrganizationUsers/Interfaces/IRemoveOrganizationUserCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,11 @@ public interface IRemoveOrganizationUserCommand
/// </returns>
Task<IEnumerable<(Guid OrganizationUserId, string ErrorMessage)>> RemoveUsersAsync(
Guid organizationId, IEnumerable<Guid> organizationUserIds, EventSystemUser eventSystemUser);

/// <summary>
/// Removes a user from an organization when they have left voluntarily. This should only be called by the same user who is being removed.
/// </summary>
/// <param name="organizationId">Organization to leave.</param>
/// <param name="userId">User to leave.</param>
Task UserLeaveAsync(Guid organizationId, Guid userId);
}
12 changes: 11 additions & 1 deletion ...Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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);
}
}

Expand Down
4 changes: 2 additions & 2 deletions test/Api.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand Down
136 changes: 132 additions & 4 deletions ...AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommandTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,14 @@ await sutProvider.GetDependency<IHasConfirmedOwnersExceptQuery>()
.HasConfirmedOwnersExceptAsync(
organizationUser.OrganizationId,
Arg.Is<IEnumerable<Guid>>(i => i.Contains(organizationUser.Id)), true);

await sutProvider.GetDependency<IOrganizationUserRepository>()
.DidNotReceiveWithAnyArgs()
.DeleteAsync(default);

await sutProvider.GetDependency<IEventService>()
.DidNotReceiveWithAnyArgs()
.LogOrganizationUserEventAsync((OrganizationUser)default, default);
}

[Theory, BitAutoData]
Expand Down Expand Up @@ -346,9 +354,7 @@ public async Task RemoveUser_ByUserId_Success(
[OrganizationUser(type: OrganizationUserType.User)] OrganizationUser organizationUser,
SutProvider<RemoveOrganizationUserCommand> sutProvider)
{
var organizationUserRepository = sutProvider.GetDependency<IOrganizationUserRepository>();

organizationUserRepository
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetByOrganizationAsync(organizationUser.OrganizationId, organizationUser.UserId!.Value)
.Returns(organizationUser);

Expand All @@ -361,7 +367,13 @@ public async Task RemoveUser_ByUserId_Success(

await sutProvider.Sut.RemoveUserAsync(organizationUser.OrganizationId, organizationUser.UserId.Value);

await sutProvider.GetDependency<IEventService>().Received(1).LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Removed);
await sutProvider.GetDependency<IOrganizationUserRepository>()
.Received(1)
.DeleteAsync(organizationUser);

await sutProvider.GetDependency<IEventService>()
.Received(1)
.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Removed);
}

[Theory, BitAutoData]
Expand All @@ -370,6 +382,14 @@ public async Task RemoveUser_ByUserId_NotFound_ThrowsException(
{
// Act & Assert
await Assert.ThrowsAsync<NotFoundException>(async () => await sutProvider.Sut.RemoveUserAsync(organizationId, userId));

await sutProvider.GetDependency<IOrganizationUserRepository>()
.DidNotReceiveWithAnyArgs()
.DeleteAsync(default);

await sutProvider.GetDependency<IEventService>()
.DidNotReceiveWithAnyArgs()
.LogOrganizationUserEventAsync((OrganizationUser)default, default);
}

[Theory, BitAutoData]
Expand Down Expand Up @@ -413,6 +433,14 @@ await sutProvider.GetDependency<IHasConfirmedOwnersExceptQuery>()
organizationUser.OrganizationId,
Arg.Is<IEnumerable<Guid>>(i => i.Contains(organizationUser.Id)),
Arg.Any<bool>());

await sutProvider.GetDependency<IOrganizationUserRepository>()
.DidNotReceiveWithAnyArgs()
.DeleteAsync(default);

await sutProvider.GetDependency<IEventService>()
.DidNotReceiveWithAnyArgs()
.LogOrganizationUserEventAsync((OrganizationUser)default, default);
}

[Theory, BitAutoData]
Expand All @@ -424,6 +452,7 @@ public async Task RemoveUsers_WithDeletingUserId_Success(
var sutProvider = SutProviderFactory();
var eventDate = sutProvider.GetDependency<FakeTimeProvider>().GetUtcNow().UtcDateTime;
orgUser1.OrganizationId = orgUser2.OrganizationId = deletingUser.OrganizationId;

var organizationUsers = new[] { orgUser1, orgUser2 };
var organizationUserIds = organizationUsers.Select(u => u.Id);

Expand Down Expand Up @@ -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<RemoveOrganizationUserCommand> sutProvider)
{
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetByOrganizationAsync(organizationUser.OrganizationId, organizationUser.UserId!.Value)
.Returns(organizationUser);

sutProvider.GetDependency<IHasConfirmedOwnersExceptQuery>()
.HasConfirmedOwnersExceptAsync(
organizationUser.OrganizationId,
Arg.Is<IEnumerable<Guid>>(i => i.Contains(organizationUser.Id)),
Arg.Any<bool>())
.Returns(true);

await sutProvider.Sut.UserLeaveAsync(organizationUser.OrganizationId, organizationUser.UserId.Value);

await sutProvider.GetDependency<IOrganizationUserRepository>()
.Received(1)
.DeleteAsync(organizationUser);

await sutProvider.GetDependency<IEventService>()
.Received(1)
.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Left);
}

[Theory, BitAutoData]
public async Task UserLeave_NotFound_ThrowsException(SutProvider<RemoveOrganizationUserCommand> sutProvider,
Guid organizationId, Guid userId)
{
await Assert.ThrowsAsync<NotFoundException>(async () => await sutProvider.Sut.UserLeaveAsync(organizationId, userId));

await sutProvider.GetDependency<IOrganizationUserRepository>()
.DidNotReceiveWithAnyArgs()
.DeleteAsync(default);

await sutProvider.GetDependency<IEventService>()
.DidNotReceiveWithAnyArgs()
.LogOrganizationUserEventAsync((OrganizationUser)default, default);
}

[Theory, BitAutoData]
public async Task UserLeave_InvalidUser_ThrowsException(OrganizationUser organizationUser,
SutProvider<RemoveOrganizationUserCommand> sutProvider)
{
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetByOrganizationAsync(organizationUser.OrganizationId, organizationUser.UserId!.Value)
.Returns(organizationUser);

var exception = await Assert.ThrowsAsync<NotFoundException>(
() => sutProvider.Sut.UserLeaveAsync(Guid.NewGuid(), organizationUser.UserId.Value));

Assert.Contains(RemoveOrganizationUserCommand.UserNotFoundErrorMessage, exception.Message);

await sutProvider.GetDependency<IOrganizationUserRepository>()
.DidNotReceiveWithAnyArgs()
.DeleteAsync(default);

await sutProvider.GetDependency<IEventService>()
.DidNotReceiveWithAnyArgs()
.LogOrganizationUserEventAsync((OrganizationUser)default, default);
}

[Theory, BitAutoData]
public async Task UserLeave_RemovingLastOwner_ThrowsException(
[OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser organizationUser,
SutProvider<RemoveOrganizationUserCommand> sutProvider)
{
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetByOrganizationAsync(organizationUser.OrganizationId, organizationUser.UserId!.Value)
.Returns(organizationUser);
sutProvider.GetDependency<IHasConfirmedOwnersExceptQuery>()
.HasConfirmedOwnersExceptAsync(
organizationUser.OrganizationId,
Arg.Is<IEnumerable<Guid>>(i => i.Contains(organizationUser.Id)),
Arg.Any<bool>())
.Returns(false);

var exception = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.UserLeaveAsync(organizationUser.OrganizationId, organizationUser.UserId.Value));

Assert.Contains(RemoveOrganizationUserCommand.RemoveLastConfirmedOwnerErrorMessage, exception.Message);
_ = sutProvider.GetDependency<IHasConfirmedOwnersExceptQuery>()
.Received(1)
.HasConfirmedOwnersExceptAsync(
organizationUser.OrganizationId,
Arg.Is<IEnumerable<Guid>>(i => i.Contains(organizationUser.Id)),
Arg.Any<bool>());

await sutProvider.GetDependency<IOrganizationUserRepository>()
.DidNotReceiveWithAnyArgs()
.DeleteAsync(default);

await sutProvider.GetDependency<IEventService>()
.DidNotReceiveWithAnyArgs()
.LogOrganizationUserEventAsync((OrganizationUser)default, default);
}

/// <summary>
/// Returns a new SutProvider with a FakeTimeProvider registered in the Sut.
/// </summary>
Expand Down

0 comments on commit 127f1fd

Please sign in to comment.