Feat: Price selection implementation (#1158)

* init

* added buld id validation to repo

* admin done

* updated price reqs

* initial price selection strategy

* update customer seeder

* format models

* price selection strategy

* price selection testing

* update price selection tests

* update price selection strategy

* remove console.warn

* update price selection strat

* remove console.log

* fix unit tests

* update product snapshot integration tests

* fix failing unit tests

* update variant test snapshots

* intial implementation of PriceList

* integration tests for price lists

* updated admin/product integration tests

* update updateVariantPrices method

* remove comment from error handler

* add integration test for batch deleting prices associated with price list

* make update to prices through variant service limited to default prices

* update store/products.js snapshot

* add api unit tests and update product integration tests to validate that prices from Price List are ignored

* fix product test

* requested changes

* cascade

* ensure delete variant cascades to MoneyAmount

* addresses PR feedback

* removed unused endpoint

* update mock

* fix failing store integration tests

* remove medusajs ressource

* re add env.template

* price selection strategy methods

* fix integration tests

* update unit tests

* update jsdoc

* update price selection strategy parameter

* fix unit tests

* pr feedback

Co-authored-by: Kasper <kasper@medusajs.com>
Co-authored-by: Kasper Fabricius Kristensen <45367945+kasperkristensen@users.noreply.github.com>
This commit is contained in:
Philip Korsholm
2022-03-21 19:03:42 +01:00
committed by GitHub
parent 5300926db8
commit dfa3502e41
10 changed files with 701 additions and 58 deletions

View File

@@ -467,7 +467,6 @@ describe("/admin/price-lists", () => {
min_quantity: 1,
max_quantity: 100,
variant_id: "test-variant",
price_list_id: "pl_no_customer_groups",
created_at: expect.any(String),
updated_at: expect.any(String),
},
@@ -479,7 +478,6 @@ describe("/admin/price-lists", () => {
min_quantity: 101,
max_quantity: 500,
variant_id: "test-variant",
price_list_id: "pl_no_customer_groups",
created_at: expect.any(String),
updated_at: expect.any(String),
},
@@ -491,7 +489,6 @@ describe("/admin/price-lists", () => {
min_quantity: 501,
max_quantity: 1000,
variant_id: "test-variant",
price_list_id: "pl_no_customer_groups",
created_at: expect.any(String),
updated_at: expect.any(String),
},
@@ -674,7 +671,7 @@ describe("/admin/price-lists", () => {
it("Deletes a variant and ensures that prices associated with the variant are deleted from PriceList", async () => {
const api = useApi()
const deleteResponse = await api
await api
.delete("/admin/products/test-product/variants/test-variant", {
headers: {
Authorization: "Bearer test_token",
@@ -684,7 +681,6 @@ describe("/admin/price-lists", () => {
console.warn(err.response.data)
})
const response = await api.get(
"/admin/price-lists/pl_no_customer_groups",
{
@@ -694,7 +690,6 @@ describe("/admin/price-lists", () => {
}
)
expect(response.status).toEqual(200)
expect(response.data.price_list.prices.length).toEqual(0)
})

View File

@@ -202,9 +202,6 @@ describe("/store/variants", () => {
},
],
product: expect.any(Object),
options: [
{ created_at: expect.any(String), updated_at: expect.any(String) },
],
},
})
})

View File

@@ -344,11 +344,9 @@ describe("/store/products", () => {
],
prices: [
{
id: "test-money-amount",
created_at: expect.any(String),
updated_at: expect.any(String),
amount: 100,
created_at: expect.any(String),
currency_code: "usd",
deleted_at: null,
id: "test-price",
@@ -356,7 +354,6 @@ describe("/store/products", () => {
min_quantity: null,
max_quantity: null,
price_list_id: null,
updated_at: expect.any(String),
variant_id: "test-variant",
},
],
@@ -389,11 +386,9 @@ describe("/store/products", () => {
],
prices: [
{
id: "test-money-amount",
created_at: expect.any(String),
updated_at: expect.any(String),
amount: 100,
created_at: expect.any(String),
currency_code: "usd",
deleted_at: null,
id: "test-price2",
@@ -433,11 +428,9 @@ describe("/store/products", () => {
],
prices: [
{
id: "test-money-amount",
created_at: expect.any(String),
updated_at: expect.any(String),
amount: 100,
created_at: expect.any(String),
currency_code: "usd",
deleted_at: null,
id: "test-price1",
@@ -445,7 +438,6 @@ describe("/store/products", () => {
min_quantity: null,
max_quantity: null,
price_list_id: null,
updated_at: expect.any(String),
variant_id: "test-variant_1",
},
],
@@ -548,7 +540,7 @@ describe("/store/products", () => {
it("lists all published products", async () => {
const api = useApi()
//update test-product status to published
// update test-product status to published
await api
.post(
"/admin/products/test-product",

View File

@@ -40,16 +40,19 @@ module.exports = async (connection, data = {}) => {
id: "test-customer-5",
email: "test5@email.com",
})
await manager.save(customer5)
const customer6 = await manager.create(Customer, {
id: "test-customer-6",
email: "test6@email.com",
})
await manager.save(customer6)
const customer7 = await manager.create(Customer, {
id: "test-customer-7",
email: "test7@email.com",
})
await manager.save(customer7)
const deletionCustomer = await manager.create(Customer, {
id: "test-customer-delete-cg",

View File

@@ -0,0 +1,74 @@
import { EntityManager } from "typeorm"
import { MoneyAmount } from ".."
import { MoneyAmountRepository } from "../repositories/money-amount"
import { PriceListType } from "../types/price-list"
export interface IPriceSelectionStrategy {
/**
* Instantiate a new price selection strategy with the active transaction in
* order to ensure reads are accurate.
* @param manager EntityManager with the queryrunner of the active transaction
* @returns a new price selection strategy
*/
withTransaction(manager: EntityManager): IPriceSelectionStrategy
/**
* Calculate the original and discount price for a given variant in a set of
* circumstances described in the context.
* @param variant The variant id of the variant for which to retrieve prices
* @param context Details relevant to determine the correct pricing of the variant
* @return pricing details in an object containing the calculated lowest price,
* the default price an all valid prices for the given variant
*/
calculateVariantPrice(
variant_id: string,
context: PriceSelectionContext
): Promise<PriceSelectionResult>
}
export abstract class AbstractPriceSelectionStrategy
implements IPriceSelectionStrategy
{
public abstract withTransaction(
manager: EntityManager
): IPriceSelectionStrategy
public abstract calculateVariantPrice(
variant_id: string,
context: PriceSelectionContext
): Promise<PriceSelectionResult>
}
export function isPriceSelectionStrategy(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
object: any
): object is IPriceSelectionStrategy {
return (
typeof object.calculateVariantPrice === "function" &&
typeof object.withTransaction === "function"
)
}
export type PriceSelectionContext = {
cart_id?: string
customer_id?: string
quantity?: number
region_id?: string
currency_code?: string
include_discount_prices?: boolean
}
enum DefaultPriceType {
DEFAULT = "default",
}
// both exports are needed in order to get proper typing of the calculatedPriceType field.
export type PriceType = DefaultPriceType | PriceListType
export const PriceType = { ...DefaultPriceType, ...PriceListType }
export type PriceSelectionResult = {
originalPrice: number | null
calculatedPrice: number | null
calculatedPriceType?: PriceType
prices: MoneyAmount[] // prices is an array of all possible price for the input customer and region prices
}

View File

@@ -7,7 +7,7 @@ import {
Index,
ManyToMany,
PrimaryColumn,
UpdateDateColumn
UpdateDateColumn,
} from "typeorm"
import { ulid } from "ulid"
import { DbAwareColumn, resolveDbType } from "../utils/db-aware-column"
@@ -23,20 +23,14 @@ export class CustomerGroup {
@Column()
name: string
@ManyToMany(
() => Customer,
(customer) => customer.groups,
{
onDelete: "CASCADE",
}
)
@ManyToMany(() => Customer, (customer) => customer.groups, {
onDelete: "CASCADE",
})
customers: Customer[]
@ManyToMany(
() => PriceList,
(priceList) => priceList.customer_groups,
{ onDelete: "CASCADE" }
)
@ManyToMany(() => PriceList, (priceList) => priceList.customer_groups, {
onDelete: "CASCADE",
})
price_lists: PriceList[]
@CreateDateColumn({ type: resolveDbType("timestamptz") })

View File

@@ -32,19 +32,14 @@ export class ProductVariant {
@Column()
product_id: string
@ManyToOne(
() => Product,
product => product.variants,
{ eager: true }
)
@ManyToOne(() => Product, (product) => product.variants, { eager: true })
@JoinColumn({ name: "product_id" })
product: Product
@OneToMany(
() => MoneyAmount,
ma => ma.variant,
{ cascade: true, onDelete: "CASCADE" }
)
@OneToMany(() => MoneyAmount, (ma) => ma.variant, {
cascade: true,
onDelete: "CASCADE",
})
prices: MoneyAmount[]
@Column({ nullable: true })
@@ -99,11 +94,9 @@ export class ProductVariant {
@Column({ type: "int", nullable: true })
width: number
@OneToMany(
() => ProductOptionValue,
optionValue => optionValue.variant,
{ cascade: true }
)
@OneToMany(() => ProductOptionValue, (optionValue) => optionValue.variant, {
cascade: true,
})
options: ProductOptionValue[]
@CreateDateColumn({ type: resolveDbType("timestamptz") })
@@ -120,7 +113,9 @@ export class ProductVariant {
@BeforeInsert()
private beforeInsert() {
if (this.id) return
if (this.id) {
return
}
const id = ulid()
this.id = `variant_${id}`
}

View File

@@ -5,10 +5,13 @@ import {
In,
IsNull,
Not,
Repository
Repository,
} from "typeorm"
import { MoneyAmount } from "../models/money-amount"
import { PriceListPriceCreateInput, PriceListPriceUpdateInput } from "../types/price-list"
import {
PriceListPriceCreateInput,
PriceListPriceUpdateInput,
} from "../types/price-list"
type Price = Partial<
Omit<MoneyAmount, "created_at" | "updated_at" | "deleted_at">
@@ -18,7 +21,10 @@ type Price = Partial<
@EntityRepository(MoneyAmount)
export class MoneyAmountRepository extends Repository<MoneyAmount> {
public async findVariantPricesNotIn(variantId: string, prices: Price[]) {
public async findVariantPricesNotIn(
variantId: string,
prices: Price[]
): Promise<MoneyAmount[]> {
const pricesNotInPricesPayload = await this.createQueryBuilder()
.where({
variant_id: variantId,
@@ -35,7 +41,10 @@ export class MoneyAmountRepository extends Repository<MoneyAmount> {
return pricesNotInPricesPayload
}
public async upsertVariantCurrencyPrice(variantId: string, price: Price) {
public async upsertVariantCurrencyPrice(
variantId: string,
price: Price
): Promise<MoneyAmount> {
let moneyAmount = await this.findOne({
where: {
currency_code: price.currency_code,
@@ -61,12 +70,15 @@ export class MoneyAmountRepository extends Repository<MoneyAmount> {
public async addPriceListPrices(
priceListId: string,
prices: PriceListPriceCreateInput[],
overrideExisting: boolean = false
overrideExisting = false
): Promise<MoneyAmount[]> {
const toInsert = prices.map((price) => (this.create({
...price,
price_list_id: priceListId,
})))
const toInsert = prices.map((price) =>
this.create({
...price,
price_list_id: priceListId,
})
)
const insertResult = await this.createQueryBuilder()
.insert()
.orIgnore(true)
@@ -103,13 +115,78 @@ export class MoneyAmountRepository extends Repository<MoneyAmount> {
.execute()
}
public async findManyForVariantInRegion(
variant_id: string,
region_id?: string,
currency_code?: string,
customer_id?: string,
include_discount_prices?: boolean
): Promise<[MoneyAmount[], number]> {
const date = new Date()
const qb = this.createQueryBuilder("ma")
.leftJoinAndSelect(
"ma.price_list",
"price_list",
"ma.price_list_id = price_list.id "
)
.where({ variant_id: variant_id }) // "ma.variant_id = :variant_id",
.andWhere("(ma.price_list_id is null or price_list.status = 'active')")
.andWhere(
"(price_list is null or price_list.ends_at is null OR price_list.ends_at > :date) ",
{
date: date.toUTCString(),
}
)
.andWhere(
"(price_list is null or price_list.starts_at is null OR price_list.starts_at < :date)",
{
date: date.toUTCString(),
}
)
if (region_id || currency_code) {
qb.andWhere(
new Brackets((qb) =>
qb
.where({ region_id: region_id })
.orWhere({ currency_code: currency_code })
)
)
} else if (!customer_id && !include_discount_prices) {
qb.andWhere("price_list IS null")
}
if (customer_id) {
qb.leftJoin("price_list.customer_groups", "cgroup")
.leftJoin(
"customer_group_customers",
"cgc",
"cgc.customer_group_id = cgroup.id"
)
.andWhere("(cgc is null OR cgc.customer_id = :customer_id)", {
customer_id,
})
} else {
qb.leftJoin("price_list.customer_groups", "cgroup").andWhere(
"cgroup.id is null"
)
}
return await qb.getManyAndCount()
}
public async updatePriceListPrices(
priceListId: string,
updates: PriceListPriceUpdateInput[]
): Promise<MoneyAmount[]> {
const [existingPrices, newPrices] = partition(updates, (update) => update.id)
const [existingPrices, newPrices] = partition(
updates,
(update) => update.id
)
const newPriceEntities = newPrices.map((price) => (this.create({ ...price, price_list_id: priceListId })))
const newPriceEntities = newPrices.map((price) =>
this.create({ ...price, price_list_id: priceListId })
)
return await this.save([...existingPrices, ...newPriceEntities])
}

View File

@@ -0,0 +1,398 @@
import PriceSelectionStrategy from "../price-selection"
const toTest = [
[
"Variant with only default price",
{
variant_id: "test-basic-variant",
context: {
region_id: "test-region",
currency_code: "dkk",
},
validate: (value, { mockMoneyAmountRepository }) => {
expect(
mockMoneyAmountRepository.findManyForVariantInRegion
).toHaveBeenCalledWith(
"test-basic-variant",
"test-region",
"dkk",
undefined,
undefined
)
expect(value).toEqual({
originalPrice: 100,
calculatedPrice: 100,
calculatedPriceType: "default",
prices: [
{
amount: 100,
region_id: "test-region",
currency_code: "dkk",
max_quantity: null,
min_quantity: null,
price_list_id: null,
},
],
})
},
},
],
[
"Throws correct error if no default price is found, missing variant",
{
variant_id: "non-existing-variant",
context: {
region_id: "test-region",
currency_code: "dkk",
},
validate: (value, { mockMoneyAmountRepository }) => {},
validateException: (error, { mockMoneyAmountRepository }) => {
expect(error.type).toEqual("not_found")
expect(error.message).toEqual(
"Money amount for variant with id non-existing-variant in region test-region does not exist"
)
},
},
],
[
"findManyForVariantInRegion is invoked with the correct customer",
{
variant_id: "test-variant",
context: {
region_id: "test-region",
currency_code: "dkk",
customer_id: "test-customer-1",
},
validate: (value, { mockMoneyAmountRepository }) => {
expect(
mockMoneyAmountRepository.findManyForVariantInRegion
).toHaveBeenCalledWith(
"test-variant",
"test-region",
"dkk",
"test-customer-1",
undefined
)
},
},
],
[
"Lowest valid price is returned",
{
variant_id: "test-variant",
context: {
region_id: "test-region",
currency_code: "dkk",
customer_id: "test-customer-1",
},
validate: (value, { mockMoneyAmountRepository }) => {
expect(value).toEqual({
originalPrice: 100,
calculatedPrice: 50,
calculatedPriceType: "sale",
prices: [
{
amount: 100,
region_id: "test-region",
currency_code: "dkk",
max_quantity: null,
min_quantity: null,
price_list_id: null,
},
{
amount: 50,
region_id: "test-region",
currency_code: "dkk",
price_list: { type: "sale" },
max_quantity: null,
min_quantity: null,
},
],
})
},
},
],
[
"Prices with quantity limits are ignored with no provided quantity",
{
variant_id: "test-variant",
context: {
region_id: "test-region",
currency_code: "dkk",
customer_id: "test-customer-2",
},
validate: (value, { mockMoneyAmountRepository }) => {
expect(value).toEqual({
originalPrice: 100,
calculatedPrice: 100,
calculatedPriceType: "default",
prices: [
{
amount: 100,
region_id: "test-region",
currency_code: "dkk",
max_quantity: null,
min_quantity: null,
price_list_id: null,
},
{
amount: 30,
min_quantity: 10,
max_quantity: 12,
region_id: "test-region",
price_list: { type: "sale" },
currency_code: "dkk",
},
{
amount: 20,
min_quantity: 3,
max_quantity: 5,
price_list: { type: "sale" },
region_id: "test-region",
currency_code: "dkk",
},
{
amount: 50,
min_quantity: 5,
max_quantity: 10,
price_list: { type: "sale" },
region_id: "test-region",
currency_code: "dkk",
},
],
})
},
},
],
[
"Prices With quantity limits are applied correctly when a quantity is provided",
{
variant_id: "test-variant",
context: {
region_id: "test-region",
currency_code: "dkk",
customer_id: "test-customer-2",
quantity: 7,
},
validate: (value, { mockMoneyAmountRepository }) => {
expect(value).toEqual({
originalPrice: 100,
calculatedPrice: 50,
calculatedPriceType: "sale",
prices: [
{
amount: 100,
region_id: "test-region",
currency_code: "dkk",
max_quantity: null,
min_quantity: null,
price_list_id: null,
},
{
amount: 30,
min_quantity: 10,
max_quantity: 12,
region_id: "test-region",
price_list: { type: "sale" },
currency_code: "dkk",
},
{
amount: 20,
min_quantity: 3,
max_quantity: 5,
price_list: { type: "sale" },
region_id: "test-region",
currency_code: "dkk",
},
{
amount: 50,
min_quantity: 5,
max_quantity: 10,
price_list: { type: "sale" },
region_id: "test-region",
currency_code: "dkk",
},
],
})
},
},
],
[
"Prices with quantity are in prices array with no quantity set",
{
variant_id: "test-variant",
context: {
region_id: "test-region",
currency_code: "dkk",
customer_id: "test-customer-2",
},
validate: (value, { mockMoneyAmountRepository }) => {
expect(value).toEqual({
originalPrice: 100,
calculatedPrice: 100,
calculatedPriceType: "default",
prices: [
{
amount: 100,
region_id: "test-region",
currency_code: "dkk",
max_quantity: null,
min_quantity: null,
price_list_id: null,
},
{
amount: 30,
min_quantity: 10,
max_quantity: 12,
region_id: "test-region",
price_list: { type: "sale" },
currency_code: "dkk",
},
{
amount: 20,
min_quantity: 3,
max_quantity: 5,
region_id: "test-region",
price_list: { type: "sale" },
currency_code: "dkk",
},
{
amount: 50,
min_quantity: 5,
max_quantity: 10,
region_id: "test-region",
price_list: { type: "sale" },
currency_code: "dkk",
},
],
})
},
},
],
]
describe("PriceSelectionStrategy", () => {
describe("calculateVariantPrice", () => {
test.each(toTest)(
"%s",
async (title, { variant_id, context, validate, validateException }) => {
const mockMoneyAmountRepository = {
findManyForVariantInRegion: jest
.fn()
.mockImplementation(
async (
variant_id,
region_id,
currency_code,
customer_id,
useDiscountPrices
) => {
if (variant_id === "test-basic-variant") {
return [
[
{
amount: 100,
region_id,
currency_code,
price_list_id: null,
max_quantity: null,
min_quantity: null,
},
],
1,
]
}
if (customer_id === "test-customer-1") {
return [
[
{
amount: 100,
region_id,
currency_code,
price_list_id: null,
max_quantity: null,
min_quantity: null,
},
{
amount: 50,
region_id: region_id,
currency_code: currency_code,
price_list: { type: "sale" },
max_quantity: null,
min_quantity: null,
},
],
2,
]
}
if (customer_id === "test-customer-2") {
return [
[
{
amount: 100,
region_id,
currency_code,
price_list_id: null,
max_quantity: null,
min_quantity: null,
},
{
amount: 30,
min_quantity: 10,
max_quantity: 12,
price_list: { type: "sale" },
region_id: region_id,
currency_code: currency_code,
},
{
amount: 20,
min_quantity: 3,
max_quantity: 5,
price_list: { type: "sale" },
region_id: region_id,
currency_code: currency_code,
},
{
amount: 50,
min_quantity: 5,
max_quantity: 10,
price_list: { type: "sale" },
region_id: region_id,
currency_code: currency_code,
},
],
4,
]
}
return []
}
),
}
const mockEntityManager = {
getCustomRepository: (repotype) => mockMoneyAmountRepository,
}
const selectionStrategy = new PriceSelectionStrategy({
manager: mockEntityManager,
moneyAmountRepository: mockMoneyAmountRepository,
})
try {
const val = await selectionStrategy.calculateVariantPrice(
variant_id,
context
)
validate(val, { mockMoneyAmountRepository })
} catch (error) {
if (typeof validateException === "function") {
validateException(error, { mockMoneyAmountRepository })
} else {
throw error
}
}
}
)
})
})

View File

@@ -0,0 +1,118 @@
import {
AbstractPriceSelectionStrategy,
IPriceSelectionStrategy,
PriceSelectionContext,
PriceSelectionResult,
PriceType,
} from "../interfaces/price-selection-strategy"
import { MoneyAmountRepository } from "../repositories/money-amount"
import { EntityManager } from "typeorm"
class PriceSelectionStrategy extends AbstractPriceSelectionStrategy {
private moneyAmountRepository_: typeof MoneyAmountRepository
private manager_: EntityManager
constructor({ manager, moneyAmountRepository }) {
super()
this.manager_ = manager
this.moneyAmountRepository_ = moneyAmountRepository
}
withTransaction(manager: EntityManager): IPriceSelectionStrategy {
if (!manager) {
return this
}
return new PriceSelectionStrategy({
manager: manager,
moneyAmountRepository: this.moneyAmountRepository_,
})
}
async calculateVariantPrice(
variant_id: string,
context: PriceSelectionContext
): Promise<PriceSelectionResult> {
const moneyRepo = this.manager_.getCustomRepository(
this.moneyAmountRepository_
)
const [prices, count] = await moneyRepo.findManyForVariantInRegion(
variant_id,
context.region_id,
context.currency_code,
context.customer_id,
context.include_discount_prices
)
if (!count) {
return {
originalPrice: null,
calculatedPrice: null,
prices: [],
}
}
const result: PriceSelectionResult = {
originalPrice: null,
calculatedPrice: null,
prices,
}
if (!context) {
return result
}
for (const ma of prices) {
if (
context.region_id &&
ma.region_id === context.region_id &&
ma.price_list_id === null &&
ma.min_quantity === null &&
ma.max_quantity === null
) {
result.originalPrice = ma.amount
}
if (
context.currency_code &&
ma.currency_code === context.currency_code &&
ma.price_list_id === null &&
ma.min_quantity === null &&
ma.max_quantity === null &&
result.originalPrice === null // region prices take precedence
) {
result.originalPrice = ma.amount
}
if (
isValidQuantity(ma, context.quantity) &&
(result.calculatedPrice === null ||
ma.amount < result.calculatedPrice) &&
((context.currency_code &&
ma.currency_code === context.currency_code) ||
(context.region_id && ma.region_id === context.region_id))
) {
result.calculatedPrice = ma.amount
result.calculatedPriceType = ma.price_list?.type || PriceType.DEFAULT
}
}
return result
}
}
const isValidQuantity = (price, quantity): boolean =>
(typeof quantity !== "undefined" &&
isValidPriceWithQuantity(price, quantity)) ||
(typeof quantity === "undefined" && isValidPriceWithoutQuantity(price))
const isValidPriceWithoutQuantity = (price): boolean =>
(!price.max_quantity && !price.min_quantity) ||
((!price.min_quantity || price.min_quantity === 0) && price.max_quantity)
const isValidPriceWithQuantity = (price, quantity): boolean =>
(!price.min_quantity || price.min_quantity <= quantity) &&
(!price.max_quantity || price.max_quantity >= quantity)
export default PriceSelectionStrategy