fix(draft-order): create tax-inclusive with discount (#2579)

**What**
Fix system error (500) with DraftOrder create operation when payload includes discount in a tax-inclusive context.

**How**
* Ensure newly created cart contains all required relation in order to calculate line item tax-inclusive pricing with discounts.
* Add resilience to TotalsService.getLineDiscounts()
* Ensure newly generate line items have variant relation loaded.
* fix TotalsService.getLineItemTotals to use the passed lineItem instead of relying on cartOrOrder.items.

**Test**
* Unit:
  *  TotalsService.getLineDiscounts - coverage
* Integration:
  * Admin API draft-order - coverage
  * Admin API draft-order create w/ discount in tax-inclusive

Resolves: CORE-771

Co-authored-by: Oliver Windall Juhl <59018053+olivermrbl@users.noreply.github.com>
Co-authored-by: Adrien de Peretti <25098370+adrien2p@users.noreply.github.com>
This commit is contained in:
authored and GitHub committed 2022-11-16 15:34:18 +00:00
1 parent 03fc9e18e9
commit 022a84691e
17 files changed
+765 -1048

No files matched your search

@@ -1,10 +1,10 @@
import { MockRepository, MockManager } from "medusa-test-utils"
import { MockManager, MockRepository } from "medusa-test-utils"
import { EventBusServiceMock } from "../__mocks__/event-bus"
import DraftOrderService from "../draft-order"
const eventBusService = {
emit: jest.fn(),
withTransaction: function() {
withTransaction: function () {
return this
},
}
@@ -37,7 +37,7 @@ describe("DraftOrderService", () => {
const regionService = {
retrieve: () =>
Promise.resolve({ id: "test-region", countries: [{ iso_2: "dk" }] }),
withTransaction: function() {
withTransaction: function () {
return this
},
}
@@ -50,7 +50,7 @@ describe("DraftOrderService", () => {
},
})
),
withTransaction: function() {
withTransaction: function () {
return this
},
}
@@ -63,7 +63,7 @@ describe("DraftOrderService", () => {
})
),
create: jest.fn(),
withTransaction: function() {
withTransaction: function () {
return this
},
}
@@ -76,20 +76,7 @@ describe("DraftOrderService", () => {
profile_id: "test-profile",
},
}),
withTransaction: function() {
return this
},
}
const cartService = {
create: jest.fn().mockImplementation((data) =>
Promise.resolve({
id: "test-cart",
...data,
})
),
addShippingMethod: jest.fn(),
withTransaction: function() {
withTransaction: function () {
return this
},
}
@@ -108,6 +95,25 @@ describe("DraftOrderService", () => {
],
}
const cartService = {
create: jest.fn().mockImplementation((data) =>
Promise.resolve({
id: "test-cart",
...data,
})
),
retrieve: jest.fn().mockReturnValue(
Promise.resolve({
id: "test-cart",
...testOrder,
})
),
addShippingMethod: jest.fn(),
withTransaction: function () {
return this
},
}
const addressRepository = MockRepository({
create: (addr) => ({
...addr,
@@ -157,6 +163,11 @@ describe("DraftOrderService", () => {
type: "draft_order",
})
expect(cartService.retrieve).toHaveBeenCalledTimes(1)
expect(cartService.retrieve).toHaveBeenCalledWith("test-cart", {
relations: ["discounts", "discounts.rule", "items", "region"],
})
expect(cartService.addShippingMethod).toHaveBeenCalledTimes(1)
expect(cartService.addShippingMethod).toHaveBeenCalledWith(
"test-cart",
@@ -413,6 +413,14 @@ describe("LineItemService", () => {
metadata: {},
should_merge: true,
includes_tax: true,
variant: expect.objectContaining({
id: expect.any(String),
product: expect.objectContaining({
thumbnail: "",
title: "Test product",
}),
title: "Test variant",
}),
})
})
})
@@ -524,6 +532,14 @@ describe("LineItemService", () => {
metadata: {},
should_merge: true,
includes_tax: false,
variant: expect.objectContaining({
id: expect.any(String),
product: expect.objectContaining({
thumbnail: "",
title: "Test product",
}),
title: "Test variant",
}),
})
})
})
@@ -12,6 +12,7 @@ import {
Discount,
DiscountRuleType,
LineItem,
ProductVariant,
Region,
ShippingMethod,
} from "../../models"
@@ -98,6 +99,7 @@ describe("New totals service", () => {
it("should fetch the items tax lines to compute the totals", async () => {
const testItem = { ...lineItems[0] } as LineItem
testItem.variant = new ProductVariant()
testItem.tax_lines = []
const calculationContext = {
@@ -706,6 +708,7 @@ describe("New totals service", () => {
it("should fetch the tax lines to compute the totals", async () => {
const testItem = { ...lineItems[0] } as LineItem
testItem.tax_lines = []
testItem.variant = new ProductVariant()
testItem.includes_tax = true
const calculationContext = {
@@ -113,14 +113,19 @@ const calculateAdjustment = (cart, lineItem, discount) => {
}
describe("TotalsService", () => {
const getTaxLinesMock = jest.fn(() => Promise.resolve([{ id: "line1" }]))
const getTaxLinesMock = jest.fn((items) => {
const taxLines = items.some((item) => item.id === "line1")
? [{ id: "line1" }]
: []
return Promise.resolve(taxLines)
})
const featureFlagRouter = new FlagRouter({
[TaxInclusivePricingFeatureFlag.key]: false,
})
const container = {
taxProviderService: {
withTransaction: function() {
withTransaction: function () {
return this
},
getTaxLines: getTaxLinesMock,
@@ -353,6 +358,114 @@ describe("TotalsService", () => {
})
})
describe("getLineDiscounts", () => {
let res
const totalsService = new TotalsService(container)
const mockCart = {
swaps: [],
claims: [],
items: [],
}
let cart
beforeEach(() => {
jest.clearAllMocks()
cart = { ...mockCart }
})
it("returns [] if items is empty in cart", async () => {
res = totalsService.getLineDiscounts(cart, discounts.total10Fixed)
expect(res).toEqual([])
})
it("returns [] if items is undefined in cart", async () => {
cart.items = undefined
res = totalsService.getLineDiscounts(cart, discounts.total10Fixed)
expect(res).toEqual([])
})
it("returns flat list of discount amounts for a cart with multiple items, swaps, claims for a given discount", async () => {
const discountId = discounts.total10Fixed.id
const discountValue = discounts.total10Fixed.rule.value
const item1 = {
allow_discounts: true,
unit_price: 18,
adjustments: [
{
discount_id: discountId,
amount: discountValue,
},
],
}
const item2 = { ...item1, unit_price: 19 }
const item3 = { ...item1, unit_price: 20 }
const item4 = { ...item1, unit_price: 21 }
const item5 = { ...item1, unit_price: 22 }
const item6 = { ...item1, unit_price: 23 }
const item7 = { ...item1, unit_price: 24 }
const item8 = { ...item1, unit_price: 25 }
const swap1 = {
additional_items: [item3, item4],
}
const swap2 = {
additional_items: [item5],
}
const claim1 = {
additional_items: [item6, item7],
}
const claim2 = {
additional_items: [item8],
}
cart.items = [item1, item2]
cart.swaps = [swap1, swap2]
cart.claims = [claim1, claim2]
res = totalsService.getLineDiscounts(cart, discounts.total10Fixed)
expect(res).toEqual(
expect.arrayContaining([
expect.objectContaining({
amount: discountValue,
item: item1,
}),
expect.objectContaining({
amount: discountValue,
item: item2,
}),
expect.objectContaining({
amount: discountValue,
item: item3,
}),
expect.objectContaining({
amount: discountValue,
item: item4,
}),
expect.objectContaining({
amount: discountValue,
item: item5,
}),
expect.objectContaining({
amount: discountValue,
item: item6,
}),
expect.objectContaining({
amount: discountValue,
item: item7,
}),
expect.objectContaining({
amount: discountValue,
item: item8,
}),
])
)
})
})
describe("getRefundTotal", () => {
let res
const totalsService = new TotalsService(container)
@@ -718,7 +831,7 @@ describe("TotalsService", () => {
const totalsService = new TotalsService({
...container,
taxProviderService: {
withTransaction: function() {
withTransaction: function () {
return this
},
getTaxLines: getTaxLinesMock,
@@ -757,13 +870,18 @@ describe("TotalsService", () => {
let res
let totalsService
const getTaxLinesMock = jest.fn(() => Promise.resolve([{ id: "line1" }]))
const getTaxLinesMock = jest.fn((items) => {
const taxLines = items.some((item) => item.id === "line1")
? [{ id: "line1" }]
: []
return Promise.resolve(taxLines)
})
const calculateMock = jest.fn(() => Promise.resolve(20.3))
const getAllocationMapMock = jest.fn(() => ({}))
const cradle = {
taxProviderService: {
withTransaction: function() {
withTransaction: function () {
return this
},
getTaxLines: getTaxLinesMock,
@@ -854,6 +972,7 @@ describe("TotalsService", () => {
},
items: [
{
id: "line1",
unit_price: 20,
quantity: 2,
},
@@ -885,7 +1004,7 @@ describe("TotalsService", () => {
expect(getTaxLinesMock).toHaveBeenCalledTimes(2)
expect(getTaxLinesMock).toHaveBeenNthCalledWith(
2,
[{ quantity: 2, unit_price: 20 }],
[{ id: "line1", quantity: 2, unit_price: 20 }],
{
shipping_address: order.shipping_address,
shipping_methods: order.shipping_methods,
@@ -896,7 +1015,7 @@ describe("TotalsService", () => {
}
)
expect(calculateMock).toHaveBeenCalledTimes(1)
expect(calculateMock).toHaveBeenCalledTimes(3)
expect(calculateMock).toHaveBeenCalledWith(
order.items,
[{ id: "line1" }],
+17 -10
View File
@@ -10,6 +10,7 @@ import {
} from "typeorm"
import {
EventBusService,
NewTotalsService,
ProductService,
RegionService,
TotalsService,
@@ -57,6 +58,7 @@ class DiscountService extends TransactionBaseService {
protected readonly discountConditionRepository_: typeof DiscountConditionRepository
protected readonly discountConditionService_: DiscountConditionService
protected readonly totalsService_: TotalsService
protected readonly newTotalsService_: NewTotalsService
protected readonly productService_: ProductService
protected readonly regionService_: RegionService
protected readonly eventBus_: EventBusService
@@ -70,6 +72,7 @@ class DiscountService extends TransactionBaseService {
discountConditionRepository,
discountConditionService,
totalsService,
newTotalsService,
productService,
regionService,
customerService,
@@ -86,6 +89,7 @@ class DiscountService extends TransactionBaseService {
this.discountConditionRepository_ = discountConditionRepository
this.discountConditionService_ = discountConditionService
this.totalsService_ = totalsService
this.newTotalsService_ = newTotalsService
this.productService_ = productService
this.regionService_ = regionService
this.customerService_ = customerService
@@ -571,7 +575,7 @@ class DiscountService extends TransactionBaseService {
lineItem: LineItem,
cart: Cart
): Promise<number> {
return await this.atomicPhase_(async () => {
return await this.atomicPhase_(async (transactionManager) => {
let adjustment = 0
if (!lineItem.allow_discounts) {
@@ -589,15 +593,18 @@ class DiscountService extends TransactionBaseService {
) &&
lineItem.includes_tax
) {
const lineItemTotals = await this.totalsService_.getLineItemTotals(
lineItem,
cart,
{
include_tax: true,
exclude_gift_cards: true,
}
)
fullItemPrice = lineItemTotals.subtotal
const calculationContext = await this.totalsService_
.withTransaction(transactionManager)
.getCalculationContext(cart, {
exclude_shipping: true,
})
const lineItemTotals = await this.newTotalsService_
.withTransaction(transactionManager)
.getLineItemTotals([lineItem], {
includeTax: true,
calculationContext,
})
fullItemPrice = lineItemTotals[lineItem.id].subtotal
}
if (type === DiscountRuleType.PERCENTAGE) {
+4 -1
View File
@@ -274,10 +274,13 @@ class DraftOrderService extends TransactionBaseService {
}
}
const createdCart = await cartServiceTx.create({
let createdCart = await cartServiceTx.create({
type: CartType.DRAFT_ORDER,
...rawCart,
})
createdCart = await cartServiceTx.retrieve(createdCart.id, {
relations: ["discounts", "discounts.rule", "items", "region"],
})
const draftOrder = draftOrderRepo.create({
cart_id: createdCart.id,
@@ -184,14 +184,18 @@ class LineItemAdjustmentService extends TransactionBaseService {
generatedLineItem: LineItem,
context: AdjustmentContext
): Promise<GeneratedAdjustment[]> {
const lineItem = {
...generatedLineItem,
} as LineItem
return this.atomicPhase_(async (manager) => {
// if lineItem should not be discounted
// or lineItem is a return line item
// or the cart does not have any discounts
// then do nothing
if (
!generatedLineItem.allow_discounts ||
generatedLineItem.is_return ||
!lineItem.allow_discounts ||
lineItem.is_return ||
!cart?.discounts?.length
) {
return []
@@ -217,9 +221,11 @@ class LineItemAdjustmentService extends TransactionBaseService {
return []
}
// In case of a generated line item the id is not available, it is mocked instead to be used for totals calculations
lineItem.id = lineItem.id ?? new Date().getTime()
const amount = await this.discountService.calculateDiscountForLineItem(
discount.id,
generatedLineItem,
lineItem,
cart
)
+4 -1
View File
@@ -256,7 +256,10 @@ class LineItemService extends TransactionBaseService {
const lineItemRepo = transactionManager.getCustomRepository(
this.lineItemRepository_
)
const lineItem = lineItemRepo.create(rawLineItem)
const lineItem = lineItemRepo.create({
...rawLineItem,
variant,
})
if (context.cart) {
const adjustments = await this.lineItemAdjustmentService_
@@ -104,6 +104,13 @@ export default class NewTotalsService extends TransactionBaseService {
lineItemsTaxLinesMap[item.id] = item.tax_lines ?? []
})
} else {
if (items.some((item) => !item.variant)) {
throw new MedusaError(
MedusaError.Types.INVALID_DATA,
"Unable to fetch tax lines to compute line item totals as one of the item variant is missing but required for the tax lines to be computed. Might be due to a missing relation items.variant"
)
}
const { lineItemsTaxLines } = await this.taxProviderService_
.withTransaction(manager)
.getTaxLinesMap(items, calculationContext)
+3 -10
View File
@@ -703,7 +703,7 @@ class TotalsService extends TransactionBaseService {
cartOrOrder: Cart | Order,
discount: Discount
): LineDiscountAmount[] {
let merged: LineItem[] = [...cartOrOrder.items]
let merged: LineItem[] = [...(cartOrOrder.items ?? [])]
// merge items from order with items from order swaps
if ("swaps" in cartOrOrder && cartOrOrder.swaps.length) {
@@ -842,16 +842,9 @@ class TotalsService extends TransactionBaseService {
}
taxLines = lineItem.tax_lines
} else {
const orderLines = await this.taxProviderService_
taxLines = (await this.taxProviderService_
.withTransaction(this.manager_)
.getTaxLines(cartOrOrder.items, calculationContext)
taxLines = orderLines.filter((ol) => {
if ("item_id" in ol) {
return ol.item_id === lineItem.id
}
return false
}) as LineItemTaxLine[]
.getTaxLines([lineItem], calculationContext)) as LineItemTaxLine[]
}
}