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

[8.x] Fix issue with polymorphic morphMaps with literal 0 #35364

Merged
merged 2 commits into from
Nov 26, 2020

Conversation

iBotPeaches
Copy link
Contributor

Background

Ran into project where

Relation::morphMap([
 0 => Model::class
 1 => Model2::class
]);

During a legacy upgrade I noticed failures after completing the 5.8.x upgrade. At first I tracked it to this - #27175, which then resulted in changing my morphTo parameters to null, null, null, 'id' so ownerKey would be set properly.

It still failed. It appeared that getting a 0 zero value back from entity_type (entity morph) was passing the empty check in the wrong way no longer resulting in getting the morphed instance back.

The comment above the code change even says "if value is null", so probably just a mistake.

Test

Never wrote a test for the DB area before. Hope it works out. Open for feedback.

@taylorotwell taylorotwell merged commit ff1baa1 into laravel:8.x Nov 26, 2020
@dmytro-laba
Copy link

@iBotPeaches @taylorotwell Thank you for this fix. Could you tell if this change will also be added to 6.x version?
Or, what can I do to get this also fixed in 6.x?

driesvints pushed a commit that referenced this pull request Dec 4, 2020
* fix: use strict null check on morphTo type

* test: assert 0 relation_type is handled correctly
@driesvints
Copy link
Member

@dmytro-laba I've sent this to 6.x here: #35487. Thanks

@dmytro-laba
Copy link

@driesvints Thanks a lot!

taylorotwell pushed a commit that referenced this pull request Dec 4, 2020
#35364) (#35487)

* [8.x] Fix issue with polymorphic morphMaps with literal 0 (#35364)

* fix: use strict null check on morphTo type

* test: assert 0 relation_type is handled correctly

* Use setMethods for 6.x

* Update HasRelationships.php

Co-authored-by: Connor Tumbleson <[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.

4 participants