Feat: bulk delete customers from customer group (#1097)

* integration testing

* customer seeder

* initial bulk removal

* integraiton testing of deletes

* delete fix

* not found test

* remove unused code

* Apply suggestions from code review

Co-authored-by: Sebastian Rindom <skrindom@gmail.com>

* update integration tests

* pr review fixes

* update migration

* formatting

* integration tests for deletion

* pr feedback

* fix failing integration tests

* remove integration tests before merging

Co-authored-by: Sebastian Rindom <skrindom@gmail.com>

integration-tests/api/__tests__/admin/customer-groups.js

@@ -290,7 +290,6 @@ describe("/admin/customer-groups", () => {
       const db = useDb()
       await db.teardown()
     })
-
     it("gets customer group", async () => {
       const api = useApi()
 
@@ -358,4 +357,187 @@ describe("/admin/customer-groups", () => {
         })
     })
   })
+
+  describe("DELETE /admin/customer-groups/{id}/batch", () => {
+    beforeEach(async () => {
+      try {
+        await adminSeeder(dbConnection)
+        await customerSeeder(dbConnection)
+      } catch (err) {
+        console.log(err)
+        throw err
+      }
+    })
+
+    afterEach(async () => {
+      const db = useDb()
+      await db.teardown()
+    })
+
+    it("removes multiple customers from a group", async () => {
+      const api = useApi()
+
+      const payload = {
+        customer_ids: [{ id: "test-customer-5" }, { id: "test-customer-6" }],
+      }
+
+      const batchAddResponse = await api
+        .delete("/admin/customer-groups/test-group-5/customers/batch", {
+          headers: {
+            Authorization: "Bearer test_token",
+          },
+          data: payload,
+        })
+        .catch((err) => console.log(err))
+
+      expect(batchAddResponse.status).toEqual(200)
+      expect(batchAddResponse.data).toEqual({
+        customer_group: expect.objectContaining({
+          id: "test-group-5",
+          name: "test-group-5",
+        }),
+      })
+
+      const getCustomerResponse = await api.get(
+        "/admin/customers?expand=groups",
+        {
+          headers: { Authorization: "Bearer test_token" },
+        }
+      )
+
+      expect(getCustomerResponse.data.customers).toEqual(
+        expect.arrayContaining([
+          expect.objectContaining({
+            id: "test-customer-5",
+            groups: [],
+          }),
+          expect.objectContaining({
+            id: "test-customer-6",
+            groups: [],
+          }),
+        ])
+      )
+    })
+
+    it("removes customers from only one group", async () => {
+      const api = useApi()
+
+      const payload = {
+        customer_ids: [{ id: "test-customer-7" }],
+      }
+
+      const batchAddResponse = await api
+        .delete("/admin/customer-groups/test-group-5/customers/batch", {
+          headers: {
+            Authorization: "Bearer test_token",
+          },
+          data: payload,
+        })
+        .catch((err) => console.log(err))
+
+      expect(batchAddResponse.status).toEqual(200)
+      expect(batchAddResponse.data).toEqual({
+        customer_group: expect.objectContaining({
+          id: "test-group-5",
+          name: "test-group-5",
+        }),
+      })
+
+      const getCustomerResponse = await api.get(
+        "/admin/customers/test-customer-7?expand=groups",
+        {
+          headers: { Authorization: "Bearer test_token" },
+        }
+      )
+
+      expect(getCustomerResponse.data.customer).toEqual(
+        expect.objectContaining({
+          id: "test-customer-7",
+          groups: [
+            expect.objectContaining({
+              id: "test-group-6",
+              name: "test-group-6",
+            }),
+          ],
+        })
+      )
+    })
+
+    it("removes only select customers from a group", async () => {
+      const api = useApi()
+
+      // re-adding customer-1 to the customer group along with new addintion:
+      // customer-2 and some non-existing customers should cause the request to fail
+      const payload = {
+        customer_ids: [{ id: "test-customer-5" }],
+      }
+
+      await api.delete("/admin/customer-groups/test-group-5/customers/batch", {
+        headers: {
+          Authorization: "Bearer test_token",
+        },
+        data: payload,
+      })
+
+      // check that customer-1 is only added once and that customer-2 is added correctly
+      const getCustomerResponse = await api
+        .get("/admin/customers?expand=groups", {
+          headers: { Authorization: "Bearer test_token" },
+        })
+        .catch((err) => console.log(err))
+
+      expect(getCustomerResponse.data.customers).toEqual(
+        expect.arrayContaining([
+          expect.objectContaining({
+            id: "test-customer-5",
+            groups: [],
+          }),
+          expect.objectContaining({
+            id: "test-customer-6",
+            groups: [
+              expect.objectContaining({
+                name: "test-group-5",
+                id: "test-group-5",
+              }),
+            ],
+          }),
+        ])
+      )
+    })
+
+    it("removes customers from a group idempotently", async () => {
+      const api = useApi()
+
+      // re-adding customer-1 to the customer group along with new addintion:
+      // customer-2 and some non-existing customers should cause the request to fail
+      const payload = {
+        customer_ids: [{ id: "test-customer-5" }],
+      }
+
+      await api.delete("/admin/customer-groups/test-group-5/customers/batch", {
+        headers: {
+          Authorization: "Bearer test_token",
+        },
+        data: payload,
+      })
+
+      const idempotentRes = await api.delete(
+        "/admin/customer-groups/test-group-5/customers/batch",
+        {
+          headers: {
+            Authorization: "Bearer test_token",
+          },
+          data: payload,
+        }
+      )
+
+      expect(idempotentRes.status).toEqual(200)
+      expect(idempotentRes.data).toEqual({
+        customer_group: expect.objectContaining({
+          id: "test-group-5",
+          name: "test-group-5",
+        }),
+      })
+    })
+  })
 })

integration-tests/api/__tests__/admin/customer.js

@@ -56,7 +56,7 @@ describe("/admin/customers", () => {
         })
 
       expect(response.status).toEqual(200)
-      expect(response.data.count).toEqual(6)
+      expect(response.data.count).toEqual(8)
       expect(response.data.customers).toEqual(
         expect.arrayContaining([
           expect.objectContaining({

integration-tests/api/helpers/customer-seeder.js

@@ -36,12 +36,6 @@ module.exports = async (connection, data = {}) => {
     has_account: true,
   })
 
-  await manager.insert(Customer, {
-    id: "test-customer-5",
-    email: "test5@email.com",
-    groups: [{ id: "test-group-5", name: "test-group-5" }],
-  })
-
   const deletionCustomer = await manager.create(Customer, {
     id: "test-customer-delete-cg",
     email: "test-deletetion-cg@email.com",
@@ -68,11 +62,40 @@ module.exports = async (connection, data = {}) => {
     name: "test-group-4",
   })
 
-  await manager.insert(CustomerGroup, {
+  const customer5 = manager.create(Customer, {
+    id: "test-customer-5",
+    email: "test5@email.com",
+  })
+
+  const customer6 = manager.create(Customer, {
+    id: "test-customer-6",
+    email: "test6@email.com",
+  })
+
+  const customer7 = manager.create(Customer, {
+    id: "test-customer-7",
+    email: "test7@email.com",
+  })
+
+  const c_group_5 = manager.create(CustomerGroup, {
     id: "test-group-5",
     name: "test-group-5",
   })
 
+  const c_group_6 = manager.create(CustomerGroup, {
+    id: "test-group-6",
+    name: "test-group-6",
+  })
+
+  customer5.groups = [c_group_5]
+  await manager.save(customer5)
+
+  customer6.groups = [c_group_5]
+  await manager.save(customer6)
+
+  customer7.groups = [c_group_5, c_group_6]
+  await manager.save(customer7)
+
   const c_group_delete = manager.create(CustomerGroup, {
     id: "test-group-delete",
     name: "test-group-delete",

packages/medusa/src/api/routes/admin/customer-groups/delete-customers-batch.ts

@@ -0,0 +1,51 @@
+import { Type } from "class-transformer"
+import { ValidateNested } from "class-validator"
+import { CustomerGroupService } from "../../../../services"
+import { CustomerGroupsBatchCustomer } from "../../../../types/customer-groups"
+import { validator } from "../../../../utils/validator"
+
+/**
+ * @oas [delete] /customer-groups/{id}/customers/batch
+ * operationId: "DeleteCustomerGroupsGroupCustomerBatch"
+ * summary: "Remove a list of customers from a customer group "
+ * description: "Removes a list of customers, represented by id's, from a customer group."
+ * x-authenticated: true
+ * parameters:
+ *   - (path) id=* {string} The id of the customer group.
+ *   - (body) customers=* {{id: string }[]} ids of the customers to remove
+ * tags:
+ *   - CustomerGroup
+ * responses:
+ *   200:
+ *     description: OK
+ *     content:
+ *       application/json:
+ *         schema:
+ *           properties:
+ *             customerGroup:
+ *               $ref: "#/components/schemas/customergroup"
+ */
+
+export default async (req, res) => {
+  const { id } = req.params
+  const validated = await validator(
+    AdminDeleteCustomerGroupsGroupCustomerBatchReq,
+    req.body
+  )
+
+  const customerGroupService: CustomerGroupService = req.scope.resolve(
+    "customerGroupService"
+  )
+
+  const customer_group = await customerGroupService.removeCustomer(
+    id,
+    validated.customer_ids.map(({ id }) => id)
+  )
+  res.status(200).json({ customer_group })
+}
+
+export class AdminDeleteCustomerGroupsGroupCustomerBatchReq {
+  @ValidateNested({ each: true })
+  @Type(() => CustomerGroupsBatchCustomer)
+  customer_ids: CustomerGroupsBatchCustomer[]
+}

packages/medusa/src/api/routes/admin/customer-groups/index.ts

@@ -10,6 +10,11 @@ export default (app) => {
 
   route.get("/:id", middlewares.wrap(require("./get-customer-group").default))
   route.post("/", middlewares.wrap(require("./create-customer-group").default))
+  route.delete(
+    "/:id/customers/batch",
+    middlewares.wrap(require("./delete-customers-batch").default)
+  )
+
   route.delete(
     "/:id",
     middlewares.wrap(require("./delete-customer-group").default)

packages/medusa/src/migrations/1644943746861-customer_groups.ts

@@ -20,10 +20,10 @@ export class customerGroups1644943746861 implements MigrationInterface {
       `CREATE INDEX "IDX_3c6412d076292f439269abe1a2" ON "customer_group_customers" ("customer_id") `
     )
     await queryRunner.query(
-      `ALTER TABLE "customer_group_customers" ADD CONSTRAINT "FK_620330964db8d2999e67b0dbe3e" FOREIGN KEY ("customer_group_id") REFERENCES "customer_group"("id") ON DELETE CASCADE ON UPDATE CASCADE`
+      `ALTER TABLE "customer_group_customers" ADD CONSTRAINT "FK_620330964db8d2999e67b0dbe3e" FOREIGN KEY ("customer_group_id") REFERENCES "customer_group"("id") ON DELETE CASCADE ON UPDATE NO ACTION`
     )
     await queryRunner.query(
-      `ALTER TABLE "customer_group_customers" ADD CONSTRAINT "FK_3c6412d076292f439269abe1a23" FOREIGN KEY ("customer_id") REFERENCES "customer"("id") ON DELETE CASCADE ON UPDATE CASCADE`
+      `ALTER TABLE "customer_group_customers" ADD CONSTRAINT "FK_3c6412d076292f439269abe1a23" FOREIGN KEY ("customer_id") REFERENCES "customer"("id") ON DELETE CASCADE ON UPDATE NO ACTION`
     )
   }
 

packages/medusa/src/models/customer-group.ts

@@ -23,7 +23,9 @@ export class CustomerGroup {
   @Column()
   name: string
 
-  @ManyToMany(() => Customer, { cascade: true })
+  @ManyToMany(() => Customer, (customer) => customer.groups, {
+    onDelete: "CASCADE",
+  })
   @JoinTable({
     name: "customer_group_customers",
     joinColumn: {

packages/medusa/src/models/customer.ts

@@ -43,10 +43,7 @@ export class Customer {
   @JoinColumn({ name: "billing_address_id" })
   billing_address: Address
 
-  @OneToMany(
-    () => Address,
-    (address) => address.customer
-  )
+  @OneToMany(() => Address, (address) => address.customer)
   shipping_addresses: Address[]
 
   @Column({ nullable: true, select: false })
@@ -58,10 +55,7 @@ export class Customer {
   @Column({ default: false })
   has_account: boolean
 
-  @OneToMany(
-    () => Order,
-    (order) => order.customer
-  )
+  @OneToMany(() => Order, (order) => order.customer)
   orders: Order[]
 
   @JoinTable({
@@ -75,7 +69,7 @@ export class Customer {
       referencedColumnName: "id",
     },
   })
-  @ManyToMany(() => CustomerGroup, { cascade: true })
+  @ManyToMany(() => CustomerGroup, (cg) => cg.customers, { cascade: true })
   groups: CustomerGroup[]
 
   @CreateDateColumn({ type: resolveDbType("timestamptz") })

packages/medusa/src/repositories/customer-group.ts

@@ -1,5 +1,19 @@
-import { EntityRepository, Repository } from "typeorm"
+import { DeleteResult, EntityRepository, In, Repository } from "typeorm"
 import { CustomerGroup } from "../models/customer-group"
 
 @EntityRepository(CustomerGroup)
-export class CustomerGroupRepository extends Repository<CustomerGroup> {}
+export class CustomerGroupRepository extends Repository<CustomerGroup> {
+  async removeCustomers(
+    groupId: string,
+    customerIds: string[]
+  ): Promise<DeleteResult> {
+    return await this.createQueryBuilder()
+      .delete()
+      .from("customer_group_customers")
+      .where({
+        customer_group_id: groupId,
+        customer_id: In(customerIds),
+      })
+      .execute()
+  }
+}

packages/medusa/src/services/customer-group.ts

@@ -161,6 +161,34 @@ class CustomerGroupService extends BaseService {
     const query = this.buildQuery_(selector, config)
     return await cgRepo.find(query)
   }
+
+  /**
+   * Remove list of customers from a customergroup
+   *
+   * @param {string} id id of the customer group from which the customers are removed
+   * @param {string[] | string} customerIds id's of the customer to remove from group
+   * @return {Promise<CustomerGroup>} the customergroup with the provided id
+   */
+  async removeCustomer(
+    id: string,
+    customerIds: string[] | string
+  ): Promise<CustomerGroup> {
+    const cgRepo: CustomerGroupRepository = this.manager_.getCustomRepository(
+      this.customerGroupRepository_
+    )
+    let ids: string[]
+    if (typeof customerIds === "string") {
+      ids = [customerIds]
+    } else {
+      ids = customerIds
+    }
+
+    const customerGroup = await this.retrieve(id)
+
+    await cgRepo.removeCustomers(id, ids)
+
+    return customerGroup
+  }
 }
 
 export default CustomerGroupService

packages/medusa/src/types/customer-groups.ts

@@ -1,4 +1,4 @@
-import { ValidateNested } from "class-validator"
+import { IsString, ValidateNested } from "class-validator"
 import { IsType } from "../utils/validators/is-type"
 
 import { StringComparisonOperator } from "./common"
@@ -9,6 +9,10 @@ export class FilterableCustomerGroupProps {
   id?: string | string[] | StringComparisonOperator
 }
 
+export class CustomerGroupsBatchCustomer {
+  @IsString()
+  id: string
+}
 export class CustomerGroupUpdate {
   name?: string
   metadata?: object