Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(medusa): Clean response data usage for admin and store fields/expand #3323

Merged
merged 9 commits into from
Feb 28, 2023
Merged
5 changes: 5 additions & 0 deletions .changeset/mighty-ads-fold.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@medusajs/medusa": patch
---

fix(medusa): Clean response data usage for admin and store fields/expand
10 changes: 10 additions & 0 deletions integration-tests/api/__tests__/admin/order/order.js
Original file line number Diff line number Diff line change
Expand Up @@ -1615,6 +1615,16 @@ describe("/admin/orders", () => {
id: "test-billing-address",
first_name: "lebron",
}),
shipping_total: expect.any(Number),
discount_total: expect.any(Number),
tax_total: expect.any(Number),
refunded_total: expect.any(Number),
total: expect.any(Number),
subtotal: expect.any(Number),
paid_total: expect.any(Number),
refundable_amount: expect.any(Number),
gift_card_total: expect.any(Number),
gift_card_tax_total: expect.any(Number),
},
])
)
Expand Down
33 changes: 33 additions & 0 deletions integration-tests/api/__tests__/store/orders.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,17 @@ describe("/store/carts", () => {
"customer",
"payments",
"region",
// default
"shipping_total",
"discount_total",
"tax_total",
"refunded_total",
"total",
"subtotal",
"paid_total",
"refundable_amount",
"gift_card_total",
"gift_card_tax_total",
])
})

Expand All @@ -197,6 +208,17 @@ describe("/store/carts", () => {
"customer",
"payments",
"region",
// default
"shipping_total",
"discount_total",
"tax_total",
"refunded_total",
"total",
"subtotal",
"paid_total",
"refundable_amount",
"gift_card_total",
"gift_card_tax_total",
])
})

Expand All @@ -212,6 +234,17 @@ describe("/store/carts", () => {
"status",
// selected relations
"billing_address",
// default
"shipping_total",
"discount_total",
"tax_total",
"refunded_total",
"total",
"subtotal",
"paid_total",
"refundable_amount",
"gift_card_total",
"gift_card_tax_total",
])
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ export function transformIncludesOptions(
expectedIncludesFields: string[] = []
) {
return (req: Request, res: Response, next: NextFunction): void => {
if (!allowedIncludesFields.length || !req.query["fields"]) {
if (!allowedIncludesFields.length || !req.query.fields) {
return next()
}

const fields = (req.query["fields"] as string).split(",") ?? []
const fields = (req.query.fields as string).split(",") ?? []

for (const includesField of allowedIncludesFields) {
const fieldIndex = fields.indexOf(includesField as keyof Order) ?? -1
Expand All @@ -40,16 +40,16 @@ export function transformIncludesOptions(
)
}

req["includes"] = req["includes"] ?? {}
req["includes"][includesField] = true
req.includes = req.includes ?? {}
req.includes[includesField] = true
}
}

if (req.query["fields"]) {
if (req.query.fields) {
if (fields.length) {
req.query["fields"] = fields.join(",")
req.query.fields = fields.join(",")
} else {
delete req.query["fields"]
delete req.query.fields
}
}

Expand Down
92 changes: 74 additions & 18 deletions packages/medusa/src/api/middlewares/transform-query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,25 +39,17 @@ 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[]
}
req.storeAllowedProperties = getStoreAllowedProperties(
validated,
req.includes ?? {},
queryConfig
)

const includesFields = Object.keys(req["includes"] ?? {})
if (includesFields.length) {
req.allowedProperties = req.allowedProperties ?? []
req.allowedProperties.push(...includesFields)
}
req.adminAllowedProperties = getAdminAllowedProperties(
adrien2p marked this conversation as resolved.
Show resolved Hide resolved
validated,
req.includes ?? {},
queryConfig
)

if (queryConfig?.isList) {
req.listConfig = prepareListQuery(
Expand All @@ -77,3 +69,67 @@ export function transformQuery<
}
}
}

/**
* Build the store allowed props based on the custom fields query params, the defaults 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
* @param includesOptions
*/
function getStoreAllowedProperties<TEntity extends BaseEntity>(
validated: RequestQueryFields,
includesOptions: Record<string, boolean>,
queryConfig?: QueryConfig<TEntity>
): string[] {
const allowed: string[] = []
if (
(queryConfig?.defaultFields || validated.fields) &&
(queryConfig?.defaultRelations || validated.expand)
) {
adrien2p marked this conversation as resolved.
Show resolved Hide resolved
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)
}

return allowed
}

/**
* Build the admin allowed props based on the custom fields query params, the defaults and the includes options.
* Since admin can access everything, it is only in order to return what the user asked for through fields and expand query params.
* This can be used later with `cleanResponseData` in order to clean up the returned objects to the client.
* @param queryConfig
* @param validated
* @param includesOptions
*/
function getAdminAllowedProperties<TEntity extends BaseEntity>(
adrien2p marked this conversation as resolved.
Show resolved Hide resolved
validated: RequestQueryFields,
includesOptions: Record<string, boolean>,
queryConfig?: QueryConfig<TEntity>
): string[] {
const allowed: string[] = []
if (validated.fields || validated.expand) {
allowed.push(
...(validated.fields?.split(",") ?? []),
...(validated.expand?.split(",") ?? [])
)
}

const includeKeys = Object.keys(includesOptions)
if (includeKeys) {
allowed.push(...includeKeys)
}

return allowed
}
14 changes: 11 additions & 3 deletions packages/medusa/src/api/routes/admin/orders/get-order.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { OrderService } from "../../../../services"
import { FindParams } from "../../../../types/common"
import { cleanResponseData } from "../../../../utils/clean-response-data"
import { Order } from "../../../../models"

/**
* @oas [get] /admin/orders/{id}
Expand Down Expand Up @@ -60,9 +62,15 @@ export default async (req, res) => {

const orderService: OrderService = req.scope.resolve("orderService")

const order = await orderService.retrieveWithTotals(id, req.retrieveConfig, {
includes: req.includes,
})
let order: Partial<Order> = await orderService.retrieveWithTotals(
id,
req.retrieveConfig,
{
includes: req.includes,
}
)

order = cleanResponseData(order, req.adminAllowedProperties)

res.json({ order: order })
}
Expand Down
12 changes: 3 additions & 9 deletions packages/medusa/src/api/routes/admin/orders/list-orders.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { IsNumber, IsOptional, IsString } from "class-validator"

import { AdminListOrdersSelector } from "../../../../types/orders"
import { Order } from "../../../../models"
import { OrderService } from "../../../../services"
import { Type } from "class-transformer"
import { pick } from "lodash"
import { cleanResponseData } from "../../../../utils/clean-response-data"

/**
* @oas [get] /admin/orders
Expand Down Expand Up @@ -200,19 +199,14 @@ import { pick } from "lodash"
export default async (req, res) => {
const orderService: OrderService = req.scope.resolve("orderService")

const { skip, take, select, relations } = req.listConfig
const { skip, take } = req.listConfig

const [orders, count] = await orderService.listAndCount(
req.filterableFields,
req.listConfig
)

let data: Partial<Order>[] = orders

const fields = [...select, ...relations]
if (fields.length) {
data = orders.map((o) => pick(o, fields))
}
const data = cleanResponseData(orders, req.adminAllowedProperties)

res.json({ orders: data, count, offset: skip, limit: take })
}
Expand Down
2 changes: 1 addition & 1 deletion packages/medusa/src/api/routes/store/orders/get-order.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export default async (req, res) => {
const order = await orderService.retrieveWithTotals(id, req.retrieveConfig)

res.json({
order: cleanResponseData(order, req.allowedProperties || []),
order: cleanResponseData(order, req.storeAllowedProperties || []),
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,9 @@ export default async (req, res) => {

const order = orders[0]

res.json({ order: cleanResponseData(order, req.allowedProperties || []) })
res.json({
order: cleanResponseData(order, req.storeAllowedProperties || []),
})
}

export class ShippingAddressPayload {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export default async (req, res) => {
)

res.json({
product: cleanResponseData(product, req.allowedProperties || []),
product: cleanResponseData(product, req.storeAllowedProperties || []),
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,10 @@ export default async (req, res) => {
])

res.json({
products: cleanResponseData(computedProducts, req.allowedProperties || []),
products: cleanResponseData(
computedProducts,
req.storeAllowedProperties || []
),
count,
offset: validated.offset,
limit: validated.limit,
Expand Down
4 changes: 3 additions & 1 deletion packages/medusa/src/types/global.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ declare global {
listConfig: FindConfig<unknown>
retrieveConfig: FindConfig<unknown>
filterableFields: Record<string, unknown>
allowedProperties: string[]
storeAllowedProperties: string[]
adminAllowedProperties: string[]
includes?: Record<string, boolean>
errors: string[]
}
}
Expand Down
46 changes: 39 additions & 7 deletions packages/medusa/src/utils/clean-response-data.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,53 @@
import { pick } from "lodash"

// TODO: once the legacy totals decoration will be removed.
// We will be able to only compute the totals if one of the total fields is present
// and therefore avoid totals computation if the user don't want them to appear in the response
// and therefore the below const will be removed
const EXCLUDED_FIELDS = [
riqwan marked this conversation as resolved.
Show resolved Hide resolved
"shipping_total",
"discount_total",
"tax_total",
"refunded_total",
"total",
"subtotal",
"paid_total",
"refundable_amount",
"gift_card_total",
"gift_card_tax_total",
"item_tax_total",
"shipping_tax_total",
"refundable",
"original_total",
"original_tax_total",
]

/**
* Filter response data to contain props specified in the fields array.
* Filter response data to contain props specified in the `storeAllowedProperties` or `adminAllowedProperties`.
* You can read more in the transformQuery middleware utility methods.
*
* @param data - record or an array of records in the response
* @param fields - record props allowed in the response
*/
function cleanResponseData<T>(data: T, fields: string[]) {
function cleanResponseData<T extends unknown | unknown[]>(
adrien2p marked this conversation as resolved.
Show resolved Hide resolved
data: T,
fields: string[]
): T extends [] ? Partial<T>[] : Partial<T> {
if (!fields.length) {
return data
return data as T extends [] ? Partial<T>[] : Partial<T>
}

if (Array.isArray(data)) {
return data.map((record) => pick(record, fields))
}
const isDataArray = Array.isArray(data)
const fieldsSet = new Set([...fields, ...EXCLUDED_FIELDS])

fields = [...fieldsSet]

let arrayData: Partial<T>[] = isDataArray ? data : [data]
arrayData = arrayData.map((record) => pick(record, fields))

return pick(data, fields)
return (isDataArray ? arrayData : arrayData[0]) as T extends []
? Partial<T>[]
: Partial<T>
}

export { cleanResponseData }