From 3736b84eab228de4340e782515c6a862fc7a9bf5 Mon Sep 17 00:00:00 2001 From: Lucile Quirion Date: Fri, 20 Feb 2015 22:05:58 -0500 Subject: [PATCH] settings: configurable issue independent time tracking This commit add a setting option to estimate and track time for parent issue independently from children issues. This option remove parent-child interdependency: - subtasks does not overrides parent tasks time tracking attributes: 'priority', 'done_ratio', 'start_date', 'due_date', 'estimated_hours' - children tasks are not rescheduled when parent task is rescheduled. This commit also updates the views: - make time tracking attributes editable for parent task - add total estimated hours and total spent time for parent task --- app/models/issue.rb | 20 +++++- app/views/issues/_attributes.html.erb | 14 ++-- app/views/issues/show.html.erb | 8 ++- app/views/settings/_issues.html.erb | 2 + config/locales/en.yml | 3 + config/settings.yml | 2 + test/functional/issues_controller_test.rb | 41 ++++++++++++ test/unit/issue_nested_set_test.rb | 106 +++++++++++++++++++++++++++++- test/unit/issue_test.rb | 31 +++++++++ 9 files changed, 217 insertions(+), 10 deletions(-) diff --git a/app/models/issue.rb b/app/models/issue.rb index 5a49b0d..b0bb42f 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -457,7 +457,7 @@ class Issue < ActiveRecord::Base attrs = delete_unsafe_attributes(attrs, user) return if attrs.empty? - unless leaf? + unless leaf? || Issue.use_independent_time_tracking? attrs.reject! {|k,v| %w(priority_id done_ratio start_date due_date estimated_hours).include?(k)} end @@ -527,6 +527,11 @@ class Issue < ActiveRecord::Base required_attribute_names(user).include?(name.to_s) end + # Returns true if the attribute is disabled for edition + def disabled_attribute?(attribute) + !self.leaf? && !Issue.use_independent_time_tracking? + end + # Returns a hash of the workflow rule by attribute for the given user # # Examples: @@ -578,6 +583,10 @@ class Issue < ActiveRecord::Base Setting.issue_done_ratio == 'issue_field' end + def self.use_independent_time_tracking? + Setting.issue_independent_time_tracking? + 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 @@ -886,6 +895,10 @@ class Issue < ActiveRecord::Base sum("#{TimeEntry.table_name}.hours").to_f || 0.0 end + def total_estimated_hours + @total_estimated_hours ||= self_and_descendants.sum(:estimated_hours) + end + def relations @relations ||= IssueRelation::Relations.new(self, (relations_from + relations_to).sort) end @@ -1079,7 +1092,7 @@ class Issue < ActiveRecord::Base # If the issue is a parent task, this is done by rescheduling its subtasks. def reschedule_on!(date) return if date.nil? - if leaf? + if leaf? || Issue.use_independent_time_tracking? if start_date.nil? || start_date != date if start_date && start_date > date # Issue can not be moved earlier than its soonest start date @@ -1394,7 +1407,8 @@ class Issue < ActiveRecord::Base end def recalculate_attributes_for(issue_id) - if issue_id && p = Issue.find_by_id(issue_id) + if issue_id && (p = Issue.find_by_id(issue_id)) && !Issue.use_independent_time_tracking? + # priority = highest priority of children if priority_position = p.children.joins(:priority).maximum("#{IssuePriority.table_name}.position") p.priority = IssuePriority.find_by_position(priority_position) diff --git a/app/views/issues/_attributes.html.erb b/app/views/issues/_attributes.html.erb index 61ffa8a..8e7872f 100644 --- a/app/views/issues/_attributes.html.erb +++ b/app/views/issues/_attributes.html.erb @@ -11,7 +11,7 @@ <% end %> <% if @issue.safe_attribute? 'priority_id' %> -

<%= f.select :priority_id, (@priorities.collect {|p| [p.name, p.id]}), {:required => true}, :disabled => !@issue.leaf? %>

+

