jro_apps/redmine Commit - r13178:5052d48491a7

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

Jean-Philippe Lang -

r13178:5052d48491a7

parent child

Context file:

r13178:5052d48491a7

Collapse all files

app/models/issue_status.rb +18 0

             # Redmine - project management software
             # Copyright (C) 2006-2014  Jean-Philippe Lang
             #
             # This program is free software; you can redistribute it and/or
             # modify it under the terms of the GNU General Public License
             # as published by the Free Software Foundation; either version 2
             # of the License, or (at your option) any later version.
             #
             # This program is distributed in the hope that it will be useful,
             # but WITHOUT ANY WARRANTY; without even the implied warranty of
             # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
             # GNU General Public License for more details.
             #
             # You should have received a copy of the GNU General Public License
             # along with this program; if not, write to the Free Software
             # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
             class IssueStatus < ActiveRecord::Base
               before_destroy :check_integrity
               has_many :workflows, :class_name => 'WorkflowTransition', :foreign_key => "old_status_id"
               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
               validates_uniqueness_of :name
               validates_length_of :name, :maximum => 30
               validates_inclusion_of :default_done_ratio, :in => 0..100, :allow_nil => true
               attr_protected :id
               scope :sorted, lambda { order(:position) }
               scope :named, lambda {|arg| where("LOWER(#{table_name}.name) = LOWER(?)", arg.to_s.strip)}
               # Update all the +Issues+ setting their done_ratio to the value of their +IssueStatus+
               def self.update_issue_done_ratios
                 if Issue.use_status_for_done_ratio?
                   IssueStatus.where("default_done_ratio >= 0").each do |status|
                     Issue.where({:status_id => status.id}).update_all({:done_ratio => status.default_done_ratio})
                   end
                 end
                 return Issue.use_status_for_done_ratio?
               end
               # Returns an array of all statuses the given role can switch to
               # Uses association cache when called more than one time
               def new_statuses_allowed_to(roles, tracker, author=false, assignee=false)
                 if roles && tracker
                   role_ids = roles.collect(&:id)
                   transitions = workflows.select do |w|
                     role_ids.include?(w.role_id) &&
                     w.tracker_id == tracker.id &&
                     ((!w.author && !w.assignee) || (author && w.author) || (assignee && w.assignee))
                   end
                   transitions.map(&:new_status).compact.sort
                 else
                   []
                 end
               end
               # Same thing as above but uses a database query
               # More efficient than the previous method if called just once
               def find_new_statuses_allowed_to(roles, tracker, author=false, assignee=false)
                 if roles.present? && tracker
                   scope = IssueStatus.
                     joins(:workflow_transitions_as_new_status).
                     where(:workflows => {:old_status_id => id, :role_id => roles.map(&:id), :tracker_id => tracker.id})
                   unless author && assignee
                     if author || assignee
                       scope = scope.where("author = ? OR assignee = ?", author, assignee)
                     else
                       scope = scope.where("author = ? AND assignee = ?", false, false)
                     end
                   end
                   scope.uniq.to_a.sort
                 else
                   []
                 end
               end
               def <=>(status)
                 position <=> status.position
               end
               def to_s; name end
               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"
                 elsif Tracker.where(:default_status_id => id).any?
                   raise "This status is used as the default status by some trackers"
                 end
               end
               # Deletes associated workflows
               def delete_workflow_rules
                 WorkflowRule.delete_all(["old_status_id = :id OR new_status_id = :id", {:id => id}])
               end
             end

