From be23fe642065b4453e4e83bb4233412ad7abd503 Mon Sep 17 00:00:00 2001 From: Brandon Date: Thu, 15 May 2025 14:59:11 -0400 Subject: [PATCH] remove redundant code, remove looping db call, refactor tests --- .../ImportOrganizationUserCommand.cs | 36 ++++------------ .../ImportOrganizationUserCommandTests.cs | 42 +++++++++---------- 2 files changed, 26 insertions(+), 52 deletions(-) diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ImportOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ImportOrganizationUserCommand.cs index f1d70c646b..f5f9016c10 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ImportOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ImportOrganizationUserCommand.cs @@ -159,10 +159,13 @@ public class ImportOrganizationUserCommand : IImportOrganizationUserCommand var newUsersEmailsDict = newUsers.ToDictionary(u => u.Email); var usersToAttach = existingUsersEmailsDict.Keys.Intersect(newUsersEmailsDict.Keys).ToList(); var usersToUpsert = new List(); + + var orgUsers = (await _organizationUserRepository.GetManyAsync(existingUsers.Select(u => u.Id).ToList())).ToDictionary(u => u.Id); + foreach (var user in usersToAttach) { var orgUserDetails = existingUsersEmailsDict[user]; - var orgUser = await _organizationUserRepository.GetByIdAsync(orgUserDetails.Id); + var orgUser = orgUsers[existingUsersEmailsDict[user].Id]; if (orgUser != null) { orgUser.ExternalId = newUsersEmailsDict[user].ExternalId; @@ -179,38 +182,13 @@ public class ImportOrganizationUserCommand : IImportOrganizationUserCommand OrganizationUserImportData importUserData) { - var existingUsersSet = new HashSet(importUserData.ExistingExternalUsersIdDict.Keys); - var usersToAdd = importUserData.NewUsersSet.Except(existingUsersSet).ToList(); - - var seatsAvailable = int.MaxValue; - var enoughSeatsAvailable = true; - if (organization.Seats.HasValue) - { - var occupiedSeats = await _organizationUserRepository.GetOccupiedSeatCountByOrganizationIdAsync(organization.Id); - seatsAvailable = organization.Seats.Value - occupiedSeats; - enoughSeatsAvailable = seatsAvailable >= usersToAdd.Count; - } - var hasStandaloneSecretsManager = await _paymentService.HasSecretsManagerStandalone(organization); - var userInvites = new List(); + foreach (var user in newUsers) { - if (!usersToAdd.Contains(user.ExternalId) || string.IsNullOrWhiteSpace(user.Email)) - { - continue; - } - - try - { - var invite = new OrganizationUserInviteCommandModel(user.Email, user.ExternalId); - userInvites.Add(new OrganizationUserInviteCommandModel(invite, hasStandaloneSecretsManager)); - } - catch (BadRequestException) - { - // Thrown when the user is already invited to the organization - continue; - } + var invite = new OrganizationUserInviteCommandModel(user.Email, user.ExternalId); + userInvites.Add(new OrganizationUserInviteCommandModel(invite, hasStandaloneSecretsManager)); } var commandResult = await InviteUsersAsync(userInvites, organization); diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ImportOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ImportOrganizationUserCommandTests.cs index 2490651052..ac308bf8fd 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ImportOrganizationUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ImportOrganizationUserCommandTests.cs @@ -30,7 +30,7 @@ public class ImportOrganizationUserCommandTests private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory = new FakeDataProtectorTokenFactory(); [Theory, PaidOrganizationCustomize, BitAutoData] - public async Task OrgImportCreateNewUsers( + public async Task OrgImportCallsInviteOrgUserCommand( SutProvider sutProvider, Organization org, List existingUsers, @@ -39,24 +39,17 @@ public class ImportOrganizationUserCommandTests { SetupOrganizationConfigForImport(sutProvider, org, existingUsers, newUsers); - var expectedNewUsersCount = newUsers.Count - 1; - var invitedOrganizationUsers = new List(); - var invites = new List(); - - foreach (var u in newUsers) - { - invitedOrganizationUsers.Add(new OrganizationUser { Email = u.Email + "@test.com", ExternalId = u.ExternalId }); - invites.Add(new OrganizationUserInviteCommandModel(u.Email + "@test.com", u.ExternalId)); - } - - var inviteCommandModel = new InviteOrganizationUsersRequest(invites.ToArray(), new InviteOrganization(org, null), org.Id, DateTimeOffset.UtcNow); - newUsers.Add(new ImportedOrganizationUser { Email = existingUsers.First().Email, ExternalId = existingUsers.First().ExternalId }); + foreach (var u in newUsers) + { + u.Email += "@bitwardentest.com"; + } + existingUsers.First().Type = OrganizationUserType.Owner; sutProvider.GetDependency().GetByIdAsync(org.Id).Returns(org); @@ -64,18 +57,13 @@ public class ImportOrganizationUserCommandTests sutProvider.GetDependency().GetManyDetailsByOrganizationAsync(org.Id).Returns(existingUsers); sutProvider.GetDependency().GetCountByOrganizationIdAsync(org.Id).Returns(existingUsers.Count); sutProvider.GetDependency().ManageUsers(org.Id).Returns(true); - // Use the arranged command model - sutProvider.GetDependency().InviteImportedOrganizationUsersAsync(inviteCommandModel, org.Id) - // assert against this returned CommandResult response - .Returns(new Success(new InviteOrganizationUsersResponse(invitedOrganizationUsers, org.Id))); + sutProvider.GetDependency().InviteImportedOrganizationUsersAsync(Arg.Any(), org.Id) + .Returns(new Success(new InviteOrganizationUsersResponse(org.Id))); await sutProvider.Sut.ImportAsync(org.Id, newGroups, newUsers, new List(), false, EventSystemUser.PublicApi); await sutProvider.GetDependency().Received(1) - .InviteImportedOrganizationUsersAsync(Arg.Is( - // These are the invites that should get populated from the CommandResult response above - request => request.Invites.Count() == expectedNewUsersCount - ), org.Id); + .InviteImportedOrganizationUsersAsync(Arg.Any(), org.Id); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() .UpsertAsync(default); await sutProvider.GetDependency().Received(1) @@ -102,6 +90,15 @@ public class ImportOrganizationUserCommandTests var reInvitedUser = existingUsers.First(); reInvitedUser.ExternalId = null; + foreach (var u in existingUsers) + { + u.Email += "@bitwardentest.com"; + } + foreach (var u in newUsers) + { + u.Email += "@bitwardentest.com"; + } + newUsers.Add(new ImportedOrganizationUser { Email = reInvitedUser.Email, @@ -111,11 +108,10 @@ public class ImportOrganizationUserCommandTests sutProvider.GetDependency().GetByIdAsync(org.Id).Returns(org); sutProvider.GetDependency().GetManyDetailsByOrganizationAsync(org.Id).Returns(existingUsers); sutProvider.GetDependency().GetCountByOrganizationIdAsync(org.Id).Returns(existingUsers.Count); - sutProvider.GetDependency().GetByIdAsync(reInvitedUser.Id).Returns(new OrganizationUser { Id = reInvitedUser.Id }); + sutProvider.GetDependency().GetManyAsync(Arg.Any>()).Returns(new List { new OrganizationUser { Id = reInvitedUser.Id } }); sutProvider.GetDependency().InviteImportedOrganizationUsersAsync(Arg.Any(), org.Id) .Returns(new Success(new InviteOrganizationUsersResponse(org.Id))); - await sutProvider.Sut.ImportAsync(org.Id, newGroups, newUsers, new List(), false, EventSystemUser.PublicApi); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs()