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

Always return "included" where "include" has been requested #1236

Merged
merged 4 commits into from Nov 24, 2017

Conversation

Projects
None yet
2 participants
@noetix
Copy link
Contributor

noetix commented Nov 11, 2017

#1230

One additional thing that may be worthwhile is reiterating that the included param must always be an array, and that an empty array should be returned when no additional resources are available to add to the response.

Open to feedback. :)

@ethanresnick

This comment has been minimized.

Copy link
Member

ethanresnick commented Nov 11, 2017

This is great. Just so we're not repeating the clause "If an endpoint supports the include parameter and a client supplies it", maybe we can merge the paragraph you added with the one that follows?

Something like:

If an endpoint supports the include parameter and a client supplies it:

  1. The server's response MUST be a [compound document] with an included key—even if that included key holds an empty array (because the requested relationships are empty).

  2. This included section MUST NOT include [resource objects] not requested through the include parameter's value.

noetix added some commits Nov 13, 2017

@noetix

This comment has been minimized.

Copy link
Contributor Author

noetix commented Nov 13, 2017

Opted to leave the second list item as-is.

Good call on the repetition.

@ethanresnick

This comment has been minimized.

Copy link
Member

ethanresnick commented Nov 23, 2017

The server MUST respond with the included section of the [compound document] — even if

I realize I'm really nitpicking here, and it's probably more important to just get this merged, but something about the wording quoted above bothers me. Specifically, I think having an included section is part of what it means to be a compound document (i.e., you can't be a compound document without one).

Your wording seems to suggest, though, that the response is already a compound document, which it isn't until the included section is added. That's why, in my wording, I tried to say that the the response must be a compound document. Then, I added "with an included key"—even though that's technically redundant—just to maximize readability in case the full implications of "being a compound document" weren't obvious.

Can we stick with my wording for the first bullet, and then I'll get this merged?

@noetix

This comment has been minimized.

Copy link
Contributor Author

noetix commented Nov 24, 2017

@ethanresnick Agreed.

@ethanresnick

This comment has been minimized.

Copy link
Member

ethanresnick commented Nov 24, 2017

Great. Merged!

@ethanresnick ethanresnick merged commit c762b92 into json-api:gh-pages Nov 24, 2017

dgeb added a commit that referenced this pull request Feb 9, 2018

Always return "included" where "include" has been requested (#1236)
* Always return "included" where "include" has been requested

* Less repetition; clarify empty response

* Fixed em-dash; wrapping

* Feedback change

joananeves added a commit to joananeves/json-api that referenced this pull request Apr 18, 2018

Merge branch 'json-api-gh-pages' into gh-pages
* json-api-gh-pages: (259 commits)
  jsonapi-realizer is a new server side hander, intended to be a sister to jsonapi-serializers (json-api#1267)
  Added DenaliJS to the Node framework list (json-api#1264)
  Added Vox to iOS implementations (json-api#1262)
  Update scala-jsonapi link (json-api#1263)
  added sarala javascript packages to implementations (json-api#1260)
  Add SimpleJSONAPIClient to Ruby implementations (json-api#1257)
  Removed py-jsonapi (json-api#1258)
  Move fast_jsonapi to it\'s proper place in the ruby server list (json-api#1256)
  Clarify pagination links in relationship object (json-api#1251)
  move safrs to server libraries (json-api#1253)
  Add fast_jsonapi to Ruby Lists (json-api#1252)
  JSON API Playground link added to implementations (json-api#1248)
  Add SAFRS to list (json-api#1250)
  Always return "included" where "include" has been requested (json-api#1236)
  ngx-jsonapi added to implementations (json-api#1228)
  PHP library for JSON API + Doctrine
  add @crnk/angular-ngrx implementation
  Add jsonapi-mock to implementations
  Add raml-json-api to implementations
  Clarify pending async process response json-api#1223
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.