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

Do not support to include related ressources as attributes #10

Merged
merged 1 commit into from
May 2, 2016

Conversation

jelhan
Copy link
Contributor

@jelhan jelhan commented Apr 28, 2016

A related ressource should not be included as an attribute value:

Although has-one foreign keys (e.g. author_id) are often stored internally
alongside other information to be represented in a resource object, these
keys SHOULD NOT appear as attributes.

Resource objects have a relationships property which should describe it's
relationships. Relationships could be described by link objects or resource
linkage. Resource linkage must be used if it's a compound document.

Of course an attributes value may be a JSON object or an array:

Complex data structures involving JSON objects and arrays are allowed as
attribute values. However, any object that constitutes or is contained in
an attribute MUST NOT contain a relationships or links member, as those
members are reserved by this specification for future use.

Better explained in these these issue:
json-api/json-api#902

The case that is most correct is the last one (i.e. relationships are
always present). The only time when a relationship shouldn't be present
is if it's been filtered out with the ?fields parameter. In this sense,
relationships are just like attributes.

To your point about the possible overhead of including the relationship
ids in to-many relationships: this is exactly why "links" can be used as
a substitute for "data"!

@jelhan
Copy link
Contributor Author

jelhan commented Apr 28, 2016

@IanVS do you have another pair of eyes? I'm not a hundred percent sure that I understood it correctly.

@IanVS
Copy link
Contributor

IanVS commented Apr 29, 2016

Yeah I'll give it some review tonight.

@IanVS
Copy link
Contributor

IanVS commented Apr 29, 2016

It's been a long time since I've looked at any of this, but I think my goal was to shove the related data into the attributes instead of as a compound document. You've pointed out this is incorrect, and the section I linked to in the spec was regarding include query parameters from a client to specify the items to return in a compound document section. So, I think you're doing the right thing here.

t.ok(_.isArray(body.included), 'Top-lebel member included is an array');
t.ok(
// all ressources in body
body.data.every(function (ressource, index) {
Copy link
Contributor

@IanVS IanVS Apr 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resource should be spelled with one s (throughout, including includedRessource, mainRessource, etc. )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

A related ressource should not be included as an attribute value:

> Although has-one foreign keys (e.g. author_id) are often stored internally
> alongside other information to be represented in a resource object, these
> keys SHOULD NOT appear as attributes.

Resource objects have a *relationships* property which should describe it's
relationships. Relationships could be described by link objects or resource
linkage. Resource linkage must be used if it's a compound document.

Of course an attributes value may be a JSON object or an array:

> Complex data structures involving JSON objects and arrays are allowed as
> attribute values. However, any object that constitutes or is contained in
> an attribute MUST NOT contain a relationships or links member, as those
> members are reserved by this specification for future use.

Better explained in these these issue:
json-api/json-api#902

> The case that is most correct is the last one (i.e. relationships are
> always present). The only time when a relationship shouldn't be present
> is if it's been filtered out with the ?fields parameter. In this sense,
> relationships are just like attributes.
>
> To your point about the possible overhead of including the relationship
> ids in to-many relationships: this is exactly why "links" can be used as
> a substitute for "data"!
@jelhan jelhan merged commit a6317fb into sane:master May 2, 2016
@jelhan jelhan deleted the nested branch July 22, 2016 11:31
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