From 2633faa0a5b6a6320911302e9c48a35f52368bde Mon Sep 17 00:00:00 2001 From: Marius BALTEANU Date: Sun, 13 Dec 2020 11:55:29 +0200 Subject: [PATCH] Take into account the setting Setting.cross_project_subtasks when proposing the target projects for a subtask Refactor allowed_target_projects and move the logic for new issue form from the model to helper --- app/helpers/issues_helper.rb | 10 +++++ app/models/issue.rb | 52 +++++++++++++++++------ app/views/issues/_attributes.html.erb | 3 +- app/views/issues/_form.html.erb | 2 +- test/functional/issues_controller_test.rb | 24 +++++++++++ test/unit/issue_test.rb | 34 +++++++++++++++ 6 files changed, 110 insertions(+), 15 deletions(-) diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 808842325..4a9e7a2aa 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -696,4 +696,14 @@ module IssuesHelper user_default_tab end end + + def projects_for_select(issue) + if issue.parent_issue_id.present? + issue.allowed_target_projects_for_subtask(User.current) + elsif issue.new_record? && !issue.copy? + issue.allowed_target_projects(User.current, 'descendants') + else + issue.allowed_target_projects(User.current) + end + end end diff --git a/app/models/issue.rb b/app/models/issue.rb index 6132f71e6..33870176c 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -1550,29 +1550,38 @@ class Issue < ActiveRecord::Base end end + # Returns a scope of projects that user can assign the subtask + def allowed_target_projects_for_subtask(user=User.current) + if parent_issue_id.present? + scope = filter_projects_scope(Setting.cross_project_subtasks) + end + + self.class.allowed_target_projects(user, project, scope) + end + # Returns a scope of projects that user can assign the issue to - def allowed_target_projects(user=User.current, context=nil) - if new_record? && context.is_a?(Project) && !copy? - current_project = context.self_and_descendants - elsif new_record? - current_project = nil - else - current_project = project + def allowed_target_projects(user=User.current, scope=nil) + current_project = new_record? ? nil : project + if scope + scope = filter_projects_scope(scope) end - self.class.allowed_target_projects(user, current_project) + self.class.allowed_target_projects(user, current_project, scope) end # Returns a scope of projects that user can assign issues to # If current_project is given, it will be included in the scope - def self.allowed_target_projects(user=User.current, current_project=nil) + def self.allowed_target_projects(user=User.current, current_project=nil, scope=nil) condition = Project.allowed_to_condition(user, :add_issues) - if current_project.is_a?(Project) + if current_project condition = ["(#{condition}) OR #{Project.table_name}.id = ?", current_project.id] - elsif current_project - condition = ["(#{condition}) AND #{Project.table_name}.id IN (?)", current_project.map(&:id)] end - Project.where(condition).having_trackers + + if scope.nil? + scope = Project + end + + scope.where(condition).having_trackers end # Returns a scope of trackers that user can assign the issue to @@ -1906,4 +1915,21 @@ class Issue < ActiveRecord::Base self.done_ratio ||= 0 end end + + def filter_projects_scope(scope=nil) + case scope + when 'system' + Project + when 'tree' + project.root.self_and_descendants + when 'hierarchy' + project.hierarchy + when 'descendants' + project.self_and_descendants + when '' + Project.where(:id => project.id) + else + Project + end + end end diff --git a/app/views/issues/_attributes.html.erb b/app/views/issues/_attributes.html.erb index ee4ae109c..b5003c436 100644 --- a/app/views/issues/_attributes.html.erb +++ b/app/views/issues/_attributes.html.erb @@ -61,7 +61,8 @@
<% if @issue.safe_attribute? 'parent_issue_id' %>

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

+ :required => @issue.required_attribute?('parent_issue_id'), + :onchange => "updateIssueFrom('#{escape_javascript update_issue_form_path(@project, @issue)}', this)" %>

