Don't pluralize the CollectionSerializer#root for #json_key #1418
Conversation
Empact
force-pushed
the
collection-pluralize
branch
from
61d1d9f to
047f11f
Compare
| @@ -171,7 +171,7 @@ def test_render_array_using_custom_root | |||
| with_adapter :json do | |||
| get :render_array_using_custom_root | |||
| end | |||
| expected = { custom_roots: [{ name: 'Name 1', description: 'Description 1' }] } | |||
| expected = { custom_root: [{ name: 'Name 1', description: 'Description 1' }] } | |||
There was a problem hiding this comment.
Technically this is a breaking change. I think this is ok since we still haven't released 0.10 but a note in the changelog would be good
There was a problem hiding this comment.
I can see this breaking a lot of apps.. I'm wondering how we can communicate that? @rails-api/ams
There was a problem hiding this comment.
It's in the changelog under breaking changes so I think it's good now.
|
You wrote:
Would you mind pointing me to where the JSON adapter collection root is expected to be singular? Indeed for JSON API this is configurable, but not for the JSON adapter. |
Empact
force-pushed
the
collection-pluralize
branch
2 times, most recently
from
f2c3475 to
11f3d68
Compare
|
@bf4 I wrote
the latter 2 are expected to be singular (unless overriden), because they are drawn from the classes that are not collection-oriented - either the >> Profile.limit(10).name
=> "Profile"I'm not sure what you mean by |
Empact
force-pushed
the
collection-pluralize
branch
from
11f3d68 to
1989498
Compare
| singular_key = serializers.first.try(:json_key) || | ||
| object.try(:name).try(:underscore) | ||
| singular_key.try(:pluralize) | ||
| end |
There was a problem hiding this comment.
what a mess this code is. maybe have this be something like root || derived_root so it's a little easier to see what's going on? I initially missed that the fundamental change was no longer modifying root when present
|
Looks good. Just some small comments |
One of three constituents is used to provide the CollectionSerializer's #json_key: 1) the :root option - controlled by the caller 2) the #name of the first resource serializer - the root or underscored model name 3) the underscored #name of the resources object - generally equivalent to the underscored model name of rails-api#2 Of the three, only the latter 2 are out of the callers control, and only the latter two are expected to be singular by default. Not pluralizing the root gives the caller additional flexibility in defining the desired root, whether conventionally plural, unconventionally plural (e.g. objects_received:) or singular.
Empact
force-pushed
the
collection-pluralize
branch
from
1989498 to
251e33a
Compare
|
@bf4 Should be good to go |
|
@bf4, I know you said this could break a lot of apps. Do you think it's worth creating a setting for it, or just let it be part of upgrading to 0.10? |
|
@natematykiewicz good question. @joaomdmoura how about after we merge this we immediately cut rc4? |
|
Incidentally the other changes I've been putting together so far have been additive only. I think an RC to socialize this would be good, and I'm open to alternatives. |
|
LGTM. For a future PR, it might be worth extracting most of the |
|
Interesting - incidentally I also have a branch adding the https://github.com/brigade/active_model_serializers/tree/attributes-except |
Don't pluralize the CollectionSerializer#root for #json_key
One of three constituents is used to provide the CollectionSerializer's #json_key:
underscored model name
equivalent to the underscored model name of Remove Rails Dependency #2
Of the three, only the latter 2 are out of the callers control, and
only the latter two are expected to be singular by default. Not
pluralizing the root gives the caller additional flexibility in
defining the desired root, whether conventionally plural,
unconventionally plural (e.g. objects_received:) or singular.