Extracts custom field values validation from CustomValue so that they can be validated globally from the customized object (#1189).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@8717 e93f8b46-1217-0410-a6f0-8f06a7374b81

11 files changed, 249 insertions, 193 deletions

diff --git a/app/helpers/custom_fields_helper.rb b/app/helpers/custom_fields_helper.rb
index 68e7d987d..7b1927c0a 100644
--- a/app/helpers/custom_fields_helper.rb
+++ b/app/helpers/custom_fields_helper.rb
@@ -1,7 +1,7 @@
 # encoding: utf-8
 #
 # Redmine - project management software
-# Copyright (C) 2006-2011  Jean-Philippe Lang
+# Copyright (C) 2006-2012  Jean-Philippe Lang
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU General Public License
@@ -61,8 +61,7 @@ module CustomFieldsHelper
   def custom_field_label_tag(name, custom_value)
     content_tag "label", h(custom_value.custom_field.name) +
 	(custom_value.custom_field.is_required? ? " <span class=\"required\">*</span>".html_safe : ""),
-	:for => "#{name}_custom_field_values_#{custom_value.custom_field.id}",
-	:class => (custom_value.errors.empty? ? nil : "error" )
+	:for => "#{name}_custom_field_values_#{custom_value.custom_field.id}"
   end
 
   # Return custom field tag with its label tag
diff --git a/app/models/custom_field.rb b/app/models/custom_field.rb
index 87285ed66..0dd70de1d 100644
--- a/app/models/custom_field.rb
+++ b/app/models/custom_field.rb
@@ -1,5 +1,5 @@
 # Redmine - project management software
-# Copyright (C) 2006-2011  Jean-Philippe Lang
+# Copyright (C) 2006-2012  Jean-Philippe Lang
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU General Public License
@@ -27,7 +27,7 @@ class CustomField < ActiveRecord::Base
   validates_length_of :name, :maximum => 30
   validates_inclusion_of :field_format, :in => Redmine::CustomFieldFormat.available_formats
 
-  validate :validate_values
+  validate :validate_custom_field
   before_validation :set_searchable
 
   def initialize(attributes=nil, *args)
@@ -41,7 +41,7 @@ class CustomField < ActiveRecord::Base
     true
   end
 
-  def validate_values
+  def validate_custom_field
     if self.field_format == "list"
       errors.add(:possible_values, :blank) if self.possible_values.nil? || self.possible_values.empty?
       errors.add(:possible_values, :invalid) unless self.possible_values.is_a? Array
@@ -55,10 +55,9 @@ class CustomField < ActiveRecord::Base
       end
     end
 
-    # validate default value
-    v = CustomValue.new(:custom_field => self.clone, :value => default_value, :customized => nil)
-    v.custom_field.is_required = false
-    errors.add(:default_value, :invalid) unless v.valid?
+    unless valid_field_value?(default_value)
+      errors.add(:default_value, :invalid)
+    end
   end
 
   def possible_values_options(obj=nil)
@@ -161,4 +160,45 @@ class CustomField < ActiveRecord::Base
   def type_name
     nil
   end
+
+  # Returns the error message for the given value
+  # or an empty array if value is a valid value for the custom field
+  def validate_field_value(value)
+    errs = []
+    if is_required? && value.blank?
+      errs << ::I18n.t('activerecord.errors.messages.blank')
+    end
+    errs += validate_field_value_format(value)
+    errs
+  end
+
+  # Returns true if value is a valid value for the custom field
+  def valid_field_value?(value)
+    validate_field_value(value).empty?
+  end
+
+  protected
+
+  # Returns the error message for the given value regarding its format
+  def validate_field_value_format(value)
+    errs = []
+    if value.present?
+      errs << ::I18n.t('activerecord.errors.messages.invalid') unless regexp.blank? or value =~ Regexp.new(regexp)
+      errs << ::I18n.t('activerecord.errors.messages.too_short', :count => min_length) if min_length > 0 and value.length < min_length
+      errs << ::I18n.t('activerecord.errors.messages.too_long', :count => max_length) if max_length > 0 and value.length > max_length
+
+      # Format specific validations
+      case field_format
+      when 'int'
+        errs << ::I18n.t('activerecord.errors.messages.not_a_number') unless value =~ /^[+-]?\d+$/
+      when 'float'
+        begin; Kernel.Float(value); rescue; errs << ::I18n.t('activerecord.errors.messages.invalid') end
+      when 'date'
+        errs << ::I18n.t('activerecord.errors.messages.not_a_date') unless value =~ /^\d{4}-\d{2}-\d{2}$/ && begin; value.to_date; rescue; false end
+      when 'list'
+        errs << ::I18n.t('activerecord.errors.messages.inclusion') unless possible_values.include?(value)
+      end
+    end
+    errs
+  end
 end
diff --git a/app/models/custom_field_value.rb b/app/models/custom_field_value.rb
new file mode 100644
index 000000000..ec243af1d
--- /dev/null
+++ b/app/models/custom_field_value.rb
@@ -0,0 +1,50 @@
+# Redmine - project management software
+# Copyright (C) 2006-2012  Jean-Philippe Lang
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+
+class CustomFieldValue
+  attr_accessor :custom_field, :customized, :value
+
+  def custom_field_id
+    custom_field.id
+  end
+
+  def true?
+    self.value == '1'
+  end
+
+  def editable?
+    custom_field.editable?
+  end
+
+  def visible?
+    custom_field.visible?
+  end
+
+  def required?
+    custom_field.is_required?
+  end
+
+  def to_s
+    value.to_s
+  end
+
+  def validate_value
+    custom_field.validate_field_value(value).each do |message|
+      customized.errors.add(:base, custom_field.name + ' ' + message)
+    end
+  end
+end
diff --git a/app/models/custom_value.rb b/app/models/custom_value.rb
index 6cb4853f9..89d3709cc 100644
--- a/app/models/custom_value.rb
+++ b/app/models/custom_value.rb
@@ -1,5 +1,5 @@
 # Redmine - project management software
-# Copyright (C) 2006-2011  Jean-Philippe Lang
+# Copyright (C) 2006-2012  Jean-Philippe Lang
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU General Public License
@@ -19,8 +19,6 @@ class CustomValue < ActiveRecord::Base
   belongs_to :custom_field
   belongs_to :customized, :polymorphic => true
 
-  validate :validate_custom_value
-
   def initialize(attributes=nil, *args)
     super
     if new_record? && custom_field && (customized_type.blank? || (customized && customized.new_record?))
@@ -48,27 +46,4 @@ class CustomValue < ActiveRecord::Base
   def to_s
     value.to_s
   end
-
-protected
-  def validate_custom_value
-    if value.blank?
-      errors.add(:value, :blank) if custom_field.is_required? and value.blank?
-    else
-      errors.add(:value, :invalid) unless custom_field.regexp.blank? or value =~ Regexp.new(custom_field.regexp)
-      errors.add(:value, :too_short, :count => custom_field.min_length) if custom_field.min_length > 0 and value.length < custom_field.min_length
-      errors.add(:value, :too_long, :count => custom_field.max_length) if custom_field.max_length > 0 and value.length > custom_field.max_length
-
-      # Format specific validations
-      case custom_field.field_format
-      when 'int'
-        errors.add(:value, :not_a_number) unless value =~ /^[+-]?\d+$/	
-      when 'float'
-        begin; Kernel.Float(value); rescue; errors.add(:value, :invalid) end
-      when 'date'
-        errors.add(:value, :not_a_date) unless value =~ /^\d{4}-\d{2}-\d{2}$/ && begin; value.to_date; rescue; false end
-      when 'list'
-        errors.add(:value, :inclusion) unless custom_field.possible_values.include?(value)
-      end
-    end
-  end
 end
diff --git a/config/initializers/10-patches.rb b/config/initializers/10-patches.rb
index b4810f0fa..959107048 100644
--- a/config/initializers/10-patches.rb
+++ b/config/initializers/10-patches.rb
@@ -16,36 +16,6 @@ module ActiveRecord
   end
 end
 
-module ActiveRecord
-  class Errors
-    def full_messages(options = {})
-      full_messages = []
-
-      @errors.each_key do |attr|
-        @errors[attr].each do |message|
-          next unless message
-
-          if attr == "base"
-            full_messages << message
-          elsif attr == "custom_values"
-            # Replace the generic "custom values is invalid"
-            # with the errors on custom values
-            @base.custom_values.each do |value|
-              value.errors.each do |attr, msg|
-                full_messages << value.custom_field.name + ' ' + msg
-              end
-            end
-          else
-            attr_name = @base.class.human_attribute_name(attr)
-            full_messages << attr_name + ' ' + message.to_s
-          end
-        end
-      end
-      full_messages
-    end
-  end
-end
-
 module ActionView
   module Helpers
     module DateHelper
diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index 336d86e79..129faad1e 100644
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -1,5 +1,5 @@
 # Redmine - project management software
-# Copyright (C) 2006-2011  Jean-Philippe Lang
+# Copyright (C) 2006-2012  Jean-Philippe Lang
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU General Public License
@@ -1308,17 +1308,18 @@ class IssuesControllerTest < ActionController::TestCase
     field.update_attribute(:is_required, true)
 
     @request.session[:user_id] = 2
-    post :create, :project_id => 1,
-               :issue => {:tracker_id => 1,
-                          :subject => 'This is the test_new issue',
-                          :description => 'This is the description',
-                          :priority_id => 5}
+    assert_no_difference 'Issue.count' do
+      post :create, :project_id => 1,
+                 :issue => {:tracker_id => 1,
+                            :subject => 'This is the test_new issue',
+                            :description => 'This is the description',
+                            :priority_id => 5}
+    end
     assert_response :success
     assert_template 'new'
     issue = assigns(:issue)
     assert_not_nil issue
-    assert_equal I18n.translate('activerecord.errors.messages.invalid'),
-                 issue.errors[:custom_values].to_s
+    assert_error_tag :content => /Database can't be blank/
   end
 
   def test_post_create_with_watchers
diff --git a/test/unit/custom_field_test.rb b/test/unit/custom_field_test.rb
index 8b075cba8..2d1183b72 100644
--- a/test/unit/custom_field_test.rb
+++ b/test/unit/custom_field_test.rb
@@ -1,5 +1,5 @@
 # Redmine - project management software
-# Copyright (C) 2006-2011  Jean-Philippe Lang
+# Copyright (C) 2006-2012  Jean-Philippe Lang
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU General Public License
@@ -44,6 +44,14 @@ class CustomFieldTest < ActiveSupport::TestCase
     assert field.save
   end
 
+  def test_default_value_should_be_validated
+    field = CustomField.new(:name => 'Test', :field_format => 'int')
+    field.default_value = 'abc'
+    assert !field.valid?
+    field.default_value = '6'
+    assert field.valid?
+  end
+
   def test_possible_values_should_accept_an_array
     field = CustomField.new
     field.possible_values = ["One value", ""]
@@ -85,4 +93,75 @@ class CustomFieldTest < ActiveSupport::TestCase
   def test_new_subclass_instance_with_non_subclass_name_should_return_nil
     assert_nil CustomField.new_subclass_instance('Project')
   end
+
+  def test_string_field_validation_with_blank_value
+    f = CustomField.new(:field_format => 'string')
+
+    assert f.valid_field_value?(nil)
+    assert f.valid_field_value?('')
+
+    f.is_required = true
+    assert !f.valid_field_value?(nil)
+    assert !f.valid_field_value?('')
+  end
+
+  def test_string_field_validation_with_min_and_max_lengths
+    f = CustomField.new(:field_format => 'string', :min_length => 2, :max_length => 5)
+
+    assert f.valid_field_value?(nil)
+    assert f.valid_field_value?('')
+    assert f.valid_field_value?('a' * 2)
+    assert !f.valid_field_value?('a')
+    assert !f.valid_field_value?('a' * 6)
+  end
+
+  def test_string_field_validation_with_regexp
+    f = CustomField.new(:field_format => 'string', :regexp => '^[A-Z0-9]*$')
+
+    assert f.valid_field_value?(nil)
+    assert f.valid_field_value?('')
+    assert f.valid_field_value?('ABC')
+    assert !f.valid_field_value?('abc')
+  end
+
+  def test_date_field_validation
+    f = CustomField.new(:field_format => 'date')
+
+    assert f.valid_field_value?(nil)
+    assert f.valid_field_value?('')
+    assert f.valid_field_value?('1975-07-14')
+    assert !f.valid_field_value?('1975-07-33')
+    assert !f.valid_field_value?('abc')
+  end
+
+  def test_list_field_validation
+    f = CustomField.new(:field_format => 'list', :possible_values => ['value1', 'value2'])
+
+    assert f.valid_field_value?(nil)
+    assert f.valid_field_value?('')
+    assert f.valid_field_value?('value2')
+    assert !f.valid_field_value?('abc')
+  end
+
+  def test_int_field_validation
+    f = CustomField.new(:field_format => 'int')
+
+    assert f.valid_field_value?(nil)
+    assert f.valid_field_value?('')
+    assert f.valid_field_value?('123')
+    assert f.valid_field_value?('+123')
+    assert f.valid_field_value?('-123')
+    assert !f.valid_field_value?('6abc')
+  end
+
+  def test_float_field_validation
+    f = CustomField.new(:field_format => 'float')
+
+    assert f.valid_field_value?(nil)
+    assert f.valid_field_value?('')
+    assert f.valid_field_value?('11.2')
+    assert f.valid_field_value?('-6.250')
+    assert f.valid_field_value?('5')
+    assert !f.valid_field_value?('6abc')
+  end
 end
diff --git a/test/unit/custom_value_test.rb b/test/unit/custom_value_test.rb
index 95931d277..5155c5efc 100644
--- a/test/unit/custom_value_test.rb
+++ b/test/unit/custom_value_test.rb
@@ -1,5 +1,5 @@
 # Redmine - project management software
-# Copyright (C) 2006-2011  Jean-Philippe Lang
+# Copyright (C) 2006-2012  Jean-Philippe Lang
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU General Public License
@@ -20,92 +20,6 @@ require File.expand_path('../../test_helper', __FILE__)
 class CustomValueTest < ActiveSupport::TestCase
   fixtures :custom_fields, :custom_values, :users
 
-  def test_string_field_validation_with_blank_value
-    f = CustomField.new(:field_format => 'string')
-    v = CustomValue.new(:custom_field => f)
-
-    v.value = nil
-    assert v.valid?
-    v.value = ''
-    assert v.valid?
-
-    f.is_required = true
-    v.value = nil
-    assert !v.valid?
-    v.value = ''
-    assert !v.valid?
-  end
-
-  def test_string_field_validation_with_min_and_max_lengths
-    f = CustomField.new(:field_format => 'string', :min_length => 2, :max_length => 5)
-    v = CustomValue.new(:custom_field => f, :value => '')
-    assert v.valid?
-    v.value = 'a'
-    assert !v.valid?
-    v.value = 'a' * 2
-    assert v.valid?
-    v.value = 'a' * 6
-    assert !v.valid?
-  end
-
-  def test_string_field_validation_with_regexp
-    f = CustomField.new(:field_format => 'string', :regexp => '^[A-Z0-9]*$')
-    v = CustomValue.new(:custom_field => f, :value => '')
-    assert v.valid?
-    v.value = 'abc'
-    assert !v.valid?
-    v.value = 'ABC'
-    assert v.valid?
-  end
-
-  def test_date_field_validation
-    f = CustomField.new(:field_format => 'date')
-    v = CustomValue.new(:custom_field => f, :value => '')
-    assert v.valid?
-    v.value = 'abc'
-    assert !v.valid?
-    v.value = '1975-07-33'
-    assert !v.valid?
-    v.value = '1975-07-14'
-    assert v.valid?
-  end
-
-  def test_list_field_validation
-    f = CustomField.new(:field_format => 'list', :possible_values => ['value1', 'value2'])
-    v = CustomValue.new(:custom_field => f, :value => '')
-    assert v.valid?
-    v.value = 'abc'
-    assert !v.valid?
-    v.value = 'value2'
-    assert v.valid?
-  end
-
-  def test_int_field_validation
-    f = CustomField.new(:field_format => 'int')
-    v = CustomValue.new(:custom_field => f, :value => '')
-    assert v.valid?
-    v.value = 'abc'
-    assert !v.valid?
-    v.value = '123'
-    assert v.valid?
-    v.value = '+123'
-    assert v.valid?
-    v.value = '-123'
-    assert v.valid?
-  end
-
-  def test_float_field_validation
-    v = CustomValue.new(:customized => User.find(:first), :custom_field => UserCustomField.find_by_name('Money'))
-    v.value = '11.2'
-    assert v.save
-    v.value = ''
-    assert v.save
-    v.value = '-6.250'
-    assert v.save
-    v.value = '6a'
-    assert !v.save
-  end
-
   def test_default_value
     field = CustomField.find_by_default_value('Default string')
     assert_not_nil field
diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb
index b4ba86e49..2a5a14964 100644
--- a/test/unit/issue_test.rb
+++ b/test/unit/issue_test.rb
@@ -1,5 +1,5 @@
 # Redmine - project management software
-# Copyright (C) 2006-2011  Jean-Philippe Lang
+# Copyright (C) 2006-2012  Jean-Philippe Lang
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU General Public License
@@ -29,6 +29,8 @@ class IssueTest < ActiveSupport::TestCase
            :custom_fields, :custom_fields_projects, :custom_fields_trackers, :custom_values,
            :time_entries
 
+  include Redmine::I18n
+
   def test_create
     issue = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 3,
                       :status_id => 1, :priority => IssuePriority.all.first,
@@ -48,6 +50,7 @@ class IssueTest < ActiveSupport::TestCase
   end
 
   def test_create_with_required_custom_field
+    set_language_if_valid 'en'
     field = IssueCustomField.find_by_name('Database')
     field.update_attribute(:is_required, true)
 
@@ -57,18 +60,15 @@ class IssueTest < ActiveSupport::TestCase
     assert issue.available_custom_fields.include?(field)
     # No value for the custom field
     assert !issue.save
-    assert_equal I18n.translate('activerecord.errors.messages.invalid'),
-                 issue.errors[:custom_values].to_s
+    assert_equal "Database can't be blank", issue.errors[:base].to_s
     # Blank value
     issue.custom_field_values = { field.id => '' }
     assert !issue.save
-    assert_equal I18n.translate('activerecord.errors.messages.invalid'),
-                 issue.errors[:custom_values].to_s
+    assert_equal "Database can't be blank", issue.errors[:base].to_s
     # Invalid value
     issue.custom_field_values = { field.id => 'SQLServer' }
     assert !issue.save
-    assert_equal I18n.translate('activerecord.errors.messages.invalid'),
-                 issue.errors[:custom_values].to_s
+    assert_equal "Database is not included in the list", issue.errors[:base].to_s
     # Valid value
     issue.custom_field_values = { field.id => 'PostgreSQL' }
     assert issue.save
@@ -327,8 +327,7 @@ class IssueTest < ActiveSupport::TestCase
     attributes['tracker_id'] = '1'
     issue = Issue.new(:project => Project.find(1))
     issue.attributes = attributes
-    assert_not_nil issue.custom_value_for(1)
-    assert_equal 'MySQL', issue.custom_value_for(1).value
+    assert_equal 'MySQL', issue.custom_field_value(1)
   end
 
   def test_should_update_issue_with_disabled_tracker
diff --git a/test/unit/time_entry_activity_test.rb b/test/unit/time_entry_activity_test.rb
index f376dbe25..bbdc77895 100644
--- a/test/unit/time_entry_activity_test.rb
+++ b/test/unit/time_entry_activity_test.rb
@@ -1,5 +1,5 @@
 # Redmine - project management software
-# Copyright (C) 2006-2011  Jean-Philippe Lang
+# Copyright (C) 2006-2012  Jean-Philippe Lang
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU General Public License
@@ -20,6 +20,8 @@ require File.expand_path('../../test_helper', __FILE__)
 class TimeEntryActivityTest < ActiveSupport::TestCase
   fixtures :enumerations, :time_entries
 
+  include Redmine::I18n
+
   def test_should_be_an_enumeration
     assert TimeEntryActivity.ancestors.include?(Enumeration)
   end
@@ -44,13 +46,13 @@ class TimeEntryActivityTest < ActiveSupport::TestCase
   end
 
   def test_create_without_required_custom_field_should_fail
+    set_language_if_valid 'en'
     field = TimeEntryActivityCustomField.find_by_name('Billable')
     field.update_attribute(:is_required, true)
 
     e = TimeEntryActivity.new(:name => 'Custom Data')
     assert !e.save
-    assert_equal I18n.translate('activerecord.errors.messages.invalid'),
-                 e.errors[:custom_values].to_s
+    assert_equal "Billable can't be blank", e.errors[:base].to_s
   end
 
   def test_create_with_required_custom_field_should_succeed
@@ -62,7 +64,8 @@ class TimeEntryActivityTest < ActiveSupport::TestCase
     assert e.save
   end
 
-  def test_update_issue_with_required_custom_field_change
+  def test_update_with_required_custom_field_change
+    set_language_if_valid 'en'
     field = TimeEntryActivityCustomField.find_by_name('Billable')
     field.update_attribute(:is_required, true)
 
@@ -73,7 +76,7 @@ class TimeEntryActivityTest < ActiveSupport::TestCase
     # Blanking custom field, save should fail
     e.custom_field_values = {field.id => ""}
     assert !e.save
-    assert e.errors[:custom_values]
+    assert_equal "Billable can't be blank", e.errors[:base].to_s
 
     # Update custom field to valid value, save should succeed
     e.custom_field_values = {field.id => "0"}
@@ -81,6 +84,5 @@ class TimeEntryActivityTest < ActiveSupport::TestCase
     e.reload
     assert_equal "0", e.custom_value_for(field).value
   end
-
 end
 
diff --git a/vendor/plugins/acts_as_customizable/lib/acts_as_customizable.rb b/vendor/plugins/acts_as_customizable/lib/acts_as_customizable.rb
index f6c036a1f..943cc441c 100644
--- a/vendor/plugins/acts_as_customizable/lib/acts_as_customizable.rb
+++ b/vendor/plugins/acts_as_customizable/lib/acts_as_customizable.rb
@@ -1,5 +1,5 @@
 # Redmine - project management software
-# Copyright (C) 2006-2011  Jean-Philippe Lang
+# Copyright (C) 2006-2012  Jean-Philippe Lang
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU General Public License
@@ -30,12 +30,10 @@ module Redmine
           has_many :custom_values, :as => :customized,
                                    :include => :custom_field,
                                    :order => "#{CustomField.table_name}.position",
-                                   :dependent => :delete_all
-          before_validation { |customized| customized.custom_field_values if customized.new_record? }
-          # Trigger validation only if custom values were changed
-          validates_associated :custom_values, :on => :update, :if => Proc.new { |customized| customized.custom_field_values_changed? }
+                                   :dependent => :delete_all,
+                                   :validate => false
           send :include, Redmine::Acts::Customizable::InstanceMethods
-          # Save custom values when saving the customized object
+          validate :validate_custom_field_values
           after_save :save_custom_field_values
         end
       end
@@ -66,15 +64,28 @@ module Redmine
         # Sets the values of the object's custom fields
         # values is a hash like {'1' => 'foo', 2 => 'bar'}
         def custom_field_values=(values)
-          @custom_field_values_changed = true
           values = values.stringify_keys
-          custom_field_values.each do |custom_value|
-            custom_value.value = values[custom_value.custom_field_id.to_s] if values.has_key?(custom_value.custom_field_id.to_s)
-          end if values.is_a?(Hash)
+          
+          custom_field_values.each do |custom_field_value|
+            key = custom_field_value.custom_field_id.to_s
+            if values.has_key?(key)
+              value = values[key]
+              custom_field_value.value = value
+            end
+          end
+          @custom_field_values_changed = true
         end
         
         def custom_field_values
-          @custom_field_values ||= available_custom_fields.collect { |x| custom_values.detect { |v| v.custom_field == x } || custom_values.build(:customized => self, :custom_field => x, :value => nil) }
+          @custom_field_values ||= available_custom_fields.collect do |field|
+            x = CustomFieldValue.new
+            x.custom_field = field
+            x.customized = self
+            cv = custom_values.detect { |v| v.custom_field == field }
+            cv ||= custom_values.build(:customized => self, :custom_field => field, :value => nil)
+            x.value = cv.value
+            x
+          end
         end
         
         def visible_custom_field_values
@@ -90,18 +101,34 @@ module Redmine
           custom_values.detect {|v| v.custom_field_id == field_id }
         end
         
+        def custom_field_value(c)
+          field_id = (c.is_a?(CustomField) ? c.id : c.to_i)
+          custom_field_values.detect {|v| v.custom_field_id == field_id }.try(:value)
+        end
+        
+        def validate_custom_field_values
+          if new_record? || custom_field_values_changed?
+            custom_field_values.each(&:validate_value)
+          end
+        end
+        
         def save_custom_field_values
-          self.custom_values = custom_field_values
-          custom_field_values.each(&:save)
+          target_custom_values = []
+          custom_field_values.each do |custom_field_value|
+            target = custom_values.detect {|cv| cv.custom_field == custom_field_value.custom_field}
+            target ||= custom_values.build(:customized => self, :custom_field => custom_field_value.custom_field)
+            target.value = custom_field_value.value
+            target_custom_values << target
+          end
+          self.custom_values = target_custom_values
+          custom_values.each(&:save)
           @custom_field_values_changed = false
-          @custom_field_values = nil
+          true
         end
         
         def reset_custom_values!
           @custom_field_values = nil
           @custom_field_values_changed = true
-          values = custom_values.inject({}) {|h,v| h[v.custom_field_id] = v.value; h}
-          custom_values.each {|cv| cv.destroy unless custom_field_values.include?(cv)}
         end
         
         module ClassMethods