asyavee/medusa-store
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

fix(medusa): calculates correct taxes and totals on order with gift cards (#1807)

Browse Source 
**What**
Since the release of the Tax API the line item totals calculations on orders with gift cards have been wrong. To understand the bug consider the below order:

Region:
- tax_rate: 25%
- gift_cards_taxable: true

Order:
- applied gift card: 1000
- items: 
  - A: unit_price: 1000
  - B: unit_price: 500
- Subtotal: 1500

**Previous calculation method**
1. Determine how much of the gift card is used for each item using `item_total / subtotal * gift_card_amount`:
  - Item A: 1000/1500 * 1000 = 666.67
  - Item B: 500/1500 * 1000 = 333.33
2. Calculate line item totals including taxes using `(unit_price - gift_card) * (1 + tax_rate)`
  - Item A: 1000 - 666.67 = 333.33; vat amount -> 83.33
  - Item B: 500 - 333.33 = 166.67; vat amount -> 41.67
3. Add up the line item totals: order subtotal = 500; vat amount = 125; total = 625

This is all correct at the totals level; but at the line item level we should still use the "original prices" i.e. the line item total for item a should be (1000 * 1.25) = 1250 with a tax amount of 250. 

**New calculation method**
1. Use default totals calculations
  - Item A: subtotal: 1000, tax_total: 250, total: 1250
  - Item B: subtotal: 500, tax_total: 125, total: 625
2. Add up the line item totals: subtotal: 1500, tax_total: 375, total: 1875
3. Reduce total with gift card: subtotal: 1500 - 1000 = 500, tax_total: 375 - 250 = 125, total = 625

Totals can now be forwarded correctly to accounting plugins.

Fixes CORE-310.
This commit is contained in:
Sebastian Rindom
2022-07-11 12:18:43 +00:00
committed by GitHub
parent 3e197e3adf
commit 39f2c0c15e
19 changed files with 413 additions and 83 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/medusa/src/migrations/1657098186554-taxed_gift_card_transactions.ts
+25
View File
@@ -0,0 +1,25 @@
import { MigrationInterface, QueryRunner } from "typeorm"
export class taxedGiftCardTransactions1657098186554
implements MigrationInterface
{
name = "taxedGiftCardTransactions1657098186554"
public async up(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(
`ALTER TABLE "gift_card_transaction" ADD "is_taxable" boolean`
)
await queryRunner.query(
`ALTER TABLE "gift_card_transaction" ADD "tax_rate" real`
)
}
public async down(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(
`ALTER TABLE "gift_card_transaction" DROP COLUMN "is_taxable"`
)
await queryRunner.query(
`ALTER TABLE "gift_card_transaction" DROP COLUMN "tax_rate"`
)
}
}
packages/medusa/src/models/cart.ts
+1
View File
@@ -252,6 +252,7 @@ export class Cart extends SoftDeletableEntity {
subtotal?: number
refundable_amount?: number
gift_card_total?: number
gift_card_tax_total?: number
@AfterLoad()
private afterLoad(): void {
packages/medusa/src/models/gift-card-transaction.ts
+6
View File
@@ -42,6 +42,12 @@ export class GiftCardTransaction {
@CreateDateColumn({ type: resolveDbType("timestamptz") })
created_at: Date
@Column({ nullable: true })
is_taxable: boolean
@Column({ type: "real", nullable: true })
tax_rate: number | null
@BeforeInsert()
private beforeInsert(): void {
this.id = generateEntityId(this.id, "gct")
packages/medusa/src/models/order.ts
+1
View File
@@ -249,6 +249,7 @@ export class Order extends BaseEntity {
paid_total: number
refundable_amount: number
gift_card_total: number
gift_card_tax_total: number
@BeforeInsert()
private async beforeInsert(): Promise<void> {
packages/medusa/src/services/__mocks__/totals.js
+8 -2
View File
@@ -1,13 +1,19 @@
import { IdMap } from "medusa-test-utils"
export const TotalsServiceMock = {
getTotal: jest.fn().mockImplementation(cart => {
getTotal: jest.fn().mockImplementation((cart) => {
if (cart.total) {
return cart.total
}
return 0
}),
getSubtotal: jest.fn().mockImplementation(cart => {
getGiftCardableAmount: jest.fn().mockImplementation((cart) => {
if (cart.subtotal) {
return cart.subtotal
}
return 0
}),
getSubtotal: jest.fn().mockImplementation((cart) => {
if (cart.subtotal) {
return cart.subtotal
}
packages/medusa/src/services/__tests__/order.js
+24 -20
View File
@@ -8,6 +8,9 @@ describe("OrderService", () => {
getTotal: (o) => {
return o.total || 0
},
getGiftCardableAmount: (o) => {
return o.subtotal || 0
},
getRefundedTotal: (o) => {
return o.refunded_total || 0
},
@@ -33,7 +36,7 @@ describe("OrderService", () => {
const eventBusService = {
emit: jest.fn(),
withTransaction: function() {
withTransaction: function () {
return this
},
}
@@ -78,20 +81,20 @@ describe("OrderService", () => {
})
const lineItemService = {
update: jest.fn(),
withTransaction: function() {
withTransaction: function () {
return this
},
}
const shippingOptionService = {
updateShippingMethod: jest.fn(),
withTransaction: function() {
withTransaction: function () {
return this
},
}
const giftCardService = {
update: jest.fn(),
createTransaction: jest.fn(),
withTransaction: function() {
withTransaction: function () {
return this
},
}
@@ -103,7 +106,7 @@ describe("OrderService", () => {
cancelPayment: jest.fn().mockImplementation((payment) => {
return Promise.resolve({ ...payment, status: "cancelled" })
}),
withTransaction: function() {
withTransaction: function () {
return this
},
}
@@ -142,7 +145,7 @@ describe("OrderService", () => {
total: 100,
})
}),
withTransaction: function() {
withTransaction: function () {
return this
},
}
@@ -281,6 +284,7 @@ describe("OrderService", () => {
id: "test",
currency_code: "eur",
name: "test",
gift_cards_taxable: true,
tax_rate: 25,
},
shipping_address_id: "1234",
@@ -339,6 +343,8 @@ describe("OrderService", () => {
expect(giftCardService.createTransaction).toHaveBeenCalledWith({
gift_card_id: "gid",
order_id: "id",
is_taxable: true,
tax_rate: 25,
amount: 80,
})
@@ -633,14 +639,14 @@ describe("OrderService", () => {
const fulfillmentService = {
cancelFulfillment: jest.fn(),
withTransaction: function() {
withTransaction: function () {
return this
},
}
const paymentProviderService = {
cancelPayment: jest.fn(),
withTransaction: function() {
withTransaction: function () {
return this
},
}
@@ -737,7 +743,7 @@ describe("OrderService", () => {
? Promise.reject()
: Promise.resolve({ ...p, captured_at: "notnull" })
),
withTransaction: function() {
withTransaction: function () {
return this
},
}
@@ -842,7 +848,7 @@ describe("OrderService", () => {
const lineItemService = {
update: jest.fn(),
withTransaction: function() {
withTransaction: function () {
return this
},
}
@@ -855,7 +861,7 @@ describe("OrderService", () => {
},
])
}),
withTransaction: function() {
withTransaction: function () {
return this
},
}
@@ -1022,7 +1028,7 @@ describe("OrderService", () => {
})
}
}),
withTransaction: function() {
withTransaction: function () {
return this
},
}
@@ -1091,7 +1097,7 @@ describe("OrderService", () => {
.mockImplementation((p) =>
p.id === "payment_fail" ? Promise.reject() : Promise.resolve()
),
withTransaction: function() {
withTransaction: function () {
return this
},
}
@@ -1232,7 +1238,7 @@ describe("OrderService", () => {
.fn()
.mockImplementation(() => Promise.resolve({})),
withTransaction: function() {
withTransaction: function () {
return this
},
}
@@ -1366,7 +1372,7 @@ describe("OrderService", () => {
const lineItemService = {
update: jest.fn(),
withTransaction: function() {
withTransaction: function () {
return this
},
}
@@ -1389,7 +1395,7 @@ describe("OrderService", () => {
],
})
}),
withTransaction: function() {
withTransaction: function () {
return this
},
}
@@ -1416,9 +1422,7 @@ describe("OrderService", () => {
)
expect(fulfillmentService.createShipment).toHaveBeenCalledTimes(1)
expect(
fulfillmentService.createShipment
).toHaveBeenCalledWith(
expect(fulfillmentService.createShipment).toHaveBeenCalledWith(
IdMap.getId("fulfillment"),
[{ tracking_number: "1234" }, { tracking_number: "2345" }],
{ metadata: undefined, no_notification: true }
@@ -1510,7 +1514,7 @@ describe("OrderService", () => {
refundPayment: jest
.fn()
.mockImplementation((p) => Promise.resolve({ id: "ref" })),
withTransaction: function() {
withTransaction: function () {
return this
},
}
packages/medusa/src/services/cart.ts
+5 -2
View File
@@ -234,9 +234,12 @@ class CartService extends TransactionBaseService<CartService> {
options.force_taxes
)
break
case "gift_card_total":
totals.gift_card_total = this.totalsService_.getGiftCardTotal(cart)
case "gift_card_total": {
const giftCardBreakdown = this.totalsService_.getGiftCardTotal(cart)
totals.gift_card_total = giftCardBreakdown.total
totals.gift_card_tax_total = giftCardBreakdown.tax_total
break
}
case "subtotal":
totals.subtotal = this.totalsService_.getSubtotal(cart)
break
packages/medusa/src/services/order.js
+37 -35
View File
@@ -464,20 +464,21 @@ class OrderService extends BaseService {
*/
async createFromCart(cartId) {
return this.atomicPhase_(async (manager) => {
const cart = await this.cartService_
.withTransaction(manager)
.retrieve(cartId, {
select: ["subtotal", "total"],
relations: [
"region",
"payment",
"items",
"discounts",
"discounts.rule",
"gift_cards",
"shipping_methods",
],
})
const cartService = this.cartService_.withTransaction(manager)
const inventoryService = this.inventoryService_.withTransaction(manager)
const cart = await cartService.retrieve(cartId, {
select: ["subtotal", "total"],
relations: [
"region",
"payment",
"items",
"discounts",
"discounts.rule",
"gift_cards",
"shipping_methods",
],
})
if (cart.items.length === 0) {
throw new MedusaError(
@@ -490,18 +491,17 @@ class OrderService extends BaseService {
for (const item of cart.items) {
try {
await this.inventoryService_
.withTransaction(manager)
.confirmInventory(item.variant_id, item.quantity)
await inventoryService.confirmInventory(
item.variant_id,
item.quantity
)
} catch (err) {
if (payment) {
await this.paymentProviderService_
.withTransaction(manager)
.cancelPayment(payment)
}
await this.cartService_
.withTransaction(manager)
.update(cart.id, { payment_authorized_at: null })
await cartService.update(cart.id, { payment_authorized_at: null })
throw err
}
}
@@ -564,8 +564,7 @@ class OrderService extends BaseService {
toCreate.no_notification = draft.no_notification_order
}
const o = await orderRepo.create(toCreate)
const o = orderRepo.create(toCreate)
const result = await orderRepo.save(o)
if (total !== 0) {
@@ -576,19 +575,25 @@ class OrderService extends BaseService {
})
}
let gcBalance = cart.subtotal
let gcBalance = await this.totalsService_.getGiftCardableAmount(cart)
const gcService = this.giftCardService_.withTransaction(manager)
for (const g of cart.gift_cards) {
const newBalance = Math.max(0, g.balance - gcBalance)
const usage = g.balance - newBalance
await this.giftCardService_.withTransaction(manager).update(g.id, {
await gcService.update(g.id, {
balance: newBalance,
disabled: newBalance === 0,
})
await this.giftCardService_.withTransaction(manager).createTransaction({
await gcService.createTransaction({
gift_card_id: g.id,
order_id: result.id,
amount: usage,
is_taxable: cart.region.gift_cards_taxable,
tax_rate: cart.region.gift_cards_taxable
? cart.region.tax_rate
: null,
})
gcBalance = gcBalance - usage
@@ -600,16 +605,13 @@ class OrderService extends BaseService {
.updateShippingMethod(method.id, { order_id: result.id })
}
const lineItemService = this.lineItemService_.withTransaction(manager)
for (const item of cart.items) {
await this.lineItemService_
.withTransaction(manager)
.update(item.id, { order_id: result.id })
await lineItemService.update(item.id, { order_id: result.id })
}
for (const item of cart.items) {
await this.inventoryService_
.withTransaction(manager)
.adjustInventory(item.variant_id, -item.quantity)
await inventoryService.adjustInventory(item.variant_id, -item.quantity)
}
await this.eventBus_
@@ -619,9 +621,7 @@ class OrderService extends BaseService {
no_notification: result.no_notification,
})
await this.cartService_
.withTransaction(manager)
.update(cart.id, { completed_at: new Date() })
await cartService.update(cart.id, { completed_at: new Date() })
return result
})
@@ -1383,7 +1383,9 @@ class OrderService extends BaseService {
break
}
case "gift_card_total": {
order.gift_card_total = this.totalsService_.getGiftCardTotal(order)
const giftCardBreakdown = this.totalsService_.getGiftCardTotal(order)
order.gift_card_total = giftCardBreakdown.total
order.gift_card_tax_total = giftCardBreakdown.tax_total
break
}
case "discount_total": {
packages/medusa/src/services/totals.ts
+85 -17
View File
@@ -1,4 +1,3 @@
import _ from "lodash"
import { MedusaError } from "medusa-core-utils"
import { BaseService } from "medusa-interfaces"
import { ITaxCalculationStrategy, TaxCalculationContext } from "../interfaces"
@@ -64,6 +63,7 @@ type TotalsServiceProps = {
}
type GetTotalsOptions = {
exclude_gift_cards?: boolean
force_taxes?: boolean
}
@@ -111,10 +111,14 @@ class TotalsService extends BaseService {
const taxTotal =
(await this.getTaxTotal(cartOrOrder, options.force_taxes)) || 0
const discountTotal = this.getDiscountTotal(cartOrOrder)
const giftCardTotal = this.getGiftCardTotal(cartOrOrder)
const giftCardTotal = options.exclude_gift_cards
? { total: 0 }
: this.getGiftCardTotal(cartOrOrder)
const shippingTotal = this.getShippingTotal(cartOrOrder)
return subtotal + taxTotal + shippingTotal - discountTotal - giftCardTotal
return (
subtotal + taxTotal + shippingTotal - discountTotal - giftCardTotal.total
)
}
/**
@@ -292,6 +296,7 @@ class TotalsService extends BaseService {
}
const calculationContext = this.getCalculationContext(cartOrOrder)
const giftCardTotal = this.getGiftCardTotal(cartOrOrder)
let taxLines: (ShippingMethodTaxLine | LineItemTaxLine)[]
if (isOrder(cartOrOrder)) {
@@ -317,9 +322,8 @@ class TotalsService extends BaseService {
const subtotal = this.getSubtotal(cartOrOrder)
const shippingTotal = this.getShippingTotal(cartOrOrder)
const discountTotal = this.getDiscountTotal(cartOrOrder)
const giftCardTotal = this.getGiftCardTotal(cartOrOrder)
return this.rounded(
(subtotal - discountTotal - giftCardTotal + shippingTotal) *
(subtotal - discountTotal - giftCardTotal.total + shippingTotal) *
(cartOrOrder.tax_rate / 100)
)
}
@@ -354,6 +358,10 @@ class TotalsService extends BaseService {
calculationContext
)
if (cartOrOrder.region.gift_cards_taxable) {
return this.rounded(toReturn - giftCardTotal.tax_total)
}
return this.rounded(toReturn)
}
@@ -385,13 +393,13 @@ class TotalsService extends BaseService {
if (allocationMap[ld.item.id]) {
allocationMap[ld.item.id].discount = {
amount: ld.amount,
unit_amount: ld.amount / ld.item.quantity,
unit_amount: Math.round(ld.amount / ld.item.quantity),
}
} else {
allocationMap[ld.item.id] = {
discount: {
amount: ld.amount,
unit_amount: ld.amount / ld.item.quantity,
unit_amount: Math.round(ld.amount / ld.item.quantity),
},
}
}
@@ -406,13 +414,13 @@ class TotalsService extends BaseService {
// If the fixed discount exceeds the subtotal we should
// calculate a 100% discount
const nominator = Math.min(giftCardTotal, subtotal)
const nominator = Math.min(giftCardTotal.total, subtotal)
const percentage = nominator / subtotal
lineGiftCards = orderOrCart.items.map((l) => {
return {
item: l,
amount: l.unit_price * l.quantity * percentage,
amount: Math.round(l.unit_price * l.quantity * percentage),
}
})
}
@@ -421,13 +429,13 @@ class TotalsService extends BaseService {
if (allocationMap[lgc.item.id]) {
allocationMap[lgc.item.id].gift_card = {
amount: lgc.amount,
unit_amount: lgc.amount / lgc.item.quantity,
unit_amount: Math.round(lgc.amount / lgc.item.quantity),
}
} else {
allocationMap[lgc.item.id] = {
discount: {
gift_card: {
amount: lgc.amount,
unit_amount: lgc.amount / lgc.item.quantity,
unit_amount: Math.round(lgc.amount / lgc.item.quantity),
},
}
}
@@ -821,31 +829,91 @@ class TotalsService extends BaseService {
return toReturn
}
/**
* Gets the amount that can be gift carded on a cart. In regions where gift
* cards are taxable this amount should exclude taxes.
* @param cartOrOrder - the cart or order to get gift card amount for
* @return the gift card amount applied to the cart or order
*/
async getGiftCardableAmount(cartOrOrder: Cart | Order): Promise<number> {
if (cartOrOrder.region?.gift_cards_taxable) {
return this.getSubtotal(cartOrOrder) - this.getDiscountTotal(cartOrOrder)
}
return await this.getTotal(cartOrOrder, {
exclude_gift_cards: true,
})
}
/**
* Gets the gift card amount on a cart or order.
* @param cartOrOrder - the cart or order to get gift card amount for
* @return the gift card amount applied to the cart or order
*/
getGiftCardTotal(cartOrOrder: Cart | Order): number {
getGiftCardTotal(cartOrOrder: Cart | Order): {
total: number
tax_total: number
} {
const giftCardable =
this.getSubtotal(cartOrOrder) - this.getDiscountTotal(cartOrOrder)
if ("gift_card_transactions" in cartOrOrder) {
// gift_card_transactions only exist on orders so we can
// safely calculate the total based on the gift card transactions
return cartOrOrder.gift_card_transactions.reduce(
(acc, next) => acc + next.amount,
0
(acc, next) => {
let taxMultiplier = (next.tax_rate || 0) / 100
// Previously we did not record whether a gift card was taxable or not.
// All gift cards where is_taxable === null are from the old system,
// where we defaulted to taxable gift cards.
//
// This is a backwards compatability fix for orders that were created
// before we added the gift card tax rate.
if (
next.is_taxable === null &&
cartOrOrder.region?.gift_cards_taxable
) {
taxMultiplier = cartOrOrder.region.tax_rate / 100
}
return {
total: acc.total + next.amount,
tax_total: acc.tax_total + next.amount * taxMultiplier,
}
},
{
total: 0,
tax_total: 0,
}
)
}
if (!cartOrOrder.gift_cards || !cartOrOrder.gift_cards.length) {
return 0
return {
total: 0,
tax_total: 0,
}
}
const toReturn = cartOrOrder.gift_cards.reduce(
(acc, next) => acc + next.balance,
0
)
return Math.min(giftCardable, toReturn)
const orderGiftCardAmount = Math.min(giftCardable, toReturn)
if (cartOrOrder.region?.gift_cards_taxable) {
return {
total: orderGiftCardAmount,
tax_total: (orderGiftCardAmount * cartOrOrder.region.tax_rate) / 100,
}
}
return {
total: orderGiftCardAmount,
tax_total: 0,
}
}
/**
packages/medusa/src/strategies/__tests__/tax-calculation.js
+1 -1
View File
@@ -64,7 +64,7 @@ const toTest = [
* Taxline 2 = 180 * 0.125 = 23
* Total tax = 38
*/
expected: 38,
expected: 40,
items: [
{
id: "item_1",
packages/medusa/src/strategies/tax-calculation.ts
-5
View File
@@ -38,11 +38,6 @@ class TaxCalculationStrategy implements ITaxCalculationStrategy {
let taxableAmount = i.quantity * i.unit_price
if (context.region.gift_cards_taxable) {
taxableAmount -=
(allocations.gift_card && allocations.gift_card.amount) || 0
}
taxableAmount -=
((allocations.discount && allocations.discount.unit_amount) || 0) *
i.quantity
packages/medusa/src/types/common.ts
+1
View File
@@ -67,6 +67,7 @@ export type TotalField =
| "subtotal"
| "refundable_amount"
| "gift_card_total"
| "gift_card_tax_total"
export interface FindConfig<Entity> {
select?: (keyof Entity)[]