From d2fe9b0807e67a63283f4e697cee50920fdea0d2 Mon Sep 17 00:00:00 2001 From: Jarrod Flesch <30633324+JarrodMFlesch@users.noreply.github.com> Date: Mon, 10 Feb 2025 16:29:08 -0500 Subject: [PATCH] fix(db-mongodb): ensures same level operators are respected (#11087) ### What? If you had multiple operator constraints on a single field, the last one defined would be the only one used. Example: ```ts where: { id: { in: [doc2.id], not_in: [], // <-- only respected this operator constraint }, } ``` and ```ts where: { id: { not_in: [], in: [doc2.id], // <-- only respected this operator constraint }, } ``` They would yield different results. ### Why? The results were not merged into an `$and` query inside parseParams. ### How? Merges the results within an `$and` constraint. Fixes https://github.com/payloadcms/payload/issues/10944 Supersedes https://github.com/payloadcms/payload/pull/11011 --- .../db-mongodb/src/queries/parseParams.ts | 47 +++++++----- .../drizzle/src/queries/sanitizeQueryValue.ts | 4 +- test/database/int.spec.ts | 76 ++++++++++++++++--- test/versions/int.spec.ts | 15 ++-- 4 files changed, 107 insertions(+), 35 deletions(-) diff --git a/packages/db-mongodb/src/queries/parseParams.ts b/packages/db-mongodb/src/queries/parseParams.ts index 8fbb8e70f..b27a9b545 100644 --- a/packages/db-mongodb/src/queries/parseParams.ts +++ b/packages/db-mongodb/src/queries/parseParams.ts @@ -52,30 +52,37 @@ export async function parseParams({ // So we need to loop on keys again here to handle each operator independently const pathOperators = where[relationOrPath] if (typeof pathOperators === 'object') { - for (const operator of Object.keys(pathOperators)) { - if (validOperatorSet.has(operator as Operator)) { - const searchParam = await buildSearchParam({ - collectionSlug, - fields, - globalSlug, - incomingPath: relationOrPath, - locale, - operator, - payload, - val: pathOperators[operator], - }) + const validOperators = Object.keys(pathOperators).filter((operator) => + validOperatorSet.has(operator as Operator), + ) + for (const operator of validOperators) { + const searchParam = await buildSearchParam({ + collectionSlug, + fields, + globalSlug, + incomingPath: relationOrPath, + locale, + operator, + payload, + val: pathOperators[operator], + }) - if (searchParam?.value && searchParam?.path) { - result = { - ...result, - [searchParam.path]: searchParam.value, + if (searchParam?.value && searchParam?.path) { + if (validOperators.length > 1) { + if (!result.$and) { + result.$and = [] } - } else if (typeof searchParam?.value === 'object') { - result = deepMergeWithCombinedArrays(result, searchParam.value, { - // dont clone Types.ObjectIDs - clone: false, + result.$and.push({ + [searchParam.path]: searchParam.value, }) + } else { + result[searchParam.path] = searchParam.value } + } else if (typeof searchParam?.value === 'object') { + result = deepMergeWithCombinedArrays(result, searchParam.value, { + // dont clone Types.ObjectIDs + clone: false, + }) } } } diff --git a/packages/drizzle/src/queries/sanitizeQueryValue.ts b/packages/drizzle/src/queries/sanitizeQueryValue.ts index c8ca2f3bd..2a7cfc8c9 100644 --- a/packages/drizzle/src/queries/sanitizeQueryValue.ts +++ b/packages/drizzle/src/queries/sanitizeQueryValue.ts @@ -87,9 +87,11 @@ export const sanitizeQueryValue = ({ if (field.type === 'number') { formattedValue = formattedValue.map((arrayVal) => parseFloat(arrayVal)) } + } else if (typeof formattedValue === 'number') { + formattedValue = [formattedValue] } - if (!Array.isArray(formattedValue) || formattedValue.length === 0) { + if (!Array.isArray(formattedValue)) { return null } } diff --git a/test/database/int.spec.ts b/test/database/int.spec.ts index 0a2cd81ca..bf1987538 100644 --- a/test/database/int.spec.ts +++ b/test/database/int.spec.ts @@ -22,12 +22,13 @@ import { } from 'payload' import { fileURLToPath } from 'url' +import type { Global2 } from './payload-types.js' + import { devUser } from '../credentials.js' import { initPayloadInt } from '../helpers/initPayloadInt.js' import { isMongoose } from '../helpers/isMongoose.js' import removeFiles from '../helpers/removeFiles.js' import { errorOnUnnamedFieldsSlug, postsSlug } from './shared.js' -import { Global2 } from './payload-types.js' const filename = fileURLToPath(import.meta.url) const dirname = path.dirname(filename) @@ -893,7 +894,7 @@ describe('database', () => { errorMessage = e.message } - await expect(errorMessage).toBe('The following field is invalid: Title') + expect(errorMessage).toBe('The following field is invalid: Title') }) it('should return validation errors in response', async () => { @@ -913,7 +914,7 @@ describe('database', () => { }, }) } catch (e: any) { - await expect(e.message).toMatch( + expect(e.message).toMatch( payload.db.name === 'mongoose' ? 'posts validation failed: D1.D2.D3.D4: Cast to string failed for value "{}" (type Object) at path "D4"' : payload.db.name === 'sqlite' @@ -1360,11 +1361,11 @@ describe('database', () => { }) it('payload.db.createGlobal should have globalType, updatedAt, createdAt fields', async () => { - let timestamp = Date.now() + const timestamp = Date.now() let result = (await payload.db.createGlobal({ slug: 'global-2', data: { text: 'this is global-2' }, - })) as Global2 & { globalType: string } + })) as { globalType: string } & Global2 expect(result.text).toBe('this is global-2') expect(result.globalType).toBe('global-2') @@ -1376,7 +1377,7 @@ describe('database', () => { result = (await payload.db.updateGlobal({ slug: 'global-2', data: { text: 'this is global-2 but updated' }, - })) as Global2 & { globalType: string } + })) as { globalType: string } & Global2 expect(result.text).toBe('this is global-2 but updated') expect(result.globalType).toBe('global-2') @@ -1385,11 +1386,11 @@ describe('database', () => { }) it('payload.updateGlobal should have globalType, updatedAt, createdAt fields', async () => { - let timestamp = Date.now() + const timestamp = Date.now() let result = (await payload.updateGlobal({ slug: 'global-3', data: { text: 'this is global-3' }, - })) as Global2 & { globalType: string } + })) as { globalType: string } & Global2 expect(result.text).toBe('this is global-3') expect(result.globalType).toBe('global-3') @@ -1401,11 +1402,68 @@ describe('database', () => { result = (await payload.updateGlobal({ slug: 'global-3', data: { text: 'this is global-3 but updated' }, - })) as Global2 & { globalType: string } + })) as { globalType: string } & Global2 expect(result.text).toBe('this is global-3 but updated') expect(result.globalType).toBe('global-3') expect(createdAt).toEqual(new Date(result.createdAt as string).getTime()) expect(createdAt).toBeLessThan(new Date(result.updatedAt as string).getTime()) }) + + it('should group where conditions with AND', async () => { + // create 2 docs + await payload.create({ + collection: postsSlug, + data: { + title: 'post 1', + }, + }) + + const doc2 = await payload.create({ + collection: postsSlug, + data: { + title: 'post 2', + }, + }) + + const query1 = await payload.find({ + collection: postsSlug, + where: { + id: { + // where order, `in` last + not_in: [], + in: [doc2.id], + }, + }, + }) + + const query2 = await payload.find({ + collection: postsSlug, + where: { + id: { + // where order, `in` first + in: [doc2.id], + not_in: [], + }, + }, + }) + + const query3 = await payload.find({ + collection: postsSlug, + where: { + and: [ + { + id: { + in: [doc2.id], + not_in: [], + }, + }, + ], + }, + }) + + expect(query1.totalDocs).toEqual(1) + expect(query2.totalDocs).toEqual(1) + expect(query3.totalDocs).toEqual(1) + }) }) diff --git a/test/versions/int.spec.ts b/test/versions/int.spec.ts index 8ac7bdfd0..82e1e34b1 100644 --- a/test/versions/int.spec.ts +++ b/test/versions/int.spec.ts @@ -1,8 +1,8 @@ -import { createLocalReq, Payload } from 'payload' -import { schedulePublishHandler } from '@payloadcms/ui/utilities/schedulePublishHandler' +import type { Payload } from 'payload'; +import { schedulePublishHandler } from '@payloadcms/ui/utilities/schedulePublishHandler' import path from 'path' -import { ValidationError } from 'payload' +import { createLocalReq , ValidationError } from 'payload' import { wait } from 'payload/shared' import { fileURLToPath } from 'url' @@ -1153,6 +1153,11 @@ describe('Versions', () => { const allDocs = await payload.find({ collection: 'draft-posts', draft: true, + where: { + title: { + like: 'title', + }, + }, }) expect(allDocs.docs.length).toBeGreaterThan(1) @@ -1169,14 +1174,14 @@ describe('Versions', () => { }, { title: { - like: 'Published', + like: 'title', }, }, ], }, }) - expect(results.docs).toHaveLength(1) + expect(results.docs).toHaveLength(allDocs.docs.length - 1) }) })