[PS-1806] Fix UpdateUsersAsync for EF (#2387)

* Fix UpdateUsersAsync

* Update to make single call to DB
* Loop through requested CollectionUsers
* Delete unused code

* Address PR Feedback

src/Infrastructure.EntityFramework/Repositories/CollectionRepository.cs

@@ -233,17 +233,48 @@ public class CollectionRepository : Repository<Core.Entities.Collection, Collect
         }
     }
 
-    public async Task UpdateUsersAsync(Guid id, IEnumerable<SelectionReadOnly> users)
+    public async Task UpdateUsersAsync(Guid id, IEnumerable<SelectionReadOnly> requestedUsers)
     {
         using (var scope = ServiceScopeFactory.CreateScope())
         {
             var dbContext = GetDatabaseContext(scope);
-            var procedure = new CollectionUserUpdateUsersQuery(id, users);
+
-            var updateData = await procedure.Update.BuildInMemory(dbContext);
+            var organizationId = await dbContext.Collections
-            dbContext.UpdateRange(updateData);
+                .Where(c => c.Id == id)
-            var insertData = await procedure.Insert.BuildInMemory(dbContext);
+                .Select(c => c.OrganizationId)
-            await dbContext.AddRangeAsync(insertData);
+                .FirstOrDefaultAsync();
-            dbContext.RemoveRange(await procedure.Delete.Run(dbContext).ToListAsync());
+
+            var existingCollectionUsers = await dbContext.CollectionUsers
+                .Where(cu => cu.CollectionId == id)
+                .ToListAsync();
+
+            foreach (var requestedUser in requestedUsers)
+            {
+                var existingCollectionUser = existingCollectionUsers.FirstOrDefault(cu => cu.OrganizationUserId == requestedUser.Id);
+                if (existingCollectionUser == null)
+                {
+                    // This is a brand new entry
+                    dbContext.CollectionUsers.Add(new CollectionUser
+                    {
+                        CollectionId = id,
+                        OrganizationUserId = requestedUser.Id,
+                        HidePasswords = requestedUser.HidePasswords,
+                        ReadOnly = requestedUser.ReadOnly,
+                    });
+                    continue;
+                }
+
+                // It already exists, update it
+                existingCollectionUser.HidePasswords = requestedUser.HidePasswords;
+                existingCollectionUser.ReadOnly = requestedUser.ReadOnly;
+                dbContext.CollectionUsers.Update(existingCollectionUser);
+            }
+
+            // Remove all existing ones that are no longer requested
+            var requestedUserIds = requestedUsers.Select(u => u.Id);
+            dbContext.CollectionUsers.RemoveRange(existingCollectionUsers.Where(cu => !requestedUserIds.Contains(cu.OrganizationUserId)));
+            await UserBumpAccountRevisionDateByCollectionId(id, organizationId);
+            await dbContext.SaveChangesAsync();
         }
     }
 }

src/Infrastructure.EntityFramework/Repositories/OrganizationUserRepository.cs

@@ -333,7 +333,7 @@ public class OrganizationUserRepository : Repository<Core.Entities.OrganizationU
                         HidePasswords = requestedCollection.HidePasswords,
                         ReadOnly = requestedCollection.ReadOnly,
                     });
-                    break;
+                    continue;
                 }
 
                 // It already exists, update it

src/Infrastructure.EntityFramework/Repositories/Queries/CollectionUserUpdateUsersQuery.cs

