From 22f3f2af93fcebef5ebf89ce66cd926393ae5d25 Mon Sep 17 00:00:00 2001 From: Kasper Fabricius Kristensen <45367945+kasperkristensen@users.noreply.github.com> Date: Fri, 1 Oct 2021 08:18:56 +0200 Subject: [PATCH] fix: shipping option updates (#426) * fix to remove req * tested fix --- ...te_date_on_shipping_option_requirements.ts | 18 ++++++++ .../src/models/shipping-option-requirement.ts | 5 +- .../src/services/__tests__/shipping-option.js | 38 ++++++--------- .../medusa/src/services/shipping-option.js | 46 ++++++++++++------- 4 files changed, 66 insertions(+), 41 deletions(-) create mode 100644 packages/medusa/src/migrations/1632828114899-delete_date_on_shipping_option_requirements.ts diff --git a/packages/medusa/src/migrations/1632828114899-delete_date_on_shipping_option_requirements.ts b/packages/medusa/src/migrations/1632828114899-delete_date_on_shipping_option_requirements.ts new file mode 100644 index 0000000000..12011f31a9 --- /dev/null +++ b/packages/medusa/src/migrations/1632828114899-delete_date_on_shipping_option_requirements.ts @@ -0,0 +1,18 @@ +import { MigrationInterface, QueryRunner } from "typeorm" + +export class deleteDateOnShippingOptionRequirements1632828114899 + implements MigrationInterface { + name = "deleteDateOnShippingOptionRequirements1632828114899" + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query( + `ALTER TABLE "shipping_option_requirement" ADD "deleted_at" TIMESTAMP WITH TIME ZONE` + ) + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query( + `ALTER TABLE "shipping_option_requirement" DROP COLUMN "deleted_at"` + ) + } +} diff --git a/packages/medusa/src/models/shipping-option-requirement.ts b/packages/medusa/src/models/shipping-option-requirement.ts index b847687d02..547ddf58cd 100644 --- a/packages/medusa/src/models/shipping-option-requirement.ts +++ b/packages/medusa/src/models/shipping-option-requirement.ts @@ -16,7 +16,7 @@ import { JoinTable, } from "typeorm" import { ulid } from "ulid" -import { DbAwareColumn } from "../utils/db-aware-column" +import { DbAwareColumn, resolveDbType } from "../utils/db-aware-column" import { ShippingOption } from "./shipping-option" @@ -44,6 +44,9 @@ export class ShippingOptionRequirement { @Column({ type: "int" }) amount: number + @DeleteDateColumn({ type: resolveDbType("timestamptz") }) + deleted_at: Date + @BeforeInsert() private beforeInsert() { if (this.id) return diff --git a/packages/medusa/src/services/__tests__/shipping-option.js b/packages/medusa/src/services/__tests__/shipping-option.js index e72c4d74b3..8001065535 100644 --- a/packages/medusa/src/services/__tests__/shipping-option.js +++ b/packages/medusa/src/services/__tests__/shipping-option.js @@ -296,24 +296,19 @@ describe("ShippingOptionService", () => { }) describe("removeRequirement", () => { - const shippingOptionRepository = MockRepository({ - findOne: q => { - switch (q.where.id) { - default: - return Promise.resolve({ - requirements: [ - { - id: IdMap.getId("requirement_id"), - }, - ], - }) - } + const shippingOptionRequirementRepository = MockRepository({ + softRemove: q => { + return Promise.resolve() }, + findOne: i => + i.where.id === IdMap.getId("requirement_id") + ? { id: IdMap.getId("requirement_id") } + : null, }) const optionService = new ShippingOptionService({ manager: MockManager, - shippingOptionRepository, + shippingOptionRequirementRepository, }) beforeEach(() => { @@ -321,22 +316,19 @@ describe("ShippingOptionService", () => { }) it("remove requirement successfully", async () => { - await optionService.removeRequirement( - IdMap.getId("validId"), - IdMap.getId("requirement_id") - ) + await optionService.removeRequirement(IdMap.getId("requirement_id")) - expect(shippingOptionRepository.save).toBeCalledTimes(1) - expect(shippingOptionRepository.save).toBeCalledWith({ requirements: [] }) + expect(shippingOptionRequirementRepository.findOne).toBeCalledTimes(1) + expect(shippingOptionRequirementRepository.findOne).toBeCalledWith({ + where: { id: IdMap.getId("requirement_id") }, + }) + expect(shippingOptionRequirementRepository.softRemove).toBeCalledTimes(1) }) it("is idempotent", async () => { await optionService.removeRequirement(IdMap.getId("validId"), "something") - expect(shippingOptionRepository.save).toBeCalledTimes(1) - expect(shippingOptionRepository.save).toBeCalledWith({ - requirements: [{ id: IdMap.getId("requirement_id") }], - }) + expect(shippingOptionRequirementRepository.softRemove).toBeCalledTimes(1) }) }) diff --git a/packages/medusa/src/services/shipping-option.js b/packages/medusa/src/services/shipping-option.js index c745bfbfb1..ad2cbf8724 100644 --- a/packages/medusa/src/services/shipping-option.js +++ b/packages/medusa/src/services/shipping-option.js @@ -450,7 +450,9 @@ class ShippingOptionService extends BaseService { */ async update(optionId, update) { return this.atomicPhase_(async manager => { - const option = await this.retrieve(optionId) + const option = await this.retrieve(optionId, { + relations: ["requirements"], + }) if ("metadata" in update) { option.metadata = await this.setMetadata_(option, update.metadata) @@ -498,6 +500,20 @@ class ShippingOptionService extends BaseService { acc.push(validated) } + + if (option.requirements) { + const accReqs = acc.map(a => a.id) + const toRemove = option.requirements.filter( + r => !accReqs.includes(r.id) + ) + await Promise.all( + toRemove.map(async req => { + await this.removeRequirement(req.id) + }) + ) + } + + option.requirements = acc } if ("price_type" in update) { @@ -585,28 +601,24 @@ class ShippingOptionService extends BaseService { /** * Removes a requirement from a shipping option - * @param {string} optionId - the shipping option to remove from * @param {string} requirementId - the id of the requirement to remove * @return {Promise} the result of update */ - async removeRequirement(optionId, requirementId) { + async removeRequirement(requirementId) { return this.atomicPhase_(async manager => { - const option = await this.retrieve(optionId, { - relations: "requirements", - }) - const newReqs = option.requirements.map(r => { - if (r.id === requirementId) { - return null - } else { - return r - } - }) + try { + const reqRepo = manager.getCustomRepository(this.requirementRepository_) + const requirement = await reqRepo.findOne({ + where: { id: requirementId }, + }) - option.requirements = newReqs.filter(Boolean) + const result = await reqRepo.softRemove(requirement) - const optionRepo = manager.getCustomRepository(this.optionRepository_) - const result = await optionRepo.save(option) - return result + return result + } catch (error) { + // Delete is idempotent, but we return a promise to allow then-chaining + return Promise.resolve() + } }) }