From c9989529ed8c347fefe10d57b3d23fa60aa35fe7 Mon Sep 17 00:00:00 2001 From: Philip Korsholm <88927411+pKorsholm@users.noreply.github.com> Date: Mon, 24 Jul 2023 11:10:17 +0200 Subject: [PATCH] fix(medusa): Price selection strategy bug with customer groups without customers (#4578) * change up condition for joining price lists * add changeset * naming * update tests --- .changeset/new-items-warn.md | 5 + .../api/__tests__/admin/price-list.js | 8 +- .../api/__tests__/admin/variant.js | 6 + .../api/__tests__/price-selection/index.js | 59 ++++++++++ .../helpers/price-selection-seeder.js | 108 +++++++++++++++++- .../src/api/middlewares/error-handler.ts | 3 +- .../medusa/src/repositories/money-amount.ts | 20 ++-- .../medusa/src/strategies/price-selection.ts | 13 ++- 8 files changed, 200 insertions(+), 22 deletions(-) create mode 100644 .changeset/new-items-warn.md diff --git a/.changeset/new-items-warn.md b/.changeset/new-items-warn.md new file mode 100644 index 0000000000..d63512df50 --- /dev/null +++ b/.changeset/new-items-warn.md @@ -0,0 +1,5 @@ +--- +"@medusajs/medusa": patch +--- + +fix(medusa): update how price lists are filtered in price selection strategy diff --git a/integration-tests/api/__tests__/admin/price-list.js b/integration-tests/api/__tests__/admin/price-list.js index bf763f33dd..e8087774c5 100644 --- a/integration-tests/api/__tests__/admin/price-list.js +++ b/integration-tests/api/__tests__/admin/price-list.js @@ -1299,9 +1299,11 @@ describe("/admin/price-lists", () => { expect(response.status).toBe(200) expect(response.data).toEqual({ - ids: product1.variants.map((variant, i) => { - return getCustomPriceIdFromVariant(variant.id, i) - }), + ids: expect.arrayContaining( + product1.variants.map((variant, i) => { + return getCustomPriceIdFromVariant(variant.id, i) + }) + ), object: "money-amount", deleted: true, }) diff --git a/integration-tests/api/__tests__/admin/variant.js b/integration-tests/api/__tests__/admin/variant.js index b82e045b3c..aba64158f4 100644 --- a/integration-tests/api/__tests__/admin/variant.js +++ b/integration-tests/api/__tests__/admin/variant.js @@ -262,6 +262,12 @@ describe("/admin/products", () => { product_id: productData.id, sku: `test-product_filtering_by_variant_id-${i}`, title: `test-product_filtering_by_variant_id-${i}`, + options: [ + { + option_id: "test-product_filtering_by_variant_id-option", + value: `Large-${i}`, + }, + ], }) } diff --git a/integration-tests/api/__tests__/price-selection/index.js b/integration-tests/api/__tests__/price-selection/index.js index 47e318b500..f4bcdf6ec9 100644 --- a/integration-tests/api/__tests__/price-selection/index.js +++ b/integration-tests/api/__tests__/price-selection/index.js @@ -6,6 +6,16 @@ const { useDb, initDb } = require("../../../environment-helpers/use-db") const adminSeeder = require("../../../helpers/admin-seeder") const promotionsSeeder = require("../../../helpers/price-selection-seeder") +const { + Product, + ShippingProfileType, + ShippingProfile, + ProductOption, + CustomerGroup, + PriceList, + ProductVariant, + Customer, +} = require("@medusajs/medusa") jest.setTimeout(30000) @@ -449,6 +459,55 @@ describe("Promotions", () => { ) }) + it("should only join price lists with customer groups for the customer, not empty groups", async () => { + const api = useApi() + + const authResponse = await api.post("/store/auth", { + email: "test5@email-pl.com", + password: "test", + }) + + const [authCookie] = authResponse.headers["set-cookie"][0].split(";") + + const res = await api + .get("/store/products/test-product-pl?cart_id=test-cart", { + headers: { + Cookie: authCookie, + }, + }) + .catch((error) => console.log(error)) + + const variant = res.data.product.variants[0] + + expect(variant.prices.length).toEqual(2) + + expect(variant).toEqual( + expect.objectContaining({ + id: "test-variant-pl", + calculated_price: 110, + }) + ) + + expect(variant.prices.length).toEqual(2) + expect(variant.prices).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: "test-price1120", + region_id: "test-region", + currency_code: "usd", + amount: 120, + }), + expect.objectContaining({ + id: "test-price2120", + region_id: "test-region", + currency_code: "usd", + price_list_id: "pl_current_pl", + amount: 110, + }), + ]) + ) + }) + it("fetches product with groups and quantities in money amounts with login", async () => { const api = useApi() diff --git a/integration-tests/helpers/price-selection-seeder.js b/integration-tests/helpers/price-selection-seeder.js index 6d3f7d2599..03f6564335 100644 --- a/integration-tests/helpers/price-selection-seeder.js +++ b/integration-tests/helpers/price-selection-seeder.js @@ -278,7 +278,7 @@ module.exports = async (dataSource, data = {}) => { id: "test-price2", region_id: "test-region", currency_code: "usd", - amount: 130, + amount: 90, price_list_id: "pl_2", }, { @@ -881,4 +881,110 @@ module.exports = async (dataSource, data = {}) => { }) await manager.save(cart_region1) + + const customerPl = await manager.create(Customer, { + id: "test-customer-5-pl", + email: "test5@email-pl.com", + first_name: "John", + last_name: "Deere", + password_hash: + "c2NyeXB0AAEAAAABAAAAAVMdaddoGjwU1TafDLLlBKnOTQga7P2dbrfgf3fB+rCD/cJOMuGzAvRdKutbYkVpuJWTU39P7OpuWNkUVoEETOVLMJafbI8qs8Qx/7jMQXkN", // password matching "test" + has_account: true, + }) + await manager.save(customerPl) + + const pPl = await manager.create(Product, { + id: "test-product-pl", + handle: "test-product-pl", + title: "Test product", + profile_id: defaultProfile.id, + description: "test-product-description1", + collection_id: "test-collection", + tags: [], + }) + + await manager.save(pPl) + + await manager.save(ProductOption, { + id: "test-option-pl", + title: "test-option", + product_id: "test-product-pl", + }) + + const c_group_pl = await manager.create(CustomerGroup, { + id: "test-group-pl", + name: "test-group-pl", + }) + await manager.save(c_group_pl) + + customerPl.groups = [c_group_pl] + await manager.save(customerPl) + + const c_group_not_pl = await manager.create(CustomerGroup, { + id: "test-group-not-pl", + name: "test-group-not-pl", + }) + await manager.save(c_group_not_pl) + + const priceListPl = await manager.create(PriceList, { + id: "pl_current_pl", + name: "Past winter sale", + description: "Winter sale for key accounts.", + type: "sale", + status: "active", + }) + + priceListPl.customer_groups = [c_group_pl] + await manager.save(priceListPl) + + const priceListNotPL = await manager.create(PriceList, { + id: "pl_current_not_pl", + name: "Past winter sale", + description: "Winter sale for key accounts.", + type: "sale", + status: "active", + }) + priceListNotPL.customer_groups = [c_group_not_pl] + + await manager.save(priceListNotPL) + + const variantPl = await manager.create(ProductVariant, { + id: "test-variant-pl", + inventory_quantity: 10, + title: "Test variant", + sku: "test-sku-pl", + status: "published", + product_id: "test-product-pl", + prices: [ + { + id: "test-price120", + region_id: "test-region", + currency_code: "usd", + amount: 90, + price_list_id: "pl_current_not_pl", + }, + { + id: "test-price1120", + region_id: "test-region", + currency_code: "usd", + amount: 120, + }, + { + id: "test-price2120", + region_id: "test-region", + currency_code: "usd", + amount: 110, + price_list_id: "pl_current_pl", + }, + ], + options: [ + { + id: "test-variant-option-pl", + value: "Default variant", + option_id: "test-option-pl", + }, + ], + }) + + await manager.save(variantPl) } diff --git a/packages/medusa/src/api/middlewares/error-handler.ts b/packages/medusa/src/api/middlewares/error-handler.ts index ccb2727e02..ed2d7e5242 100644 --- a/packages/medusa/src/api/middlewares/error-handler.ts +++ b/packages/medusa/src/api/middlewares/error-handler.ts @@ -1,6 +1,7 @@ import { NextFunction, Request, Response } from "express" -import { MedusaError } from "medusa-core-utils" + import { Logger } from "../../types/global" +import { MedusaError } from "medusa-core-utils" import { formatException } from "../../utils" const QUERY_RUNNER_RELEASED = "QueryRunnerAlreadyReleasedError" diff --git a/packages/medusa/src/repositories/money-amount.ts b/packages/medusa/src/repositories/money-amount.ts index 94a8f64329..b7b85787c3 100644 --- a/packages/medusa/src/repositories/money-amount.ts +++ b/packages/medusa/src/repositories/money-amount.ts @@ -1,4 +1,3 @@ -import partition from "lodash/partition" import { Brackets, In, @@ -7,16 +6,18 @@ import { ObjectLiteral, WhereExpressionBuilder, } from "typeorm" -import { QueryDeepPartialEntity } from "typeorm/query-builder/QueryPartialEntity" -import { dataSource } from "../loaders/database" -import { MoneyAmount } from "../models" import { PriceListPriceCreateInput, PriceListPriceUpdateInput, } from "../types/price-list" + +import { MoneyAmount } from "../models" import { ProductVariantPrice } from "../types/product-variant" -import { isString } from "../utils" +import { QueryDeepPartialEntity } from "typeorm/query-builder/QueryPartialEntity" +import { dataSource } from "../loaders/database" import { groupBy } from "lodash" +import { isString } from "../utils" +import partition from "lodash/partition" type Price = Partial< Omit @@ -316,12 +317,9 @@ export const MoneyAmountRepository = dataSource "cgc", "cgc.customer_group_id = cgroup.id" ) - .andWhere( - "(cgc.customer_group_id is null OR cgc.customer_id = :customer_id)", - { - customer_id, - } - ) + .andWhere("(cgroup.id is null OR cgc.customer_id = :customer_id)", { + customer_id, + }) } else { qb.leftJoin("price_list.customer_groups", "cgroup").andWhere( "cgroup.id is null" diff --git a/packages/medusa/src/strategies/price-selection.ts b/packages/medusa/src/strategies/price-selection.ts index 853ba2cae1..dcbb2471c1 100644 --- a/packages/medusa/src/strategies/price-selection.ts +++ b/packages/medusa/src/strategies/price-selection.ts @@ -1,16 +1,17 @@ -import { ICacheService } from "@medusajs/types" -import { isDefined } from "medusa-core-utils" -import { EntityManager } from "typeorm" import { AbstractPriceSelectionStrategy, PriceSelectionContext, PriceSelectionResult, PriceType, } from "../interfaces" -import TaxInclusivePricingFeatureFlag from "../loaders/feature-flags/tax-inclusive-pricing" -import { MoneyAmountRepository } from "../repositories/money-amount" -import { TaxServiceRate } from "../types/tax-service" + +import { EntityManager } from "typeorm" import { FlagRouter } from "../utils/flag-router" +import { ICacheService } from "@medusajs/types" +import { MoneyAmountRepository } from "../repositories/money-amount" +import TaxInclusivePricingFeatureFlag from "../loaders/feature-flags/tax-inclusive-pricing" +import { TaxServiceRate } from "../types/tax-service" +import { isDefined } from "medusa-core-utils" class PriceSelectionStrategy extends AbstractPriceSelectionStrategy { protected manager_: EntityManager