Skip to content

Commit 4f452ac

Browse files
authored
fix: isValidID validation (#15217)
## Scripts ### `script:setup-figma` Sets up the local development environment for testing with content-api: - Clones `enterprise-plugins` repo if not present (sibling directory) - Runs `pnpm install` in enterprise-plugins - Removes `@payloadcms/figma` from `test/package.json` to rely on vitest alias instead ### `test:int:summary` Shows less noisy output for integration tests - reports how many tests failed without detailed error messages. The test suites to run can be easily modified via a hardcoded array. This script is particularly useful when many tests fail (e.g., when starting to develop a new db-adapter). To run it for a specific adapter, prefix with `PAYLOAD_DATABASE` env var. Can be removed once content-api is complete if desired. ## Bug Fixes ### Fixed `isValidID` validation - Now properly checks that value type matches the expected ID type - Rejects numbers when `type === 'text'` (fixes content-api adapter validation) - Only accepts numbers when `type === 'number'` - Added explicit `return false` for invalid cases ## Test Improvements ### Refactored "Schema generation" tests Moved conditional logic inside `it()` blocks instead of wrapping them, preventing "no tests found" errors with new db-adapters. ### Added type casting tests Tests for automatic type coercion in database adapters: - String-to-number conversion in `hasMany` number fields - Date field storage and retrieval as ISO strings - Unix timestamp to ISO string conversion See rationale in the comments of [this PR](payloadcms/enterprise-plugins#300). ### Configured Vitest to run tests from `src/` instead of `dist/` This was very difficult with Jest and eliminates the need to build in watch mode during development. --- **Note:** Current setup is designed for local content-api and db-adapter development. Small modifications will be made in the future to run these tests in CI against a deployed staging content-api URL.
1 parent 591f9d2 commit 4f452ac

10 files changed

Lines changed: 544 additions & 146 deletions

File tree

package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@
122122
"script:license-check": "pnpm --filter scripts license-check",
123123
"script:list-published": "pnpm --filter releaser list-published",
124124
"script:pack": "pnpm --filter scripts pack-all-to-dest",
125+
"script:setup-figma": "./scripts/setup-figma-dev.sh",
125126
"pretest": "pnpm build",
126127
"test": "pnpm test:int && pnpm test:components && pnpm test:e2e",
127128
"test:e2e": "pnpm runts ./test/runE2E.ts",
@@ -135,6 +136,7 @@
135136
"test:int:firestore": "cross-env NODE_OPTIONS=\"--no-deprecation --no-experimental-strip-types\" NODE_NO_WARNINGS=1 PAYLOAD_DATABASE=firestore DISABLE_LOGGING=true vitest --project int",
136137
"test:int:postgres": "cross-env NODE_OPTIONS=\"--no-deprecation --no-experimental-strip-types\" NODE_NO_WARNINGS=1 PAYLOAD_DATABASE=postgres DISABLE_LOGGING=true vitest --project int",
137138
"test:int:sqlite": "cross-env NODE_OPTIONS=\"--no-deprecation --no-experimental-strip-types\" NODE_NO_WARNINGS=1 PAYLOAD_DATABASE=sqlite DISABLE_LOGGING=true vitest --project int",
139+
"test:int:summary": "pnpm runts ./test/runTestsWithSummary.ts",
138140
"test:types": "tstyche",
139141
"test:unit": "vitest run --project unit",
140142
"translateNewKeys": "pnpm --filter @tools/scripts run generateTranslations:core"

packages/payload/src/utilities/isValidID.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ const ObjectId = 'default' in ObjectIdImport ? ObjectIdImport.default : ObjectId
55
export const isValidID = (
66
value: number | string,
77
type: 'number' | 'ObjectID' | 'text',
8-
// @ts-expect-error - vestiges of when tsconfig was not strict. Feel free to improve
98
): boolean => {
109
if (type === 'text' && value) {
1110
if (['object', 'string'].includes(typeof value)) {
@@ -15,11 +14,13 @@ export const isValidID = (
1514
return false
1615
}
1716

18-
if (typeof value === 'number' && !Number.isNaN(value)) {
17+
if (type === 'number' && typeof value === 'number' && !Number.isNaN(value)) {
1918
return true
2019
}
2120

2221
if (type === 'ObjectID') {
2322
return ObjectId.isValid(String(value))
2423
}
24+
25+
return false
2526
}

scripts/setup-figma-dev.sh

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
#!/bin/bash
2+
set -e
3+
4+
# Get the absolute path to the repo root
5+
REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
6+
ENTERPRISE_DIR="$REPO_ROOT/../enterprise-plugins"
7+
8+
echo "Checking for enterprise-plugins at $ENTERPRISE_DIR..."
9+
10+
if [ ! -d "$ENTERPRISE_DIR" ]; then
11+
echo "Directory not found. Cloning enterprise-plugins..."
12+
# Using git clone
13+
git clone https://github.com/payloadcms/enterprise-plugins "$ENTERPRISE_DIR"
14+
else
15+
echo "enterprise-plugins found."
16+
fi
17+
18+
# Install dependencies in enterprise-plugins
19+
echo "Installing dependencies in enterprise-plugins..."
20+
cd "$ENTERPRISE_DIR"
21+
pnpm install
22+
cd "$REPO_ROOT"
23+
24+
# Remove local file dependency from test/package.json if it exists
25+
# We want to rely on the vitest alias instead of package.json file: protocol
26+
if grep -q "@payloadcms/figma" "$REPO_ROOT/test/package.json"; then
27+
echo "Removing @payloadcms/figma from test/package.json dependencies..."
28+
cd "$REPO_ROOT/test"
29+
pnpm remove @payloadcms/figma || true # ignore if not found by pnpm but grep found it
30+
cd "$REPO_ROOT"
31+
fi
32+
33+
echo "Setup complete. You can now run 'PAYLOAD_DATABASE=content-api pnpm test:int _community'."

test/database/getConfig.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,15 @@ export const getConfig: () => Partial<Config> = () => ({
205205
type: 'number',
206206
defaultValue: 1,
207207
},
208+
{
209+
name: 'numbersHasMany',
210+
type: 'number',
211+
hasMany: true,
212+
},
213+
{
214+
name: 'publishDate',
215+
type: 'date',
216+
},
208217
{
209218
type: 'blocks',
210219
name: 'blocks',

test/database/int.spec.ts

Lines changed: 103 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -2806,61 +2806,60 @@ describe('database', () => {
28062806
})
28072807

28082808
describe('Schema generation', { db: 'drizzle' }, () => {
2809-
if (
2810-
process.env.PAYLOAD_DATABASE.includes('postgres') ||
2811-
process.env.PAYLOAD_DATABASE === 'supabase'
2812-
) {
2813-
it('should generate Drizzle Postgres schema', async () => {
2814-
const generatedAdapterName = process.env.PAYLOAD_DATABASE
2809+
it('should generate Drizzle Postgres schema', async () => {
2810+
const generatedAdapterName = process.env.PAYLOAD_DATABASE
2811+
if (!generatedAdapterName?.includes('postgres') && generatedAdapterName !== 'supabase') {
2812+
return
2813+
}
28152814

2816-
const outputFile = path.resolve(dirname, `${generatedAdapterName}.generated-schema.ts`)
2815+
const outputFile = path.resolve(dirname, `${generatedAdapterName}.generated-schema.ts`)
28172816

2818-
await payload.db.generateSchema({
2819-
outputFile,
2820-
})
2817+
await payload.db.generateSchema({
2818+
outputFile,
2819+
})
28212820

2822-
const module = await import(outputFile)
2821+
const module = await import(outputFile)
28232822

2824-
// Confirm that the generated module exports every relation
2825-
for (const relation in payload.db.relations) {
2826-
expect(module).toHaveProperty(relation)
2827-
}
2823+
// Confirm that the generated module exports every relation
2824+
for (const relation in payload.db.relations) {
2825+
expect(module).toHaveProperty(relation)
2826+
}
28282827

2829-
// Confirm that module exports every table
2830-
for (const table in payload.db.tables) {
2831-
expect(module).toHaveProperty(table)
2832-
}
2828+
// Confirm that module exports every table
2829+
for (const table in payload.db.tables) {
2830+
expect(module).toHaveProperty(table)
2831+
}
28332832

2834-
// Confirm that module exports every enum
2835-
for (const enumName in payload.db.enums) {
2836-
expect(module).toHaveProperty(enumName)
2837-
}
2838-
})
2839-
}
2833+
// Confirm that module exports every enum
2834+
for (const enumName in payload.db.enums) {
2835+
expect(module).toHaveProperty(enumName)
2836+
}
2837+
})
28402838

2841-
if (process.env.PAYLOAD_DATABASE.includes('sqlite')) {
2842-
it('should generate Drizzle SQLite schema', async () => {
2843-
const generatedAdapterName = process.env.PAYLOAD_DATABASE
2839+
it('should generate Drizzle SQLite schema', async () => {
2840+
const generatedAdapterName = process.env.PAYLOAD_DATABASE
2841+
if (!generatedAdapterName?.includes('sqlite')) {
2842+
return
2843+
}
28442844

2845-
const outputFile = path.resolve(dirname, `${generatedAdapterName}.generated-schema.ts`)
2845+
const outputFile = path.resolve(dirname, `${generatedAdapterName}.generated-schema.ts`)
28462846

2847-
await payload.db.generateSchema({
2848-
outputFile,
2849-
})
2847+
await payload.db.generateSchema({
2848+
outputFile,
2849+
})
28502850

2851-
const module = await import(outputFile)
2851+
const module = await import(outputFile)
28522852

2853-
// Confirm that the generated module exports every relation
2854-
for (const relation in payload.db.relations) {
2855-
expect(module).toHaveProperty(relation)
2856-
}
2853+
// Confirm that the generated module exports every relation
2854+
for (const relation in payload.db.relations) {
2855+
expect(module).toHaveProperty(relation)
2856+
}
28572857

2858-
// Confirm that module exports every table
2859-
for (const table in payload.db.tables) {
2860-
expect(module).toHaveProperty(table)
2861-
}
2862-
})
2863-
}
2858+
// Confirm that module exports every table
2859+
for (const table in payload.db.tables) {
2860+
expect(module).toHaveProperty(table)
2861+
}
2862+
})
28642863
})
28652864

28662865
describe('drizzle: schema hooks', () => {
@@ -3573,6 +3572,63 @@ describe('database', () => {
35733572
expect(result.text).toStrictEqual('1')
35743573
})
35753574

3575+
it('should convert strings to numbers in hasMany number fields', async () => {
3576+
const result = await payload.create({
3577+
collection: postsSlug,
3578+
data: {
3579+
title: 'testing-numbers-hasMany',
3580+
// @ts-expect-error passing strings when numbers are expected
3581+
numbersHasMany: ['10', '20', '30'],
3582+
},
3583+
})
3584+
3585+
expect(result.numbersHasMany).toEqual([10, 20, 30])
3586+
})
3587+
3588+
it('should store and retrieve date fields as ISO strings', async () => {
3589+
const testDate = new Date('2024-01-15T10:30:00.000Z')
3590+
3591+
const result = await payload.create({
3592+
collection: postsSlug,
3593+
data: {
3594+
title: 'testing-date-field',
3595+
publishDate: testDate,
3596+
},
3597+
})
3598+
3599+
// Dates should be stored as ISO strings
3600+
expect(typeof result.publishDate).toBe('string')
3601+
expect(result.publishDate).toBe('2024-01-15T10:30:00.000Z')
3602+
3603+
// Reading back should also return ISO string
3604+
const retrieved = await payload.findByID({
3605+
collection: postsSlug,
3606+
id: result.id,
3607+
})
3608+
3609+
expect(typeof retrieved.publishDate).toBe('string')
3610+
expect(retrieved.publishDate).toBe('2024-01-15T10:30:00.000Z')
3611+
})
3612+
3613+
it('should convert Unix timestamps to ISO strings for date fields', async () => {
3614+
// Unix timestamp for 2024-01-15T10:30:00.000Z
3615+
const unixTimestamp = 1705314600000
3616+
3617+
// Using payload.db.create to bypass Payload's validation and test adapter coercion
3618+
const result = await payload.db.create({
3619+
collection: postsSlug,
3620+
data: {
3621+
title: 'testing-date-coercion',
3622+
publishDate: unixTimestamp,
3623+
},
3624+
req: {} as any,
3625+
})
3626+
3627+
// Should be converted to ISO string
3628+
expect(typeof result.publishDate).toBe('string')
3629+
expect(result.publishDate).toBe('2024-01-15T10:30:00.000Z')
3630+
})
3631+
35763632
it('should not allow to query by a field with `virtual: true`', async () => {
35773633
await expect(
35783634
payload.find({
@@ -3585,10 +3641,13 @@ describe('database', () => {
35853641
it('should not allow document creation with relationship data to an invalid document ID', async () => {
35863642
let invalidDoc
35873643

3644+
// mongo requires ObjectId, postgres UUID and content-api number (wrong type for text ID)
3645+
const invalidId = payload.db.name === 'content_api' ? 1 : 'not-real-id'
3646+
35883647
try {
35893648
invalidDoc = await payload.create({
35903649
collection: 'relation-b',
3591-
data: { relationship: 'not-real-id', title: 'invalid' },
3650+
data: { relationship: invalidId, title: 'invalid' },
35923651
})
35933652
} catch (error) {
35943653
// instanceof checks don't work with libsql

test/database/payload-types.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,8 @@ export interface Post {
264264
text?: string | null;
265265
number?: number | null;
266266
numberDefault?: number | null;
267+
numbersHasMany?: number[] | null;
268+
publishDate?: string | null;
267269
blocks?:
268270
| {
269271
nested?:
@@ -926,6 +928,8 @@ export interface PostsSelect<T extends boolean = true> {
926928
text?: T;
927929
number?: T;
928930
numberDefault?: T;
931+
numbersHasMany?: T;
932+
publishDate?: T;
929933
blocks?:
930934
| T
931935
| {
@@ -1520,6 +1524,6 @@ export interface Auth {
15201524

15211525

15221526
declare module 'payload' {
1523-
// @ts-ignore
1527+
// @ts-ignore
15241528
export interface GeneratedTypes extends Config {}
1525-
}
1529+
}

test/generateDatabaseAdapter.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,16 @@ export const allDatabaseAdapters = {
9595
},
9696
readReplicas: [process.env.POSTGRES_REPLICA_URL],
9797
})
98+
`,
99+
'content-api': `
100+
import { contentAPIAdapter } from '@payloadcms/figma'
101+
export const databaseAdapter = contentAPIAdapter({
102+
auth: {
103+
mode: 'devJwt',
104+
},
105+
url: process.env.CONTENT_API_URL || 'http://localhost:8080',
106+
contentSystemId: process.env.CONTENT_SYSTEM_ID || '00000000-0000-4000-8000-000000000001',
107+
})
98108
`,
99109
'vercel-postgres-read-replica': `
100110
import { vercelPostgresAdapter } from '@payloadcms/db-vercel-postgres'

0 commit comments

Comments
 (0)