Skip to content

Add support for jsonapi top level member #1147

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Add support for jsonapi top level member #1147

wants to merge 1 commit into from

Conversation

@beauby
Copy link
Contributor

@beauby beauby commented Sep 14, 2015

C.f. http://jsonapi.org/format/#document-top-level

Supports :

  • a global boolean config option jsonapi_toplevel_member, that decides whether a top level jsonapi member will be included in each serialized resource, which defaults to false,
  • a global option jsonapi_version, which defaults to 1.0,
  • an adapter option jsonapi_toplevel_meta.

This PR is based on #1121 for convenience.

@beauby beauby force-pushed the add-top-level-jsonapi branch 2 times, most recently from 2f40b25 to 336972d Compare September 14, 2015 04:47
@beauby beauby force-pushed the add-top-level-jsonapi branch 5 times, most recently from 47509d8 to c3c992c Compare September 15, 2015 08:20
joaomdmoura
joaomdmoura reviewed Sep 15, 2015
View reviewed changes
docs/general/configuration_options.md
Copy link
Member

@joaomdmoura joaomdmoura Sep 15, 2015

Choose a reason for hiding this comment

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

Nice 👍

@joaomdmoura
Copy link
Member

joaomdmoura commented Sep 15, 2015

This is cool, I'm not sure about the adapter options, feels like edge cases for me that might add complexity to maintain.
It also makes me thing we indeed need to add namespace to the config options 😁 It will get mess quickly if we keep it this way and adding options.

@beauby
Copy link
Contributor Author

beauby commented Sep 15, 2015

Hmm namespacing config options... I remember someone very wise mentioning that in #1097 :p
I don't mind removing the option for now.

@joaomdmoura
Copy link
Member

joaomdmoura commented Sep 15, 2015

So lets remove the adapter options? So can add it if someone supports/needs it 😁
We can handle the namespace config on other PR 😄

@beauby beauby force-pushed the add-top-level-jsonapi branch 2 times, most recently from 9b8716b to 1a167f8 Compare September 15, 2015 10:17
@beauby beauby force-pushed the add-top-level-jsonapi branch from 1a167f8 to 8ab4c4a Compare September 15, 2015 10:18
@beauby
Copy link
Contributor Author

beauby commented Sep 15, 2015

Done 👍

@NullVoxPopuli
Copy link
Contributor

NullVoxPopuli commented Sep 16, 2015

I'm good with this. 👍

bf4
bf4 reviewed Sep 16, 2015
View reviewed changes
test/adapter/json_api/toplevel_jsonapi_test.rb
Copy link
Member

@bf4 bf4 Sep 16, 2015

Choose a reason for hiding this comment

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

no need for with_adapter. Just pass in adapter: :json_api

@bf4
Copy link
Member

bf4 commented Sep 16, 2015

ref: https://github.com/rails-api/active_model_serializers/pull/1050/files#diff-ae1e38e96a8bf28ae37918a65c448c73R7

bf4
bf4 reviewed Sep 16, 2015
View reviewed changes
lib/active_model/serializer/configuration.rb
Copy link
Member

@bf4 bf4 Sep 16, 2015

Choose a reason for hiding this comment

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

should be in the JsonApi adapter IMHO

Copy link
Contributor Author

@beauby beauby Sep 16, 2015

Choose a reason for hiding this comment

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

Yeah, that makes sense.

bf4 added a commit to bf4/active_model_serializers that referenced this pull request Sep 16, 2015
@bf4
Move JsonApi object config to JsonApi adapter
87d587e 
Finish rails-api#1147
bf4 added a commit to bf4/active_model_serializers that referenced this pull request Sep 16, 2015
@bf4
Move JsonApi object config to JsonApi adapter
bea3f7e 
Finish rails-api#1147
bf4 added a commit to bf4/active_model_serializers that referenced this pull request Sep 16, 2015
@bf4
Move JsonApi object config to JsonApi adapter
e821811 
Finish rails-api#1147
@bf4
Copy link
Member

bf4 commented Sep 16, 2015

Alternative implementation at #1050 which was rebased to include this work

bf4 added a commit to bf4/active_model_serializers that referenced this pull request Sep 16, 2015
@bf4
Move JsonApi object config to JsonApi adapter
a48485d 
Finish rails-api#1147
bf4 added a commit to bf4/active_model_serializers that referenced this pull request Sep 18, 2015
@bf4
Add basic JsonApiObject behavior
73f0e87 
Finish rails-api#1147
bf4 added a commit to bf4/active_model_serializers that referenced this pull request Sep 18, 2015
@bf4
Add basic JsonApiObject behavior
bd221f5 
Finish rails-api#1147
bf4 added a commit to bf4/active_model_serializers that referenced this pull request Sep 18, 2015
@bf4
Add basic JsonApiObject behavior
7789371 
Finish rails-api#1147
bf4 added a commit to bf4/active_model_serializers that referenced this pull request Sep 18, 2015
@bf4
Add basic JsonApiObject behavior
4775d8f 
Finish rails-api#1147
bf4 added a commit to bf4/active_model_serializers that referenced this pull request Sep 18, 2015
@bf4
Add basic JsonApiObject behavior
eb22741 
Finish rails-api#1147
@NullVoxPopuli
Copy link
Contributor

NullVoxPopuli commented Sep 19, 2015

needs rebase :-(

@beauby
Copy link
Contributor Author

beauby commented Sep 21, 2015

Closing in favor of #1050.

@beauby beauby closed this Sep 21, 2015
bf4 added a commit to beauby/active_model_serializers that referenced this pull request Sep 21, 2015
@bf4 @beauby
Add basic JsonApiObject behavior
d4be344 
Finish rails-api#1147
bf4 added a commit to bf4/active_model_serializers that referenced this pull request Sep 21, 2015
@bf4 @beauby
Add basic JsonApiObject behavior
caa1ca5 
Finish rails-api#1147
bf4 added a commit to bf4/active_model_serializers that referenced this pull request Sep 21, 2015
@bf4 @beauby
Add basic JsonApiObject behavior
c912203 
Finish rails-api#1147
bf4 added a commit to bf4/active_model_serializers that referenced this pull request Sep 21, 2015
@bf4
Add basic JsonApiObject behavior
1360b6a 
Finish rails-api#1147
bf4 added a commit to bf4/active_model_serializers that referenced this pull request Sep 21, 2015
@bf4
Add basic JsonApiObject behavior
8223f41 
Finish rails-api#1147
bf4 added a commit to bf4/active_model_serializers that referenced this pull request Sep 21, 2015
@bf4
Add basic JsonApiObject behavior
e34c760 
Finish rails-api#1147
bf4 added a commit to bf4/active_model_serializers that referenced this pull request Sep 21, 2015
@bf4 @beauby
Add basic JsonApiObject behavior
d8802fb 
Finish rails-api#1147
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

C: Feature C: JSON API S: Needs Code Review V: 0.10.x

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@beauby @joaomdmoura @NullVoxPopuli @bf4