From de6f61b05f184119dcc44e8bf1b1eaa3d092982e Mon Sep 17 00:00:00 2001 From: Riqwan Thamir Date: Wed, 4 Sep 2024 15:11:04 +0200 Subject: [PATCH] feat: added totals tests for end 2 end RMA flow (#8906) what: I've added some specs and comments in the specs for where I think totals are incorrect. Lets get this test merged in as the status quo and proceed to fix these issues one by one. Additionally, this also removes the temporary_difference as its not something we use. RESOLVES CC-346 Co-authored-by: Carlos R. L. Rodrigues <37986729+carlos-r-l-rodrigues@users.noreply.github.com> --- .../http/__tests__/claims/claims.spec.ts | 2 - .../http/__tests__/fixtures/order.ts | 5 +- .../__tests__/order/admin/rma-flows.spec.ts | 670 ++++++++++++++++++ .../__tests__/payment/admin/payment.spec.ts | 58 +- .../__tests__/util/actions/exchanges.ts | 1 - .../modules/order/src/types/utils/index.ts | 2 - .../order/src/utils/actions/return-item.ts | 1 - .../order/src/utils/calculate-order-change.ts | 14 - 8 files changed, 673 insertions(+), 80 deletions(-) create mode 100644 integration-tests/http/__tests__/order/admin/rma-flows.spec.ts diff --git a/integration-tests/http/__tests__/claims/claims.spec.ts b/integration-tests/http/__tests__/claims/claims.spec.ts index a9ca0965b4..0eeb158b07 100644 --- a/integration-tests/http/__tests__/claims/claims.spec.ts +++ b/integration-tests/http/__tests__/claims/claims.spec.ts @@ -437,7 +437,6 @@ medusaIntegrationTestRunner({ pending_difference: 0, current_order_total: 61, original_order_total: 61, - temporary_difference: 0, }) ) @@ -1106,7 +1105,6 @@ medusaIntegrationTestRunner({ pending_difference: -11, current_order_total: 50, original_order_total: 60, - temporary_difference: 15, }) ) diff --git a/integration-tests/http/__tests__/fixtures/order.ts b/integration-tests/http/__tests__/fixtures/order.ts index bc857f8491..cc67541fba 100644 --- a/integration-tests/http/__tests__/fixtures/order.ts +++ b/integration-tests/http/__tests__/fixtures/order.ts @@ -28,9 +28,7 @@ export async function createOrderSeeder({ api }) { const inventoryItem = ( await api.post( `/admin/inventory-items`, - { - sku: `12345-${stockLocation.id}`, - }, + { sku: "test-variant" }, adminHeaders ) ).data.inventory_item @@ -70,6 +68,7 @@ export async function createOrderSeeder({ api }) { variants: [ { title: "Test variant", + sku: "test-variant", inventory_items: [ { inventory_item_id: inventoryItem.id, diff --git a/integration-tests/http/__tests__/order/admin/rma-flows.spec.ts b/integration-tests/http/__tests__/order/admin/rma-flows.spec.ts new file mode 100644 index 0000000000..c79c5fe1fb --- /dev/null +++ b/integration-tests/http/__tests__/order/admin/rma-flows.spec.ts @@ -0,0 +1,670 @@ +import { + ClaimType, + ModuleRegistrationName, + RuleOperator, +} from "@medusajs/utils" +import { medusaIntegrationTestRunner } from "medusa-test-utils" +import { + adminHeaders, + createAdminUser, +} from "../../../../helpers/create-admin-user" +import { setupTaxStructure } from "../../../../modules/__tests__/fixtures" +import { createOrderSeeder } from "../../fixtures/order" + +jest.setTimeout(30000) + +medusaIntegrationTestRunner({ + testSuite: ({ dbConnection, getContainer, api }) => { + let order + let shippingProfile + let fulfillmentSet + let location + let item + let returnShippingOption + const shippingProviderId = "manual_test-provider" + + beforeEach(async () => { + const container = getContainer() + + await setupTaxStructure(container.resolve(ModuleRegistrationName.TAX)) + await createAdminUser(dbConnection, adminHeaders, container) + order = await createOrderSeeder({ api }) + + shippingProfile = ( + await api.post( + `/admin/shipping-profiles`, + { name: "Test", type: "default" }, + adminHeaders + ) + ).data.shipping_profile + + location = ( + await api.post( + `/admin/stock-locations`, + { name: "Test location" }, + adminHeaders + ) + ).data.stock_location + + location = ( + await api.post( + `/admin/stock-locations/${location.id}/fulfillment-sets?fields=*fulfillment_sets`, + { name: "Test", type: "test-type" }, + adminHeaders + ) + ).data.stock_location + + fulfillmentSet = ( + await api.post( + `/admin/fulfillment-sets/${location.fulfillment_sets[0].id}/service-zones`, + { + name: "Test", + geo_zones: [{ type: "country", country_code: "us" }], + }, + adminHeaders + ) + ).data.fulfillment_set + + const inventoryItem = ( + await api.get(`/admin/inventory-items?sku=test-variant`, adminHeaders) + ).data.inventory_items[0] + + await api.post( + `/admin/inventory-items/${inventoryItem.id}/location-levels`, + { + location_id: location.id, + stocked_quantity: 10, + }, + adminHeaders + ) + + await api.post( + `/admin/stock-locations/${location.id}/fulfillment-providers`, + { add: [shippingProviderId] }, + adminHeaders + ) + + const shippingOptionPayload = { + name: "Return shipping", + service_zone_id: fulfillmentSet.service_zones[0].id, + shipping_profile_id: shippingProfile.id, + provider_id: shippingProviderId, + price_type: "flat", + type: { + label: "Test type", + description: "Test description", + code: "test-code", + }, + prices: [ + { + currency_code: "usd", + amount: 15, + }, + ], + rules: [ + { + operator: RuleOperator.EQ, + attribute: "is_return", + value: "true", + }, + ], + } + + const outboundShippingOptionPayload = { + name: "Oubound shipping", + service_zone_id: fulfillmentSet.service_zones[0].id, + shipping_profile_id: shippingProfile.id, + provider_id: shippingProviderId, + price_type: "flat", + type: { + label: "Test type", + description: "Test description", + code: "test-code", + }, + prices: [ + { + currency_code: "usd", + amount: 20, + }, + ], + rules: [ + { + operator: RuleOperator.EQ, + attribute: "is_return", + value: "false", + }, + { + operator: RuleOperator.EQ, + attribute: "enabled_in_store", + value: "true", + }, + ], + } + + returnShippingOption = ( + await api.post( + "/admin/shipping-options", + shippingOptionPayload, + adminHeaders + ) + ).data.shipping_option + + const outboundShippingOption = ( + await api.post( + "/admin/shipping-options", + outboundShippingOptionPayload, + adminHeaders + ) + ).data.shipping_option + + item = order.items[0] + }) + + describe("RMA Flows", () => { + it.only("should verify order summary at each level", async () => { + /* Case: + Purchased: + items: { + unit_price: 25, + qty: 2 + tax_total: 0 + total: 50 + } + shipping_methods: { + unit_price: 10, + qty: 1 + tax_total: 1 + total: 11 + } + */ + + // Fulfill any existing items + let orderResult = ( + await api.get(`/admin/orders/${order.id}`, adminHeaders) + ).data.order + + let fulfillableItem = orderResult.items.find( + (item) => item.detail.fulfilled_quantity < item.detail.quantity + ) + + await api.post( + `/admin/orders/${order.id}/fulfillments`, + { + location_id: location.id, + items: [ + { + id: fulfillableItem.id, + quantity: item.detail.quantity - item.detail.fulfilled_quantity, + }, + ], + }, + adminHeaders + ) + + orderResult = (await api.get(`/admin/orders/${order.id}`, adminHeaders)) + .data.order + + fulfillableItem = orderResult.items.find( + (item) => item.detail.fulfilled_quantity < item.detail.quantity + ) + + // Ensure that there are no more fulfillable items + expect(fulfillableItem).toBeUndefined() + + orderResult = (await api.get(`/admin/orders/${order.id}`, adminHeaders)) + .data.order + + expect(orderResult).toEqual( + expect.objectContaining({ + total: 106, + subtotal: 100, + tax_total: 6, + summary: expect.objectContaining({ + paid_total: 0, + difference_sum: 0, + refunded_total: 0, + transaction_total: 0, + pending_difference: 106, + current_order_total: 106, + original_order_total: 106, + }), + }) + ) + + /* + Create a claim with a single outbound item + */ + const singleOutboundClaim = ( + await api.post( + "/admin/claims", + { + order_id: order.id, + type: ClaimType.REPLACE, + description: "Base claim", + }, + adminHeaders + ) + ).data.claim + + orderResult = (await api.get(`/admin/orders/${order.id}`, adminHeaders)) + .data.order + + expect(orderResult).toEqual( + expect.objectContaining({ + total: 106, + subtotal: 100, + tax_total: 6, + summary: expect.objectContaining({ + paid_total: 0, + difference_sum: 0, + refunded_total: 0, + transaction_total: 0, + pending_difference: 106, + current_order_total: 106, + original_order_total: 106, + }), + }) + ) + + await api.post( + `/admin/claims/${singleOutboundClaim.id}/outbound/items`, + { + items: [ + { + variant_id: order.items[0].variant_id, + quantity: 1, + }, + ], + }, + adminHeaders + ) + + await api.post( + `/admin/claims/${singleOutboundClaim.id}/request`, + {}, + adminHeaders + ) + + orderResult = (await api.get(`/admin/orders/${order.id}`, adminHeaders)) + .data.order + + // After confirming a claim with an outbound item, the tax totals are not updated + // I suspect this will be the same for promotions and discount totals + // Additionally, the items during claim don't have taxes included in them. + // TODO: this needs to be fixed + expect(orderResult).toEqual( + expect.objectContaining({ + total: 206, + subtotal: 200, + tax_total: 6, + summary: expect.objectContaining({ + paid_total: 0, + difference_sum: 100, + refunded_total: 0, + transaction_total: 0, + pending_difference: 200, + // TODO: I think the current_order_total and original_order_total should include taxes and adjustments as well + current_order_total: 200, + original_order_total: 100, + }), + }) + ) + + let pendingPaymentCollection = orderResult.payment_collections.find( + (pc) => pc.status === "not_paid" + ) + + expect(pendingPaymentCollection).toEqual( + expect.objectContaining({ + status: "not_paid", + // TODO: The payment should also include taxes + amount: 200, + }) + ) + + let paymentCollection = ( + await api.post( + `/admin/payment-collections/${pendingPaymentCollection.id}/mark-as-paid`, + { order_id: order.id }, + adminHeaders + ) + ).data.payment_collection + + // TODO: The payment, payment sessions and collection should also include taxes + expect(paymentCollection).toEqual( + expect.objectContaining({ + amount: 200, + // Q: Shouldn't this be paid? + status: "authorized", + payment_sessions: [ + expect.objectContaining({ + status: "authorized", + amount: 200, + }), + ], + payments: [ + expect.objectContaining({ + provider_id: "pp_system_default", + amount: 200, + }), + ], + }) + ) + + orderResult = (await api.get(`/admin/orders/${order.id}`, adminHeaders)) + .data.order + + // Totals summary after payment has been marked as paid + expect(orderResult).toEqual( + expect.objectContaining({ + total: 206, + subtotal: 200, + tax_total: 6, + summary: expect.objectContaining({ + // TODO: Paid total should include taxes + paid_total: 200, + // TODO: difference_sum should include taxes + difference_sum: 100, + refunded_total: 0, + // TODO: difference_sum should include taxes + transaction_total: 200, + pending_difference: 0, + // TODO: difference_sum should include taxes + current_order_total: 200, + // TODO: difference_sum should include taxes + original_order_total: 100, + }), + }) + ) + + fulfillableItem = orderResult.items.find( + (item) => item.detail.fulfilled_quantity < item.detail.quantity + ) + + await api.post( + `/admin/orders/${order.id}/fulfillments`, + { + location_id: location.id, + items: [ + { + id: fulfillableItem.id, + quantity: item.detail.quantity - item.detail.fulfilled_quantity, + }, + ], + }, + adminHeaders + ) + + orderResult = (await api.get(`/admin/orders/${order.id}`, adminHeaders)) + .data.order + + fulfillableItem = orderResult.items.find( + (item) => item.detail.fulfilled_quantity < item.detail.quantity + ) + + // Ensure that there are no more fulfillable items + expect(fulfillableItem).toBeUndefined() + + // After fulfillment, the taxes seems to now be considered. + // The case now is that you need to now request additional payment from + // the customer + // TODO: This shouldn't be a surprise during fulfillment + expect(orderResult).toEqual( + expect.objectContaining({ + total: 206, + subtotal: 200, + tax_total: 6, + summary: expect.objectContaining({ + paid_total: 200, + difference_sum: 0, + refunded_total: 0, + transaction_total: 200, + pending_difference: 6, + current_order_total: 206, + original_order_total: 206, + }), + }) + ) + + /* + We can see that fulfillment affects the totals, so lets create and confirm 2 claims + and fulfill after + */ + + let claimWithInboundAndOutbound = ( + await api.post( + "/admin/claims", + { + order_id: order.id, + type: ClaimType.REPLACE, + description: "Base claim", + }, + adminHeaders + ) + ).data.claim + + orderResult = (await api.get(`/admin/orders/${order.id}`, adminHeaders)) + .data.order + + // Nothing changes from the previous expectation + expect(orderResult).toEqual( + expect.objectContaining({ + total: 206, + subtotal: 200, + tax_total: 6, + summary: expect.objectContaining({ + paid_total: 200, + difference_sum: 0, + refunded_total: 0, + transaction_total: 200, + pending_difference: 6, + current_order_total: 206, + original_order_total: 206, + }), + }) + ) + + let inboundItem = orderResult.items[0] + + await api.post( + `/admin/claims/${claimWithInboundAndOutbound.id}/inbound/items`, + { items: [{ id: inboundItem.id, quantity: 1 }] }, + adminHeaders + ) + + await api.post( + `/admin/claims/${claimWithInboundAndOutbound.id}/inbound/shipping-method`, + { shipping_option_id: returnShippingOption.id }, + adminHeaders + ) + + await api.post( + `/admin/claims/${claimWithInboundAndOutbound.id}/outbound/items`, + { + items: [ + { + variant_id: order.items[0].variant_id, + quantity: 1, + }, + ], + }, + adminHeaders + ) + + await api.post( + `/admin/claims/${claimWithInboundAndOutbound.id}/request`, + {}, + adminHeaders + ) + + orderResult = (await api.get(`/admin/orders/${order.id}`, adminHeaders)) + .data.order + + // Totals summary after all + expect(orderResult).toEqual( + expect.objectContaining({ + // This now adds a shipping_tax_total, but the item_tax_total hasn't been updated + total: 321.9, + subtotal: 315, + tax_total: 6.9, + summary: expect.objectContaining({ + paid_total: 200, + difference_sum: 15, + refunded_total: 0, + transaction_total: 200, + // TODO: what happened to the previous pending difference of 6 + pending_difference: 15, + current_order_total: 215, + original_order_total: 200, + }), + }) + ) + + // Lets create one more claim without fulfilling the previous one + claimWithInboundAndOutbound = ( + await api.post( + "/admin/claims", + { + order_id: order.id, + type: ClaimType.REPLACE, + description: "Base claim", + }, + adminHeaders + ) + ).data.claim + + orderResult = (await api.get(`/admin/orders/${order.id}`, adminHeaders)) + .data.order + + // Nothing changes from the previous expectation + expect(orderResult).toEqual( + expect.objectContaining({ + total: 321.9, + subtotal: 315, + tax_total: 6.9, + summary: expect.objectContaining({ + paid_total: 200, + difference_sum: 15, + refunded_total: 0, + transaction_total: 200, + pending_difference: 15, + current_order_total: 215, + original_order_total: 200, + }), + }) + ) + + inboundItem = orderResult.items[0] + + await api.post( + `/admin/claims/${claimWithInboundAndOutbound.id}/inbound/items`, + { items: [{ id: inboundItem.id, quantity: 1 }] }, + adminHeaders + ) + + await api.post( + `/admin/claims/${claimWithInboundAndOutbound.id}/outbound/items`, + { + items: [ + { + variant_id: order.items[0].variant_id, + quantity: 1, + }, + ], + }, + adminHeaders + ) + + await api.post( + `/admin/claims/${claimWithInboundAndOutbound.id}/request`, + {}, + adminHeaders + ) + + orderResult = (await api.get(`/admin/orders/${order.id}`, adminHeaders)) + .data.order + + expect(orderResult).toEqual( + expect.objectContaining({ + total: 421.9, + subtotal: 415, + tax_total: 6.9, + summary: expect.objectContaining({ + paid_total: 200, + difference_sum: 0, + refunded_total: 0, + transaction_total: 200, + // TODO: Tax totals seems to be added after every claim confirmation as well + pending_difference: 115.9, + current_order_total: 315.9, + original_order_total: 315.9, + }), + }) + ) + + pendingPaymentCollection = orderResult.payment_collections.find( + (pc) => pc.status === "not_paid" + ) + + expect(pendingPaymentCollection).toEqual( + expect.objectContaining({ + status: "not_paid", + // TODO: The payment should also include taxes + amount: 115.9, + }) + ) + + paymentCollection = ( + await api.post( + `/admin/payment-collections/${pendingPaymentCollection.id}/mark-as-paid`, + { order_id: order.id }, + adminHeaders + ) + ).data.payment_collection + + // TODO: The payment, payment sessions and collection should also include taxes + expect(paymentCollection).toEqual( + expect.objectContaining({ + amount: 115.9, + // Q: Shouldn't this be paid? + status: "authorized", + payment_sessions: [ + expect.objectContaining({ + status: "authorized", + amount: 115.9, + }), + ], + payments: [ + expect.objectContaining({ + provider_id: "pp_system_default", + amount: 115.9, + }), + ], + }) + ) + + orderResult = (await api.get(`/admin/orders/${order.id}`, adminHeaders)) + .data.order + + // Totals summary after payment has been marked as paid + // TODO: There is a discrepancy between total and paid_total + expect(orderResult).toEqual( + expect.objectContaining({ + total: 421.9, + subtotal: 415, + tax_total: 6.9, + summary: expect.objectContaining({ + paid_total: 315.9, + difference_sum: 0, + refunded_total: 0, + transaction_total: 315.9, + pending_difference: 0, + current_order_total: 315.9, + original_order_total: 315.9, + }), + }) + ) + }) + }) + }, +}) diff --git a/integration-tests/http/__tests__/payment/admin/payment.spec.ts b/integration-tests/http/__tests__/payment/admin/payment.spec.ts index f52a5a78a0..a40904c4c8 100644 --- a/integration-tests/http/__tests__/payment/admin/payment.spec.ts +++ b/integration-tests/http/__tests__/payment/admin/payment.spec.ts @@ -1,7 +1,5 @@ -import { ClaimType, ModuleRegistrationName } from "@medusajs/utils" +import { ClaimType } from "@medusajs/utils" import { adminHeaders } from "../../../../helpers/create-admin-user" -import { seedStorefrontDefaults } from "../../../../helpers/seed-storefront-defaults" -import { setupTaxStructure } from "../../../../modules/__tests__/fixtures" import { medusaIntegrationTestRunner } from "medusa-test-utils" import { createAdminUser } from "../../../../helpers/create-admin-user" @@ -228,60 +226,6 @@ medusaIntegrationTestRunner({ "Cannot refund more than pending difference - 75" ) }) - - it("should not update payment collection of other orders", async () => { - await setupTaxStructure(container.resolve(ModuleRegistrationName.TAX)) - await seedStorefrontDefaults(container, "dkk") - - let order1 = await createOrderSeeder({ api }) - - expect(order1).toEqual( - expect.objectContaining({ - id: expect.any(String), - payment_status: "authorized", - }) - ) - - const order1Payment = order1.payment_collections[0].payments[0] - - await api.post( - `/admin/payments/${order1Payment.id}/capture?fields=*payment_collection`, - { amount: order1Payment.amount }, - adminHeaders - ) - - order1 = (await api.get(`/admin/orders/${order1.id}`, adminHeaders)) - .data.order - - expect(order1).toEqual( - expect.objectContaining({ - id: order1.id, - payment_status: "captured", - }) - ) - - let order2 = await createOrderSeeder({ api }) - - order2 = (await api.get(`/admin/orders/${order2.id}`, adminHeaders)) - .data.order - - expect(order2).toEqual( - expect.objectContaining({ - id: expect.any(String), - payment_status: "authorized", - }) - ) - - order1 = (await api.get(`/admin/orders/${order1.id}`, adminHeaders)) - .data.order - - expect(order1).toEqual( - expect.objectContaining({ - id: expect.any(String), - payment_status: "captured", - }) - ) - }) }) it("should throw if outstanding amount is not present", async () => { diff --git a/packages/modules/order/src/services/__tests__/util/actions/exchanges.ts b/packages/modules/order/src/services/__tests__/util/actions/exchanges.ts index feb9fd7fd8..541809b230 100644 --- a/packages/modules/order/src/services/__tests__/util/actions/exchanges.ts +++ b/packages/modules/order/src/services/__tests__/util/actions/exchanges.ts @@ -103,7 +103,6 @@ describe("Order Exchange - Actions", function () { transaction_total: 0, original_order_total: 270, current_order_total: 312.5, - temporary_difference: 62.5, pending_difference: 312.5, difference_sum: 42.5, paid_total: 0, diff --git a/packages/modules/order/src/types/utils/index.ts b/packages/modules/order/src/types/utils/index.ts index a39e06ced0..7b82fccb9a 100644 --- a/packages/modules/order/src/types/utils/index.ts +++ b/packages/modules/order/src/types/utils/index.ts @@ -66,7 +66,6 @@ export interface OrderSummaryCalculated { original_order_total: BigNumberInput transaction_total: BigNumberInput pending_difference: BigNumberInput - temporary_difference: BigNumberInput difference_sum: BigNumberInput paid_total: BigNumberInput refunded_total: BigNumberInput @@ -113,7 +112,6 @@ export type OrderReferences = { export interface ActionTypeDefinition { isDeduction?: boolean - awaitRequired?: boolean operation?: (obj: OrderReferences) => BigNumberInput | void validate?: (obj: OrderReferences) => void [key: string]: unknown diff --git a/packages/modules/order/src/utils/actions/return-item.ts b/packages/modules/order/src/utils/actions/return-item.ts index f09bd028f1..031a20f9d0 100644 --- a/packages/modules/order/src/utils/actions/return-item.ts +++ b/packages/modules/order/src/utils/actions/return-item.ts @@ -4,7 +4,6 @@ import { setActionReference } from "../set-action-reference" OrderChangeProcessing.registerActionType(ChangeActionType.RETURN_ITEM, { isDeduction: true, - awaitRequired: true, operation({ action, currentOrder, options }) { const existing = currentOrder.items.find( (item) => item.id === action.details.reference_id diff --git a/packages/modules/order/src/utils/calculate-order-change.ts b/packages/modules/order/src/utils/calculate-order-change.ts index 4552c3579e..8dd3b36cc7 100644 --- a/packages/modules/order/src/utils/calculate-order-change.ts +++ b/packages/modules/order/src/utils/calculate-order-change.ts @@ -71,7 +71,6 @@ export class OrderChangeProcessing { transformPropertiesToBigNumber(this.order.metadata) this.summary = { - temporary_difference: 0, pending_difference: 0, difference_sum: 0, current_order_total: this.order.total ?? 0, @@ -121,13 +120,6 @@ export class OrderChangeProcessing { ) } - if (type.awaitRequired) { - summary.temporary_difference = MathBN.add( - summary.temporary_difference, - amount - ) - } - if (!this.isEventDone(action) && !action.change_id) { summary.difference_sum = MathBN.add(summary.difference_sum, amount) } @@ -145,11 +137,6 @@ export class OrderChangeProcessing { ...this.transactions.map((tr) => tr.amount) ) - summary.temporary_difference = MathBN.sub( - summary.difference_sum, - summary.temporary_difference - ) - summary.pending_difference = MathBN.sub( summary.current_order_total, summary.transaction_total @@ -208,7 +195,6 @@ export class OrderChangeProcessing { transaction_total: new BigNumber(summary.transaction_total), original_order_total: new BigNumber(summary.original_order_total), current_order_total: new BigNumber(summary.current_order_total), - temporary_difference: new BigNumber(summary.temporary_difference), pending_difference: new BigNumber(summary.pending_difference), difference_sum: new BigNumber(summary.difference_sum), paid_total: new BigNumber(summary.paid_total),