aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--actionpack/lib/action_controller/metal/strong_parameters.rb19
-rw-r--r--actionpack/test/controller/parameters/dup_test.rb43
-rw-r--r--actionpack/test/controller/parameters/parameters_permit_test.rb5
-rw-r--r--activemodel/lib/active_model/type/boolean.rb15
-rw-r--r--activerecord/CHANGELOG.md5
-rw-r--r--activerecord/lib/active_record/model_schema.rb8
-rw-r--r--activerecord/lib/active_record/relation/calculations.rb10
-rw-r--r--activerecord/test/cases/calculations_test.rb7
-rw-r--r--activesupport/CHANGELOG.md9
-rw-r--r--activesupport/lib/active_support/core_ext/class/attribute.rb2
-rw-r--r--activesupport/lib/active_support/duration/iso8601_serializer.rb4
-rw-r--r--activesupport/lib/active_support/values/time_zone.rb1
-rw-r--r--activesupport/test/core_ext/duration_test.rb1
-rw-r--r--activesupport/test/time_zone_test.rb7
14 files changed, 106 insertions, 30 deletions
diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb
index b326695ce2..26794c67b7 100644
--- a/actionpack/lib/action_controller/metal/strong_parameters.rb
+++ b/actionpack/lib/action_controller/metal/strong_parameters.rb
@@ -572,20 +572,6 @@ module ActionController
convert_value_to_parameters(@parameters.values_at(*keys))
end
- # Returns an exact copy of the <tt>ActionController::Parameters</tt>
- # instance. +permitted+ state is kept on the duped object.
- #
- # params = ActionController::Parameters.new(a: 1)
- # params.permit!
- # params.permitted? # => true
- # copy_params = params.dup # => <ActionController::Parameters {"a"=>1} permitted: true>
- # copy_params.permitted? # => true
- def dup
- super.tap do |duplicate|
- duplicate.permitted = @permitted
- end
- end
-
# Returns a new <tt>ActionController::Parameters</tt> with all keys from
# +other_hash+ merges into current hash.
def merge(other_hash)
@@ -783,6 +769,11 @@ module ActionController
end
end
end
+
+ def initialize_copy(source)
+ super
+ @parameters = @parameters.dup
+ end
end
# == Strong \Parameters
diff --git a/actionpack/test/controller/parameters/dup_test.rb b/actionpack/test/controller/parameters/dup_test.rb
new file mode 100644
index 0000000000..66bc8155c8
--- /dev/null
+++ b/actionpack/test/controller/parameters/dup_test.rb
@@ -0,0 +1,43 @@
+require 'abstract_unit'
+require 'action_controller/metal/strong_parameters'
+
+class ParametersDupTest < ActiveSupport::TestCase
+ setup do
+ ActionController::Parameters.permit_all_parameters = false
+
+ @params = ActionController::Parameters.new(
+ person: {
+ age: '32',
+ name: {
+ first: 'David',
+ last: 'Heinemeier Hansson'
+ },
+ addresses: [{city: 'Chicago', state: 'Illinois'}]
+ }
+ )
+ end
+
+ test "a duplicate maintains the original's permitted status" do
+ @params.permit!
+ dupped_params = @params.dup
+ assert dupped_params.permitted?
+ end
+
+ test "a duplicate maintains the original's parameters" do
+ @params.permit!
+ dupped_params = @params.dup
+ assert_equal @params.to_h, dupped_params.to_h
+ end
+
+ test "changes to a duplicate's parameters do not affect the original" do
+ dupped_params = @params.dup
+ dupped_params.delete(:person)
+ assert_not_equal @params, dupped_params
+ end
+
+ test "changes to a duplicate's permitted status do not affect the original" do
+ dupped_params = @params.dup
+ dupped_params.permit!
+ assert_not_equal @params, dupped_params
+ end
+end
diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb
index 2eed2996f6..2dd94c7230 100644
--- a/actionpack/test/controller/parameters/parameters_permit_test.rb
+++ b/actionpack/test/controller/parameters/parameters_permit_test.rb
@@ -245,11 +245,6 @@ class ParametersPermitTest < ActiveSupport::TestCase
assert_equal "Jonas", @params[:person][:family][:brother]
end
- test "permit state is kept on a dup" do
- @params.permit!
- assert_equal @params.permitted?, @params.dup.permitted?
- end
-
test "permit is recursive" do
@params.permit!
assert @params.permitted?
diff --git a/activemodel/lib/active_model/type/boolean.rb b/activemodel/lib/active_model/type/boolean.rb
index c1bce98c87..4e9d06a3ce 100644
--- a/activemodel/lib/active_model/type/boolean.rb
+++ b/activemodel/lib/active_model/type/boolean.rb
@@ -1,9 +1,20 @@
module ActiveModel
module Type
- class Boolean < Value # :nodoc:
+ # == Active \Model \Type \Boolean
+ #
+ # A class that behaves like a boolean type, including rules for coercion of user input.
+ #
+ # === Coercion
+ # Values set from user input will first be coerced into the appropriate ruby type.
+ # Coercion behavior is roughly mapped to Ruby's boolean semantics.
+ #
+ # - "false", "f" , "0", +0+ or any other value in +FALSE_VALUES+ will be coerced to +false+
+ # - Empty strings are coerced to +nil+
+ # - All other values will be coerced to +true+
+ class Boolean < Value
FALSE_VALUES = [false, 0, '0', 'f', 'F', 'false', 'FALSE', 'off', 'OFF'].to_set
- def type
+ def type # :nodoc:
:boolean
end
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 5d04551125..c91e6662c9 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,8 @@
+* Using `group` with an attribute that has a custom type will properly cast
+ the hash keys after calling a calculation method like `count`. Fixes #25595.
+
+ *Sean Griffin*
+
* Fix the generated `#to_param` method to use `omission: ''` so that
the resulting output is actually up to 20 characters, not
effectively 17 to leave room for the default "...".
diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb
index 7996c32bbc..114686c5d3 100644
--- a/activerecord/lib/active_record/model_schema.rb
+++ b/activerecord/lib/active_record/model_schema.rb
@@ -282,8 +282,12 @@ module ActiveRecord
#
# +attr_name+ The name of the attribute to retrieve the type for. Must be
# a string
- def type_for_attribute(attr_name)
- attribute_types[attr_name]
+ def type_for_attribute(attr_name, &block)
+ if block
+ attribute_types.fetch(attr_name, &block)
+ else
+ attribute_types[attr_name]
+ end
end
# Returns a hash where the keys are column names and the values are
diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb
index d6d92b8607..a97b71815a 100644
--- a/activerecord/lib/active_record/relation/calculations.rb
+++ b/activerecord/lib/active_record/relation/calculations.rb
@@ -312,8 +312,10 @@ module ActiveRecord
Hash[calculated_data.map do |row|
key = group_columns.map { |aliaz, col_name|
- column = calculated_data.column_types.fetch(aliaz) do
- type_for(col_name)
+ column = type_for(col_name) do
+ calculated_data.column_types.fetch(aliaz) do
+ Type::Value.new
+ end
end
type_cast_calculated_value(row[aliaz], column)
}
@@ -346,9 +348,9 @@ module ActiveRecord
@klass.connection.table_alias_for(table_name)
end
- def type_for(field)
+ def type_for(field, &block)
field_name = field.respond_to?(:name) ? field.name.to_s : field.to_s.split('.').last
- @klass.type_for_attribute(field_name)
+ @klass.type_for_attribute(field_name, &block)
end
def type_cast_calculated_value(value, type, operation = nil)
diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb
index cfae700159..6acfec0621 100644
--- a/activerecord/test/cases/calculations_test.rb
+++ b/activerecord/test/cases/calculations_test.rb
@@ -1,4 +1,5 @@
require "cases/helper"
+require "models/book"
require 'models/club'
require 'models/company'
require "models/contract"
@@ -25,7 +26,7 @@ class NumericData < ActiveRecord::Base
end
class CalculationsTest < ActiveRecord::TestCase
- fixtures :companies, :accounts, :topics, :speedometers, :minivans
+ fixtures :companies, :accounts, :topics, :speedometers, :minivans, :books
def test_should_sum_field
assert_equal 318, Account.sum(:credit_limit)
@@ -793,4 +794,8 @@ class CalculationsTest < ActiveRecord::TestCase
assert_equal 50, result[1].credit_limit
assert_equal 50, result[2].credit_limit
end
+
+ def test_group_by_attribute_with_custom_type
+ assert_equal({ "proposed" => 2, "published" => 2 }, Book.group(:status).count)
+ end
end
diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 1c6f340fba..a8d875640e 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,3 +1,12 @@
+* Fix `ActiveSupport::TimeZone#strptime`. Now raises `ArgumentError` when the
+ given time doesn't match the format. The error is the same as the one given
+ by Ruby's `Date.strptime`. Previously it raised
+ `NoMethodError: undefined method empty? for nil:NilClass.` due to a bug.
+
+ Fixes #25701.
+
+ *John Gesimondo*
+
* `travel/travel_to` travel time helpers, now raise on nested calls,
as this can lead to confusing time stubbing.
diff --git a/activesupport/lib/active_support/core_ext/class/attribute.rb b/activesupport/lib/active_support/core_ext/class/attribute.rb
index 802d988af2..aa0e2a1a88 100644
--- a/activesupport/lib/active_support/core_ext/class/attribute.rb
+++ b/activesupport/lib/active_support/core_ext/class/attribute.rb
@@ -27,7 +27,7 @@ class Class
# This matches normal Ruby method inheritance: think of writing an attribute
# on a subclass as overriding the reader method. However, you need to be aware
# when using +class_attribute+ with mutable structures as +Array+ or +Hash+.
- # In such cases, you don't want to do changes in places but use setters:
+ # In such cases, you don't want to do changes in place. Instead use setters:
#
# Base.setting = []
# Base.setting # => []
diff --git a/activesupport/lib/active_support/duration/iso8601_serializer.rb b/activesupport/lib/active_support/duration/iso8601_serializer.rb
index 05c6a083a9..eb8136e208 100644
--- a/activesupport/lib/active_support/duration/iso8601_serializer.rb
+++ b/activesupport/lib/active_support/duration/iso8601_serializer.rb
@@ -12,8 +12,10 @@ module ActiveSupport
# Builds and returns output string.
def serialize
- output = 'P'
parts, sign = normalize
+ return "PT0S".freeze if parts.empty?
+
+ output = 'P'
output << "#{parts[:years]}Y" if parts.key?(:years)
output << "#{parts[:months]}M" if parts.key?(:months)
output << "#{parts[:weeks]}W" if parts.key?(:weeks)
diff --git a/activesupport/lib/active_support/values/time_zone.rb b/activesupport/lib/active_support/values/time_zone.rb
index 19420cee5e..eb89a6d4c5 100644
--- a/activesupport/lib/active_support/values/time_zone.rb
+++ b/activesupport/lib/active_support/values/time_zone.rb
@@ -447,6 +447,7 @@ module ActiveSupport
private
def parts_to_time(parts, now)
+ raise ArgumentError, "invalid date" if parts.nil?
return if parts.empty?
time = Time.new(
diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb
index 502e2811fa..a24915dfef 100644
--- a/activesupport/test/core_ext/duration_test.rb
+++ b/activesupport/test/core_ext/duration_test.rb
@@ -279,6 +279,7 @@ class DurationTest < ActiveSupport::TestCase
['PT1S', 1.second ],
['PT1.4S', (1.4).seconds ],
['P1Y1M1DT1H', 1.year + 1.month + 1.day + 1.hour],
+ ['PT0S', 0.minutes ],
]
expectations.each do |expected_output, duration|
assert_equal expected_output, duration.iso8601, expected_output.inspect
diff --git a/activesupport/test/time_zone_test.rb b/activesupport/test/time_zone_test.rb
index a15d5c6a0e..76cbb5ce8b 100644
--- a/activesupport/test/time_zone_test.rb
+++ b/activesupport/test/time_zone_test.rb
@@ -388,6 +388,13 @@ class TimeZoneTest < ActiveSupport::TestCase
end
end
+ def test_strptime_with_malformed_string
+ with_env_tz 'US/Eastern' do
+ zone = ActiveSupport::TimeZone['Eastern Time (US & Canada)']
+ assert_raise(ArgumentError) { zone.strptime('1999-12-31', '%Y/%m/%d') }
+ end
+ end
+
def test_utc_offset_lazy_loaded_from_tzinfo_when_not_passed_in_to_initialize
tzinfo = TZInfo::Timezone.get('America/New_York')
zone = ActiveSupport::TimeZone.create(tzinfo.name, nil, tzinfo)