From 43bb11fa09932c1cefca1cbecfc9da2b00f5d2ae Mon Sep 17 00:00:00 2001 From: Gregor Schmidt Date: Thu, 12 Jul 2018 14:31:30 +0200 Subject: [PATCH] Calculate done_ratio based on logged time --- app/models/issue.rb | 47 +++++++++++++++++++++++++----- app/models/time_entry.rb | 16 ++++++++++ config/locales/de.yml | 1 + config/locales/en.yml | 1 + test/unit/issue_subtasking_test.rb | 41 +++++++++++++++++++++++++- test/unit/issue_test.rb | 21 +++++++++---- test/unit/time_entry_test.rb | 34 +++++++++++++++++++++ 7 files changed, 148 insertions(+), 13 deletions(-) diff --git a/app/models/issue.rb b/app/models/issue.rb index 114d96208..feb297379 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -52,7 +52,7 @@ class Issue < ActiveRecord::Base acts_as_activity_provider :scope => preload(:project, :author, :tracker, :status), :author_key => :author_id - DONE_RATIO_OPTIONS = %w(issue_field issue_status) + DONE_RATIO_OPTIONS = %w(issue_field issue_status logged_time) attr_accessor :deleted_attachment_ids attr_reader :current_journal @@ -106,7 +106,7 @@ class Issue < ActiveRecord::Base before_validation :default_assign, on: :create before_validation :clear_disabled_fields - before_save :close_duplicates, :update_done_ratio_from_issue_status, + before_save :close_duplicates, :update_done_ratio, :force_updated_on_change, :update_closed_on after_save {|issue| issue.send :after_project_change if !issue.saved_change_to_id? && issue.saved_change_to_project_id?} after_save :reschedule_following_issues, :update_nested_set_attributes, @@ -687,8 +687,22 @@ class Issue < ActiveRecord::Base private :workflow_rule_by_attribute def done_ratio - if Issue.use_status_for_done_ratio? && status && status.default_done_ratio + if use_status_for_done_ratio? status.default_done_ratio + + elsif use_time_for_done_ratio? + ratio = + if done_ratio_derived? && total_estimated_hours.to_f > 0 + (total_spent_hours.to_f / total_estimated_hours.to_f) + + elsif !done_ratio_derived? && estimated_hours.to_f > 0 + (spent_hours.to_f / estimated_hours.to_f) + + else + 0.0 + end + + [ratio * 100, 100].min.to_i else read_attribute(:done_ratio) end @@ -697,10 +711,23 @@ class Issue < ActiveRecord::Base def self.use_status_for_done_ratio? Setting.issue_done_ratio == 'issue_status' end + def use_status_for_done_ratio? + Issue.use_status_for_done_ratio? && status && status.default_done_ratio + end + + def self.use_time_for_done_ratio? + Setting.issue_done_ratio == 'logged_time' + end + def use_time_for_done_ratio? + Issue.use_time_for_done_ratio? + end def self.use_field_for_done_ratio? Setting.issue_done_ratio == 'issue_field' end + def use_field_for_done_ratio? + !(use_status_for_done_ratio? || use_time_for_done_ratio?) + end def validate_issue if due_date && start_date && (start_date_changed? || due_date_changed?) && due_date < start_date @@ -799,12 +826,18 @@ class Issue < ActiveRecord::Base # Set the done_ratio using the status if that setting is set. This will keep the done_ratios # even if the user turns off the setting later - 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 + def update_done_ratio + unless use_field_for_done_ratio? + self.done_ratio = self.done_ratio end end + def update_done_ratio! + self.init_journal(User.current, "") + self.update_done_ratio + self.save + end + def init_journal(user, notes = "") @current_journal ||= Journal.new(:journalized => self, :user => user, :notes => notes) end @@ -1702,7 +1735,7 @@ class Issue < ActiveRecord::Base if p.done_ratio_derived? # done ratio = average ratio of children weighted with their total estimated hours - unless Issue.use_status_for_done_ratio? && p.status && p.status.default_done_ratio + if p.use_field_for_done_ratio? children = p.children.to_a if children.any? child_with_total_estimated_hours = children.select {|c| c.total_estimated_hours.to_f > 0.0} diff --git a/app/models/time_entry.rb b/app/models/time_entry.rb index d64c311fa..b1e30cd37 100644 --- a/app/models/time_entry.rb +++ b/app/models/time_entry.rb @@ -46,6 +46,8 @@ class TimeEntry < ActiveRecord::Base validates_length_of :comments, :maximum => 1024, :allow_nil => true validates :spent_on, :date => true before_validation :set_project_if_nil + after_save :update_done_ratio + after_destroy :update_done_ratio validate :validate_time_entry scope :visible, lambda {|*args| @@ -138,6 +140,20 @@ class TimeEntry < ActiveRecord::Base errors.add :activity_id, :inclusion if activity_id_changed? && project && !project.activities.include?(activity) end + def update_done_ratio + if issue && Issue.use_time_for_done_ratio? + # Only create a new journal for this update if we don't have any other + # changes pending on the issue. In that case, we will save the issue + # later anyway (which we thus expect here and don't save the issue + # ourselfes) + if issue.changed? + issue.update_done_ratio + else + issue.update_done_ratio! + end + end + end + def hours=(h) write_attribute :hours, (h.is_a?(String) ? (h.to_hours || h) : h) end diff --git a/config/locales/de.yml b/config/locales/de.yml index 13be99421..1b6adec6c 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -1011,6 +1011,7 @@ de: setting_issue_done_ratio: Berechne den Ticket-Fortschritt mittels setting_issue_done_ratio_issue_field: Ticket-Feld % erledigt setting_issue_done_ratio_issue_status: Ticket-Status + setting_issue_done_ratio_logged_time: geschätzter und gebuchter Zeit setting_issue_group_assignment: Ticketzuweisung an Gruppen erlauben setting_issue_list_default_columns: Standard-Spalten in der Ticket-Auflistung setting_issues_export_limit: Max. Anzahl Tickets bei CSV/PDF-Export diff --git a/config/locales/en.yml b/config/locales/en.yml index d8f09431f..227f202c9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -435,6 +435,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_logged_time: Use the logged and estimated time 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/test/unit/issue_subtasking_test.rb b/test/unit/issue_subtasking_test.rb index 00d64a052..62455e77d 100644 --- a/test/unit/issue_subtasking_test.rb +++ b/test/unit/issue_subtasking_test.rb @@ -21,7 +21,7 @@ class IssueSubtaskingTest < ActiveSupport::TestCase fixtures :projects, :users, :roles, :members, :member_roles, :trackers, :projects_trackers, :issue_statuses, :issue_categories, :enumerations, - :issues, + :issues, :time_entries, :enabled_modules, :workflows @@ -168,6 +168,45 @@ class IssueSubtaskingTest < ActiveSupport::TestCase end end + def test_parent_done_ratio_via_logged_time_if_set_to_derived + with_settings :parent_issue_done_ratio => 'derived', :issue_done_ratio => 'logged_time' do + parent = Issue.generate!(:estimated_hours => 2) + TimeEntry.generate!(:issue => parent, :hours => 2) + + child = parent.generate_child!(:estimated_hours => 8) + TimeEntry.generate!(:issue => child, :hours => 2) + + # Estimated time: 10h, Spent time: 4h => 40 % done + assert_equal 40, parent.reload.done_ratio + end + end + + def test_parent_done_ratio_via_logged_time_if_set_to_independent + with_settings :parent_issue_done_ratio => 'independent', :issue_done_ratio => 'logged_time' do + parent = Issue.generate!(:estimated_hours => 2) + TimeEntry.generate!(:issue => parent, :hours => 2) + + child = parent.generate_child!(:estimated_hours => 8) + TimeEntry.generate!(:issue => child, :hours => 2) + + # Estimated time: 2h, Spent time: 2h => 100 % done + assert_equal 100, parent.reload.done_ratio + end + end + + def test_parent_done_ratio_via_logged_time_does_not_exceed_100 + with_settings :parent_issue_done_ratio => 'derived', :issue_done_ratio => 'logged_time' do + parent = Issue.generate!(:estimated_hours => 2) + TimeEntry.generate!(:issue => parent, :hours => 2) + + child = parent.generate_child!(:estimated_hours => 2) + TimeEntry.generate!(:issue => child, :hours => 8) + + # Estimated time: 4h, Spent time: 10h => 250 % == 100 % done + assert_equal 100, parent.reload.done_ratio + end + end + def test_parent_done_ratio_should_be_weighted_by_estimated_times_if_any with_settings :parent_issue_done_ratio => 'derived' do parent = Issue.generate! diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index 40be6b66a..af3622912 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -2730,29 +2730,40 @@ class IssueTest < ActiveSupport::TestCase end end - test "#update_done_ratio_from_issue_status should update done_ratio according to Setting.issue_done_ratio" do + test "#update_done_ratio should update done_ratio according to Setting.issue_done_ratio" do @issue = Issue.find(1) + @issue.update!(:estimated_hours => 308.5) @issue_status = IssueStatus.find(1) @issue_status.update!(:default_done_ratio => 50) + @issue2 = Issue.find(2) @issue_status2 = IssueStatus.find(2) @issue_status2.update!(:default_done_ratio => 0) with_settings :issue_done_ratio => 'issue_field' do - @issue.update_done_ratio_from_issue_status - @issue2.update_done_ratio_from_issue_status + @issue.update_done_ratio + @issue2.update_done_ratio assert_equal 0, @issue.read_attribute(:done_ratio) assert_equal 30, @issue2.read_attribute(:done_ratio) end with_settings :issue_done_ratio => 'issue_status' do - @issue.update_done_ratio_from_issue_status - @issue2.update_done_ratio_from_issue_status + @issue.update_done_ratio + @issue2.update_done_ratio assert_equal 50, @issue.read_attribute(:done_ratio) assert_equal 0, @issue2.read_attribute(:done_ratio) end + + with_settings :issue_done_ratio => 'logged_time' do + @issue.update_done_ratio + @issue2.update_done_ratio + + assert_equal 50, @issue.read_attribute(:done_ratio) + assert_equal 0, @issue2.read_attribute(:done_ratio) + end + end test "#by_tracker" do diff --git a/test/unit/time_entry_test.rb b/test/unit/time_entry_test.rb index 9b3fbf78b..d9c943551 100644 --- a/test/unit/time_entry_test.rb +++ b/test/unit/time_entry_test.rb @@ -212,4 +212,38 @@ class TimeEntryTest < ActiveSupport::TestCase assert_equal ["Comment cannot be blank", "Issue cannot be blank"], entry.errors.full_messages.sort end end + + def test_create_updates_issues_done_ratio + with_settings :issue_done_ratio => 'logged_time' do + issue = Issue.generate!(:estimated_hours => 10) + assert_equal 0, issue.done_ratio + + TimeEntry.generate!(:issue => issue, :hours => 5) + assert_equal 50, issue.reload.done_ratio + end + end + + def test_update_updates_issues_done_ratio + with_settings :issue_done_ratio => 'logged_time' do + issue = Issue.generate!(:estimated_hours => 10) + + te = TimeEntry.generate!(:issue => issue, :hours => 5) + assert_equal 50, issue.reload.done_ratio + + te.update_attribute(:hours, 10) + assert_equal 100, issue.reload.done_ratio + end + end + + def test_destroy_updates_issues_done_ratio + with_settings :issue_done_ratio => 'logged_time' do + issue = Issue.generate!(:estimated_hours => 10) + + te = TimeEntry.generate!(:issue => issue, :hours => 5) + assert_equal 50, issue.reload.done_ratio + + te.destroy + assert_equal 0, issue.reload.done_ratio + end + end end -- 2.18.0