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

Don't try to add jsonapi_serialize as helper unless it's possible to do so #10

Conversation

mecampbellsoup
Copy link
Contributor

@mecampbellsoup mecampbellsoup commented May 9, 2016

Description

From: heartcombo/devise#3732

Currently Devise::Controllers::Helpers depends on #helper_method, which is defined inside AbstractController::Helpers. This module is included in ActionController::Base, but it is not included by default in ActionController::Metal.

helper_method is not essential to the functionality of the methods inside Devise::Controllers::Helpers, so this PR wraps all Devise #helper_method calls with respond_to?(:helper_method) checks.

This console output elucidates the issue:

[14] pry(main)> ActionController::Base.ancestors.map(&:to_s).grep(/Helpers/)
=> ["ActionDispatch::Routing::RouteSet::MountedHelpers", "ActionController::Helpers", "AbstractController::Helpers"]
[15] pry(main)> ApplicationController.ancestors.map(&:to_s).grep(/Helpers/)
=> ["ActionDispatch::Routing::RouteSet::MountedHelpers"]

Background

  • Rails 5.0.0.rc1
  • RAILS_ENV=test (was trying to run a new API's first request specs)

@mecampbellsoup
Copy link
Contributor Author

@tiagopog updated with description

@tiagopog
Copy link
Owner

Good one, @mecampbellsoup 👍

@tiagopog tiagopog merged commit 82abb14 into tiagopog:master May 10, 2016
@mecampbellsoup mecampbellsoup deleted the wrap-helper-method-check-in-included-macro branch May 10, 2016 11:57
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.

2 participants