From e22a383f4738e8bc80394ccaba3ac9a4ae678955 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Frane=20Poli=C4=87?= <16856471+fPolic@users.noreply.github.com> Date: Mon, 6 Feb 2023 16:18:23 +0100 Subject: [PATCH] fix(medusa): `fields` params usage in the storefront endpoints (#2980) --- .changeset/smooth-trees-talk.md | 5 + .../api/__tests__/store/orders.js | 52 +++++++++ .../api/__tests__/store/products.js | 108 +++++++++++------- .../src/api/middlewares/transform-query.ts | 14 +++ .../routes/store/gift-cards/get-gift-card.ts | 2 +- .../src/api/routes/store/orders/get-order.ts | 10 +- .../src/api/routes/store/orders/index.ts | 50 +++++++- .../api/routes/store/orders/lookup-order.ts | 16 ++- .../routes/store/payment-collections/index.ts | 4 +- .../store/products/__tests__/list-products.js | 6 +- .../api/routes/store/products/get-product.ts | 18 ++- .../src/api/routes/store/products/index.ts | 58 +++++++++- .../routes/store/products/list-products.ts | 7 +- packages/medusa/src/types/global.ts | 1 + .../medusa/src/utils/clean-response-data.ts | 21 ++++ 15 files changed, 308 insertions(+), 64 deletions(-) create mode 100644 .changeset/smooth-trees-talk.md create mode 100644 packages/medusa/src/utils/clean-response-data.ts diff --git a/.changeset/smooth-trees-talk.md b/.changeset/smooth-trees-talk.md new file mode 100644 index 0000000000..3234361141 --- /dev/null +++ b/.changeset/smooth-trees-talk.md @@ -0,0 +1,5 @@ +--- +"@medusajs/medusa": patch +--- + +fix(medusa): `fields` param in store products/orders endpoints diff --git a/integration-tests/api/__tests__/store/orders.js b/integration-tests/api/__tests__/store/orders.js index ccf9a2bd7a..2e3a792788 100644 --- a/integration-tests/api/__tests__/store/orders.js +++ b/integration-tests/api/__tests__/store/orders.js @@ -155,6 +155,58 @@ describe("/store/carts", () => { ) }) + it("lookup order response contains only fields defined with `fields` param", async () => { + const api = useApi() + + const response = await api + .get( + "/store/orders?display_id=111&email=test@email.com&fields=status,object" + ) + .catch((err) => { + return err.response + }) + + expect(Object.keys(response.data.order)).toEqual([ + // fields + "status", + "object", + // relations + "shipping_address", + "fulfillments", + "items", + "shipping_methods", + "discounts", + "customer", + "payments", + "region", + ]) + }) + + it("get order response contains only fields defined with `fields` param", async () => { + const api = useApi() + + const response = await api + .get("/store/orders/order_test?fields=status,object") + .catch((err) => { + return err.response + }) + + expect(Object.keys(response.data.order)).toEqual([ + // fields + "status", + "object", + // relations + "shipping_address", + "fulfillments", + "items", + "shipping_methods", + "discounts", + "customer", + "payments", + "region", + ]) + }) + it("looks up order", async () => { const api = useApi() diff --git a/integration-tests/api/__tests__/store/products.js b/integration-tests/api/__tests__/store/products.js index 5998001731..e6e509b3c8 100644 --- a/integration-tests/api/__tests__/store/products.js +++ b/integration-tests/api/__tests__/store/products.js @@ -5,7 +5,7 @@ const { initDb, useDb } = require("../../../helpers/use-db") const { simpleProductFactory, - simpleProductCategoryFactory + simpleProductCategoryFactory, } = require("../../factories") const productSeeder = require("../../helpers/store-product-seeder") @@ -194,6 +194,26 @@ describe("/store/products", () => { expect(testProduct2Index).toBe(2) // 200 }) + it("products contain only fields defined with `fields` param", async () => { + const api = useApi() + + const response = await api.get("/store/products?fields=handle") + + expect(response.status).toEqual(200) + + expect(Object.keys(response.data.products[0])).toEqual([ + // fields + "handle", + // relations + "variants", + "options", + "images", + "tags", + "collection", + "type", + ]) + }) + it("returns a list of ordered products by id ASC and filtered with free text search", async () => { const api = useApi() @@ -455,7 +475,10 @@ describe("/store/products", () => { }) describe("Product Category filtering", () => { - let categoryWithProduct, categoryWithoutProduct, nestedCategoryWithProduct, nested2CategoryWithProduct + let categoryWithProduct, + categoryWithoutProduct, + nestedCategoryWithProduct, + nested2CategoryWithProduct const nestedCategoryWithProductId = "nested-category-with-product-id" const nested2CategoryWithProductId = "nested2-category-with-product-id" const categoryWithProductId = "category-with-product-id" @@ -463,14 +486,11 @@ describe("/store/products", () => { beforeEach(async () => { const manager = dbConnection.manager - categoryWithProduct = await simpleProductCategoryFactory( - dbConnection, - { - id: categoryWithProductId, - name: "category with Product", - products: [{ id: testProductId }], - } - ) + categoryWithProduct = await simpleProductCategoryFactory(dbConnection, { + id: categoryWithProductId, + name: "category with Product", + products: [{ id: testProductId }], + }) nestedCategoryWithProduct = await simpleProductCategoryFactory( dbConnection, @@ -504,49 +524,36 @@ describe("/store/products", () => { it("returns a list of products in product category without category children", async () => { const api = useApi() const params = `category_id[]=${categoryWithProductId}` - const response = await api - .get( - `/store/products?${params}`, - ) + const response = await api.get(`/store/products?${params}`) expect(response.status).toEqual(200) expect(response.data.products).toHaveLength(1) - expect(response.data.products).toEqual( - [ - expect.objectContaining({ - id: testProductId, - }), - ] - ) + expect(response.data.products).toEqual([ + expect.objectContaining({ + id: testProductId, + }), + ]) }) it("returns a list of products in product category without category children explicitly set to false", async () => { const api = useApi() const params = `category_id[]=${categoryWithProductId}&include_category_children=false` - const response = await api - .get( - `/store/products?${params}`, - ) + const response = await api.get(`/store/products?${params}`) expect(response.status).toEqual(200) expect(response.data.products).toHaveLength(1) - expect(response.data.products).toEqual( - [ - expect.objectContaining({ - id: testProductId, - }), - ] - ) + expect(response.data.products).toEqual([ + expect.objectContaining({ + id: testProductId, + }), + ]) }) it("returns a list of products in product category with category children", async () => { const api = useApi() const params = `category_id[]=${categoryWithProductId}&include_category_children=true` - const response = await api - .get( - `/store/products?${params}`, - ) + const response = await api.get(`/store/products?${params}`) expect(response.status).toEqual(200) expect(response.data.products).toHaveLength(3) @@ -560,7 +567,7 @@ describe("/store/products", () => { }), expect.objectContaining({ id: testProductFilteringId1, - }) + }), ]) ) }) @@ -569,10 +576,7 @@ describe("/store/products", () => { const api = useApi() const params = `category_id[]=${categoryWithoutProductId}&include_category_children=true` - const response = await api - .get( - `/store/products?${params}`, - ) + const response = await api.get(`/store/products?${params}`) expect(response.status).toEqual(200) expect(response.data.products).toHaveLength(0) @@ -1082,5 +1086,27 @@ describe("/store/products", () => { ]) ) }) + + it("response contains only fields defined with `fields` param", async () => { + const api = useApi() + + const response = await api.get( + "/store/products/test-product?fields=handle" + ) + + expect(response.status).toEqual(200) + + expect(Object.keys(response.data.product)).toEqual([ + // fields + "handle", + // relations + "variants", + "options", + "images", + "tags", + "collection", + "type", + ]) + }) }) }) diff --git a/packages/medusa/src/api/middlewares/transform-query.ts b/packages/medusa/src/api/middlewares/transform-query.ts index ce134d847a..1fe4b967c7 100644 --- a/packages/medusa/src/api/middlewares/transform-query.ts +++ b/packages/medusa/src/api/middlewares/transform-query.ts @@ -39,6 +39,20 @@ export function transformQuery< ]) req.filterableFields = removeUndefinedProperties(req.filterableFields) + if ( + (queryConfig?.defaultFields || validated.fields) && + (queryConfig?.defaultRelations || validated.expand) + ) { + req.allowedProperties = [ + ...(validated.fields + ? validated.fields.split(",") + : queryConfig?.allowedFields || [])!, + ...(validated.expand + ? validated.expand.split(",") + : queryConfig?.allowedRelations || [])!, + ] as unknown as string[] + } + if (queryConfig?.isList) { req.listConfig = prepareListQuery( validated, diff --git a/packages/medusa/src/api/routes/store/gift-cards/get-gift-card.ts b/packages/medusa/src/api/routes/store/gift-cards/get-gift-card.ts index 7a8cf76107..df56821f34 100644 --- a/packages/medusa/src/api/routes/store/gift-cards/get-gift-card.ts +++ b/packages/medusa/src/api/routes/store/gift-cards/get-gift-card.ts @@ -6,7 +6,7 @@ import GiftCardService from "../../../../services/gift-card" * @oas [get] /gift-cards/{code} * operationId: "GetGiftCardsCode" * summary: "Get Gift Card by Code" - * description: "Retrieves a Gift Card by its associated unqiue code." + * description: "Retrieves a Gift Card by its associated unique code." * parameters: * - (path) code=* {string} The unique Gift Card code. * x-codegen: diff --git a/packages/medusa/src/api/routes/store/orders/get-order.ts b/packages/medusa/src/api/routes/store/orders/get-order.ts index 3b62c3ea4f..8f0d17ca38 100644 --- a/packages/medusa/src/api/routes/store/orders/get-order.ts +++ b/packages/medusa/src/api/routes/store/orders/get-order.ts @@ -1,6 +1,8 @@ import { defaultStoreOrdersFields, defaultStoreOrdersRelations } from "./index" import { OrderService } from "../../../../services" +import { FindParams } from "../../../../types/common" +import { cleanResponseData } from "../../../../utils/clean-response-data" /** * @oas [get] /orders/{id} @@ -9,6 +11,8 @@ import { OrderService } from "../../../../services" * description: "Retrieves an Order" * parameters: * - (path) id=* {string} The id of the Order. + * - (query) fields {string} (Comma separated) Which fields should be included in the result. + * - (query) expand {string} (Comma separated) Which fields should be expanded in the result. * x-codegen: * method: retrieve * x-codeSamples: @@ -54,5 +58,9 @@ export default async (req, res) => { relations: defaultStoreOrdersRelations, }) - res.json({ order }) + res.json({ + order: cleanResponseData(order, req.allowedProperties || []), + }) } + +export class StoreGetOrderParams extends FindParams {} diff --git a/packages/medusa/src/api/routes/store/orders/index.ts b/packages/medusa/src/api/routes/store/orders/index.ts index ee87d174a6..7ece67b6fa 100644 --- a/packages/medusa/src/api/routes/store/orders/index.ts +++ b/packages/medusa/src/api/routes/store/orders/index.ts @@ -1,10 +1,15 @@ import { Router } from "express" import "reflect-metadata" import { Order } from "../../../.." -import middlewares, { transformBody } from "../../../middlewares" +import middlewares, { + transformBody, + transformQuery, +} 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" const route = Router() @@ -14,12 +19,31 @@ export default (app) => { /** * Lookup */ - route.get("/", middlewares.wrap(require("./lookup-order").default)) + route.get( + "/", + transformQuery(StoreGetOrdersParams, { + defaultFields: defaultStoreOrdersFields, + defaultRelations: defaultStoreOrdersRelations, + allowedFields: allowedStoreOrdersFields, + allowedRelations: allowedStoreOrdersRelations, + isList: true, + }), + middlewares.wrap(require("./lookup-order").default) + ) /** * Retrieve Order */ - route.get("/:id", middlewares.wrap(require("./get-order").default)) + route.get( + "/:id", + transformQuery(StoreGetOrderParams, { + defaultFields: defaultStoreOrdersFields, + defaultRelations: defaultStoreOrdersRelations, + allowedFields: allowedStoreOrdersFields, + allowedRelations: allowedStoreOrdersRelations, + }), + middlewares.wrap(require("./get-order").default) + ) /** * Retrieve by Cart Id @@ -60,6 +84,11 @@ export const defaultStoreOrdersRelations = [ "region", ] +export const allowedStoreOrdersRelations = [ + ...defaultStoreOrdersRelations, + "billing_address", +] + export const defaultStoreOrdersFields = [ "id", "status", @@ -83,6 +112,21 @@ export const defaultStoreOrdersFields = [ "total", ] as (keyof Order)[] +export const allowedStoreOrdersFields = [ + ...defaultStoreOrdersFields, + "object", + "shipping_total", + "discount_total", + "tax_total", + "refunded_total", + "total", + "subtotal", + "paid_total", + "refundable_amount", + "gift_card_total", + "gift_card_tax_total", +] + /** * @schema StoreOrdersRes * type: object diff --git a/packages/medusa/src/api/routes/store/orders/lookup-order.ts b/packages/medusa/src/api/routes/store/orders/lookup-order.ts index 77b7bd3ae4..7b19151c84 100644 --- a/packages/medusa/src/api/routes/store/orders/lookup-order.ts +++ b/packages/medusa/src/api/routes/store/orders/lookup-order.ts @@ -5,11 +5,13 @@ import { IsString, ValidateNested, } from "class-validator" -import { defaultStoreOrdersFields, defaultStoreOrdersRelations } from "." +import { Type } from "class-transformer" import { OrderService } from "../../../../services" -import { Type } from "class-transformer" -import { validator } from "../../../../utils/validator" +import { cleanResponseData } from "../../../../utils/clean-response-data" + +import { defaultStoreOrdersFields, defaultStoreOrdersRelations } from "." +import { FindParams } from "../../../../types/common" /** * @oas [get] /orders @@ -18,6 +20,8 @@ import { validator } from "../../../../utils/validator" * description: "Look up an order using filters." * parameters: * - (query) display_id=* {number} The display id given to the Order. + * - (query) fields {string} (Comma separated) Which fields should be included in the result. + * - (query) expand {string} (Comma separated) Which fields should be expanded in the result. * - in: query * name: email * style: form @@ -79,7 +83,7 @@ import { validator } from "../../../../utils/validator" * $ref: "#/components/responses/500_error" */ export default async (req, res) => { - const validated = await validator(StoreGetOrdersParams, req.query) + const validated = req.validatedQuery as StoreGetOrdersParams const orderService: OrderService = req.scope.resolve("orderService") @@ -101,7 +105,7 @@ export default async (req, res) => { const order = orders[0] - res.json({ order }) + res.json({ order: cleanResponseData(order, req.allowedProperties || []) }) } export class ShippingAddressPayload { @@ -110,7 +114,7 @@ export class ShippingAddressPayload { postal_code?: string } -export class StoreGetOrdersParams { +export class StoreGetOrdersParams extends FindParams { @IsNumber() @Type(() => Number) display_id: number diff --git a/packages/medusa/src/api/routes/store/payment-collections/index.ts b/packages/medusa/src/api/routes/store/payment-collections/index.ts index 8cc97f6a40..1e7d521b40 100644 --- a/packages/medusa/src/api/routes/store/payment-collections/index.ts +++ b/packages/medusa/src/api/routes/store/payment-collections/index.ts @@ -20,7 +20,7 @@ export default (app, container) => { "/:id", transformQuery(GetPaymentCollectionsParams, { defaultFields: defaultPaymentCollectionFields, - defaultRelations: defaulPaymentCollectionRelations, + defaultRelations: defaultPaymentCollectionRelations, isList: false, }), middlewares.wrap(require("./get-payment-collection").default) @@ -69,7 +69,7 @@ export const defaultPaymentCollectionFields = [ "metadata", ] -export const defaulPaymentCollectionRelations = ["region", "payment_sessions"] +export const defaultPaymentCollectionRelations = ["region", "payment_sessions"] /** * @schema StorePaymentCollectionsRes diff --git a/packages/medusa/src/api/routes/store/products/__tests__/list-products.js b/packages/medusa/src/api/routes/store/products/__tests__/list-products.js index dd686d719a..88f050719f 100644 --- a/packages/medusa/src/api/routes/store/products/__tests__/list-products.js +++ b/packages/medusa/src/api/routes/store/products/__tests__/list-products.js @@ -1,5 +1,5 @@ import { IdMap } from "medusa-test-utils" -import { defaultStoreProductsRelations } from ".." +import { defaultStoreProductsFields, defaultStoreProductsRelations } from ".." import { request } from "../../../../../helpers/test-request" import { ProductServiceMock } from "../../../../../services/__mocks__/product" @@ -21,9 +21,9 @@ describe("GET /store/products", () => { { status: ["published"] }, { relations: defaultStoreProductsRelations, + select: defaultStoreProductsFields, skip: 0, take: 100, - select: undefined, order: { created_at: "DESC", }, @@ -52,12 +52,12 @@ describe("GET /store/products", () => { { is_giftcard: true, status: ["published"] }, { relations: defaultStoreProductsRelations, + select: defaultStoreProductsFields, skip: 0, take: 100, order: { created_at: "DESC", }, - select: undefined, } ) }) diff --git a/packages/medusa/src/api/routes/store/products/get-product.ts b/packages/medusa/src/api/routes/store/products/get-product.ts index 290120f492..b29dd6873d 100644 --- a/packages/medusa/src/api/routes/store/products/get-product.ts +++ b/packages/medusa/src/api/routes/store/products/get-product.ts @@ -10,7 +10,7 @@ import { } from "../../../../services" import { PriceSelectionParams } from "../../../../types/price-selection" import { FlagRouter } from "../../../../utils/flag-router" -import { validator } from "../../../../utils/validator" +import { cleanResponseData } from "../../../../utils/clean-response-data" /** * @oas [get] /products/{id} @@ -22,6 +22,8 @@ import { validator } from "../../../../utils/validator" * - (query) sales_channel_id {string} The sales channel used when fetching the product. * - (query) cart_id {string} The ID of the customer's cart. * - (query) region_id {string} The ID of the region the customer is using. This is helpful to ensure correct prices are retrieved for a region. + * - (query) fields {string} (Comma separated) Which fields should be included in the result. + * - (query) expand {string} (Comma separated) Which fields should be expanded in each product of the result. * - in: query * name: currency_code * style: form @@ -72,7 +74,7 @@ import { validator } from "../../../../utils/validator" export default async (req, res) => { const { id } = req.params - const validated = await validator(StoreGetProductsProductParams, req.query) + const validated = req.validatedQuery as StoreGetProductsProductParams const customer_id = req.user?.customer_id @@ -123,11 +125,21 @@ export default async (req, res) => { sales_channel_id ) - res.json({ product }) + res.json({ + product: cleanResponseData(product, req.allowedProperties || []), + }) } export class StoreGetProductsProductParams extends PriceSelectionParams { @IsString() @IsOptional() sales_channel_id?: string + + @IsString() + @IsOptional() + fields?: string + + @IsString() + @IsOptional() + expand?: string } diff --git a/packages/medusa/src/api/routes/store/products/index.ts b/packages/medusa/src/api/routes/store/products/index.ts index 3f9e570adf..784c50d9c6 100644 --- a/packages/medusa/src/api/routes/store/products/index.ts +++ b/packages/medusa/src/api/routes/store/products/index.ts @@ -10,6 +10,7 @@ import PublishableAPIKeysFeatureFlag from "../../../../loaders/feature-flags/pub import { validateProductSalesChannelAssociation } from "../../../middlewares/publishable-api-key/validate-product-sales-channel-association" import { validateSalesChannelParam } from "../../../middlewares/publishable-api-key/validate-sales-channel-param" import { StoreGetProductsParams } from "./list-products" +import { StoreGetProductsProductParams } from "./get-product" const route = Router() @@ -29,11 +30,25 @@ export default (app, featureFlagRouter: FlagRouter) => { "/", transformQuery(StoreGetProductsParams, { defaultRelations: defaultStoreProductsRelations, + defaultFields: defaultStoreProductsFields, + allowedFields: allowedStoreProductsFields, + allowedRelations: allowedStoreProductsRelations, isList: true, }), middlewares.wrap(require("./list-products").default) ) - route.get("/:id", middlewares.wrap(require("./get-product").default)) + + route.get( + "/:id", + transformQuery(StoreGetProductsProductParams, { + defaultRelations: defaultStoreProductsRelations, + defaultFields: defaultStoreProductsFields, + allowedFields: allowedStoreProductsFields, + allowedRelations: allowedStoreProductsRelations, + }), + middlewares.wrap(require("./get-product").default) + ) + route.post("/search", middlewares.wrap(require("./search").default)) return app @@ -51,6 +66,47 @@ export const defaultStoreProductsRelations = [ "type", ] +export const defaultStoreProductsFields: (keyof Product)[] = [ + "id", + "title", + "subtitle", + "status", + "external_id", + "description", + "handle", + "is_giftcard", + "discountable", + "thumbnail", + "profile_id", + "collection_id", + "type_id", + "weight", + "length", + "height", + "width", + "hs_code", + "origin_country", + "mid_code", + "material", + "created_at", + "updated_at", + "deleted_at", + "metadata", +] + +export const allowedStoreProductsFields = [ + ...defaultStoreProductsFields, + // TODO: order prop validation + "variants.title", + "variants.prices.amount", +] + +export const allowedStoreProductsRelations = [ + ...defaultStoreProductsRelations, + "variants.title", + "variants.prices.amount", +] + export * from "./list-products" export * from "./search" diff --git a/packages/medusa/src/api/routes/store/products/list-products.ts b/packages/medusa/src/api/routes/store/products/list-products.ts index 4aea03d145..164b245bb9 100644 --- a/packages/medusa/src/api/routes/store/products/list-products.ts +++ b/packages/medusa/src/api/routes/store/products/list-products.ts @@ -22,6 +22,7 @@ import { optionalBooleanMapper } from "../../../../utils/validators/is-boolean" import { IsType } from "../../../../utils/validators/is-type" import { FlagRouter } from "../../../../utils/flag-router" import PublishableAPIKeysFeatureFlag from "../../../../loaders/feature-flags/publishable-api-keys" +import { cleanResponseData } from "../../../../utils/clean-response-data" /** * @oas [get] /products @@ -137,8 +138,8 @@ import PublishableAPIKeysFeatureFlag from "../../../../loaders/feature-flags/pub * - (query) include_category_children {boolean} Include category children when filtering by category_id. * - (query) offset=0 {integer} How many products to skip in the result. * - (query) limit=100 {integer} Limit the number of products returned. - * - (query) expand {string} (Comma separated) Which fields should be expanded in each order of the result. - * - (query) fields {string} (Comma separated) Which fields should be included in each order of the result. + * - (query) expand {string} (Comma separated) Which fields should be expanded in each product of the result. + * - (query) fields {string} (Comma separated) Which fields should be included in each product of the result. * - (query) order {string} the field used to order the products. * - (query) cart_id {string} The id of the Cart to set prices based on. * - (query) region_id {string} The id of the Region to set prices based on. @@ -241,7 +242,7 @@ export default async (req, res) => { ) res.json({ - products, + products: cleanResponseData(products, req.allowedProperties || []), count, offset: validated.offset, limit: validated.limit, diff --git a/packages/medusa/src/types/global.ts b/packages/medusa/src/types/global.ts index 9743341469..9a81f7acc4 100644 --- a/packages/medusa/src/types/global.ts +++ b/packages/medusa/src/types/global.ts @@ -16,6 +16,7 @@ declare global { listConfig: FindConfig retrieveConfig: FindConfig filterableFields: Record + allowedProperties: string[] errors: string[] } } diff --git a/packages/medusa/src/utils/clean-response-data.ts b/packages/medusa/src/utils/clean-response-data.ts new file mode 100644 index 0000000000..cca1002f7f --- /dev/null +++ b/packages/medusa/src/utils/clean-response-data.ts @@ -0,0 +1,21 @@ +import { pick } from "lodash" + +/** + * Filter response data to contain props specified in the fields array. + * + * @param data - record or an array of records in the response + * @param fields - record props allowed in the response + */ +function cleanResponseData(data: T, fields: string[]) { + if (!fields.length) { + return data + } + + if (Array.isArray(data)) { + return data.map((record) => pick(record, fields)) + } + + return pick(data, fields) +} + +export { cleanResponseData }