Support for multiple issue update keywords/rules in commit messages (#4911).

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

9 files changed, 229 insertions, 50 deletions

diff --git a/app/controllers/settings_controller.rb b/app/controllers/settings_controller.rb
index 586c23956..05550bc65 100644
--- a/app/controllers/settings_controller.rb
+++ b/app/controllers/settings_controller.rb
@@ -33,9 +33,7 @@ class SettingsController < ApplicationController
     if request.post? && params[:settings] && params[:settings].is_a?(Hash)
       settings = (params[:settings] || {}).dup.symbolize_keys
       settings.each do |name, value|
-        # remove blank values in array settings
-        value.delete_if {|v| v.blank? } if value.is_a?(Array)
-        Setting[name] = value
+        Setting.set_from_params name, value
       end
       flash[:notice] = l(:notice_successful_update)
       redirect_to settings_path(:tab => params[:tab])
@@ -48,6 +46,9 @@ class SettingsController < ApplicationController
       @guessed_host_and_path = request.host_with_port.dup
       @guessed_host_and_path << ('/'+ Redmine::Utils.relative_url_root.gsub(%r{^\/}, '')) unless Redmine::Utils.relative_url_root.blank?
 
+      @commit_update_keywords = Setting.commit_update_keywords.dup
+      @commit_update_keywords[''] = {} if @commit_update_keywords.blank?
+
       Redmine::Themes.rescan
     end
   end
diff --git a/app/models/changeset.rb b/app/models/changeset.rb
index d3fa8ab2b..7920ae078 100644
--- a/app/models/changeset.rb
+++ b/app/models/changeset.rb
@@ -118,21 +118,21 @@ class Changeset < ActiveRecord::Base
     ref_keywords = Setting.commit_ref_keywords.downcase.split(",").collect(&:strip)
     ref_keywords_any = ref_keywords.delete('*')
     # keywords used to fix issues
-    fix_keywords = Setting.commit_fix_keywords.downcase.split(",").collect(&:strip)
+    fix_keywords = Setting.commit_update_by_keyword.keys
 
     kw_regexp = (ref_keywords + fix_keywords).collect{|kw| Regexp.escape(kw)}.join("|")
 
     referenced_issues = []
 
     comments.scan(/([\s\(\[,-]|^)((#{kw_regexp})[\s:]+)?(#\d+(\s+@#{TIMELOG_RE})?([\s,;&]+#\d+(\s+@#{TIMELOG_RE})?)*)(?=[[:punct:]]|\s|<|$)/i) do |match|
-      action, refs = match[2], match[3]
+      action, refs = match[2].to_s.downcase, match[3]
       next unless action.present? || ref_keywords_any
 
       refs.scan(/#(\d+)(\s+@#{TIMELOG_RE})?/).each do |m|
         issue, hours = find_referenced_issue_by_id(m[0].to_i), m[2]
         if issue
           referenced_issues << issue
-          fix_issue(issue) if fix_keywords.include?(action.to_s.downcase)
+          fix_issue(issue, action) if fix_keywords.include?(action)
           log_time(issue, hours) if hours && Setting.commit_logtime_enabled?
         end
       end
@@ -210,12 +210,10 @@ class Changeset < ActiveRecord::Base
 
   private
 
-  def fix_issue(issue)
-    status = IssueStatus.find_by_id(Setting.commit_fix_status_id.to_i)
-    if status.nil?
-      logger.warn("No status matches commit_fix_status_id setting (#{Setting.commit_fix_status_id})") if logger
-      return issue
-    end
+  # Updates the +issue+ according to +action+
+  def fix_issue(issue, action)
+    updates = Setting.commit_update_by_keyword[action]
+    return unless updates.is_a?(Hash)
 
     # the issue may have been updated by the closure of another one (eg. duplicate)
     issue.reload
@@ -223,10 +221,7 @@ class Changeset < ActiveRecord::Base
     return if issue.status && issue.status.is_closed?
 
     journal = issue.init_journal(user || User.anonymous, ll(Setting.default_language, :text_status_changed_by_changeset, text_tag(issue.project)))
-    issue.status = status
-    unless Setting.commit_fix_done_ratio.blank?
-      issue.done_ratio = Setting.commit_fix_done_ratio.to_i
-    end
+    issue.assign_attributes updates.slice(*Issue.attribute_names)
     Redmine::Hook.call_hook(:model_changeset_scan_commit_for_issue_ids_pre_issue_update,
                             { :changeset => self, :issue => issue })
     unless issue.save
diff --git a/app/models/setting.rb b/app/models/setting.rb
index a0ab18b5b..71ef15208 100644
--- a/app/models/setting.rb
+++ b/app/models/setting.rb
@@ -132,15 +132,87 @@ class Setting < ActiveRecord::Base
     def self.#{name}=(value)
       self[:#{name}] = value
     end
-    END_SRC
+END_SRC
     class_eval src, __FILE__, __LINE__
   end
 
+  # Sets a setting value from params
+  def self.set_from_params(name, params)
+    params = params.dup
+    params.delete_if {|v| v.blank? } if params.is_a?(Array)
+
+    m = "#{name}_from_params"
+    if respond_to? m
+      self[name.to_sym] = send m, params
+    else
+      self[name.to_sym] = params
+    end
+  end
+
+  # Returns a hash suitable for commit_update_keywords setting
+  #
+  # Example:
+  # params = {:keywords => ['fixes', 'closes'], :status_id => ["3", "5"], :done_ratio => ["", "100"]}
+  # Setting.commit_update_keywords_from_params(params)
+  # # => {'fixes' => {'status_id' => "3"}, 'closes' => {'status_id' => "5", 'done_ratio' => "100"}}
+  def self.commit_update_keywords_from_params(params)
+    s = {}
+    if params.is_a?(Hash) && params.key?(:keywords) && params.values.all? {|v| v.is_a? Array}
+      attributes = params.except(:keywords).keys
+      params[:keywords].each_with_index do |keywords, i|
+        next if keywords.blank?
+        s[keywords] = attributes.inject({}) {|h, a|
+          value = params[a][i].to_s
+          h[a.to_s] = value if value.present?
+          h
+        }
+      end
+    end
+    s
+  end
+
   # Helper that returns an array based on per_page_options setting
   def self.per_page_options_array
     per_page_options.split(%r{[\s,]}).collect(&:to_i).select {|n| n > 0}.sort
   end
 
+  # Helper that returns a Hash with single update keywords as keys
+  def self.commit_update_by_keyword
+    h = {}
+    if commit_update_keywords.is_a?(Hash)
+      commit_update_keywords.each do |keywords, attribute_updates|
+        next unless attribute_updates.is_a?(Hash)
+        attribute_updates = attribute_updates.dup
+        attribute_updates.delete_if {|k, v| v.blank?}
+        keywords.to_s.split(",").map(&:strip).reject(&:blank?).each do |keyword|
+          h[keyword.downcase] = attribute_updates
+        end
+      end
+    end
+    h
+  end
+
+  def self.commit_fix_keywords
+     ActiveSupport::Deprecation.warn "Setting.commit_fix_keywords is deprecated and will be removed in Redmine 3"
+    if commit_update_keywords.is_a?(Hash)
+      commit_update_keywords.keys.first
+    end
+  end
+
+  def self.commit_fix_status_id
+     ActiveSupport::Deprecation.warn "Setting.commit_fix_status_id is deprecated and will be removed in Redmine 3"
+    if commit_update_keywords.is_a?(Hash)
+      commit_update_keywords[commit_fix_keywords]['status_id']
+    end
+  end
+
+  def self.commit_fix_done_ratio
+     ActiveSupport::Deprecation.warn "Setting.commit_fix_done_ratio is deprecated and will be removed in Redmine 3"
+    if commit_update_keywords.is_a?(Hash)
+      commit_update_keywords[commit_fix_keywords]['done_ratio']
+    end
+  end
+
   def self.openid?
     Object.const_defined?(:OpenID) && self[:openid].to_i > 0
   end
diff --git a/app/views/settings/_repositories.html.erb b/app/views/settings/_repositories.html.erb
index 3341de313..d90c212ec 100644
--- a/app/views/settings/_repositories.html.erb
+++ b/app/views/settings/_repositories.html.erb
@@ -65,19 +65,6 @@
 <p><%= setting_text_field :commit_ref_keywords, :size => 30 %>
 <em class="info"><%= l(:text_comma_separated) %></em></p>
 
-<p><%= setting_text_field :commit_fix_keywords, :size => 30 %>
-&nbsp;<%= l(:label_applied_status) %>: <%= setting_select :commit_fix_status_id,
-                                                          [["", 0]] +
-                                                              IssueStatus.sorted.all.collect{
-                                                                 |status| [status.name, status.id.to_s]
-                                                              },
-                                                          :label => false %>
-&nbsp;<%= l(:field_done_ratio) %>: <%= setting_select :commit_fix_done_ratio,
-                                                       (0..10).to_a.collect {|r| ["#{r*10} %", "#{r*10}"] },
-                                                       :blank => :label_no_change_option,
-                                                       :label => false %>
-<em class="info"><%= l(:text_comma_separated) %></em></p>
-
 <p><%= setting_check_box :commit_cross_project_ref %></p>
 
 <p><%= setting_check_box :commit_logtime_enabled,
@@ -90,5 +77,48 @@
                       :disabled => !Setting.commit_logtime_enabled?%></p>
 </fieldset>
 
-<%= submit_tag l(:button_save) %>
+<table class="list" id="commit-keywords">
+  <thead>
+    <tr>
+      <th><%= l(:setting_commit_fix_keywords) %></th>
+      <th><%= l(:label_applied_status) %></th>
+      <th><%= l(:field_done_ratio) %></th>
+      <th class="buttons"></th>
+    </tr>
+  </thead>
+  <tbody>
+    <% @commit_update_keywords.each do |keywords, updates| %>
+    <tr class="commit-keywords">
+      <td><%= text_field_tag "settings[commit_update_keywords][keywords][]", keywords, :size => 30 %></td>
+      <td><%= select_tag "settings[commit_update_keywords][status_id][]", options_for_select([["", 0]] + IssueStatus.sorted.all.collect{|status| [status.name, status.id.to_s]}, updates['status_id']) %></td>
+      <td><%= select_tag "settings[commit_update_keywords][done_ratio][]", options_for_select([["", ""]] + (0..10).to_a.collect {|r| ["#{r*10} %", "#{r*10}"] }, updates['done_ratio']) %></td>
+      <td class="buttons"><%= link_to image_tag('delete.png'), '#', :class => 'delete-commit-keywords' %></td>
+    </tr>
+    <% end %>
+    <tr>
+      <td><em class="info"><%= l(:text_comma_separated) %></em></td>
+      <td></td>
+      <td></td>
+      <td class="buttons"><%= link_to image_tag('add.png'), '#', :class => 'add-commit-keywords' %></td>
+    </tr>
+  </tbody>
+</table>
+
+<p><%= submit_tag l(:button_save) %></p>
+<% end %>
+
+<%= javascript_tag do %>
+$('#commit-keywords').on('click', 'a.delete-commit-keywords', function(e){
+  e.preventDefault();
+  if ($('#commit-keywords tbody tr.commit-keywords').length > 1) {
+    $(this).parents('#commit-keywords tr').remove();
+  } else {
+    $('#commit-keywords tbody tr.commit-keywords').find('input, select').val('');
+  }
+});
+$('#commit-keywords').on('click', 'a.add-commit-keywords', function(e){
+  e.preventDefault();
+  var row = $('#commit-keywords tr.commit-keywords:last');
+  row.clone().insertAfter(row).find('input, select').val('');
+});
 <% end %>
diff --git a/config/settings.yml b/config/settings.yml
index 61c24c798..05779accc 100644
--- a/config/settings.yml
+++ b/config/settings.yml
@@ -106,13 +106,9 @@ commit_cross_project_ref:
   default: 0
 commit_ref_keywords:
   default: 'refs,references,IssueID'
-commit_fix_keywords:
-  default: 'fixes,closes'
-commit_fix_status_id:
-  format: int
-  default: 0
-commit_fix_done_ratio:
-  default: 100
+commit_update_keywords:
+  serialized: true
+  default: {}
 commit_logtime_enabled:
   default: 0
 commit_logtime_activity_id:
diff --git a/db/migrate/20131004113137_support_for_multiple_commit_keywords.rb b/db/migrate/20131004113137_support_for_multiple_commit_keywords.rb
new file mode 100644
index 000000000..77989d9f3
--- /dev/null
+++ b/db/migrate/20131004113137_support_for_multiple_commit_keywords.rb
@@ -0,0 +1,17 @@
+class SupportForMultipleCommitKeywords < ActiveRecord::Migration
+  def up
+    # Replaces commit_fix_keywords, commit_fix_status_id, commit_fix_done_ratio settings
+    # with commit_update_keywords setting
+    keywords = Setting.where(:name => 'commit_fix_keywords').limit(1).pluck(:value).first
+    status_id = Setting.where(:name => 'commit_fix_status_id').limit(1).pluck(:value).first
+    done_ratio = Setting.where(:name => 'commit_fix_done_ratio').limit(1).pluck(:value).first
+    if keywords.present?
+      Setting.commit_update_keywords = {keywords => {'status_id' => status_id, 'done_ratio' => done_ratio}}
+    end
+    Setting.where(:name => %w(commit_fix_keywords commit_fix_status_id commit_fix_done_ratio)).delete_all
+  end
+
+  def down
+    Setting.where(:name => 'commit_update_keywords').delete_all
+  end
+end
diff --git a/test/functional/settings_controller_test.rb b/test/functional/settings_controller_test.rb
index df4abd6b7..57dd629b2 100644
--- a/test/functional/settings_controller_test.rb
+++ b/test/functional/settings_controller_test.rb
@@ -80,6 +80,57 @@ class SettingsControllerTest < ActionController::TestCase
     Setting.clear_cache
   end
 
+  def test_edit_commit_update_keywords
+    with_settings :commit_update_keywords => {
+      "fixes, resolves" => {"status_id" => "3"},
+      "closes" => {"status_id" => "5", "done_ratio" => "100"}
+    } do
+      get :edit
+    end
+    assert_response :success
+    assert_select 'tr.commit-keywords', 2
+    assert_select 'tr.commit-keywords:nth-child(1)' do
+      assert_select 'input[name=?][value=?]', 'settings[commit_update_keywords][keywords][]', 'fixes, resolves'
+      assert_select 'select[name=?]', 'settings[commit_update_keywords][status_id][]' do
+        assert_select 'option[value=3][selected=selected]'
+      end
+    end
+    assert_select 'tr.commit-keywords:nth-child(2)' do
+      assert_select 'input[name=?][value=?]', 'settings[commit_update_keywords][keywords][]', 'closes'
+      assert_select 'select[name=?]', 'settings[commit_update_keywords][status_id][]' do
+        assert_select 'option[value=5][selected=selected]'
+      end
+      assert_select 'select[name=?]', 'settings[commit_update_keywords][done_ratio][]' do
+        assert_select 'option[value=100][selected=selected]'
+      end
+    end
+  end
+
+  def test_edit_without_commit_update_keywords_should_show_blank_line
+    with_settings :commit_update_keywords => {} do
+      get :edit
+    end
+    assert_response :success
+    assert_select 'tr.commit-keywords', 1 do
+      assert_select 'input[name=?][value=?]', 'settings[commit_update_keywords][keywords][]', ''
+    end
+  end
+
+  def test_post_edit_commit_update_keywords
+    post :edit, :settings => {
+      :commit_update_keywords => {
+        :keywords => ["resolves", "closes"],
+        :status_id => ["3", "5"],
+        :done_ratio => ["", "100"]
+      }
+    }
+    assert_redirected_to '/settings'
+    assert_equal({
+      "resolves" => {"status_id" => "3"},
+      "closes" => {"status_id" => "5", "done_ratio" => "100"}
+    }, Setting.commit_update_keywords)
+  end
+
   def test_get_plugin_settings
     Setting.stubs(:plugin_foo).returns({'sample_setting' => 'Plugin setting value'})
     ActionController::Base.append_view_path(File.join(Rails.root, "test/fixtures/plugins"))
diff --git a/test/unit/changeset_test.rb b/test/unit/changeset_test.rb
index a97dc37d2..3ead6b843 100644
--- a/test/unit/changeset_test.rb
+++ b/test/unit/changeset_test.rb
@@ -30,10 +30,8 @@ class ChangesetTest < ActiveSupport::TestCase
 
   def test_ref_keywords_any
     ActionMailer::Base.deliveries.clear
-    Setting.commit_fix_status_id = IssueStatus.where(:is_closed => true).first.id
-    Setting.commit_fix_done_ratio = '90'
     Setting.commit_ref_keywords = '*'
-    Setting.commit_fix_keywords = 'fixes , closes'
+    Setting.commit_update_keywords = {'fixes , closes' => {'status_id' => '5', 'done_ratio' => '90'}}
 
     c = Changeset.new(:repository   => Project.find(1).repository,
                       :committed_on => Time.now,
@@ -49,7 +47,7 @@ class ChangesetTest < ActiveSupport::TestCase
 
   def test_ref_keywords
     Setting.commit_ref_keywords = 'refs'
-    Setting.commit_fix_keywords = ''
+    Setting.commit_update_keywords = ''
     c = Changeset.new(:repository   => Project.find(1).repository,
                       :committed_on => Time.now,
                       :comments     => 'Ignores #2. Refs #1',
@@ -60,7 +58,7 @@ class ChangesetTest < ActiveSupport::TestCase
 
   def test_ref_keywords_any_only
     Setting.commit_ref_keywords = '*'
-    Setting.commit_fix_keywords = ''
+    Setting.commit_update_keywords = ''
     c = Changeset.new(:repository   => Project.find(1).repository,
                       :committed_on => Time.now,
                       :comments     => 'Ignores #2. Refs #1',
@@ -112,9 +110,12 @@ class ChangesetTest < ActiveSupport::TestCase
   end
 
   def test_ref_keywords_closing_with_timelog
-    Setting.commit_fix_status_id = IssueStatus.where(:is_closed => true).first.id
     Setting.commit_ref_keywords = '*'
-    Setting.commit_fix_keywords = 'fixes , closes'
+    Setting.commit_update_keywords = {
+      'fixes , closes' => {
+        'status_id' => IssueStatus.where(:is_closed => true).first.id.to_s
+      }
+    }
     Setting.commit_logtime_enabled = '1'
 
     c = Changeset.new(:repository   => Project.find(1).repository,
@@ -163,6 +164,23 @@ class ChangesetTest < ActiveSupport::TestCase
     assert_equal [1,2,3], c.issue_ids.sort
   end
 
+  def test_update_keywords_with_multiple_rules
+    Setting.commit_update_keywords = {
+      'fixes, closes' => {'status_id' => '5'},
+      'resolves' => {'status_id' => '3'}
+    }
+    issue1 = Issue.generate!
+    issue2 = Issue.generate!
+
+    c = Changeset.new(:repository   => Project.find(1).repository,
+                      :committed_on => Time.now,
+                      :comments     => "Closes ##{issue1.id}\nResolves ##{issue2.id}",
+                      :revision     => '12345')
+    assert c.save
+    assert_equal 5, issue1.reload.status_id
+    assert_equal 3, issue2.reload.status_id
+  end
+
   def test_commit_referencing_a_subproject_issue
     c = Changeset.new(:repository   => Project.find(1).repository,
                       :committed_on => Time.now,
@@ -174,7 +192,7 @@ class ChangesetTest < ActiveSupport::TestCase
   end
 
   def test_commit_closing_a_subproject_issue
-    with_settings :commit_fix_status_id => 5, :commit_fix_keywords => 'closes',
+    with_settings :commit_update_keywords => {'closes' => {'status_id' => '5'}},
                   :default_language => 'en' do
       issue = Issue.find(5)
       assert !issue.closed?
diff --git a/test/unit/repository_test.rb b/test/unit/repository_test.rb
index 390a5334e..3ff9fc51e 100644
--- a/test/unit/repository_test.rb
+++ b/test/unit/repository_test.rb
@@ -182,11 +182,10 @@ class RepositoryTest < ActiveSupport::TestCase
   def test_scan_changesets_for_issue_ids
     Setting.default_language = 'en'
 
-    # choosing a status to apply to fix issues
-    Setting.commit_fix_status_id = IssueStatus.where(:is_closed => true).first.id
-    Setting.commit_fix_done_ratio = "90"
     Setting.commit_ref_keywords = 'refs , references, IssueID'
-    Setting.commit_fix_keywords = 'fixes , closes'
+    Setting.commit_update_keywords = {
+      'fixes , closes' => {'status_id' => IssueStatus.where(:is_closed => true).first.id, 'done_ratio' => '90'}
+    }
     Setting.default_language = 'en'
     ActionMailer::Base.deliveries.clear