aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--actionpack/lib/action_dispatch/http/mime_negotiation.rb3
-rw-r--r--actionpack/test/controller/filters_test.rb20
-rw-r--r--actionpack/test/dispatch/request_test.rb7
-rw-r--r--actionview/test/template/tag_helper_test.rb5
-rw-r--r--activemodel/test/cases/callbacks_test.rb19
-rw-r--r--activemodel/test/cases/validations/callbacks_test.rb15
-rw-r--r--activesupport/lib/active_support/callbacks.rb9
-rw-r--r--activesupport/test/callbacks_test.rb48
-rw-r--r--guides/source/4_2_release_notes.md27
-rw-r--r--guides/source/constant_autoloading_and_reloading.md27
-rw-r--r--railties/lib/rails/tasks/statistics.rake6
11 files changed, 111 insertions, 75 deletions
diff --git a/actionpack/lib/action_dispatch/http/mime_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb
index 9c8f65deac..53a98c5d0a 100644
--- a/actionpack/lib/action_dispatch/http/mime_negotiation.rb
+++ b/actionpack/lib/action_dispatch/http/mime_negotiation.rb
@@ -72,11 +72,12 @@ module ActionDispatch
end
end
end
+
# Sets the \variant for template.
def variant=(variant)
if variant.is_a?(Symbol)
@variant = [variant]
- elsif variant.is_a?(Array) && variant.any? && variant.all?{ |v| v.is_a?(Symbol) }
+ elsif variant.nil? || variant.is_a?(Array) && variant.any? && variant.all?{ |v| v.is_a?(Symbol) }
@variant = variant
else
raise ArgumentError, "request.variant must be set to a Symbol or an Array of Symbols, not a #{variant.class}. " \
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/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb
index beb9085abe..ee8e915610 100644
--- a/actionpack/test/dispatch/request_test.rb
+++ b/actionpack/test/dispatch/request_test.rb
@@ -1143,6 +1143,13 @@ class RequestVariant < BaseRequestTest
end
end
+ test "reset variant" do
+ request = stub_request
+
+ request.variant = nil
+ assert_equal nil, request.variant
+ end
+
test "setting variant with non symbol value" do
request = stub_request
assert_raise ArgumentError do
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/4_2_release_notes.md b/guides/source/4_2_release_notes.md
index 1e9107a8c6..d8700539c4 100644
--- a/guides/source/4_2_release_notes.md
+++ b/guides/source/4_2_release_notes.md
@@ -509,6 +509,17 @@ Please refer to the [Changelog][action-pack] for detailed changes.
serving assets from your Rails server in production.
([Pull Request](https://github.com/rails/rails/pull/16466))
+* When calling the `process` helpers in an integration test the path needs to have
+ a leading slash. Previously you could omit it but that was a byproduct of the
+ implementation and not an intentional feature, e.g.:
+
+ ```ruby
+ test "list all posts" do
+ get "/posts"
+ assert_response :success
+ end
+ ```
+
Action View
-----------
@@ -544,17 +555,6 @@ Please refer to the [Changelog][action-view] for detailed changes.
* Placeholder I18n follows the same convention as `label` I18n.
([Pull Request](https://github.com/rails/rails/pull/16438))
-* When calling the `process` helpers in an integration test the path needs to have
- a leading slash. Previously you could omit it but that was a byproduct of the
- implementation and not an intentional feature, e.g.:
-
- ```ruby
-   test "list all posts" do
-     get "/posts"
-     assert_response :success 
- end
- ```
-
Action Mailer
-------------
@@ -640,6 +640,10 @@ Please refer to the [Changelog][active-record] for detailed changes.
`Relation` for performing queries and updates is the preferred API.
([Commit](https://github.com/rails/rails/commit/d5902c9e))
+* Deprecated `add_timestamps` and `t.timestamps` without passing the `:null`
+ option. The default of `null: true` will change in Rails 5 to `null: false`.
+ ([Pull Request](https://github.com/rails/rails/pull/16481))
+
* Deprecated `Reflection#source_macro` without replacement as it is no longer
needed in Active Record.
([Pull Request](https://github.com/rails/rails/pull/16373))
@@ -824,6 +828,7 @@ Please refer to the [Changelog][active-support] for detailed changes.
`module Foo; extend ActiveSupport::Concern; end` boilerplate.
([Commit](https://github.com/rails/rails/commit/b16c36e688970df2f96f793a759365b248b582ad))
+* New [guide](constant_autoloading_and_reloading.html) about constant autoloading and reloading.
Credits
-------
diff --git a/guides/source/constant_autoloading_and_reloading.md b/guides/source/constant_autoloading_and_reloading.md
index ac4ceb6023..5f435ddbd8 100644
--- a/guides/source/constant_autoloading_and_reloading.md
+++ b/guides/source/constant_autoloading_and_reloading.md
@@ -6,15 +6,10 @@ This guide documents how constant autoloading and reloading works.
After reading this guide, you will know:
* Key aspects of Ruby constants
-
* What is `autoload_paths`
-
* How constant autoloading works
-
* What is `require_dependency`
-
* How constant reloading works
-
* Solutions to common autoloading gotchas
--------------------------------------------------------------------------------
@@ -585,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
@@ -632,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
@@ -954,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),
generated by cgit v1.2.3 (git 2.25.1) at 2025-04-26 10:34:04 +0000