From 33c6ccf0591b8ef527f3b10a70b51e29751b4998 Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Tue, 7 Mar 2023 12:52:14 +0100 Subject: [PATCH] fix(medeusa): Transform query includes options should only be added to allowed props if there is already at least one allowed props (#3362) **What** when `fields` only contain includes options, it should return the entire object plus the include options. If the fields contains the included options + other fields, it should only return the requested fields + the included options --- .changeset/cuddly-seahorses-thank.md | 5 ++ .../api/__tests__/admin/order/order.js | 87 ++++++++++++++++++- .../api/__tests__/admin/swaps.js | 9 +- .../middlewares/transform-includes-options.ts | 37 ++++---- .../src/api/middlewares/transform-query.ts | 42 ++++----- .../src/api/routes/admin/orders/index.ts | 63 +++++++------- 6 files changed, 160 insertions(+), 83 deletions(-) create mode 100644 .changeset/cuddly-seahorses-thank.md diff --git a/.changeset/cuddly-seahorses-thank.md b/.changeset/cuddly-seahorses-thank.md new file mode 100644 index 0000000000..db30a714bb --- /dev/null +++ b/.changeset/cuddly-seahorses-thank.md @@ -0,0 +1,5 @@ +--- +"@medusajs/medusa": patch +--- + +fix(medeusa): Transform query includes options should only be added to allowed props if there is already at least one allowed props diff --git a/integration-tests/api/__tests__/admin/order/order.js b/integration-tests/api/__tests__/admin/order/order.js index eb1f100e4a..f1506068cd 100644 --- a/integration-tests/api/__tests__/admin/order/order.js +++ b/integration-tests/api/__tests__/admin/order/order.js @@ -2443,11 +2443,90 @@ describe("/admin/orders", () => { ) expect(order.status).toEqual(200) - expect(order.data.order).toEqual( - expect.objectContaining({ - id: "test-order", - }) + // id + totals + region relation + expect(order.data.order).toEqual({ + id: "test-order", + region: expect.any(Object), + shipping_total: 1000, + discount_total: 800, + tax_total: 0, + refunded_total: 0, + total: 8200, + subtotal: 8000, + paid_total: 0, + refundable_amount: 0, + gift_card_total: 0, + gift_card_tax_total: 0, + }) + }) + + it("retrieves an order with expand returnable_items only should return the entire object and only returnable_items as relation", async () => { + const api = useApi() + + const order = await api.get( + `/admin/orders/${testOrderId}?expand=returnable_items`, + adminReqConfig ) + + expect(order.status).toEqual(200) + // all order properties + totals + returnable_items relation + expect(order.data.order).toEqual({ + id: "test-order", + status: "pending", + fulfillment_status: "fulfilled", + payment_status: "captured", + display_id: expect.any(Number), + cart_id: null, + draft_order_id: null, + customer_id: "test-customer", + email: "test@email.com", + region_id: "test-region", + currency_code: "usd", + tax_rate: 0, + canceled_at: null, + created_at: expect.any(String), + updated_at: expect.any(String), + metadata: null, + no_notification: null, + sales_channel_id: null, + returnable_items: expect.any(Array), + shipping_total: 1000, + discount_total: 800, + tax_total: 0, + refunded_total: 0, + total: 8200, + subtotal: 8000, + paid_total: 10000, + refundable_amount: 10000, + gift_card_total: 0, + gift_card_tax_total: 0, + }) + }) + + it("retrieves an order with expand returnable_items and field id should return the id and the retunable_items", async () => { + const api = useApi() + + const order = await api.get( + `/admin/orders/${testOrderId}?expand=returnable_items&fields=id`, + adminReqConfig + ) + + expect(order.status).toEqual(200) + // id + totals + returnable_items relation + expect(order.data.order).toEqual({ + id: "test-order", + returnable_items: expect.any(Array), + shipping_total: 1000, + discount_total: 800, + tax_total: 0, + refunded_total: 0, + total: 8200, + subtotal: 8000, + paid_total: 10000, + refundable_amount: 10000, + gift_card_total: 0, + gift_card_tax_total: 0, + }) }) it("retrieves an order should include the items totals", async () => { diff --git a/integration-tests/api/__tests__/admin/swaps.js b/integration-tests/api/__tests__/admin/swaps.js index 814da8fe22..c75ea5fdd6 100644 --- a/integration-tests/api/__tests__/admin/swaps.js +++ b/integration-tests/api/__tests__/admin/swaps.js @@ -301,7 +301,7 @@ describe("/admin/swaps", () => { // ********* CREATE SWAP ********* const createSwap = await api.post( - `/admin/orders/${completedOrder.data.data.id}/swaps?fields=returnable_items`, + `/admin/orders/${completedOrder.data.data.id}/swaps`, { return_items: [ { @@ -318,13 +318,6 @@ describe("/admin/swaps", () => { } ) - expect(createSwap.data.order.returnable_items).toHaveLength(1) - expect(createSwap.data.order.returnable_items[0]).toEqual( - expect.objectContaining({ - id: "line-item", - }) - ) - let swap = createSwap.data.order.swaps[0] // ********* PREPARE SWAP CART ********* diff --git a/packages/medusa/src/api/middlewares/transform-includes-options.ts b/packages/medusa/src/api/middlewares/transform-includes-options.ts index 0e4593b41e..54b898f423 100644 --- a/packages/medusa/src/api/middlewares/transform-includes-options.ts +++ b/packages/medusa/src/api/middlewares/transform-includes-options.ts @@ -1,38 +1,37 @@ import { NextFunction, Request, Response } from "express" -import { Order } from "../../models" import { MedusaError } from "medusa-core-utils" /** * Retrieve the includes options from the fields query param. * If the include option is present then assigned it to includes on req - * @param allowedIncludesFields The list of fields that can be passed and assign to req.includes - * @param expectedIncludesFields The list of fields that the consumer can pass to the end point using this middleware. It is a subset of `allowedIncludesFields` + * @param allowedIncludes The list of fields that can be passed and assign to req.includes + * @param expectedIncludes The list of fields that the consumer can pass to the end point using this middleware. It is a subset of `allowedIncludes` */ export function transformIncludesOptions( - allowedIncludesFields: string[] = [], - expectedIncludesFields: string[] = [] + allowedIncludes: string[] = [], + expectedIncludes: string[] = [] ) { return (req: Request, res: Response, next: NextFunction): void => { - if (!allowedIncludesFields.length || !req.query.fields) { + if (!allowedIncludes.length || !req.query.expand) { return next() } - const fields = (req.query.fields as string).split(",") ?? [] + const expand = (req.query.expand as string).split(",") ?? [] - for (const includesField of allowedIncludesFields) { - const fieldIndex = fields.indexOf(includesField as keyof Order) ?? -1 + for (const includes of allowedIncludes) { + const fieldIndex = expand.indexOf(includes) ?? -1 const isPresent = fieldIndex !== -1 if (isPresent) { - fields.splice(fieldIndex, 1) + expand.splice(fieldIndex, 1) - if (!expectedIncludesFields.includes(includesField)) { + if (!expectedIncludes.includes(includes)) { throw new MedusaError( MedusaError.Types.INVALID_DATA, - `The field "${includesField}" is not supported by this end point. ${ - expectedIncludesFields.length - ? `The includes fields can be one of entity properties or in [${expectedIncludesFields.join( + `The field "${includes}" is not supported by this end point. ${ + expectedIncludes.length + ? `The includes fields can be one of entity properties or in [${expectedIncludes.join( ", " )}]` : "" @@ -41,15 +40,15 @@ export function transformIncludesOptions( } req.includes = req.includes ?? {} - req.includes[includesField] = true + req.includes[includes] = true } } - if (req.query.fields) { - if (fields.length) { - req.query.fields = fields.join(",") + if (req.query.expand) { + if (expand.length) { + req.query.expand = expand.join(",") } else { - delete req.query.fields + delete req.query.expand } } diff --git a/packages/medusa/src/api/middlewares/transform-query.ts b/packages/medusa/src/api/middlewares/transform-query.ts index 31dccfa918..b074368cf0 100644 --- a/packages/medusa/src/api/middlewares/transform-query.ts +++ b/packages/medusa/src/api/middlewares/transform-query.ts @@ -129,7 +129,7 @@ function attachListOrRetrieveConfig( } } /** - * Build the store allowed props based on the custom fields query params, the defaults and the includes options. + * Build the store allowed props based on the custom fields query params, the allowed props config and the includes options. * This can be used later with `cleanResponseData` in order to clean up the returned objects to the client. * @param queryConfig * @param validated @@ -141,19 +141,17 @@ function getStoreAllowedProperties( queryConfig?: QueryConfig ): string[] { const allowed: string[] = [] - allowed.push( - ...(validated.fields - ? validated.fields.split(",") - : queryConfig?.allowedFields || []), - ...(validated.expand - ? validated.expand.split(",") - : queryConfig?.allowedRelations || []) - ) const includeKeys = Object.keys(includesOptions) - if (includeKeys) { - allowed.push(...includeKeys) - } + const fields = validated.fields + ? validated.fields?.split(",") + : queryConfig?.allowedFields || [] + const expand = + validated.expand || includeKeys.length + ? [...(validated.expand?.split(",") || []), ...includeKeys] + : queryConfig?.allowedRelations || [] + + allowed.push(...fields, ...expand) return allowed } @@ -171,16 +169,18 @@ function getAllowedProperties( includesOptions: Record, queryConfig?: QueryConfig ): string[] { - const allowed: string[] = [] - allowed.push( - ...(validated.fields?.split(",") ?? []), - ...(validated.expand?.split(",") ?? []) - ) + const allowed: (string | keyof TEntity)[] = [] const includeKeys = Object.keys(includesOptions) - if (includeKeys) { - allowed.push(...includeKeys) - } + const fields = validated.fields + ? validated.fields?.split(",") + : queryConfig?.defaultFields || [] + const expand = + validated.expand || includeKeys.length + ? [...(validated.expand?.split(",") || []), ...includeKeys] + : queryConfig?.defaultRelations || [] - return allowed + allowed.push(...fields, ...expand) + + return allowed as string[] } diff --git a/packages/medusa/src/api/routes/admin/orders/index.ts b/packages/medusa/src/api/routes/admin/orders/index.ts index cedf1c331a..14fab96c71 100644 --- a/packages/medusa/src/api/routes/admin/orders/index.ts +++ b/packages/medusa/src/api/routes/admin/orders/index.ts @@ -91,7 +91,7 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.get( "/", - transformIncludesOptions(allowedOrderIncludesFields), + transformIncludesOptions(allowedOrderIncludes), transformQuery(AdminGetOrdersParams, { defaultRelations: relations, defaultFields: defaultAdminOrdersFields, @@ -105,8 +105,8 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.get( "/:id", - transformIncludesOptions(allowedOrderIncludesFields, [ - AvailableOrderIncludesFields.RETURNABLE_ITEMS, + transformIncludesOptions(allowedOrderIncludes, [ + AvailableOrderIncludes.RETURNABLE_ITEMS, ]), transformQuery(AdminPostOrdersOrderParams, { defaultRelations: relations, @@ -121,7 +121,7 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.post( "/:id", - transformIncludesOptions(allowedOrderIncludesFields), + transformIncludesOptions(allowedOrderIncludes), transformBody(AdminPostOrdersOrderReq), transformQuery(FindParams, { defaultRelations: relations, @@ -136,7 +136,7 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.post( "/:id/complete", - transformIncludesOptions(allowedOrderIncludesFields), + transformIncludesOptions(allowedOrderIncludes), transformQuery(AdminPostOrdersOrderCompleteParams, { defaultRelations: relations, defaultFields: defaultFields, @@ -150,7 +150,7 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.post( "/:id/refund", - transformIncludesOptions(allowedOrderIncludesFields), + transformIncludesOptions(allowedOrderIncludes), transformBody(AdminPostOrdersOrderRefundsReq), transformQuery(AdminPostOrdersOrderRefundsParams, { defaultRelations: relations, @@ -165,7 +165,7 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.post( "/:id/capture", - transformIncludesOptions(allowedOrderIncludesFields), + transformIncludesOptions(allowedOrderIncludes), transformQuery(AdminPostOrdersOrderCaptureParams, { defaultRelations: relations, defaultFields: defaultFields, @@ -179,7 +179,7 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.post( "/:id/fulfillment", - transformIncludesOptions(allowedOrderIncludesFields), + transformIncludesOptions(allowedOrderIncludes), transformBody(AdminPostOrdersOrderFulfillmentsReq), transformQuery(AdminPostOrdersOrderFulfillmentsParams, { defaultRelations: relations, @@ -194,7 +194,7 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.post( "/:id/fulfillments/:fulfillment_id/cancel", - transformIncludesOptions(allowedOrderIncludesFields), + transformIncludesOptions(allowedOrderIncludes), transformQuery(AdminPostOrdersOrderFulfillementsCancelParams, { defaultRelations: relations, defaultFields: defaultFields, @@ -208,7 +208,7 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.post( "/:id/swaps/:swap_id/fulfillments/:fulfillment_id/cancel", - transformIncludesOptions(allowedOrderIncludesFields), + transformIncludesOptions(allowedOrderIncludes), transformQuery(AdminPostOrdersOrderSwapFulfillementsCancelParams, { defaultRelations: relations, defaultFields: defaultFields, @@ -222,7 +222,7 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.post( "/:id/claims/:claim_id/fulfillments/:fulfillment_id/cancel", - transformIncludesOptions(allowedOrderIncludesFields), + transformIncludesOptions(allowedOrderIncludes), transformQuery(AdminPostOrdersClaimFulfillmentsCancelParams, { defaultRelations: relations, defaultFields: defaultFields, @@ -236,7 +236,7 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.post( "/:id/shipment", - transformIncludesOptions(allowedOrderIncludesFields), + transformIncludesOptions(allowedOrderIncludes), transformBody(AdminPostOrdersOrderShipmentReq), transformQuery(AdminPostOrdersOrderShipmentParams, { defaultRelations: relations, @@ -251,7 +251,7 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.post( "/:id/return", - transformIncludesOptions(allowedOrderIncludesFields), + transformIncludesOptions(allowedOrderIncludes), transformBody(AdminPostOrdersOrderReturnsReq), transformQuery(AdminPostOrdersOrderReturnsParams, { defaultRelations: relations, @@ -266,7 +266,7 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.post( "/:id/cancel", - transformIncludesOptions(allowedOrderIncludesFields), + transformIncludesOptions(allowedOrderIncludes), transformQuery(AdminPostOrdersOrderCancel, { defaultRelations: relations, defaultFields: defaultFields, @@ -280,7 +280,7 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.post( "/:id/shipping-methods", - transformIncludesOptions(allowedOrderIncludesFields), + transformIncludesOptions(allowedOrderIncludes), transformBody(AdminPostOrdersOrderShippingMethodsReq), transformQuery(AdminPostOrdersOrderShippingMethodsParams, { defaultRelations: relations, @@ -295,7 +295,7 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.post( "/:id/archive", - transformIncludesOptions(allowedOrderIncludesFields), + transformIncludesOptions(allowedOrderIncludes), transformQuery(AdminPostOrdersOrderArchiveParams, { defaultRelations: relations, defaultFields: defaultFields, @@ -309,8 +309,8 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.post( "/:id/swaps", - transformIncludesOptions(allowedOrderIncludesFields, [ - AvailableOrderIncludesFields.RETURNABLE_ITEMS, + transformIncludesOptions(allowedOrderIncludes, [ + AvailableOrderIncludes.RETURNABLE_ITEMS, ]), transformBody(AdminPostOrdersOrderSwapsReq), transformQuery(AdminPostOrdersOrderSwapsParams, { @@ -326,7 +326,7 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.post( "/:id/swaps/:swap_id/cancel", - transformIncludesOptions(allowedOrderIncludesFields), + transformIncludesOptions(allowedOrderIncludes), transformQuery(AdminPostOrdersSwapCancelParams, { defaultRelations: relations, defaultFields: defaultFields, @@ -340,7 +340,7 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.post( "/:id/swaps/:swap_id/fulfillments", - transformIncludesOptions(allowedOrderIncludesFields), + transformIncludesOptions(allowedOrderIncludes), transformQuery(AdminPostOrdersOrderSwapsSwapFulfillmentsParams, { defaultRelations: relations, defaultFields: defaultFields, @@ -354,7 +354,7 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.post( "/:id/swaps/:swap_id/shipments", - transformIncludesOptions(allowedOrderIncludesFields), + transformIncludesOptions(allowedOrderIncludes), transformBody(AdminPostOrdersOrderSwapsSwapShipmentsReq), transformQuery(AdminPostOrdersOrderSwapsSwapShipmentsParams, { defaultRelations: relations, @@ -369,7 +369,7 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.post( "/:id/swaps/:swap_id/process-payment", - transformIncludesOptions(allowedOrderIncludesFields), + transformIncludesOptions(allowedOrderIncludes), transformQuery(AdminPostOrdersOrderSwapsSwapProcessPaymentParams, { defaultRelations: relations, defaultFields: defaultFields, @@ -383,7 +383,7 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.post( "/:id/claims", - transformIncludesOptions(allowedOrderIncludesFields), + transformIncludesOptions(allowedOrderIncludes), transformBody(AdminPostOrdersOrderClaimsReq), transformQuery(AdminPostOrdersOrderClaimsParams, { defaultRelations: relations, @@ -398,7 +398,7 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.post( "/:id/claims/:claim_id/cancel", - transformIncludesOptions(allowedOrderIncludesFields), + transformIncludesOptions(allowedOrderIncludes), transformQuery(AdminPostOrdersClaimCancel, { defaultRelations: relations, defaultFields: defaultFields, @@ -412,7 +412,7 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.post( "/:id/claims/:claim_id", - transformIncludesOptions(allowedOrderIncludesFields), + transformIncludesOptions(allowedOrderIncludes), transformBody(AdminPostOrdersOrderClaimsClaimReq), transformQuery(AdminPostOrdersOrderClaimsClaimParams, { defaultRelations: relations, @@ -427,7 +427,7 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.post( "/:id/claims/:claim_id/fulfillments", - transformIncludesOptions(allowedOrderIncludesFields), + transformIncludesOptions(allowedOrderIncludes), transformBody(AdminPostOrdersOrderClaimsClaimFulfillmentsReq), transformQuery(AdminPostOrdersOrderClaimsClaimFulfillmentsParams, { defaultRelations: relations, @@ -442,7 +442,7 @@ export default (app, featureFlagRouter: FlagRouter) => { */ route.post( "/:id/claims/:claim_id/shipments", - transformIncludesOptions(allowedOrderIncludesFields), + transformIncludesOptions(allowedOrderIncludes), transformBody(AdminPostOrdersOrderClaimsClaimShipmentsReq), transformQuery(AdminPostOrdersOrderClaimsClaimShipmentsParams, { defaultRelations: relations, @@ -525,6 +525,9 @@ export const defaultAdminOrdersRelations = [ "discounts.rule", "shipping_methods", "payments", + "items", + "refunds", + "region", "fulfillments", "fulfillments.tracking_links", "fulfillments.items", @@ -603,13 +606,11 @@ export const filterableAdminOrdersFields = [ "updated_at", ] -export const AvailableOrderIncludesFields = { +export const AvailableOrderIncludes = { RETURNABLE_ITEMS: "returnable_items", } -export const allowedOrderIncludesFields = [ - AvailableOrderIncludesFields.RETURNABLE_ITEMS, -] +export const allowedOrderIncludes = [AvailableOrderIncludes.RETURNABLE_ITEMS] export * from "./add-shipping-method" export * from "./archive-order"