Skip to content

Conversation

@jimbeaudoin
Copy link

Purpose

Add more documentation! :)

Changes

Add documentation for root in docs/general/serializers.md

```ruby
def root
"posts"
end
Copy link
Member

Choose a reason for hiding this comment

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

@jimbeaudoin Thanks for this! mind taking a look at #1782 and see what you think? cc @beauby

Copy link
Author

Choose a reason for hiding this comment

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

@bf4 I don't know about moving root at the adapter level. I think it's fine
here but it's my opinion.
Ex: if I have a model Api::V1:Post the root object name of the payload is
going to be api/v1/posts not posts. This is why I used this settings in
the serializer.
But this is the first time I separate my models like that (Api::V1::x). I
don't know if it's a good practice or not. This is a new experience on a
new API in development.

What you can do is merge this small documentation block to master because
this is how you can update root right now and it can help others on
v0.10.x.
And then update the doc in PR #1782 if she got merged. Like you, I think
it's going to need a deprecation warning before removing the old code.

On Thu, Jun 9, 2016 at 8:48 PM, Benjamin Fleischer <notifications@github.com

wrote:

In docs/general/serializers.md
#1794 (comment)
:

@@ -217,7 +217,15 @@ The object being serialized.

#root

-PR please :)
+Allows you to change the root object name of the payload.
+
+e.g.
+
+```ruby
+def root

  • "posts"
    +end

@jimbeaudoin https://github.com/jimbeaudoin Thanks for this! mind
taking a look at #1782
#1782 and see
what you think? cc @beauby https://github.com/beauby


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rails-api/active_model_serializers/pull/1794/files/8c00f3abd269d1116ace4ffb6c9b824eaa80de30#r66546734,
or mute the thread
https://github.com/notifications/unsubscribe/AG8xmBkgNaXRncHPB1pKkN-gZeUwBy68ks5qKLRMgaJpZM4IyU0-
.

Jimmy Beaudoin
email@jimmy-beaudoin.com

Copy link
Member

Choose a reason for hiding this comment

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

@jimbeaudoin well, the thing is that root at this time is an options that is passed into the serializer. The method you would want to override is actually json_key or even set the type. (That's why I referenced the other pr which describes some of this in the diff). I'd certainly merge this change as an option to setting the resource root, but am hesitant for the documentation to advocate an unintended usage...

Copy link
Author

Choose a reason for hiding this comment

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

@bf4 Ok I see, this PR can be closed then. json_key to the rescue! Thanks! :)

@jgrau
Copy link
Contributor

jgrau commented Aug 31, 2016

The submitter is fine with closing this one so maybe we should do that(?)

@bf4
Copy link
Member

bf4 commented Aug 31, 2016

Sure. It's really the wrong change. Thanks @jgrau

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants