From 4af676e959133089d8a97e2435551f4164e5c041 Mon Sep 17 00:00:00 2001 From: Brandon Date: Tue, 3 Jun 2025 17:32:31 -0400 Subject: [PATCH] cr feedback --- .../OrganizationGroupImportData.cs | 23 ++++++-- .../OrganizationUserImportData.cs | 22 ++++++-- .../ImportOrganizationUserCommand.cs | 53 +++++-------------- 3 files changed, 49 insertions(+), 49 deletions(-) diff --git a/src/Core/AdminConsole/Models/Data/Organizations/OrganizationGroupImportData.cs b/src/Core/AdminConsole/Models/Data/Organizations/OrganizationGroupImportData.cs index 562fa7e766..ab059086ec 100644 --- a/src/Core/AdminConsole/Models/Data/Organizations/OrganizationGroupImportData.cs +++ b/src/Core/AdminConsole/Models/Data/Organizations/OrganizationGroupImportData.cs @@ -5,8 +5,23 @@ namespace Bit.Core.Models.Data.Organizations; public class OrganizationGroupImportData { - public IEnumerable Groups { get; set; } - public ICollection ExistingGroups { get; set; } - public IEnumerable ExistingExternalGroups { get; set; } - public IDictionary GroupsDict { get; set; } + public IEnumerable Groups { get; init; } + public ICollection ExistingGroups { get; init; } + public IEnumerable ExistingExternalGroups { get; init; } + public IDictionary GroupsDict { get; init; } + + public OrganizationGroupImportData(IEnumerable groups, ICollection existingGroups) + { + Groups = groups; + GroupsDict = groups.ToDictionary(g => g.Group.ExternalId); + ExistingGroups = existingGroups; + ExistingExternalGroups = GetExistingExternalGroups(existingGroups); + } + + private IEnumerable GetExistingExternalGroups(ICollection existingGroups) + { + return existingGroups + .Where(u => !string.IsNullOrWhiteSpace(u.ExternalId)) + .ToList(); + } } diff --git a/src/Core/AdminConsole/Models/Data/Organizations/OrganizationUsers/OrganizationUserImportData.cs b/src/Core/AdminConsole/Models/Data/Organizations/OrganizationUsers/OrganizationUserImportData.cs index aa779b8fd4..d9d7fd3282 100644 --- a/src/Core/AdminConsole/Models/Data/Organizations/OrganizationUsers/OrganizationUserImportData.cs +++ b/src/Core/AdminConsole/Models/Data/Organizations/OrganizationUsers/OrganizationUserImportData.cs @@ -2,9 +2,23 @@ public class OrganizationUserImportData { - public HashSet NewUsersSet { get; set; } - public ICollection ExistingUsers { get; set; } - public IEnumerable ExistingExternalUsers { get; set; } - public Dictionary ExistingExternalUsersIdDict { get; set; } + public HashSet NewUsersSet { get; init; } + public ICollection ExistingUsers { get; init; } + public IEnumerable ExistingExternalUsers { get; init; } + public Dictionary ExistingExternalUsersIdDict { get; init; } + public OrganizationUserImportData(ICollection existingUsers, HashSet newUsersSet) + { + NewUsersSet = newUsersSet; + ExistingUsers = existingUsers; + ExistingExternalUsers = GetExistingExternalUsers(existingUsers); + ExistingExternalUsersIdDict = GetExistingExternalUsers(existingUsers).ToDictionary(u => u.ExternalId, u => u.Id); + } + + private IEnumerable GetExistingExternalUsers(ICollection existingUsers) + { + return existingUsers + .Where(u => !string.IsNullOrWhiteSpace(u.ExternalId)) + .ToList(); + } } diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ImportOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ImportOrganizationUserCommand.cs index 3ab1a2d6b1..1111d01f3c 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ImportOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ImportOrganizationUserCommand.cs @@ -29,6 +29,7 @@ public class ImportOrganizationUserCommand : IImportOrganizationUserCommand private readonly IOrganizationService _organizationService; private readonly IInviteOrganizationUsersCommand _inviteOrganizationUsersCommand; private readonly IPricingClient _pricingClient; + private readonly TimeProvider _timeProvider; private readonly EventSystemUser _EventSystemUser = EventSystemUser.PublicApi; @@ -40,7 +41,8 @@ public class ImportOrganizationUserCommand : IImportOrganizationUserCommand ICurrentContext currentContext, IOrganizationService organizationService, IInviteOrganizationUsersCommand inviteOrganizationUsersCommand, - IPricingClient pricingClient + IPricingClient pricingClient, + TimeProvider timeProvider ) { _organizationRepository = organizationRepository; @@ -52,6 +54,7 @@ public class ImportOrganizationUserCommand : IImportOrganizationUserCommand _organizationService = organizationService; _inviteOrganizationUsersCommand = inviteOrganizationUsersCommand; _pricingClient = pricingClient; + _timeProvider = timeProvider; } public async Task ImportAsync(Guid organizationId, @@ -72,15 +75,7 @@ public class ImportOrganizationUserCommand : IImportOrganizationUserCommand } var existingUsers = await _organizationUserRepository.GetManyDetailsByOrganizationAsync(organizationId); - - var importUserData = new OrganizationUserImportData - { - NewUsersSet = new HashSet(newUsers?.Select(u => u.ExternalId) ?? new List()), - ExistingUsers = existingUsers, - ExistingExternalUsers = GetExistingExternalUsers(existingUsers), - ExistingExternalUsersIdDict = GetExistingExternalUsers(existingUsers).ToDictionary(u => u.ExternalId, u => u.Id) - }; - + var importUserData = new OrganizationUserImportData(existingUsers, new HashSet(newUsers?.Select(u => u.ExternalId) ?? new List())); var events = new List<(OrganizationUserUserDetails ou, EventType e, DateTime? d)>(); await RemoveExistingExternalUsers(removeUserExternalIds, events, importUserData); @@ -90,7 +85,7 @@ public class ImportOrganizationUserCommand : IImportOrganizationUserCommand await OverwriteExisting(events, importUserData); } - await UpsertExistingUsers(organization, newUsers, importUserData); + await UpsertExistingUsers(newUsers, importUserData); await AddNewUsers(organization, newUsers, importUserData); @@ -136,10 +131,7 @@ public class ImportOrganizationUserCommand : IImportOrganizationUserCommand ); } - private async Task UpsertExistingUsers(Organization organization, - IEnumerable newUsers, - OrganizationUserImportData importUserData - ) + private async Task UpsertExistingUsers(IEnumerable newUsers, OrganizationUserImportData importUserData) { if (!newUsers.Any()) { @@ -157,7 +149,9 @@ public class ImportOrganizationUserCommand : IImportOrganizationUserCommand foreach (var user in newAndExistingUsersIntersection) { - var organizationUser = organizationUsers[existingUsersEmailsDict[user].Id]; + existingUsersEmailsDict.TryGetValue(user, out var existingUser); + organizationUsers.TryGetValue(existingUser.Id, out var organizationUser); + if (organizationUser != null) { organizationUser.ExternalId = newUsersEmailsDict[user].ExternalId; @@ -172,14 +166,11 @@ public class ImportOrganizationUserCommand : IImportOrganizationUserCommand IEnumerable newUsers, OrganizationUserImportData importUserData) { - - var hasStandaloneSecretsManager = await _paymentService.HasSecretsManagerStandalone(organization); var userInvites = new List(); foreach (var user in newUsers) { - var invite = new OrganizationUserInviteCommandModel(user.Email, user.ExternalId); - userInvites.Add(new OrganizationUserInviteCommandModel(invite, hasStandaloneSecretsManager)); + userInvites.Add(new OrganizationUserInviteCommandModel(user.Email, user.ExternalId)); } var commandResult = await InviteUsersAsync(userInvites, organization); @@ -246,13 +237,7 @@ public class ImportOrganizationUserCommand : IImportOrganizationUserCommand } var existingGroups = await _groupRepository.GetManyByOrganizationIdAsync(organization.Id); - var importGroupData = new OrganizationGroupImportData - { - Groups = groups, - GroupsDict = groups.ToDictionary(g => g.Group.ExternalId), - ExistingGroups = existingGroups, - ExistingExternalGroups = GetExistingExternalGroups(existingGroups) - }; + var importGroupData = new OrganizationGroupImportData(groups, existingGroups); await SaveNewGroups(importGroupData, importUserData); await UpdateExistingGroups(importGroupData, importUserData, organization); @@ -316,20 +301,6 @@ public class ImportOrganizationUserCommand : IImportOrganizationUserCommand } - private IEnumerable GetExistingExternalUsers(ICollection existingUsers) - { - return existingUsers - .Where(u => !string.IsNullOrWhiteSpace(u.ExternalId)) - .ToList(); - } - - private IEnumerable GetExistingExternalGroups(ICollection existingGroups) - { - return existingGroups - .Where(u => !string.IsNullOrWhiteSpace(u.ExternalId)) - .ToList(); - } - private async Task GetOrgById(Guid id) { return await _organizationRepository.GetByIdAsync(id);