Add top-level jsonapi member #1050

bf4 · 2015-08-12T02:26:06Z

Ref: http://jsonapi.org/format/#document-jsonapi-object

jsonapi_include_toplevel_object: Whether to include a top level JSON API member
in the response document.
Default: false.
Used when jsonapi_include_toplevel_object is true:
- jsonapi_version: The latest version of the spec the API conforms to.
  Default: '1.0'.
- jsonapi_toplevel_meta: Optional metadata. Not included if empty.
  Default: {}.

Example meta usage:

              jsonapi_include_toplevel_object = true,
              jsonapi_toplevel_meta = {
                'copyright' => 'Copyright 2015 Example Corp.'
              }

joaomdmoura · 2015-08-17T04:02:02Z

Yup! I'm interest on this @bf4

bf4 · 2015-08-21T05:40:19Z

This is a little harder than I thought due the somewhat recursive way adapters work.

joaomdmoura · 2015-08-21T05:42:56Z

No worries, take your time, let me know if there is something I can do in order to help you.

bf4 · 2015-08-21T05:48:59Z

Well, I think I'm going to work on other stuff for a bit. I was just looking at warnings, and also at how using class Adapter as both a namespace and subclass causes some circular requires, and if maybe we should separate AbstractAdapter out of Adapter, leaving just the create and adapter_class methods (plus those added by the #1017 )

joaomdmoura · 2015-08-21T05:50:45Z

Yep, please, a warnings clearing would be great! 😄

bf4 · 2015-09-16T14:57:52Z

lib/active_model/serializer/adapter/json_api.rb

@@ -54,7 +78,7 @@ def serializable_hash_for_single_resource(serializer, options)
          primary_data = primary_data_for(serializer, options)
          relationships = relationships_for(serializer)
          included = included_for(serializer)
-          hash = { data: primary_data }
+          hash[:data] = primary_data # FIXME: should be an [], not a {}


n/m, I guess this is legal

Primary data MUST be either:

a single resource object, a single resource identifier object, or null, for requests that target single resources

an array of resource objects, an array of resource identifier objects, or an empty array ([]), for requests that target resource collections

It is, indeed.

More than legal, it is actually the right behavior when targeting a single resource.

rebased to remove comment, but this comment remains...

bf4 · 2015-09-16T14:59:59Z

docs/general/configuration_options.md

