From bb5ea9d5ca92d96a64652cbb679284c817b68bff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josip=20Mati=C4=87?= Date: Tue, 5 Sep 2023 10:14:45 +0200 Subject: [PATCH] fix(medusa): Double tax issue on return refund amount (#4899) Closes #4686 Co-authored-by: Oli Juhl <59018053+olivermrbl@users.noreply.github.com> --- .changeset/bright-apples-prove.md | 5 + .../returns/ff-tax-inclusive-pricing.js | 59 +++++++ .../api/__tests__/returns/index.js | 50 ++++++ .../src/services/__mocks__/shipping-option.js | 8 + .../medusa/src/services/__tests__/return.js | 163 ++++++++++++++++++ packages/medusa/src/services/return.ts | 34 +++- 6 files changed, 316 insertions(+), 3 deletions(-) create mode 100644 .changeset/bright-apples-prove.md diff --git a/.changeset/bright-apples-prove.md b/.changeset/bright-apples-prove.md new file mode 100644 index 0000000000..d513422436 --- /dev/null +++ b/.changeset/bright-apples-prove.md @@ -0,0 +1,5 @@ +--- +"@medusajs/medusa": patch +--- + +fix(medusa): Double tax issue on return refund amount diff --git a/integration-tests/api/__tests__/returns/ff-tax-inclusive-pricing.js b/integration-tests/api/__tests__/returns/ff-tax-inclusive-pricing.js index dbbeea9819..977dc56e7a 100644 --- a/integration-tests/api/__tests__/returns/ff-tax-inclusive-pricing.js +++ b/integration-tests/api/__tests__/returns/ff-tax-inclusive-pricing.js @@ -8,6 +8,7 @@ const { useDb } = require("../../../environment-helpers/use-db") const { simpleProductFactory, simpleOrderFactory, + simpleShippingOptionFactory, } = require("../../../factories") const adminSeeder = require("../../../helpers/admin-seeder") @@ -275,4 +276,62 @@ describe("[MEDUSA_FF_TAX_INCLUSIVE_PRICING] /store/carts", () => { ]) ) }) + + it("creates a store return with tax inclusive shipping option", async () => { + await adminSeeder(dbConnection) + + const order = await createReturnableOrder(dbConnection, { + includes_tax: true, + }) + const returnOption = await simpleShippingOptionFactory(dbConnection, { + name: "Return method", + region_id: "test-region", + is_return: true, + price: 1000, + includes_tax: true, + }) + + const api = useApi() + + const response = await api.post( + `/store/returns`, + { + order_id: order.id, + return_shipping: { + option_id: returnOption.id, + }, + items: [ + { + item_id: "test-item", + quantity: 1, + note: "TOO SMALL", + }, + ], + }, + { + headers: { + authorization: "Bearer test_token", + }, + } + ) + + expect(response.status).toEqual(200) + + /* + * Region has default tax rate 12.5 but line item has tax rate 20 + * therefore refund amount should be 1000 * 1.2 = 1200 + * shipping method will have tax inclusive price of 1000 + */ + expect(response.data.return.refund_amount).toEqual(200) + expect(response.data.return.items).toHaveLength(1) + expect(response.data.return.items).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + item_id: "test-item", + quantity: 1, + note: "TOO SMALL", + }), + ]) + ) + }) }) diff --git a/integration-tests/api/__tests__/returns/index.js b/integration-tests/api/__tests__/returns/index.js index c71fb1e8fd..96fe09db82 100644 --- a/integration-tests/api/__tests__/returns/index.js +++ b/integration-tests/api/__tests__/returns/index.js @@ -189,6 +189,56 @@ describe("/admin/orders", () => { ) }) + test("creates a store return with tax exclusive shipping option", async () => { + await adminSeeder(dbConnection) + const order = await createReturnableOrder(dbConnection, { oldTaxes: false }) + const returnOption = await simpleShippingOptionFactory(dbConnection, { + name: "Return method", + region_id: "test-region", + is_return: true, + price: 1000, + }) + + const api = useApi() + + const response = await api.post( + `/store/returns`, + { + order_id: order.id, + return_shipping: { + option_id: returnOption.id, + }, + items: [ + { + item_id: "test-item", + quantity: 1, + note: "TOO SMALL", + }, + ], + }, + adminHeaders + ) + + expect(response.status).toEqual(200) + + /* + * Region has default tax rate 12.5 but line item has tax rate 20 + * therefore refund amount should be 1000 * 1.2 = 1200 + * shipping method will have 12.5 rate 1000 * 1.125 = 1125 + */ + expect(response.data.return.refund_amount).toEqual(75) + expect(response.data.return.items).toHaveLength(1) + expect(response.data.return.items).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + item_id: "test-item", + quantity: 1, + note: "TOO SMALL", + }), + ]) + ) + }) + test("creates a return w. discount", async () => { await adminSeeder(dbConnection) const order = await createReturnableOrder(dbConnection, { diff --git a/packages/medusa/src/services/__mocks__/shipping-option.js b/packages/medusa/src/services/__mocks__/shipping-option.js index e5c1a0ed5c..19c8980910 100644 --- a/packages/medusa/src/services/__mocks__/shipping-option.js +++ b/packages/medusa/src/services/__mocks__/shipping-option.js @@ -147,6 +147,14 @@ export const ShippingOptionServiceMock = { return Promise.resolve({ _id: methodId }) }), delete: jest.fn().mockReturnValue(Promise.resolve()), + createShippingMethod: jest.fn().mockImplementation((optionId, data, config) => { + return Promise.resolve({ + ...config, + id: "test-shipping-method", + shipping_option_id: optionId, + data, + }) + }), } const mock = jest.fn().mockImplementation(() => { diff --git a/packages/medusa/src/services/__tests__/return.js b/packages/medusa/src/services/__tests__/return.js index 2414673735..16fd907207 100644 --- a/packages/medusa/src/services/__tests__/return.js +++ b/packages/medusa/src/services/__tests__/return.js @@ -1,7 +1,13 @@ import { IdMap, MockManager, MockRepository } from "medusa-test-utils" +import { FlagRouter } from "@medusajs/utils" + import idMap from "medusa-test-utils/dist/id-map" import ReturnService from "../return" import { ProductVariantInventoryServiceMock } from "../__mocks__/product-variant-inventory" +import { ShippingOptionServiceMock } from "../__mocks__/shipping-option" + +import TaxInclusivePricingFeatureFlag from "../../loaders/feature-flags/tax-inclusive-pricing" + describe("ReturnService", () => { describe("receive", () => { const returnRepository = MockRepository({ @@ -334,4 +340,161 @@ describe("ReturnService", () => { ).rejects.toThrow("Cannot update a canceled return") }) }) + + describe("create", () => { + const returnRepository = MockRepository({ + findOne: (query) => { + switch (query.where.id) { + case IdMap.getId("test-return"): + return Promise.resolve({ + status: "canceled", + }) + default: + return Promise.resolve({}) + } + }, + create: (data) => data, + save: (data) => data, + }) + + const returnItemRepository = MockRepository({ + create: (data) => data, + }) + + const totalsService = { + getTotal: jest.fn().mockImplementation((cart) => { + return 1000 + }), + getRefundTotal: jest.fn().mockImplementation((order, lineItems) => { + return 100 + }), + getCalculationContext: jest + .fn() + .mockImplementation((order, lineItems) => { + return Promise.resolve({}) + }), + } + + const orderService = { + retrieve: jest.fn().mockImplementation(() => { + return Promise.resolve({ + items: [ + { + id: IdMap.getId("test-line"), + quantity: 10, + returned_quantity: 0, + variant_id: "test-variant", + }, + { + id: IdMap.getId("test-line-2"), + quantity: 10, + returned_quantity: 0, + variant_id: "test-variant-2", + }, + ], + payments: [{ id: "payment_test" }], + }) + }), + withTransaction: function () { + return this + }, + } + + const lineItemService = { + retrieve: jest.fn().mockImplementation((data) => { + return Promise.resolve({ ...data, returned_quantity: 0 }) + }), + update: jest.fn(), + withTransaction: function () { + return this + }, + } + + const returnReasonService = { + withTransaction: function () { + return this + }, + list: jest.fn().mockImplementation(() => { + return Promise.resolve([ + { + id: IdMap.getId("test-return-reason"), + value: "test-return-reason", + label: "Test Return Reason", + description: null, + parent_return_reason_id: null, + return_reason_children: [], + metadata: {}, + }, + ]) + }, {}), + } + + const shippingOptionService = ShippingOptionServiceMock + const taxProviderService = { + withTransaction: function () { + return this + }, + createShippingTaxLines: jest.fn().mockImplementation((shippingMethod) => { + return Promise.resolve([ + { + rate: 25, + }, + ]) + }), + } + + const featureFlagRouter = new FlagRouter({ + [TaxInclusivePricingFeatureFlag.key]: false, + }) + + const returnService = new ReturnService({ + manager: MockManager, + lineItemService, + orderService, + totalsService, + returnReasonService, + returnRepository, + returnItemRepository, + shippingOptionService, + taxProviderService, + featureFlagRouter, + }) + + beforeEach(async () => { + jest.clearAllMocks() + }) + + it("successfully creates a return", async () => { + await returnService.create({ + order_id: IdMap.getId("test-order"), + items: [ + { + item_id: IdMap.getId("test-line"), + quantity: 10, + }, + ], + shipping_method: { + option_id: "taxincl-option", + price: 80, + }, + }) + + expect(returnRepository.save).toHaveBeenCalledTimes(2) + expect(returnRepository.save).toHaveBeenCalledWith({ + order_id: IdMap.getId("test-order"), + items: [ + { + item_id: IdMap.getId("test-line"), + quantity: 10, + metadata: undefined, + note: undefined, + reason_id: undefined, + requested_quantity: 10, + }, + ], + status: "requested", + refund_amount: 0, + }) + }) + }) }) diff --git a/packages/medusa/src/services/return.ts b/packages/medusa/src/services/return.ts index e4a81e2381..f5685add7a 100644 --- a/packages/medusa/src/services/return.ts +++ b/packages/medusa/src/services/return.ts @@ -21,7 +21,9 @@ import { ReturnStatus, } from "../models" import { MedusaError, isDefined } from "medusa-core-utils" -import { buildQuery, setMetadata } from "../utils" +import { FlagRouter } from "@medusajs/utils" +import TaxInclusivePricingFeatureFlag from "../loaders/feature-flags/tax-inclusive-pricing" +import { buildQuery, setMetadata, calculatePriceTaxAmount } from "../utils" import { OrdersReturnItem } from "../types/orders" import { ReturnItemRepository } from "../repositories/return-item" @@ -40,6 +42,7 @@ type InjectedDependencies = { fulfillmentProviderService: FulfillmentProviderService orderService: OrderService productVariantInventoryService: ProductVariantInventoryService + featureFlagRouter: FlagRouter } type Transformer = ( @@ -60,6 +63,7 @@ class ReturnService extends TransactionBaseService { protected readonly orderService_: OrderService // eslint-disable-next-line protected readonly productVariantInventoryService_: ProductVariantInventoryService + protected readonly featureFlagRouter_: FlagRouter constructor({ totalsService, @@ -72,6 +76,7 @@ class ReturnService extends TransactionBaseService { fulfillmentProviderService, orderService, productVariantInventoryService, + featureFlagRouter, }: InjectedDependencies) { // eslint-disable-next-line prefer-rest-params super(arguments[0]) @@ -86,6 +91,7 @@ class ReturnService extends TransactionBaseService { this.returnReasonService_ = returnReasonService this.orderService_ = orderService this.productVariantInventoryService_ = productVariantInventoryService + this.featureFlagRouter_ = featureFlagRouter } /** @@ -486,11 +492,33 @@ class ReturnService extends TransactionBaseService { .withTransaction(manager) .createShippingTaxLines(shippingMethod, calculationContext) + const includesTax = + this.featureFlagRouter_.isFeatureEnabled( + TaxInclusivePricingFeatureFlag.key + ) && shippingMethod.includes_tax + + const taxRate = taxLines.reduce((acc, curr) => { + return acc + curr.rate / 100 + }, 0) + + const taxAmountIncludedInPrice = !includesTax + ? 0 + : Math.round( + calculatePriceTaxAmount({ + price: shippingMethod.price, + taxRate, + includesTax, + }) + ) + + const shippingPriceWithoutTax = + shippingMethod.price - taxAmountIncludedInPrice + const shippingTotal = - shippingMethod.price + + shippingPriceWithoutTax + taxLines.reduce( (acc, tl) => - acc + Math.round(shippingMethod.price * (tl.rate / 100)), + acc + Math.round(shippingPriceWithoutTax * (tl.rate / 100)), 0 )