From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Sat, 16 Feb 2013 09:38:01 +0000 (+0000)
Subject: Adds closed_on column that stores the time of the last closing (#824).
X-Git-Tag: 2.3.0~115
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=a45c0dc55057e8c10c2d17f8dc26be5d9b771f8d;p=redmine.git

Adds closed_on column that stores the time of the last closing (#824).

The value is preserved when reopening the issue.

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@11402 e93f8b46-1217-0410-a6f0-8f06a7374b81
---

diff --git a/app/models/issue.rb b/app/models/issue.rb
index 5d2bf24c7..fd7cc0b50 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -91,7 +91,7 @@ class Issue < ActiveRecord::Base
   }
 
   before_create :default_assign
-  before_save :close_duplicates, :update_done_ratio_from_issue_status, :force_updated_on_change
+  before_save :close_duplicates, :update_done_ratio_from_issue_status, :force_updated_on_change, :update_closed_on
   after_save {|issue| issue.send :after_project_change if !issue.id_changed? && issue.project_id_changed?} 
   after_save :reschedule_following_issues, :update_nested_set_attributes, :update_parent_attributes, :create_journal
   # Should be after_create but would be called before previous after_save callbacks
@@ -1307,10 +1307,23 @@ class Issue < ActiveRecord::Base
     end
   end
 
-  # Make sure updated_on is updated when adding a note
+  # Make sure updated_on is updated when adding a note and set updated_on now
+  # so we can set closed_on with the same value on closing
   def force_updated_on_change
-    if @current_journal
+    if @current_journal || changed?
       self.updated_on = current_time_from_proper_timezone
+      if new_record?
+        self.created_on = updated_on
+      end
+    end
+  end
+
+  # Callback for setting closed_on when the issue is closed.
+  # The closed_on attribute stores the time of the last closing
+  # and is preserved when the issue is reopened.
+  def update_closed_on
+    if closing? || (new_record? && closed?)
+      self.closed_on = updated_on
     end
   end
 
@@ -1320,7 +1333,7 @@ class Issue < ActiveRecord::Base
     if @current_journal
       # attributes changes
       if @attributes_before_change
-        (Issue.column_names - %w(id root_id lft rgt lock_version created_on updated_on)).each {|c|
+        (Issue.column_names - %w(id root_id lft rgt lock_version created_on updated_on closed_on)).each {|c|
           before = @attributes_before_change[c]
           after = send(c)
           next if before == after || (before.blank? && after.blank?)
diff --git a/db/migrate/20130215111127_add_issues_closed_on.rb b/db/migrate/20130215111127_add_issues_closed_on.rb
new file mode 100644
index 000000000..2670deba5
--- /dev/null
+++ b/db/migrate/20130215111127_add_issues_closed_on.rb
@@ -0,0 +1,9 @@
+class AddIssuesClosedOn < ActiveRecord::Migration
+  def up
+    add_column :issues, :closed_on, :datetime, :default => nil
+  end
+
+  def down
+    remove_column :issues, :closed_on
+  end
+end
diff --git a/db/migrate/20130215111141_populate_issues_closed_on.rb b/db/migrate/20130215111141_populate_issues_closed_on.rb
new file mode 100644
index 000000000..b2a828be4
--- /dev/null
+++ b/db/migrate/20130215111141_populate_issues_closed_on.rb
@@ -0,0 +1,25 @@
+class PopulateIssuesClosedOn < ActiveRecord::Migration
+  def up
+    closed_status_ids = IssueStatus.where(:is_closed => true).pluck(:id)
+    if closed_status_ids.any?
+      # First set closed_on for issues that have been closed once
+      closed_status_values = closed_status_ids.map {|status_id| "'#{status_id}'"}.join(',')
+      subselect = "SELECT MAX(#{Journal.table_name}.created_on)" +
+        " FROM #{Journal.table_name}, #{JournalDetail.table_name}" +
+        " WHERE #{Journal.table_name}.id = #{JournalDetail.table_name}.journal_id" +
+        " AND #{Journal.table_name}.journalized_type = 'Issue' AND #{Journal.table_name}.journalized_id = #{Issue.table_name}.id" +
+        " AND #{JournalDetail.table_name}.property = 'attr' AND #{JournalDetail.table_name}.prop_key = 'status_id'" +
+        " AND #{JournalDetail.table_name}.old_value NOT IN (#{closed_status_values})" +
+        " AND #{JournalDetail.table_name}.value IN (#{closed_status_values})"
+      Issue.update_all "closed_on = (#{subselect})"
+
+      # Then set closed_on for closed issues that weren't up updated by the above UPDATE
+      # No journal was found so we assume that they were closed on creation
+      Issue.update_all "closed_on = created_on", {:status_id => closed_status_ids, :closed_on => nil}
+    end
+  end
+
+  def down
+    Issue.update_all :closed_on => nil
+  end
+end
diff --git a/test/fixtures/issues.yml b/test/fixtures/issues.yml
index 531089de0..8eb8ae1a5 100644
--- a/test/fixtures/issues.yml
+++ b/test/fixtures/issues.yml
@@ -152,6 +152,7 @@ issues_008:
   root_id: 8
   lft: 1
   rgt: 2
+  closed_on: <%= 3.days.ago.to_s(:db) %>
 issues_009: 
   created_on: <%= 1.minute.ago.to_s(:db) %>
   project_id: 5
@@ -209,6 +210,7 @@ issues_011:
   root_id: 11
   lft: 1
   rgt: 2
+  closed_on: <%= 1.day.ago.to_s(:db) %>
 issues_012: 
   created_on: <%= 3.days.ago.to_s(:db) %>
   project_id: 1
@@ -228,6 +230,7 @@ issues_012:
   root_id: 12
   lft: 1
   rgt: 2
+  closed_on: <%= 1.day.ago.to_s(:db) %>
 issues_013:
   created_on: <%= 5.days.ago.to_s(:db) %>
   project_id: 3
diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb
index 1ffc24dd5..ad88933e5 100644
--- a/test/unit/issue_test.rb
+++ b/test/unit/issue_test.rb
@@ -1917,4 +1917,58 @@ class IssueTest < ActiveSupport::TestCase
     assert_equal 3, issue.reload.attachments.count
     assert_equal %w(upload foo bar), issue.attachments.map(&:filename)
   end
+
+  def test_closed_on_should_be_nil_when_creating_an_open_issue
+    issue = Issue.generate!(:status_id => 1).reload
+    assert !issue.closed?
+    assert_nil issue.closed_on
+  end
+
+  def test_closed_on_should_be_set_when_creating_a_closed_issue
+    issue = Issue.generate!(:status_id => 5).reload
+    assert issue.closed?
+    assert_not_nil issue.closed_on
+    assert_equal issue.updated_on, issue.closed_on
+    assert_equal issue.created_on, issue.closed_on
+  end
+
+  def test_closed_on_should_be_nil_when_updating_an_open_issue
+    issue = Issue.find(1)
+    issue.subject = 'Not closed yet'
+    issue.save!
+    issue.reload
+    assert_nil issue.closed_on
+  end
+
+  def test_closed_on_should_be_set_when_closing_an_open_issue
+    issue = Issue.find(1)
+    issue.subject = 'Now closed'
+    issue.status_id = 5
+    issue.save!
+    issue.reload
+    assert_not_nil issue.closed_on
+    assert_equal issue.updated_on, issue.closed_on
+  end
+
+  def test_closed_on_should_not_be_updated_when_updating_a_closed_issue
+    issue = Issue.open(false).first
+    was_closed_on = issue.closed_on
+    assert_not_nil was_closed_on
+    issue.subject = 'Updating a closed issue'
+    issue.save!
+    issue.reload
+    assert_equal was_closed_on, issue.closed_on
+  end
+
+  def test_closed_on_should_be_preserved_when_reopening_a_closed_issue
+    issue = Issue.open(false).first
+    was_closed_on = issue.closed_on
+    assert_not_nil was_closed_on
+    issue.subject = 'Reopening a closed issue'
+    issue.status_id = 1
+    issue.save!
+    issue.reload
+    assert !issue.closed?
+    assert_equal was_closed_on, issue.closed_on
+  end
 end