From b4aa7fb9a752d3be383f60cb2f600db536856f4d Mon Sep 17 00:00:00 2001 From: Stevche Radevski Date: Tue, 2 Jul 2024 17:06:58 +0200 Subject: [PATCH] fix: Disallow creating duplicate prices (#7866) * fix: Disallow creating duplicate prices * fix: Don't pass id to manager create in upsertWithReplace --- .../price-list/admin/price-list.spec.ts | 109 ++-- .../steps/create-price-list-prices.ts | 7 +- .../steps/update-price-list-prices.ts | 8 +- .../src/dal/mikro-orm/mikro-orm-repository.ts | 16 +- .../pricing-module/calculate-price.spec.ts | 2 +- .../pricing-module/price-list.spec.ts | 120 +++++ .../services/pricing-module/price-set.spec.ts | 126 ++++- .../pricing/src/repositories/pricing.ts | 5 +- .../pricing/src/services/pricing-module.ts | 484 ++++++++---------- .../pricing/src/types/services/index.ts | 1 + .../pricing/src/types/services/price.ts | 8 + packages/modules/pricing/src/utils/events.ts | 24 + 12 files changed, 560 insertions(+), 350 deletions(-) create mode 100644 packages/modules/pricing/src/types/services/price.ts diff --git a/integration-tests/http/__tests__/price-list/admin/price-list.spec.ts b/integration-tests/http/__tests__/price-list/admin/price-list.spec.ts index 6e569a1016..f2f2c1cf27 100644 --- a/integration-tests/http/__tests__/price-list/admin/price-list.spec.ts +++ b/integration-tests/http/__tests__/price-list/admin/price-list.spec.ts @@ -113,7 +113,9 @@ medusaIntegrationTestRunner({ { amount: 105, currency_code: region1.currency_code, - region_id: region1.id, + rules: { + region_id: region1.id, + }, variant_id: product1.variants[0].id, }, ], @@ -508,84 +510,61 @@ medusaIntegrationTestRunner({ ) }) - it.skip("Adds a batch of new prices to a price list overriding existing prices", async () => { - // BREAKING: There is no support for overriding configuration + it("Adds a batch of new prices to a price list overriding existing prices", async () => { + // BREAKING: The payload of the batch request changed + // BREAKING: The create dataset does an upsert (before an explicit `override` flag was passed) const payload = { - prices: [ + create: [ { amount: 45, currency_code: "usd", - variant_id: "test-variant", - min_quantity: 1001, - max_quantity: 2000, + variant_id: product1.variants[0].id, + min_quantity: 1, + max_quantity: 100, }, { amount: 35, currency_code: "usd", - variant_id: "test-variant", - min_quantity: 2001, - max_quantity: 3000, - }, - { - amount: 25, - currency_code: "usd", - variant_id: "test-variant", - min_quantity: 3001, - max_quantity: 4000, + variant_id: product1.variants[0].id, + min_quantity: 101, + max_quantity: 500, }, ], - override: true, } - const response = await api.post( - "/admin/price-lists/pl_no_customer_groups/prices/batch", + await api.post( + `/admin/price-lists/${pricelist1.id}/prices/batch`, payload, adminHeaders ) + const response = await api.get( + `/admin/price-lists/${pricelist1.id}`, + adminHeaders + ) + expect(response.status).toEqual(200) - expect(response.data.price_list.prices.length).toEqual(3) - expect(response.data.price_list.prices).toMatchSnapshot([ - { - id: expect.any(String), - price_list_id: "pl_no_customer_groups", - amount: 45, - currency_code: "usd", - variant_id: "test-variant", - min_quantity: 1001, - max_quantity: 2000, - created_at: expect.any(String), - updated_at: expect.any(String), - variant: expect.any(Object), - variants: expect.any(Array), - }, - { - id: expect.any(String), - price_list_id: "pl_no_customer_groups", - amount: 35, - currency_code: "usd", - variant_id: "test-variant", - min_quantity: 2001, - max_quantity: 3000, - created_at: expect.any(String), - updated_at: expect.any(String), - variant: expect.any(Object), - variants: expect.any(Array), - }, - { - id: expect.any(String), - price_list_id: "pl_no_customer_groups", - amount: 25, - currency_code: "usd", - variant_id: "test-variant", - min_quantity: 3001, - max_quantity: 4000, - created_at: expect.any(String), - updated_at: expect.any(String), - variant: expect.any(Object), - variants: expect.any(Array), - }, - ]) + expect(response.data.price_list.prices.length).toEqual(2) + expect(response.data.price_list.prices).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: expect.any(String), + amount: 45, + currency_code: "usd", + min_quantity: "1", + max_quantity: "100", + variant_id: product1.variants[0].id, + }), + expect.objectContaining({ + id: expect.any(String), + amount: 35, + currency_code: "usd", + min_quantity: "101", + max_quantity: "500", + variant_id: product1.variants[0].id, + }), + ]) + ) }) it("Adds a batch of new prices where a MA record have a `region_id` instead of `currency_code`", async () => { @@ -596,7 +575,9 @@ medusaIntegrationTestRunner({ amount: 100, variant_id: product1.variants[0].id, currency_code: "eur", - region_id: region1.id, + rules: { + region_id: region1.id, + }, }, { amount: 200, @@ -640,7 +621,7 @@ medusaIntegrationTestRunner({ id: expect.any(String), amount: 100, currency_code: "eur", - // region_id: region1.id, + rules: { region_id: region1.id }, variant_id: product1.variants[0].id, }), expect.objectContaining({ diff --git a/packages/core/core-flows/src/price-list/steps/create-price-list-prices.ts b/packages/core/core-flows/src/price-list/steps/create-price-list-prices.ts index 451302aaa4..273e697b65 100644 --- a/packages/core/core-flows/src/price-list/steps/create-price-list-prices.ts +++ b/packages/core/core-flows/src/price-list/steps/create-price-list-prices.ts @@ -2,6 +2,7 @@ import { ModuleRegistrationName } from "@medusajs/modules-sdk" import { AddPriceListPricesDTO, CreatePriceListPriceDTO, + CreatePriceListPriceWorkflowDTO, CreatePriceListPricesWorkflowStepDTO, IPricingModuleService, } from "@medusajs/types" @@ -22,10 +23,12 @@ export const createPriceListPricesStep = createStep( const pricesToAdd: CreatePriceListPriceDTO[] = [] for (const price of prices) { - pricesToAdd.push({ + const toPush = { ...price, price_set_id: variantPriceSetMap[price.variant_id!], - }) + } as CreatePriceListPriceDTO + delete (toPush as Partial).variant_id + pricesToAdd.push(toPush) } if (pricesToAdd.length) { diff --git a/packages/core/core-flows/src/price-list/steps/update-price-list-prices.ts b/packages/core/core-flows/src/price-list/steps/update-price-list-prices.ts index d4f36999b0..7b0f83bd80 100644 --- a/packages/core/core-flows/src/price-list/steps/update-price-list-prices.ts +++ b/packages/core/core-flows/src/price-list/steps/update-price-list-prices.ts @@ -4,6 +4,7 @@ import { PriceDTO, UpdatePriceListPriceDTO, UpdatePriceListPricesDTO, + UpdatePriceListPriceWorkflowDTO, UpdatePriceListPriceWorkflowStepDTO, } from "@medusajs/types" import { buildPriceSetPricesForModule } from "@medusajs/utils" @@ -25,10 +26,13 @@ export const updatePriceListPricesStep = createStep( const { prices = [], id } = priceListData for (const price of prices) { - pricesToUpdate.push({ + const toPush = { ...price, price_set_id: variantPriceSetMap[price.variant_id!], - }) + } as UpdatePriceListPriceDTO + delete (toPush as Partial).variant_id + + pricesToUpdate.push(toPush) if (price.id) { priceIds.push(price.id) diff --git a/packages/core/utils/src/dal/mikro-orm/mikro-orm-repository.ts b/packages/core/utils/src/dal/mikro-orm/mikro-orm-repository.ts index 25ddee234e..c96d3ab134 100644 --- a/packages/core/utils/src/dal/mikro-orm/mikro-orm-repository.ts +++ b/packages/core/utils/src/dal/mikro-orm/mikro-orm-repository.ts @@ -747,16 +747,22 @@ export function mikroOrmBaseRepositoryFactory( entityName: string, data: any ): Record & { id: string } { - const created = manager.create(entityName, data, { - managed: false, - persist: false, - }) + // We set the id to undefined to make sure the entity isn't fetched from the entity map if it is an update, + // giving us incorrect data for the bignumberdata field (I though managed: false and persist: false would already do this) + const created = manager.create( + entityName, + { ...data, id: undefined }, + { + managed: false, + persist: false, + } + ) const resp = { // `create` will omit non-existent fields, but we want to pass the data the user provided through so the correct errors get thrown ...data, ...(created as any).__helper.__bignumberdata, - id: (created as any).id, + id: data.id ?? (created as any).id, } // Non-persist relation columns should be removed before we do the upsert. diff --git a/packages/modules/pricing/integration-tests/__tests__/services/pricing-module/calculate-price.spec.ts b/packages/modules/pricing/integration-tests/__tests__/services/pricing-module/calculate-price.spec.ts index 7be89fd8f2..5fc8b63dec 100644 --- a/packages/modules/pricing/integration-tests/__tests__/services/pricing-module/calculate-price.spec.ts +++ b/packages/modules/pricing/integration-tests/__tests__/services/pricing-module/calculate-price.spec.ts @@ -799,7 +799,7 @@ moduleIntegrationTestRunner({ { id: "price-set-PLN", is_calculated_price_price_list: true, - calculated_amount: 116, + calculated_amount: 232, is_original_price_price_list: false, original_amount: 400, currency_code: "PLN", diff --git a/packages/modules/pricing/integration-tests/__tests__/services/pricing-module/price-list.spec.ts b/packages/modules/pricing/integration-tests/__tests__/services/pricing-module/price-list.spec.ts index 8e37791c8f..4800a0a178 100644 --- a/packages/modules/pricing/integration-tests/__tests__/services/pricing-module/price-list.spec.ts +++ b/packages/modules/pricing/integration-tests/__tests__/services/pricing-module/price-list.spec.ts @@ -586,6 +586,63 @@ moduleIntegrationTestRunner({ }) ) }) + + it("should take the later price when passing two equivalent prices twice", async () => { + const [created] = await service.createPriceLists([ + { + title: "test", + description: "test", + starts_at: "10/10/2010", + ends_at: "10/20/2030", + prices: [ + { + amount: 400, + currency_code: "EUR", + price_set_id: "price-set-1", + rules: { + region_id: "DE", + }, + }, + { + amount: 600, + currency_code: "EUR", + price_set_id: "price-set-1", + rules: { + region_id: "DE", + }, + }, + ], + }, + ]) + + const [priceList] = await service.listPriceLists( + { + id: [created.id], + }, + { + relations: ["prices", "prices.price_rules"], + } + ) + + expect(priceList).toEqual( + expect.objectContaining({ + id: expect.any(String), + prices: [ + expect.objectContaining({ + rules_count: 1, + price_rules: [ + expect.objectContaining({ + id: expect.any(String), + value: "DE", + }), + ], + amount: 600, + currency_code: "EUR", + }), + ], + }) + ) + }) }) describe("addPriceListPrices", () => { @@ -810,6 +867,69 @@ moduleIntegrationTestRunner({ }) ) }) + + it("should do an update to the price if an equivalent price already exists", async () => { + const [priceSet] = await service.createPriceSets([{}]) + + await service.addPriceListPrices([ + { + price_list_id: "price-list-1", + prices: [ + { + id: "test-price-id", + amount: 123, + currency_code: "EUR", + price_set_id: priceSet.id, + rules: { + region_id: "test", + }, + }, + ], + }, + ]) + + await service.addPriceListPrices([ + { + price_list_id: "price-list-1", + prices: [ + { + id: "test-price-id", + amount: 234, + currency_code: "EUR", + price_set_id: priceSet.id, + rules: { + region_id: "test", + }, + }, + ], + }, + ]) + + const [priceList] = await service.listPriceLists( + { id: ["price-list-1"] }, + { + relations: ["prices", "prices.price_rules"], + } + ) + + expect(priceList).toEqual( + expect.objectContaining({ + id: expect.any(String), + prices: expect.arrayContaining([ + expect.objectContaining({ + price_rules: [ + expect.objectContaining({ + value: "test", + attribute: "region_id", + }), + ], + amount: 234, + currency_code: "EUR", + }), + ]), + }) + ) + }) }) describe("removePrices", () => { diff --git a/packages/modules/pricing/integration-tests/__tests__/services/pricing-module/price-set.spec.ts b/packages/modules/pricing/integration-tests/__tests__/services/pricing-module/price-set.spec.ts index 216f2f63bb..221234cd10 100644 --- a/packages/modules/pricing/integration-tests/__tests__/services/pricing-module/price-set.spec.ts +++ b/packages/modules/pricing/integration-tests/__tests__/services/pricing-module/price-set.spec.ts @@ -398,6 +398,40 @@ moduleIntegrationTestRunner({ ]) ) }) + + it("should upsert the later price when setting a price set with existing equivalent rules", async () => { + await service.updatePriceSets(id, { + prices: [ + { + amount: 100, + currency_code: "USD", + rules: { region_id: "1234" }, + }, + { + amount: 200, + currency_code: "USD", + rules: { region_id: "1234" }, + }, + ], + }) + + const priceSet = await service.retrievePriceSet(id, { + relations: ["prices", "prices.price_rules"], + }) + + expect(priceSet.prices).toEqual([ + expect.objectContaining({ + amount: 200, + currency_code: "USD", + price_rules: [ + expect.objectContaining({ + attribute: "region_id", + value: "1234", + }), + ], + }), + ]) + }) }) describe("create", () => { @@ -512,6 +546,43 @@ moduleIntegrationTestRunner({ }) ) }) + + it("should take the later price when passing two prices with equivalent rules", async () => { + await service.createPriceSets([ + { + id: "price-set-new", + prices: [ + { + amount: 100, + currency_code: "USD", + rules: { region_id: "1234" }, + }, + { + amount: 200, + currency_code: "USD", + rules: { region_id: "1234" }, + }, + ], + } as unknown as CreatePriceSetDTO, + ]) + + const priceSet = await service.retrievePriceSet("price-set-new", { + relations: ["prices", "prices.price_rules"], + }) + + expect(priceSet.prices).toEqual([ + expect.objectContaining({ + amount: 200, + currency_code: "USD", + price_rules: [ + expect.objectContaining({ + attribute: "region_id", + value: "1234", + }), + ], + }), + ]) + }) }) describe("addPrices", () => { @@ -523,7 +594,7 @@ moduleIntegrationTestRunner({ { amount: 100, currency_code: "USD", - rules: { currency_code: "USD" }, + rules: { region_id: "1234" }, }, ], }, @@ -574,7 +645,7 @@ moduleIntegrationTestRunner({ { amount: 100, currency_code: "USD", - rules: { currency_code: "USD" }, + rules: { region_id: "region-1" }, }, ], }, @@ -616,6 +687,57 @@ moduleIntegrationTestRunner({ }), ]) }) + + it("should do an update if a price exists with the equivalent rules", async () => { + await service.addPrices([ + { + priceSetId: "price-set-1", + prices: [ + { + amount: 100, + currency_code: "USD", + rules: { region_id: "123" }, + }, + ], + }, + ]) + + await service.addPrices([ + { + priceSetId: "price-set-1", + prices: [ + { + amount: 200, + currency_code: "USD", + rules: { region_id: "123" }, + }, + ], + }, + ]) + + const priceSet = await service.retrievePriceSet("price-set-1", { + relations: ["prices", "prices.price_rules"], + }) + + expect( + priceSet.prices?.sort((a: any, b: any) => a.amount - b.amount) + ).toEqual([ + expect.objectContaining({ + amount: 200, + currency_code: "USD", + price_rules: [ + expect.objectContaining({ + attribute: "region_id", + value: "123", + }), + ], + }), + expect.objectContaining({ + amount: 500, + currency_code: "USD", + }), + ]) + }) }) }) }, diff --git a/packages/modules/pricing/src/repositories/pricing.ts b/packages/modules/pricing/src/repositories/pricing.ts index f4b606125f..a20bf83583 100644 --- a/packages/modules/pricing/src/repositories/pricing.ts +++ b/packages/modules/pricing/src/repositories/pricing.ts @@ -163,6 +163,9 @@ export class PricingRepository pl_rules_count: "price.pl_rules_count", price_list_type: "price.pl_type", price_list_id: "price.price_list_id", + all_rules_count: knex.raw( + "COALESCE(price.rules_count, 0) + COALESCE(price.pl_rules_count, 0)" + ), }) .join(priceSubQueryKnex.as("price"), "price.price_set_id", "ps.id") .leftJoin("price_rule as pr", "pr.price_id", "price.id") @@ -171,8 +174,8 @@ export class PricingRepository .orderBy([ { column: "price.has_price_list", order: "asc" }, + { column: "all_rules_count", order: "desc" }, { column: "amount", order: "asc" }, - { column: "rules_count", order: "desc" }, ]) if (quantity) { diff --git a/packages/modules/pricing/src/services/pricing-module.ts b/packages/modules/pricing/src/services/pricing-module.ts index 62ccd9aee5..ec5dbf1b07 100644 --- a/packages/modules/pricing/src/services/pricing-module.ts +++ b/packages/modules/pricing/src/services/pricing-module.ts @@ -9,7 +9,6 @@ import { InternalModuleDeclaration, ModuleJoinerConfig, ModulesSdkTypes, - PriceDTO, PriceSetDTO, PricingContext, PricingFilters, @@ -32,6 +31,7 @@ import { PriceListType, promiseAll, removeNullish, + simpleHash, } from "@medusajs/utils" import { Price, PriceList, PriceListRule, PriceRule, PriceSet } from "@models" @@ -39,7 +39,7 @@ import { Price, PriceList, PriceListRule, PriceRule, PriceSet } from "@models" import { ServiceTypes } from "@types" import { eventBuilders, validatePriceListDates } from "@utils" import { entityNameToLinkableKeysMap, joinerConfig } from "../joiner-config" -import { CreatePriceListDTO } from "src/types/services" +import { CreatePriceListDTO, UpsertPriceDTO } from "src/types/services" type InjectedDependencies = { baseRepository: DAL.RepositoryService @@ -426,35 +426,6 @@ export default class PricingModuleService return isString(idOrSelector) ? priceSets[0] : priceSets } - private async normalizeUpdateData(data: ServiceTypes.UpdatePriceSetInput[]) { - return data.map((priceSet) => { - const prices = priceSet.prices?.map((price) => { - const rules = Object.entries(price.rules ?? {}).map( - ([attribute, value]) => { - return { - attribute, - value, - } - } - ) - - const hasRulesInput = isPresent(price.rules) - delete price.rules - return { - ...price, - price_set_id: priceSet.id, - price_rules: hasRulesInput ? rules : undefined, - rules_count: hasRulesInput ? rules.length : undefined, - } - }) - - return { - ...priceSet, - prices, - } - }) - } - @InjectTransactionManager("baseRepository_") protected async updatePriceSets_( data: ServiceTypes.UpdatePriceSetInput[], @@ -496,6 +467,67 @@ export default class PricingModuleService return priceSets } + private async normalizeUpdateData(data: ServiceTypes.UpdatePriceSetInput[]) { + return data.map((priceSet) => { + return { + ...priceSet, + prices: this.normalizePrices( + priceSet.prices?.map((p) => ({ ...p, price_set_id: priceSet.id })), + [] + ), + } + }) + } + + private normalizePrices( + data: CreatePricesDTO[] | undefined, + existingPrices: PricingTypes.PriceDTO[], + priceListId?: string | undefined + ) { + const pricesToUpsert = new Map< + string, + CreatePricesDTO & { price_rules?: CreatePriceRuleDTO[] } + >() + const existingPricesMap = new Map() + existingPrices?.forEach((price) => { + existingPricesMap.set(hashPrice(price), price) + }) + + data?.forEach((price) => { + const cleanRules = price.rules ? removeNullish(price.rules) : {} + const ruleEntries = Object.entries(cleanRules) + const rules = ruleEntries.map(([attribute, value]) => { + return { + attribute, + value, + } + }) + + const hasRulesInput = isPresent(price.rules) + const entry = { + ...price, + price_list_id: priceListId, + price_rules: hasRulesInput ? rules : undefined, + rules_count: hasRulesInput ? ruleEntries.length : undefined, + } as UpsertPriceDTO + delete (entry as CreatePricesDTO).rules + + const entryHash = hashPrice(entry) + + // We want to keep the existing rules as they might already have ids, but any other data should come from the updated input + const existing = existingPricesMap.get(entryHash) + pricesToUpsert.set(entryHash, { + ...entry, + id: existing?.id ?? entry.id, + price_rules: existing?.price_rules ?? entry.price_rules, + }) + + return entry + }) + + return Array.from(pricesToUpsert.values()) + } + async addPrices( data: AddPricesDTO, sharedContext?: Context @@ -619,35 +651,8 @@ export default class PricingModuleService const toCreate = input.map((inputData) => { const entry = { ...inputData, + prices: this.normalizePrices(inputData.prices, []), } - - if (!inputData.prices) { - return entry - } - - const pricesData: CreatePricesDTO[] = inputData.prices.map((price) => { - let { rules: priceRules = {}, ...rest } = price - const cleanRules = priceRules ? removeNullish(priceRules) : {} - const rules = Object.entries(cleanRules) - const numberOfRules = rules.length - - const rulesDataMap = new Map() - rules.map(([attribute, value]) => { - const rule = { - attribute, - value, - } - rulesDataMap.set(JSON.stringify(rule), rule) - }) - - return { - ...rest, - rules_count: numberOfRules, - price_rules: Array.from(rulesDataMap.values()), - } - }) - - entry.prices = pricesData return entry }) @@ -710,86 +715,74 @@ export default class PricingModuleService ) { const priceSets = await this.listPriceSets( { id: input.map((d) => d.priceSetId) }, - {}, + { take: null, relations: ["prices", "prices.price_rules"] }, sharedContext ) - const priceSetMap = new Map(priceSets.map((p) => [p.id, p])) - input.forEach(({ priceSetId }) => { - const priceSet = priceSetMap.get(priceSetId) + const existingPrices = priceSets + .map((p) => p.prices) + .flat() as PricingTypes.PriceDTO[] + + const pricesToUpsert = input + .map((addPrice) => + this.normalizePrices( + addPrice.prices?.map((p) => ({ + ...p, + price_set_id: addPrice.priceSetId, + })), + existingPrices + ) + ) + .filter(Boolean) + .flat() as UpsertPriceDTO[] + + const priceSetMap = new Map( + priceSets.map((p) => [p.id, p]) + ) + pricesToUpsert.forEach((price) => { + const priceSet = priceSetMap.get(price.price_set_id) if (!priceSet) { throw new MedusaError( MedusaError.Types.INVALID_DATA, - `Price set with id: ${priceSetId} not found` + `Price set with id: ${price.price_set_id} not found` ) } }) - const pricesToCreate: PricingTypes.CreatePriceDTO[] = input.flatMap( - ({ priceSetId, prices }) => - prices.map((price) => { - const numberOfRules = Object.entries(price?.rules ?? {}).length - - const priceRules = Object.entries(price.rules ?? {}).map( - ([attribute, value]) => ({ - price_set_id: priceSetId, - attribute: attribute, - value, - }) - ) - - return { - ...price, - price_set_id: priceSetId, - rules_count: numberOfRules, - price_rules: priceRules, - } - }) - ) - - const prices = await this.priceService_.create( - pricesToCreate, - sharedContext - ) - - /** - * Preparing data for emitting events - */ - const eventsData = prices.reduce( - (eventsData, price) => { - eventsData.prices.push({ - id: price.id, - }) - price.price_rules.map((priceRule) => { - eventsData.priceRules.push({ - id: priceRule.id, - }) - }) - return eventsData - }, - { - priceRules: [], - prices: [], - } as { - priceRules: { id: string }[] - prices: { id: string }[] - } - ) - - /** - * Emitting events for all created entities - */ + const { entities, performedActions } = + await this.priceService_.upsertWithReplace( + pricesToUpsert, + { relations: ["price_rules"] }, + sharedContext + ) eventBuilders.createdPrice({ - data: eventsData.prices, + data: performedActions.created[Price.name] ?? [], sharedContext, }) - eventBuilders.createdPriceRule({ - data: eventsData.priceRules, + eventBuilders.updatedPrice({ + data: performedActions.updated[Price.name] ?? [], + sharedContext, + }) + eventBuilders.deletedPrice({ + data: performedActions.deleted[Price.name] ?? [], sharedContext, }) - return prices + eventBuilders.createdPriceRule({ + data: performedActions.created[PriceRule.name] ?? [], + sharedContext, + }) + eventBuilders.updatedPriceRule({ + data: performedActions.updated[PriceRule.name] ?? [], + sharedContext, + }) + eventBuilders.deletedPriceRule({ + data: performedActions.deleted[PriceRule.name] ?? [], + sharedContext, + }) + + return entities } @InjectTransactionManager("baseRepository_") @@ -807,29 +800,10 @@ export default class PricingModuleService } as CreatePriceListDTO if (priceListData.prices) { - const pricesData = priceListData.prices.map((price) => { - let { rules: priceRules = {}, ...rest } = price - const cleanRules = priceRules ? removeNullish(priceRules) : {} - const rules = Object.entries(cleanRules) - const numberOfRules = rules.length - - const rulesDataMap = new Map() - rules.map(([attribute, value]) => { - const rule = { - attribute, - value, - } - rulesDataMap.set(JSON.stringify(rule), rule) - }) - - return { - ...rest, - rules_count: numberOfRules, - price_rules: Array.from(rulesDataMap.values()), - } - }) - - entry.prices = pricesData + entry.prices = this.normalizePrices( + priceListData.prices, + [] + ) as UpsertPriceDTO[] } if (priceListData.rules) { @@ -994,39 +968,29 @@ export default class PricingModuleService data: PricingTypes.UpdatePriceListPricesDTO[], sharedContext: Context = {} ): Promise { - const priceListIds: string[] = [] - const priceIds: string[] = [] - - for (const priceListData of data) { - priceListIds.push(priceListData.price_list_id) - - for (const price of priceListData.prices) { - priceIds.push(price.id) - } - } - - const prices = await this.listPrices( - { id: priceIds }, - { take: null, relations: ["price_rules"] }, - sharedContext - ) - - const priceMap: Map = new Map( - prices.map((price) => [price.id, price]) - ) - const priceLists = await this.listPriceLists( - { id: priceListIds }, - { take: null }, + { id: data.map((p) => p.price_list_id) }, + { take: null, relations: ["prices", "prices.price_rules"] }, sharedContext ) + const existingPrices = priceLists + .map((p) => p.prices ?? []) + .flat() as PricingTypes.PriceDTO[] + + const pricesToUpsert = data + .map((addPrice) => + this.normalizePrices( + addPrice.prices as UpsertPriceDTO[], + existingPrices, + addPrice.price_list_id + ) + ) + .filter(Boolean) + .flat() as UpsertPriceDTO[] + const priceListMap = new Map(priceLists.map((p) => [p.id, p])) - const pricesToUpdate: Partial[] = [] - const priceRuleIdsToDelete: string[] = [] - const priceRulesToCreate: CreatePriceRuleDTO[] = [] - for (const { price_list_id: priceListId, prices } of data) { const priceList = priceListMap.get(priceListId) @@ -1036,38 +1000,15 @@ export default class PricingModuleService `Price list with id: ${priceListId} not found` ) } - - for (const priceData of prices) { - const { rules = {}, price_set_id, ...rest } = priceData - const price = priceMap.get(rest.id)! - const priceRules = price.price_rules! - - priceRulesToCreate.push( - ...Object.entries(rules).map(([ruleAttribute, ruleValue]) => ({ - price_set_id, - attribute: ruleAttribute, - value: ruleValue, - price_id: price.id, - })) - ) - - pricesToUpdate.push({ - ...rest, - rules_count: Object.keys(rules).length, - } as unknown as Price) - - priceRuleIdsToDelete.push(...priceRules.map((pr) => pr.id)) - } } - const [_deletedPriceRule, _createdPriceRule, updatedPrices] = - await promiseAll([ - this.priceRuleService_.delete(priceRuleIdsToDelete), - this.priceRuleService_.create(priceRulesToCreate), - this.priceService_.update(pricesToUpdate), - ]) + const { entities } = await this.priceService_.upsertWithReplace( + pricesToUpsert, + { relations: ["price_rules"] }, + sharedContext + ) - return updatedPrices + return entities } @InjectTransactionManager("baseRepository_") @@ -1083,97 +1024,73 @@ export default class PricingModuleService data: PricingTypes.AddPriceListPricesDTO[], sharedContext: Context = {} ): Promise { - const priceListIds: string[] = [] - - for (const priceListData of data) { - priceListIds.push(priceListData.price_list_id) - } - const priceLists = await this.listPriceLists( - { id: priceListIds }, - {}, + { id: data.map((p) => p.price_list_id) }, + { take: null, relations: ["prices", "prices.price_rules"] }, sharedContext ) + const existingPrices = priceLists + .map((p) => p.prices ?? []) + .flat() as PricingTypes.PriceDTO[] + + const pricesToUpsert = data + .map((addPrice) => + this.normalizePrices( + addPrice.prices, + existingPrices, + addPrice.price_list_id + ) + ) + .filter(Boolean) + .flat() as UpsertPriceDTO[] + const priceListMap = new Map(priceLists.map((p) => [p.id, p])) - - const pricesToCreate: Partial[] = [] - - for (const { price_list_id: priceListId, prices } of data) { - const priceList = priceListMap.get(priceListId) + pricesToUpsert.forEach((price) => { + const priceList = priceListMap.get(price.price_list_id!) if (!priceList) { throw new MedusaError( MedusaError.Types.INVALID_DATA, - `Price list with id: ${priceListId} not found` + `Price list with id: ${price.price_list_id} not found` ) } + }) - const priceListPricesToCreate = prices.map((priceData) => { - const priceRules = priceData.rules || {} - const noOfRules = Object.keys(priceRules).length - - const priceRulesToCreate = Object.entries(priceRules).map( - ([ruleAttribute, ruleValue]) => { - return { - price_list_id: priceData.price_set_id, - attribute: ruleAttribute, - value: ruleValue, - } - } - ) - - return { - ...priceData, - price_set_id: priceData.price_set_id, - title: "test", - price_list_id: priceList.id, - rules_count: noOfRules, - price_rules: priceRulesToCreate, - } as unknown as Price - }) - - pricesToCreate.push(...priceListPricesToCreate) - } - - const createdPrices = await this.priceService_.create( - pricesToCreate, - sharedContext - ) - - const eventsData = createdPrices.reduce( - (eventsData, price) => { - eventsData.prices.push({ - id: price.id, - }) - - price.price_rules.map((priceRule) => { - eventsData.priceRules.push({ - id: priceRule.id, - }) - }) - - return eventsData - }, - { - priceRules: [], - prices: [], - } as { - priceRules: { id: string }[] - prices: { id: string }[] - } - ) + const { entities, performedActions } = + await this.priceService_.upsertWithReplace( + pricesToUpsert, + { relations: ["price_rules"] }, + sharedContext + ) eventBuilders.createdPrice({ - data: eventsData.prices, + data: performedActions.created[Price.name] ?? [], sharedContext, }) - eventBuilders.createdPriceRule({ - data: eventsData.priceRules, + eventBuilders.updatedPrice({ + data: performedActions.updated[Price.name] ?? [], + sharedContext, + }) + eventBuilders.deletedPrice({ + data: performedActions.deleted[Price.name] ?? [], sharedContext, }) - return createdPrices + eventBuilders.createdPriceRule({ + data: performedActions.created[PriceRule.name] ?? [], + sharedContext, + }) + eventBuilders.updatedPriceRule({ + data: performedActions.updated[PriceRule.name] ?? [], + sharedContext, + }) + eventBuilders.deletedPriceRule({ + data: performedActions.deleted[PriceRule.name] ?? [], + sharedContext, + }) + + return entities } @InjectTransactionManager("baseRepository_") @@ -1335,3 +1252,24 @@ export default class PricingModuleService } } } + +const hashPrice = ( + price: PricingTypes.PriceDTO | PricingTypes.CreatePricesDTO +): string => { + const data = Object.entries({ + currency_code: price.currency_code, + price_set_id: "price_set_id" in price ? price.price_set_id ?? null : null, + price_list_id: + "price_list_id" in price ? price.price_list_id ?? null : null, + min_quantity: price.min_quantity ? price.min_quantity.toString() : null, + max_quantity: price.max_quantity ? price.max_quantity.toString() : null, + ...("price_rules" in price + ? price.price_rules?.reduce((agg, pr) => { + agg[pr.attribute] = pr.value + return agg + }, {}) + : {}), + }).sort(([a], [b]) => a.localeCompare(b)) + + return simpleHash(JSON.stringify(data)) +} diff --git a/packages/modules/pricing/src/types/services/index.ts b/packages/modules/pricing/src/types/services/index.ts index 0834669391..b32edafa95 100644 --- a/packages/modules/pricing/src/types/services/index.ts +++ b/packages/modules/pricing/src/types/services/index.ts @@ -1,2 +1,3 @@ export * from "./price-list" export * from "./price-set" +export * from "./price" diff --git a/packages/modules/pricing/src/types/services/price.ts b/packages/modules/pricing/src/types/services/price.ts new file mode 100644 index 0000000000..5db82435c0 --- /dev/null +++ b/packages/modules/pricing/src/types/services/price.ts @@ -0,0 +1,8 @@ +import { PricingTypes } from "@medusajs/types" + +export interface UpsertPriceDTO + extends Omit { + id?: string + price_list_id?: string + price_rules: PricingTypes.CreatePriceRuleDTO[] +} diff --git a/packages/modules/pricing/src/utils/events.ts b/packages/modules/pricing/src/utils/events.ts index 76389b159b..9856220fbd 100644 --- a/packages/modules/pricing/src/utils/events.ts +++ b/packages/modules/pricing/src/utils/events.ts @@ -42,4 +42,28 @@ export const eventBuilders = { object: "price_list_rule", eventsEnum: PricingEvents, }), + updatedPrice: eventBuilderFactory({ + source: Modules.PRICING, + action: CommonEvents.UPDATED, + object: "price", + eventsEnum: PricingEvents, + }), + updatedPriceRule: eventBuilderFactory({ + source: Modules.PRICING, + action: CommonEvents.UPDATED, + object: "price_rule", + eventsEnum: PricingEvents, + }), + deletedPrice: eventBuilderFactory({ + source: Modules.PRICING, + action: CommonEvents.DELETED, + object: "price", + eventsEnum: PricingEvents, + }), + deletedPriceRule: eventBuilderFactory({ + source: Modules.PRICING, + action: CommonEvents.DELETED, + object: "price_rule", + eventsEnum: PricingEvents, + }), }