<%= f.select :priority_id, (@priorities.collect {|p| [p.name, p.id]}), {:required => true}, :disabled => @issue.disabled_attribute?('priority_id') %>

<% end %> <% if @issue.safe_attribute? 'assigned_to_id' %> @@ -48,7 +48,8 @@ <% if @issue.safe_attribute? 'start_date' %>

- <%= f.text_field(:start_date, :size => 10, :disabled => !@issue.leaf?, + <%= f.text_field(:start_date, :size => 10, + :disabled => @issue.disabled_attribute?('start_date'), :required => @issue.required_attribute?('start_date')) %> <%= calendar_for('issue_start_date') if @issue.leaf? %>

@@ -56,17 +57,20 @@ <% if @issue.safe_attribute? 'due_date' %>

- <%= f.text_field(:due_date, :size => 10, :disabled => !@issue.leaf?, + <%= f.text_field(:due_date, :size => 10, + :disabled => @issue.disabled_attribute?('due_date'), :required => @issue.required_attribute?('due_date')) %> <%= calendar_for('issue_due_date') if @issue.leaf? %>

<% end %> <% if @issue.safe_attribute? 'estimated_hours' %> -

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

+

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

<% end %> -<% if @issue.safe_attribute?('done_ratio') && @issue.leaf? && Issue.use_field_for_done_ratio? %> +<% if @issue.safe_attribute?('done_ratio') && !@issue.disabled_attribute?('done_ratio') && Issue.use_field_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/show.html.erb b/app/views/issues/show.html.erb index 6a514ac..45d4b27 100644 --- a/app/views/issues/show.html.erb +++ b/app/views/issues/show.html.erb @@ -61,9 +61,15 @@ unless @issue.estimated_hours.nil? rows.right l(:field_estimated_hours), l_hours(@issue.estimated_hours), :class => 'estimated-hours' end + unless @issue.leaf? || !Issue.use_independent_time_tracking? || @issue.total_estimated_hours.nil? + rows.right l(:field_total_estimated_hours), l_hours(@issue.total_estimated_hours), :class => 'estimated_hours' + end end if User.current.allowed_to?(:view_time_entries, @project) - rows.right l(:label_spent_time), (@issue.total_spent_hours > 0 ? link_to(l_hours(@issue.total_spent_hours), issue_time_entries_path(@issue)) : "-"), :class => 'spent-time' + rows.right l(:label_spent_time), (@issue.spent_hours > 0 ? link_to(l_hours(@issue.spent_hours), issue_time_entries_path(@issue)) : "-"), :class => 'spent-time' + if (!@issue.leaf? && Issue.use_independent_time_tracking?) + rows.right l(:label_total_spent_time), (@issue.total_spent_hours > 0 ? link_to(l_hours(@issue.total_spent_hours), issue_time_entries_path(@issue)) : "-"), :class => 'spent-time' + end end end %> <%= render_custom_fields_rows(@issue) %> diff --git a/app/views/settings/_issues.html.erb b/app/views/settings/_issues.html.erb index c1e8022..47c33ed 100644 --- a/app/views/settings/_issues.html.erb +++ b/app/views/settings/_issues.html.erb @@ -15,6 +15,8 @@

<%= setting_select :issue_done_ratio, Issue::DONE_RATIO_OPTIONS.collect {|i| [l("setting_issue_done_ratio_#{i}"), i]} %>

+

<%= setting_check_box :issue_independent_time_tracking %>

+

<%= setting_multiselect :non_working_week_days, (1..7).map {|d| [day_name(d), d.to_s]}, :inline => true %>

<%= setting_text_field :issues_export_limit, :size => 6 %>

diff --git a/config/locales/en.yml b/config/locales/en.yml index f1a04a0..b4a3b25 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -300,6 +300,7 @@ en: field_delay: Delay field_assignable: Issues can be assigned to this role field_redirect_existing_links: Redirect existing links + field_total_estimated_hours: Total estimated time field_estimated_hours: Estimated time field_column_names: Columns field_time_entries: Log time @@ -392,6 +393,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_independent_time_tracking: Estimate and track time for parent issue independently from children issues setting_start_of_week: Start calendars on setting_rest_api_enabled: Enable REST web service setting_cache_formatted_text: Cache formatted text @@ -744,6 +746,7 @@ en: label_changes_details: Details of all changes label_issue_tracking: Issue tracking label_spent_time: Spent time + label_total_spent_time: Total spent time label_overall_spent_time: Overall spent time label_f_hour: "%{value} hour" label_f_hour_plural: "%{value} hours" diff --git a/config/settings.yml b/config/settings.yml index d2f0ff9..6378904 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -174,6 +174,8 @@ display_subprojects_issues: default: 1 issue_done_ratio: default: 'issue_field' +issue_independent_time_tracking: + default: 0 default_projects_public: default: 1 default_projects_modules: diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 3d048ed..d94f7fe 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -1368,6 +1368,47 @@ class IssuesControllerTest < ActionController::TestCase assert_select 'div.thumbnails', 0 end + def test_show_should_disable_parent_time_tracking_attributes + issue = Issue.create!(:project_id => 1, :author_id => 2, :tracker_id => 1, :parent_issue_id => 2, :subject => 'Child Issue') + + @request.session[:user_id] = 2 + get :show, :id => 2 + assert_response :success + + assert_select 'form#issue-form' do + assert_select 'select[disabled="disabled"]' do + assert_select '[name=?]', 'issue[priority_id]', true, "priority_id shall be disabled" + end + assert_select 'input[disabled="disabled"]' do + assert_select '[name=?]', 'issue[start_date]', true, "start_date shall be disabled" + assert_select '[name=?]', 'issue[due_date]', true, "due_date shall be disabled" + assert_select '[name=?]', 'issue[estimated_hours]', true, "estimated_hours shall be disabled" + end + assert_select 'select[name=?]', 'issue[done_ratio]', false, "done_ratio shall not be present" + end + end + + def test_show_should_enable_parent_time_tracking_attributes_with_independent_time_tracking + with_settings :issue_independent_time_tracking => '1' do + issue = Issue.create!(:project_id => 1, :author_id => 2, :tracker_id => 1, :parent_issue_id => 2, :subject => 'Child Issue') + + @request.session[:user_id] = 2 + get :show, :id => 2 + assert_response :success + + assert_select 'form#issue-form' do + assert_select 'select[disabled="disabled"]', 0 + assert_select 'input[disabled="disabled"]', 0 + + assert_select 'select[name=?]', 'issue[priority_id]', true, "priority_id shall be editable" + assert_select 'input[name=?]', 'issue[start_date]', true, "start_date shall be editable" + assert_select 'input[name=?]', 'issue[due_date]', true, "due_date shall be editable" + assert_select 'input[name=?]', 'issue[estimated_hours]', true, "estimated_hours shall be editable" + assert_select 'select[name=?]', 'issue[done_ratio]', true, "done_ratio shall be editable" + end + end + end + def test_show_with_multi_custom_field field = CustomField.find(1) field.update_attribute :multiple, true diff --git a/test/unit/issue_nested_set_test.rb b/test/unit/issue_nested_set_test.rb index 6fb5b00..31da839 100644 --- a/test/unit/issue_nested_set_test.rb +++ b/test/unit/issue_nested_set_test.rb @@ -306,6 +306,28 @@ class IssueNestedSetTest < ActiveSupport::TestCase assert_equal 'Normal', parent.reload.priority.name end + def test_parent_priority_should_not_be_the_highest_child_priority_with_independent_time_tracking + with_settings :issue_independent_time_tracking => '1' do + parent = Issue.generate!(:priority => IssuePriority.find_by_name('Normal')) + # Create children + child1 = parent.generate_child!(:priority => IssuePriority.find_by_name('High')) + assert_equal 'Normal', parent.reload.priority.name + child2 = child1.generate_child!(:priority => IssuePriority.find_by_name('Immediate')) + assert_equal 'High', child1.reload.priority.name + assert_equal 'Normal', parent.reload.priority.name + child3 = parent.generate_child!(:priority => IssuePriority.find_by_name('Low')) + assert_equal 'Normal', parent.reload.priority.name + # Destroy a child + child1.destroy + assert_equal 'Normal', parent.reload.priority.name + # Update a child + child3.reload.priority = IssuePriority.find_by_name('Normal') + child3.save! + assert_equal 'Normal', parent.reload.priority.name + end + + end + def test_parent_dates_should_be_lowest_start_and_highest_due_dates parent = Issue.generate! parent.generate_child!(:start_date => '2010-01-25', :due_date => '2010-02-15') @@ -316,6 +338,18 @@ class IssueNestedSetTest < ActiveSupport::TestCase assert_equal Date.parse('2010-02-22'), parent.due_date end + def test_parent_dates_should_not_be_lowest_start_and_highest_due_dates_with_independent_time_tracking + with_settings :issue_independent_time_tracking => '1' do + parent = Issue.generate! + parent.generate_child!(:start_date => '2010-01-25', :due_date => '2010-02-15') + parent.generate_child!( :due_date => '2010-02-13') + parent.generate_child!(:start_date => '2010-02-01', :due_date => '2010-02-22') + parent.reload + assert_equal nil, parent.start_date + assert_equal nil, parent.due_date + end + end + def test_parent_done_ratio_should_be_average_done_ratio_of_leaves parent = Issue.generate! parent.generate_child!(:done_ratio => 20) @@ -331,6 +365,23 @@ class IssueNestedSetTest < ActiveSupport::TestCase assert_equal 40, parent.reload.done_ratio end + def test_parent_done_ratio_should_not_be_average_done_ratio_of_leaves_with_independent_time_tracking + with_settings :issue_independent_time_tracking => '1' do + parent = Issue.generate! + parent.generate_child!(:done_ratio => 20) + assert_equal 0, parent.reload.done_ratio + parent.generate_child!(:done_ratio => 70) + assert_equal 0, parent.reload.done_ratio + + child = parent.generate_child!(:done_ratio => 0) + assert_equal 0, parent.reload.done_ratio + + child.generate_child!(:done_ratio => 30) + assert_equal 0, child.reload.done_ratio + assert_equal 0, parent.reload.done_ratio + end + end + def test_parent_done_ratio_should_be_weighted_by_estimated_times_if_any parent = Issue.generate! parent.generate_child!(:estimated_hours => 10, :done_ratio => 20) @@ -350,6 +401,19 @@ class IssueNestedSetTest < ActiveSupport::TestCase assert_equal 100, parent.reload.done_ratio end + def test_parent_done_ratio_should_not_reach_100_when_child_is_closed_with_independent_time_tracking + parent = Issue.generate! + with_settings :issue_independent_time_tracking => '1' do + issue1 = parent.generate_child! + issue2 = parent.generate_child!(:estimated_hours => 0) + assert_equal 0, parent.reload.done_ratio + issue1.reload.close! + assert_equal 0, parent.reload.done_ratio + issue2.reload.close! + assert_equal 0, parent.reload.done_ratio + end + end + def test_parent_estimate_should_be_sum_of_leaves parent = Issue.generate! parent.generate_child!(:estimated_hours => nil) @@ -360,6 +424,18 @@ class IssueNestedSetTest < ActiveSupport::TestCase assert_equal 12, parent.reload.estimated_hours end + def test_parent_estimated_should_not_be_the_sum_of_leaves_with_independent_time_tracking + parent = Issue.generate! + with_settings :issue_independent_time_tracking => '1' do + parent.generate_child!(:estimated_hours => nil) + assert_equal nil, parent.reload.estimated_hours + parent.generate_child!(:estimated_hours => 5) + assert_equal nil, parent.reload.estimated_hours + parent.generate_child!(:estimated_hours => 7) + assert_equal nil, parent.reload.estimated_hours + end + end + def test_done_ratio_of_parent_with_a_child_without_estimated_time_should_not_exceed_100 parent = Issue.generate! parent.generate_child!(:estimated_hours => 40) @@ -390,7 +466,19 @@ class IssueNestedSetTest < ActiveSupport::TestCase assert_nil first_parent.reload.estimated_hours end - def test_reschuling_a_parent_should_reschedule_subtasks + def test_move_parent_should_not_updates_old_parent_attributes_with_independent_time_tracking + with_settings :issue_independent_time_tracking => '1' do + first_parent = Issue.generate! + second_parent = Issue.generate! + child = first_parent.generate_child!(:estimated_hours => 5) + assert_nil first_parent.reload.estimated_hours + child.update_attributes(:estimated_hours => 7, :parent_issue_id => second_parent.id) + assert_nil second_parent.reload.estimated_hours + assert_nil first_parent.reload.estimated_hours + end + end + + def test_rescheduling_a_parent_should_reschedule_subtasks parent = Issue.generate! c1 = parent.generate_child!(:start_date => '2010-05-12', :due_date => '2010-05-18') c2 = parent.generate_child!(:start_date => '2010-06-03', :due_date => '2010-06-10') @@ -404,6 +492,22 @@ class IssueNestedSetTest < ActiveSupport::TestCase assert_equal [Date.parse('2010-06-02'), Date.parse('2010-06-10')], [parent.start_date, parent.due_date] end + def test_rescheduling_a_parent_should_not_reschedule_subtasks_with_independent_time_tracking + with_settings :issue_independent_time_tracking => '1' do + parent = Issue.generate!(:start_date => '2010-05-12', :due_date => '2010-06-10') + c1 = parent.generate_child!(:start_date => '2010-05-12', :due_date => '2010-05-18') + c2 = parent.generate_child!(:start_date => '2010-06-03', :due_date => '2010-06-10') + parent.reload + parent.reschedule_on!(Date.parse('2010-06-02')) + c1.reload + assert_equal [Date.parse('2010-05-12'), Date.parse('2010-05-18')], [c1.start_date, c1.due_date], "child 1" + c2.reload + assert_equal [Date.parse('2010-06-03'), Date.parse('2010-06-10')], [c2.start_date, c2.due_date], "child 2" # no change + parent.reload + assert_equal [Date.parse('2010-06-02'), Date.parse('2010-07-01')], [parent.start_date, parent.due_date], "parent" + end + end + def test_project_copy_should_copy_issue_tree p = Project.create!(:name => 'Tree copy', :identifier => 'tree-copy', :tracker_ids => [1, 2]) i1 = Issue.generate!(:project => p, :subject => 'i1') diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index 33635e7..e872045 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -2215,6 +2215,37 @@ class IssueTest < ActiveSupport::TestCase end end + test "#time tracking attributes should be independent according to Setting.issue_independent_time_tracking" do + @issue = Issue.find(1) + @issue.update_attribute(:estimated_hours, '1:30') + + with_settings :issue_independent_time_tracking => '1' do + issue = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 3, :parent_id => 1, + :status_id => 1, :priority => IssuePriority.all.first, + :subject => 'test_independent_time_tracking', + :description => 'IssueTest#test_independent_time_tracking', :estimated_hours => '0:30') + assert issue.save + issue.reload + @issue.reload + assert_equal 0.5, issue.estimated_hours + assert_equal 1.5, @issue.estimated_hours + end + + with_settings :issue_independent_time_tracking => '0' do + issue = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 3, :parent_id => 1, + :status_id => 1, :priority => IssuePriority.all.first, + :subject => 'test_independent_time_tracking', + :description => 'IssueTest#test_independent_time_tracking', :estimated_hours => '0:30') + assert issue.save + issue.reload + @issue.reload + assert_equal 0.5, issue.estimated_hours + assert_equal 0.5, issue.estimated_hours + end + + end + + test "#by_tracker" do User.current = User.anonymous groups = Issue.by_tracker(Project.find(1)) -- 2.1.0