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

Including the resource once if an 'id' is present. #126

Merged
merged 1 commit into from
Nov 19, 2014

Conversation

renan
Copy link
Contributor

@renan renan commented Nov 19, 2014

Fix #84

From the Specification:

Each resource object SHOULD contain a unique identifier, or ID, when available.

From the Specification:
> Each resource object SHOULD contain a unique identifier, or ID, when available.
philsturgeon added a commit that referenced this pull request Nov 19, 2014
Including the resource once if an 'id' is present.
@philsturgeon philsturgeon merged commit cceab47 into thephpleague:master Nov 19, 2014
@philsturgeon
Copy link
Member

Awesome! Followup thought, could you add a test that tries to put the same bit of included data in twice (linked by two different parents) then prove it only gets placed in once?

@renan renan deleted the jsonapi-unique-results branch November 19, 2014 12:46
@renan
Copy link
Contributor Author

renan commented Nov 19, 2014

I've added this test: https://github.com/renan/fractal/compare/deep-includes

Which is now failing because the review author doesn't get included:

 Array (
     'book' => Array (...)
     'linked' => Array (
         'author' => Array (
             0 => Array (...)
-            1 => Array (...)
         )
         'reviews' => Array (...)
     )
 )

I tested with the code before the PR was applied and it was adding a linked key inside the already existing linked key:

             1 => Array (
-                'id' => 2
-                'name' => 'Bob'
+                'id' => 1
+                'name' => 'Dave'
             )
         )
         'reviews' => Array (...)
+        'linked' => Array (...)
     )
 )

I am not sure how to fix this.

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.

Unique JsonApiSerializer return objects
3 participants