Convert pending group addition table into isPending field

backend/src/ee/services/group/group-dal.ts

@@ -84,36 +84,14 @@ export const groupDALFactory = (db: TDbClient) => {
             db.raw("?", [groupId])
           );
         })
-        .leftJoin(TableName.PendingGroupAddition, function () {
-          this.on(`${TableName.PendingGroupAddition}.userId`, "=", `${TableName.Users}.id`).andOn(
-            `${TableName.PendingGroupAddition}.groupId`,
-            "=",
-            db.raw("?", [groupId])
-          );
-        })
-        .select<
-          {
-            id: string;
-            groupId: string;
-            email: string;
-            username: string;
-            firstName: string;
-            lastName: string;
-            userId: string;
-            isPartOfGroup: boolean;
-          }[]
-        >(
+        .select(
           db.ref("id").withSchema(TableName.OrgMembership),
           db.ref("groupId").withSchema(TableName.UserGroupMembership),
           db.ref("email").withSchema(TableName.Users),
           db.ref("username").withSchema(TableName.Users),
           db.ref("firstName").withSchema(TableName.Users),
           db.ref("lastName").withSchema(TableName.Users),
-          db.ref("id").withSchema(TableName.Users).as("userId"),
-          db.raw('CASE WHEN ?? IS NOT NULL OR ?? IS NOT NULL THEN TRUE ELSE FALSE END AS "isPartOfGroup"', [
-            `${TableName.UserGroupMembership}.groupId`,
-            `${TableName.PendingGroupAddition}.groupId`
-          ])
+          db.ref("id").withSchema(TableName.Users).as("userId")
         )
         .where({ isGhost: false })
         .offset(offset);
@@ -128,14 +106,16 @@ export const groupDALFactory = (db: TDbClient) => {
 
       const members = await query;
 
-      return members.map(({ email, username: memberUsername, firstName, lastName, userId, isPartOfGroup }) => ({
-        id: userId,
-        email,
-        username: memberUsername,
-        firstName,
-        lastName,
-        isPartOfGroup
-      }));
+      return members.map(
+        ({ email, username: memberUsername, firstName, lastName, userId, groupId: memberGroupId }) => ({
+          id: userId,
+          email,
+          username: memberUsername,
+          firstName,
+          lastName,
+          isPartOfGroup: !!memberGroupId
+        })
+      );
     } catch (error) {
       throw new DatabaseError({ error, name: "Find all org members" });
     }

backend/src/ee/services/group/group-fns.ts

@@ -5,24 +5,143 @@ import { decryptAsymmetric, encryptAsymmetric, infisicalSymmetricDecrypt } from
 import { BadRequestError, ScimRequestError } from "@app/lib/errors";
 
 import {
+  TAddUsersToGroup,
   TAddUsersToGroupByUserIds,
-  TAddUsersToGroupDirectly,
-  TAddUsersToPendingGroupAdditions,
   TConvertPendingGroupAdditionsToGroupMemberships,
-  TRemoveUsersFromGroupByUserIds,
-  TRemoveUsersFromGroupDirectly,
-  TRemoveUsersFromPendingGroupAdditions
+  TRemoveUsersFromGroupByUserIds
 } from "./group-types";
 
-/**
- * Add users with usernames [usernames] to group [group] directly.
- * - Users must have finished completing their account and have private key(s).
- * @param {group} group - group to add user(s) to
- * @param {string[]} usernames - username(s) of user(s) to add to group
- */
-export const addUsersToGroupDirectly = async ({
+const addAcceptedUsersToGroup = async ({
+  userIds,
   group,
-  usernames,
+  userGroupMembershipDAL,
+  userDAL,
+  groupProjectDAL,
+  projectKeyDAL,
+  projectDAL,
+  projectBotDAL,
+  tx
+}: TAddUsersToGroup) => {
+  console.log("addAcceptedUsersToGroup args: ", {
+    userIds,
+    group
+  });
+  const users = await userDAL.findUserEncKeyByUserIdsBatch(
+    {
+      userIds
+    },
+    tx
+  );
+
+  await userGroupMembershipDAL.insertMany(
+    users.map((user) => ({
+      userId: user.userId,
+      groupId: group.id,
+      isPending: false
+    })),
+    tx
+  );
+
+  // check which projects the group is part of
+  const projectIds = Array.from(
+    new Set(
+      (
+        await groupProjectDAL.find(
+          {
+            groupId: group.id
+          },
+          { tx }
+        )
+      ).map((gp) => gp.projectId)
+    )
+  );
+
+  const keys = await projectKeyDAL.find(
+    {
+      $in: {
+        projectId: projectIds,
+        receiverId: users.map((u) => u.id)
+      }
+    },
+    { tx }
+  );
+
+  const userKeysSet = new Set(keys.map((k) => `${k.projectId}-${k.receiverId}`));
+
+  for await (const projectId of projectIds) {
+    const usersToAddProjectKeyFor = users.filter((u) => !userKeysSet.has(`${projectId}-${u.userId}`));
+
+    if (usersToAddProjectKeyFor.length) {
+      // there are users who need to be shared keys
+      // process adding bulk users to projects for each project individually
+      const ghostUser = await projectDAL.findProjectGhostUser(projectId, tx);
+
+      if (!ghostUser) {
+        throw new BadRequestError({
+          message: "Failed to find sudo user"
+        });
+      }
+
+      const ghostUserLatestKey = await projectKeyDAL.findLatestProjectKey(ghostUser.id, projectId, tx);
+
+      if (!ghostUserLatestKey) {
+        throw new BadRequestError({
+          message: "Failed to find sudo user latest key"
+        });
+      }
+
+      const bot = await projectBotDAL.findOne({ projectId }, tx);
+
+      if (!bot) {
+        throw new BadRequestError({
+          message: "Failed to find bot"
+        });
+      }
+
+      const botPrivateKey = infisicalSymmetricDecrypt({
+        keyEncoding: bot.keyEncoding as SecretKeyEncoding,
+        iv: bot.iv,
+        tag: bot.tag,
+        ciphertext: bot.encryptedPrivateKey
+      });
+
+      const plaintextProjectKey = decryptAsymmetric({
+        ciphertext: ghostUserLatestKey.encryptedKey,
+        nonce: ghostUserLatestKey.nonce,
+        publicKey: ghostUserLatestKey.sender.publicKey,
+        privateKey: botPrivateKey
+      });
+
+      const projectKeysToAdd = usersToAddProjectKeyFor.map((user) => {
+        const { ciphertext: encryptedKey, nonce } = encryptAsymmetric(
+          plaintextProjectKey,
+          user.publicKey,
+          botPrivateKey
+        );
+        return {
+          encryptedKey,
+          nonce,
+          senderId: ghostUser.id,
+          receiverId: user.userId,
+          projectId
+        };
+      });
+
+      await projectKeyDAL.insertMany(projectKeysToAdd, tx);
+    }
+  }
+};
+
+/**
+ * Add users with user ids [userIds] to group [group].
+ * - Users may or may not have finished completing their accounts; this function will
+ * handle both adding users to groups directly and via pending group additions.
+ * @param {group} group - group to add user(s) to
+ * @param {string[]} userIds - id(s) of user(s) to add to group
+ */
+export const addUsersToGroupByUserIds = async ({
+  group,
+  userIds,
   userDAL,
   userGroupMembershipDAL,
   orgDAL,
@@ -31,33 +150,27 @@ export const addUsersToGroupDirectly = async ({
   projectDAL,
   projectBotDAL,
   tx: outerTx
-}: TAddUsersToGroupDirectly) => {
+}: TAddUsersToGroupByUserIds) => {
   const processAddition = async (tx: Knex) => {
-    const users = await userDAL.findUserEncKeyByUsernameBatch(
+    const foundMembers = await userDAL.find(
       {
-        usernames
+        $in: {
+          id: userIds
+        }
       },
-      tx
+      { tx }
     );
 
-    const usersUsernamesSet = new Set(users.map((u) => u.username));
-    usernames.forEach((username) => {
-      if (!usersUsernamesSet.has(username)) {
-        throw new BadRequestError({
-          message: `Failed to find user with username ${username}`
-        });
-      }
-    });
+    const foundMembersIdsSet = new Set(foundMembers.map((member) => member.id));
 
-    const userIds = users.map((u) => {
-      if (!u.isAccepted) {
-        throw new BadRequestError({
-          message: `User ${u.username} cannot be added to group because they have not confirmed their account`
-        });
-      }
+    const isCompleteMatch = userIds.every((userId) => foundMembersIdsSet.has(userId));
 
-      return u.userId;
-    });
+    if (!isCompleteMatch) {
+      throw new ScimRequestError({
+        detail: "Members not found",
+        status: 404
+      });
+    }
 
     // check if user(s) group membership(s) already exists
     const existingUserGroupMemberships = await userGroupMembershipDAL.find(
@@ -87,194 +200,6 @@ export const addUsersToGroupDirectly = async ({
       { tx }
     );
 
-    const existingUserOrgMembershipsUsernamesSet = new Set(existingUserOrgMemberships.map((u) => u.username));
-
-    usernames.forEach((username) => {
-      if (!existingUserOrgMembershipsUsernamesSet.has(username))
-        throw new BadRequestError({
-          message: `User ${username} is not part of the organization`
-        });
-    });
-
-    await userGroupMembershipDAL.insertMany(
-      userIds.map((userId) => ({
-        userId,
-        groupId: group.id
-      })),
-      tx
-    );
-
-    // check which projects the group is part of
-    const projectIds = Array.from(
-      new Set(
-        (
-          await groupProjectDAL.find(
-            {
-              groupId: group.id
-            },
-            { tx }
-          )
-        ).map((gp) => gp.projectId)
-      )
-    );
-
-    const keys = await projectKeyDAL.find(
-      {
-        $in: {
-          projectId: projectIds,
-          receiverId: userIds
-        }
-      },
-      { tx }
-    );
-
-    const userKeysSet = new Set(keys.map((k) => `${k.projectId}-${k.receiverId}`));
-
-    for await (const projectId of projectIds) {
-      const usersToAddProjectKeyFor = users.filter((u) => !userKeysSet.has(`${projectId}-${u.userId}`));
-
-      if (usersToAddProjectKeyFor.length) {
-        // there are users who need to be shared keys
-        // process adding bulk users to projects for each project individually
-        const ghostUser = await projectDAL.findProjectGhostUser(projectId, tx);
-
-        if (!ghostUser) {
-          throw new BadRequestError({
-            message: "Failed to find sudo user"
-          });
-        }
-
-        const ghostUserLatestKey = await projectKeyDAL.findLatestProjectKey(ghostUser.id, projectId, tx);
-
-        if (!ghostUserLatestKey) {
-          throw new BadRequestError({
-            message: "Failed to find sudo user latest key"
-          });
-        }
-
-        const bot = await projectBotDAL.findOne({ projectId }, tx);
-
-        if (!bot) {
-          throw new BadRequestError({
-            message: "Failed to find bot"
-          });
-        }
-
-        const botPrivateKey = infisicalSymmetricDecrypt({
-          keyEncoding: bot.keyEncoding as SecretKeyEncoding,
-          iv: bot.iv,
-          tag: bot.tag,
-          ciphertext: bot.encryptedPrivateKey
-        });
-
-        const plaintextProjectKey = decryptAsymmetric({
-          ciphertext: ghostUserLatestKey.encryptedKey,
-          nonce: ghostUserLatestKey.nonce,
-          publicKey: ghostUserLatestKey.sender.publicKey,
-          privateKey: botPrivateKey
-        });
-
-        const projectKeysToAdd = usersToAddProjectKeyFor.map((user) => {
-          const { ciphertext: encryptedKey, nonce } = encryptAsymmetric(
-            plaintextProjectKey,
-            user.publicKey,
-            botPrivateKey
-          );
-          return {
-            encryptedKey,
-            nonce,
-            senderId: ghostUser.id,
-            receiverId: user.userId,
-            projectId
-          };
-        });
-
-        await projectKeyDAL.insertMany(projectKeysToAdd, tx);
-      }
-    }
-
-    return users;
-  };
-
-  if (outerTx) {
-    return processAddition(outerTx);
-  }
-  return userDAL.transaction(async (tx) => {
-    return processAddition(tx);
-  });
-};
-
-/**
- * Add users with user ids [userIds] to group [group] via pending group additions.
- * - Users must have not finished completing their accounts (i.e. they don't have private key(s) yet).
- * @param {group} group - group to add user(s) to
- * @param {string[]} userIds - id(s) of user(s) to add to group
- */
-export const addUsersToPendingGroupAdditions = async ({
-  group,
-  userIds,
-  pendingGroupAdditionDAL,
-  userDAL,
-  orgDAL,
-  tx: outerTx
-}: TAddUsersToPendingGroupAdditions) => {
-  const processAddition = async (tx: Knex) => {
-    const users = await userDAL.find(
-      {
-        $in: {
-          id: userIds
-        }
-      },
-      { tx }
-    );
-
-    const usersUserIdsSet = new Set(users.map((u) => u.id));
-    userIds.forEach((userId) => {
-      if (!usersUserIdsSet.has(userId)) {
-        throw new BadRequestError({
-          message: `Failed to find user with id ${userId}`
-        });
-      }
-    });
-
-    users.map((u) => {
-      if (u.isAccepted) {
-        throw new BadRequestError({
-          message: `User ${u.username} cannot be added to a pending group addition because they have confirmed their account`
-        });
-      }
-
-      return u.id;
-    });
-
-    // check if user(s) pending group addition(s) already exist
-    const existingPendingGroupAdditions = await pendingGroupAdditionDAL.find(
-      {
-        groupId: group.id,
-        $in: {
-          userId: userIds
-        }
-      },
-      { tx }
-    );
-
-    if (existingPendingGroupAdditions.length) {
-      throw new BadRequestError({
-        message: `User(s) are already part of the group ${group.slug}`
-      });
-    }
-
-    // check if all user(s) are part of the organization
-    const existingUserOrgMemberships = await orgDAL.findMembership(
-      {
-        orgId: group.orgId,
-        $in: {
-          userId: userIds
-        }
-      },
-      { tx }
-    );
-
     const existingUserOrgMembershipsUserIdsSet = new Set(existingUserOrgMemberships.map((u) => u.userId));
 
     userIds.forEach((userId) => {
@@ -284,109 +209,45 @@ export const addUsersToPendingGroupAdditions = async ({
         });
     });
 
-    await pendingGroupAdditionDAL.insertMany(
-      users.map((user) => ({
-        userId: user.id,
-        groupId: group.id
-      })),
-      tx
-    );
-
-    return users;
-  };
-
-  if (outerTx) {
-    return processAddition(outerTx);
-  }
-  return userDAL.transaction(async (tx) => {
-    return processAddition(tx);
-  });
-};
-
-/**
- * Add users with user ids [userIds] to group [group].
- * - Users may or may not have finished completing their accounts; this function will
- * handle both adding users to groups directly and via pending group additions.
- * @param {group} group - group to add user(s) to
- * @param {string[]} userIds - id(s) of user(s) to add to group
- */
-export const addUsersToGroupByUserIds = async ({
-  group,
-  userIds,
-  userDAL,
-  userGroupMembershipDAL,
-  orgDAL,
-  groupProjectDAL,
-  pendingGroupAdditionDAL,
-  projectKeyDAL,
-  projectDAL,
-  projectBotDAL,
-  tx: outerTx
-}: TAddUsersToGroupByUserIds) => {
-  const processAddition = async (tx: Knex) => {
-    const foundMembers = await userDAL.find({
-      $in: {
-        id: userIds
-      }
-    });
-
-    const foundMembersIdsSet = new Set(foundMembers.map((member) => member.id));
-
-    const isCompleteMatch = userIds.every((userId) => foundMembersIdsSet.has(userId));
-
-    if (!isCompleteMatch) {
-      throw new ScimRequestError({
-        detail: "Members not found",
-        status: 404
-      });
-    }
-
-    const membersToAddToGroupDirectly: TUsers[] = [];
+    const membersToAddToGroupNonPending: TUsers[] = [];
     const membersToAddToGroupPending: TUsers[] = [];
 
     foundMembers.forEach((member) => {
       if (member.isAccepted) {
         // add accepted member to group
-        membersToAddToGroupDirectly.push(member);
+        membersToAddToGroupNonPending.push(member);
       } else {
         // add incomplete member to pending group addition
         membersToAddToGroupPending.push(member);
       }
     });
 
-    let addedUsers: TUsers[] = [];
-
-    if (membersToAddToGroupDirectly.length) {
-      addedUsers = addedUsers.concat(
-        await addUsersToGroupDirectly({
-          group,
-          usernames: membersToAddToGroupDirectly.map((member) => member.username),
-          userDAL,
-          userGroupMembershipDAL,
-          orgDAL,
-          groupProjectDAL,
-          projectKeyDAL,
-          projectDAL,
-          projectBotDAL,
-          tx
-        })
-      );
+    if (membersToAddToGroupNonPending.length) {
+      await addAcceptedUsersToGroup({
+        userIds: membersToAddToGroupNonPending.map((member) => member.id),
+        group,
+        userDAL,
+        userGroupMembershipDAL,
+        groupProjectDAL,
+        projectKeyDAL,
+        projectDAL,
+        projectBotDAL,
+        tx
+      });
     }
 
     if (membersToAddToGroupPending.length) {
-      addedUsers = addedUsers.concat(
-        await addUsersToPendingGroupAdditions({
-          group,
-          userIds: membersToAddToGroupPending.map((member) => member.id),
-          pendingGroupAdditionDAL,
-          userDAL,
-          orgDAL,
-          tx
-        })
+      await userGroupMembershipDAL.insertMany(
+        membersToAddToGroupPending.map((member) => ({
+          userId: member.id,
+          groupId: group.id,
+          isPending: true
+        })),
+        tx
       );
     }
 
-    return addedUsers;
+    return membersToAddToGroupNonPending.concat(membersToAddToGroupPending);
   };
 
   if (outerTx) {
@@ -399,185 +260,7 @@ export const addUsersToGroupByUserIds = async ({
 
 /**
  * Remove users with user ids [userIds] from group [group].
- * - Users must be directly added to the group.
- * @param {group} group - group to remove user(s) from
- * @param {string[]} userIds - id(s) of user(s) to remove from group
- */
-export const removeUsersFromGroupDirectly = async ({
-  group,
-  userIds,
-  userDAL,
-  userGroupMembershipDAL,
-  groupProjectDAL,
-  projectKeyDAL,
-  tx: outerTx
-}: TRemoveUsersFromGroupDirectly) => {
-  const processRemoval = async (tx: Knex) => {
-    const users = await userDAL.find(
-      {
-        $in: {
-          id: userIds
-        }
-      },
-      { tx }
-    );
-
-    const usersUserIdsSet = new Set(users.map((u) => u.id));
-    userIds.forEach((userId) => {
-      if (!usersUserIdsSet.has(userId)) {
-        throw new BadRequestError({
-          message: `Failed to find user with id ${userId}`
-        });
-      }
-    });
-
-    // check if user group membership already exists
-    const existingUserGroupMemberships = await userGroupMembershipDAL.find(
-      {
-        groupId: group.id,
-        $in: {
-          userId: userIds
-        }
-      },
-      { tx }
-    );
-
-    const existingUserGroupMembershipsUserIdsSet = new Set(existingUserGroupMemberships.map((u) => u.userId));
-
-    userIds.forEach((userId) => {
-      if (!existingUserGroupMembershipsUserIdsSet.has(userId))
-        throw new BadRequestError({
-          message: `User(s) are not part of the group ${group.slug}`
-        });
-    });
-
-    // check which projects the group is part of
-    const projectIds = Array.from(
-      new Set(
-        (
-          await groupProjectDAL.find(
-            {
-              groupId: group.id
-            },
-            { tx }
-          )
-        ).map((gp) => gp.projectId)
-      )
-    );
-
-    // TODO: this part can be optimized
-    for await (const userId of userIds) {
-      const t = await userGroupMembershipDAL.filterProjectsByUserMembership(userId, group.id, projectIds, tx);
-      const projectsToDeleteKeyFor = projectIds.filter((p) => !t.has(p));
-
-      if (projectsToDeleteKeyFor.length) {
-        await projectKeyDAL.delete(
-          {
-            receiverId: userId,
-            $in: {
-              projectId: projectsToDeleteKeyFor
-            }
-          },
-          tx
-        );
-      }
-
-      await userGroupMembershipDAL.delete(
-        {
-          groupId: group.id,
-          userId
-        },
-        tx
-      );
-    }
-
-    return users;
-  };
-
-  if (outerTx) {
-    return processRemoval(outerTx);
-  }
-  return userDAL.transaction(async (tx) => {
-    return processRemoval(tx);
-  });
-};
-
-/**
- * Remove users with user ids [userIds] from group [group] via pending group additions.
- * - Users must have pending group additions to the group.
- * @param {group} group - group to remove user(s) from
- * @param {string[]} userIds - id(s) of user(s) to remove from group
- */
-export const removeUsersFromPendingGroupAdditions = async ({
-  group,
-  userIds,
-  userDAL,
-  pendingGroupAdditionDAL,
-  tx: outerTx
-}: TRemoveUsersFromPendingGroupAdditions) => {
-  const processRemoval = async (tx: Knex) => {
-    const users = await userDAL.find(
-      {
-        $in: {
-          id: userIds
-        }
-      },
-      { tx }
-    );
-
-    const usersUserIdsSet = new Set(users.map((u) => u.id));
-    userIds.forEach((userId) => {
-      if (!usersUserIdsSet.has(userId)) {
-        throw new BadRequestError({
-          message: `Failed to find user with id ${userId}`
-        });
-      }
-    });
-
-    // check if user pending group addition already exists
-    const existingPendingGroupAdditions = await pendingGroupAdditionDAL.find(
-      {
-        groupId: group.id,
-        $in: {
-          userId: userIds
-        }
-      },
-      { tx }
-    );
-
-    const existingPendingGroupAdditionsUserIdsSet = new Set(existingPendingGroupAdditions.map((u) => u.userId));
-
-    userIds.forEach((userId) => {
-      if (!existingPendingGroupAdditionsUserIdsSet.has(userId))
-        throw new BadRequestError({
-          message: `User(s) are not part of the group ${group.slug}`
-        });
-    });
-
-    await pendingGroupAdditionDAL.delete(
-      {
-        groupId: group.id,
-        $in: {
-          userId: userIds
-        }
-      },
-      tx
-    );
-
-    return users;
-  };
-
-  if (outerTx) {
-    return processRemoval(outerTx);
-  }
-  return userDAL.transaction(async (tx) => {
-    return processRemoval(tx);
-  });
-};
-
-/**
- * Remove users with user ids [userIds] from group [group].
- * - Users may be part of the group directly or via pending group additions;
+ * - Users may be part of the group (non-pending + pending);
  * this function will handle both cases.
  * @param {group} group - group to remove user(s) from
  * @param {string[]} userIds - id(s) of user(s) to remove from group
@@ -588,7 +271,6 @@ export const removeUsersFromGroupByUserIds = async ({
   userDAL,
   userGroupMembershipDAL,
   groupProjectDAL,
-  pendingGroupAdditionDAL,
   projectKeyDAL,
   tx: outerTx
 }: TRemoveUsersFromGroupByUserIds) => {
@@ -610,48 +292,91 @@ export const removeUsersFromGroupByUserIds = async ({
       });
     }
 
-    const membersToRemoveFromGroupDirectly: TUsers[] = [];
+    // check if user group membership already exists
+    const existingUserGroupMemberships = await userGroupMembershipDAL.find(
+      {
+        groupId: group.id,
+        $in: {
+          userId: userIds
+        }
+      },
+      { tx }
+    );
+
+    const existingUserGroupMembershipsUserIdsSet = new Set(existingUserGroupMemberships.map((u) => u.userId));
+
+    userIds.forEach((userId) => {
+      if (!existingUserGroupMembershipsUserIdsSet.has(userId))
+        throw new BadRequestError({
+          message: `User(s) are not part of the group ${group.slug}`
+        });
+    });
+
+    const membersToRemoveFromGroupNonPending: TUsers[] = [];
     const membersToRemoveFromGroupPending: TUsers[] = [];
 
     foundMembers.forEach((member) => {
       if (member.isAccepted) {
         // remove accepted member from group
-        membersToRemoveFromGroupDirectly.push(member);
+        membersToRemoveFromGroupNonPending.push(member);
       } else {
         // remove incomplete member from pending group addition
         membersToRemoveFromGroupPending.push(member);
       }
     });
 
-    let removedUsers: TUsers[] = [];
-
-    if (membersToRemoveFromGroupDirectly.length) {
-      removedUsers = removedUsers.concat(
-        await removeUsersFromGroupDirectly({
-          group,
-          userIds: membersToRemoveFromGroupDirectly.map((member) => member.id),
-          userDAL,
-          userGroupMembershipDAL,
-          groupProjectDAL,
-          projectKeyDAL,
-          tx
-        })
+    if (membersToRemoveFromGroupNonPending.length) {
+      // check which projects the group is part of
+      const projectIds = Array.from(
+        new Set(
+          (
+            await groupProjectDAL.find(
+              {
+                groupId: group.id
+              },
+              { tx }
+            )
+          ).map((gp) => gp.projectId)
+        )
       );
+
+      // TODO: this part can be optimized
+      for await (const userId of userIds) {
+        const t = await userGroupMembershipDAL.filterProjectsByUserMembership(userId, group.id, projectIds, tx);
+        const projectsToDeleteKeyFor = projectIds.filter((p) => !t.has(p));
+
+        if (projectsToDeleteKeyFor.length) {
+          await projectKeyDAL.delete(
+            {
+              receiverId: userId,
+              $in: {
+                projectId: projectsToDeleteKeyFor
+              }
+            },
+            tx
+          );
+        }
+
+        await userGroupMembershipDAL.delete(
+          {
+            groupId: group.id,
+            userId
+          },
+          tx
+        );
+      }
     }
 
     if (membersToRemoveFromGroupPending.length) {
-      removedUsers = removedUsers.concat(
-        await removeUsersFromPendingGroupAdditions({
-          group,
-          userIds: membersToRemoveFromGroupPending.map((member) => member.id),
-          pendingGroupAdditionDAL,
-          userDAL,
-          tx
-        })
-      );
+      await userGroupMembershipDAL.delete({
+        groupId: group.id,
+        $in: {
+          userId: membersToRemoveFromGroupPending.map((member) => member.id)
+        }
+      });
     }
 
-    return removedUsers;
+    return membersToRemoveFromGroupNonPending.concat(membersToRemoveFromGroupPending);
   };
 
   if (outerTx) {
@@ -669,9 +394,7 @@ export const removeUsersFromGroupByUserIds = async ({
 export const convertPendingGroupAdditionsToGroupMemberships = async ({
   userIds,
   userDAL,
-  pendingGroupAdditionDAL,
   userGroupMembershipDAL,
-  orgDAL,
   groupProjectDAL,
   projectKeyDAL,
   projectDAL,
@@ -705,15 +428,14 @@ export const convertPendingGroupAdditionsToGroupMemberships = async ({
       }
     });
 
-    const pendingGroupAdditions = await pendingGroupAdditionDAL.deletePendingGroupAdditionsByUserIds(userIds, tx);
+    const pendingGroupAdditions = await userGroupMembershipDAL.deletePendingUserGroupMembershipsByUserIds(userIds, tx);
 
     for await (const pendingGroupAddition of pendingGroupAdditions) {
-      await addUsersToGroupDirectly({
+      await addAcceptedUsersToGroup({
+        userIds: [pendingGroupAddition.user.id],
         group: pendingGroupAddition.group,
-        usernames: [pendingGroupAddition.user.username],
         userDAL,
         userGroupMembershipDAL,
-        orgDAL,
         groupProjectDAL,
         projectKeyDAL,
         projectDAL,

backend/src/ee/services/group/group-service.ts

@@ -2,7 +2,6 @@ import { ForbiddenError } from "@casl/ability";
 import slugify from "@sindresorhus/slugify";
 
 import { OrgMembershipRole, TOrgRoles } from "@app/db/schemas";
-import { TPendingGroupAdditionDALFactory } from "@app/ee/services/group/pending-group-addition-dal";
 import { isAtLeastAsPrivileged } from "@app/lib/casl";
 import { BadRequestError, ForbiddenRequestError } from "@app/lib/errors";
 import { alphaNumericNanoId } from "@app/lib/nanoid";
@@ -29,7 +28,7 @@ import {
 import { TUserGroupMembershipDALFactory } from "./user-group-membership-dal";
 
 type TGroupServiceFactoryDep = {
-  userDAL: Pick<TUserDALFactory, "find" | "findUserEncKeyByUsernameBatch" | "transaction" | "findOne">;
+  userDAL: Pick<TUserDALFactory, "find" | "findUserEncKeyByUserIdsBatch" | "transaction" | "findOne">;
   groupDAL: Pick<TGroupDALFactory, "create" | "findOne" | "update" | "delete" | "findAllGroupMembers">;
   groupProjectDAL: Pick<TGroupProjectDALFactory, "find">;
   orgDAL: Pick<TOrgDALFactory, "findMembership" | "countAllOrgMembers">;
@@ -40,7 +39,6 @@ type TGroupServiceFactoryDep = {
   projectDAL: Pick<TProjectDALFactory, "findProjectGhostUser">;
   projectBotDAL: Pick<TProjectBotDALFactory, "findOne">;
   projectKeyDAL: Pick<TProjectKeyDALFactory, "find" | "delete" | "findLatestProjectKey" | "insertMany">;
-  pendingGroupAdditionDAL: TPendingGroupAdditionDALFactory; // remove?
   permissionService: Pick<TPermissionServiceFactory, "getOrgPermission" | "getOrgPermissionByRole">;
   licenseService: Pick<TLicenseServiceFactory, "getPlan">;
 };
@@ -56,7 +54,6 @@ export const groupServiceFactory = ({
   projectDAL,
   projectBotDAL,
   projectKeyDAL,
-  pendingGroupAdditionDAL,
   permissionService,
   licenseService
 }: TGroupServiceFactoryDep) => {
@@ -279,7 +276,6 @@ export const groupServiceFactory = ({
       userGroupMembershipDAL,
       orgDAL,
       groupProjectDAL,
-      pendingGroupAdditionDAL,
       projectKeyDAL,
       projectDAL,
       projectBotDAL
@@ -333,7 +329,6 @@ export const groupServiceFactory = ({
       userIds: [user.id],
       userDAL,
       userGroupMembershipDAL,
-      pendingGroupAdditionDAL,
       groupProjectDAL,
       projectKeyDAL
     });

backend/src/ee/services/group/group-types.ts

@@ -1,7 +1,6 @@
 import { Knex } from "knex";
 
 import { TGroups } from "@app/db/schemas";
-import { TPendingGroupAdditionDALFactory } from "@app/ee/services/group/pending-group-addition-dal";
 import { TUserGroupMembershipDALFactory } from "@app/ee/services/group/user-group-membership-dal";
 import { TGenericPermission } from "@app/lib/types";
 import { TGroupProjectDALFactory } from "@app/services/group-project/group-project-dal";
@@ -49,24 +48,22 @@ export type TRemoveUserFromGroupDTO = {
 
 // group fns types
 
+export type TAddUsersToGroup = {
+  userIds: string[];
+  group: TGroups;
+  userDAL: Pick<TUserDALFactory, "findUserEncKeyByUserIdsBatch">;
+  userGroupMembershipDAL: Pick<TUserGroupMembershipDALFactory, "find" | "transaction" | "insertMany">;
+  groupProjectDAL: Pick<TGroupProjectDALFactory, "find">;
+  projectKeyDAL: Pick<TProjectKeyDALFactory, "find" | "findLatestProjectKey" | "insertMany">;
+  projectDAL: Pick<TProjectDALFactory, "findProjectGhostUser">;
+  projectBotDAL: Pick<TProjectBotDALFactory, "findOne">;
+  tx: Knex;
+};
+
 export type TAddUsersToGroupByUserIds = {
   group: TGroups;
   userIds: string[];
-  userDAL: Pick<TUserDALFactory, "find" | "findUserEncKeyByUsernameBatch" | "transaction">;
-  userGroupMembershipDAL: Pick<TUserGroupMembershipDALFactory, "find" | "transaction" | "insertMany">;
-  orgDAL: Pick<TOrgDALFactory, "findMembership">;
-  groupProjectDAL: Pick<TGroupProjectDALFactory, "find">;
-  pendingGroupAdditionDAL: Pick<TPendingGroupAdditionDALFactory, "insertMany" | "find">;
-  projectKeyDAL: Pick<TProjectKeyDALFactory, "find" | "findLatestProjectKey" | "insertMany">;
-  projectDAL: Pick<TProjectDALFactory, "findProjectGhostUser">;
-  projectBotDAL: Pick<TProjectBotDALFactory, "findOne">;
-  tx?: Knex;
-};
-
-export type TAddUsersToGroupDirectly = {
-  group: TGroups;
-  usernames: string[];
-  userDAL: Pick<TUserDALFactory, "findUserEncKeyByUsernameBatch" | "transaction">;
+  userDAL: Pick<TUserDALFactory, "find" | "findUserEncKeyByUserIdsBatch" | "transaction">;
   userGroupMembershipDAL: Pick<TUserGroupMembershipDALFactory, "find" | "transaction" | "insertMany">;
   orgDAL: Pick<TOrgDALFactory, "findMembership">;
   groupProjectDAL: Pick<TGroupProjectDALFactory, "find">;
@@ -76,50 +73,23 @@ export type TAddUsersToGroupDirectly = {
   tx?: Knex;
 };
 
-export type TAddUsersToPendingGroupAdditions = {
-  userIds: string[];
-  group: TGroups;
-  pendingGroupAdditionDAL: Pick<TPendingGroupAdditionDALFactory, "find" | "insertMany">;
-  userDAL: Pick<TUserDALFactory, "find" | "transaction">;
-  orgDAL: Pick<TOrgDALFactory, "findMembership">;
-  tx?: Knex;
-};
-
 export type TRemoveUsersFromGroupByUserIds = {
   group: TGroups;
   userIds: string[];
   userDAL: Pick<TUserDALFactory, "find" | "transaction">;
   userGroupMembershipDAL: Pick<TUserGroupMembershipDALFactory, "find" | "filterProjectsByUserMembership" | "delete">;
-  pendingGroupAdditionDAL: Pick<TPendingGroupAdditionDALFactory, "find" | "delete">;
   groupProjectDAL: Pick<TGroupProjectDALFactory, "find">;
   projectKeyDAL: Pick<TProjectKeyDALFactory, "delete">;
   tx?: Knex;
 };
 
-export type TRemoveUsersFromGroupDirectly = {
-  group: TGroups;
-  userIds: string[];
-  userDAL: Pick<TUserDALFactory, "find" | "transaction">;
-  userGroupMembershipDAL: Pick<TUserGroupMembershipDALFactory, "find" | "filterProjectsByUserMembership" | "delete">;
-  groupProjectDAL: Pick<TGroupProjectDALFactory, "find">;
-  projectKeyDAL: Pick<TProjectKeyDALFactory, "delete">;
-  tx?: Knex;
-};
-
-export type TRemoveUsersFromPendingGroupAdditions = {
-  group: TGroups;
-  userIds: string[];
-  pendingGroupAdditionDAL: Pick<TPendingGroupAdditionDALFactory, "find" | "delete">;
-  userDAL: Pick<TUserDALFactory, "find" | "transaction">;
-  tx?: Knex;
-};
-
 export type TConvertPendingGroupAdditionsToGroupMemberships = {
   userIds: string[];
-  pendingGroupAdditionDAL: Pick<TPendingGroupAdditionDALFactory, "deletePendingGroupAdditionsByUserIds">;
-  userDAL: Pick<TUserDALFactory, "findUserEncKeyByUsernameBatch" | "transaction" | "find" | "findById">;
-  userGroupMembershipDAL: Pick<TUserGroupMembershipDALFactory, "find" | "transaction" | "insertMany">;
-  orgDAL: Pick<TOrgDALFactory, "findMembership">;
+  userDAL: Pick<TUserDALFactory, "findUserEncKeyByUserIdsBatch" | "transaction" | "find" | "findById">;
+  userGroupMembershipDAL: Pick<
+    TUserGroupMembershipDALFactory,
+    "find" | "transaction" | "insertMany" | "deletePendingUserGroupMembershipsByUserIds"
+  >;
   groupProjectDAL: Pick<TGroupProjectDALFactory, "find">;
   projectKeyDAL: Pick<TProjectKeyDALFactory, "find" | "findLatestProjectKey" | "insertMany">;
   projectDAL: Pick<TProjectDALFactory, "findProjectGhostUser">;

backend/src/ee/services/group/pending-group-addition-dal.ts

@@ -1,55 +0,0 @@
-import { Knex } from "knex";
-
-import { TDbClient } from "@app/db";
-import { TableName } from "@app/db/schemas";
-import { DatabaseError } from "@app/lib/errors";
-import { ormify } from "@app/lib/knex";
-
-export type TPendingGroupAdditionDALFactory = ReturnType<typeof pendingGroupAdditionDALFactory>;
-
-export const pendingGroupAdditionDALFactory = (db: TDbClient) => {
-  const pendingGroupAdditionOrm = ormify(db, TableName.PendingGroupAddition);
-
-  // special query
-  const deletePendingGroupAdditionsByUserIds = async (userIds: string[], tx?: Knex) => {
-    try {
-      const pendingGroupAdditions = await (tx || db)(TableName.PendingGroupAddition)
-        .whereIn(`${TableName.PendingGroupAddition}.userId`, userIds)
-        .join(TableName.Groups, `${TableName.PendingGroupAddition}.groupId`, `${TableName.Groups}.id`)
-        .join(TableName.Users, `${TableName.PendingGroupAddition}.userId`, `${TableName.Users}.id`);
-
-      await pendingGroupAdditionOrm.delete(
-        {
-          $in: {
-            userId: userIds
-          }
-        },
-        tx
-      );
-
-      return pendingGroupAdditions.map(({ userId, username, groupId, orgId, name, slug, role, roleId }) => ({
-        user: {
-          id: userId,
-          username
-        },
-        group: {
-          id: groupId,
-          orgId,
-          name,
-          slug,
-          role,
-          roleId,
-          createdAt: new Date(),
-          updatedAt: new Date()
-        }
-      }));
-    } catch (error) {
-      throw new DatabaseError({ error, name: "Filter projects by user membership" });
-    }
-  };
-
-  return {
-    ...pendingGroupAdditionOrm,
-    deletePendingGroupAdditionsByUserIds
-  };
-};

backend/src/ee/services/group/user-group-membership-dal.ts

@@ -122,10 +122,49 @@ export const userGroupMembershipDALFactory = (db: TDbClient) => {
     }
   };
 
+  const deletePendingUserGroupMembershipsByUserIds = async (userIds: string[], tx?: Knex) => {
+    try {
+      const members = await (tx || db)(TableName.UserGroupMembership)
+        .whereIn(`${TableName.UserGroupMembership}.userId`, userIds)
+        .where(`${TableName.UserGroupMembership}.isPending`, true)
+        .join(TableName.Groups, `${TableName.UserGroupMembership}.groupId`, `${TableName.Groups}.id`)
+        .join(TableName.Users, `${TableName.UserGroupMembership}.userId`, `${TableName.Users}.id`);
+
+      await userGroupMembershipOrm.delete(
+        {
+          $in: {
+            userId: userIds
+          }
+        },
+        tx
+      );
+
+      return members.map(({ userId, username, groupId, orgId, name, slug, role, roleId }) => ({
+        user: {
+          id: userId,
+          username
+        },
+        group: {
+          id: groupId,
+          orgId,
+          name,
+          slug,
+          role,
+          roleId,
+          createdAt: new Date(),
+          updatedAt: new Date()
+        }
+      }));
+    } catch (error) {
+      throw new DatabaseError({ error, name: "Delete pending user group memberships by user ids" });
+    }
+  };
+
   return {
     ...userGroupMembershipOrm,
     filterProjectsByUserMembership,
     findUserGroupMembershipsInProject,
-    findGroupMembersNotInProject
+    findGroupMembersNotInProject,
+    deletePendingUserGroupMembershipsByUserIds
   };
 };