From d3425debff4caf091712b299beeaab6e19341b4e Mon Sep 17 00:00:00 2001 From: fkdkent Date: Sat, 22 Aug 2020 16:39:04 +0900 Subject: [PATCH 01/11] Add tests Written by Juno NISHIZAKI and Mizuki ISHIKAWA at Redmine Patch Party. --- test/unit/issue_test.rb | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index 188230022..621438e1f 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -2921,6 +2921,36 @@ def test_saving_twice_should_not_duplicate_journal_details end end + # done_ratio should change to 100 when the status is closed and Setting.issue_done_ratio equal issue_field_and_closed_status + test "when updating to closed status should update done_ratio according to Setting.issue_done_ratio" do + issue_status = IssueStatus.find(5) + issue_status.update!(:default_done_ratio => 90) + + with_settings :issue_done_ratio => 'issue_field' do + issue = Issue.generate!(:status_id => 1, :done_ratio => 30) + issue.status = issue_status + issue.save! + + assert_equal 30, issue.read_attribute(:done_ratio) + end + + with_settings :issue_done_ratio => 'issue_status' do + issue = Issue.generate!(:status_id => 1, :done_ratio => 30) + issue.status = issue_status + issue.save! + + assert_equal 90, issue.read_attribute(:done_ratio) + end + + with_settings :issue_done_ratio => 'issue_field_and_closed_status' do + issue = Issue.generate!(:status_id => 1, :done_ratio => 30) + issue.status = issue_status + issue.save! + + assert_equal 100, issue.read_attribute(:done_ratio) + end + end + test "#by_tracker" do User.current = User.find(2) groups = Issue.by_tracker(Project.find(1)) From d90a5271f68f54ef1a8094d438c1b7c9d158655e Mon Sep 17 00:00:00 2001 From: fkdkent Date: Mon, 21 Sep 2020 15:01:29 +0900 Subject: [PATCH 02/11] Implement --- app/models/issue.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/models/issue.rb b/app/models/issue.rb index d6d6c7c8b..e261227ff 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -735,6 +735,8 @@ def workflow_rule_by_attribute(user=nil) def done_ratio if Issue.use_status_for_done_ratio? && status && status.default_done_ratio status.default_done_ratio + elsif Issue.use_issue_field_and_closed_status_for_done_ratio? && status && status.is_closed? + 100 else read_attribute(:done_ratio) end @@ -748,6 +750,10 @@ def self.use_field_for_done_ratio? Setting.issue_done_ratio == 'issue_field' end + def self.use_issue_field_and_closed_status_for_done_ratio? + Setting.issue_done_ratio == 'issue_field_and_closed_status' + end + def validate_issue if due_date && start_date && (start_date_changed? || due_date_changed?) && due_date < start_date errors.add :due_date, :greater_than_start_date @@ -860,6 +866,8 @@ def validate_custom_field_values def update_done_ratio_from_issue_status if Issue.use_status_for_done_ratio? && status && status.default_done_ratio self.done_ratio = status.default_done_ratio + elsif Issue.use_issue_field_and_closed_status_for_done_ratio? && status && status.is_closed? + self.done_ratio = 100 end end From aa1a63f2ef054cb8b770df0c52dbe5aae94b382f Mon Sep 17 00:00:00 2001 From: fkdkent Date: Mon, 21 Sep 2020 15:11:46 +0900 Subject: [PATCH 03/11] Clarify variable name --- test/unit/issue_test.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index 621438e1f..4b8fbf427 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -2923,12 +2923,12 @@ def test_saving_twice_should_not_duplicate_journal_details # done_ratio should change to 100 when the status is closed and Setting.issue_done_ratio equal issue_field_and_closed_status test "when updating to closed status should update done_ratio according to Setting.issue_done_ratio" do - issue_status = IssueStatus.find(5) - issue_status.update!(:default_done_ratio => 90) + closed_status = IssueStatus.find(5) + closed_status.update!(:default_done_ratio => 90) with_settings :issue_done_ratio => 'issue_field' do issue = Issue.generate!(:status_id => 1, :done_ratio => 30) - issue.status = issue_status + issue.status = closed_status issue.save! assert_equal 30, issue.read_attribute(:done_ratio) @@ -2936,7 +2936,7 @@ def test_saving_twice_should_not_duplicate_journal_details with_settings :issue_done_ratio => 'issue_status' do issue = Issue.generate!(:status_id => 1, :done_ratio => 30) - issue.status = issue_status + issue.status = closed_status issue.save! assert_equal 90, issue.read_attribute(:done_ratio) @@ -2944,7 +2944,7 @@ def test_saving_twice_should_not_duplicate_journal_details with_settings :issue_done_ratio => 'issue_field_and_closed_status' do issue = Issue.generate!(:status_id => 1, :done_ratio => 30) - issue.status = issue_status + issue.status = closed_status issue.save! assert_equal 100, issue.read_attribute(:done_ratio) From 9f8babe4d19740b4b564c2df75b6d6bb01f84cc7 Mon Sep 17 00:00:00 2001 From: fkdkent Date: Mon, 21 Sep 2020 15:12:36 +0900 Subject: [PATCH 04/11] Use update --- test/unit/issue_test.rb | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index 4b8fbf427..2df33a5a3 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -2928,24 +2928,21 @@ def test_saving_twice_should_not_duplicate_journal_details with_settings :issue_done_ratio => 'issue_field' do issue = Issue.generate!(:status_id => 1, :done_ratio => 30) - issue.status = closed_status - issue.save! + issue.update!(:status => closed_status) assert_equal 30, issue.read_attribute(:done_ratio) end with_settings :issue_done_ratio => 'issue_status' do issue = Issue.generate!(:status_id => 1, :done_ratio => 30) - issue.status = closed_status - issue.save! + issue.update!(:status => closed_status) assert_equal 90, issue.read_attribute(:done_ratio) end with_settings :issue_done_ratio => 'issue_field_and_closed_status' do issue = Issue.generate!(:status_id => 1, :done_ratio => 30) - issue.status = closed_status - issue.save! + issue.update!(:status => closed_status) assert_equal 100, issue.read_attribute(:done_ratio) end From 143911ae6c8c0f0afd6c715c743403b80a1ae036 Mon Sep 17 00:00:00 2001 From: fkdkent Date: Mon, 21 Sep 2020 15:22:51 +0900 Subject: [PATCH 05/11] Add tests --- test/unit/issue_test.rb | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index 2df33a5a3..6c6d25e4b 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -2948,6 +2948,29 @@ def test_saving_twice_should_not_duplicate_journal_details end end + test "done ratio of existing issues follows the new issue_done_ratio setting" do + closed_status = IssueStatus.find(5) + closed_status.update!(:default_done_ratio => 90) + + issue = Issue.generate!(:status_id => 1, :done_ratio => 30) + issue.update!(:status => closed_status) + + with_settings :issue_done_ratio => 'issue_status' do + assert_equal 90, issue.done_ratio + assert_equal 30, issue.read_attribute(:done_ratio) + end + + with_settings :issue_done_ratio => 'issue_field_and_closed_status' do + assert_equal 100, issue.done_ratio + assert_equal 30, issue.read_attribute(:done_ratio) + end + + with_settings :issue_done_ratio => 'issue_field' do + assert_equal 30, issue.done_ratio + assert_equal 30, issue.read_attribute(:done_ratio) + end + end + test "#by_tracker" do User.current = User.find(2) groups = Issue.by_tracker(Project.find(1)) From 6baf8c399daf1f9306840dd73d7da59d31133689 Mon Sep 17 00:00:00 2001 From: fkdkent Date: Mon, 21 Sep 2020 15:28:51 +0900 Subject: [PATCH 06/11] Add assertions --- test/unit/issue_test.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index 6c6d25e4b..9f53c9494 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -2930,6 +2930,7 @@ def test_saving_twice_should_not_duplicate_journal_details issue = Issue.generate!(:status_id => 1, :done_ratio => 30) issue.update!(:status => closed_status) + assert_equal 30, issue.done_ratio assert_equal 30, issue.read_attribute(:done_ratio) end @@ -2937,6 +2938,7 @@ def test_saving_twice_should_not_duplicate_journal_details issue = Issue.generate!(:status_id => 1, :done_ratio => 30) issue.update!(:status => closed_status) + assert_equal 90, issue.done_ratio assert_equal 90, issue.read_attribute(:done_ratio) end @@ -2944,6 +2946,7 @@ def test_saving_twice_should_not_duplicate_journal_details issue = Issue.generate!(:status_id => 1, :done_ratio => 30) issue.update!(:status => closed_status) + assert_equal 100, issue.done_ratio assert_equal 100, issue.read_attribute(:done_ratio) end end From 04c67e997499bda06c1c16194394651d6f137612 Mon Sep 17 00:00:00 2001 From: fkdkent Date: Mon, 21 Sep 2020 15:45:05 +0900 Subject: [PATCH 07/11] Update setting view --- app/models/issue.rb | 2 +- config/locales/en.yml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/issue.rb b/app/models/issue.rb index e261227ff..d66121a7a 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -56,7 +56,7 @@ class Issue < ActiveRecord::Base acts_as_mentionable :attributes => ['description'] - DONE_RATIO_OPTIONS = %w(issue_field issue_status) + DONE_RATIO_OPTIONS = %w(issue_field issue_status issue_field_and_closed_status) attr_reader :transition_warning attr_writer :deleted_attachment_ids diff --git a/config/locales/en.yml b/config/locales/en.yml index f9064c866..8a18bee7a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -476,6 +476,7 @@ en: setting_issue_done_ratio: Calculate the issue done ratio with setting_issue_done_ratio_issue_field: Use the issue field setting_issue_done_ratio_issue_status: Use the issue status + setting_issue_done_ratio_issue_field_and_closed_status: Use the issue field and closed status setting_start_of_week: Start calendars on setting_rest_api_enabled: Enable REST web service setting_cache_formatted_text: Cache formatted text From 26034be1fbc4cbf395a116d5658914b7ae1893c5 Mon Sep 17 00:00:00 2001 From: fkdkent Date: Mon, 21 Sep 2020 15:53:49 +0900 Subject: [PATCH 08/11] Improve setting labels --- config/locales/en.yml | 2 +- config/locales/ja.yml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index 8a18bee7a..5ca89603e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -476,7 +476,7 @@ en: setting_issue_done_ratio: Calculate the issue done ratio with setting_issue_done_ratio_issue_field: Use the issue field setting_issue_done_ratio_issue_status: Use the issue status - setting_issue_done_ratio_issue_field_and_closed_status: Use the issue field and closed status + setting_issue_done_ratio_issue_field_and_closed_status: Use the issue field (100% if closed) setting_start_of_week: Start calendars on setting_rest_api_enabled: Enable REST web service setting_cache_formatted_text: Cache formatted text diff --git a/config/locales/ja.yml b/config/locales/ja.yml index 0215e7555..8716e476e 100644 --- a/config/locales/ja.yml +++ b/config/locales/ja.yml @@ -392,6 +392,7 @@ ja: setting_issue_done_ratio: 進捗率の算出方法 setting_issue_done_ratio_issue_field: チケットのフィールドを使用 setting_issue_done_ratio_issue_status: チケットのステータスに連動 + setting_issue_done_ratio_issue_field_and_closed_status: チケットのフィールドを使用（終了したチケットは100%） setting_start_of_week: 週の開始曜日 setting_rest_api_enabled: RESTによるWebサービスを有効にする setting_default_notification_option: デフォルトのメール通知オプション From 8f97623fcbe178ec34587a4c1c60943cc8b1f64e Mon Sep 17 00:00:00 2001 From: fkdkent Date: Mon, 21 Sep 2020 16:08:05 +0900 Subject: [PATCH 09/11] Fix bug that done ratio selector is not shown --- app/views/issues/_attributes.html.erb | 2 +- app/views/issues/bulk_edit.html.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/issues/_attributes.html.erb b/app/views/issues/_attributes.html.erb index b5003c436..da755d9ee 100644 --- a/app/views/issues/_attributes.html.erb +++ b/app/views/issues/_attributes.html.erb @@ -84,7 +84,7 @@

<%= f.hours_field :estimated_hours, :size => 3, :required => @issue.required_attribute?('estimated_hours') %> <%= l(:field_hours) %>

<% end %> -<% if @issue.safe_attribute?('done_ratio') && Issue.use_field_for_done_ratio? %> +<% if @issue.safe_attribute?('done_ratio') && (Issue.use_field_for_done_ratio? || Issue.use_issue_field_and_closed_status_for_done_ratio?) %>

<%= f.select :done_ratio, ((0..10).to_a.collect {|r| ["#{r*10} %", r*10] }), :required => @issue.required_attribute?('done_ratio') %>

<% end %> diff --git a/app/views/issues/bulk_edit.html.erb b/app/views/issues/bulk_edit.html.erb index 4fe457b32..d9f349599 100644 --- a/app/views/issues/bulk_edit.html.erb +++ b/app/views/issues/bulk_edit.html.erb @@ -175,7 +175,7 @@

<% end %> -<% if @safe_attributes.include?('done_ratio') && Issue.use_field_for_done_ratio? %> +<% if @safe_attributes.include?('done_ratio') && (Issue.use_field_for_done_ratio? || Issue.use_issue_field_and_closed_status_for_done_ratio?) %>

<%= select_tag 'issue[done_ratio]', options_for_select([[l(:label_no_change_option), '']] + (0..10).to_a.collect {|r| ["#{r*10} %", r*10] }, @issue_params[:done_ratio]) %> From 82c4b8b5604532d6b3dbe1362f4f077169213807 Mon Sep 17 00:00:00 2001 From: fkdkent Date: Mon, 21 Sep 2020 16:18:28 +0900 Subject: [PATCH 10/11] Refactor --- app/models/issue.rb | 4 ++++ app/views/issues/_attributes.html.erb | 2 +- app/views/issues/bulk_edit.html.erb | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/models/issue.rb b/app/models/issue.rb index d66121a7a..7348134a2 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -754,6 +754,10 @@ def self.use_issue_field_and_closed_status_for_done_ratio? Setting.issue_done_ratio == 'issue_field_and_closed_status' end + def self.done_ratio_editable? + use_status_for_done_ratio? || use_issue_field_and_closed_status_for_done_ratio? + end + def validate_issue if due_date && start_date && (start_date_changed? || due_date_changed?) && due_date < start_date errors.add :due_date, :greater_than_start_date diff --git a/app/views/issues/_attributes.html.erb b/app/views/issues/_attributes.html.erb index da755d9ee..5b160eff0 100644 --- a/app/views/issues/_attributes.html.erb +++ b/app/views/issues/_attributes.html.erb @@ -84,7 +84,7 @@

