From 7f6dc44beb9789088f6d6b796f7545beb3653094 Mon Sep 17 00:00:00 2001 From: Oliver Windall Juhl <59018053+olivermrbl@users.noreply.github.com> Date: Tue, 11 Apr 2023 11:40:13 +0200 Subject: [PATCH] fix(medusa): Add totals when retrieving order by cart id (#3777) * fix(medusa): Add totals when retrieving order by cart id * fix: Unit test * Create .changeset/stupid-rockets-smile.md --- .changeset/stupid-rockets-smile.md | 5 ++ .../api/__tests__/store/orders.js | 51 +++++++++++++++++++ .../api/factories/simple-order-factory.ts | 34 ++++++------- .../orders/__tests__/get-order-by-cart.js | 6 ++- .../routes/store/orders/get-order-by-cart.ts | 11 ++-- .../src/api/routes/store/orders/index.ts | 6 +-- .../medusa/src/services/__mocks__/order.js | 3 ++ .../medusa/src/services/__tests__/order.js | 21 +++++--- packages/medusa/src/services/order.ts | 48 +++++++++-------- 9 files changed, 129 insertions(+), 56 deletions(-) create mode 100644 .changeset/stupid-rockets-smile.md diff --git a/.changeset/stupid-rockets-smile.md b/.changeset/stupid-rockets-smile.md new file mode 100644 index 0000000000..52cd33739e --- /dev/null +++ b/.changeset/stupid-rockets-smile.md @@ -0,0 +1,5 @@ +--- +"@medusajs/medusa": patch +--- + +fix(medusa): Add totals when retrieving order by cart id diff --git a/integration-tests/api/__tests__/store/orders.js b/integration-tests/api/__tests__/store/orders.js index 683d346331..b39fe6d3b9 100644 --- a/integration-tests/api/__tests__/store/orders.js +++ b/integration-tests/api/__tests__/store/orders.js @@ -19,6 +19,7 @@ const { simpleProductFactory, simpleCartFactory, simpleShippingOptionFactory, + simpleOrderFactory, } = require("../../factories") const { MedusaError } = require("medusa-core-utils") @@ -157,6 +158,56 @@ describe("/store/carts", () => { ) }) + it("retrieves an order by cart id, with totals", async () => { + const api = useApi() + + const region = await simpleRegionFactory(dbConnection) + const product = await simpleProductFactory(dbConnection) + + const cartRes = await api.post("/store/carts", { + region_id: region.id, + }) + + const cartId = cartRes.data.cart.id + + await api.post(`/store/carts/${cartId}/line-items`, { + variant_id: product.variants[0].id, + quantity: 1, + }) + + await api.post(`/store/carts/${cartId}`, { + email: "testmailer@medusajs.com", + }) + + await api.post(`/store/carts/${cartId}/payment-sessions`).catch((err) => { + console.error("Error creating payment session: ", err.response.data) + return err.response + }) + + const responseSuccess = await api.post(`/store/carts/${cartId}/complete`) + + const orderId = responseSuccess.data.data.id + + const response = await api.get("/store/orders/cart/" + cartId) + + expect(response.status).toEqual(200) + expect(response.data.order).toEqual( + expect.objectContaining({ + id: orderId, + cart_id: cartId, + total: 100, + gift_card_total: 0, + gift_card_tax_total: 0, + tax_total: 0, + subtotal: 100, + discount_total: 0, + shipping_total: 0, + refunded_total: 0, + paid_total: 100, + }) + ) + }) + it("lookup order response contains only fields defined with `fields` param", async () => { const api = useApi() diff --git a/integration-tests/api/factories/simple-order-factory.ts b/integration-tests/api/factories/simple-order-factory.ts index 66e549da94..9ff96e4512 100644 --- a/integration-tests/api/factories/simple-order-factory.ts +++ b/integration-tests/api/factories/simple-order-factory.ts @@ -1,5 +1,3 @@ -import { DataSource } from "typeorm" -import faker from "faker" import { Discount, FulfillmentStatus, @@ -7,31 +5,33 @@ import { PaymentStatus, Refund, } from "@medusajs/medusa" -import { - DiscountFactoryData, - simpleDiscountFactory, -} from "./simple-discount-factory" -import { RegionFactoryData, simpleRegionFactory } from "./simple-region-factory" -import { - LineItemFactoryData, - simpleLineItemFactory, -} from "./simple-line-item-factory" +import faker from "faker" +import { DataSource } from "typeorm" import { AddressFactoryData, simpleAddressFactory, } from "./simple-address-factory" import { - ShippingMethodFactoryData, - simpleShippingMethodFactory, -} from "./simple-shipping-method-factory" + CustomerFactoryData, + simpleCustomerFactory, +} from "./simple-customer-factory" +import { + DiscountFactoryData, + simpleDiscountFactory, +} from "./simple-discount-factory" +import { + LineItemFactoryData, + simpleLineItemFactory, +} from "./simple-line-item-factory" +import { RegionFactoryData, simpleRegionFactory } from "./simple-region-factory" import { SalesChannelFactoryData, simpleSalesChannelFactory, } from "./simple-sales-channel-factory" import { - CustomerFactoryData, - simpleCustomerFactory, -} from "./simple-customer-factory" + ShippingMethodFactoryData, + simpleShippingMethodFactory, +} from "./simple-shipping-method-factory" export type OrderFactoryData = { id?: string diff --git a/packages/medusa/src/api/routes/store/orders/__tests__/get-order-by-cart.js b/packages/medusa/src/api/routes/store/orders/__tests__/get-order-by-cart.js index d0955d3005..37bc33cf59 100644 --- a/packages/medusa/src/api/routes/store/orders/__tests__/get-order-by-cart.js +++ b/packages/medusa/src/api/routes/store/orders/__tests__/get-order-by-cart.js @@ -19,8 +19,10 @@ describe("GET /store/orders", () => { }) it("calls orderService retrieve", () => { - expect(OrderServiceMock.retrieveByCartId).toHaveBeenCalledTimes(1) - expect(OrderServiceMock.retrieveByCartId).toHaveBeenCalledWith( + expect(OrderServiceMock.retrieveByCartIdWithTotals).toHaveBeenCalledTimes( + 1 + ) + expect(OrderServiceMock.retrieveByCartIdWithTotals).toHaveBeenCalledWith( IdMap.getId("test-cart"), { select: defaultStoreOrdersFields, diff --git a/packages/medusa/src/api/routes/store/orders/get-order-by-cart.ts b/packages/medusa/src/api/routes/store/orders/get-order-by-cart.ts index fb866958ea..72634d9380 100644 --- a/packages/medusa/src/api/routes/store/orders/get-order-by-cart.ts +++ b/packages/medusa/src/api/routes/store/orders/get-order-by-cart.ts @@ -1,5 +1,3 @@ -import { defaultStoreOrdersFields, defaultStoreOrdersRelations } from "." - import { OrderService } from "../../../../services" import { cleanResponseData } from "../../../../utils/clean-response-data" @@ -50,10 +48,11 @@ export default async (req, res) => { const { cart_id } = req.params const orderService: OrderService = req.scope.resolve("orderService") - const order = await orderService.retrieveByCartId(cart_id, { - select: defaultStoreOrdersFields, - relations: defaultStoreOrdersRelations, - }) + + const order = await orderService.retrieveByCartIdWithTotals( + cart_id, + req.retrieveConfig + ) res.json({ order: cleanResponseData(order, []) }) } diff --git a/packages/medusa/src/api/routes/store/orders/index.ts b/packages/medusa/src/api/routes/store/orders/index.ts index adb510e744..6e8a3830a1 100644 --- a/packages/medusa/src/api/routes/store/orders/index.ts +++ b/packages/medusa/src/api/routes/store/orders/index.ts @@ -1,16 +1,16 @@ import { Router } from "express" import "reflect-metadata" import { Order } from "../../../.." +import { FindParams } from "../../../../types/common" import middlewares, { transformBody, transformStoreQuery, } from "../../../middlewares" import requireCustomerAuthentication from "../../../middlewares/require-customer-authentication" -import { StorePostCustomersCustomerOrderClaimReq } from "./request-order" import { StorePostCustomersCustomerAcceptClaimReq } from "./confirm-order-request" import { StoreGetOrderParams } from "./get-order" import { StoreGetOrdersParams } from "./lookup-order" -import { FindParams } from "../../../../types/common" +import { StorePostCustomersCustomerOrderClaimReq } from "./request-order" const route = Router() @@ -219,6 +219,6 @@ export type StoreOrdersRes = { order: Order } -export * from "./lookup-order" export * from "./confirm-order-request" +export * from "./lookup-order" export * from "./request-order" diff --git a/packages/medusa/src/services/__mocks__/order.js b/packages/medusa/src/services/__mocks__/order.js index 8c3980ecaf..0aa252803a 100644 --- a/packages/medusa/src/services/__mocks__/order.js +++ b/packages/medusa/src/services/__mocks__/order.js @@ -187,6 +187,9 @@ export const OrderServiceMock = { retrieveByCartId: jest.fn().mockImplementation((cartId) => { return Promise.resolve({ id: IdMap.getId("test-order") }) }), + retrieveByCartIdWithTotals: jest.fn().mockImplementation((cartId) => { + return Promise.resolve({ id: IdMap.getId("test-order") }) + }), decorate: jest.fn().mockImplementation((order) => { order.decorated = true return order diff --git a/packages/medusa/src/services/__tests__/order.js b/packages/medusa/src/services/__tests__/order.js index 186f1f1273..c93fdc21ab 100644 --- a/packages/medusa/src/services/__tests__/order.js +++ b/packages/medusa/src/services/__tests__/order.js @@ -1,9 +1,9 @@ import { IdMap, MockManager, MockRepository } from "medusa-test-utils" -import OrderService from "../order" import { LineItemServiceMock } from "../__mocks__/line-item" import { newTotalsServiceMock } from "../__mocks__/new-totals" import { ProductVariantInventoryServiceMock } from "../__mocks__/product-variant-inventory" import { taxProviderServiceMock } from "../__mocks__/tax-provider" +import OrderService from "../order" describe("OrderService", () => { const totalsService = { @@ -519,12 +519,13 @@ describe("OrderService", () => { }) }) - describe("retrieveByCartId", () => { + describe("retrieveByCartIdWithTotals", () => { const orderRepo = MockRepository({ - findOne: (q) => { + findOneWithRelations: (q) => { return Promise.resolve({}) }, }) + const orderService = new OrderService({ totalsService, newTotalsService: newTotalsServiceMock, @@ -537,11 +538,15 @@ describe("OrderService", () => { }) it("calls order model functions", async () => { - await orderService.retrieveByCartId(IdMap.getId("test-cart")) - expect(orderRepo.findOne).toHaveBeenCalledTimes(1) - expect(orderRepo.findOne).toHaveBeenCalledWith({ - where: { cart_id: IdMap.getId("test-cart") }, - }) + await orderService.retrieveByCartIdWithTotals(IdMap.getId("test-cart")) + + expect(orderRepo.findOneWithRelations).toHaveBeenCalledTimes(1) + expect(orderRepo.findOneWithRelations).toHaveBeenCalledWith( + expect.any(Object), + { + where: { cart_id: IdMap.getId("test-cart") }, + } + ) }) }) diff --git a/packages/medusa/src/services/order.ts b/packages/medusa/src/services/order.ts index fb30b6ccd9..6dd906fa1f 100644 --- a/packages/medusa/src/services/order.ts +++ b/packages/medusa/src/services/order.ts @@ -487,35 +487,43 @@ class OrderService extends TransactionBaseService { cartId: string, config: FindConfig = {} ): Promise { - const orderRepo = this.activeManager_.withRepository(this.orderRepository_) - - const { select, relations, totalsToSelect } = - this.transformQueryForTotals(config) - - const query = { - where: { cart_id: cartId }, - } as FindConfig - - if (relations && relations.length > 0) { - query.relations = relations + if (!isDefined(cartId)) { + throw new MedusaError( + MedusaError.Types.NOT_FOUND, + `"cartId" must be defined` + ) } - query.select = select?.length ? select : undefined + const orderRepo = this.activeManager_.withRepository(this.orderRepository_) - const raw = await orderRepo.findOne(query) + const query = buildQuery({ cart_id: cartId }, config) + + if (!(config.select || []).length) { + query.select = undefined + } + + const queryRelations = { ...query.relations } + delete query.relations + + const raw = await orderRepo.findOneWithRelations(queryRelations, query) if (!raw) { throw new MedusaError( MedusaError.Types.NOT_FOUND, - `Order with cart id: ${cartId} was not found` + `Order with cart id ${cartId} was not found` ) } - if (!totalsToSelect?.length) { - return raw - } + return raw + } - return await this.decorateTotals(raw, totalsToSelect) + async retrieveByCartIdWithTotals( + cartId: string, + options: FindConfig = {} + ): Promise { + const relations = this.getTotalsRelations(options) + const order = await this.retrieveByCartId(cartId, { ...options, relations }) + return await this.decorateTotals(order, {}) } /** @@ -659,7 +667,7 @@ class OrderService extends TransactionBaseService { // Is the cascade insert really used? Also, is it really necessary to pass the entire entities when creating or updating? // We normally should only pass what is needed? const shippingMethods = cart.shipping_methods.map((method) => { - ;(method.tax_lines as any) = undefined + (method.tax_lines as any) = undefined return method }) @@ -767,7 +775,7 @@ class OrderService extends TransactionBaseService { // TODO: Due to cascade insert we have to remove the tax_lines that have been added by the cart decorate totals. // Is the cascade insert really used? Also, is it really necessary to pass the entire entities when creating or updating? // We normally should only pass what is needed? - ;(method.tax_lines as any) = undefined + (method.tax_lines as any) = undefined return shippingOptionServiceTx.updateShippingMethod(method.id, { order_id: order.id, })