From ee1a91ee7c008afac4f4ebcc41bab85444bae4b4 Mon Sep 17 00:00:00 2001 From: Jarrod Flesch <30633324+JarrodMFlesch@users.noreply.github.com> Date: Thu, 21 Nov 2024 14:16:01 -0500 Subject: [PATCH] fix: unable to load documents with non-standard ids (#9407) ### What? Non-standard ids caused an issue when finding the document on the server. This is an odd regression, in 2.0 we were fetching the document on the client so the request would handle decoding the url. Now we are fetching the document on the server and need to do this manually when reading id from route params. ### Why? The slug pulled out of the url for an id of `id 1` would equate to `id%201` which would fail in the `payload.find` call since there is not an id stored as `id%201` but instead `id 1`. ### How? Wherever we are calling payload.find in the views and querying by `id` it gets ran through a helper function that decodes it properly. Fixes #9373 --- .../src/views/Document/getDocPreferences.ts | 4 ++- .../src/views/Document/getDocumentData.ts | 5 +++- .../next/src/views/Document/getIsLocked.ts | 4 ++- .../next/src/views/Document/getVersions.ts | 5 +++- packages/ui/src/elements/IDLabel/index.tsx | 3 +- .../src/elements/Table/DefaultCell/index.tsx | 2 +- packages/ui/src/exports/shared/index.ts | 1 + packages/ui/src/utilities/sanitizeID.ts | 11 +++++++ test/collections-rest/config.ts | 4 +-- test/fields/collections/CustomID/index.ts | 14 +++++++++ test/fields/config.ts | 2 ++ test/fields/e2e.spec.ts | 29 +++++++++++++++++++ test/fields/payload-types.ts | 28 ++++++++++++++++-- test/fields/slugs.ts | 1 + 14 files changed, 103 insertions(+), 10 deletions(-) create mode 100644 packages/ui/src/utilities/sanitizeID.ts create mode 100644 test/fields/collections/CustomID/index.ts diff --git a/packages/next/src/views/Document/getDocPreferences.ts b/packages/next/src/views/Document/getDocPreferences.ts index ae0f06ea260..e56dc5ce21d 100644 --- a/packages/next/src/views/Document/getDocPreferences.ts +++ b/packages/next/src/views/Document/getDocPreferences.ts @@ -1,5 +1,7 @@ import type { DocumentPreferences, Payload, TypedUser } from 'payload' +import { sanitizeID } from '@payloadcms/ui/shared' + type Args = { collectionSlug?: string globalSlug?: string @@ -44,7 +46,7 @@ export const getDocPreferences = async ({ }, { 'user.value': { - equals: user.id, + equals: sanitizeID(user.id), }, }, ], diff --git a/packages/next/src/views/Document/getDocumentData.ts b/packages/next/src/views/Document/getDocumentData.ts index 03f61170d05..b1cdb8357b6 100644 --- a/packages/next/src/views/Document/getDocumentData.ts +++ b/packages/next/src/views/Document/getDocumentData.ts @@ -1,5 +1,7 @@ import type { Locale, Payload, TypedUser, TypeWithID } from 'payload' +import { sanitizeID } from '@payloadcms/ui/shared' + type Args = { collectionSlug?: string globalSlug?: string @@ -10,13 +12,14 @@ type Args = { } export const getDocumentData = async ({ - id, + id: idArg, collectionSlug, globalSlug, locale, payload, user, }: Args): Promise | TypeWithID> => { + const id = sanitizeID(idArg) let resolvedData: Record | TypeWithID = null try { diff --git a/packages/next/src/views/Document/getIsLocked.ts b/packages/next/src/views/Document/getIsLocked.ts index 312e268378f..e88d813b477 100644 --- a/packages/next/src/views/Document/getIsLocked.ts +++ b/packages/next/src/views/Document/getIsLocked.ts @@ -6,6 +6,8 @@ import type { Where, } from 'payload' +import { sanitizeID } from '@payloadcms/ui/shared' + type Args = { collectionConfig?: SanitizedCollectionConfig globalConfig?: SanitizedGlobalConfig @@ -48,7 +50,7 @@ export const getIsLocked = async ({ where.and = [ { 'document.value': { - equals: id, + equals: sanitizeID(id), }, }, { diff --git a/packages/next/src/views/Document/getVersions.ts b/packages/next/src/views/Document/getVersions.ts index d35557875f1..f18209f7ea7 100644 --- a/packages/next/src/views/Document/getVersions.ts +++ b/packages/next/src/views/Document/getVersions.ts @@ -6,6 +6,8 @@ import type { TypedUser, } from 'payload' +import { sanitizeID } from '@payloadcms/ui/shared' + type Args = { collectionConfig?: SanitizedCollectionConfig docPermissions: SanitizedDocumentPermissions @@ -26,7 +28,7 @@ type Result = Promise<{ // TODO: in the future, we can parallelize some of these queries // this will speed up the API by ~30-100ms or so export const getVersions = async ({ - id, + id: idArg, collectionConfig, docPermissions, globalConfig, @@ -34,6 +36,7 @@ export const getVersions = async ({ payload, user, }: Args): Result => { + const id = sanitizeID(idArg) let publishedQuery let hasPublishedDoc = false let mostRecentVersionIsAutosaved = false diff --git a/packages/ui/src/elements/IDLabel/index.tsx b/packages/ui/src/elements/IDLabel/index.tsx index 52986c60217..c224c392039 100644 --- a/packages/ui/src/elements/IDLabel/index.tsx +++ b/packages/ui/src/elements/IDLabel/index.tsx @@ -2,6 +2,7 @@ import React from 'react' import './index.scss' +import { sanitizeID } from '../../utilities/sanitizeID.js' const baseClass = 'id-label' @@ -13,6 +14,6 @@ export const IDLabel: React.FC<{ className?: string; id: string; prefix?: string
{prefix}   - {id} + {sanitizeID(id)}
) diff --git a/packages/ui/src/elements/Table/DefaultCell/index.tsx b/packages/ui/src/elements/Table/DefaultCell/index.tsx index 3e3efddccad..07385576b89 100644 --- a/packages/ui/src/elements/Table/DefaultCell/index.tsx +++ b/packages/ui/src/elements/Table/DefaultCell/index.tsx @@ -60,7 +60,7 @@ export const DefaultCell: React.FC = (props) => { wrapElementProps.href = collectionConfig?.slug ? formatAdminURL({ adminRoute, - path: `/collections/${collectionConfig?.slug}/${rowData.id}`, + path: `/collections/${collectionConfig?.slug}/${encodeURIComponent(rowData.id)}`, }) : '' } diff --git a/packages/ui/src/exports/shared/index.ts b/packages/ui/src/exports/shared/index.ts index 33cad82d318..ae7387f8ebf 100644 --- a/packages/ui/src/exports/shared/index.ts +++ b/packages/ui/src/exports/shared/index.ts @@ -27,3 +27,4 @@ export { hasSavePermission } from '../../utilities/hasSavePermission.js' export { isClientUserObject } from '../../utilities/isClientUserObject.js' export { isEditing } from '../../utilities/isEditing.js' export { mergeListSearchAndWhere } from '../../utilities/mergeListSearchAndWhere.js' +export { sanitizeID } from '../../utilities/sanitizeID.js' diff --git a/packages/ui/src/utilities/sanitizeID.ts b/packages/ui/src/utilities/sanitizeID.ts new file mode 100644 index 00000000000..5e6f0e604c8 --- /dev/null +++ b/packages/ui/src/utilities/sanitizeID.ts @@ -0,0 +1,11 @@ +export function sanitizeID(id: number | string): number | string { + if (id === undefined) { + return id + } + + if (typeof id === 'number') { + return id + } + + return decodeURIComponent(id) +} diff --git a/test/collections-rest/config.ts b/test/collections-rest/config.ts index 97c8a891f70..d2b383621fa 100644 --- a/test/collections-rest/config.ts +++ b/test/collections-rest/config.ts @@ -273,8 +273,8 @@ export default buildConfigWithDefaults({ ], endpoints: [ { - handler: async ({ req }) => { - await req.payload.sendEmail({ + handler: async ({ payload }) => { + await payload.sendEmail({ from: 'dev@payloadcms.com', html: 'This is a test email.', subject: 'Test Email', diff --git a/test/fields/collections/CustomID/index.ts b/test/fields/collections/CustomID/index.ts new file mode 100644 index 00000000000..2b38a2a8c58 --- /dev/null +++ b/test/fields/collections/CustomID/index.ts @@ -0,0 +1,14 @@ +import type { CollectionConfig } from 'payload' + +import { customIdSlug } from '../../slugs.js' + +export const CustomIdCollection: CollectionConfig = { + slug: customIdSlug, + versions: true, + fields: [ + { + name: 'id', + type: 'text', + }, + ], +} diff --git a/test/fields/config.ts b/test/fields/config.ts index 10a68beadb6..8bc8abec7d7 100644 --- a/test/fields/config.ts +++ b/test/fields/config.ts @@ -12,6 +12,7 @@ import CheckboxFields from './collections/Checkbox/index.js' import CodeFields from './collections/Code/index.js' import CollapsibleFields from './collections/Collapsible/index.js' import ConditionalLogic from './collections/ConditionalLogic/index.js' +import { CustomIdCollection } from './collections/CustomID/index.js' import DateFields from './collections/Date/index.js' import EmailFields from './collections/Email/index.js' import GroupFields from './collections/Group/index.js' @@ -68,6 +69,7 @@ export const collectionSlugs: CollectionConfig[] = [ CodeFields, CollapsibleFields, ConditionalLogic, + CustomIdCollection, DateFields, EmailFields, RadioFields, diff --git a/test/fields/e2e.spec.ts b/test/fields/e2e.spec.ts index a98b5a12ed5..00037578173 100644 --- a/test/fields/e2e.spec.ts +++ b/test/fields/e2e.spec.ts @@ -1,6 +1,7 @@ import type { Page } from '@playwright/test' import { expect, test } from '@playwright/test' +import { navigateToDoc } from 'helpers/e2e/navigateToDoc.js' import path from 'path' import { wait } from 'payload/shared' import { fileURLToPath } from 'url' @@ -19,6 +20,7 @@ import { arrayFieldsSlug, blockFieldsSlug, collapsibleFieldsSlug, + customIdSlug, tabsFields2Slug, tabsFieldsSlug, } from './slugs.js' @@ -588,4 +590,31 @@ describe('fields', () => { }) }) }) + + describe('id', () => { + let url: AdminUrlUtil + beforeAll(() => { + url = new AdminUrlUtil(serverURL, customIdSlug) + }) + + function createCustomIDDoc(id: string) { + return payload.create({ + collection: customIdSlug, + data: { + id, + }, + }) + } + + test('allow create of non standard ID', async () => { + await createCustomIDDoc('id 1') + await page.goto(url.list) + + await navigateToDoc(page, url) + + // Page should load and ID should be correct + await expect(page.locator('#field-id')).toHaveValue('id 1') + await expect(page.locator('.id-label')).toContainText('id 1') + }) + }) }) diff --git a/test/fields/payload-types.ts b/test/fields/payload-types.ts index 68ad61a01d7..9296fdd2e72 100644 --- a/test/fields/payload-types.ts +++ b/test/fields/payload-types.ts @@ -39,6 +39,7 @@ export interface Config { 'code-fields': CodeField; 'collapsible-fields': CollapsibleField; 'conditional-logic': ConditionalLogic; + 'custom-id': CustomId; 'date-fields': DateField; 'email-fields': EmailField; 'radio-fields': RadioField; @@ -80,6 +81,7 @@ export interface Config { 'code-fields': CodeFieldsSelect | CodeFieldsSelect; 'collapsible-fields': CollapsibleFieldsSelect | CollapsibleFieldsSelect; 'conditional-logic': ConditionalLogicSelect | ConditionalLogicSelect; + 'custom-id': CustomIdSelect | CustomIdSelect; 'date-fields': DateFieldsSelect | DateFieldsSelect; 'email-fields': EmailFieldsSelect | EmailFieldsSelect; 'radio-fields': RadioFieldsSelect | RadioFieldsSelect; @@ -121,9 +123,9 @@ export interface Config { user: User & { collection: 'users'; }; - jobs: { + jobs?: { tasks: unknown; - workflows: unknown; + workflows?: unknown; }; } export interface UserAuthOperations { @@ -948,6 +950,15 @@ export interface ConditionalLogic { updatedAt: string; createdAt: string; } +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "custom-id". + */ +export interface CustomId { + id: string; + updatedAt: string; + createdAt: string; +} /** * This interface was referenced by `Config`'s JSON-Schema * via the `definition` "date-fields". @@ -1796,6 +1807,10 @@ export interface PayloadLockedDocument { relationTo: 'conditional-logic'; value: string | ConditionalLogic; } | null) + | ({ + relationTo: 'custom-id'; + value: string | CustomId; + } | null) | ({ relationTo: 'date-fields'; value: string | DateField; @@ -2587,6 +2602,15 @@ export interface ConditionalLogicSelect { updatedAt?: T; createdAt?: T; } +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "custom-id_select". + */ +export interface CustomIdSelect { + id?: T; + updatedAt?: T; + createdAt?: T; +} /** * This interface was referenced by `Config`'s JSON-Schema * via the `definition` "date-fields_select". diff --git a/test/fields/slugs.ts b/test/fields/slugs.ts index 300e0991591..95830d1175c 100644 --- a/test/fields/slugs.ts +++ b/test/fields/slugs.ts @@ -5,6 +5,7 @@ export const checkboxFieldsSlug = 'checkbox-fields' export const codeFieldsSlug = 'code-fields' export const collapsibleFieldsSlug = 'collapsible-fields' export const conditionalLogicSlug = 'conditional-logic' +export const customIdSlug = 'custom-id' export const dateFieldsSlug = 'date-fields' export const emailFieldsSlug = 'email-fields' export const groupFieldsSlug = 'group-fields'