[6.x] Fix reordering a paginated orderable collection corrupting the tree - #15238
Merged
jasonvarga merged 5 commits intoAug 21, 2026
Merged
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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()throwsDuplicate 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 returnsnull,Entry::order()doesnull + 1, and the new entry is indexed as1— colliding with whichever entry legitimately sits at tree index 0. That value gets persisted to theorderindex.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.ReorderEntriesControllerzipped the tree's page slice against the submitted ids andput()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 onTreewas only ever cleared inTree::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.ReorderEntriesControllernow 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.