From a45c0dc55057e8c10c2d17f8dc26be5d9b771f8d Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sat, 16 Feb 2013 09:38:01 +0000 Subject: [PATCH] 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 --- app/models/issue.rb | 21 ++++++-- .../20130215111127_add_issues_closed_on.rb | 9 ++++ ...0130215111141_populate_issues_closed_on.rb | 25 +++++++++ test/fixtures/issues.yml | 3 ++ test/unit/issue_test.rb | 54 +++++++++++++++++++ 5 files changed, 108 insertions(+), 4 deletions(-) create mode 100644 db/migrate/20130215111127_add_issues_closed_on.rb create mode 100644 db/migrate/20130215111141_populate_issues_closed_on.rb 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