Nemcio/Redmine
1
0
Fork 0
mirror of https://github.com/redmine/redmine.git synced 2025-11-08 06:15:59 +01:00
Code Issues Packages Projects Releases Activity

Enforce issue assignee validation (#23921).

Browse Source 
git-svn-id: http://svn.redmine.org/redmine/trunk@16055 e93f8b46-1217-0410-a6f0-8f06a7374b81
This commit is contained in:
Jean-Philippe Lang
2016-12-10 12:02:37 +00:00
parent 575f4032a2
commit 3a2b6c7c96
8 changed files with 106 additions and 96 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
app/models/issue.rb
View File

@@ -377,6 +377,11 @@ class Issue < ActiveRecord::Base
if category
self.category = project.issue_categories.find_by_name(category.name)
end
# Clear the assignee if not available in the new project for new issues (eg. copy)
# For existing issue, the previous assignee is still valid, so we keep it
if new_record? && assigned_to && !assignable_users.include?(assigned_to)
self.assigned_to_id = nil
end
# Keep the fixed_version if it's still valid in the new_project
if fixed_version && fixed_version.project != project && !project.shared_versions.include?(fixed_version)
self.fixed_version = nil
@@ -538,14 +543,7 @@ class Issue < ActiveRecord::Base
self.status = statuses_allowed.first || default_status
end
if (u = attrs.delete('assigned_to_id')) && safe_attribute?('assigned_to_id')
if u.blank?
self.assigned_to_id = nil
else
u = u.to_i
if assignable_users.any?{|assignable_user| assignable_user.id == u}
self.assigned_to_id = u
end
end
self.assigned_to_id = u
end
@@ -708,6 +706,12 @@ class Issue < ActiveRecord::Base
end
end
if assigned_to_id_changed? && assigned_to_id.present?
unless assignable_users.include?(assigned_to)
errors.add :assigned_to_id, :invalid
end
end
# Checks parent issue assignment
if @invalid_parent_issue_id.present?
errors.add :parent_issue_id, :invalid
@@ -868,7 +872,9 @@ class Issue < ActiveRecord::Base
def assignable_users
users = project.assignable_users(tracker).to_a
users << author if author && author.active?
users << assigned_to if assigned_to
if assigned_to_id_was.present? && assignee = Principal.find_by_id(assigned_to_id_was)
users << assignee
end
users.uniq.sort
end

2
app/models/project.rb
View File

@@ -780,7 +780,7 @@ class Project < ActiveRecord::Base
def copy(project, options={})
project = project.is_a?(Project) ? project : Project.find(project)
to_be_copied = %w(wiki versions issue_categories issues members queries boards)
to_be_copied = %w(members wiki versions issue_categories issues queries boards)
to_be_copied = to_be_copied & Array.wrap(options[:only]) unless options[:only].nil?
Project.transaction do

4
test/functional/issues_controller_test.rb
View File

@@ -4440,7 +4440,7 @@ class IssuesControllerTest < Redmine::ControllerTest
:assigned_to_id => nil),
Issue.create!(:project_id => 2, :tracker_id => 3, :status_id => 2,
:priority_id => 1, :subject => 'issue 2', :author_id => 2,
:assigned_to_id => 3)
:assigned_to_id => 2)
]
assert_difference 'Issue.count', issues.size do
post :bulk_update, :ids => issues.map(&:id), :copy => '1',
@@ -4496,7 +4496,7 @@ class IssuesControllerTest < Redmine::ControllerTest
post :bulk_update, :ids => [1], :copy => '1',
:notes => 'Copying one issue',
:issue => {
:project_id => '', :tracker_id => '', :assigned_to_id => '4',
:project_id => '', :tracker_id => '',
:status_id => '3', :start_date => '2009-12-01', :due_date => '2009-12-31'
}
end

8
test/integration/api_test/issues_test.rb
View File

@@ -689,6 +689,14 @@ JSON
assert_select 'errors error', :text => "Subject cannot be blank"
end
test "PUT /issues/:id.xml with invalid assignee should return error" do
user = User.generate!
put '/issues/6.xml', {:issue => {:assigned_to_id => user.id}}, credentials('jsmith')
assert_response :unprocessable_entity
assert_select 'errors error', :text => "Assignee is invalid"
end
test "PUT /issues/:id.json" do
assert_difference('Journal.count') do
put '/issues/6.json',

128
test/unit/issue_test.rb
View File

@@ -243,14 +243,14 @@ class IssueTest < ActiveSupport::TestCase
def test_anonymous_should_not_see_private_issues_with_issues_visibility_set_to_default
Role.anonymous.update!(:issues_visibility => 'default')
issue = Issue.generate!(:author => User.anonymous, :assigned_to => User.anonymous, :is_private => true)
issue = Issue.generate!(:author => User.anonymous, :is_private => true)
assert_nil Issue.where(:id => issue.id).visible(User.anonymous).first
assert !issue.visible?(User.anonymous)
end
def test_anonymous_should_not_see_private_issues_with_issues_visibility_set_to_own
assert Role.anonymous.update!(:issues_visibility => 'own')
issue = Issue.generate!(:author => User.anonymous, :assigned_to => User.anonymous, :is_private => true)
issue = Issue.generate!(:author => User.anonymous, :is_private => true)
assert_nil Issue.where(:id => issue.id).visible(User.anonymous).first
assert !issue.visible?(User.anonymous)
end
@@ -344,24 +344,27 @@ class IssueTest < ActiveSupport::TestCase
def test_visible_scope_for_member_with_groups_should_return_assigned_issues
user = User.find(8)
assert user.groups.any?
Member.create!(:principal => user.groups.first, :project_id => 1, :role_ids => [2])
group = user.groups.first
Member.create!(:principal => group, :project_id => 1, :role_ids => [2])
Role.non_member.remove_permission!(:view_issues)
issue = Issue.create!(:project_id => 1, :tracker_id => 1, :author_id => 3,
:status_id => 1, :priority => IssuePriority.all.first,
:subject => 'Assignment test',
:assigned_to => user.groups.first,
:is_private => true)
Role.find(2).update! :issues_visibility => 'default'
issues = Issue.visible(User.find(8)).to_a
assert issues.any?
assert issues.include?(issue)
Role.find(2).update! :issues_visibility => 'own'
issues = Issue.visible(User.find(8)).to_a
assert issues.any?
assert_include issue, issues
with_settings :issue_group_assignment => '1' do
issue = Issue.create!(:project_id => 1, :tracker_id => 1, :author_id => 3,
:status_id => 1, :priority => IssuePriority.all.first,
:subject => 'Assignment test',
:assigned_to => group,
:is_private => true)
Role.find(2).update! :issues_visibility => 'default'
issues = Issue.visible(User.find(8)).to_a
assert issues.any?
assert issues.include?(issue)
Role.find(2).update! :issues_visibility => 'own'
issues = Issue.visible(User.find(8)).to_a
assert issues.any?
assert_include issue, issues
end
end
def test_visible_scope_for_member_with_limited_tracker_ids
@@ -458,6 +461,7 @@ class IssueTest < ActiveSupport::TestCase
def test_visible_and_nested_set_scopes
user = User.generate!
Member.create!(:project_id => 1, :principal => user, :role_ids => [1])
parent = Issue.generate!(:assigned_to => user)
assert parent.visible?(user)
child1 = Issue.generate!(:parent_issue_id => parent.id, :assigned_to => user)
@@ -761,13 +765,6 @@ class IssueTest < ActiveSupport::TestCase
:project_id => 1, :author => user,
:assigned_to => user)
assert_equal [1, 2, 3, 4, 5], issue.new_statuses_allowed_to(user).map(&:id)
group = Group.generate!
group.users << user
issue = Issue.generate!(:tracker => tracker, :status => status,
:project_id => 1, :author => user,
:assigned_to => group)
assert_equal [1, 2, 3, 4, 5], issue.new_statuses_allowed_to(user).map(&:id)
end
def test_new_statuses_allowed_to_should_consider_group_assignment
@@ -775,12 +772,16 @@ class IssueTest < ActiveSupport::TestCase
WorkflowTransition.create!(:role_id => 1, :tracker_id => 1,
:old_status_id => 1, :new_status_id => 4,
:author => false, :assignee => true)
user = User.find(2)
group = Group.generate!
Member.create!(:project_id => 1, :principal => group, :role_ids => [1])
user = User.find(2)
group.users << user
issue = Issue.generate!(:author_id => 1, :assigned_to => group)
assert_include 4, issue.new_statuses_allowed_to(user).map(&:id)
with_settings :issue_group_assignment => '1' do
issue = Issue.generate!(:author_id => 1, :assigned_to => group)
assert_include 4, issue.new_statuses_allowed_to(user).map(&:id)
end
end
def test_new_statuses_allowed_to_should_return_all_transitions_for_admin
@@ -895,40 +896,6 @@ class IssueTest < ActiveSupport::TestCase
assert_nil issue.custom_field_value(cf2)
end
def test_safe_attributes_should_ignore_unassignable_assignee
issue = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 3,
:status_id => 1, :priority => IssuePriority.all.first,
:subject => 'test_create')
assert issue.valid?
# locked user, not allowed
issue.safe_attributes=({'assigned_to_id' => '5'})
assert_nil issue.assigned_to_id
# no member
issue.safe_attributes=({'assigned_to_id' => '1'})
assert_nil issue.assigned_to_id
# user 2 is ok
issue.safe_attributes=({'assigned_to_id' => '2'})
assert_equal 2, issue.assigned_to_id
assert issue.save
issue.reload
assert_equal 2, issue.assigned_to_id
issue.safe_attributes=({'assigned_to_id' => '5'})
assert_equal 2, issue.assigned_to_id
issue.safe_attributes=({'assigned_to_id' => '1'})
assert_equal 2, issue.assigned_to_id
# user 3 is also ok
issue.safe_attributes=({'assigned_to_id' => '3'})
assert_equal 3, issue.assigned_to_id
assert issue.save
# removal of assignee
issue.safe_attributes=({'assigned_to_id' => ''})
assert_nil issue.assigned_to_id
assert issue.save
end
def test_editable_custom_field_values_should_return_non_readonly_custom_values
cf1 = IssueCustomField.create!(:name => 'Writable field', :field_format => 'string',
:is_for_all => true, :tracker_ids => [1, 2])
@@ -1250,6 +1217,15 @@ class IssueTest < ActiveSupport::TestCase
assert_equal "125", issue.custom_value_for(2).value
end
def test_copy_to_another_project_should_clear_assignee_if_not_valid
issue = Issue.generate!(:project_id => 1, :assigned_to_id => 2)
project = Project.generate!
issue = Issue.new.copy_from(1)
issue.project = project
assert_nil issue.assigned_to
end
def test_copy_should_copy_status
orig = Issue.find(8)
assert orig.status != orig.default_status
@@ -1759,14 +1735,16 @@ class IssueTest < ActiveSupport::TestCase
end
test "#copy should not create a journal" do
copy = Issue.find(1).copy({:project_id => 3, :tracker_id => 2, :assigned_to_id => 3}, :link => false)
copy = Issue.find(1).copy({:project_id => 3, :tracker_id => 2}, :link => false)
copy.save!
assert_equal 0, copy.reload.journals.size
end
test "#copy should allow assigned_to changes" do
copy = Issue.find(1).copy(:project_id => 3, :tracker_id => 2, :assigned_to_id => 3)
assert_equal 3, copy.assigned_to_id
user = User.generate!
Member.create!(:project_id => 3, :principal => user, :role_ids => [1])
copy = Issue.find(1).copy(:project_id => 3, :tracker_id => 2, :assigned_to_id => user.id)
assert_equal user.id, copy.assigned_to_id
end
test "#copy should allow status changes" do
@@ -2297,8 +2275,9 @@ class IssueTest < ActiveSupport::TestCase
assert !issue.assignable_users.include?(user)
end
test "#assignable_users should include the current assignee" do
def test_assignable_users_should_include_the_current_assignee
user = User.generate!
Member.create!(:project_id => 1, :principal => user, :role_ids => [1])
issue = Issue.generate!(:assigned_to => user)
user.lock!
@@ -2674,7 +2653,9 @@ class IssueTest < ActiveSupport::TestCase
end
test "Issue#recipients should include the assigned to user if the assigned to user is active" do
issue = Issue.generate!(:assigned_to => User.generate!)
user = User.generate!
Member.create!(:project_id => 1, :principal => user, :role_ids => [1])
issue = Issue.generate!(:assigned_to => user)
assert issue.assigned_to, "No assigned_to set for Issue"
assert issue.recipients.include?(issue.assigned_to.mail)
end
@@ -2692,7 +2673,9 @@ class IssueTest < ActiveSupport::TestCase
end
test "Issue#recipients should not include the assigned user if they are only notified of owned issues" do
issue = Issue.generate!(:assigned_to => User.generate!)
user = User.generate!
Member.create!(:project_id => 1, :principal => user, :role_ids => [1])
issue = Issue.generate!(:assigned_to => user)
issue.assigned_to.update!(:mail_notification => :only_owner)
assert !issue.recipients.include?(issue.assigned_to.mail)
end
@@ -2980,10 +2963,13 @@ class IssueTest < ActiveSupport::TestCase
def test_assigned_to_was_with_a_group
group = Group.find(10)
Member.create!(:project_id => 1, :principal => group, :role_ids => [1])
issue = Issue.generate!(:assigned_to => group)
issue.reload.assigned_to = nil
assert_equal group, issue.assigned_to_was
with_settings :issue_group_assignment => '1' do
issue = Issue.generate!(:assigned_to => group)
issue.reload.assigned_to = nil
assert_equal group, issue.assigned_to_was
end
end
def test_issue_overdue_should_respect_user_timezone

