Skip to content

Commit 17a0d19

Browse files
authored
fix: scope orderable join reordering by parent relation (#15842)
## Context The root problem was scope leakage in join-based ordering. Reorder operations were effectively treating join items as one shared list, so moving an item in one parent context could influence ordering in a different parent context. ## Changes - Reorder key generation, adjacent-row lookup, and initial missing-key backfill now run in the same join `on` scope as the target item. - Fast drag/drop cases that send `pending` now resolve against scoped context before computing new keys. - `index.ts` keeps orchestration, and scoped query logic is extracted into focused helpers to keep behavior changes easier to review. - Replaced a local where-wrapper with the shared `combineWhereConstraints` utility. ## Impact Users can reorder join rows without cross-parent side effects. Items in unrelated parent relations no longer jump or get new keys from another parent's ordering context. --------- Co-authored-by: German Jablonski <GermanJablo@users.noreply.github.com>
1 parent fe36dde commit 17a0d19

File tree

8 files changed

+517
-32
lines changed

8 files changed

+517
-32
lines changed

packages/payload/src/config/orderable/index.ts

Lines changed: 64 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,15 @@ import type { Endpoint, PayloadHandler, SanitizedConfig } from '../types.js'
66

77
import { executeAccess } from '../../auth/executeAccess.js'
88
import { APIError } from '../../errors/index.js'
9+
import { combineWhereConstraints } from '../../utilities/combineWhereConstraints.js'
910
import { commitTransaction } from '../../utilities/commitTransaction.js'
1011
import { initTransaction } from '../../utilities/initTransaction.js'
1112
import { killTransaction } from '../../utilities/killTransaction.js'
1213
import { traverseFields } from '../../utilities/traverseFields.js'
1314
import { generateKeyBetween, generateNKeysBetween } from './fractional-indexing.js'
15+
import { getJoinScopeContext } from './utils/getJoinScopeContext.js'
16+
import { getJoinScopeWhereFromDocData } from './utils/getJoinScopeWhereFromDocData.js'
17+
import { resolvePendingTargetKey } from './utils/resolvePendingTargetKey.js'
1418

1519
/**
1620
* This function creates:
@@ -22,6 +26,7 @@ import { generateKeyBetween, generateNKeysBetween } from './fractional-indexing.
2226
*/
2327
export const setupOrderable = (config: SanitizedConfig) => {
2428
const fieldsToAdd = new Map<CollectionConfig, string[]>()
29+
const joinFieldPathsByCollection = new Map<string, Map<string, string>>()
2530

2631
config.collections.forEach((collection) => {
2732
if (collection.orderable) {
@@ -58,28 +63,32 @@ export const setupOrderable = (config: SanitizedConfig) => {
5863
const currentFields = fieldsToAdd.get(relationshipCollection) || []
5964
// @ts-expect-error ref is untyped
6065
const prefix = parentRef?.prefix ? `${parentRef.prefix}_` : ''
61-
fieldsToAdd.set(relationshipCollection, [
62-
...currentFields,
63-
`_${field.collection}_${prefix}${field.name}_order`,
64-
])
66+
const joinOrderableFieldName = `_${field.collection}_${prefix}${field.name}_order`
67+
fieldsToAdd.set(relationshipCollection, [...currentFields, joinOrderableFieldName])
68+
69+
const currentJoinFieldPaths =
70+
joinFieldPathsByCollection.get(relationshipCollection.slug) || new Map<string, string>()
71+
currentJoinFieldPaths.set(joinOrderableFieldName, field.on)
72+
joinFieldPathsByCollection.set(relationshipCollection.slug, currentJoinFieldPaths)
6573
}
6674
},
6775
fields: collection.fields,
6876
})
6977
})
7078

7179
Array.from(fieldsToAdd.entries()).forEach(([collection, orderableFields]) => {
72-
addOrderableFieldsAndHook(collection, orderableFields)
80+
addOrderableFieldsAndHook(collection, orderableFields, joinFieldPathsByCollection)
7381
})
7482

7583
if (fieldsToAdd.size > 0) {
76-
addOrderableEndpoint(config)
84+
addOrderableEndpoint(config, joinFieldPathsByCollection)
7785
}
7886
}
7987

8088
export const addOrderableFieldsAndHook = (
8189
collection: CollectionConfig,
8290
orderableFieldNames: string[],
91+
joinFieldPathsByCollection?: Map<string, Map<string, string>>,
8392
) => {
8493
// 1. Add field
8594
orderableFieldNames.forEach((orderableFieldName) => {
@@ -119,6 +128,14 @@ export const addOrderableFieldsAndHook = (
119128
const orderBeforeChangeHook: BeforeChangeHook = async ({ data, originalDoc, req }) => {
120129
for (const orderableFieldName of orderableFieldNames) {
121130
if (!data[orderableFieldName] && !originalDoc?.[orderableFieldName]) {
131+
const joinScopeWhere = getJoinScopeWhereFromDocData({
132+
collectionSlug: collection.slug,
133+
data,
134+
joinFieldPathsByCollection,
135+
orderableFieldName,
136+
originalDoc,
137+
})
138+
122139
const lastDoc = await req.payload.find({
123140
collection: collection.slug,
124141
depth: 0,
@@ -127,11 +144,14 @@ export const addOrderableFieldsAndHook = (
127144
req,
128145
select: { [orderableFieldName]: true },
129146
sort: `-${orderableFieldName}`,
130-
where: {
131-
[orderableFieldName]: {
132-
exists: true,
147+
where: combineWhereConstraints([
148+
{
149+
[orderableFieldName]: {
150+
exists: true,
151+
},
133152
},
134-
},
153+
joinScopeWhere ?? undefined,
154+
]),
135155
})
136156

137157
const lastOrderValue = lastDoc.docs[0]?.[orderableFieldName] || null
@@ -160,7 +180,10 @@ export type OrderableEndpointBody = {
160180
}
161181
}
162182

163-
export const addOrderableEndpoint = (config: SanitizedConfig) => {
183+
export const addOrderableEndpoint = (
184+
config: SanitizedConfig,
185+
joinFieldPathsByCollection: Map<string, Map<string, string>>,
186+
) => {
164187
// 3. Add endpoint
165188
const reorderHandler: PayloadHandler = async (req) => {
166189
const body = (await req.json?.()) as OrderableEndpointBody
@@ -193,6 +216,14 @@ export const addOrderableEndpoint = (config: SanitizedConfig) => {
193216
})
194217
}
195218

219+
const { joinScopeWhere, targetDoc } = await getJoinScopeContext({
220+
collectionSlug: collection.slug,
221+
joinFieldPathsByCollection,
222+
orderableFieldName,
223+
req,
224+
target,
225+
})
226+
196227
// Prevent reordering if user doesn't have editing permissions
197228
if (collection.access?.update) {
198229
await executeAccess(
@@ -222,11 +253,14 @@ export const addOrderableEndpoint = (config: SanitizedConfig) => {
222253
limit: 0,
223254
req,
224255
select: { [orderableFieldName]: true },
225-
where: {
226-
[orderableFieldName]: {
227-
exists: false,
256+
where: combineWhereConstraints([
257+
{
258+
[orderableFieldName]: {
259+
exists: false,
260+
},
228261
},
229-
},
262+
joinScopeWhere ?? undefined,
263+
]),
230264
})
231265
await initTransaction(req)
232266
// We cannot update all documents in a single operation with `payload.update`,
@@ -269,19 +303,14 @@ export const addOrderableEndpoint = (config: SanitizedConfig) => {
269303
}
270304

271305
const targetId = target.id
272-
let targetKey = target.key
273-
274-
// If targetKey = pending, we need to find its current key.
275-
// This can only happen if the user reorders rows quickly with a slow connection.
276-
if (targetKey === 'pending') {
277-
const beforeDoc = await req.payload.findByID({
278-
id: targetId,
279-
collection: collection.slug,
280-
depth: 0,
281-
select: { [orderableFieldName]: true },
282-
})
283-
targetKey = beforeDoc?.[orderableFieldName] || null
284-
}
306+
const targetKey = await resolvePendingTargetKey({
307+
collectionSlug: collection.slug,
308+
orderableFieldName,
309+
req,
310+
targetDoc,
311+
targetID: targetId,
312+
targetKey: target.key,
313+
})
285314

286315
// The reason the endpoint does not receive this docId as an argument is that there
287316
// are situations where the user may not see or know what the next or previous one is. For
@@ -293,11 +322,14 @@ export const addOrderableEndpoint = (config: SanitizedConfig) => {
293322
pagination: false,
294323
select: { [orderableFieldName]: true },
295324
sort: newKeyWillBe === 'greater' ? orderableFieldName : `-${orderableFieldName}`,
296-
where: {
297-
[orderableFieldName]: {
298-
[newKeyWillBe === 'greater' ? 'greater_than' : 'less_than']: targetKey,
325+
where: combineWhereConstraints([
326+
{
327+
[orderableFieldName]: {
328+
[newKeyWillBe === 'greater' ? 'greater_than' : 'less_than']: targetKey,
329+
},
299330
},
300-
},
331+
joinScopeWhere ?? undefined,
332+
]),
301333
})
302334
const adjacentDocKey = adjacentDoc.docs?.[0]?.[orderableFieldName] || null
303335

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import type { Where } from '../../../types/index.js'
2+
3+
/**
4+
* Builds a `where` fragment that scopes order operations to docs sharing the
5+
* same join `on` field value.
6+
*/
7+
export function buildJoinScopeWhere(args: {
8+
joinOnFieldPath: string
9+
scopeValue: unknown
10+
}): null | Where {
11+
const { joinOnFieldPath, scopeValue } = args
12+
13+
if (typeof scopeValue === 'undefined') {
14+
return null
15+
}
16+
17+
if (Array.isArray(scopeValue)) {
18+
return buildJoinScopeWhere({
19+
joinOnFieldPath,
20+
scopeValue: scopeValue[0],
21+
})
22+
}
23+
24+
if (
25+
scopeValue &&
26+
typeof scopeValue === 'object' &&
27+
'relationTo' in scopeValue &&
28+
'value' in scopeValue
29+
) {
30+
const relation = (scopeValue as { relationTo?: unknown }).relationTo
31+
const value = (scopeValue as { value?: unknown }).value
32+
33+
if (typeof relation === 'undefined' || typeof value === 'undefined') {
34+
return null
35+
}
36+
37+
return {
38+
and: [
39+
{
40+
[`${joinOnFieldPath}.relationTo`]: {
41+
equals: relation,
42+
},
43+
},
44+
{
45+
[`${joinOnFieldPath}.value`]: {
46+
equals: value,
47+
},
48+
},
49+
],
50+
}
51+
}
52+
53+
return {
54+
[joinOnFieldPath]: {
55+
equals: scopeValue,
56+
},
57+
}
58+
}
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import type { PayloadHandler } from '../../types.js'
2+
3+
import { buildJoinScopeWhere } from './buildJoinScopeWhere.js'
4+
import { getValueAtPath } from './getValueAtPath.js'
5+
6+
/**
7+
* Resolves join scope and target document context for reorder operations.
8+
*/
9+
export async function getJoinScopeContext(args: {
10+
collectionSlug: string
11+
joinFieldPathsByCollection: Map<string, Map<string, string>>
12+
orderableFieldName: string
13+
req: Parameters<PayloadHandler>[0]
14+
target: unknown
15+
}): Promise<{
16+
joinScopeWhere: ReturnType<typeof buildJoinScopeWhere>
17+
targetDoc: null | Record<string, unknown>
18+
}> {
19+
const { collectionSlug, joinFieldPathsByCollection, orderableFieldName, req, target } = args
20+
21+
const joinOnFieldPath = joinFieldPathsByCollection.get(collectionSlug)?.get(orderableFieldName)
22+
let targetDoc: null | Record<string, unknown> = null
23+
24+
if (
25+
typeof target === 'object' &&
26+
target &&
27+
'id' in target &&
28+
(joinOnFieldPath || ('key' in target && target.key === 'pending'))
29+
) {
30+
const targetID = (target as { id?: unknown }).id
31+
32+
if (typeof targetID === 'number' || typeof targetID === 'string') {
33+
targetDoc = await req.payload.findByID({
34+
id: targetID,
35+
collection: collectionSlug,
36+
depth: 0,
37+
req,
38+
select: {
39+
...(joinOnFieldPath ? { [joinOnFieldPath]: true } : {}),
40+
[orderableFieldName]: true,
41+
},
42+
})
43+
}
44+
}
45+
46+
if (!joinOnFieldPath) {
47+
return {
48+
joinScopeWhere: null,
49+
targetDoc,
50+
}
51+
}
52+
53+
const joinScopeValue = getValueAtPath(targetDoc, joinOnFieldPath)
54+
55+
return {
56+
joinScopeWhere: buildJoinScopeWhere({
57+
joinOnFieldPath,
58+
scopeValue: joinScopeValue,
59+
}),
60+
targetDoc,
61+
}
62+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import { buildJoinScopeWhere } from './buildJoinScopeWhere.js'
2+
import { getValueAtPath } from './getValueAtPath.js'
3+
4+
/**
5+
* Builds a join-scope filter for order key generation during beforeChange.
6+
*/
7+
export function getJoinScopeWhereFromDocData(args: {
8+
collectionSlug: string
9+
data: Record<string, unknown>
10+
joinFieldPathsByCollection?: Map<string, Map<string, string>>
11+
orderableFieldName: string
12+
originalDoc?: Record<string, unknown>
13+
}): ReturnType<typeof buildJoinScopeWhere> {
14+
const { collectionSlug, data, joinFieldPathsByCollection, orderableFieldName, originalDoc } = args
15+
16+
const joinOnFieldPath = joinFieldPathsByCollection?.get(collectionSlug)?.get(orderableFieldName)
17+
18+
if (!joinOnFieldPath) {
19+
return null
20+
}
21+
22+
const scopeValue =
23+
getValueAtPath(data, joinOnFieldPath) ?? getValueAtPath(originalDoc, joinOnFieldPath)
24+
25+
return buildJoinScopeWhere({
26+
joinOnFieldPath,
27+
scopeValue,
28+
})
29+
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* Reads a nested value from an object using dot-notation path syntax.
3+
*/
4+
export function getValueAtPath(data: unknown, path: string): unknown {
5+
if (!data || typeof data !== 'object') {
6+
return undefined
7+
}
8+
9+
const segments = path.split('.')
10+
let currentValue: unknown = data
11+
12+
for (const segment of segments) {
13+
if (!currentValue || typeof currentValue !== 'object') {
14+
return undefined
15+
}
16+
17+
currentValue = (currentValue as Record<string, unknown>)[segment]
18+
}
19+
20+
return currentValue
21+
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import type { PayloadHandler } from '../../types.js'
2+
3+
/**
4+
* Resolves the target key when the client sends the temporary `pending` marker.
5+
*/
6+
export async function resolvePendingTargetKey(args: {
7+
collectionSlug: string
8+
orderableFieldName: string
9+
req: Parameters<PayloadHandler>[0]
10+
targetDoc: null | Record<string, unknown>
11+
targetID: string
12+
targetKey: string
13+
}): Promise<null | string> {
14+
const { collectionSlug, orderableFieldName, req, targetDoc, targetID, targetKey } = args
15+
16+
if (targetKey !== 'pending') {
17+
return targetKey
18+
}
19+
20+
const targetDocKey = targetDoc?.[orderableFieldName]
21+
if (typeof targetDocKey === 'string') {
22+
return targetDocKey
23+
}
24+
25+
const beforeDoc = await req.payload.findByID({
26+
id: targetID,
27+
collection: collectionSlug,
28+
depth: 0,
29+
req,
30+
select: { [orderableFieldName]: true },
31+
})
32+
33+
return beforeDoc?.[orderableFieldName] || null
34+
}

0 commit comments

Comments
 (0)