From a77b462a53a02dbead1042bd12060177ade7b22a Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sat, 5 Oct 2013 09:41:11 +0000 Subject: [PATCH] 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 --- app/controllers/settings_controller.rb | 7 +- app/models/changeset.rb | 21 ++---- app/models/setting.rb | 74 ++++++++++++++++++- app/views/settings/_repositories.html.erb | 58 +++++++++++---- config/settings.yml | 10 +-- ...37_support_for_multiple_commit_keywords.rb | 17 +++++ test/functional/settings_controller_test.rb | 51 +++++++++++++ test/unit/changeset_test.rb | 34 +++++++-- test/unit/repository_test.rb | 7 +- 9 files changed, 229 insertions(+), 50 deletions(-) create mode 100644 db/migrate/20131004113137_support_for_multiple_commit_keywords.rb 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 @@

<%= setting_text_field :commit_ref_keywords, :size => 30 %> <%= l(:text_comma_separated) %>

-

<%= setting_text_field :commit_fix_keywords, :size => 30 %> - <%= l(:label_applied_status) %>: <%= setting_select :commit_fix_status_id, - [["", 0]] + - IssueStatus.sorted.all.collect{ - |status| [status.name, status.id.to_s] - }, - :label => false %> - <%= 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 %> -<%= l(:text_comma_separated) %>

-

<%= setting_check_box :commit_cross_project_ref %>

<%= setting_check_box :commit_logtime_enabled, @@ -90,5 +77,48 @@ :disabled => !Setting.commit_logtime_enabled?%>

-<%= submit_tag l(:button_save) %> + + + + + + + + + + + <% @commit_update_keywords.each do |keywords, updates| %> + + + + + + + <% end %> + + + + + + + +
<%= l(:setting_commit_fix_keywords) %><%= l(:label_applied_status) %><%= l(:field_done_ratio) %>
<%= text_field_tag "settings[commit_update_keywords][keywords][]", keywords, :size => 30 %><%= 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']) %><%= select_tag "settings[commit_update_keywords][done_ratio][]", options_for_select([["", ""]] + (0..10).to_a.collect {|r| ["#{r*10} %", "#{r*10}"] }, updates['done_ratio']) %><%= link_to image_tag('delete.png'), '#', :class => 'delete-commit-keywords' %>
<%= l(:text_comma_separated) %><%= link_to image_tag('add.png'), '#', :class => 'add-commit-keywords' %>
+ +

<%= submit_tag l(:button_save) %>

+<% 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