Allow overriding the adapter with render option #718
Conversation
|
Can you elaborate on why you think this change is important? |
|
I need a method to use multiple adapters in an app. Version 2 on my API uses the json adapter and version 3 will use json_api. I didn't see an easier way to do that. Or were you referring to the 'default_adapter' bit? It just made it seem less constant, I can take that part out easily enough. |
ggordon
force-pushed
the
adapter_override
branch
2 times, most recently
from
efb02d1
to
e61bc08
Compare
|
Summoning @steveklabnik. |
|
Conceptually, this seems legit. I don't mind letting people override adapters in the controller like this. |
|
Agree with @steveklabnik, sounds legit in my opinion |
|
@ggordon could you fix the broken tests please? |
ggordon
force-pushed
the
adapter_override
branch
from
e61bc08
to
768224c
Compare
|
@kurko Fixed and rebased. |
| yield | ||
| ensure | ||
| ActiveModel::Serializer.config.adapter = old_adapter | ||
| end |
There was a problem hiding this comment.
@ggordon you're testing the adapter: :json_api option everywhere. We want to still not need to put it, but use a default adapter and override only where need.
Could you leave some tests where JSONAPI is the default adapter and you don't pass the adapter: :json_api? We need to assert that :json_api as default adapter will work.
|
@kurko No problem. |
Make it easy to use multiple adapters in an app. use "adapter: false" to not use ams make a test override config.adapter
ggordon
force-pushed
the
adapter_override
branch
from
768224c
to
5560b49
Compare
|
It's not obvious from the commit, but serialization_test.rb still includes a test that sets config.adapter to :json_api instead of the adapter option. |
Allow overriding the adapter with render option
|
@kurko Thanks for merging! |
Renamed config.adapter to config.default_adapter
Undid the rename, this request is just for the adapter option to render now.