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

Fix For Unexpected Behaviour When To-One Relationship Data Is NULL - Closes #7 #8

Merged
merged 4 commits into from
Nov 17, 2017

Conversation

ginnj-gilmore
Copy link
Contributor

Please see issue #7 for full information.

JSONAPI spec states to-one relationships with no associated resource should return {"data": null} but Yang does not properly account for this when converting Relationship to an array. This pull request fixes Yang returning an empty to-many relationship (['data' => []]) instead of an empty to-one relationship (['data' => null]).

@ginnj-gilmore
Copy link
Contributor Author

ginnj-gilmore commented Nov 16, 2017

@kocsismate That's strange... Travis says a specific test fails relating to the changes I made, but PHPUnit on my local doesn't through any errors. Do you want me to update the WoohooLabs\Yang\Tests\JsonApi\Schema\ResourceObjectTest::toArray() and WoohooLabs\Yang\Tests\JsonApi\Schema\RelationshipTest::toArray() tests?

@kocsismate kocsismate merged commit c53bf21 into woohoolabs:master Nov 17, 2017
@kocsismate
Copy link
Member

I'll take care of the tests, thank you :) And I'll get back to you when I release the new versions with your fix.

kocsismate pushed a commit that referenced this pull request Nov 17, 2017
…loses #7 (#8)

* Fixed to-one relationships with null data being returned as empty to-many relationship

* Fixed toArray() not returning expected ['data' => null] in nullable relationships

* Changed is_null to identical comparison when checking for 'data' in Relationship::createFromArray()

* Applying fixes for StyleCI
@kocsismate
Copy link
Member

I managed to fix the problem, but it was a bit more tricky than I thought. :) Furthermore, I have just cherry-picked our commits to the 1.2 branch. So I'd like to ask you to test it. If all is good, then I'll release 1.2.1 and 1.3.1.

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.

2 participants