Merge pull request #6873 from mauricio/fix-for-6825

Fixes #6825, adds tests covering cases and error possibilities

activerecord/lib/active_record/attribute_assignment.rb

@@ -158,11 +158,12 @@ module ActiveRecord
         begin
           send(name + "=", read_value_from_parameter(name, values_with_empty_parameters))
         rescue => ex
-          errors << AttributeAssignmentError.new("error on assignment #{values_with_empty_parameters.values.inspect} to #{name}", ex, name)
+          errors << AttributeAssignmentError.new("error on assignment #{values_with_empty_parameters.values.inspect} to #{name} (#{ex.message})", ex, name)
         end
       end
       unless errors.empty?
-        raise MultiparameterAssignmentErrors.new(errors), "#{errors.size} error(s) on assignment of multiparameter attributes"
+        error_descriptions = errors.map { |ex| ex.message }.join(",")
+        raise MultiparameterAssignmentErrors.new(errors), "#{errors.size} error(s) on assignment of multiparameter attributes [#{error_descriptions}]"
       end
     end
 
@@ -180,12 +181,24 @@ module ActiveRecord
     end
 
     def read_time_parameter_value(name, values_hash_from_param)
-      # If Date bits were not provided, error
-      raise "Missing Parameter" if [1,2,3].any?{|position| !values_hash_from_param.has_key?(position)}
-      max_position = extract_max_param_for_multiparameter_attributes(values_hash_from_param, 6)
+      # If column is a :time (and not :date or :timestamp) there is no need to validate if
+      # there are year/month/day fields
+      if column_for_attribute(name).type == :time
+        # if the column is a time set the values to their defaults as January 1, 1970, but only if they're nil
+        {1 => 1970, 2 => 1, 3 => 1}.each do |key,value|
+          values_hash_from_param[key] ||= value
+        end
+      else
+        # else column is a timestamp, so if Date bits were not provided, error
+        if missing_parameter = [1,2,3].detect{ |position| !values_hash_from_param.has_key?(position) }
+          raise ArgumentError.new("Missing Parameter - #{name}(#{missing_parameter}i)")
+        end
+
         # If Date bits were provided but blank, then return nil
         return nil if (1..3).any? { |position| values_hash_from_param[position].blank? }
+      end
 
+      max_position = extract_max_param_for_multiparameter_attributes(values_hash_from_param, 6)
       set_values = (1..max_position).collect{ |position| values_hash_from_param[position] }
       # If Time bits are not there, then default to 0
       (3..5).each { |i| set_values[i] = set_values[i].blank? ? 0 : set_values[i] }

activerecord/lib/active_record/errors.rb

@@ -106,13 +106,11 @@ module ActiveRecord
     attr_reader :record, :attempted_action
 
     def initialize(record, attempted_action)
+      super("Attempted to #{attempted_action} a stale object: #{record.class.name}")
       @record = record
       @attempted_action = attempted_action
     end
 
-    def message
-      "Attempted to #{attempted_action} a stale object: #{record.class.name}"
-    end
   end
 
   # Raised when association is being configured improperly or
@@ -168,9 +166,9 @@ module ActiveRecord
   class AttributeAssignmentError < ActiveRecordError
     attr_reader :exception, :attribute
     def initialize(message, exception, attribute)
+      super(message)
       @exception = exception
       @attribute = attribute
-      @message = message
     end
   end
 
@@ -189,12 +187,10 @@ module ActiveRecord
     attr_reader :model
 
     def initialize(model)
+      super("Unknown primary key for table #{model.table_name} in model #{model}.")
       @model = model
     end
 
-    def message
-      "Unknown primary key for table #{model.table_name} in model #{model}."
-    end
   end
 
   class ImmutableRelation < ActiveRecordError

activerecord/test/cases/attribute_methods_test.rb

@@ -791,6 +791,39 @@ class AttributeMethodsTest < ActiveRecord::TestCase
     assert_equal "lol", topic.author_name
   end
 
+  def test_setting_time_attribute
+    topic = Topic.new( "bonus_time(4i)"=> "01", "bonus_time(5i)" => "05" )
+    assert_equal 1, topic.bonus_time.hour
+    assert_equal 5, topic.bonus_time.min
+  end
+
+  def test_setting_date_attribute
+    topic = Topic.new( "written_on(1i)" => "1952", "written_on(2i)" => "3", "written_on(3i)" => "11" )
+    assert_equal 1952, topic.written_on.year
+    assert_equal 3, topic.written_on.month
+    assert_equal 11, topic.written_on.day
+  end
+
+  def test_setting_date_and_time_attribute
+    topic = Topic.new(
+        "written_on(1i)" => "1952",
+        "written_on(2i)" => "3",
+        "written_on(3i)" => "11",
+        "written_on(4i)" => "13",
+        "written_on(5i)" => "55")
+    assert_equal 1952, topic.written_on.year
+    assert_equal 3, topic.written_on.month
+    assert_equal 11, topic.written_on.day
+    assert_equal 13, topic.written_on.hour
+    assert_equal 55, topic.written_on.min
+  end
+
+  def test_setting_time_but_not_date_on_date_field
+    assert_raise( ActiveRecord::MultiparameterAssignmentErrors ) do
+      Topic.new( "written_on(4i)" => "13", "written_on(5i)" => "55" )
+    end
+  end
+
   private
   def cached_columns
     Topic.columns.find_all { |column|