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
Call block to #redirect_to in controller context #33735
Call block to #redirect_to in controller context #33735
Conversation
Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @pixeltrix (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. This repository is being automatically checked for code quality issues using Code Climate. You can see results for this analysis in the PR status below. Newly introduced issues should be fixed before a Pull Request is considered ready to review. Please see the contribution instructions for more information. |
The documentation for ActionController::Redirecting states that a Proc argument "will be executed in the controller's context." However, unless #instance_eval is used (removed in 6b3ad0c), that statement is false for procs defined outside of the controller instance. This commit restores the documented behavior. Fixes rails#33731.
5e50a81
to
071d926
Compare
@@ -119,6 +119,15 @@ def redirect_to_with_block_and_options | |||
redirect_to proc { { action: "hello_world" } } | |||
end | |||
|
|||
def redirect_to_out_of_scope_block | |||
redirect_to self.class.class_eval(<<~END) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe stead this class_eval
we could define this proc as a constant in Workshop?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally wrote it as a constant (in the RedirectController class), but I thought it was easier to understand the test, especially with the raise
, if the proc wasn't ~100 lines away.
I'll add that change, though. I can see how the significance of the class_eval
might not be immediately apparent.
Per @rafaelfranca's suggestion.
* Call block to #redirect_to in controller context The documentation for ActionController::Redirecting states that a Proc argument "will be executed in the controller's context." However, unless #instance_eval is used (removed in 6b3ad0c), that statement is false for procs defined outside of the controller instance. This commit restores the documented behavior. Fixes #33731. * Move test proc into a constant in another class Per @rafaelfranca's suggestion. [Steven Peckins + Rafael Mendonça França]
The documentation for ActionController::Redirecting states that a Proc
argument "will be executed in the controller's context." However,
unless #instance_eval is used (removed in 6b3ad0c), that statement is
false for procs defined outside of the controller instance.
This commit restores the documented behavior.
Fixes 33731.