From 0394be36ef877f694e2d5518ccf6967a2828dbf4 Mon Sep 17 00:00:00 2001 From: Philip Korsholm <88927411+pKorsholm@users.noreply.github.com> Date: Fri, 25 Feb 2022 15:29:13 +0100 Subject: [PATCH] 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 * 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 --- .../api/__tests__/admin/customer-groups.js | 184 +++++++++++++++++- .../api/__tests__/admin/customer.js | 2 +- .../api/helpers/customer-seeder.js | 37 +++- .../customer-groups/delete-customers-batch.ts | 51 +++++ .../api/routes/admin/customer-groups/index.ts | 5 + .../1644943746861-customer_groups.ts | 4 +- packages/medusa/src/models/customer-group.ts | 4 +- packages/medusa/src/models/customer.ts | 12 +- .../medusa/src/repositories/customer-group.ts | 18 +- .../medusa/src/services/customer-group.ts | 28 +++ packages/medusa/src/types/customer-groups.ts | 6 +- 11 files changed, 327 insertions(+), 24 deletions(-) create mode 100644 packages/medusa/src/api/routes/admin/customer-groups/delete-customers-batch.ts diff --git a/integration-tests/api/__tests__/admin/customer-groups.js b/integration-tests/api/__tests__/admin/customer-groups.js index 95dd8d9e7e..c42eae7590 100644 --- a/integration-tests/api/__tests__/admin/customer-groups.js +++ b/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", + }), + }) + }) + }) }) diff --git a/integration-tests/api/__tests__/admin/customer.js b/integration-tests/api/__tests__/admin/customer.js index bf0d9c8fea..df4fdbc6f7 100644 --- a/integration-tests/api/__tests__/admin/customer.js +++ b/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({ diff --git a/integration-tests/api/helpers/customer-seeder.js b/integration-tests/api/helpers/customer-seeder.js index 92224e2cc9..34e6d56811 100644 --- a/integration-tests/api/helpers/customer-seeder.js +++ b/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", diff --git a/packages/medusa/src/api/routes/admin/customer-groups/delete-customers-batch.ts b/packages/medusa/src/api/routes/admin/customer-groups/delete-customers-batch.ts new file mode 100644 index 0000000000..1ea51f6ca4 --- /dev/null +++ b/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[] +} diff --git a/packages/medusa/src/api/routes/admin/customer-groups/index.ts b/packages/medusa/src/api/routes/admin/customer-groups/index.ts index 7bbed35516..bfcb68e536 100644 --- a/packages/medusa/src/api/routes/admin/customer-groups/index.ts +++ b/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) diff --git a/packages/medusa/src/migrations/1644943746861-customer_groups.ts b/packages/medusa/src/migrations/1644943746861-customer_groups.ts index a84e7b7aa6..39bdeba12b 100644 --- a/packages/medusa/src/migrations/1644943746861-customer_groups.ts +++ b/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` ) } diff --git a/packages/medusa/src/models/customer-group.ts b/packages/medusa/src/models/customer-group.ts index c7b7685064..20ed261cc9 100644 --- a/packages/medusa/src/models/customer-group.ts +++ b/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: { diff --git a/packages/medusa/src/models/customer.ts b/packages/medusa/src/models/customer.ts index 7e67a86c32..3c66552a9f 100644 --- a/packages/medusa/src/models/customer.ts +++ b/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") }) diff --git a/packages/medusa/src/repositories/customer-group.ts b/packages/medusa/src/repositories/customer-group.ts index cb38231ef3..9a9f09e447 100644 --- a/packages/medusa/src/repositories/customer-group.ts +++ b/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 {} +export class CustomerGroupRepository extends Repository { + async removeCustomers( + groupId: string, + customerIds: string[] + ): Promise { + return await this.createQueryBuilder() + .delete() + .from("customer_group_customers") + .where({ + customer_group_id: groupId, + customer_id: In(customerIds), + }) + .execute() + } +} diff --git a/packages/medusa/src/services/customer-group.ts b/packages/medusa/src/services/customer-group.ts index 671b9e48e8..d6bba1c194 100644 --- a/packages/medusa/src/services/customer-group.ts +++ b/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} the customergroup with the provided id + */ + async removeCustomer( + id: string, + customerIds: string[] | string + ): Promise { + 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 diff --git a/packages/medusa/src/types/customer-groups.ts b/packages/medusa/src/types/customer-groups.ts index 8812821c16..607d5056ae 100644 --- a/packages/medusa/src/types/customer-groups.ts +++ b/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