Allow addition/removal of subtasks to show in parent's history (#6033).

Patch by Yuichi HARADA.


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

7 files changed, 134 insertions, 29 deletions

diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index 81b23acb3..c226288df 100644
--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -547,6 +547,12 @@ module IssuesHelper
         value = "##{detail.value}" unless detail.value.blank?
         old_value = "##{detail.old_value}" unless detail.old_value.blank?
 
+      when 'child_id'
+        label = l(:label_subtask)
+        value = "##{detail.value}" unless detail.value.blank?
+        old_value = "##{detail.old_value}" unless detail.old_value.blank?
+        multiple = true
+
       when 'is_private'
         value = l(detail.value == "0" ? :general_text_No : :general_text_Yes) unless detail.value.blank?
         old_value = l(detail.old_value == "0" ? :general_text_No : :general_text_Yes) unless detail.old_value.blank?
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 55962f0fe..5c1911eb8 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -117,8 +117,8 @@ class Issue < ActiveRecord::Base
   after_save :reschedule_following_issues, :update_nested_set_attributes,
              :update_parent_attributes, :delete_selected_attachments, :create_journal
   # Should be after_create but would be called before previous after_save callbacks
-  after_save :after_create_from_copy
-  after_destroy :update_parent_attributes
+  after_save :after_create_from_copy, :create_parent_issue_journal
+  after_destroy :update_parent_attributes, :create_parent_issue_journal
   after_create_commit :send_notification
 
   # Returns a SQL conditions string used to find all issues visible by the specified user
@@ -1987,6 +1987,32 @@ class Issue < ActiveRecord::Base
     end
   end
 
+  def create_parent_issue_journal
+    return if persisted? && !saved_change_to_parent_id?
+    return if destroyed? && @without_nested_set_update
+
+    child_id = self.id
+    old_parent_id, new_parent_id =
+      if persisted?
+        [parent_id_before_last_save, parent_id]
+      elsif destroyed?
+        [parent_id, nil]
+      else
+        [nil, parent_id]
+      end
+
+    if old_parent_id.present? && old_parent_issue = Issue.visible.find_by_id(old_parent_id)
+      old_parent_issue.init_journal(User.current)
+      old_parent_issue.current_journal.__send__(:add_attribute_detail, 'child_id', child_id, nil)
+      old_parent_issue.save
+    end
+    if new_parent_id.present? && new_parent_issue = Issue.visible.find_by_id(new_parent_id)
+      new_parent_issue.init_journal(User.current)
+      new_parent_issue.current_journal.__send__(:add_attribute_detail, 'child_id', nil, child_id)
+      new_parent_issue.save
+    end
+  end
+
   def send_notification
     if notify? && Setting.notified_events.include?('issue_added')
       Mailer.deliver_issue_add(self)
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 18d7e2615..053e22197 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -996,6 +996,7 @@ en:
   label_missing_api_access_key: Missing an API access key
   label_api_access_key_created_on: "API access key created %{value} ago"
   label_profile: Profile
+  label_subtask: Subtask
   label_subtask_plural: Subtasks
   label_project_copy_notifications: Send email notifications during the project copy
   label_import_notifications: Send email notifications during the import
diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index f9432d311..434775e59 100644
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -2344,7 +2344,7 @@ class IssuesControllerTest < Redmine::ControllerTest
   end
 
   def test_show_should_list_subtasks
-    Issue.
+    issue = Issue.
       create!(
         :project_id => 1, :author_id => 1, :tracker_id => 1,
         :parent_issue_id => 1, :subject => 'Child Issue'
@@ -2354,6 +2354,11 @@ class IssuesControllerTest < Redmine::ControllerTest
     assert_select 'div#issue_tree' do
       assert_select 'td.subject', :text => /Child Issue/
     end
+    assert_select 'div#tab-content-history' do
+      assert_select 'div[id=?]', "change-#{Issue.find(1).journals.last.id}" do
+        assert_select 'ul.details', :text => "Subtask ##{issue.id} added"
+      end
+    end
   end
 
   def test_show_should_show_subtasks_stats
@@ -8223,6 +8228,31 @@ class IssuesControllerTest < Redmine::ControllerTest
     assert !(Issue.find_by_id(1) || Issue.find_by_id(2) || Issue.find_by_id(6))
   end
 
+  def test_destroy_child_issue
+    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)
+
+    @request.session[:user_id] = 2
+    assert_difference 'Issue.count', -1 do
+      delete :destroy, :params => {:id => child.id}
+    end
+    assert_response :found
+    assert_redirected_to :action => 'index', :project_id => 'ecookbook'
+
+    parent.reload
+    assert_equal 2, parent.journals.count
+
+    get :show, :params => {:id => parent.id}
+    assert_response :success
+
+    assert_select 'div#tab-content-history' do
+      assert_select 'div[id=?]', "change-#{parent.journals.last.id}" do
+        assert_select 'ul.details', :text => "Subtask deleted (##{child.id})"
+      end
+    end
+  end
+
   def test_destroy_parent_and_child_issues
     parent = Issue.create!(:project_id => 1, :author_id => 1,
                            :tracker_id => 1, :subject => 'Parent Issue')
