Custom fields refactoring: most of code moved from controllers to models (using new module ActsAsCustomizable).

git-svn-id: http://redmine.rubyforge.org/svn/trunk@1592 e93f8b46-1217-0410-a6f0-8f06a7374b81

5 files changed, 172 insertions, 11 deletions

diff --git a/test/fixtures/custom_fields.yml b/test/fixtures/custom_fields.yml
index 9d88bc6fb..1005edae4 100644
--- a/test/fixtures/custom_fields.yml
+++ b/test/fixtures/custom_fields.yml
@@ -7,7 +7,10 @@ custom_fields_001:
   is_filter: true

   type: IssueCustomField

   max_length: 0

-  possible_values: MySQL|PostgreSQL|Oracle

+  possible_values: 

+  - MySQL

+  - PostgreSQL

+  - Oracle

   id: 1

   is_required: false

   field_format: list

@@ -33,7 +36,11 @@ custom_fields_003:
   is_filter: true

   type: ProjectCustomField

   max_length: 0

-  possible_values: Stable|Beta|Alpha|Planning

+  possible_values: 

+  - Stable

+  - Beta

+  - Alpha

+  - Planning

   id: 3

   is_required: true

   field_format: list

diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index 32d2a7f41..ac1f4f834 100644
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -170,7 +170,7 @@ class IssuesControllerTest < Test::Unit::TestCase
     assert_response :success
     assert_template 'new'
     
-    assert_tag :tag => 'input', :attributes => { :name => 'custom_fields[2]',
+    assert_tag :tag => 'input', :attributes => { :name => 'issue[custom_field_values][2]',
                                                  :value => 'Default string' }
   end
 
@@ -203,8 +203,8 @@ class IssuesControllerTest < Test::Unit::TestCase
                           :subject => 'This is the test_new issue',
                           :description => 'This is the description',
                           :priority_id => 5,
-                          :estimated_hours => ''},
-               :custom_fields => {'2' => 'Value for field 2'}
+                          :estimated_hours => '',
+                          :custom_field_values => {'2' => 'Value for field 2'}}
     assert_redirected_to 'issues/show'
     
     issue = Issue.find_by_subject('This is the test_new issue')
@@ -226,6 +226,50 @@ class IssuesControllerTest < Test::Unit::TestCase
     assert_redirected_to 'issues/show'
   end
   
+  def test_post_new_with_required_custom_field_and_without_custom_fields_param
+    field = IssueCustomField.find_by_name('Database')
+    field.update_attribute(:is_required, true)
+
+    @request.session[:user_id] = 2
+    post :new, :project_id => 1, 
+               :issue => {:tracker_id => 1,
+                          :subject => 'This is the test_new issue',
+                          :description => 'This is the description',
+                          :priority_id => 5}
+    assert_response :success
+    assert_template 'new'
+    issue = assigns(:issue)
+    assert_not_nil issue
+    assert_equal 'activerecord_error_invalid', issue.errors.on(:custom_values)
+  end
+  
+  def test_post_should_preserve_fields_values_on_validation_failure
+    @request.session[:user_id] = 2
+    post :new, :project_id => 1, 
+               :issue => {:tracker_id => 1,
+                          :subject => 'This is the test_new issue',
+                          # empty description
+                          :description => '',
+                          :priority_id => 6,
+                          :custom_field_values => {'1' => 'Oracle', '2' => 'Value for field 2'}}
+    assert_response :success
+    assert_template 'new'
+    
+    assert_tag :input, :attributes => { :name => 'issue[subject]',
+                                        :value => 'This is the test_new issue' }
+    assert_tag :select, :attributes => { :name => 'issue[priority_id]' },
+                        :child => { :tag => 'option', :attributes => { :selected => 'selected',
+                                                                       :value => '6' },
+                                                      :content => 'High' }  
+    # Custom fields
+    assert_tag :select, :attributes => { :name => 'issue[custom_field_values][1]' },
+                        :child => { :tag => 'option', :attributes => { :selected => 'selected',
+                                                                       :value => 'Oracle' },
+                                                      :content => 'Oracle' }  
+    assert_tag :input, :attributes => { :name => 'issue[custom_field_values][2]',
+                                        :value => 'Value for field 2'}
+  end
+  
   def test_copy_issue
     @request.session[:user_id] = 2
     get :new, :project_id => 1, :copy_from => 1
@@ -280,18 +324,28 @@ class IssuesControllerTest < Test::Unit::TestCase
     assert_select_rjs :show, "update"
   end
 
-  def test_post_edit
+  def test_post_edit_without_custom_fields_param
     @request.session[:user_id] = 2
     ActionMailer::Base.deliveries.clear
     
     issue = Issue.find(1)
+    assert_equal '125', issue.custom_value_for(2).value
     old_subject = issue.subject
     new_subject = 'Subject modified by IssuesControllerTest#test_post_edit'
     
-    post :edit, :id => 1, :issue => {:subject => new_subject}
+    assert_difference('Journal.count') do
+      assert_difference('JournalDetail.count', 2) do
+        post :edit, :id => 1, :issue => {:subject => new_subject,
+                                         :priority_id => '6',
+                                         :category_id => '1' # no change
+                                        }
+      end
+    end
     assert_redirected_to 'issues/show/1'
     issue.reload
     assert_equal new_subject, issue.subject
+    # Make sure custom fields were not cleared
+    assert_equal '125', issue.custom_value_for(2).value
     
     mail = ActionMailer::Base.deliveries.last
     assert_kind_of TMail::Mail, mail
@@ -299,6 +353,29 @@ class IssuesControllerTest < Test::Unit::TestCase
     assert mail.body.include?("Subject changed from #{old_subject} to #{new_subject}")
   end
   
+  def test_post_edit_with_custom_field_change
+    @request.session[:user_id] = 2
+    issue = Issue.find(1)
+    assert_equal '125', issue.custom_value_for(2).value
+    
+    assert_difference('Journal.count') do
+      assert_difference('JournalDetail.count', 3) do
+        post :edit, :id => 1, :issue => {:subject => 'Custom field change',
+                                         :priority_id => '6',
+                                         :category_id => '1', # no change
+                                         :custom_field_values => { '2' => 'New custom value' }
+                                        }
+      end
+    end
+    assert_redirected_to 'issues/show/1'
+    issue.reload
+    assert_equal 'New custom value', issue.custom_value_for(2).value
+    
+    mail = ActionMailer::Base.deliveries.last
+    assert_kind_of TMail::Mail, mail
+    assert mail.body.include?("Searchable field changed from 125 to New custom value")
+  end
+  
   def test_post_edit_with_status_and_assignee_change
     issue = Issue.find(1)
     assert_equal 1, issue.status_id
diff --git a/test/functional/projects_controller_test.rb b/test/functional/projects_controller_test.rb
index 5b7c29b18..88c0319ba 100644
--- a/test/functional/projects_controller_test.rb
+++ b/test/functional/projects_controller_test.rb
@@ -83,7 +83,7 @@ class ProjectsControllerTest < Test::Unit::TestCase
   def test_edit
     @request.session[:user_id] = 2 # manager
     post :edit, :id => 1, :project => {:name => 'Test changed name',
-                                       :custom_field_ids => ['']}
+                                       :issue_custom_field_ids => ['']}
     assert_redirected_to 'projects/settings/ecookbook'
     project = Project.find(1)
     assert_equal 'Test changed name', project.name
diff --git a/test/integration/admin_test.rb b/test/integration/admin_test.rb
index a424247cc..6e385873e 100644
--- a/test/integration/admin_test.rb
+++ b/test/integration/admin_test.rb
@@ -48,8 +48,9 @@ class AdminTest < ActionController::IntegrationTest
     post "projects/add", :project => { :name => "blog", 
                                        :description => "weblog",
                                        :identifier => "blog",
-                                       :is_public => 1 },
-                         'custom_fields[3]' => 'Beta'
+                                       :is_public => 1,
+                                       :custom_field_values => { '3' => 'Beta' }
+                                       }
     assert_redirected_to "admin/projects"
     assert_equal 'Successful creation.', flash[:notice]
     
diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb
index 999c4480d..0d98f89d2 100644
--- a/test/unit/issue_test.rb
+++ b/test/unit/issue_test.rb
@@ -18,7 +18,13 @@
 require File.dirname(__FILE__) + '/../test_helper'
 
 class IssueTest < Test::Unit::TestCase
-  fixtures :projects, :users, :members, :trackers, :projects_trackers, :issue_statuses, :issue_categories, :enumerations, :issues, :custom_fields, :custom_values, :time_entries
+  fixtures :projects, :users, :members,
+           :trackers, :projects_trackers,
+           :issue_statuses, :issue_categories,
+           :enumerations,
+           :issues,
+           :custom_fields, :custom_fields_projects, :custom_fields_trackers, :custom_values,
+           :time_entries
 
   def test_create
     issue = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 3, :status_id => 1, :priority => Enumeration.get_values('IPRI').first, :subject => 'test_create', :description => 'IssueTest#test_create', :estimated_hours => '1:30')
@@ -27,6 +33,76 @@ class IssueTest < Test::Unit::TestCase
     assert_equal 1.5, issue.estimated_hours
   end
   
+  def test_create_with_required_custom_field
+    field = IssueCustomField.find_by_name('Database')
+    field.update_attribute(:is_required, true)
+    
+    issue = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 1, :status_id => 1, :subject => 'test_create', :description => 'IssueTest#test_create_with_required_custom_field')
+    assert issue.available_custom_fields.include?(field)
+    # No value for the custom field
+    assert !issue.save
+    assert_equal 'activerecord_error_invalid', issue.errors.on(:custom_values)
+    # Blank value
+    issue.custom_field_values = { field.id => '' }
+    assert !issue.save
+    assert_equal 'activerecord_error_invalid', issue.errors.on(:custom_values)
+    # Invalid value
+    issue.custom_field_values = { field.id => 'SQLServer' }
+    assert !issue.save
+    assert_equal 'activerecord_error_invalid', issue.errors.on(:custom_values)
+    # Valid value
+    issue.custom_field_values = { field.id => 'PostgreSQL' }
+    assert issue.save
+    issue.reload
+    assert_equal 'PostgreSQL', issue.custom_value_for(field).value
+  end
+  
+  def test_update_issue_with_required_custom_field
+    field = IssueCustomField.find_by_name('Database')
+    field.update_attribute(:is_required, true)
+    
+    issue = Issue.find(1)
+    assert_nil issue.custom_value_for(field)
+    assert issue.available_custom_fields.include?(field)
+    # No change to custom values, issue can be saved
+    assert issue.save
+    # Blank value
+    issue.custom_field_values = { field.id => '' }
+    assert !issue.save
+    # Valid value
+    issue.custom_field_values = { field.id => 'PostgreSQL' }
+    assert issue.save
+    issue.reload
+    assert_equal 'PostgreSQL', issue.custom_value_for(field).value
+  end
+  
+  def test_should_not_update_attributes_if_custom_fields_validation_fails
+    issue = Issue.find(1)
+    field = IssueCustomField.find_by_name('Database')
+    assert issue.available_custom_fields.include?(field)
+    
+    issue.custom_field_values = { field.id => 'Invalid' }
+    issue.subject = 'Should be not be saved'
+    assert !issue.save
+    
+    issue.reload
+    assert_equal "Can't print recipes", issue.subject
+  end
+  
+  def test_should_not_recreate_custom_values_objects_on_update
+    field = IssueCustomField.find_by_name('Database')
+    
+    issue = Issue.find(1)
+    issue.custom_field_values = { field.id => 'PostgreSQL' }
+    assert issue.save
+    custom_value = issue.custom_value_for(field)
+    issue.reload
+    issue.custom_field_values = { field.id => 'MySQL' }
+    assert issue.save
+    issue.reload
+    assert_equal custom_value.id, issue.custom_value_for(field).id
+  end
+  
   def test_category_based_assignment
     issue = Issue.create(:project_id => 1, :tracker_id => 1, :author_id => 3, :status_id => 1, :priority => Enumeration.get_values('IPRI').first, :subject => 'Assignment test', :description => 'Assignment test', :category_id => 1)
     assert_equal IssueCategory.find(1).assigned_to, issue.assigned_to