Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed Undefined Key error in nested relationship #552

Open
wants to merge 1 commit into
base: v6
Choose a base branch
from

Conversation

kohlerdominik
Copy link

@kohlerdominik kohlerdominik commented Feb 21, 2022

Same Fix as #547, which is already merged to v5

But this time directed to v6. @lazychaser is there a particular reason that v5 is the default branch? If i understand correctly, v6 is the current release and therefore should be default?

When using nested, optional relationships (which might be quite an edge case) there is a small possibility that the library is throwing an error.

It is correctly described in the comment above the problematic line, that The first model in the array is always the parent. This can be retrieved in most cases by index 0.

BUT, if the previously called filter in Illuminate/Database/Eloquent/Collection.php:227 happens to filter the model with index 0 (which is perfectly possible, as otherwise the filter would be unnecessary), then we have an Undefined array key 0 error.

Therefore, I propose to change the given implementation to use reset() to get the first model, as this truly returns the first model in the array rather than the model with index 0.

ildyria added a commit to LycheeOrg/laravel-nestedset that referenced this pull request Apr 10, 2022
ildyria added a commit to LycheeOrg/laravel-nestedset that referenced this pull request Apr 10, 2022
…#7)

* Adjusted composer.json
* Adjusted `readme.md`
* Fixes upstream issue lazychaser#538
* exclude 8.1 low
* Fixed order of deletion. (#5)
* Added ORDER BY to delete operation.
* lazychaser#552
* bumping version of phpunit to 9.5.20

Co-authored-by: Matthias Nagel <[email protected]>
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.

1 participant