From 83bcc1f043511d85414ca3cafc1e10dc84b9da24 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Wed, 10 Oct 2012 17:38:17 +0000 Subject: [PATCH] Adds a setting to allow subtasks to belong to other projects (#5487). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@10587 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/helpers/issues_helper.rb | 4 +- app/helpers/settings_helper.rb | 12 ++++++ app/models/issue.rb | 31 +++++++++++--- app/views/issues/_attributes.html.erb | 2 +- app/views/settings/_issues.html.erb | 2 + config/locales/en.yml | 1 + config/locales/fr.yml | 1 + config/settings.yml | 3 ++ test/fixtures/projects_trackers.yml | 3 ++ test/test_helper.rb | 8 ++++ test/unit/issue_nested_set_test.rb | 10 ++++- test/unit/issue_test.rb | 60 +++++++++++++++++++++++++++ 12 files changed, 128 insertions(+), 9 deletions(-) diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 1ba4c4635..2cc387ff8 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -65,7 +65,7 @@ module IssuesHelper s = '' ancestors = issue.root? ? [] : issue.ancestors.visible.all ancestors.each do |ancestor| - s << '
' + content_tag('p', link_to_issue(ancestor)) + s << '
' + content_tag('p', link_to_issue(ancestor, :project => (issue.project_id != ancestor.project_id))) end s << '
' subject = h(issue.subject) @@ -82,7 +82,7 @@ module IssuesHelper issue_list(issue.descendants.visible.sort_by(&:lft)) do |child, level| s << content_tag('tr', content_tag('td', check_box_tag("ids[]", child.id, false, :id => nil), :class => 'checkbox') + - content_tag('td', link_to_issue(child, :truncate => 60), :class => 'subject') + + content_tag('td', link_to_issue(child, :truncate => 60, :project => (issue.project_id != child.project_id)), :class => 'subject') + content_tag('td', h(child.status)) + content_tag('td', link_to_user(child.assigned_to)) + content_tag('td', progress_bar(child.done_ratio, :width => '80px')), diff --git a/app/helpers/settings_helper.rb b/app/helpers/settings_helper.rb index dd20f24c6..c1fa8ea64 100644 --- a/app/helpers/settings_helper.rb +++ b/app/helpers/settings_helper.rb @@ -91,4 +91,16 @@ module SettingsHelper l_or_humanize(notifiable.name, :prefix => 'label_').html_safe, :class => notifiable.parent.present? ? "parent" : '').html_safe end + + def cross_project_subtasks_options + options = [ + [:label_disabled, ''], + [:label_cross_project_system, 'system'], + [:label_cross_project_tree, 'tree'], + [:label_cross_project_hierarchy, 'hierarchy'], + [:label_cross_project_descendants, 'descendants'] + ] + + options.map {|label, value| [l(label), value.to_s]} + end end diff --git a/app/models/issue.rb b/app/models/issue.rb index 37a19a9c4..520d2c3dc 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -285,7 +285,8 @@ class Issue < ActiveRecord::Base if fixed_version && fixed_version.project != project && !project.shared_versions.include?(fixed_version) self.fixed_version = nil end - if parent && parent.project_id != project_id + # Clear the parent task if it's no longer valid + unless valid_parent_project? self.parent_issue_id = nil end @custom_field_values = nil @@ -550,8 +551,8 @@ class Issue < ActiveRecord::Base # Checks parent issue assignment if @parent_issue - if @parent_issue.project_id != project_id - errors.add :parent_issue_id, :not_same_project + if !valid_parent_project?(@parent_issue) + errors.add :parent_issue_id, :invalid elsif !new_record? # moving an existing issue if @parent_issue.root_id != root_id @@ -559,7 +560,7 @@ class Issue < ActiveRecord::Base elsif move_possible?(@parent_issue) # move accepted inside tree else - errors.add :parent_issue_id, :not_a_valid_parent + errors.add :parent_issue_id, :invalid end end end @@ -963,6 +964,25 @@ class Issue < ActiveRecord::Base end end + # Returns true if issue's project is a valid + # parent issue project + def valid_parent_project?(issue=parent) + return true if issue.nil? || issue.project_id == project_id + + case Setting.cross_project_subtasks + when 'system' + true + when 'tree' + issue.project.root == project.root + when 'hierarchy' + issue.project.is_or_is_ancestor_of?(project) || issue.project.is_descendant_of?(project) + when 'descendants' + issue.project.is_or_is_ancestor_of?(project) + else + false + end + end + # Extracted from the ReportsController. def self.by_tracker(project) count_and_group_by(:project => project, @@ -1042,8 +1062,9 @@ class Issue < ActiveRecord::Base relations_to.clear end - # Move subtasks + # Move subtasks that were in the same project children.each do |child| + next unless child.project_id == project_id_was # Change project and keep project child.send :project=, project, true unless child.save diff --git a/app/views/issues/_attributes.html.erb b/app/views/issues/_attributes.html.erb index 2b73623ea..9b4c508cd 100644 --- a/app/views/issues/_attributes.html.erb +++ b/app/views/issues/_attributes.html.erb @@ -43,7 +43,7 @@
<% if @issue.safe_attribute? 'parent_issue_id' %>

<%= f.text_field :parent_issue_id, :size => 10, :required => @issue.required_attribute?('parent_issue_id') %>

-<%= javascript_tag "observeAutocompleteField('issue_parent_issue_id', '#{escape_javascript auto_complete_issues_path(:project_id => @issue.project)}')" %> +<%= javascript_tag "observeAutocompleteField('issue_parent_issue_id', '#{escape_javascript auto_complete_issues_path}')" %> <% end %> <% if @issue.safe_attribute? 'start_date' %> diff --git a/app/views/settings/_issues.html.erb b/app/views/settings/_issues.html.erb index 6c9bf4553..a8d004daa 100644 --- a/app/views/settings/_issues.html.erb +++ b/app/views/settings/_issues.html.erb @@ -3,6 +3,8 @@

<%= setting_check_box :cross_project_issue_relations %>

+

<%= setting_select :cross_project_subtasks, cross_project_subtasks_options %>

+

<%= setting_check_box :issue_group_assignment %>

<%= setting_check_box :default_issue_start_date_to_creation_date %>

diff --git a/config/locales/en.yml b/config/locales/en.yml index 50373ed83..050ee0279 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -357,6 +357,7 @@ en: setting_date_format: Date format setting_time_format: Time format setting_cross_project_issue_relations: Allow cross-project issue relations + setting_cross_project_subtasks: Allow cross-project subtasks setting_issue_list_default_columns: Default columns displayed on the issue list setting_repositories_encodings: Attachments and repositories encodings setting_emails_header: Emails header diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 2aa9ab1ee..6c6067b84 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -356,6 +356,7 @@ fr: setting_date_format: Format de date setting_time_format: Format d'heure setting_cross_project_issue_relations: Autoriser les relations entre demandes de différents projets + setting_cross_project_subtasks: Autoriser les sous-tâches dans des projets différents setting_issue_list_default_columns: Colonnes affichées par défaut sur la liste des demandes setting_emails_footer: Pied-de-page des emails setting_protocol: Protocole diff --git a/config/settings.yml b/config/settings.yml index 01906609f..6aa224166 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -133,6 +133,9 @@ user_format: format: symbol cross_project_issue_relations: default: 0 +# Enables subtasks to be in other projects +cross_project_subtasks: + default: 'tree' issue_group_assignment: default: 0 default_issue_start_date_to_creation_date: diff --git a/test/fixtures/projects_trackers.yml b/test/fixtures/projects_trackers.yml index 4766a9f82..2c5707c5e 100644 --- a/test/fixtures/projects_trackers.yml +++ b/test/fixtures/projects_trackers.yml @@ -41,4 +41,7 @@ projects_trackers_013: projects_trackers_014: project_id: 1 tracker_id: 3 +projects_trackers_015: + project_id: 6 + tracker_id: 1 \ No newline at end of file diff --git a/test/test_helper.rb b/test/test_helper.rb index 12e70b9bf..66a6f2646 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -156,6 +156,14 @@ class ActiveSupport::TestCase hs end + def assert_save(object) + saved = object.save + message = "#{object.class} could not be saved" + errors = object.errors.full_messages.map {|m| "- #{m}"} + message << ":\n#{errors.join("\n")}" if errors.any? + assert_equal true, saved, message + end + def assert_error_tag(options={}) assert_tag({:attributes => { :id => 'errorExplanation' }}.merge(options)) end diff --git a/test/unit/issue_nested_set_test.rb b/test/unit/issue_nested_set_test.rb index 6716759b5..921cddc99 100644 --- a/test/unit/issue_nested_set_test.rb +++ b/test/unit/issue_nested_set_test.rb @@ -49,7 +49,15 @@ class IssueNestedSetTest < ActiveSupport::TestCase assert_equal [parent.id, parent.id, 2, 3], [child.root_id, child.parent_id, child.lft, child.rgt] end - def test_creating_a_child_in_different_project_should_not_validate + def test_creating_a_child_in_a_subproject_should_validate + issue = create_issue! + child = Issue.new(:project_id => 3, :tracker_id => 2, :author_id => 1, + :subject => 'child', :parent_issue_id => issue.id) + assert_save child + assert_equal issue, child.reload.parent + end + + def test_creating_a_child_in_an_invalid_project_should_not_validate issue = create_issue! child = Issue.new(:project_id => 2, :tracker_id => 1, :author_id => 1, :subject => 'child', :parent_issue_id => issue.id) diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index 7fdd9cf0f..4bd718de1 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -906,6 +906,24 @@ class IssueTest < ActiveSupport::TestCase assert_equal 7, issue.fixed_version_id end + def test_move_to_another_project_should_keep_parent_if_valid + issue = Issue.find(1) + issue.update_attribute(:parent_issue_id, 2) + issue.project = Project.find(3) + assert issue.save + issue.reload + assert_equal 2, issue.parent_id + end + + def test_move_to_another_project_should_clear_parent_if_not_valid + issue = Issue.find(1) + issue.update_attribute(:parent_issue_id, 2) + issue.project = Project.find(2) + assert issue.save + issue.reload + assert_nil issue.parent_id + end + def test_move_to_another_project_with_disabled_tracker issue = Issue.find(1) target = Project.find(2) @@ -996,6 +1014,48 @@ class IssueTest < ActiveSupport::TestCase end end + def test_valid_parent_project + issue = Issue.find(1) + issue_in_same_project = Issue.find(2) + issue_in_child_project = Issue.find(5) + issue_in_grandchild_project = Issue.generate!(:project_id => 6, :tracker_id => 1) + issue_in_other_child_project = Issue.find(6) + issue_in_different_tree = Issue.find(4) + + with_settings :cross_project_subtasks => '' do + assert_equal true, issue.valid_parent_project?(issue_in_same_project) + assert_equal false, issue.valid_parent_project?(issue_in_child_project) + assert_equal false, issue.valid_parent_project?(issue_in_grandchild_project) + assert_equal false, issue.valid_parent_project?(issue_in_different_tree) + end + + with_settings :cross_project_subtasks => 'system' do + assert_equal true, issue.valid_parent_project?(issue_in_same_project) + assert_equal true, issue.valid_parent_project?(issue_in_child_project) + assert_equal true, issue.valid_parent_project?(issue_in_different_tree) + end + + with_settings :cross_project_subtasks => 'tree' do + assert_equal true, issue.valid_parent_project?(issue_in_same_project) + assert_equal true, issue.valid_parent_project?(issue_in_child_project) + assert_equal true, issue.valid_parent_project?(issue_in_grandchild_project) + assert_equal false, issue.valid_parent_project?(issue_in_different_tree) + + assert_equal true, issue_in_child_project.valid_parent_project?(issue_in_same_project) + assert_equal true, issue_in_child_project.valid_parent_project?(issue_in_other_child_project) + end + + with_settings :cross_project_subtasks => 'descendants' do + assert_equal true, issue.valid_parent_project?(issue_in_same_project) + assert_equal false, issue.valid_parent_project?(issue_in_child_project) + assert_equal false, issue.valid_parent_project?(issue_in_grandchild_project) + assert_equal false, issue.valid_parent_project?(issue_in_different_tree) + + assert_equal true, issue_in_child_project.valid_parent_project?(issue) + assert_equal false, issue_in_child_project.valid_parent_project?(issue_in_other_child_project) + end + end + def test_recipients_should_include_previous_assignee user = User.find(3) user.members.update_all ["mail_notification = ?", false]