Fixed that setting a status as closed should update issue closed_on attribute (#18280).

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

2 files changed, 52 insertions, 0 deletions

diff --git a/app/models/issue_status.rb b/app/models/issue_status.rb
index b8eb6d40e..e14cf5a0a 100644
--- a/app/models/issue_status.rb
+++ b/app/models/issue_status.rb
@@ -21,6 +21,7 @@ class IssueStatus < ActiveRecord::Base
   has_many :workflow_transitions_as_new_status, :class_name => 'WorkflowTransition', :foreign_key => "new_status_id"
   acts_as_list
 
+  after_update :handle_is_closed_change
   before_destroy :delete_workflow_rules
 
   validates_presence_of :name
@@ -89,6 +90,23 @@ class IssueStatus < ActiveRecord::Base
 
   private
 
+  # Updates issues closed_on attribute when an existing status is set as closed.
+  def handle_is_closed_change
+    if is_closed_changed? && is_closed == true
+      # First we update issues that have a journal for when the current status was set,
+      # a subselect is used to update all issues with a single query
+      subselect = "SELECT MAX(j.created_on) FROM #{Journal.table_name} j" +
+        " JOIN #{JournalDetail.table_name} d ON d.journal_id = j.id" +
+        " WHERE j.journalized_type = 'Issue' AND j.journalized_id = #{Issue.table_name}.id" +
+        " AND d.property = 'attr' AND d.prop_key = 'status_id' AND d.value = :status_id"
+      Issue.where(:status_id => id, :closed_on => nil).update_all(["closed_on = (#{subselect})", :status_id => id.to_s])
+
+      # Then we update issues that don't have a journal which means the
+      # current status was set on creation
+      Issue.where(:status_id => id, :closed_on => nil).update_all("closed_on = created_on")
+    end
+  end
+
   def check_integrity
     if Issue.where(:status_id => id).any?
       raise "This status is used by some issues"
diff --git a/test/unit/issue_status_test.rb b/test/unit/issue_status_test.rb
index cb4ed90b8..71fcfa56e 100644
--- a/test/unit/issue_status_test.rb
+++ b/test/unit/issue_status_test.rb
@@ -96,4 +96,38 @@ class IssueStatusTest < ActiveSupport::TestCase
     assert_not_nil status
     assert_equal "Resolved", status.name
   end
+
+  def test_setting_status_as_closed_should_set_closed_on_for_issues_without_status_journal
+    issue = Issue.generate!(:status_id => 1, :created_on => 2.days.ago)
+    assert_nil issue.closed_on
+
+    issue.status.update! :is_closed => true
+
+    issue.reload
+    assert issue.closed?
+    assert_equal issue.created_on, issue.closed_on
+  end
+
+  def test_setting_status_as_closed_should_set_closed_on_for_issues_with_status_journal
+    issue = Issue.generate!(:status_id => 1, :created_on => 2.days.ago)
+    issue.init_journal(User.find(1))
+    issue.status_id = 2
+    issue.save!
+
+    issue.status.update! :is_closed => true
+
+    issue.reload
+    assert issue.closed?
+    assert_equal issue.journals.first.created_on, issue.closed_on
+  end
+
+  def test_setting_status_as_closed_should_not_set_closed_on_for_issues_with_other_status
+    issue = Issue.generate!(:status_id => 2)
+
+    IssueStatus.find(1).update! :is_closed => true
+
+    issue.reload
+    assert !issue.closed?
+    assert_nil issue.closed_on
+  end
 end