diff options
| -rw-r--r-- | actionpack/CHANGELOG.md | 5 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/metal/strong_parameters.rb | 10 | ||||
| -rw-r--r-- | actionpack/test/controller/parameters/parameters_permit_test.rb | 13 | ||||
| -rw-r--r-- | activerecord/CHANGELOG.md | 7 | ||||
| -rw-r--r-- | activerecord/lib/active_record/associations/builder/belongs_to.rb | 16 | ||||
| -rw-r--r-- | activerecord/test/cases/counter_cache_test.rb | 13 | ||||
| -rw-r--r-- | activerecord/test/cases/locking_test.rb | 2 | ||||
| -rw-r--r-- | activerecord/test/models/aircraft.rb | 1 | ||||
| -rw-r--r-- | activerecord/test/schema/schema.rb | 1 | ||||
| -rw-r--r-- | activesupport/CHANGELOG.md | 8 | ||||
| -rw-r--r-- | activesupport/lib/active_support/time_with_zone.rb | 2 | ||||
| -rw-r--r-- | activesupport/test/core_ext/time_with_zone_test.rb | 4 |
12 files changed, 73 insertions, 9 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 4b081591c0..6a68c057ac 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,8 @@ +* Fix `ActionController::Parameters#fetch` overwriting `KeyError` returned by + default block. + + *Jonas Schuber Erlandsson*, *Roque Pinel* + * `ActionController::Parameters` no longer inherits from `HashWithIndifferentAccess` diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 06d625e4d5..cf6a64009f 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -380,11 +380,15 @@ module ActionController def fetch(key, *args, &block) convert_hashes_to_parameters( key, - @parameters.fetch(key, *args, &block), + @parameters.fetch(key) { + if block_given? + yield + else + args.fetch(0) { raise ActionController::ParameterMissing.new(key) } + end + }, false ) - rescue KeyError - raise ActionController::ParameterMissing.new(key) end # Returns a new <tt>ActionController::Parameters</tt> instance that diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index 05532ec21b..2dd2826196 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -194,6 +194,19 @@ class ParametersPermitTest < ActiveSupport::TestCase assert_equal "monkey", @params.fetch(:foo) { "monkey" } end + test "fetch doesnt raise ParameterMissing exception if there is a default that is nil" do + assert_equal nil, @params.fetch(:foo, nil) + assert_equal nil, @params.fetch(:foo) { nil } + end + + test 'KeyError in fetch block should not be coverd up' do + params = ActionController::Parameters.new + e = assert_raises(KeyError) do + params.fetch(:missing_key) { {}.fetch(:also_missing) } + end + assert_match(/:also_missing$/, e.message) + end + test "not permitted is sticky beyond merges" do assert !@params.merge(a: "b").permitted? end diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 997abcb48d..3fa24f3837 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,10 @@ +* Fix a bug where counter_cache doesn't always work with polymorphic + relations. + + Fixes #16407. + + *Stefan Kanev & Sean Griffin* + * Ensure that cyclic associations with autosave don't cause duplicate errors to be added to the parent record. diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 97eb007f62..6e4a53f7fb 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -33,16 +33,24 @@ module ActiveRecord::Associations::Builder if (@_after_create_counter_called ||= false) @_after_create_counter_called = false - elsif attribute_changed?(foreign_key) && !new_record? && reflection.constructable? - model = reflection.klass + elsif attribute_changed?(foreign_key) && !new_record? + if reflection.polymorphic? + model = attribute(reflection.foreign_type).try(:constantize) + model_was = attribute_was(reflection.foreign_type).try(:constantize) + else + model = reflection.klass + model_was = reflection.klass + end + foreign_key_was = attribute_was foreign_key foreign_key = attribute foreign_key if foreign_key && model.respond_to?(:increment_counter) model.increment_counter(cache_column, foreign_key) end - if foreign_key_was && model.respond_to?(:decrement_counter) - model.decrement_counter(cache_column, foreign_key_was) + + if foreign_key_was && model_was.respond_to?(:decrement_counter) + model_was.decrement_counter(cache_column, foreign_key_was) end end end diff --git a/activerecord/test/cases/counter_cache_test.rb b/activerecord/test/cases/counter_cache_test.rb index 1f5055b2a2..922cb59280 100644 --- a/activerecord/test/cases/counter_cache_test.rb +++ b/activerecord/test/cases/counter_cache_test.rb @@ -1,6 +1,7 @@ require 'cases/helper' require 'models/topic' require 'models/car' +require 'models/aircraft' require 'models/wheel' require 'models/engine' require 'models/reply' @@ -198,4 +199,16 @@ class CounterCacheTest < ActiveRecord::TestCase assert_equal 2, car.engines_count assert_equal 2, car.reload.engines_count end + + test "update counters in a polymorphic relationship" do + aircraft = Aircraft.create! + + assert_difference 'aircraft.reload.wheels_count' do + aircraft.wheels << Wheel.create! + end + + assert_difference 'aircraft.reload.wheels_count', -1 do + aircraft.wheels.first.destroy + end + end end diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index dbdcc84b7d..2e1363334d 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -270,7 +270,7 @@ class OptimisticLockingTest < ActiveRecord::TestCase car.wheels << Wheel.create! end assert_difference 'car.wheels.count', -1 do - car.destroy + car.reload.destroy end assert car.destroyed? end diff --git a/activerecord/test/models/aircraft.rb b/activerecord/test/models/aircraft.rb index 1f35ef45da..c4404a8094 100644 --- a/activerecord/test/models/aircraft.rb +++ b/activerecord/test/models/aircraft.rb @@ -1,4 +1,5 @@ class Aircraft < ActiveRecord::Base self.pluralize_table_names = false has_many :engines, :foreign_key => "car_id" + has_many :wheels, as: :wheelable end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 2fc806f181..7bab675b2a 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -37,6 +37,7 @@ ActiveRecord::Schema.define do create_table :aircraft, force: true do |t| t.string :name + t.integer :wheels_count, default: 0, null: false end create_table :articles, force: true do |t| diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 1c69e0eb12..7148f289bb 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,11 @@ +* Fix `TimeWithZone#eql?` to properly handle `TimeWithZone` created from `DateTime`: + twz = DateTime.now.in_time_zone + twz.eql?(twz.dup) => true + + Fixes #14178. + + *Roque Pinel* + * ActiveSupport::HashWithIndifferentAccess `select` and `reject` will now return enumerator if called without block. diff --git a/activesupport/lib/active_support/time_with_zone.rb b/activesupport/lib/active_support/time_with_zone.rb index 412c72d27c..f8f1b9ac2c 100644 --- a/activesupport/lib/active_support/time_with_zone.rb +++ b/activesupport/lib/active_support/time_with_zone.rb @@ -247,7 +247,7 @@ module ActiveSupport end def eql?(other) - utc.eql?(other) + other.eql?(utc) end def hash diff --git a/activesupport/test/core_ext/time_with_zone_test.rb b/activesupport/test/core_ext/time_with_zone_test.rb index 477a42114b..ccb7f02331 100644 --- a/activesupport/test/core_ext/time_with_zone_test.rb +++ b/activesupport/test/core_ext/time_with_zone_test.rb @@ -253,10 +253,14 @@ class TimeWithZoneTest < ActiveSupport::TestCase end def test_eql? + assert_equal true, @twz.eql?(@twz.dup) assert_equal true, @twz.eql?(Time.utc(2000)) assert_equal true, @twz.eql?( ActiveSupport::TimeWithZone.new(Time.utc(2000), ActiveSupport::TimeZone["Hawaii"]) ) assert_equal false, @twz.eql?( Time.utc(2000, 1, 1, 0, 0, 1) ) assert_equal false, @twz.eql?( DateTime.civil(1999, 12, 31, 23, 59, 59) ) + + other_twz = ActiveSupport::TimeWithZone.new(DateTime.now.utc, @time_zone) + assert_equal true, other_twz.eql?(other_twz.dup) end def test_hash |