diff options
| -rw-r--r-- | actionpack/test/controller/filters_test.rb | 20 | ||||
| -rw-r--r-- | actionview/test/template/tag_helper_test.rb | 5 | ||||
| -rw-r--r-- | activemodel/test/cases/callbacks_test.rb | 19 | ||||
| -rw-r--r-- | activemodel/test/cases/validations/callbacks_test.rb | 15 | ||||
| -rw-r--r-- | activesupport/lib/active_support/callbacks.rb | 9 | ||||
| -rw-r--r-- | activesupport/test/callbacks_test.rb | 48 | ||||
| -rw-r--r-- | guides/source/constant_autoloading_and_reloading.md | 22 | ||||
| -rw-r--r-- | railties/lib/rails/tasks/statistics.rake | 6 |
8 files changed, 86 insertions, 58 deletions
diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index 38533dbf23..829729eb1b 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -504,7 +504,6 @@ class FilterTest < ActionController::TestCase def non_yielding_action @filters << "it didn't yield" - @filter_return_value end def action_three @@ -528,32 +527,15 @@ class FilterTest < ActionController::TestCase end end - def test_non_yielding_around_actions_not_returning_false_do_not_raise + def test_non_yielding_around_actions_do_not_raise controller = NonYieldingAroundFilterController.new - controller.instance_variable_set "@filter_return_value", true assert_nothing_raised do test_process(controller, "index") end end - def test_non_yielding_around_actions_returning_false_do_not_raise - controller = NonYieldingAroundFilterController.new - controller.instance_variable_set "@filter_return_value", false - assert_nothing_raised do - test_process(controller, "index") - end - end - - def test_after_actions_are_not_run_if_around_action_returns_false - controller = NonYieldingAroundFilterController.new - controller.instance_variable_set "@filter_return_value", false - test_process(controller, "index") - assert_equal ["filter_one", "it didn't yield"], controller.assigns['filters'] - end - def test_after_actions_are_not_run_if_around_action_does_not_yield controller = NonYieldingAroundFilterController.new - controller.instance_variable_set "@filter_return_value", true test_process(controller, "index") assert_equal ["filter_one", "it didn't yield"], controller.assigns['filters'] end diff --git a/actionview/test/template/tag_helper_test.rb b/actionview/test/template/tag_helper_test.rb index 7fc798dec9..d037447567 100644 --- a/actionview/test/template/tag_helper_test.rb +++ b/actionview/test/template/tag_helper_test.rb @@ -50,6 +50,11 @@ class TagHelperTest < ActionView::TestCase assert_dom_equal "<div>Hello world!</div>", buffer end + def test_content_tag_with_block_in_erb_containing_non_displayed_erb + buffer = render_erb("<%= content_tag(:p) do %><% 1 %><% end %>") + assert_dom_equal "<p></p>", buffer + end + def test_content_tag_with_block_and_options_in_erb buffer = render_erb("<%= content_tag(:div, :class => 'green') do %>Hello world!<% end %>") assert_dom_equal %(<div class="green">Hello world!</div>), buffer diff --git a/activemodel/test/cases/callbacks_test.rb b/activemodel/test/cases/callbacks_test.rb index 5fede098d1..2ac681b8d8 100644 --- a/activemodel/test/cases/callbacks_test.rb +++ b/activemodel/test/cases/callbacks_test.rb @@ -7,6 +7,7 @@ class CallbacksTest < ActiveModel::TestCase model.callbacks << :before_around_create yield model.callbacks << :after_around_create + false end end @@ -24,16 +25,20 @@ class CallbacksTest < ActiveModel::TestCase after_create do |model| model.callbacks << :after_create + false end after_create "@callbacks << :final_callback" - def initialize(valid=true) - @callbacks, @valid = [], valid + def initialize(options = {}) + @callbacks = [] + @valid = options[:valid] + @before_create_returns = options[:before_create_returns] end def before_create @callbacks << :before_create + @before_create_returns end def create @@ -51,14 +56,20 @@ class CallbacksTest < ActiveModel::TestCase :after_around_create, :after_create, :final_callback] end - test "after callbacks are always appended" do + test "the callback chain is not halted when around or after callbacks return false" do model = ModelCallbacks.new model.create assert_equal model.callbacks.last, :final_callback end + test "the callback chain is halted when a before callback returns false" do + model = ModelCallbacks.new(before_create_returns: false) + model.create + assert_equal model.callbacks.last, :before_create + end + test "after callbacks are not executed if the block returns false" do - model = ModelCallbacks.new(false) + model = ModelCallbacks.new(valid: false) model.create assert_equal model.callbacks, [ :before_create, :before_around_create, :create, :after_around_create] diff --git a/activemodel/test/cases/validations/callbacks_test.rb b/activemodel/test/cases/validations/callbacks_test.rb index 6cd0f4ed4d..5d6d48b824 100644 --- a/activemodel/test/cases/validations/callbacks_test.rb +++ b/activemodel/test/cases/validations/callbacks_test.rb @@ -30,11 +30,16 @@ class DogWithTwoValidators < Dog before_validation { self.history << 'before_validation_marker2' } end -class DogValidatorReturningFalse < Dog +class DogBeforeValidatorReturningFalse < Dog before_validation { false } before_validation { self.history << 'before_validation_marker2' } end +class DogAfterValidatorReturningFalse < Dog + after_validation { false } + after_validation { self.history << 'after_validation_marker' } +end + class DogWithMissingName < Dog before_validation { self.history << 'before_validation_marker' } validates_presence_of :name @@ -82,12 +87,18 @@ class CallbacksWithMethodNamesShouldBeCalled < ActiveModel::TestCase end def test_further_callbacks_should_not_be_called_if_before_validation_returns_false - d = DogValidatorReturningFalse.new + d = DogBeforeValidatorReturningFalse.new output = d.valid? assert_equal [], d.history assert_equal false, output end + def test_further_callbacks_should_be_called_if_after_validation_returns_false + d = DogAfterValidatorReturningFalse.new + d.valid? + assert_equal ['after_validation_marker'], d.history + end + def test_validation_test_should_be_done d = DogWithMissingName.new output = d.valid? diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index 24c702b602..95dbc9a0cb 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -659,16 +659,17 @@ module ActiveSupport # ===== Options # # * <tt>:terminator</tt> - Determines when a before filter will halt the - # callback chain, preventing following callbacks from being called and - # the event from being triggered. This should be a lambda to be executed. + # callback chain, preventing following before and around callbacks from + # being called and the event from being triggered. + # This should be a lambda to be executed. # The current object and the return result of the callback will be called # with the lambda. # # define_callbacks :validate, terminator: ->(target, result) { result == false } # # In this example, if any before validate callbacks returns +false+, - # other callbacks are not executed. Defaults to +false+, meaning no value - # halts the chain. + # any successive before and around callback is not executed. + # Defaults to +false+, meaning no value halts the chain. # # * <tt>:skip_after_callbacks_if_terminated</tt> - Determines if after # callbacks should be terminated by the <tt>:terminator</tt> option. By diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb index 32c2dfdfc0..d19e5fd6e7 100644 --- a/activesupport/test/callbacks_test.rb +++ b/activesupport/test/callbacks_test.rb @@ -49,7 +49,7 @@ module CallbacksTest def self.before(model) model.history << [:before_save, :class] end - + def self.after(model) model.history << [:after_save, :class] end @@ -501,21 +501,20 @@ module CallbacksTest end end - class CallbackTerminator + class AbstractCallbackTerminator include ActiveSupport::Callbacks - define_callbacks :save, :terminator => ->(_,result) { result == :halt } - - set_callback :save, :before, :first - set_callback :save, :before, :second - set_callback :save, :around, :around_it - set_callback :save, :before, :third - set_callback :save, :after, :first - set_callback :save, :around, :around_it - set_callback :save, :after, :second - set_callback :save, :around, :around_it - set_callback :save, :after, :third - + def self.set_save_callbacks + set_callback :save, :before, :first + set_callback :save, :before, :second + set_callback :save, :around, :around_it + set_callback :save, :before, :third + set_callback :save, :after, :first + set_callback :save, :around, :around_it + set_callback :save, :after, :second + set_callback :save, :around, :around_it + set_callback :save, :after, :third + end attr_reader :history, :saved, :halted def initialize @@ -552,6 +551,17 @@ module CallbacksTest end end + class CallbackTerminator < AbstractCallbackTerminator + define_callbacks :save, terminator: ->(_,result) { result == :halt } + set_save_callbacks + end + + class CallbackTerminatorSkippingAfterCallbacks < AbstractCallbackTerminator + define_callbacks :save, terminator: ->(_,result) { result == :halt }, + skip_after_callbacks_if_terminated: true + set_save_callbacks + end + class CallbackObject def before(caller) caller.record << "before" @@ -688,7 +698,7 @@ module CallbacksTest end class CallbackTerminatorTest < ActiveSupport::TestCase - def test_termination + def test_termination_skips_following_before_and_around_callbacks terminator = CallbackTerminator.new terminator.save assert_equal ["first", "second", "third", "second", "first"], terminator.history @@ -707,6 +717,14 @@ module CallbacksTest end end + class CallbackTerminatorSkippingAfterCallbacksTest < ActiveSupport::TestCase + def test_termination_skips_after_callbacks + terminator = CallbackTerminatorSkippingAfterCallbacks.new + terminator.save + assert_equal ["first", "second"], terminator.history + end + end + class HyphenatedKeyTest < ActiveSupport::TestCase def test_save obj = HyphenatedCallbacks.new diff --git a/guides/source/constant_autoloading_and_reloading.md b/guides/source/constant_autoloading_and_reloading.md index 94546c7bb2..5f435ddbd8 100644 --- a/guides/source/constant_autoloading_and_reloading.md +++ b/guides/source/constant_autoloading_and_reloading.md @@ -580,8 +580,8 @@ file is loaded. If the file actually defines `Post` all is fine, otherwise ### Qualified References When a qualified constant is missing Rails does not look for it in the parent -namespaces. But there's a caveat: unfortunately, when a constant is missing -Rails is not able to say if the trigger was a relative or qualified reference. +namespaces. But there is a caveat: When a constant is missing, Rails is +unable to tell if the trigger was a relative reference or a qualified one. For example, consider @@ -627,17 +627,17 @@ been triggered in the first place. Thus, Rails assumes a qualified reference and considers the file `admin/user.rb` and directory `admin/user` to be the only valid options. -In practice this works quite well as long as the nesting matches all parent +In practice, this works quite well as long as the nesting matches all parent namespaces respectively and the constants that make the rule apply are known at that time. -But since autoloading happens on demand, if the top-level `User` by chance was -not yet loaded then Rails has no way to know whether `Admin::User` should load it -or raise `NameError`. +However, autoloading happens on demand. If by chance the top-level `User` was +not yet loaded, then Rails has no way to know whether `Admin::User` should load +it or raise `NameError`. -These kind of name conflicts are rare in practice but, in case there's one, -`require_dependency` provides a solution by making sure the constant needed to -trigger the heuristic is defined in the conflicting place. +Naming conflicts of this kind are rare in practice, but if one occurs, +`require_dependency` provides a solution by ensuring that the constant needed +to trigger the heuristic is defined in the conflicting place. ### Automatic Modules @@ -949,8 +949,8 @@ end require_dependency ‘square’ ``` -Only the leaves that are **at least grandchildren** have to be loaded that -way. Direct subclasses do not need to be preloaded and, if the hierarchy is +Only the leaves that are **at least grandchildren** need to be loaded this +way. Direct subclasses do not need to be preloaded. If the hierarchy is deeper, intermediate classes will be autoloaded recursively from the bottom because their constant will appear in the class definitions as superclass. diff --git a/railties/lib/rails/tasks/statistics.rake b/railties/lib/rails/tasks/statistics.rake index 4f09ded31d..ba6168e208 100644 --- a/railties/lib/rails/tasks/statistics.rake +++ b/railties/lib/rails/tasks/statistics.rake @@ -1,6 +1,6 @@ -# while having global constant is not good, -# many 3rd party tools depend on it, like rspec-rails, cucumber-rails, etc -# so if will be removed - deprecation warning is needed +# While global constants are bad, many 3rd party tools depend on this one (e.g +# rspec-rails & cucumber-rails). So a deprecation warning is needed if we want +# to remove it. STATS_DIRECTORIES = [ %w(Controllers app/controllers), %w(Helpers app/helpers), |