Skip to content

fix(plugin-nested-docs): exclude descendants from parent options - #17769

Open
Shweta-singh24 wants to merge 2 commits into
payloadcms:mainfrom
Shweta-singh24:fix/nested-docs-parent-filter-postgres
Open

fix(plugin-nested-docs): exclude descendants from parent options#17769
Shweta-singh24 wants to merge 2 commits into
payloadcms:mainfrom
Shweta-singh24:fix/nested-docs-parent-filter-postgres

Conversation

@Shweta-singh24

@Shweta-singh24 Shweta-singh24 commented Aug 12, 2026

Copy link
Copy Markdown

What does this PR do?

Fixes #17658 by preventing a document from selecting its own descendants as its parent when using PostgreSQL.

Why?

Filtering breadcrumbs.doc with not_in on relational adapters can produce incorrect results because array relationships are joined and evaluated per row. This allows descendant documents to incorrectly appear in the parent picker.

What changed?

  • Replaced the breadcrumbs.doc exclusion with descendant resolution using the parent relationship.
  • Excludes the current document and all descendants using id: { not_in: [...] }.
  • Passes parentFieldSlug into the default filter so custom parent field names (for example owner) are supported.
  • Adds a regression test covering a Root → Child → Grandchild hierarchy.

Verification

Verified with PostgreSQL:

pnpm test:int:postgres plugin-nested-docs

Result: 13/13 tests passing.

@sam9191

sam9191 commented Aug 13, 2026

Copy link
Copy Markdown

Thanks for picking this up — the approach is right: resolving descendants with a positive equals lookup and excluding by plain id: { not_in } sidesteps the broken join semantics entirely.

Two suggestions on the implementation:

  1. overrideAccess: false weakens the protection and can throw where the old code couldn't. Any descendant the current user lacks read access to won't be excluded, so a user with partial read access can still select an unreadable descendant and create a cycle. And if filterOptions ever runs without a user against a collection whose read access requires one, the find throws Forbidden — the old code never queried, so this is a new failure path. Since the result is only used structurally (IDs inside a where, never returned as data), overrideAccess: true seems more correct here.

  2. This still trusts breadcrumbs data rather than the actual parent links. If breadcrumbs lag reality (drafts, a failed resaveChildren, or an install already corrupted by the cycles this bug allowed in), those descendants won't carry the edited doc in their breadcrumbs and remain selectable. Walking the stored parent relationships instead — parent: { in: frontier } per level, accumulating IDs — costs one query per tree depth rather than one total, but is immune to stale breadcrumbs; there's a working version in plugin-nested-docs: default parentFilterOptions exclude nothing on Postgres — parent picker offers the doc's own descendants #17658 under "Suggested direction".

On scope, for whoever triages this: I've filed the underlying adapter bug as #17770not_in/not_equals on to-many paths match when any row differs rather than when no row matches. This PR is the right near-term fix for the plugin's default filter, but it shouldn't close #17770: custom filterOptions and user queries using the same clause stay silently broken, and any ID-materialization approach (this PR and my workaround alike) is inherently O(descendants) in query size — the adapter-level NOT EXISTS fix is what makes the original single-clause filter both correct and scalable. Separately, the missing cycle guards (#16517) mean a cycle that gets in via the API still hangs even with this merged.

@Shweta-singh24

Copy link
Copy Markdown
Author

Thanks for picking this up — the approach is right: resolving descendants with a positive equals lookup and excluding by plain id: { not_in } sidesteps the broken join semantics entirely.

Two suggestions on the implementation:

  1. overrideAccess: false weakens the protection and can throw where the old code couldn't. Any descendant the current user lacks read access to won't be excluded, so a user with partial read access can still select an unreadable descendant and create a cycle. And if filterOptions ever runs without a user against a collection whose read access requires one, the find throws Forbidden — the old code never queried, so this is a new failure path. Since the result is only used structurally (IDs inside a where, never returned as data), overrideAccess: true seems more correct here.
  2. This still trusts breadcrumbs data rather than the actual parent links. If breadcrumbs lag reality (drafts, a failed resaveChildren, or an install already corrupted by the cycles this bug allowed in), those descendants won't carry the edited doc in their breadcrumbs and remain selectable. Walking the stored parent relationships instead — parent: { in: frontier } per level, accumulating IDs — costs one query per tree depth rather than one total, but is immune to stale breadcrumbs; there's a working version in plugin-nested-docs: default parentFilterOptions exclude nothing on Postgres — parent picker offers the doc's own descendants #17658 under "Suggested direction".

On scope, for whoever triages this: I've filed the underlying adapter bug as #17770not_in/not_equals on to-many paths match when any row differs rather than when no row matches. This PR is the right near-term fix for the plugin's default filter, but it shouldn't close #17770: custom filterOptions and user queries using the same clause stay silently broken, and any ID-materialization approach (this PR and my workaround alike) is inherently O(descendants) in query size — the adapter-level NOT EXISTS fix is what makes the original single-clause filter both correct and scalable. Separately, the missing cycle guards (#16517) mean a cycle that gets in via the API still hangs even with this merged.

Thanks for the review! I addressed both suggestions.

Changes made:

Switched descendant resolution to traverse the actual parent relationship instead of breadcrumbs.

Used overrideAccess: true for the internal structural lookup.

Added support for custom parentFieldSlug.

Re-ran pnpm test:int:postgres plugin-nested-docs — 13/13 tests passing.

Let me know if you'd like any further changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

plugin-nested-docs: default parentFilterOptions exclude nothing on Postgres — parent picker offers the doc's own descendants

2 participants