From 5ac7f08e4d013491ecc4dc0ed497e665560dfd1e Mon Sep 17 00:00:00 2001 From: Oliver Windall Juhl <59018053+olivermrbl@users.noreply.github.com> Date: Wed, 24 Aug 2022 16:07:44 +0200 Subject: [PATCH] fix(medusa): Incorrect swap difference due (#2086) ### What Creating a swap on an order with a discount leads to an incorrect difference due. **Scenario** - Create a store with minimum 2 products (Prod A, Prod B) - Create a discount that only works for Prod A - Create an order for Prod A with the discount applied - Create a swap between Prod A and Prod B **Expected outcome** We would expect the difference_due amount to come out to the sum of: - -1 * (price of prod a - discount applied to prod a) - price of prod b **Actual outcome** Instead the discount is applied across both products when calculating difference due. This results in a total that is instead the sum of: - -1 * (price of prod a - discount applied to prod a) - price of prod b - discount on prod b ignoring the condition ### How Adds `line_item.adjustments` to relations in cart retrieval prior to setting the difference_due to car total Fixes CORE-361 --- .../api/__tests__/admin/swaps.js | 215 ++++++++++++++++++ .../api/__tests__/totals/orders.js | 1 + .../simple-shipping-option-factory.ts | 6 +- integration-tests/api/package.json | 6 +- integration-tests/api/yarn.lock | 86 ++++--- packages/medusa/src/services/swap.js | 44 ++-- 6 files changed, 289 insertions(+), 69 deletions(-) diff --git a/integration-tests/api/__tests__/admin/swaps.js b/integration-tests/api/__tests__/admin/swaps.js index cf53d3c25e..e2a954c076 100644 --- a/integration-tests/api/__tests__/admin/swaps.js +++ b/integration-tests/api/__tests__/admin/swaps.js @@ -8,6 +8,17 @@ const orderSeeder = require("../../helpers/order-seeder") const swapSeeder = require("../../helpers/swap-seeder") const adminSeeder = require("../../helpers/admin-seeder") +const { + simpleProductFactory, + simpleCartFactory, + simpleDiscountFactory, + simpleRegionFactory, + simpleShippingOptionFactory, +} = require("../../factories") +const { + simpleCustomerFactory, +} = require("../../factories/simple-customer-factory") + jest.setTimeout(30000) describe("/admin/swaps", () => { @@ -144,4 +155,208 @@ describe("/admin/swaps", () => { ) }) }) + + describe("Complete swap flow", () => { + beforeEach(async () => { + try { + await adminSeeder(dbConnection) + } catch (err) { + console.log(err) + throw err + } + }) + + afterEach(async () => { + const db = useDb() + await db.teardown() + }) + + it("completes swap and ensures difference due", async () => { + // ********* FACTORIES ********* + const prodA = await simpleProductFactory(dbConnection, { + id: "prod-a", + variants: [ + { id: "prod-a-var", prices: [{ amount: 1000, currency: "dkk" }] }, + ], + }) + + await simpleProductFactory(dbConnection, { + id: "prod-b", + variants: [ + { id: "prod-b-var", prices: [{ amount: 1000, currency: "dkk" }] }, + ], + }) + + await simpleRegionFactory(dbConnection, { + id: "test-region", + currency_code: "dkk", + }) + + await simpleDiscountFactory(dbConnection, { + id: "test-discount", + regions: ["test-region"], + code: "TEST", + rule: { + type: "percentage", + value: "10", + allocation: "total", + conditions: [ + { + type: "products", + operator: "in", + products: [prodA.id], + }, + ], + }, + }) + + await simpleCustomerFactory(dbConnection, { + id: "test-customer", + email: "test@customer.com", + }) + + const so = await simpleShippingOptionFactory(dbConnection, { + region_id: "test-region", + }) + + await simpleCartFactory(dbConnection, { + customer: "test-customer", + id: "cart-test", + line_items: [ + { + id: "line-item", + variant_id: "prod-a-var", + cart_id: "cart-test", + unit_price: 1000, + quantity: 1, + }, + ], + region: "test-region", + shipping_address: { + address_1: "test", + country_code: "us", + first_name: "chris", + last_name: "rock", + postal_code: "101", + }, + }) + + const api = useApi() + + // ********* PREPARE CART ********* + + try { + await api.post("/store/carts/cart-test", { + discounts: [{ code: "TEST" }], + }) + } catch (error) { + console.log(error) + } + + await api.post("/store/carts/cart-test/shipping-methods", { + option_id: so.id, + data: {}, + }) + await api.post("/store/carts/cart-test/payment-sessions") + const TEST = await api.post("/store/carts/cart-test/payment-session", { + provider_id: "test-pay", + }) + + console.log("Testing, ", TEST.data.cart.items[0]) + + // ********* COMPLETE CART ********* + const completedOrder = await api.post("/store/carts/cart-test/complete") + + // ********* PREPARE ORDER ********* + const orderId = completedOrder.data.data.id + const fulfilledOrder = await api.post( + `/admin/orders/${orderId}/fulfillment`, + { + items: [{ item_id: "line-item", quantity: 1 }], + }, + { + headers: { + Authorization: "Bearer test_token", + }, + } + ) + + const fulfillmentId = fulfilledOrder.data.order.fulfillments[0].id + + await api.post( + `/admin/orders/${orderId}/shipment`, + { + fulfillment_id: fulfillmentId, + }, + { + headers: { + Authorization: "Bearer test_token", + }, + } + ) + + await api.post( + `/admin/orders/${orderId}/capture`, + {}, + { + headers: { + Authorization: "Bearer test_token", + }, + } + ) + + // ********* CREATE SWAP ********* + const createSwap = await api.post( + `/admin/orders/${completedOrder.data.data.id}/swaps`, + { + return_items: [ + { + item_id: "line-item", + quantity: 1, + }, + ], + additional_items: [{ variant_id: "prod-b-var", quantity: 1 }], + }, + { + headers: { + authorization: "Bearer test_token", + }, + } + ) + + let swap = createSwap.data.order.swaps[0] + + // ********* PREPARE SWAP CART ********* + await api.post(`/store/carts/${swap.cart_id}/shipping-methods`, { + option_id: so.id, + data: {}, + }) + + await api.post(`/store/carts/${swap.cart_id}/payment-sessions`) + await api.post(`/store/carts/${swap.cart_id}/payment-session`, { + provider_id: "test-pay", + }) + + // ********* COMPLETE SWAP CART ********* + await api.post(`/store/carts/${swap.cart_id}/complete`) + + swap = await api + .get(`/admin/swaps/${swap.id}`, { + headers: { + Authorization: "Bearer test_token", + }, + }) + .catch((err) => { + console.log(err) + }) + + const swapCart = await api.get( + `/store/carts/${swap.data.swap.cart_id}`, + {} + ) + + // ********* VALIDATE ********* + expect(swap.data.swap.difference_due).toBe(swapCart.data.cart.total) + }) + }) }) diff --git a/integration-tests/api/__tests__/totals/orders.js b/integration-tests/api/__tests__/totals/orders.js index cd642a30b7..0551dc0fd4 100644 --- a/integration-tests/api/__tests__/totals/orders.js +++ b/integration-tests/api/__tests__/totals/orders.js @@ -30,6 +30,7 @@ describe("Order Totals", () => { medusaProcess = await setupServer({ cwd }) } catch (error) { console.log(error) + throw error } }) diff --git a/integration-tests/api/factories/simple-shipping-option-factory.ts b/integration-tests/api/factories/simple-shipping-option-factory.ts index c7a1dcbf07..a85d683e4b 100644 --- a/integration-tests/api/factories/simple-shipping-option-factory.ts +++ b/integration-tests/api/factories/simple-shipping-option-factory.ts @@ -1,11 +1,11 @@ -import { Connection } from "typeorm" -import faker from "faker" import { + ShippingOption, ShippingOptionPriceType, ShippingProfile, - ShippingOption, ShippingProfileType, } from "@medusajs/medusa" +import faker from "faker" +import { Connection } from "typeorm" export type ShippingOptionFactoryData = { name?: string diff --git a/integration-tests/api/package.json b/integration-tests/api/package.json index e63e33a033..2b19e220cf 100644 --- a/integration-tests/api/package.json +++ b/integration-tests/api/package.json @@ -8,16 +8,16 @@ "build": "babel src -d dist --extensions \".ts,.js\"" }, "dependencies": { - "@medusajs/medusa": "1.3.4-dev-1658251581042", + "@medusajs/medusa": "1.3.5-dev-1661328147668", "faker": "^5.5.3", - "medusa-interfaces": "1.3.1-dev-1658251581042", + "medusa-interfaces": "1.3.2-dev-1661328147668", "typeorm": "^0.2.31" }, "devDependencies": { "@babel/cli": "^7.12.10", "@babel/core": "^7.12.10", "@babel/node": "^7.12.10", - "babel-preset-medusa-package": "1.1.19-dev-1658251581042", + "babel-preset-medusa-package": "1.1.19-dev-1661328147668", "jest": "^26.6.3" } } diff --git a/integration-tests/api/yarn.lock b/integration-tests/api/yarn.lock index b9adc5d8e0..8b91d51a71 100644 --- a/integration-tests/api/yarn.lock +++ b/integration-tests/api/yarn.lock @@ -1825,9 +1825,9 @@ __metadata: languageName: node linkType: hard -"@medusajs/medusa-cli@npm:^1.3.1": - version: 1.3.1 - resolution: "@medusajs/medusa-cli@npm:1.3.1" +"@medusajs/medusa-cli@npm:1.3.1-dev-1661328147668": + version: 1.3.1-dev-1661328147668 + resolution: "@medusajs/medusa-cli@npm:1.3.1-dev-1661328147668" dependencies: "@babel/polyfill": ^7.8.7 "@babel/runtime": ^7.9.6 @@ -1845,8 +1845,8 @@ __metadata: is-valid-path: ^0.1.1 joi-objectid: ^3.0.1 meant: ^1.0.1 - medusa-core-utils: ^0.1.27 - medusa-telemetry: ^0.0.11 + medusa-core-utils: 1.1.31-dev-1661328147668 + medusa-telemetry: 0.0.11-dev-1661328147668 netrc-parser: ^3.1.6 open: ^8.0.6 ora: ^5.4.1 @@ -1861,16 +1861,16 @@ __metadata: yargs: ^15.3.1 bin: medusa: cli.js - checksum: a58f39cdfce3fd1361944323b600fddf34f79437b01d366f5d221e4cf93204a672abdbb2a901736387f13872f1ea868e08ccc7db33038e3156f1e7b663d9f1e5 + checksum: 6f8e1d3548d6a7b987011473a9913f95291e6bb01a38f11fac43903c8831e44cb3b6d04126d5718307063f0f60fa56d78a8f88f1180f26c1a1ebf5d2ae8f0d16 languageName: node linkType: hard -"@medusajs/medusa@npm:1.3.4-dev-1658251581042": - version: 1.3.4-dev-1658251581042 - resolution: "@medusajs/medusa@npm:1.3.4-dev-1658251581042" +"@medusajs/medusa@npm:1.3.5-dev-1661328147668": + version: 1.3.5-dev-1661328147668 + resolution: "@medusajs/medusa@npm:1.3.5-dev-1661328147668" dependencies: "@hapi/joi": ^16.1.8 - "@medusajs/medusa-cli": ^1.3.1 + "@medusajs/medusa-cli": 1.3.1-dev-1661328147668 "@types/lodash": ^4.14.168 awilix: ^4.2.3 body-parser: ^1.19.0 @@ -1893,8 +1893,8 @@ __metadata: joi: ^17.3.0 joi-objectid: ^3.0.1 jsonwebtoken: ^8.5.1 - medusa-core-utils: ^1.1.31 - medusa-test-utils: ^1.1.37 + medusa-core-utils: 1.1.31-dev-1661328147668 + medusa-test-utils: 1.1.37-dev-1661328147668 morgan: ^1.9.1 multer: ^1.4.2 node-schedule: ^2.1.0 @@ -1915,11 +1915,11 @@ __metadata: uuid: ^8.3.1 winston: ^3.2.1 peerDependencies: - medusa-interfaces: 1.x + medusa-interfaces: 1.3.2 typeorm: 0.2.x bin: medusa: cli.js - checksum: 4f33990f8dd1a454a3b2d6f27dad3b2f179d91376cadc339cb6f9435339d05621602f4a3a60772662561ea0c88781efabbb1fd3b6b7c54901ec597e91077e409 + checksum: 41a155c9486f18104e184987e7119c0255b6e15d264b94800474d44f8296e6835e159efe98c0c8b804219effd54c3f1b2783d7eac01d19dbde71996b1352c56b languageName: node linkType: hard @@ -2491,11 +2491,11 @@ __metadata: "@babel/cli": ^7.12.10 "@babel/core": ^7.12.10 "@babel/node": ^7.12.10 - "@medusajs/medusa": 1.3.4-dev-1658251581042 - babel-preset-medusa-package: 1.1.19-dev-1658251581042 + "@medusajs/medusa": 1.3.5-dev-1661328147668 + babel-preset-medusa-package: 1.1.19-dev-1661328147668 faker: ^5.5.3 jest: ^26.6.3 - medusa-interfaces: 1.3.1-dev-1658251581042 + medusa-interfaces: 1.3.2-dev-1661328147668 typeorm: ^0.2.31 languageName: unknown linkType: soft @@ -2802,9 +2802,9 @@ __metadata: languageName: node linkType: hard -"babel-preset-medusa-package@npm:1.1.19-dev-1658251581042": - version: 1.1.19-dev-1658251581042 - resolution: "babel-preset-medusa-package@npm:1.1.19-dev-1658251581042" +"babel-preset-medusa-package@npm:1.1.19-dev-1661328147668": + version: 1.1.19-dev-1661328147668 + resolution: "babel-preset-medusa-package@npm:1.1.19-dev-1661328147668" dependencies: "@babel/plugin-proposal-class-properties": ^7.12.1 "@babel/plugin-proposal-decorators": ^7.12.1 @@ -2818,7 +2818,7 @@ __metadata: core-js: ^3.7.0 peerDependencies: "@babel/core": ^7.11.6 - checksum: 55699a4aad97ed1da82a04bdfc75e16d051496ad734188e87d16dfabf1c237c829f42be054ad0a1f01d33a7943e804602f935a758b5e0a3af9f3bacffb9ec2cd + checksum: cf6bbf1400549e0641edbd2e67569bd81d3a8b611c7a56ce789784669fa77923c025d117c2105717dcc024104ad6fe14a006518368efa907c9a92fa5b09a11cb languageName: node linkType: hard @@ -6951,39 +6951,29 @@ __metadata: languageName: node linkType: hard -"medusa-core-utils@npm:^0.1.27": - version: 0.1.39 - resolution: "medusa-core-utils@npm:0.1.39" - dependencies: - "@hapi/joi": ^16.1.8 - joi-objectid: ^3.0.1 - checksum: 7c5d51de35e96312fd34e7c7b3b23cdcce197bbdad672234d766a44abe1b22cfccb28855d496b5e422558d72e87808e826ef2ef80189eaa45bdfba84942f2c8c - languageName: node - linkType: hard - -"medusa-core-utils@npm:^1.1.31, medusa-core-utils@npm:^1.1.32": - version: 1.1.32 - resolution: "medusa-core-utils@npm:1.1.32" +"medusa-core-utils@npm:1.1.31-dev-1661328147668": + version: 1.1.31-dev-1661328147668 + resolution: "medusa-core-utils@npm:1.1.31-dev-1661328147668" dependencies: joi: ^17.3.0 joi-objectid: ^3.0.1 - checksum: 6bbc326d58fcc6fb150c6fa464f3eaa87764bc3d43e4c861f1d318dd8ba1db8b6dff9e5b7624ba30d592fc870ec0857863bc07c4a8fdc12a99b668135f8cb883 + checksum: d61c4c089f8afaef3096b648d666eb41569c3d6e0bea8213fc86139c84870c836dc5f6c3fdd1d1f3031671b86ed61016227d6d090f7a9fa62b1c166198ce5bda languageName: node linkType: hard -"medusa-interfaces@npm:1.3.1-dev-1658251581042": - version: 1.3.1-dev-1658251581042 - resolution: "medusa-interfaces@npm:1.3.1-dev-1658251581042" +"medusa-interfaces@npm:1.3.2-dev-1661328147668": + version: 1.3.2-dev-1661328147668 + resolution: "medusa-interfaces@npm:1.3.2-dev-1661328147668" peerDependencies: medusa-core-utils: ^1.1.31 typeorm: 0.x - checksum: b6cbfa915233629779053cc70234c2f702c93351ed02c1230e21ad161eef8659676733f3ae12c34998d62f00296dd78a258d9b38ae0a5862cf1b8154f6f7bed3 + checksum: cc4cf53af7d5dd7fe19018c74a6d63789fd8d87696e367ffa7083d1dedd8b4a662cd6967117ede37a7a91186efcc76811c20fd1ebd4f33d23f748748027c951a languageName: node linkType: hard -"medusa-telemetry@npm:^0.0.11": - version: 0.0.11 - resolution: "medusa-telemetry@npm:0.0.11" +"medusa-telemetry@npm:0.0.11-dev-1661328147668": + version: 0.0.11-dev-1661328147668 + resolution: "medusa-telemetry@npm:0.0.11-dev-1661328147668" dependencies: axios: ^0.21.1 axios-retry: ^3.1.9 @@ -6994,18 +6984,18 @@ __metadata: is-docker: ^2.2.1 remove-trailing-slash: ^0.1.1 uuid: ^8.3.2 - checksum: f8223788eb2928b3c2bbfb29c32825216159aa062980717cc89e71904296b79907b99a514e17485436f0bd7e3b32dbc855589e8fa2cb1ecf5b75a1169ceef9d9 + checksum: 9dc2ff232dc20e49025a24d2b2aceb48294350b9a1c1f3e8042d636dd5250cbba0dd82c5c029b2c3dd43151f549898216709a176c907ac7b9c69e1a9c7a9ca6a languageName: node linkType: hard -"medusa-test-utils@npm:^1.1.37": - version: 1.1.38 - resolution: "medusa-test-utils@npm:1.1.38" +"medusa-test-utils@npm:1.1.37-dev-1661328147668": + version: 1.1.37-dev-1661328147668 + resolution: "medusa-test-utils@npm:1.1.37-dev-1661328147668" dependencies: "@babel/plugin-transform-classes": ^7.9.5 - medusa-core-utils: ^1.1.32 + medusa-core-utils: 1.1.31-dev-1661328147668 randomatic: ^3.1.1 - checksum: 613799b6bef71e857878b4b87efc6d19120cffc15171cebe116ec7b77050a3a5bfc2c53e35281d8177267281c3b10f55a0f4f5321bc098b897a3c21e978cdb4c + checksum: 23318eebf80e0b206935fa88a624638b6427fa524b5a268f0e0641089fcc702b0f631fd9a9aec631eea453e58752b1c879bef723cdb08170e30d1b8fe8b81d03 languageName: node linkType: hard diff --git a/packages/medusa/src/services/swap.js b/packages/medusa/src/services/swap.js index 58a7359508..657cdc1a58 100644 --- a/packages/medusa/src/services/swap.js +++ b/packages/medusa/src/services/swap.js @@ -184,8 +184,11 @@ class SwapService extends BaseService { const validatedId = this.validateId_(id) - const { cartSelects, cartRelations, ...newConfig } = - this.transformQueryForCart_(config) + const { + cartSelects, + cartRelations, + ...newConfig + } = this.transformQueryForCart_(config) const query = this.buildQuery_({ id: validatedId }, newConfig) @@ -600,8 +603,9 @@ class SwapService extends BaseService { }, }) - const customShippingOptionServiceTx = - this.customShippingOptionService_.withTransaction(manager) + const customShippingOptionServiceTx = this.customShippingOptionService_.withTransaction( + manager + ) for (const customShippingOption of customShippingOptions) { await customShippingOptionServiceTx.create({ cart_id: cart.id, @@ -611,8 +615,9 @@ class SwapService extends BaseService { } const lineItemServiceTx = this.lineItemService_.withTransaction(manager) - const lineItemAdjustmentServiceTx = - this.lineItemAdjustmentService_.withTransaction(manager) + const lineItemAdjustmentServiceTx = this.lineItemAdjustmentService_.withTransaction( + manager + ) for (const item of swap.additional_items) { await lineItemServiceTx.update(item.id, { cart_id: cart.id, @@ -691,7 +696,12 @@ class SwapService extends BaseService { const cart = await this.cartService_.retrieve(swap.cart_id, { select: ["total"], - relations: ["payment", "shipping_methods", "items"], + relations: [ + "payment", + "shipping_methods", + "items", + "items.adjustments", + ], }) const { payment } = cart @@ -699,10 +709,12 @@ class SwapService extends BaseService { const items = cart.items if (!swap.allow_backorder) { - const inventoryServiceTx = - this.inventoryService_.withTransaction(manager) - const paymentProviderServiceTx = - this.paymentProviderService_.withTransaction(manager) + const inventoryServiceTx = this.inventoryService_.withTransaction( + manager + ) + const paymentProviderServiceTx = this.paymentProviderService_.withTransaction( + manager + ) const cartServiceTx = this.cartService_.withTransaction(manager) for (const item of items) { @@ -750,8 +762,9 @@ class SwapService extends BaseService { order_id: swap.order_id, }) - const inventoryServiceTx = - this.inventoryService_.withTransaction(manager) + const inventoryServiceTx = this.inventoryService_.withTransaction( + manager + ) for (const item of items) { await inventoryServiceTx.adjustInventory( @@ -771,8 +784,9 @@ class SwapService extends BaseService { const swapRepo = manager.getCustomRepository(this.swapRepository_) const result = await swapRepo.save(swap) - const shippingOptionServiceTx = - this.shippingOptionService_.withTransaction(manager) + const shippingOptionServiceTx = this.shippingOptionService_.withTransaction( + manager + ) for (const method of cart.shipping_methods) { await shippingOptionServiceTx.updateShippingMethod(method.id, {