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

@@ -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

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 () => {

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 *********

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
       }
     }
 

packages/medusa/src/api/middlewares/transform-query.ts

@@ -129,7 +129,7 @@ function attachListOrRetrieveConfig<TEntity extends BaseEntity>(
   }
 }
 /**
- * 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<TEntity extends BaseEntity>(
   queryConfig?: QueryConfig<TEntity>
 ): 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<TEntity extends BaseEntity>(
   includesOptions: Record<string, boolean>,
   queryConfig?: QueryConfig<TEntity>
 ): 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[]
 }

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"