diff --git a/test/unit/issue_nested_set_test.rb b/test/unit/issue_nested_set_test.rb
index e60607b59..02b6670e2 100644
--- a/test/unit/issue_nested_set_test.rb
+++ b/test/unit/issue_nested_set_test.rb
@@ -49,7 +49,10 @@ class IssueNestedSetTest < ActiveSupport::TestCase
   def test_create_child_issue
     lft = new_issue_lft
     parent = Issue.generate!
-    child =  parent.generate_child!
+    child = nil
+    assert_difference 'Journal.count', 1 do
+      child = parent.generate_child!
+    end
     parent.reload
     child.reload
     assert_equal [parent.id, nil,       lft,     lft + 3], [parent.root_id, parent.parent_id, parent.lft, parent.rgt]
@@ -58,17 +61,23 @@ class IssueNestedSetTest < ActiveSupport::TestCase
 
   def test_creating_a_child_in_a_subproject_should_validate
     issue = Issue.generate!
-    child = Issue.new(:project_id => 3, :tracker_id => 2, :author_id => 1,
-                      :subject => 'child', :parent_issue_id => issue.id)
-    assert_save child
+    child = nil
+    assert_difference 'Journal.count', 1 do
+      child = Issue.new(:project_id => 3, :tracker_id => 2, :author_id => 1,
+                        :subject => 'child', :parent_issue_id => issue.id)
+      assert_save child
+    end
     assert_equal issue, child.reload.parent
   end
 
   def test_creating_a_child_in_an_invalid_project_should_not_validate
     issue = Issue.generate!
-    child = Issue.new(:project_id => 2, :tracker_id => 1, :author_id => 1,
-                      :subject => 'child', :parent_issue_id => issue.id)
-    assert !child.save
+    child = nil
+    assert_no_difference 'Journal.count' do
+      child = Issue.new(:project_id => 2, :tracker_id => 1, :author_id => 1,
+                        :subject => 'child', :parent_issue_id => issue.id)
+      assert !child.save
+    end
     assert_not_equal [], child.errors[:parent_issue_id]
   end
 
@@ -77,8 +86,11 @@ class IssueNestedSetTest < ActiveSupport::TestCase
     parent1 = Issue.generate!
     parent2 = Issue.generate!
     child = parent1.generate_child!
-    parent2.parent_issue_id = parent1.id
-    parent2.save!
+    assert_difference 'Journal.count', 2 do
+      parent2.init_journal(User.find(2))
+      parent2.parent_issue_id = parent1.id
+      parent2.save!
+    end
     child.reload
     parent1.reload
     parent2.reload
@@ -94,8 +106,11 @@ class IssueNestedSetTest < ActiveSupport::TestCase
     parent2 = Issue.generate!
     lft3 = new_issue_lft
     child = parent1.generate_child!
-    child.parent_issue_id = nil
-    child.save!
+    assert_difference 'Journal.count', 2 do
+      child.init_journal(User.find(2))
+      child.parent_issue_id = nil
+      child.save!
+    end
     child.reload
     parent1.reload
     parent2.reload
@@ -110,8 +125,11 @@ class IssueNestedSetTest < ActiveSupport::TestCase
     lft2 = new_issue_lft
     parent2 = Issue.generate!
     child = parent1.generate_child!
-    child.parent_issue_id = parent2.id
-    child.save!
+    assert_difference 'Journal.count', 3 do
+      child.init_journal(User.find(2))
+      child.parent_issue_id = parent2.id
+      child.save!
+    end
     child.reload
     parent1.reload
     parent2.reload
@@ -154,8 +172,13 @@ class IssueNestedSetTest < ActiveSupport::TestCase
     grandchild = child.generate_child!
     lft4 = new_issue_lft
     child.reload