3
test/unit/mailer_test.rb
View File

@@ -627,8 +627,9 @@ class MailerTest < ActiveSupport::TestCase
end
def test_reminder_should_include_issues_assigned_to_groups
with_settings :default_language => 'en' do
with_settings :default_language => 'en', :issue_group_assignment => '1' do
group = Group.generate!
Member.create!(:project_id => 1, :principal => group, :role_ids => [1])
group.users << User.find(2)
group.users << User.find(3)

30
test/unit/query_test.rb
View File

@@ -676,19 +676,25 @@ class QueryTest < ActiveSupport::TestCase
def test_filter_assigned_to_me
user = User.find(2)
group = Group.find(10)
User.current = user
i1 = Issue.generate!(:project_id => 1, :tracker_id => 1, :assigned_to => user)
i2 = Issue.generate!(:project_id => 1, :tracker_id => 1, :assigned_to => group)
i3 = Issue.generate!(:project_id => 1, :tracker_id => 1, :assigned_to => Group.find(11))
group.users << user
other_group = Group.find(11)
Member.create!(:project_id => 1, :principal => group, :role_ids => [1])
Member.create!(:project_id => 1, :principal => other_group, :role_ids => [1])
User.current = user
query = IssueQuery.new(:name => '_', :filters => { 'assigned_to_id' => {:operator => '=', :values => ['me']}})
result = query.issues
assert_equal Issue.visible.where(:assigned_to_id => ([2] + user.reload.group_ids)).sort_by(&:id), result.sort_by(&:id)
assert result.include?(i1)
assert result.include?(i2)
assert !result.include?(i3)
with_settings :issue_group_assignment => '1' do
i1 = Issue.generate!(:project_id => 1, :tracker_id => 1, :assigned_to => user)
i2 = Issue.generate!(:project_id => 1, :tracker_id => 1, :assigned_to => group)
i3 = Issue.generate!(:project_id => 1, :tracker_id => 1, :assigned_to => other_group)
query = IssueQuery.new(:name => '_', :filters => { 'assigned_to_id' => {:operator => '=', :values => ['me']}})
result = query.issues
assert_equal Issue.visible.where(:assigned_to_id => ([2] + user.reload.group_ids)).sort_by(&:id), result.sort_by(&:id)
assert result.include?(i1)
assert result.include?(i2)
assert !result.include?(i3)
end
end
def test_user_custom_field_filtered_on_me
@@ -1689,7 +1695,7 @@ class QueryTest < ActiveSupport::TestCase
@issue1 = Issue.generate!(:project => @project, :assigned_to_id => @manager.id)
@issue2 = Issue.generate!(:project => @project, :assigned_to_id => @developer.id)
@issue3 = Issue.generate!(:project => @project, :assigned_to_id => @boss.id)
@issue4 = Issue.generate!(:project => @project, :assigned_to_id => @guest.id)
@issue4 = Issue.generate!(:project => @project, :author_id => @guest.id, :assigned_to_id => @guest.id)
@issue5 = Issue.generate!(:project => @project)
@query = IssueQuery.new(:name => '_', :project => @project)

3
test/unit/user_test.rb
View File

@@ -1140,6 +1140,7 @@ class UserTest < ActiveSupport::TestCase
project = Project.find(1)
author = User.generate!
assignee = User.generate!
Member.create!(:user => assignee, :project => project, :role_ids => [1])
member = User.generate!
Member.create!(:user => member, :project => project, :role_ids => [1])
issue = Issue.generate!(:project => project, :assigned_to => assignee, :author => author)
@@ -1160,7 +1161,9 @@ class UserTest < ActiveSupport::TestCase
def test_notify_about_issue_for_previous_assignee
assignee = User.generate!(:mail_notification => 'only_assigned')
Member.create!(:user => assignee, :project_id => 1, :role_ids => [1])
new_assignee = User.generate!(:mail_notification => 'only_assigned')
Member.create!(:user => new_assignee, :project_id => 1, :role_ids => [1])
issue = Issue.generate!(:assigned_to => assignee)
assert assignee.notify_about?(issue)