Skip to content

[6.x] Fix reordering a paginated orderable collection corrupting the tree - #15238

Merged
jasonvarga merged 5 commits into
statamic:6.xfrom
marcorieser:fix/reorder-duplicate-tree-entries
Aug 21, 2026
Merged

[6.x] Fix reordering a paginated orderable collection corrupting the tree#15238
jasonvarga merged 5 commits into
statamic:6.xfrom
marcorieser:fix/reorder-duplicate-tree-entries

Conversation

@marcorieser

Copy link
Copy Markdown
Contributor

Fixes #15116
Fixes #11209

Reordering a paginated orderable collection can write a duplicate entry into the tree and silently drop another one. Once that happens, CollectionStructure::validateTree() throws Duplicate entry [...] in [...] collection's structure. on every read, so the CP listing and the front-end both 500 until the tree YAML is edited by hand.

What goes wrong

1. A new entry gets a colliding order.

Tree::tree() memoises the healed tree (validateTree() appends entries that aren't in the stored tree yet) under a key built from the stored tree only. Creating an entry changes the entry set but not the stored tree, so the key is unchanged and the stale healed tree is served for the rest of the request. entryOrder() then returns null, Entry::order() does null + 1, and the new entry is indexed as 1 — colliding with whichever entry legitimately sits at tree index 0. That value gets persisted to the order index.

2. The reorder then corrupts the tree.

The listing is sorted by order, so the new entry shows up on page 1 while the tree still has it last. ReorderEntriesController zipped the tree's page slice against the submitted ids and put() the results, which is only safe when both are the same set. When they aren't, one id is written twice and another is never written back.

The fix

  • Structure::flushCache() / Tree::flushCache() — the flattened-page memoisation on Tree was only ever cleared in Tree::save(), so clearing the Blink entries alone wasn't enough. Extracted it so it can be flushed on demand, optionally scoped to a single site.
  • Entry::save() flushes those caches and recalculates the order for a new entry in an orderable collection, so it's resolved against a tree that actually knows about it.
  • ReorderEntriesController now checks the submitted ids are a rearrangement of the page being reordered, and aborts with a 409 instead of writing something it can't write correctly. Happy to change this to silently reconciling if you'd prefer it never errors.

Also fixes reordering in a collection with sort_dir: desc, which was corrupting the tree on any paginated reorder — the payload was reversed but the slice was still taken from the front of the tree. The controller now works in listing order and flips back before saving.

Note on performance

Creating entries in an orderable collection is slower, because each new entry now re-heals the tree instead of reusing a cached (and wrong) one. Bulk-creating 500 entries went from ~540ms to ~1200ms locally. Per-entry correctness needs a per-entry heal, since each new entry changes what the correct healed tree is, so I didn't see a way around it without giving up the guarantee.

Worth flagging separately: validateTree() caches the query builder and chains ->where('site', $locale) onto that same object on every call, so where-clauses accumulate. flushCache() clears that entry, but any code path that heals repeatedly within one request is quadratic today — with that cache left in place the 500-entry case above takes 16s.

@jasonvarga
jasonvarga merged commit 73c2f74 into statamic:6.x Aug 21, 2026
61 checks passed
@marcorieser
marcorieser deleted the fix/reorder-duplicate-tree-entries branch August 28, 2026 06:47
duncanmcclean added a commit to statamic/eloquent-driver that referenced this pull request Aug 31, 2026
the previous fix hardcoded the expected `order` values, which only
held for statamic/cms v6.29+. older supported versions (still within
our ^6.10 constraint) don't have the fix from statamic/cms#15238, so
new entries in an orderable collection still collide at `order = 1`
until `updateOrders()` is called explicitly.

comparing against a snapshot taken before the structure save is
tested regardless of which behaviour is installed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants