diff --git a/src/Entries/UpdateStructuredEntryOrderAndParent.php b/src/Entries/UpdateStructuredEntryOrderAndParent.php index bd7a5624518..623da3707bb 100644 --- a/src/Entries/UpdateStructuredEntryOrderAndParent.php +++ b/src/Entries/UpdateStructuredEntryOrderAndParent.php @@ -16,12 +16,14 @@ public function handle(CollectionTreeSaved $event) $moved = $diff->moved(); $ids = array_merge($moved, $diff->added()); - if (empty($ids)) { - return; + if (! empty($ids)) { + $collection->updateEntryParent($ids); } - $collection->updateEntryParent($ids); - $collection->updateEntryOrder($this->idsWithDescendants($tree, $moved, $ids)); + // Moved entries and their descendants are kept since stale references in the raw tree can hide their shift. + if ($orderIds = $this->idsWithDescendants($tree, $moved, array_merge($moved, $diff->orderChanged()))) { + $collection->updateEntryOrder($orderIds); + } } private function idsWithDescendants(CollectionTree $tree, array $moved, array $ids): array diff --git a/src/Stache/Stores/CollectionEntriesStore.php b/src/Stache/Stores/CollectionEntriesStore.php index 09109f073da..41a293e34d7 100644 --- a/src/Stache/Stores/CollectionEntriesStore.php +++ b/src/Stache/Stores/CollectionEntriesStore.php @@ -275,10 +275,15 @@ private function updateEntriesWithinIndex($index, $ids) return $index->update(); } + // Write the index to the cache once, rather than once per entry like updateItem() does. + $index->load(); + collect($ids) ->map(fn ($id) => Entry::find($id)) ->filter() - ->each(fn ($entry) => $index->updateItem($entry)); + ->each(fn ($entry) => $index->put($this->getItemKey($entry), $index->getItemValue($entry))); + + $index->cache(); } private function updateEntriesWithinStore($ids) diff --git a/src/Structures/CollectionTreeDiff.php b/src/Structures/CollectionTreeDiff.php index ba907b9022c..f74a9712552 100644 --- a/src/Structures/CollectionTreeDiff.php +++ b/src/Structures/CollectionTreeDiff.php @@ -8,6 +8,7 @@ class CollectionTreeDiff protected $removed = []; protected $moved = []; protected $ancestryChanged = []; + protected $orderChanged = []; protected $expectsRoot = false; private $positions; @@ -25,6 +26,7 @@ public function analyze($old, $new, $expectsRoot = false) $this->removed = $old->keys()->diff($new->keys())->values()->all(); $this->moved = $this->analyzeMoved(); $this->ancestryChanged = $this->analyzeAncestryChanges(); + $this->orderChanged = $this->analyzeOrderChanges($old, $new); return $this; } @@ -105,6 +107,15 @@ public function ancestryChanged() return $this->ancestryChanged; } + /** + * Items whose position in the flattened tree, and so their "order", has changed. + * Unlike moved(), this includes items shifted by changes elsewhere in the tree. + */ + public function orderChanged() + { + return $this->orderChanged; + } + private function preparePositions($old, $new) { $positions = []; @@ -137,4 +148,11 @@ private function analyzeAncestryChanges() return $item[0]['path'] !== $item[1]['path']; })->keys()->all(); } + + private function analyzeOrderChanges($old, $new) + { + $oldOrder = $old->keys()->flip(); + + return $new->keys()->filter(fn ($id, $i) => $oldOrder->get($id) !== $i)->values()->all(); + } } diff --git a/tests/Data/Structures/CollectionTreeTest.php b/tests/Data/Structures/CollectionTreeTest.php index 57e9f9ca816..91858dc9296 100644 --- a/tests/Data/Structures/CollectionTreeTest.php +++ b/tests/Data/Structures/CollectionTreeTest.php @@ -3,6 +3,7 @@ namespace Tests\Data\Structures; use Facades\Tests\Factories\EntryFactory; +use Illuminate\Support\Arr; use Illuminate\Support\Facades\Event; use PHPUnit\Framework\Attributes\Test; use Statamic\Events\CollectionTreeEntriesMovedOrRemoved; @@ -320,6 +321,61 @@ public function reordering_a_parent_keeps_the_order_index_of_its_descendants_in_ ], $this->cachedOrderIndex('pages')); } + #[Test] + public function entries_shifted_by_a_tree_change_keep_their_order_index_in_sync() + { + $collection = tap(Collection::make('pages')->routes('{slug}')->structureContents(['root' => false]))->save(); + + foreach (['alfa', 'a1', 'a2', 'bravo', 'b1'] as $id) { + EntryFactory::collection('pages')->id($id)->slug($id)->data(['title' => $id])->create(); + } + + $collection->structure()->in('en')->tree([ + ['entry' => 'alfa', 'children' => [ + ['entry' => 'a1'], + ]], + ['entry' => 'bravo', 'children' => [ + ['entry' => 'b1'], + ]], + ])->save(); + + // Adding "a2" shifts "bravo" and "b1" without changing their parent or index. + $collection->structure()->in('en')->tree([ + ['entry' => 'alfa', 'children' => [ + ['entry' => 'a1'], + ['entry' => 'a2'], + ]], + ['entry' => 'bravo', 'children' => [ + ['entry' => 'b1'], + ]], + ])->save(); + + $this->assertEquals([ + 'alfa' => 1, + 'a1' => 2, + 'a2' => 3, + 'bravo' => 4, + 'b1' => 5, + ], $this->cachedOrderIndex('pages')); + + // Removing "a2" shifts them back, with nothing moved or added. + $collection->structure()->in('en')->tree([ + ['entry' => 'alfa', 'children' => [ + ['entry' => 'a1'], + ]], + ['entry' => 'bravo', 'children' => [ + ['entry' => 'b1'], + ]], + ])->save(); + + $this->assertEquals([ + 'alfa' => 1, + 'a1' => 2, + 'bravo' => 3, + 'b1' => 4, + ], Arr::except($this->cachedOrderIndex('pages'), 'a2')); + } + private function cachedOrderIndex(string $collection): array { return Stache::store('entries')->store($collection)->index('order')->items()->all();