Skip to content

Commit

Permalink
fix: unable to load documents with non-standard ids (#9407)
Browse files Browse the repository at this point in the history
### 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
  • Loading branch information
JarrodMFlesch authored Nov 21, 2024
1 parent 304ecd2 commit ee1a91e
Show file tree
Hide file tree
Showing 14 changed files with 103 additions and 10 deletions.
4 changes: 3 additions & 1 deletion packages/next/src/views/Document/getDocPreferences.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import type { DocumentPreferences, Payload, TypedUser } from 'payload'

import { sanitizeID } from '@payloadcms/ui/shared'

type Args = {
collectionSlug?: string
globalSlug?: string
Expand Down Expand Up @@ -44,7 +46,7 @@ export const getDocPreferences = async ({
},
{
'user.value': {
equals: user.id,
equals: sanitizeID(user.id),
},
},
],
Expand Down
5 changes: 4 additions & 1 deletion packages/next/src/views/Document/getDocumentData.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import type { Locale, Payload, TypedUser, TypeWithID } from 'payload'

import { sanitizeID } from '@payloadcms/ui/shared'

type Args = {
collectionSlug?: string
globalSlug?: string
Expand All @@ -10,13 +12,14 @@ type Args = {
}

export const getDocumentData = async ({
id,
id: idArg,
collectionSlug,
globalSlug,
locale,
payload,
user,
}: Args): Promise<null | Record<string, unknown> | TypeWithID> => {
const id = sanitizeID(idArg)
let resolvedData: Record<string, unknown> | TypeWithID = null

try {
Expand Down
4 changes: 3 additions & 1 deletion packages/next/src/views/Document/getIsLocked.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import type {
Where,
} from 'payload'

import { sanitizeID } from '@payloadcms/ui/shared'

type Args = {
collectionConfig?: SanitizedCollectionConfig
globalConfig?: SanitizedGlobalConfig
Expand Down Expand Up @@ -48,7 +50,7 @@ export const getIsLocked = async ({
where.and = [
{
'document.value': {
equals: id,
equals: sanitizeID(id),
},
},
{
Expand Down
5 changes: 4 additions & 1 deletion packages/next/src/views/Document/getVersions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import type {
TypedUser,
} from 'payload'

import { sanitizeID } from '@payloadcms/ui/shared'

type Args = {
collectionConfig?: SanitizedCollectionConfig
docPermissions: SanitizedDocumentPermissions
Expand All @@ -26,14 +28,15 @@ 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,
locale,
payload,
user,
}: Args): Result => {
const id = sanitizeID(idArg)
let publishedQuery
let hasPublishedDoc = false
let mostRecentVersionIsAutosaved = false
Expand Down
3 changes: 2 additions & 1 deletion packages/ui/src/elements/IDLabel/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import React from 'react'

import './index.scss'
import { sanitizeID } from '../../utilities/sanitizeID.js'

const baseClass = 'id-label'

Expand All @@ -13,6 +14,6 @@ export const IDLabel: React.FC<{ className?: string; id: string; prefix?: string
<div className={[baseClass, className].filter(Boolean).join(' ')} title={id}>
{prefix}
&nbsp;
{id}
{sanitizeID(id)}
</div>
)
2 changes: 1 addition & 1 deletion packages/ui/src/elements/Table/DefaultCell/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export const DefaultCell: React.FC<DefaultCellComponentProps> = (props) => {
wrapElementProps.href = collectionConfig?.slug
? formatAdminURL({
adminRoute,
path: `/collections/${collectionConfig?.slug}/${rowData.id}`,
path: `/collections/${collectionConfig?.slug}/${encodeURIComponent(rowData.id)}`,
})
: ''
}
Expand Down
1 change: 1 addition & 0 deletions packages/ui/src/exports/shared/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
11 changes: 11 additions & 0 deletions packages/ui/src/utilities/sanitizeID.ts
Original file line number Diff line number Diff line change
@@ -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)
}
4 changes: 2 additions & 2 deletions test/collections-rest/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
14 changes: 14 additions & 0 deletions test/fields/collections/CustomID/index.ts
Original file line number Diff line number Diff line change
@@ -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',
},
],
}
2 changes: 2 additions & 0 deletions test/fields/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -68,6 +69,7 @@ export const collectionSlugs: CollectionConfig[] = [
CodeFields,
CollapsibleFields,
ConditionalLogic,
CustomIdCollection,
DateFields,
EmailFields,
RadioFields,
Expand Down
29 changes: 29 additions & 0 deletions test/fields/e2e.spec.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -19,6 +20,7 @@ import {
arrayFieldsSlug,
blockFieldsSlug,
collapsibleFieldsSlug,
customIdSlug,
tabsFields2Slug,
tabsFieldsSlug,
} from './slugs.js'
Expand Down Expand Up @@ -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')
})
})
})
28 changes: 26 additions & 2 deletions test/fields/payload-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -80,6 +81,7 @@ export interface Config {
'code-fields': CodeFieldsSelect<false> | CodeFieldsSelect<true>;
'collapsible-fields': CollapsibleFieldsSelect<false> | CollapsibleFieldsSelect<true>;
'conditional-logic': ConditionalLogicSelect<false> | ConditionalLogicSelect<true>;
'custom-id': CustomIdSelect<false> | CustomIdSelect<true>;
'date-fields': DateFieldsSelect<false> | DateFieldsSelect<true>;
'email-fields': EmailFieldsSelect<false> | EmailFieldsSelect<true>;
'radio-fields': RadioFieldsSelect<false> | RadioFieldsSelect<true>;
Expand Down Expand Up @@ -121,9 +123,9 @@ export interface Config {
user: User & {
collection: 'users';
};
jobs: {
jobs?: {
tasks: unknown;
workflows: unknown;
workflows?: unknown;
};
}
export interface UserAuthOperations {
Expand Down Expand Up @@ -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".
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -2587,6 +2602,15 @@ export interface ConditionalLogicSelect<T extends boolean = true> {
updatedAt?: T;
createdAt?: T;
}
/**
* This interface was referenced by `Config`'s JSON-Schema
* via the `definition` "custom-id_select".
*/
export interface CustomIdSelect<T extends boolean = true> {
id?: T;
updatedAt?: T;
createdAt?: T;
}
/**
* This interface was referenced by `Config`'s JSON-Schema
* via the `definition` "date-fields_select".
Expand Down
1 change: 1 addition & 0 deletions test/fields/slugs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down

0 comments on commit ee1a91e

Please sign in to comment.