@@ -1,116 +0,0 @@
-using Bit.Core.Models.Data;
-using Bit.Infrastructure.EntityFramework.Models;
-using Microsoft.EntityFrameworkCore;
-
-namespace Bit.Infrastructure.EntityFramework.Repositories.Queries;
-
-public class CollectionUserUpdateUsersQuery
-{
-    public readonly CollectionUserUpdateUsersInsertQuery Insert;
-    public readonly CollectionUserUpdateUsersUpdateQuery Update;
-    public readonly CollectionUserUpdateUsersDeleteQuery Delete;
-
-    public CollectionUserUpdateUsersQuery(Guid collectionId, IEnumerable<SelectionReadOnly> users)
-    {
-        Insert = new CollectionUserUpdateUsersInsertQuery(collectionId, users);
-        Update = new CollectionUserUpdateUsersUpdateQuery(collectionId, users);
-        Delete = new CollectionUserUpdateUsersDeleteQuery(collectionId, users);
-    }
-}
-
-public class CollectionUserUpdateUsersInsertQuery : IQuery<OrganizationUser>
-{
-    private readonly Guid _collectionId;
-    private readonly IEnumerable<SelectionReadOnly> _users;
-
-    public CollectionUserUpdateUsersInsertQuery(Guid collectionId, IEnumerable<SelectionReadOnly> users)
-    {
-        _collectionId = collectionId;
-        _users = users;
-    }
-
-    public IQueryable<OrganizationUser> Run(DatabaseContext dbContext)
-    {
-        var orgId = dbContext.Collections.FirstOrDefault(c => c.Id == _collectionId)?.OrganizationId;
-        var organizationUserIds = _users.Select(u => u.Id);
-        var insertQuery = from ou in dbContext.OrganizationUsers
-                          where
-                              organizationUserIds.Contains(ou.Id) &&
-                              ou.OrganizationId == orgId &&
-                              !dbContext.CollectionUsers.Any(
-                                  x => x.CollectionId != _collectionId && x.OrganizationUserId == ou.Id)
-                          select ou;
-        return insertQuery;
-    }
-
-    public async Task<IEnumerable<CollectionUser>> BuildInMemory(DatabaseContext dbContext)
-    {
-        var data = await Run(dbContext).ToListAsync();
-        var collectionUsers = data.Select(x => new CollectionUser()
-        {
-            CollectionId = _collectionId,
-            OrganizationUserId = x.Id,
-            ReadOnly = _users.FirstOrDefault(u => u.Id.Equals(x.Id)).ReadOnly,
-            HidePasswords = _users.FirstOrDefault(u => u.Id.Equals(x.Id)).HidePasswords,
-        });
-        return collectionUsers;
-    }
-}
-
-public class CollectionUserUpdateUsersUpdateQuery : IQuery<CollectionUser>
-{
-    private readonly Guid _collectionId;
-    private readonly IEnumerable<SelectionReadOnly> _users;
-
-    public CollectionUserUpdateUsersUpdateQuery(Guid collectionId, IEnumerable<SelectionReadOnly> users)
-    {
-        _collectionId = collectionId;
-        _users = users;
-    }
-
-    public IQueryable<CollectionUser> Run(DatabaseContext dbContext)
-    {
-        var orgId = dbContext.Collections.FirstOrDefault(c => c.Id == _collectionId)?.OrganizationId;
-        var ids = _users.Select(x => x.Id);
-        var updateQuery = from target in dbContext.CollectionUsers
-                          where target.CollectionId == _collectionId &&
-                              ids.Contains(target.OrganizationUserId)
-                          select target;
-        return updateQuery;
-    }
-
-    public async Task<IEnumerable<CollectionUser>> BuildInMemory(DatabaseContext dbContext)
-    {
-        var data = await Run(dbContext).ToListAsync();
-        var collectionUsers = data.Select(x => new CollectionUser
-        {
-            CollectionId = _collectionId,
-            OrganizationUserId = x.OrganizationUserId,
-            ReadOnly = _users.FirstOrDefault(u => u.Id.Equals(x.OrganizationUserId)).ReadOnly,
-            HidePasswords = _users.FirstOrDefault(u => u.Id.Equals(x.OrganizationUserId)).HidePasswords,
-        });
-        return collectionUsers;
-    }
-}
-
-public class CollectionUserUpdateUsersDeleteQuery : IQuery<CollectionUser>
-{
-    private readonly Guid _collectionId;
-    private readonly IEnumerable<SelectionReadOnly> _users;
-
-    public CollectionUserUpdateUsersDeleteQuery(Guid collectionId, IEnumerable<SelectionReadOnly> users)
-    {
-        _collectionId = collectionId;
-        _users = users;
-    }
-
-    public IQueryable<CollectionUser> Run(DatabaseContext dbContext)
-    {
-        var orgId = dbContext.Collections.FirstOrDefault(c => c.Id == _collectionId)?.OrganizationId;
-        var deleteQuery = from cu in dbContext.CollectionUsers
-                          where !dbContext.Users.Any(
-                              u => u.Id == cu.OrganizationUserId)
-                          select cu;
-        return deleteQuery;
-    }
-}