@@ -9,3 +9,9 @@ The following configuration options can be set on `ActiveModel::Serializer.confi
 ## JSON API

 - `jsonapi_resource_type`: Whether the `type` attributes of resources should be singular or plural. Possible values: `:singular, :plural`. Default: `:plural`.
+- `jsonapi_toplevel_member`: Whether to include a [top level JSON API member](http://jsonapi.org/format/#document-jsonapi-object)


defined in JsonApi adapter

beauby · 2015-09-16T15:02:56Z

CHANGELOG.md

@@ -11,4 +11,5 @@
  * remove root key option and split JSON adapter [@joaomdmoura]
  * adds FlattenJSON as default adapter [@joaomdmoura]
  * adds support for `pagination links` at top level of JsonApi adapter [@bacarini]
-  * adds extended format for `include` option to JSONAPI adapter [@beauby]
+  * adds extended format for `include` option to JsonApi adapter [@beauby]
+  * adds support for top level jsonapi member support [@beauby]


There's an extraneous "support" at the end. Plus it should read [@beauby, @bf4] :)

beauby · 2015-09-17T11:48:49Z

lib/active_model/serializer/adapter/json_api.rb

+              object = {
+                jsonapi: {
+                  version: ActiveModel::Serializer.config.jsonapi_version,
+                  meta: ActiveModel::Serializer.config.jsonapi_toplevel_meta


Meta is optional as per the spec, so it should't be included by default.

beauby · 2015-09-21T15:55:39Z

lib/active_model/serializer/adapter/json_api.rb

+              ActiveModel::Serializer.config.jsonapi_include_toplevel_member
+            end
+
+            def add!(document)


Why the side-effects 😿

what side-effects?

Having add! modify its argument instead of returning a value.

But, isn't this question the same as why do I prefer foo.merge!(bar) over foo.merge!(build_bar)?

Hmm not really, to me, it's more a matter of do_some_magic(foo) vs foo[:attribute] = compute_something.

Would you mind just adding a commit to my branch? You obviously have a pretty strong opinions I don't share, so it would be faster if you changed it

I really don't care that strongly since I see it as internal for now till we figure out how we'd like to use it, if at all, as long as it's a function

Will do immediately.

Since you have the (first and) last commit, do you might (rebasing against master and) getting it passing on ci?

beauby · 2015-09-21T19:13:32Z

lib/active_model/serializer/adapter/json_api.rb

+              serializable_hash_for_single_resource(options)
+            end
+
+          ApiObjects::JsonApi.include_member? && hash.merge!(ApiObjects::JsonApi.member)


Do you prefer it like this?

I do, esp. if you're ok with it... I'm willing to be persuaded... the whole ApiObjects::JsonApi is just an idea I'm playing with.. not very useful if it's the only one, but might lead to good abstractions for other objects, like errors, paginations, meta, links, etc.

What do you think of renaming member to object? It would align with serializer.object and make sense in context.

I'm ok with renaming member to object, although both those names are a bit confusing.

beauby · 2015-09-30T00:05:53Z

@bf4 Let's pair on this one when you have time and get it over with.

NullVoxPopuli · 2015-09-30T11:38:24Z

👍

bf4 · 2015-10-01T17:55:09Z

lib/active_model/serializer/adapter/json_api/api_objects.rb

+            end
+
+            def object
+              object = {


change to @object ||=

bf4 · 2015-10-01T18:24:19Z

@beauby squashed to one commit, set you as the 'committer' git log --pretty='%cn' -n1 HEAD shows you (I'm the 'author')

didn't do the jsonapi.config as it was funky and not high value

bf4 · 2015-10-01T18:24:51Z

lib/active_model/serializer/adapter/json_api.rb

+            ActiveModel::Serializer.config.jsonapi_version = '1.0'
+            ActiveModel::Serializer.config.jsonapi_toplevel_meta = {}
+            # Make JSON API top-level jsonapi member opt-in
+            # ref: http://jsonapi.org/format/#document-top-level


Add top-level jsonapi member to JSON API adapter

joaomdmoura added this to the 0.10 milestone Aug 17, 2015

joaomdmoura added the V: 0.10.x label Aug 17, 2015

bf4 mentioned this pull request Aug 31, 2015

Make Adapters registerable so they are not namespace-constrained #1017

Merged

9 tasks

beauby added the C: Feature label Sep 14, 2015

bf4 force-pushed the json_api_member branch from fadd2b0 to 87d587e Compare September 16, 2015 14:57

bf4 reviewed Sep 16, 2015
View reviewed changes

bf4 force-pushed the json_api_member branch 2 times, most recently from bea3f7e to e821811 Compare September 16, 2015 14:59

bf4 reviewed Sep 16, 2015
View reviewed changes

bf4 mentioned this pull request Sep 16, 2015

Add support for jsonapi top level member #1147

Closed

beauby reviewed Sep 16, 2015
View reviewed changes

bf4 force-pushed the json_api_member branch from e821811 to a48485d Compare September 16, 2015 15:17

joaomdmoura force-pushed the master branch from e40fd5a to 1ca73e0 Compare September 16, 2015 15:20

beauby reviewed Sep 17, 2015
View reviewed changes

bf4 force-pushed the json_api_member branch 4 times, most recently from 7789371 to 4775d8f Compare September 18, 2015 04:43

bf4 force-pushed the json_api_member branch 2 times, most recently from 8223f41 to e34c760 Compare September 21, 2015 15:41

beauby reviewed Sep 21, 2015
View reviewed changes

beauby force-pushed the json_api_member branch 4 times, most recently from 7b22480 to 43c56ac Compare September 21, 2015 19:13

beauby reviewed Sep 21, 2015
View reviewed changes

bf4 reviewed Oct 1, 2015
View reviewed changes

lib/active_model/serializer/adapter/json_api/api_objects.rb

end

def object

object = {

Copy link

Member Author

bf4 Oct 1, 2015

Choose a reason for hiding this comment

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

change to @object ||=

bf4 force-pushed the json_api_member branch from c1d8e95 to 06ae191 Compare October 1, 2015 18:23

bf4 reviewed Oct 1, 2015
View reviewed changes

Add support for top level jsonapi member.

9d7da8a

bf4 force-pushed the json_api_member branch from 06ae191 to 9d7da8a Compare October 2, 2015 02:22

bf4 added a commit that referenced this pull request Oct 2, 2015

Merge pull request #1050 from bf4/json_api_member

a2bfe19

Add top-level jsonapi member to JSON API adapter

bf4 merged commit a2bfe19 into rails-api:master Oct 2, 2015

bf4 deleted the json_api_member branch October 2, 2015 04:03

joaomdmoura mentioned this pull request Oct 6, 2015

Full JSON-API support #1235

Open

11 tasks

bf4 mentioned this pull request Jan 29, 2016

Document JSON API implementation defs and progress in class #1482

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add top-level jsonapi member #1050

Add top-level jsonapi member #1050

bf4 commented Aug 12, 2015

joaomdmoura commented Aug 17, 2015

bf4 commented Aug 21, 2015

joaomdmoura commented Aug 21, 2015

bf4 commented Aug 21, 2015

joaomdmoura commented Aug 21, 2015

bf4 Sep 16, 2015

bf4 Sep 16, 2015

beauby Sep 16, 2015

beauby Sep 16, 2015

bf4 Sep 16, 2015

bf4 Sep 16, 2015

bf4 Sep 16, 2015

beauby Sep 16, 2015

beauby Sep 17, 2015

beauby Sep 21, 2015

bf4 Sep 21, 2015

beauby Sep 21, 2015

bf4 Sep 21, 2015

beauby Sep 21, 2015

bf4 Sep 21, 2015

bf4 Sep 21, 2015

beauby Sep 21, 2015

beauby Sep 21, 2015

bf4 Sep 21, 2015

beauby Sep 21, 2015

bf4 Sep 21, 2015

beauby Sep 22, 2015

beauby commented Sep 30, 2015

NullVoxPopuli commented Sep 30, 2015

bf4 Oct 1, 2015

bf4 commented Oct 1, 2015

bf4 Oct 1, 2015

Add top-level jsonapi member #1050

Add top-level jsonapi member #1050

Conversation

bf4 commented Aug 12, 2015

joaomdmoura commented Aug 17, 2015

bf4 commented Aug 21, 2015

joaomdmoura commented Aug 21, 2015

bf4 commented Aug 21, 2015

joaomdmoura commented Aug 21, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

beauby commented Sep 30, 2015

NullVoxPopuli commented Sep 30, 2015

Choose a reason for hiding this comment

bf4 commented Oct 1, 2015

Choose a reason for hiding this comment