aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--actionpack/lib/action_dispatch/routing/mapper.rb13
-rw-r--r--actionpack/lib/action_dispatch/routing/route_set.rb13
-rw-r--r--actionpack/test/abstract_unit.rb4
-rw-r--r--actionview/CHANGELOG.md8
-rw-r--r--actionview/lib/action_view/view_paths.rb41
-rw-r--r--actionview/test/actionpack/abstract/abstract_controller_test.rb48
-rw-r--r--actionview/test/template/sanitize_helper_test.rb2
-rw-r--r--activerecord/CHANGELOG.md7
-rw-r--r--activerecord/lib/active_record/associations/collection_association.rb2
-rw-r--r--activerecord/lib/active_record/associations/has_many_association.rb26
-rw-r--r--activerecord/lib/active_record/associations/has_many_through_association.rb8
-rw-r--r--activerecord/lib/active_record/store.rb19
-rw-r--r--activerecord/test/cases/store_test.rb16
-rw-r--r--activesupport/CHANGELOG.md4
-rw-r--r--activesupport/lib/active_support/callbacks.rb6
15 files changed, 157 insertions, 60 deletions
diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 4c20974ac7..58c7f5330e 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -578,18 +578,17 @@ module ActionDispatch
_route = @set.named_routes.routes[name.to_sym]
_routes = @set
app.routes.define_mounted_helper(name)
- app.routes.singleton_class.class_eval do
- redefine_method :mounted? do
- true
- end
-
- redefine_method :_generate_prefix do |options|
+ app.routes.extend Module.new {
+ def mounted?; true; end
+ define_method :find_script_name do |options|
+ super(options) || begin
prefix_options = options.slice(*_route.segment_keys)
# we must actually delete prefix segment keys to avoid passing them to next url_for
_route.segment_keys.each { |k| options.delete(k) }
_routes.url_helpers.send("#{name}_path", prefix_options)
+ end
end
- end
+ }
end
end
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index f0c0810a3c..fd163a47f4 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -641,8 +641,8 @@ module ActionDispatch
!mounted? && default_url_options.empty?
end
- def _generate_prefix(options = {})
- nil
+ def find_script_name(options)
+ options.delete :script_name
end
# The +options+ argument must be a hash whose keys are *symbols*.
@@ -656,10 +656,10 @@ module ActionDispatch
password = options.delete :password
end
- recall = options.delete(:_recall)
+ recall = options.delete(:_recall) { {} }
- original_script_name = options.delete(:original_script_name).presence
- script_name = options.delete(:script_name).presence || _generate_prefix(options)
+ original_script_name = options.delete(:original_script_name)
+ script_name = find_script_name options
if script_name && original_script_name
script_name = original_script_name + script_name
@@ -667,9 +667,8 @@ module ActionDispatch
path_options = options.dup
RESERVED_OPTIONS.each { |ro| path_options.delete ro }
- path_options = yield(path_options) if block_given?
- path, params = generate(path_options, recall || {})
+ path, params = generate(path_options, recall)
params.merge!(options[:params] || {})
ActionDispatch::Http::URL.url_for(options.merge!({
diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb
index 03a4741f42..46de36317e 100644
--- a/actionpack/test/abstract_unit.rb
+++ b/actionpack/test/abstract_unit.rb
@@ -320,8 +320,8 @@ module ActionDispatch
end
module RoutingTestHelpers
- def url_for(set, options, recall = nil)
- set.send(:url_for, options.merge(:only_path => true, :_recall => recall))
+ def url_for(set, options, recall = {})
+ set.url_for options.merge(:only_path => true, :_recall => recall)
end
end
diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md
index a6f6ac78db..e39fa68b26 100644
--- a/actionview/CHANGELOG.md
+++ b/actionview/CHANGELOG.md
@@ -1,4 +1,10 @@
-* Take label values into account when doing I18n lookups for model attributes.
+* Deprecate `AbstractController::Base.parent_prefixes`.
+ Override `AbstractController::Base.local_prefixes` when you want to change
+ where to find views.
+
+ *Nick Sutterer*
+
+* Take label values into account when doing I18n lookups for model attributes.
The following:
diff --git a/actionview/lib/action_view/view_paths.rb b/actionview/lib/action_view/view_paths.rb
index 6c349feb1d..80a41f2418 100644
--- a/actionview/lib/action_view/view_paths.rb
+++ b/actionview/lib/action_view/view_paths.rb
@@ -14,27 +14,38 @@ module ActionView
:locale, :locale=, :to => :lookup_context
module ClassMethods
- def parent_prefixes
- @parent_prefixes ||= begin
- parent_controller = superclass
- prefixes = []
-
- until parent_controller.abstract?
- prefixes << parent_controller.controller_path
- parent_controller = parent_controller.superclass
- end
+ def _prefixes # :nodoc:
+ @_prefixes ||= begin
+ deprecated_prefixes = handle_deprecated_parent_prefixes
+ if deprecated_prefixes
+ deprecated_prefixes
+ else
+ return local_prefixes if superclass.abstract?
- prefixes
+ local_prefixes + superclass._prefixes
+ end
end
end
+
+ private
+
+ # Override this method in your controller if you want to change paths prefixes for finding views.
+ # Prefixes defined here will still be added to parents' <tt>._prefixes</tt>.
+ def local_prefixes
+ [controller_path]
+ end
+
+ def handle_deprecated_parent_prefixes # TODO: remove in 4.3/5.0.
+ return unless respond_to?(:parent_prefixes)
+
+ ActiveSupport::Deprecation.warn "Overriding ActionController::Base::parent_prefixes is deprecated, override .local_prefixes instead."
+ local_prefixes + parent_prefixes
+ end
end
# The prefixes used in render "foo" shortcuts.
- def _prefixes
- @_prefixes ||= begin
- parent_prefixes = self.class.parent_prefixes
- parent_prefixes.dup.unshift(controller_path)
- end
+ def _prefixes # :nodoc:
+ self.class._prefixes
end
# LookupContext is the object responsible to hold all information required to lookup
diff --git a/actionview/test/actionpack/abstract/abstract_controller_test.rb b/actionview/test/actionpack/abstract/abstract_controller_test.rb
index 40d3b17131..e653b12d32 100644
--- a/actionview/test/actionpack/abstract/abstract_controller_test.rb
+++ b/actionview/test/actionpack/abstract/abstract_controller_test.rb
@@ -150,6 +150,54 @@ module AbstractController
end
end
+ class OverridingLocalPrefixes < AbstractController::Base
+ include AbstractController::Rendering
+ include ActionView::Rendering
+ append_view_path File.expand_path(File.join(File.dirname(__FILE__), "views"))
+
+ def index
+ render
+ end
+
+ def self.local_prefixes
+ # this would usually return "abstract_controller/testing/overriding_local_prefixes"
+ super + ["abstract_controller/testing/me3"]
+ end
+
+ class Inheriting < self
+ end
+ end
+
+ class OverridingLocalPrefixesTest < ActiveSupport::TestCase # TODO: remove me in 5.0/4.3.
+ test "overriding .local_prefixes adds prefix" do
+ @controller = OverridingLocalPrefixes.new
+ @controller.process(:index)
+ assert_equal "Hello from me3/index.erb", @controller.response_body
+ end
+
+ test ".local_prefixes is inherited" do
+ @controller = OverridingLocalPrefixes::Inheriting.new
+ @controller.process(:index)
+ assert_equal "Hello from me3/index.erb", @controller.response_body
+ end
+ end
+
+ class DeprecatedParentPrefixes < OverridingLocalPrefixes
+ def self.parent_prefixes
+ ["abstract_controller/testing/me3"]
+ end
+ end
+
+ class DeprecatedParentPrefixesTest < ActiveSupport::TestCase # TODO: remove me in 5.0/4.3.
+ test "overriding .parent_prefixes is deprecated" do
+ @controller = DeprecatedParentPrefixes.new
+ assert_deprecated do
+ @controller.process(:index)
+ end
+ assert_equal "Hello from me3/index.erb", @controller.response_body
+ end
+ end
+
# Test rendering with layouts
# ====
# self._layout is used when defined
diff --git a/actionview/test/template/sanitize_helper_test.rb b/actionview/test/template/sanitize_helper_test.rb
index 12d5260a9d..f7c8f36b78 100644
--- a/actionview/test/template/sanitize_helper_test.rb
+++ b/actionview/test/template/sanitize_helper_test.rb
@@ -1,6 +1,6 @@
require 'abstract_unit'
-# The exhaustive tests are in test/controller/html/sanitizer_test.rb.
+# The exhaustive tests are in test/template/html-scanner/sanitizer_test.rb
# This tests the that the helpers hook up correctly to the sanitizer classes.
class SanitizeHelperTest < ActionView::TestCase
tests ActionView::Helpers::SanitizeHelper
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index f6cd5efb45..6d4ea2b682 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,10 @@
+* Fix `stored_attributes` to correctly merge the details of stored
+ attributes defined in parent classes.
+
+ Fixes #14672.
+
+ *Brad Bennett*, *Jessica Yao*, *Lakshmi Parthasarathy*
+
* `change_column_default` allows `[]` as argument to `change_column_default`.
Fixes #11586.
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index 48628230c7..caf4e612f9 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -194,7 +194,7 @@ module ActiveRecord
options[:dependent]
end
- delete_records(:all, dependent).tap do
+ delete_or_nullify_all_records(dependent).tap do
reset
loaded!
end
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index aac85a36c8..f5e911c739 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -105,23 +105,27 @@ module ActiveRecord
}
end
+ def delete_count(method, scope)
+ if method == :delete_all
+ scope.delete_all
+ else
+ scope.update_all(reflection.foreign_key => nil)
+ end
+ end
+
+ def delete_or_nullify_all_records(method)
+ count = delete_count(method, self.scope)
+ update_counter(-count)
+ end
+
# Deletes the records according to the <tt>:dependent</tt> option.
def delete_records(records, method)
if method == :destroy
records.each(&:destroy!)
update_counter(-records.length) unless inverse_updates_counter_cache?
else
- if records == :all || !reflection.klass.primary_key
- scope = self.scope
- else
- scope = self.scope.where(reflection.klass.primary_key => records)
- end
-
- if method == :delete_all
- update_counter(-scope.delete_all)
- else
- update_counter(-scope.update_all(reflection.foreign_key => nil))
- end
+ scope = self.scope.where(reflection.klass.primary_key => records)
+ update_counter(-delete_count(method, scope))
end
end
diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb
index aeb77e2753..35ad512537 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -130,13 +130,13 @@ module ActiveRecord
end
end
+ def delete_or_nullify_all_records(method)
+ delete_records(load_target, method)
+ end
+
def delete_records(records, method)
ensure_not_nested
- # This is unoptimised; it will load all the target records
- # even when we just want to delete everything.
- records = load_target if records == :all
-
scope = through_association.scope
scope.where! construct_join_attributes(*records)
diff --git a/activerecord/lib/active_record/store.rb b/activerecord/lib/active_record/store.rb
index 79a6ccbda0..7014bc6d45 100644
--- a/activerecord/lib/active_record/store.rb
+++ b/activerecord/lib/active_record/store.rb
@@ -66,8 +66,9 @@ module ActiveRecord
extend ActiveSupport::Concern
included do
- class_attribute :stored_attributes, instance_accessor: false
- self.stored_attributes = {}
+ class << self
+ attr_accessor :local_stored_attributes
+ end
end
module ClassMethods
@@ -93,9 +94,9 @@ module ActiveRecord
# assign new store attribute and create new hash to ensure that each class in the hierarchy
# has its own hash of stored attributes.
- self.stored_attributes = {} if self.stored_attributes.blank?
- self.stored_attributes[store_attribute] ||= []
- self.stored_attributes[store_attribute] |= keys
+ self.local_stored_attributes ||= {}
+ self.local_stored_attributes[store_attribute] ||= []
+ self.local_stored_attributes[store_attribute] |= keys
end
def _store_accessors_module
@@ -105,6 +106,14 @@ module ActiveRecord
mod
end
end
+
+ def stored_attributes
+ parent = superclass.respond_to?(:stored_attributes) ? superclass.stored_attributes : {}
+ if self.local_stored_attributes
+ parent.merge!(self.local_stored_attributes) { |k, a, b| a | b }
+ end
+ parent
+ end
end
protected
diff --git a/activerecord/test/cases/store_test.rb b/activerecord/test/cases/store_test.rb
index 978cee9cfb..6a34c55011 100644
--- a/activerecord/test/cases/store_test.rb
+++ b/activerecord/test/cases/store_test.rb
@@ -163,6 +163,22 @@ class StoreTest < ActiveRecord::TestCase
assert_equal [:width, :height], second_model.stored_attributes[:data]
end
+ test "stored_attributes are tracked per subclass" do
+ first_model = Class.new(ActiveRecord::Base) do
+ store_accessor :data, :color
+ end
+ second_model = Class.new(first_model) do
+ store_accessor :data, :width, :height
+ end
+ third_model = Class.new(first_model) do
+ store_accessor :data, :area, :volume
+ end
+
+ assert_equal [:color], first_model.stored_attributes[:data]
+ assert_equal [:color, :width, :height], second_model.stored_attributes[:data]
+ assert_equal [:color, :area, :volume], third_model.stored_attributes[:data]
+ end
+
test "YAML coder initializes the store when a Nil value is given" do
assert_equal({}, @john.params)
end
diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 68b327e0ad..77224a208a 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,3 +1,7 @@
+* Remove deprecated string based terminators for `ActiveSupport::Callbacks`.
+
+ *Eileen M. Uchitelle*
+
* Fixed an issue when using
`ActiveSupport::NumberHelper::NumberToDelimitedConverter` to
convert a value that is an `ActiveSupport::SafeBuffer` introduced
diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb
index 0a99cbd004..06505bddf9 100644
--- a/activesupport/lib/active_support/callbacks.rb
+++ b/activesupport/lib/active_support/callbacks.rb
@@ -724,12 +724,6 @@ module ActiveSupport
# would call <tt>Audit#save</tt>.
def define_callbacks(*names)
options = names.extract_options!
- if options.key?(:terminator) && String === options[:terminator]
- ActiveSupport::Deprecation.warn "String based terminators are deprecated, please use a lambda"
- value = options[:terminator]
- line = class_eval "lambda { |result| #{value} }", __FILE__, __LINE__
- options[:terminator] = lambda { |target, result| target.instance_exec(result, &line) }
- end
names.each do |name|
class_attribute "_#{name}_callbacks"