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

Wrap helper_method calls in respond_to?(:helper_method) #3732

Merged

Conversation

posgarou
Copy link
Contributor

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.


test 'defines methods like current_user' do
assert @controller.respond_to?(:current_user)
end

Choose a reason for hiding this comment

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

Please use the builtin methods: assert_includes, assert_respond_to and refute_respond_to. They provide nicer error messages in case of test failures.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, thank you.

@betesh
Copy link
Contributor

betesh commented Aug 31, 2015

+1

@ballPointPenguin
Copy link

This seems like a good solution, rather than adding mock support for this method in Rails 5 api code.

@posgarou
Copy link
Contributor Author

Any updates on this PR? Thanks!

@lucasmazza
Copy link
Contributor

Sorry, for the delay, thanks for the fix ❤️

lucasmazza added a commit that referenced this pull request Sep 26, 2015
Wrap helper_method calls in respond_to?(:helper_method)
@lucasmazza lucasmazza merged commit 7df57d5 into heartcombo:master Sep 26, 2015
@posgarou
Copy link
Contributor Author

Thanks!

On Sat, Sep 26, 2015, 10:05 AM Lucas Mazza [email protected] wrote:

Merged #3732 #3732.


Reply to this email directly or view it on GitHub
#3732 (comment).

djsegal added a commit to djsegal/devise that referenced this pull request Dec 18, 2015
lucasmazza added a commit that referenced this pull request Dec 18, 2015
Add #3732 helper logic to devise controller
Envek added a commit to Envek/monban that referenced this pull request May 9, 2016
Currently `Monban::ControllerHelpers` 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`. So it's not available in Ruby on Rails 5.0 applications in API mode (rails-api).

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

See rails/rails#21067 and heartcombo/devise#3732 for reference.
halogenandtoast pushed a commit to halogenandtoast/oath that referenced this pull request May 24, 2016
Currently `Monban::ControllerHelpers` 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`. So it's not available in Ruby on Rails 5.0 applications in API mode (rails-api).

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

See rails/rails#21067 and heartcombo/devise#3732 for reference.
jcoyne added a commit to projectblacklight/blacklight that referenced this pull request Jun 4, 2018
In API mode, the view layer is not available and these trigger
method_missing errors. This fix is similar to how devise solved this
problem: heartcombo/devise#3732
jcoyne added a commit to projectblacklight/blacklight that referenced this pull request Jun 4, 2018
In API mode, the view layer is not available and these trigger
method_missing errors. This fix is similar to how devise solved this
problem: heartcombo/devise#3732
cbeer pushed a commit to projectblacklight/blacklight that referenced this pull request Nov 7, 2018
In API mode, the view layer is not available and these trigger
method_missing errors. This fix is similar to how devise solved this
problem: heartcombo/devise#3732
cbeer pushed a commit to projectblacklight/blacklight that referenced this pull request Nov 8, 2018
In API mode, the view layer is not available and these trigger
method_missing errors. This fix is similar to how devise solved this
problem: heartcombo/devise#3732
cbeer pushed a commit to projectblacklight/blacklight that referenced this pull request Nov 8, 2018
In API mode, the view layer is not available and these trigger
method_missing errors. This fix is similar to how devise solved this
problem: heartcombo/devise#3732
cbeer pushed a commit to projectblacklight/blacklight that referenced this pull request Dec 15, 2018
In API mode, the view layer is not available and these trigger
method_missing errors. This fix is similar to how devise solved this
problem: heartcombo/devise#3732
cbeer pushed a commit to projectblacklight/blacklight that referenced this pull request Dec 17, 2018
In API mode, the view layer is not available and these trigger
method_missing errors. This fix is similar to how devise solved this
problem: heartcombo/devise#3732
cbeer pushed a commit to projectblacklight/blacklight that referenced this pull request Dec 17, 2018
In API mode, the view layer is not available and these trigger
method_missing errors. This fix is similar to how devise solved this
problem: heartcombo/devise#3732
cbeer pushed a commit to projectblacklight/blacklight that referenced this pull request Dec 17, 2018
In API mode, the view layer is not available and these trigger
method_missing errors. This fix is similar to how devise solved this
problem: heartcombo/devise#3732
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants