fix(has-many): keep children when a nullable collection is reordered - #576
Merged
Merged
Conversation
…relation Reordering, prepending to or replacing inside a nullable HasMany collection nulls the foreign key of children that stay in it, because the `calcDeleted()` comparator never returns a positive value and `array_udiff()` reports live items as removed (#575). Assisted-By: Claude Opus 5.5 <noreply@anthropic.com>
The `array_udiff()` comparator returned only 0 or -1, so the sort-based diff reported children that stayed in the collection as removed, and a nullable relation nulled their foreign keys. A lookup by `spl_object_id()` needs no ordering contract and runs in linear time. Fixes #575. Assisted-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 2.x #576 +/- ##
=========================================
Coverage 91.62% 91.62%
- Complexity 2028 2031 +3
=========================================
Files 132 132
Lines 5275 5277 +2
=========================================
+ Hits 4833 4835 +2
Misses 442 442 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
1 participant
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.
🔍 What was changed
HasMany::calcDeleted()finds removed children with anspl_object_id()lookup instead ofarray_udiff(), so the result no longer depends on the order of items in the collection.Case575reproduces the bug for all drivers: reordering, prepending to, and replacing inside a nullableHasManycollection.Note
The bug lost data, not just broke a contract: with a nullable
HasManywhose child has aBelongsToback to the parent, children that stayed in the collection had their foreign key set toNULL. Prepending a new item detached every existing child. Non-nullable relations were unaffected because the followingattachStore()turned the wrong delete back into a store.🤔 Why?
The old comparator returned only
0or-1.array_udiff()sorts both arrays with it, so the diff reported live items as removed on every PHP version from 8.1 to 8.5.📝 Checklist