fix(utils): build query withDeleted remove auto detection (#12788)
**What**
Currently, filtering data providing a `deleted_at` value will automatically apply the `withDeleted` flag which in turns remove the default constraint apply to all queries `deleted_at: null`. The problem is that it does not account for the value assign to `deleted_at` leading to inconsistent behaviour depending on the value. e.g filtering with `deleted_at: { $eq: null }` where the expectation is to only filter the non deleted record will end up returning deleted record as well by applying the `withDeleted` filters.
This pr revert this auto detection if favor of the user providing `withDeleted` explicitly, as it is already supported , plus the filters.
Further more, some integration tests demonstrate how to filter deleted records (e.g product) from the api. While the api did not properly support it, this pr adds support to pass with_deleted flags to the query and being handled accordingly to our api support. Validators have been updated and product list end point benefit from it. Also, the list config type was already accepting such value which I have translated to the remote query config.
Also, since the previous pr was adjusting the product types, I ve adjusted them to match the expectation
This commit is contained in:
committed by
GitHub
parent
9d61bb7e71
commit
a833c3c98c
@@ -449,7 +449,7 @@ medusaIntegrationTestRunner({
|
||||
// BREAKING: Comparison operators changed, so eg. `gt` became `$gt`
|
||||
const response = await api
|
||||
.get(
|
||||
`/admin/products?deleted_at[$gt]=01-26-1990&q=test`,
|
||||
`/admin/products?deleted_at[$gt]=01-26-1990&with_deleted=true&q=test`,
|
||||
adminHeaders
|
||||
)
|
||||
.catch((err) => {
|
||||
@@ -519,7 +519,10 @@ medusaIntegrationTestRunner({
|
||||
|
||||
it("returns a list of deleted products", async () => {
|
||||
const response = await api
|
||||
.get(`/admin/products?deleted_at[$gt]=01-26-1990`, adminHeaders)
|
||||
.get(
|
||||
`/admin/products?deleted_at[$gt]=01-26-1990&with_deleted=true`,
|
||||
adminHeaders
|
||||
)
|
||||
.catch((err) => {
|
||||
console.log(err)
|
||||
})
|
||||
|
||||
@@ -141,6 +141,7 @@ export interface MedusaRequest<
|
||||
queryConfig: {
|
||||
fields: string[]
|
||||
pagination: { order?: Record<string, string>; skip: number; take?: number }
|
||||
withDeleted?: boolean
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -101,7 +101,13 @@ export function prepareListQuery<T extends RequestQueryFields, TEntity>(
|
||||
defaultLimit = 50,
|
||||
isList,
|
||||
} = queryConfig
|
||||
const { order, fields, limit = defaultLimit, offset = 0 } = validated
|
||||
const {
|
||||
order,
|
||||
fields,
|
||||
limit = defaultLimit,
|
||||
offset = 0,
|
||||
with_deleted,
|
||||
} = validated
|
||||
|
||||
// e.g *product.variants meaning that we want all fields from the product.variants
|
||||
// in that case it wont be part of the select but it will be part of the relations.
|
||||
@@ -220,6 +226,7 @@ export function prepareListQuery<T extends RequestQueryFields, TEntity>(
|
||||
skip: offset,
|
||||
take: limit,
|
||||
order: finalOrder,
|
||||
withDeleted: with_deleted,
|
||||
},
|
||||
remoteQueryConfig: {
|
||||
// Add starFields that are relations only on which we want all properties with a dedicated format to the remote query
|
||||
@@ -234,6 +241,7 @@ export function prepareListQuery<T extends RequestQueryFields, TEntity>(
|
||||
order: finalOrder,
|
||||
}
|
||||
: {},
|
||||
withDeleted: with_deleted,
|
||||
},
|
||||
}
|
||||
}
|
||||
@@ -255,6 +263,7 @@ export function prepareRetrieveQuery<T extends RequestQueryFields, TEntity>(
|
||||
remoteQueryConfig: {
|
||||
fields: remoteQueryConfig.fields,
|
||||
pagination: {},
|
||||
withDeleted: remoteQueryConfig.withDeleted,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
@@ -11,7 +11,8 @@ export const refetchEntities = async (
|
||||
idOrFilter: string | object,
|
||||
scope: MedusaContainer,
|
||||
fields: string[],
|
||||
pagination?: MedusaRequest["queryConfig"]["pagination"]
|
||||
pagination?: MedusaRequest["queryConfig"]["pagination"],
|
||||
withDeleted?: boolean
|
||||
) => {
|
||||
const remoteQuery = scope.resolve(ContainerRegistrationKeys.REMOTE_QUERY)
|
||||
const filters = isString(idOrFilter) ? { id: idOrFilter } : idOrFilter
|
||||
@@ -25,7 +26,7 @@ export const refetchEntities = async (
|
||||
delete filters.context
|
||||
}
|
||||
|
||||
const variables = { filters, ...context, ...pagination }
|
||||
const variables = { filters, ...context, ...pagination, withDeleted }
|
||||
|
||||
const queryObject = remoteQueryObjectFromString({
|
||||
entryPoint,
|
||||
|
||||
@@ -77,9 +77,10 @@ export function validateAndTransformQuery<TEntity extends BaseEntity>(
|
||||
}
|
||||
|
||||
delete req.allowed
|
||||
const query = normalizeQuery(req)
|
||||
const query = normalizeQuery(req) as Record<string, any>
|
||||
|
||||
const validated = await zodValidator(zodSchema, query)
|
||||
|
||||
const cnf = queryConfig.isList
|
||||
? prepareListQuery(validated, { ...queryConfig, allowed, restricted })
|
||||
: prepareRetrieveQuery(validated, {
|
||||
@@ -88,7 +89,8 @@ export function validateAndTransformQuery<TEntity extends BaseEntity>(
|
||||
restricted,
|
||||
})
|
||||
|
||||
req.validatedQuery = validated
|
||||
const { with_deleted, ...validatedQueryFilters } = validated
|
||||
req.validatedQuery = validatedQueryFilters
|
||||
req.filterableFields = getFilterableFields(req.validatedQuery)
|
||||
req.queryConfig = cnf.remoteQueryConfig as any
|
||||
req.remoteQueryConfig = req.queryConfig
|
||||
|
||||
@@ -80,7 +80,7 @@ export interface FindConfig<Entity> {
|
||||
|
||||
/**
|
||||
* An array of strings, each being relation names of the entity to retrieve in the result.
|
||||
*
|
||||
*
|
||||
* You can only retrieve data models defined in the same module. To retrieve linked data models
|
||||
* from other modules, use [Query](https://docs.medusajs.com/learn/fundamentals/module-links/query) instead.
|
||||
*/
|
||||
@@ -150,6 +150,11 @@ export type RequestQueryFields = {
|
||||
* The field to sort the data by. By default, the sort order is ascending. To change the order to descending, prefix the field name with `-`.
|
||||
*/
|
||||
order?: string
|
||||
|
||||
/**
|
||||
* Whether to include deleted records in the result.
|
||||
*/
|
||||
with_deleted?: boolean
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -688,27 +688,30 @@ export interface FilterableProductProps
|
||||
/**
|
||||
* The status to filter products by
|
||||
*/
|
||||
status?: ProductStatus | ProductStatus[]
|
||||
status?:
|
||||
| ProductStatus
|
||||
| ProductStatus[]
|
||||
| OperatorMap<ProductStatus | ProductStatus[]>
|
||||
/**
|
||||
* The titles to filter products by.
|
||||
*/
|
||||
title?: string | string[]
|
||||
title?: string | string[] | OperatorMap<string | string[]>
|
||||
/**
|
||||
* The handles to filter products by.
|
||||
*/
|
||||
handle?: string | string[]
|
||||
handle?: string | string[] | OperatorMap<string | string[]>
|
||||
/**
|
||||
* The skus to filter products by.
|
||||
*/
|
||||
sku?: string | string[]
|
||||
sku?: string | string[] | OperatorMap<string | string[]>
|
||||
/**
|
||||
* The IDs to filter products by.
|
||||
*/
|
||||
id?: string | string[]
|
||||
id?: string | string[] | OperatorMap<string | string[]>
|
||||
/**
|
||||
* The external IDs to filter products by.
|
||||
*/
|
||||
external_id?: string | string[]
|
||||
external_id?: string | string[] | OperatorMap<string | string[]>
|
||||
/**
|
||||
* Filters only or excluding gift card products
|
||||
*/
|
||||
@@ -736,27 +739,27 @@ export interface FilterableProductProps
|
||||
/**
|
||||
* Filter a product by the ID of the associated type
|
||||
*/
|
||||
type_id?: string | string[]
|
||||
type_id?: string | string[] | OperatorMap<string | string[]>
|
||||
/**
|
||||
* Filter a product by the IDs of their associated categories.
|
||||
*/
|
||||
categories?: { id: OperatorMap<string> } | { id: OperatorMap<string[]> }
|
||||
categories?: { id: string | string[] | OperatorMap<string | string[]> }
|
||||
/**
|
||||
* Filters a product by the IDs of their associated collections.
|
||||
*/
|
||||
collection_id?: string | string[] | OperatorMap<string>
|
||||
collection_id?: string | string[] | OperatorMap<string | string[]>
|
||||
/**
|
||||
* Filters a product based on when it was created
|
||||
*/
|
||||
created_at?: OperatorMap<string>
|
||||
created_at?: string | OperatorMap<string>
|
||||
/**
|
||||
* Filters a product based on when it was updated
|
||||
*/
|
||||
updated_at?: OperatorMap<string>
|
||||
updated_at?: string | OperatorMap<string>
|
||||
/**
|
||||
* Filters soft-deleted products based on the date they were deleted at.
|
||||
*/
|
||||
deleted_at?: OperatorMap<string>
|
||||
deleted_at?: string | OperatorMap<string>
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -78,7 +78,7 @@ describe("buildQuery", () => {
|
||||
|
||||
test("should handle withDeleted flag", () => {
|
||||
const filters = { deleted_at: "some-value" }
|
||||
const result = buildQuery(filters)
|
||||
const result = buildQuery(filters, { withDeleted: true })
|
||||
expect(result.options.filters).toHaveProperty(SoftDeletableFilterKey)
|
||||
expect(result.options.filters?.[SoftDeletableFilterKey]).toEqual({
|
||||
withDeleted: true,
|
||||
|
||||
@@ -3,13 +3,6 @@ import { deduplicate, isObject } from "../common"
|
||||
|
||||
import { SoftDeletableFilterKey } from "../dal/mikro-orm/mikro-orm-soft-deletable-filter"
|
||||
|
||||
// Following convention here is fine, we can make it configurable if needed.
|
||||
const DELETED_AT_FIELD_NAME = "deleted_at"
|
||||
|
||||
type FilterFlags = {
|
||||
withDeleted?: boolean
|
||||
}
|
||||
|
||||
export function buildQuery<const T = any>(
|
||||
filters: Record<string, any> = {},
|
||||
config: FindConfig<InferRepositoryReturnType<T>> & {
|
||||
@@ -17,8 +10,7 @@ export function buildQuery<const T = any>(
|
||||
} = {}
|
||||
): Required<DAL.FindOptions<T>> {
|
||||
const where = {} as DAL.FilterQuery<T>
|
||||
const filterFlags: FilterFlags = {}
|
||||
buildWhere(filters, where, filterFlags)
|
||||
buildWhere(filters, where)
|
||||
|
||||
delete config.primaryKeyFields
|
||||
|
||||
@@ -41,7 +33,7 @@ export function buildQuery<const T = any>(
|
||||
>["options"]["orderBy"]
|
||||
}
|
||||
|
||||
if (config.withDeleted || filterFlags.withDeleted) {
|
||||
if (config.withDeleted) {
|
||||
findOptions.filters ??= {}
|
||||
findOptions.filters[SoftDeletableFilterKey] = {
|
||||
withDeleted: true,
|
||||
@@ -63,16 +55,8 @@ export function buildQuery<const T = any>(
|
||||
return { where, options: findOptions } as Required<DAL.FindOptions<T>>
|
||||
}
|
||||
|
||||
function buildWhere(
|
||||
filters: Record<string, any> = {},
|
||||
where = {},
|
||||
flags: FilterFlags = {}
|
||||
) {
|
||||
function buildWhere(filters: Record<string, any> = {}, where = {}) {
|
||||
for (let [prop, value] of Object.entries(filters)) {
|
||||
if (prop === DELETED_AT_FIELD_NAME) {
|
||||
flags.withDeleted = true
|
||||
}
|
||||
|
||||
if (["$or", "$and"].includes(prop)) {
|
||||
if (!Array.isArray(value)) {
|
||||
throw new Error(`Expected array for ${prop} but got ${value}`)
|
||||
@@ -80,7 +64,7 @@ function buildWhere(
|
||||
|
||||
where[prop] = value.map((val) => {
|
||||
const deepWhere = {}
|
||||
buildWhere(val, deepWhere, flags)
|
||||
buildWhere(val, deepWhere)
|
||||
return deepWhere
|
||||
})
|
||||
continue
|
||||
@@ -93,7 +77,7 @@ function buildWhere(
|
||||
|
||||
if (isObject(value)) {
|
||||
where[prop] = {}
|
||||
buildWhere(value, where[prop], flags)
|
||||
buildWhere(value, where[prop])
|
||||
continue
|
||||
}
|
||||
|
||||
|
||||
@@ -43,7 +43,8 @@ async function getProducts(
|
||||
req.filterableFields,
|
||||
req.scope,
|
||||
selectFields,
|
||||
req.queryConfig.pagination
|
||||
req.queryConfig.pagination,
|
||||
req.queryConfig.withDeleted
|
||||
)
|
||||
|
||||
res.json({
|
||||
@@ -75,6 +76,7 @@ async function getProductsWithIndexEngine(
|
||||
fields: req.queryConfig.fields ?? [],
|
||||
filters: filters,
|
||||
pagination: req.queryConfig.pagination,
|
||||
withDeleted: req.queryConfig.withDeleted,
|
||||
})
|
||||
|
||||
res.json({
|
||||
|
||||
@@ -92,6 +92,12 @@ export const createFindParams = ({
|
||||
order: order
|
||||
? z.string().optional().default(order)
|
||||
: z.string().optional(),
|
||||
with_deleted: z.preprocess((val) => {
|
||||
if (val && typeof val === "string") {
|
||||
return val === "true" ? true : val === "false" ? false : val
|
||||
}
|
||||
return val
|
||||
}, z.boolean().optional()),
|
||||
})
|
||||
)
|
||||
}
|
||||
|
||||
@@ -1277,9 +1277,14 @@ moduleIntegrationTestRunner<IProductModuleService>({
|
||||
|
||||
await service.softDeleteProducts([products[0].id])
|
||||
|
||||
const softDeleted = await service.listProducts({
|
||||
deleted_at: { $gt: "01-01-2022" },
|
||||
})
|
||||
const softDeleted = await service.listProducts(
|
||||
{
|
||||
deleted_at: { $gt: "01-01-2022" },
|
||||
},
|
||||
{
|
||||
withDeleted: true,
|
||||
}
|
||||
)
|
||||
|
||||
expect(softDeleted).toHaveLength(1)
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user