chore: Refactor and remove new tables

backend/src/ee/services/access-approval-policy/access-approval-policy-dal.ts

@@ -5,6 +5,8 @@ import { AccessApprovalPoliciesSchema, TableName, TAccessApprovalPolicies } from
 import { DatabaseError } from "@app/lib/errors";
 import { buildFindFilter, ormify, selectAllTableCols, sqlNestRelationships, TFindFilter } from "@app/lib/knex";
 
+import { ApproverType } from "./access-approval-policy-types";
+
 export type TAccessApprovalPolicyDALFactory = ReturnType<typeof accessApprovalPolicyDALFactory>;
 
 export const accessApprovalPolicyDALFactory = (db: TDbClient) => {
@@ -20,13 +22,8 @@ export const accessApprovalPolicyDALFactory = (db: TDbClient) => {
         `${TableName.AccessApprovalPolicy}.id`,
         `${TableName.AccessApprovalPolicyApprover}.policyId`
       )
-      .leftJoin(
-        TableName.AccessApprovalPolicyGroupApprover,
-        `${TableName.AccessApprovalPolicy}.id`,
-        `${TableName.AccessApprovalPolicyGroupApprover}.policyId`
-      )
       .select(tx.ref("approverUserId").withSchema(TableName.AccessApprovalPolicyApprover))
-      .select(tx.ref("approverGroupId").withSchema(TableName.AccessApprovalPolicyGroupApprover))
+      .select(tx.ref("approverGroupId").withSchema(TableName.AccessApprovalPolicyApprover))
       .select(tx.ref("name").withSchema(TableName.Environment).as("envName"))
       .select(tx.ref("slug").withSchema(TableName.Environment).as("envSlug"))
       .select(tx.ref("id").withSchema(TableName.Environment).as("envId"))
@@ -36,10 +33,10 @@ export const accessApprovalPolicyDALFactory = (db: TDbClient) => {
     return result;
   };
 
-  const findById = async (id: string, tx?: Knex) => {
+  const findById = async (policyId: string, tx?: Knex) => {
     try {
       const doc = await accessApprovalPolicyFindQuery(tx || db.replicaNode(), {
-        [`${TableName.AccessApprovalPolicy}.id` as "id"]: id
+        [`${TableName.AccessApprovalPolicy}.id` as "id"]: policyId
       });
       const formattedDoc = sqlNestRelationships({
         data: doc,
@@ -56,16 +53,18 @@ export const accessApprovalPolicyDALFactory = (db: TDbClient) => {
         childrenMapper: [
           {
             key: "approverUserId",
-            label: "userApprovers" as const,
-            mapper: ({ approverUserId }) => ({
-              userId: approverUserId
+            label: "approvers" as const,
+            mapper: ({ approverUserId: id }) => ({
+              id,
+              type: "user"
             })
           },
           {
             key: "approverGroupId",
-            label: "groupApprovers" as const,
-            mapper: ({ approverGroupId }) => ({
-              groupId: approverGroupId
+            label: "approvers" as const,
+            mapper: ({ approverGroupId: id }) => ({
+              id,
+              type: "group"
             })
           }
         ]
@@ -97,16 +96,18 @@ export const accessApprovalPolicyDALFactory = (db: TDbClient) => {
         childrenMapper: [
           {
             key: "approverUserId",
-            label: "userApprovers" as const,
-            mapper: ({ approverUserId }) => ({
-              userId: approverUserId
+            label: "approvers" as const,
+            mapper: ({ approverUserId: id }) => ({
+              id,
+              type: ApproverType.User
             })
           },
           {
             key: "approverGroupId",
-            label: "groupApprovers" as const,
-            mapper: ({ approverGroupId }) => ({
-              groupId: approverGroupId
+            label: "approvers" as const,
+            mapper: ({ approverGroupId: id }) => ({
+              id,
+              type: ApproverType.Group
             })
           }
         ]

backend/src/ee/services/access-approval-policy/access-approval-policy-group-approver-dal.ts

@@ -1,12 +0,0 @@
-import { TDbClient } from "@app/db";
-import { TableName } from "@app/db/schemas";
-import { ormify } from "@app/lib/knex";
-
-export type TAccessApprovalPolicyGroupApproverDALFactory = ReturnType<
-  typeof accessApprovalPolicyGroupApproverDALFactory
->;
-
-export const accessApprovalPolicyGroupApproverDALFactory = (db: TDbClient) => {
-  const accessApprovalPolicyGroupApproverOrm = ormify(db, TableName.AccessApprovalPolicyGroupApprover);
-  return { ...accessApprovalPolicyGroupApproverOrm };
-};

backend/src/ee/services/access-approval-policy/access-approval-policy-service.ts

@@ -11,8 +11,8 @@ import { TGroupDALFactory } from "../group/group-dal";
 import { TAccessApprovalPolicyApproverDALFactory } from "./access-approval-policy-approver-dal";
 import { TAccessApprovalPolicyDALFactory } from "./access-approval-policy-dal";
 import { verifyApprovers } from "./access-approval-policy-fns";
-import { TAccessApprovalPolicyGroupApproverDALFactory } from "./access-approval-policy-group-approver-dal";
 import {
+  ApproverType,
   TCreateAccessApprovalPolicy,
   TDeleteAccessApprovalPolicy,
   TGetAccessPolicyCountByEnvironmentDTO,
@@ -28,14 +28,12 @@ type TSecretApprovalPolicyServiceFactoryDep = {
   accessApprovalPolicyApproverDAL: TAccessApprovalPolicyApproverDALFactory;
   projectMembershipDAL: Pick<TProjectMembershipDALFactory, "find">;
   groupDAL: TGroupDALFactory;
-  accessApprovalPolicyGroupApproverDAL: TAccessApprovalPolicyGroupApproverDALFactory;
 };
 
 export type TAccessApprovalPolicyServiceFactory = ReturnType<typeof accessApprovalPolicyServiceFactory>;
 
 export const accessApprovalPolicyServiceFactory = ({
   accessApprovalPolicyDAL,
-  accessApprovalPolicyGroupApproverDAL,
   accessApprovalPolicyApproverDAL,
   groupDAL,
   permissionService,
@@ -51,7 +49,6 @@ export const accessApprovalPolicyServiceFactory = ({
     actorAuthMethod,
     approvals,
     approvers,
-    groupApprovers,
     projectSlug,
     environment,
     enforcementLevel
@@ -59,11 +56,15 @@ export const accessApprovalPolicyServiceFactory = ({
     const project = await projectDAL.findProjectBySlug(projectSlug, actorOrgId);
     if (!project) throw new BadRequestError({ message: "Project not found" });
 
-    if (!groupApprovers && !approvers)
-      throw new BadRequestError({ message: "Either of approvers or group approvers must be provided" });
-
     // If there is a group approver people might be added to the group later to meet the approvers quota
-    if (!groupApprovers && approvals > approvers.length)
+    const groupApprovers = approvers
+      .filter((approver) => approver.type === ApproverType.Group)
+      .map((approver) => approver.id);
+    const userApprovers = approvers
+      .filter((approver) => approver.type === ApproverType.User)
+      .map((approver) => approver.id);
+
+    if (!groupApprovers && approvals > userApprovers.length)
       throw new BadRequestError({ message: "Approvals cannot be greater than approvers" });
 
     const { permission } = await permissionService.getProjectPermission(
@@ -80,7 +81,7 @@ export const accessApprovalPolicyServiceFactory = ({
     const env = await projectEnvDAL.findOne({ slug: environment, projectId: project.id });
     if (!env) throw new BadRequestError({ message: "Environment not found" });
 
-    const verifyAllApprovers = approvers;
+    const verifyAllApprovers = userApprovers;
     const usersPromises: Promise<
       {
         id: string;
@@ -119,21 +120,25 @@ export const accessApprovalPolicyServiceFactory = ({
         },
         tx
       );
-      await accessApprovalPolicyApproverDAL.insertMany(
-        approvers.map((userId) => ({
-          approverUserId: userId,
-          policyId: doc.id
-        })),
-        tx
-      );
+      if (userApprovers) {
+        await accessApprovalPolicyApproverDAL.insertMany(
+          userApprovers.map((userId) => ({
+            approverUserId: userId,
+            policyId: doc.id
+          })),
+          tx
+        );
+      }
 
-      await accessApprovalPolicyGroupApproverDAL.insertMany(
-        groupApprovers.map((groupId) => ({
-          approverGroupId: groupId,
-          policyId: doc.id
-        })),
-        tx
-      );
+      if (groupApprovers) {
+        await accessApprovalPolicyApproverDAL.insertMany(
+          groupApprovers.map((groupId) => ({
+            approverGroupId: groupId,
+            policyId: doc.id
+          })),
+          tx
+        );
+      }
 
       return doc;
     });
@@ -167,7 +172,6 @@ export const accessApprovalPolicyServiceFactory = ({
   const updateAccessApprovalPolicy = async ({
     policyId,
     approvers,
-    groupApprovers,
     secretPath,
     name,
     actorId,
@@ -177,7 +181,19 @@ export const accessApprovalPolicyServiceFactory = ({
     approvals,
     enforcementLevel
   }: TUpdateAccessApprovalPolicy) => {
+    const groupApprovers = approvers
+      ?.filter((approver) => approver.type === ApproverType.Group)
+      .map((approver) => approver.id);
+    const userApprovers = approvers
+      ?.filter((approver) => approver.type === ApproverType.User)
+      .map((approver) => approver.id);
+
     const accessApprovalPolicy = await accessApprovalPolicyDAL.findById(policyId);
+    const currentAppovals = approvals || accessApprovalPolicy.approvals;
+    if (groupApprovers?.length === 0 && userApprovers && currentAppovals > userApprovers.length) {
+      throw new BadRequestError({ message: "Approvals cannot be greater than approvers" });
+    }
+
     if (!accessApprovalPolicy) throw new BadRequestError({ message: "Secret approval policy not found" });
     const { permission } = await permissionService.getProjectPermission(
       actor,
@@ -200,7 +216,10 @@ export const accessApprovalPolicyServiceFactory = ({
         },
         tx
       );
-      if (approvers) {
+
+      await accessApprovalPolicyApproverDAL.delete({ policyId: doc.id }, tx);
+
+      if (userApprovers) {
         await verifyApprovers({
           projectId: accessApprovalPolicy.projectId,
           orgId: actorOrgId,
@@ -208,11 +227,10 @@ export const accessApprovalPolicyServiceFactory = ({
           secretPath: doc.secretPath!,
           actorAuthMethod,
           permissionService,
-          userIds: approvers
+          userIds: userApprovers
         });
-        await accessApprovalPolicyApproverDAL.delete({ policyId: doc.id }, tx);
         await accessApprovalPolicyApproverDAL.insertMany(
-          approvers.map((userId) => ({
+          userApprovers.map((userId) => ({
             approverUserId: userId,
             policyId: doc.id
           })),
@@ -246,8 +264,7 @@ export const accessApprovalPolicyServiceFactory = ({
           permissionService,
           userIds: verifyGroupApprovers
         });
-        await accessApprovalPolicyGroupApproverDAL.delete({ policyId: doc.id }, tx);
-        await accessApprovalPolicyGroupApproverDAL.insertMany(
+        await accessApprovalPolicyApproverDAL.insertMany(
           groupApprovers.map((groupId) => ({
             approverGroupId: groupId,
             policyId: doc.id

backend/src/ee/services/access-approval-policy/access-approval-policy-types.ts

@@ -13,12 +13,16 @@ export type TVerifyApprovers = {
   orgId: string;
 };
 
+export enum ApproverType {
+  Group = "group",
+  User = "user"
+}
+
 export type TCreateAccessApprovalPolicy = {
   approvals: number;
   secretPath: string;
   environment: string;
-  approvers: string[];
-  groupApprovers: string[];
+  approvers: { type: ApproverType; id: string }[];
   projectSlug: string;
   name: string;
   enforcementLevel: EnforcementLevel;
@@ -27,8 +31,7 @@ export type TCreateAccessApprovalPolicy = {
 export type TUpdateAccessApprovalPolicy = {
   policyId: string;
   approvals?: number;
-  approvers?: string[];
-  groupApprovers?: string[];
+  approvers?: { type: ApproverType; id: string }[];
   secretPath?: string;
   name?: string;
   enforcementLevel?: EnforcementLevel;