Add the ability to change the author of an issue (#1739).

Patch by Vladimir Kovacik, Jiri Stepanek, Aighan Pacobilch, Olivier Houdas, Takenori TAKAKI, and Mizuki ISHIKAWA.


git-svn-id: https://svn.redmine.org/redmine/trunk@21958 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/helpers/issues_helper.rb

@@ -534,7 +534,7 @@ module IssuesHelper
         old_value = format_date(detail.old_value.to_date) if detail.old_value
 
       when 'project_id', 'status_id', 'tracker_id', 'assigned_to_id',
-            'priority_id', 'category_id', 'fixed_version_id'
+            'priority_id', 'category_id', 'fixed_version_id', 'author_id'
         value = find_name_by_reflection(field, detail.value)
         old_value = find_name_by_reflection(field, detail.old_value)
 
@@ -778,4 +778,23 @@ module IssuesHelper
       projects
     end
   end
+
+  def author_options_for_select(issue, project)
+    users = issue.assignable_users.select {|m| m.is_a?(User) && m.allowed_to?(:add_issues, project) }
+
+    if issue.new_record?
+      if users.include?(User.current)
+        principals_options_for_select(users, issue.author)
+      else
+        principals_options_for_select([User.current] + users)
+      end
+    elsif issue.persisted?
+      if users.include?(issue.author)
+        principals_options_for_select(users, issue.author)
+      else
+        author_principal = Principal.find(issue.author_id)
+        principals_options_for_select([author_principal] + users, author_principal)
+      end
+    end
+  end
 end

app/models/issue.rb

@@ -111,6 +111,7 @@ class Issue < ActiveRecord::Base
   before_validation :clear_disabled_fields
   before_save :close_duplicates, :update_done_ratio_from_issue_status,
               :force_updated_on_change, :update_closed_on
+  before_create :set_author_journal
   after_save do |issue|
     if !issue.saved_change_to_id? && issue.saved_change_to_project_id?
       issue.send :after_project_change
@@ -519,6 +520,9 @@ class Issue < ActiveRecord::Base
   safe_attributes(
     'deleted_attachment_ids',
     :if => lambda {|issue, user| issue.attachments_deletable?(user)})
+  safe_attributes(
+    'author_id',
+    :if => lambda {|issue, user| user.allowed_to?(:change_issue_author, issue.project)})
 
   def safe_attribute_names(user=nil)
     names = super
@@ -2020,6 +2024,14 @@ class Issue < ActiveRecord::Base
     end
   end
 
+  def set_author_journal
+    return unless new_record?
+    return unless self.author.present? && User.current.present? && self.author != User.current
+
+    self.init_journal(User.current)
+    self.current_journal.__send__(:add_attribute_detail, 'author_id', User.current.id, self.author.id)
+  end
+
   def send_notification
     if notify? && Setting.notified_events.include?('issue_added')
       Mailer.deliver_issue_add(self)

app/views/issues/_attributes.html.erb

@@ -3,6 +3,9 @@
 <div class="splitcontent">
 <div class="splitcontentleft">
 <% if @issue.safe_attribute?('status_id') && @allowed_statuses.present? %>
+<% if User.current.allowed_to?(:change_issue_author, @project) %>
+  <p><%= f.select :author_id, author_options_for_select(@issue, @project), :include_blank => false, :required => true %></p>
+<% end %>
 <p>
   <%= f.select :status_id, (@allowed_statuses.collect {|p| [p.name, p.id]}), {:required => true},
                 :onchange => "updateIssueFrom('#{escape_javascript(update_issue_form_path(@project, @issue))}', this)" %>

config/locales/en.yml

@@ -541,6 +541,7 @@ en:
   permission_view_private_notes: View private notes
   permission_set_notes_private: Set notes as private
   permission_delete_issues: Delete issues
+  permission_change_issue_author: Change issue author
   permission_manage_public_queries: Manage public queries
   permission_save_queries: Save queries
   permission_view_gantt: View gantt chart

lib/redmine/preparation.rb

@@ -72,6 +72,7 @@ module Redmine
           map.permission :view_private_notes, {}, :read => true, :require => :member
           map.permission :set_notes_private, {}, :require => :member
           map.permission :delete_issues, {:issues => :destroy}, :require => :member
+          map.permission :change_issue_author, {:issues => [:edit, :update]}
           map.permission :mention_users, {}
           # Watchers
           map.permission :view_issue_watchers, {}, :read => true

test/functional/issues_controller_test.rb

@@ -8306,6 +8306,7 @@ class IssuesControllerTest < Redmine::ControllerTest
   end
 
   def test_destroy_child_issue
+    User.current = User.find(1)
     parent = Issue.create!(:project_id => 1, :author_id => 1, :tracker_id => 1, :subject => 'Parent Issue')
     child = Issue.create!(:project_id => 1, :author_id => 1, :tracker_id => 1, :subject => 'Child Issue', :parent_issue_id => parent.id)
     assert child.is_descendant_of?(parent.reload)

test/functional/versions_controller_test.rb

@@ -103,6 +103,7 @@ class VersionsControllerTest < Redmine::ControllerTest
   end
 
   def test_index_should_show_issue_assignee
+    User.current = User.find_by_login('jsmith')
     with_settings :gravatar_enabled => '1' do
       Issue.generate!(:project_id => 3, :fixed_version_id => 4, :assigned_to => User.find_by_login('jsmith'))
       Issue.generate!(:project_id => 3, :fixed_version_id => 4)

test/helpers/issues_helper_test.rb

@@ -465,4 +465,62 @@ class IssuesHelperTest < Redmine::HelperTest
     assert_include "<a href=\"/issues?issue_id=#{open_issue.id}%2C#{closed_issue.id}&amp;set_filter=true&amp;status_id=o\">1 open</a>", html
     assert_include "<a href=\"/issues?issue_id=#{open_issue.id}%2C#{closed_issue.id}&amp;set_filter=true&amp;status_id=c\">1 closed</a>", html
   end
+
+  def test_author_options_for_select_if_new_record_and_users_includes_current_user
+    User.current = User.find(2)
+    issue = Issue.new(project_id: 1)
+    assignable_users = [User.find(3), User.find(2)]
+
+    assert_includes assignable_users, User.current
+    assert_equal(
+      principals_options_for_select(assignable_users, nil),
+      author_options_for_select(issue, issue.project))
+  end
+
+  def test_author_options_for_select_if_new_record_and_users_not_includes_current_user
+    User.current = User.find(1)
+    issue = Issue.new(project_id: 1)
+    assignable_users = [User.find(3), User.find(2)]
+    assert_not_includes assignable_users, User.current
+
+    assert_equal(
+      principals_options_for_select([User.current] + assignable_users, nil),
+      author_options_for_select(issue, issue.project))
+  end
+
+  def test_author_options_for_select_if_persisted_record_and_users_includes_author
+    User.current = User.find(2)
+    issue = Issue.find(1)
+    issue.update(author_id: 2)
+    assignable_users = [User.find(3), User.find(2)]
+
+    assert_includes assignable_users, issue.author
+    assert_equal(
+      principals_options_for_select(assignable_users, issue.author),
+      author_options_for_select(issue, issue.project))
+  end
+
+  def test_author_options_for_select_if_persisted_record_and_users_not_includes_author
+    User.current = User.find(2)
+    issue = Issue.find(1)
+    issue.update(author_id: 1)
+    assignable_users = [User.find(3), User.find(2)]
+
+    assert_not_includes assignable_users, issue.author
+    assert_equal(
+      principals_options_for_select([User.find(1)] + assignable_users, issue.author),
+      author_options_for_select(issue, issue.project))
+  end
+
+  def test_author_options_for_select_if_persisted_record_and_author_is_anonymous
+    User.current = User.find(2)
+    issue = Issue.find(1)
+    issue.update(author_id: User.anonymous.id)
+    assignable_users = [User.find(3), User.find(2)]
+
+    assert_not_includes assignable_users, issue.author
+    assert_equal(
+      principals_options_for_select([User.anonymous] + assignable_users, issue.author),
+      author_options_for_select(issue, issue.project))
+  end
 end

test/helpers/journals_helper_test.rb

@@ -22,7 +22,7 @@ require File.expand_path('../../test_helper', __FILE__)
 class JournalsHelperTest < Redmine::HelperTest
   include JournalsHelper
 
-  fixtures :projects, :trackers, :issue_statuses, :issues, :journals,
+  fixtures :projects, :trackers, :issue_statuses, :issues, :journals, :journal_details,
            :enumerations, :issue_categories,
            :projects_trackers,
            :users, :roles, :member_roles, :members,

test/object_helpers.rb

@@ -95,7 +95,7 @@ module ObjectHelpers
     issue.project ||= Project.find(1)
     issue.tracker ||= issue.project.trackers.first
     issue.subject = 'Generated' if issue.subject.blank?
-    issue.author ||= User.find(2)
+    issue.author ||= (User.current || User.find(2))
     yield issue if block_given?
     issue
   end

test/unit/issue_nested_set_test.rb

@@ -60,6 +60,7 @@ class IssueNestedSetTest < ActiveSupport::TestCase
   end
 
   def test_creating_a_child_in_a_subproject_should_validate
+    User.current = User.find(1)
     issue = Issue.generate!
     child = nil
     assert_difference 'Journal.count', 1 do

test/unit/issue_test.rb

@@ -2789,6 +2789,7 @@ class IssueTest < ActiveSupport::TestCase
   end
 
   def test_journalized_multi_custom_field
+    User.current = User.find(1)
     field = IssueCustomField.create!(:name => 'filter', :field_format => 'list',
                                      :is_filter => true, :is_for_all => true,
                                      :tracker_ids => [1],
@@ -3465,4 +3466,55 @@ class IssueTest < ActiveSupport::TestCase
     r = Issue.like('issue today')
     assert_include Issue.find(7), r
   end
+
+  def test_author_should_be_changed_when_user_with_permission_change_issue_author
+    Role.all.each do |r|
+      r.add_permission! :change_issue_author
+    end
+    User.current = User.find(2)
+
+    issue = Issue.generate!(:author => User.find(3))
+    assert_equal 3, issue.author_id
+
+    issue.safe_attributes = { 'author_id' => 4 }
+    assert_equal 4, issue.author_id
+    assert_not_equal 3, issue.author_id
+  end
+
+  def test_author_should_not_be_changed_when_user_without_permission_change_issue_author
+    Role.all.each do |r|
+      r.remove_permission! :change_issue_author
+    end
+    User.current = User.find(2)
+
+    issue = Issue.generate!(:author => User.find(3))
+    assert_equal 3, issue.author_id
+
+    issue.safe_attributes = { 'author_id' => 4 }
+    assert_not_equal 4, issue.author_id
+    assert_equal 3, issue.author_id
+  end
+
+  def test_create_should_create_journal_if_user_other_than_current_user_is_set_as_the_author
+    User.current = User.find(1)
+    issue = nil
+    assert_difference 'Journal.count' do
+      issue = Issue.generate!(author: User.find(2))
+    end
+
+    first_journal_detail = issue.journals.first.details.first
+    assert_equal 'author_id', first_journal_detail.prop_key
+    assert_equal '1', first_journal_detail.old_value
+    assert_equal '2', first_journal_detail.value
+  end
+
+  def test_create_should_create_journal_if_current_user_is_set_as_the_author
+    User.current = User.find(1)
+    issue = nil
+    assert_no_difference 'Journal.count' do
+      issue = Issue.generate!(author: User.current)
+    end
+
+    assert_not issue.journals.present?
+  end
 end