-    child.project = Project.find(2)
-    assert child.save
+    assert_difference 'Journal.count', 2 do
+      assert_difference 'JournalDetail.count', 3 do
+        child.init_journal(User.find(2))
+        child.project = Project.find(2)
+        assert child.save
+      end
+    end
     child.reload
     grandchild.reload
     parent1.reload
@@ -173,8 +196,11 @@ class IssueNestedSetTest < ActiveSupport::TestCase
     grandchild = child.generate_child!
 
     child.reload
-    child.parent_issue_id = grandchild.id
-    assert !child.save
+    assert_no_difference 'Journal.count' do
+      child.init_journal(User.find(2))
+      child.parent_issue_id = grandchild.id
+      assert !child.save
+    end
     assert_not_equal [], child.errors[:parent_issue_id]
   end
 
@@ -223,8 +249,8 @@ class IssueNestedSetTest < ActiveSupport::TestCase
     issue3.subject = 'child with journal'
     issue3.save!
     assert_difference 'Issue.count', -2 do
-      assert_difference 'Journal.count', -1 do
-        assert_difference 'JournalDetail.count', -1 do
+      assert_difference 'Journal.count', -2 do
+        assert_difference 'JournalDetail.count', -2 do
           Issue.find(issue2.id).destroy
         end
       end
@@ -244,7 +270,9 @@ class IssueNestedSetTest < ActiveSupport::TestCase
     child2 = issue.generate_child!
     issue.reload
     assert_equal [issue.id, lft1, lft1 + 5], [issue.root_id, issue.lft, issue.rgt]
-    child2.reload.destroy
+    assert_difference 'Journal.count', 1 do
+      child2.reload.destroy
+    end
     issue.reload
     assert_equal [issue.id, lft1, lft1 + 3], [issue.root_id, issue.lft, issue.rgt]
   end
@@ -255,7 +283,9 @@ class IssueNestedSetTest < ActiveSupport::TestCase
     parent.generate_child!(:start_date => 2.days.from_now)
 
     assert_difference 'Issue.count', -3 do
-      Issue.find(parent.id).destroy
+      assert_difference 'Journal.count', -2 do
+        Issue.find(parent.id).destroy
+      end
     end
   end
 
@@ -287,7 +317,9 @@ class IssueNestedSetTest < ActiveSupport::TestCase
     grandchild1 = child.generate_child!
     grandchild2 = child.generate_child!
     assert_difference 'Issue.count', -4 do
-      Issue.find(issue.id).destroy
+      assert_difference 'Journal.count', -2 do
+        Issue.find(issue.id).destroy
+      end
       parent.reload
       assert_equal [lft1, lft1 + 1], [parent.lft, parent.rgt]
     end
diff --git a/test/unit/issue_subtasking_test.rb b/test/unit/issue_subtasking_test.rb
index 04f8aa301..fadbd21fa 100644
--- a/test/unit/issue_subtasking_test.rb
+++ b/test/unit/issue_subtasking_test.rb
@@ -277,6 +277,7 @@ class IssueSubtaskingTest < ActiveSupport::TestCase
     with_settings :issue_done_ratio => 'issue_status' do
       status = IssueStatus.find(4)
       status.update_attribute :default_done_ratio, 50
+      child1.reload
       child1.update_attribute :status, status
 
       assert_equal 50, child1.done_ratio
diff --git a/test/unit/mail_handler_test.rb b/test/unit/mail_handler_test.rb
index 3fd3ce072..999cb8cea 100644
--- a/test/unit/mail_handler_test.rb
+++ b/test/unit/mail_handler_test.rb
@@ -433,10 +433,19 @@ class MailHandlerTest < ActiveSupport::TestCase
     assert issue.is_a?(Issue)
     assert !issue.new_record?
 
-    mail = ActionMailer::Base.deliveries.last
-    assert_not_nil mail
-    assert mail.subject.include?("##{issue.id}")
-    assert mail.subject.include?('New ticket on a given project')
+    assert_equal 4, issue.parent_issue_id
+    assert_equal 2, ActionMailer::Base.deliveries.size
+
+    [
+      [issue.id, 'New ticket on a given project'],
+      [4, 'Issue on project 2'],
+    ].each do |issue_id, issue_subject|
+      mail =
+        ActionMailer::Base.deliveries.detect do |m|
+          /##{issue_id}/.match?(m.subject) && /#{issue_subject}/.match?(m.subject)
+        end
+      assert_not_nil mail
+    end
   end
 
   def test_created_user_should_be_added_to_groups