<%= javascript_tag "observeAutocompleteField('issue_parent_issue_id', '#{escape_javascript(auto_complete_issues_path(:project_id => @issue.project, :scope => Setting.cross_project_subtasks, :status => @issue.closed? ? 'c' : 'o', :issue_id => @issue.id))}')" %> <% end %> diff --git a/app/views/issues/_form.html.erb b/app/views/issues/_form.html.erb index 0d3ca5e22..9654d631d 100644 --- a/app/views/issues/_form.html.erb +++ b/app/views/issues/_form.html.erb @@ -9,7 +9,7 @@

<% end %> -<% projects = @issue.allowed_target_projects(User.current, @project) %> +<% projects = projects_for_select(@issue) %> <% if (@issue.safe_attribute?('project_id') || @issue.project_id_changed?) && (@project.nil? || projects.length > 1 || @issue.copy?) %>

<%= f.select :project_id, project_tree_options_for_select(projects, :selected => @issue.project), {:required => true}, :onchange => "updateIssueFrom('#{escape_javascript update_issue_form_path(@project, @issue)}', this)" %>

diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 07533a854..9556d7339 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -3100,6 +3100,30 @@ class IssuesControllerTest < Redmine::ControllerTest assert_select 'select[name="issue[project_id]"]', 0 end + def test_get_new_should_not_show_invalid_projects_when_issue_is_a_subtask + @request.session[:user_id] = 2 + issue = Issue.find(1) + issue.parent_id = 3 + issue.save + + with_settings :cross_project_subtasks => 'tree' do + get( + :new, + :params => { + :project_id => 1, + :parent_issue_id => 1 + } + ) + end + assert_response :success + assert_select 'select[name="issue[project_id]"]' do + assert_select 'option', 3 + + # Onlinestore project should not be included + assert_select 'option[value=?]', '2', 0 + end + end + def test_get_new_with_minimal_permissions Role.find(1).update_attribute :permissions, [:add_issues] WorkflowTransition.where(:role_id => 1).delete_all diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index 95812376a..4a7c663b5 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -1708,6 +1708,17 @@ class IssueTest < ActiveSupport::TestCase assert_not_include project, Issue.allowed_target_projects(User.find(1)) end + def test_allowed_target_projects_for_subtask_should_not_include_invalid_projects + User.current = User.find(1) + issue = Issue.find(1) + issue.parent_id = 3 + + with_settings :cross_project_subtasks => 'tree' do + # Should include only the project tree + assert_equal [1, 3, 5], issue.allowed_target_projects_for_subtask.ids.sort + end + end + def test_allowed_target_trackers_with_one_role_allowed_on_all_trackers user = User.generate! role = Role.generate! @@ -3343,4 +3354,27 @@ class IssueTest < ActiveSupport::TestCase assert !child.reopenable? assert_equal l(:notice_issue_not_reopenable_by_closed_parent_issue), child.transition_warning end + + def test_filter_projects_scope + Issue.send(:public, :filter_projects_scope) + # Project eCookbook + issue = Issue.find(1) + + assert_equal Project, issue.filter_projects_scope + assert_equal Project, issue.filter_projects_scope('system') + + # Project Onlinestore (id 2) is not part of the tree + assert_equal [1, 3, 4, 5, 6], Issue.find(5).filter_projects_scope('tree').ids.sort + + # Project "Private child of eCookbook" + issue2 = Issue.find(9) + + # Projects "eCookbook Subproject 1" (id 3) and "eCookbook Subproject 1" (id 4) are not part of hierarchy + assert_equal [1, 5, 6], issue2.filter_projects_scope('hierarchy').ids.sort + + # Project "Child of private child" is descendant of "Private child of eCookbook" + assert_equal [5, 6], issue2.filter_projects_scope('descendants').ids.sort + + assert_equal [5], issue2.filter_projects_scope('').ids.sort + end end -- 2.22.0