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

JsonAPI: Including one level of linked data #137

Closed
wants to merge 3 commits into from

Conversation

renan
Copy link
Contributor

@renan renan commented Dec 4, 2014

Continuation of #126 (comment)

Right now it only includes one level deep of linked data.
It is damn hard to have it all, especially when you consider #125 and you no longer have an array of item but a single item.

I think some refactoring would need to be done in order to have it nailed, starting on Manager and Scope. One depending on each other makes things harder.
The Serializer would need to keep track of included/linked data, from top to bottom, so it can ensure uniqueness.

@philsturgeon
Copy link
Member

Can you pull recent changes and update any conflicts, then I'll get this merged. I waited a while because I planned to tackle this myself, but your solution looks good to me.

@renan
Copy link
Contributor Author

renan commented May 4, 2015

Done.

But looking at the spec again they seem to have changed it. Instead of having:

'links' => array(
    'author' => 1,
),

It seems the new spec is:

'links' => array(
    'author' => array(
        'linkage' => array(
            'type' => 'author',
            'id' => 1,
        ),
    ),
),

A lot more verbose.
May not be a very good format to support until they don't tag a version I would say.

@philsturgeon
Copy link
Member

Yeah, I might just wait until they’re done and then implement it fresh. Yay for dev releases!

-- 
Phil

From: Renan Gonçalves notifications@github.com
Reply: thephpleague/fractal reply@reply.github.com>
Date: May 4, 2015 at 5:55:50 AM
To: thephpleague/fractal fractal@noreply.github.com>
Cc: Phil Sturgeon me@philsturgeon.uk>
Subject:  Re: [fractal] JsonAPI: Including one level of linked data (#137)

Done.

But looking at the spec again they seem to have changed it. Instead of having:

'links' => array(
'author' => 1,
),

It seems the new spec is:

'links' => array(
'author' => array(
'linkage' => array(
'type' => 'author',
'id' => 1,
),
),
),

A lot more verbose.
May not be a very good format to support until they don't tag a version I would say.


Reply to this email directly or view it on GitHub.

@boris-glumpler
Copy link

According to their status page they're on RC3 atm and will only make very small tweaks from now on. Sounds like it's ok to tackle it already. Also, one of the contributors for JSON API posted this relevant comment on the FOSRestBundle repo.

@webda2l
Copy link
Contributor

webda2l commented May 20, 2015

There are some trouble with new linkage specification json-api/json-api#623.
Maybe this current PR could be merge to avoid current limitation of one level if discussion of new JsonAPI specification take time..

@jasonlewis
Copy link
Contributor

We've now merged an improved JSON API serializer.

@jasonlewis jasonlewis closed this Aug 31, 2015
@renan renan deleted the deep-includes branch September 1, 2015 09:02
@renan
Copy link
Contributor Author

renan commented Sep 1, 2015

💯 👏

@renan renan restored the deep-includes branch September 14, 2015 11:08
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.

5 participants