From 529bfe149e3e4a385556341c5ebd3bd0436022bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Jablo=C3=B1ski?= <43938777+GermanJablo@users.noreply.github.com> Date: Fri, 16 May 2025 19:21:46 -0300 Subject: [PATCH] fix: orderable with groups and tabs requires migration (#12422) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ⚠️ `orderable` fields will no longer be `required` and `unique`, so your database may prompt you to accept an automatic migration if you're using [this feature](https://payloadcms.com/docs/configuration/collections#config-options). Note that the `orderable` feature is still experimental, so it may still receive breaking changes without a major upgrade or contain bugs. Use it with caution. ___ The `orderable` fields will not have `required` and `unique` constraints at the database schema level, in order to automatically migrate collections that incorporate this property. Now, when a user adds the `orderable` property to a collection or join field, existing documents will have the order field set to undefined. The first time you try to reorder them, the documents will be automatically assigned an initial order, and you will be prompted to refresh the page. We believe this provides a better development experience than having to manually migrate data with a script. Additionally, it fixes a bug that occurred when using `orderable` in conjunction with groups and tabs fields. Closes: - #12129 - #12331 - #12212 --------- Co-authored-by: Dan Ribbens --- .../src/transform/read/traverseFields.ts | 4 - .../payload/src/config/orderable/index.ts | 93 +++++++++++++++---- .../ui/src/elements/Table/OrderableTable.tsx | 8 ++ test/sort/payload-types.ts | 8 +- 4 files changed, 86 insertions(+), 27 deletions(-) diff --git a/packages/drizzle/src/transform/read/traverseFields.ts b/packages/drizzle/src/transform/read/traverseFields.ts index 201f39c288..261336a301 100644 --- a/packages/drizzle/src/transform/read/traverseFields.ts +++ b/packages/drizzle/src/transform/read/traverseFields.ts @@ -670,10 +670,6 @@ export const traverseFields = >({ withinArrayOrBlockLocale: locale || withinArrayOrBlockLocale, }) - if ('_order' in ref) { - delete ref._order - } - return } diff --git a/packages/payload/src/config/orderable/index.ts b/packages/payload/src/config/orderable/index.ts index 26b22f7cd9..5591844108 100644 --- a/packages/payload/src/config/orderable/index.ts +++ b/packages/payload/src/config/orderable/index.ts @@ -1,8 +1,14 @@ +import { status as httpStatus } from 'http-status' + import type { BeforeChangeHook, CollectionConfig } from '../../collections/config/types.js' import type { Field } from '../../fields/config/types.js' import type { Endpoint, PayloadHandler, SanitizedConfig } from '../types.js' import executeAccess from '../../auth/executeAccess.js' +import { APIError } from '../../errors/index.js' +import { commitTransaction } from '../../utilities/commitTransaction.js' +import { initTransaction } from '../../utilities/initTransaction.js' +import { killTransaction } from '../../utilities/killTransaction.js' import { traverseFields } from '../../utilities/traverseFields.js' import { generateKeyBetween, generateNKeysBetween } from './fractional-indexing.js' @@ -37,7 +43,12 @@ export const setupOrderable = (config: SanitizedConfig) => { } if (field.type === 'join' && field.orderable === true) { if (Array.isArray(field.collection)) { - throw new Error('Orderable joins must target a single collection') + throw new APIError( + 'Orderable joins must target a single collection', + httpStatus.BAD_REQUEST, + {}, + true, + ) } const relationshipCollection = config.collections.find((c) => c.slug === field.collection) if (!relationshipCollection) { @@ -91,15 +102,6 @@ export const addOrderableFieldsAndHook = ( ], }, index: true, - required: true, - // override the schema to make order fields optional for payload.create() - typescriptSchema: [ - () => ({ - type: 'string', - required: false, - }), - ], - unique: true, } collection.fields.unshift(orderField) @@ -170,16 +172,6 @@ export const addOrderableEndpoint = (config: SanitizedConfig) => { status: 400, }) } - if ( - typeof target !== 'object' || - typeof target.id === 'undefined' || - typeof target.key !== 'string' - ) { - return new Response(JSON.stringify({ error: 'target must be an object with id and key' }), { - headers: { 'Content-Type': 'application/json' }, - status: 400, - }) - } if (newKeyWillBe !== 'greater' && newKeyWillBe !== 'less') { return new Response(JSON.stringify({ error: 'newKeyWillBe must be "greater" or "less"' }), { headers: { 'Content-Type': 'application/json' }, @@ -213,6 +205,67 @@ export const addOrderableEndpoint = (config: SanitizedConfig) => { collection.access.update, ) } + /** + * If there is no target.key, we can assume the user enabled `orderable` + * on a collection with existing documents, and that this is the first + * time they tried to reorder them. Therefore, we perform a one-time + * migration by setting the key value for all documents. We do this + * instead of enforcing `required` and `unique` at the database schema + * level, so that users don't have to run a migration when they enable + * `orderable` on a collection with existing documents. + */ + if (!target.key) { + const { docs } = await req.payload.find({ + collection: collection.slug, + depth: 0, + limit: 0, + req, + select: { [orderableFieldName]: true }, + where: { + [orderableFieldName]: { + exists: false, + }, + }, + }) + await initTransaction(req) + // We cannot update all documents in a single operation with `payload.update`, + // because they would all end up with the same order key (`a0`). + try { + for (const doc of docs) { + await req.payload.update({ + id: doc.id, + collection: collection.slug, + data: { + // no data needed since the order hooks will handle this + }, + depth: 0, + req, + }) + await commitTransaction(req) + } + } catch (e) { + await killTransaction(req) + if (e instanceof Error) { + throw new APIError(e.message, httpStatus.INTERNAL_SERVER_ERROR) + } + } + + return new Response(JSON.stringify({ message: 'initial migration', success: true }), { + headers: { 'Content-Type': 'application/json' }, + status: 200, + }) + } + + if ( + typeof target !== 'object' || + typeof target.id === 'undefined' || + typeof target.key !== 'string' + ) { + return new Response(JSON.stringify({ error: 'target must be an object with id' }), { + headers: { 'Content-Type': 'application/json' }, + status: 400, + }) + } const targetId = target.id let targetKey = target.key diff --git a/packages/ui/src/elements/Table/OrderableTable.tsx b/packages/ui/src/elements/Table/OrderableTable.tsx index f32d5813db..752d0ccf01 100644 --- a/packages/ui/src/elements/Table/OrderableTable.tsx +++ b/packages/ui/src/elements/Table/OrderableTable.tsx @@ -133,6 +133,14 @@ export const OrderableTable: React.FC = ({ 'Failed to reorder. This can happen if you reorder several rows too quickly. Please try again.', ) } + + if (response.status === 200 && (await response.json())['message'] === 'initial migration') { + throw new Error( + 'You have enabled "orderable" on a collection with existing documents' + + 'and this is the first time you have sorted documents. We have run an automatic migration ' + + 'to add an initial order to the documents. Please refresh the page and try again.', + ) + } } catch (err) { const error = err instanceof Error ? err.message : String(err) // Rollback to previous state if the request fails diff --git a/test/sort/payload-types.ts b/test/sort/payload-types.ts index 887eb15457..7dde9f7142 100644 --- a/test/sort/payload-types.ts +++ b/test/sort/payload-types.ts @@ -151,6 +151,7 @@ export interface Post { */ export interface Draft { id: string; + _order?: string | null; text?: string | null; number?: number | null; number2?: number | null; @@ -191,9 +192,9 @@ export interface Localized { */ export interface Orderable { id: string; - _orderable_orderableJoinField2_order?: string; - _orderable_orderableJoinField1_order?: string; - _order?: string; + _orderable_orderableJoinField2_order?: string | null; + _orderable_orderableJoinField1_order?: string | null; + _order?: string | null; title?: string | null; orderableField?: (string | null) | OrderableJoin; updatedAt: string; @@ -340,6 +341,7 @@ export interface PostsSelect { * via the `definition` "drafts_select". */ export interface DraftsSelect { + _order?: T; text?: T; number?: T; number2?: T;