test/unit/issue_status_test.rb +34 0

             # Redmine - project management software
             # Copyright (C) 2006-2014  Jean-Philippe Lang
             #
             # This program is free software; you can redistribute it and/or
             # modify it under the terms of the GNU General Public License
             # as published by the Free Software Foundation; either version 2
             # of the License, or (at your option) any later version.
             #
             # This program is distributed in the hope that it will be useful,
             # but WITHOUT ANY WARRANTY; without even the implied warranty of
             # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
             # GNU General Public License for more details.
             #
             # You should have received a copy of the GNU General Public License
             # along with this program; if not, write to the Free Software
             # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
             require File.expand_path('../../test_helper', __FILE__)
             class IssueStatusTest < ActiveSupport::TestCase
               fixtures :issue_statuses, :issues, :roles, :trackers
               def test_create
                 status = IssueStatus.new :name => "Assigned"
                 assert !status.save
                 # status name uniqueness
                 assert_equal 1, status.errors.count
                 status.name = "Test Status"
                 assert status.save
               end
               def test_destroy
                 status = IssueStatus.find(3)
                 assert_difference 'IssueStatus.count', -1 do
                   assert status.destroy
                 end
                 assert_nil WorkflowTransition.where(:old_status_id => status.id).first
                 assert_nil WorkflowTransition.where(:new_status_id => status.id).first
               end
               def test_destroy_status_in_use
                 # Status assigned to an Issue
                 status = Issue.find(1).status
                 assert_raise(RuntimeError, "Can't delete status") { status.destroy }
               end
               def test_new_statuses_allowed_to
                 WorkflowTransition.delete_all
                 WorkflowTransition.create!(:role_id => 1, :tracker_id => 1, :old_status_id => 1, :new_status_id => 2, :author => false, :assignee => false)
                 WorkflowTransition.create!(:role_id => 1, :tracker_id => 1, :old_status_id => 1, :new_status_id => 3, :author => true, :assignee => false)
                 WorkflowTransition.create!(:role_id => 1, :tracker_id => 1, :old_status_id => 1, :new_status_id => 4, :author => false, :assignee => true)
                 WorkflowTransition.create!(:role_id => 1, :tracker_id => 1, :old_status_id => 1, :new_status_id => 5, :author => true, :assignee => true)
                 status = IssueStatus.find(1)
                 role = Role.find(1)
                 tracker = Tracker.find(1)
                 assert_equal [2], status.new_statuses_allowed_to([role], tracker, false, false).map(&:id)
                 assert_equal [2], status.find_new_statuses_allowed_to([role], tracker, false, false).map(&:id)
                 assert_equal [2, 3, 5], status.new_statuses_allowed_to([role], tracker, true, false).map(&:id)
                 assert_equal [2, 3, 5], status.find_new_statuses_allowed_to([role], tracker, true, false).map(&:id)
                 assert_equal [2, 4, 5], status.new_statuses_allowed_to([role], tracker, false, true).map(&:id)
                 assert_equal [2, 4, 5], status.find_new_statuses_allowed_to([role], tracker, false, true).map(&:id)
                 assert_equal [2, 3, 4, 5], status.new_statuses_allowed_to([role], tracker, true, true).map(&:id)
                 assert_equal [2, 3, 4, 5], status.find_new_statuses_allowed_to([role], tracker, true, true).map(&:id)
               end
               def test_update_done_ratios_with_issue_done_ratio_set_to_issue_field_should_change_nothing
                 IssueStatus.find(1).update_attribute(:default_done_ratio, 50)
                 with_settings :issue_done_ratio => 'issue_field' do
                   IssueStatus.update_issue_done_ratios
                   assert_equal 0, Issue.where(:done_ratio => 50).count
                 end
               end
               def test_update_done_ratios_with_issue_done_ratio_set_to_issue_status_should_update_issues
                 IssueStatus.find(1).update_attribute(:default_done_ratio, 50)
                 with_settings :issue_done_ratio => 'issue_status' do
                   IssueStatus.update_issue_done_ratios
                   issues = Issue.where(:status_id => 1)
                   assert_equal [50], issues.map {|issue| issue.read_attribute(:done_ratio)}.uniq
                 end
               end
               def test_sorted_scope
                 assert_equal IssueStatus.all.sort, IssueStatus.sorted.to_a
               end
               def test_named_scope
                 status = IssueStatus.named("resolved").first
                 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

General Comments 0

Write
Preview

You need to be logged in to leave comments. Login now

No TODOs yet

	Site-wide shortcuts
/	Use quick search box
g h	Goto home page
g g	Goto my private gists page
g G	Goto my public gists page
g 0-9	Goto bookmarked items from 0-9
n r	New repository page
n g	New gist page

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages