Skip to content
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.

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

[WIP] Allow users to opt-out from the ActionController extensions #1636

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bf4
Copy link
Member

@bf4 bf4 commented Mar 30, 2016

Purpose

  • Make it possible to require active_model_serializers without mixing ActionController::Serialization into the controller

Changes

Add ActionController::Serialization.enabled = false to an initializer and it won't be mixed in.

Caveats

This doesn't exactly solve #1500 since it doesn't include any AMS code in the controller, and passing serializer or each_serializer won't work

Perhaps config should come from Rails, rather than be set on the mixin. e.g.

Rails.configuration.action_controller.render_json_with_active_model_serializers = false

Related GitHub issues

And many more

Additional helpful information

cc @trek

@bf4
Copy link
Member Author

bf4 commented Mar 30, 2016

I marked this as WIP since I'd like to think over the implementation a bit and this also needs tests.

@NullVoxPopuli
Copy link
Contributor

I like the idea of setting the option in the rails config.

class << self
attr_accessor :enabled
end
self.enabled = true
Copy link
Member Author

Choose a reason for hiding this comment

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

(pardon the awful pseudo code)

I was actually just thinking about having

  • a configurable default 'ams_ify_controller = true`
  • and then in the controller, by default we only mix in a 'shim' module that
    • has config methods, e.g. ams_ify_controller
    • so that by default ActionController::Serialization would be included in action controller
    • but if the default were set to false, then individual controllers would need to opt in by setting ams_ify_controller true .
    • the method would just extend the controller with ActionController::Serialization when true

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.

None yet

2 participants