<%= f.hours_field :estimated_hours, :size => 3, :required => @issue.required_attribute?('estimated_hours') %> <%= l(:field_hours) %>

<% end %> -<% if @issue.safe_attribute?('done_ratio') && (Issue.use_field_for_done_ratio? || Issue.use_issue_field_and_closed_status_for_done_ratio?) %> +<% if @issue.safe_attribute?('done_ratio') && Issue.done_ratio_editable? %>

<%= f.select :done_ratio, ((0..10).to_a.collect {|r| ["#{r*10} %", r*10] }), :required => @issue.required_attribute?('done_ratio') %>

<% end %> diff --git a/app/views/issues/bulk_edit.html.erb b/app/views/issues/bulk_edit.html.erb index d9f349599..31e00a5d7 100644 --- a/app/views/issues/bulk_edit.html.erb +++ b/app/views/issues/bulk_edit.html.erb @@ -175,7 +175,7 @@

<% end %> -<% if @safe_attributes.include?('done_ratio') && (Issue.use_field_for_done_ratio? || Issue.use_issue_field_and_closed_status_for_done_ratio?) %> +<% if @safe_attributes.include?('done_ratio') && Issue.done_ratio_editable? %>

<%= select_tag 'issue[done_ratio]', options_for_select([[l(:label_no_change_option), '']] + (0..10).to_a.collect {|r| ["#{r*10} %", r*10] }, @issue_params[:done_ratio]) %> From ad8b64966f393623f2b4c68ccf65cf21816d206f Mon Sep 17 00:00:00 2001 From: fkdkent Date: Sat, 24 Oct 2020 13:36:09 +0900 Subject: [PATCH 11/11] Fix wrong condition --- app/models/issue.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/issue.rb b/app/models/issue.rb index 7348134a2..42a36d10d 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -755,7 +755,7 @@ def self.use_issue_field_and_closed_status_for_done_ratio? end def self.done_ratio_editable? - use_status_for_done_ratio? || use_issue_field_and_closed_status_for_done_ratio? + use_field_for_done_ratio? || use_issue_field_and_closed_status_for_done_ratio? end def validate_issue