diff app/models/issue.rb @ 1464:261b3d9a4903 redmine-2.4

Update to Redmine 2.4 branch rev 12663

--- a/app/models/issue.rb	Fri Jun 14 09:05:06 2013 +0100
+++ b/app/models/issue.rb	Tue Jan 14 14:37:42 2014 +0000
@@ -1,5 +1,5 @@
 # Redmine - project management software
-# Copyright (C) 2006-2012  Jean-Philippe Lang
+# Copyright (C) 2006-2013  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
@@ -18,6 +18,7 @@
 class Issue < ActiveRecord::Base
   include Redmine::SafeAttributes
   include Redmine::Utils::DateCalculation
+  include Redmine::I18n
 
   belongs_to :project
   belongs_to :tracker
@@ -67,29 +68,41 @@
 
   validates_length_of :subject, :maximum => 255
   validates_inclusion_of :done_ratio, :in => 0..100
-  validates_numericality_of :estimated_hours, :allow_nil => true
+  validates :estimated_hours, :numericality => {:greater_than_or_equal_to => 0, :allow_nil => true, :message => :invalid}
+  validates :start_date, :date => true
+  validates :due_date, :date => true
   validate :validate_issue, :validate_required_fields
 
-  scope :visible,
-        lambda {|*args| { :include => :project,
-                          :conditions => Issue.visible_condition(args.shift || User.current, *args) } }
+  scope :visible, lambda {|*args|
+    includes(:project).where(Issue.visible_condition(args.shift || User.current, *args))
+  }
 
   scope :open, lambda {|*args|
     is_closed = args.size > 0 ? !args.first : false
-    {:conditions => ["#{IssueStatus.table_name}.is_closed = ?", is_closed], :include => :status}
+    includes(:status).where("#{IssueStatus.table_name}.is_closed = ?", is_closed)
   }
 
-  scope :recently_updated, :order => "#{Issue.table_name}.updated_on DESC"
-  scope :on_active_project, :include => [:status, :project, :tracker],
-                            :conditions => ["#{Project.table_name}.status=#{Project::STATUS_ACTIVE}"]
+  scope :recently_updated, lambda { order("#{Issue.table_name}.updated_on DESC") }
+  scope :on_active_project, lambda {
+    includes(:status, :project, :tracker).where("#{Project.table_name}.status = ?", Project::STATUS_ACTIVE)
+  }
+  scope :fixed_version, lambda {|versions|
+    ids = [versions].flatten.compact.map {|v| v.is_a?(Version) ? v.id : v}
+    ids.any? ? where(:fixed_version_id => ids) : where('1=0')
+  }
 
   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, :set_assigned_to_was
   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
+  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
   after_save :after_create_from_copy
   after_destroy :update_parent_attributes
+  after_create :send_notification
+  # Keep it at the end of after_save callbacks
+  after_save :clear_assigned_to_was
 
   # Returns a SQL conditions string used to find all issues visible by the specified user
   def self.visible_condition(user, options={})
@@ -99,10 +112,10 @@
         when 'all'
           nil
         when 'default'
-          user_ids = [user.id] + user.groups.map(&:id)
+          user_ids = [user.id] + user.groups.map(&:id).compact
           "(#{table_name}.is_private = #{connection.quoted_false} OR #{table_name}.author_id = #{user.id} OR #{table_name}.assigned_to_id IN (#{user_ids.join(',')}))"
         when 'own'
-          user_ids = [user.id] + user.groups.map(&:id)
+          user_ids = [user.id] + user.groups.map(&:id).compact
           "(#{table_name}.author_id = #{user.id} OR #{table_name}.assigned_to_id IN (#{user_ids.join(',')}))"
         else
           '1=0'
@@ -133,6 +146,11 @@
     end
   end
 
+  # Returns true if user or current user is allowed to edit or add a note to the issue
+  def editable?(user=User.current)
+    user.allowed_to?(:edit_issues, project) || user.allowed_to?(:add_issue_notes, project)
+  end
+
   def initialize(attributes=nil, *args)
     super
     if new_record?
@@ -143,6 +161,13 @@
     end
   end
 
+  def create_or_update
+    super
+  ensure
+    @status_was = nil
+  end
+  private :create_or_update
+
   # AR#Persistence#destroy would raise and RecordNotFound exception
   # if the issue was already deleted or updated (non matching lock_version).
   # This is a problem when bulk deleting issues or deleting a project
@@ -165,10 +190,12 @@
     super
   end
 
+  alias :base_reload :reload
   def reload(*args)
     @workflow_rule_by_attribute = nil
     @assignable_versions = nil
-    super
+    @relations = nil
+    base_reload(*args)
   end
 
   # Overrides Redmine::Acts::Customizable::InstanceMethods#available_custom_fields
@@ -176,6 +203,13 @@
     (project && tracker) ? (project.all_issue_custom_fields & tracker.custom_fields.all) : []
   end
 
+  def visible_custom_field_values(user=nil)
+    user_real = user || User.current
+    custom_field_values.select do |value|
+      value.custom_field.visible_by?(project, user_real)
+    end
+  end
+
   # Copies attributes from another issue, arg can be an id or an Issue
   def copy_from(arg, options={})
     issue = arg.is_a?(Issue) ? arg : Issue.visible.find(arg)
@@ -326,8 +360,7 @@
       if issue.new_record?
         issue.copy?
       elsif user.allowed_to?(:move_issues, issue.project)
-        projects = Issue.allowed_target_projects_on_move(user)
-        projects.include?(issue.project) && projects.size > 1
+        Issue.allowed_target_projects_on_move.count > 1
       end
     }
 
@@ -394,7 +427,7 @@
 
     # Project and Tracker must be set before since new_statuses_allowed_to depends on it.
     if (p = attrs.delete('project_id')) && safe_attribute?('project_id')
-      if allowed_target_projects(user).collect(&:id).include?(p.to_i)
+      if allowed_target_projects(user).where(:id => p.to_i).exists?
         self.project_id = p
       end
     end
@@ -424,11 +457,15 @@
     end
 
     if attrs['custom_field_values'].present?
-      attrs['custom_field_values'] = attrs['custom_field_values'].reject {|k, v| read_only_attribute_names(user).include? k.to_s}
+      editable_custom_field_ids = editable_custom_field_values(user).map {|v| v.custom_field_id.to_s}
+      # TODO: use #select when ruby1.8 support is dropped
+      attrs['custom_field_values'] = attrs['custom_field_values'].reject {|k, v| !editable_custom_field_ids.include?(k.to_s)}
     end
 
     if attrs['custom_fields'].present?
-      attrs['custom_fields'] = attrs['custom_fields'].reject {|c| read_only_attribute_names(user).include? c['id'].to_s}
+      editable_custom_field_ids = editable_custom_field_values(user).map {|v| v.custom_field_id.to_s}
+      # TODO: use #select when ruby1.8 support is dropped
+      attrs['custom_fields'] = attrs['custom_fields'].reject {|c| !editable_custom_field_ids.include?(c['id'].to_s)}
     end
 
     # mass-assignment security bypass
@@ -441,7 +478,7 @@
 
   # Returns the custom_field_values that can be edited by the given user
   def editable_custom_field_values(user=nil)
-    custom_field_values.reject do |value|
+    visible_custom_field_values(user).reject do |value|
       read_only_attribute_names(user).include?(value.custom_field_id.to_s)
     end
   end
@@ -526,20 +563,12 @@
   end
 
   def validate_issue
-    if due_date.nil? && @attributes['due_date'].present?
-      errors.add :due_date, :not_a_date
-    end
-
-    if start_date.nil? && @attributes['start_date'].present?
-      errors.add :start_date, :not_a_date
-    end
-
-    if due_date && start_date && due_date < start_date
+    if due_date && start_date && (start_date_changed? || due_date_changed?) && due_date < start_date
       errors.add :due_date, :greater_than_start_date
     end
 
-    if start_date && soonest_start && start_date < soonest_start
-      errors.add :start_date, :invalid
+    if start_date && start_date_changed? && soonest_start && start_date < soonest_start
+      errors.add :start_date, :earlier_than_minimum_start_date, :date => format_date(soonest_start)
     end
 
     if fixed_version
@@ -563,6 +592,8 @@
     elsif @parent_issue
       if !valid_parent_project?(@parent_issue)
         errors.add :parent_issue_id, :invalid
+      elsif (@parent_issue != parent) && (all_dependent_issues.include?(@parent_issue) || @parent_issue.all_dependent_issues.include?(self))
+        errors.add :parent_issue_id, :invalid
       elsif !new_record?
         # moving an existing issue
         if @parent_issue.root_id != root_id
@@ -633,6 +664,14 @@
     scope
   end
 
+  # Returns the initial status of the issue
+  # Returns nil for a new issue
+  def status_was
+    if status_id_was && status_id_was.to_i > 0
+      @status_was ||= IssueStatus.find_by_id(status_id_was)
+    end
+  end
+
   # Return true if the issue is closed, otherwise false
   def closed?
     self.status.is_closed?
@@ -653,9 +692,7 @@
   # Return true if the issue is being closed
   def closing?
     if !new_record? && status_id_changed?
-      status_was = IssueStatus.find_by_id(status_id_was)
-      status_new = IssueStatus.find_by_id(status_id)
-      if status_was && status_new && !status_was.is_closed? && status_new.is_closed?
+      if status_was && status && !status_was.is_closed? && status.is_closed?
         return true
       end
     end
@@ -670,7 +707,7 @@
   # Is the amount of work done less than it should for the due date
   def behind_schedule?
     return false if start_date.nil? || due_date.nil?
-    done_date = start_date + ((due_date - start_date+1)* done_ratio/100).floor
+    done_date = start_date + ((due_date - start_date + 1) * done_ratio / 100).floor
     return done_date <= Date.today
   end
 
@@ -723,12 +760,16 @@
         initial_status = IssueStatus.find_by_id(status_id_was)
       end
       initial_status ||= status
-  
+
+      initial_assigned_to_id = assigned_to_id_changed? ? assigned_to_id_was : assigned_to_id
+      assignee_transitions_allowed = initial_assigned_to_id.present? && 
+        (user.id == initial_assigned_to_id || user.group_ids.include?(initial_assigned_to_id))
+
       statuses = initial_status.find_new_statuses_allowed_to(
         user.admin ? Role.all : user.roles_for_project(project),
         tracker,
         author == user,
-        assigned_to_id_changed? ? assigned_to_id_was == user.id : assigned_to_id == user.id
+        assignee_transitions_allowed
         )
       statuses << initial_status unless statuses.empty?
       statuses << IssueStatus.default if include_default
@@ -737,9 +778,12 @@
     end
   end
 
+  # Returns the previous assignee if changed
   def assigned_to_was
-    if assigned_to_id_changed? && assigned_to_id_was.present?
-      @assigned_to_was ||= User.find_by_id(assigned_to_id_was)
+    # assigned_to_id_was is reset before after_save callbacks
+    user_id = @previous_assigned_to_id || assigned_to_id_was
+    if user_id && user_id != assigned_to_id
+      @assigned_to_was ||= User.find_by_id(user_id)
     end
   end
 
@@ -769,6 +813,21 @@
     notified_users.collect(&:mail)
   end
 
+  def each_notification(users, &block)
+    if users.any?
+      if custom_field_values.detect {|value| !value.custom_field.visible?}
+        users_by_custom_field_visibility = users.group_by do |user|
+          visible_custom_field_values(user).map(&:custom_field_id).sort
+        end
+        users_by_custom_field_visibility.values.each do |users|
+          yield(users)
+        end
+      else
+        yield(users)
+      end
+    end
+  end
+
   # Returns the number of hours spent on this issue
   def spent_hours
     @spent_hours ||= time_entries.sum(:hours) || 0
@@ -785,13 +844,13 @@
   end
 
   def relations
-    @relations ||= IssueRelations.new(self, (relations_from + relations_to).sort)
+    @relations ||= IssueRelation::Relations.new(self, (relations_from + relations_to).sort)
   end
 
   # Preloads relations for a collection of issues
   def self.load_relations(issues)
     if issues.any?
-      relations = IssueRelation.all(:conditions => ["issue_from_id IN (:ids) OR issue_to_id IN (:ids)", {:ids => issues.map(&:id)}])
+      relations = IssueRelation.where("issue_from_id IN (:ids) OR issue_to_id IN (:ids)", :ids => issues.map(&:id)).all
       issues.each do |issue|
         issue.instance_variable_set "@relations", relations.select {|r| r.issue_from_id == issue.id || r.issue_to_id == issue.id}
       end
@@ -801,7 +860,7 @@
   # Preloads visible spent time for a collection of issues
   def self.load_visible_spent_hours(issues, user=User.current)
     if issues.any?
-      hours_by_issue_id = TimeEntry.visible(user).sum(:hours, :group => :issue_id)
+      hours_by_issue_id = TimeEntry.visible(user).group(:issue_id).sum(:hours)
       issues.each do |issue|
         issue.instance_variable_set "@spent_hours", (hours_by_issue_id[issue.id] || 0)
       end
@@ -822,25 +881,110 @@
           relations_from.select {|relation| relation.issue_from_id == issue.id} +
           relations_to.select {|relation| relation.issue_to_id == issue.id}
 
-        issue.instance_variable_set "@relations", IssueRelations.new(issue, relations.sort)
+        issue.instance_variable_set "@relations", IssueRelation::Relations.new(issue, relations.sort)
       end
     end
   end
 
   # Finds an issue relation given its id.
   def find_relation(relation_id)
-    IssueRelation.find(relation_id, :conditions => ["issue_to_id = ? OR issue_from_id = ?", id, id])
+    IssueRelation.where("issue_to_id = ? OR issue_from_id = ?", id, id).find(relation_id)
   end
 
+  # Returns all the other issues that depend on the issue
+  # The algorithm is a modified breadth first search (bfs)
   def all_dependent_issues(except=[])
-    except << self
+    # The found dependencies
     dependencies = []
-    relations_from.each do |relation|
-      if relation.issue_to && !except.include?(relation.issue_to)
-        dependencies << relation.issue_to
-        dependencies += relation.issue_to.all_dependent_issues(except)
+
+    # The visited flag for every node (issue) used by the breadth first search
+    eNOT_DISCOVERED         = 0       # The issue is "new" to the algorithm, it has not seen it before.
+
+    ePROCESS_ALL            = 1       # The issue is added to the queue. Process both children and relations of
+                                      # the issue when it is processed.
+
+    ePROCESS_RELATIONS_ONLY = 2       # The issue was added to the queue and will be output as dependent issue,
+                                      # but its children will not be added to the queue when it is processed.
+
+    eRELATIONS_PROCESSED    = 3       # The related issues, the parent issue and the issue itself have been added to
+                                      # the queue, but its children have not been added.
+
+    ePROCESS_CHILDREN_ONLY  = 4       # The relations and the parent of the issue have been added to the queue, but
+                                      # the children still need to be processed.
+
+    eALL_PROCESSED          = 5       # The issue and all its children, its parent and its related issues have been
+                                      # added as dependent issues. It needs no further processing.
+
+    issue_status = Hash.new(eNOT_DISCOVERED)
+
+    # The queue
+    queue = []
+
+    # Initialize the bfs, add start node (self) to the queue
+    queue << self
+    issue_status[self] = ePROCESS_ALL
+
+    while (!queue.empty?) do
+      current_issue = queue.shift
+      current_issue_status = issue_status[current_issue]
+      dependencies << current_issue
+
+      # Add parent to queue, if not already in it.
+      parent = current_issue.parent
+      parent_status = issue_status[parent]
+
+      if parent && (parent_status == eNOT_DISCOVERED) && !except.include?(parent)
+        queue << parent
+        issue_status[parent] = ePROCESS_RELATIONS_ONLY
       end
-    end
+
+      # Add children to queue, but only if they are not already in it and
+      # the children of the current node need to be processed.
+      if (current_issue_status == ePROCESS_CHILDREN_ONLY || current_issue_status == ePROCESS_ALL)
+        current_issue.children.each do |child|
+          next if except.include?(child)
+
+          if (issue_status[child] == eNOT_DISCOVERED)
+            queue << child
+            issue_status[child] = ePROCESS_ALL
+          elsif (issue_status[child] == eRELATIONS_PROCESSED)
+            queue << child
+            issue_status[child] = ePROCESS_CHILDREN_ONLY
+          elsif (issue_status[child] == ePROCESS_RELATIONS_ONLY)
+            queue << child
+            issue_status[child] = ePROCESS_ALL
+          end
+        end
+      end
+
+      # Add related issues to the queue, if they are not already in it.
+      current_issue.relations_from.map(&:issue_to).each do |related_issue|
+        next if except.include?(related_issue)
+
+        if (issue_status[related_issue] == eNOT_DISCOVERED)
+          queue << related_issue
+          issue_status[related_issue] = ePROCESS_ALL
+        elsif (issue_status[related_issue] == eRELATIONS_PROCESSED)
+          queue << related_issue
+          issue_status[related_issue] = ePROCESS_CHILDREN_ONLY
+        elsif (issue_status[related_issue] == ePROCESS_RELATIONS_ONLY)
+          queue << related_issue
+          issue_status[related_issue] = ePROCESS_ALL
+        end
+      end
+
+      # Set new status for current issue
+      if (current_issue_status == ePROCESS_ALL) || (current_issue_status == ePROCESS_CHILDREN_ONLY)
+        issue_status[current_issue] = eALL_PROCESSED
+      elsif (current_issue_status == ePROCESS_RELATIONS_ONLY)
+        issue_status[current_issue] = eRELATIONS_PROCESSED
+      end
+    end # while
+
+    # Remove the issues from the "except" parameter from the result array
+    dependencies -= except
+    dependencies.delete(self)
+
     dependencies
   end
 
@@ -873,7 +1017,7 @@
     @soonest_start = nil if reload
     @soonest_start ||= (
         relations_to(reload).collect{|relation| relation.successor_soonest_start} +
-        ancestors.collect(&:soonest_start)
+        [(@parent_issue || parent).try(:soonest_start)]
       ).compact.max
   end
 
@@ -935,42 +1079,21 @@
   end
 
   # Returns a string of css classes that apply to the issue
-  def css_classes
-    s = "issue status-#{status_id} #{priority.try(:css_classes)}"
+  def css_classes(user=User.current)
+    s = "issue tracker-#{tracker_id} status-#{status_id} #{priority.try(:css_classes)}"
     s << ' closed' if closed?
     s << ' overdue' if overdue?
     s << ' child' if child?
     s << ' parent' unless leaf?
     s << ' private' if is_private?
-    s << ' created-by-me' if User.current.logged? && author_id == User.current.id
-    s << ' assigned-to-me' if User.current.logged? && assigned_to_id == User.current.id
+    if user.logged?
+      s << ' created-by-me' if author_id == user.id
+      s << ' assigned-to-me' if assigned_to_id == user.id
+      s << ' assigned-to-my-group' if user.groups.any? {|g| g.id = assigned_to_id}
+    end
     s
   end
 
-  # Saves an issue and a time_entry from the parameters
-  def save_issue_with_child_records(params, existing_time_entry=nil)
-    Issue.transaction do
-      if params[:time_entry] && (params[:time_entry][:hours].present? || params[:time_entry][:comments].present?) && User.current.allowed_to?(:log_time, project)
-        @time_entry = existing_time_entry || TimeEntry.new
-        @time_entry.project = project
-        @time_entry.issue = self
-        @time_entry.user = User.current
-        @time_entry.spent_on = User.current.today
-        @time_entry.attributes = params[:time_entry]
-        self.time_entries << @time_entry
-      end
-
-      # TODO: Rename hook
-      Redmine::Hook.call_hook(:controller_issues_edit_before_save, { :params => params, :issue => self, :time_entry => @time_entry, :journal => @current_journal})
-      if save
-        # TODO: Rename hook
-        Redmine::Hook.call_hook(:controller_issues_edit_after_save, { :params => params, :issue => self, :time_entry => @time_entry, :journal => @current_journal})
-      else
-        raise ActiveRecord::Rollback
-      end
-    end
-  end
-
   # Unassigns issues from +version+ if it's no longer shared with issue's project
   def self.update_versions_from_sharing_change(version)
     # Update issues assigned to the version
@@ -989,6 +1112,10 @@
     s = arg.to_s.strip.presence
     if s && (m = s.match(%r{\A#?(\d+)\z})) && (@parent_issue = Issue.find_by_id(m[1]))
       @parent_issue.id
+      @invalid_parent_issue_id = nil
+    elsif s.blank?
+      @parent_issue = nil
+      @invalid_parent_issue_id = nil
     else
       @parent_issue = nil
       @invalid_parent_issue_id = arg
@@ -1005,8 +1132,8 @@
     end
   end
 
-	# Returns true if issue's project is a valid
-	# parent issue project
+  # Returns true if issue's project is a valid
+  # parent issue project
   def valid_parent_project?(issue=parent)
     return true if issue.nil? || issue.project_id == project_id
 
@@ -1077,18 +1204,18 @@
   end
   # End ReportsController extraction
 
-  # Returns an array of projects that user can assign the issue to
+  # Returns a scope of projects that user can assign the issue to
   def allowed_target_projects(user=User.current)
     if new_record?
-      Project.all(:conditions => Project.allowed_to_condition(user, :add_issues))
+      Project.where(Project.allowed_to_condition(user, :add_issues))
     else
       self.class.allowed_target_projects_on_move(user)
     end
   end
 
-  # Returns an array of projects that user can move issues to
+  # Returns a scope of projects that user can move issues to
   def self.allowed_target_projects_on_move(user=User.current)
-    Project.all(:conditions => Project.allowed_to_condition(user, :move_issues))
+    Project.where(Project.allowed_to_condition(user, :move_issues))
   end
 
   private
@@ -1128,20 +1255,27 @@
     end
 
     unless @copied_from.leaf? || @copy_options[:subtasks] == false
-      @copied_from.children.each do |child|
+      copy_options = (@copy_options || {}).merge(:subtasks => false)
+      copied_issue_ids = {@copied_from.id => self.id}
+      @copied_from.reload.descendants.reorder("#{Issue.table_name}.lft").each do |child|
+        # Do not copy self when copying an issue as a descendant of the copied issue
+        next if child == self
+        # Do not copy subtasks of issues that were not copied
+        next unless copied_issue_ids[child.parent_id]
+        # Do not copy subtasks that are not visible to avoid potential disclosure of private data
         unless child.visible?
-          # Do not copy subtasks that are not visible to avoid potential disclosure of private data
           logger.error "Subtask ##{child.id} was not copied during ##{@copied_from.id} copy because it is not visible to the current user" if logger
           next
         end
-        copy = Issue.new.copy_from(child, @copy_options)
+        copy = Issue.new.copy_from(child, copy_options)
         copy.author = author
         copy.project = project
-        copy.parent_issue_id = id
-        # Children subtasks are copied recursively
+        copy.parent_issue_id = copied_issue_ids[child.parent_id]
         unless copy.save
           logger.error "Could not copy subtask ##{child.id} while copying ##{@copied_from.id} to ##{id} due to validation errors: #{copy.errors.full_messages.join(', ')}" if logger
+          next
         end
+        copied_issue_ids[child.id] = copy.id
       end
     end
     @after_create_from_copy_handled = true
@@ -1152,48 +1286,50 @@
       # issue was just created
       self.root_id = (@parent_issue.nil? ? id : @parent_issue.root_id)
       set_default_left_and_right
-      Issue.update_all("root_id = #{root_id}, lft = #{lft}, rgt = #{rgt}", ["id = ?", id])
+      Issue.update_all(["root_id = ?, lft = ?, rgt = ?", root_id, lft, rgt], ["id = ?", id])
       if @parent_issue
         move_to_child_of(@parent_issue)
       end
-      reload
     elsif parent_issue_id != parent_id
-      former_parent_id = parent_id
-      # moving an existing issue
-      if @parent_issue && @parent_issue.root_id == root_id
-        # inside the same tree
-        move_to_child_of(@parent_issue)
-      else
-        # to another tree
-        unless root?
-          move_to_right_of(root)
-          reload
-        end
-        old_root_id = root_id
-        self.root_id = (@parent_issue.nil? ? id : @parent_issue.root_id )
-        target_maxright = nested_set_scope.maximum(right_column_name) || 0
-        offset = target_maxright + 1 - lft
-        Issue.update_all("root_id = #{root_id}, lft = lft + #{offset}, rgt = rgt + #{offset}",
-                          ["root_id = ? AND lft >= ? AND rgt <= ? ", old_root_id, lft, rgt])
-        self[left_column_name] = lft + offset
-        self[right_column_name] = rgt + offset
-        if @parent_issue
-          move_to_child_of(@parent_issue)
-        end
-      end
-      reload
-      # delete invalid relations of all descendants
-      self_and_descendants.each do |issue|
-        issue.relations.each do |relation|
-          relation.destroy unless relation.valid?
-        end
-      end
-      # update former parent
-      recalculate_attributes_for(former_parent_id) if former_parent_id
+      update_nested_set_attributes_on_parent_change
     end
     remove_instance_variable(:@parent_issue) if instance_variable_defined?(:@parent_issue)
   end
 
+  # Updates the nested set for when an existing issue is moved
+  def update_nested_set_attributes_on_parent_change
+    former_parent_id = parent_id
+    # moving an existing issue
+    if @parent_issue && @parent_issue.root_id == root_id
+      # inside the same tree
+      move_to_child_of(@parent_issue)
+    else
+      # to another tree
+      unless root?
+        move_to_right_of(root)
+      end
+      old_root_id = root_id
+      self.root_id = (@parent_issue.nil? ? id : @parent_issue.root_id )
+      target_maxright = nested_set_scope.maximum(right_column_name) || 0
+      offset = target_maxright + 1 - lft
+      Issue.update_all(["root_id = ?, lft = lft + ?, rgt = rgt + ?", root_id, offset, offset],
+                        ["root_id = ? AND lft >= ? AND rgt <= ? ", old_root_id, lft, rgt])
+      self[left_column_name] = lft + offset
+      self[right_column_name] = rgt + offset
+      if @parent_issue
+        move_to_child_of(@parent_issue)
+      end
+    end
+    # delete invalid relations of all descendants
+    self_and_descendants.each do |issue|
+      issue.relations.each do |relation|
+        relation.destroy unless relation.valid?
+      end
+    end
+    # update former parent
+    recalculate_attributes_for(former_parent_id) if former_parent_id
+  end
+
   def update_parent_attributes
     recalculate_attributes_for(parent_id) if parent_id
   end
@@ -1220,7 +1356,8 @@
           if average == 0
             average = 1
           end
-          done = p.leaves.sum("COALESCE(estimated_hours, #{average}) * (CASE WHEN is_closed = #{connection.quoted_true} THEN 100 ELSE COALESCE(done_ratio, 0) END)", :joins => :status).to_f
+          done = p.leaves.sum("COALESCE(CASE WHEN estimated_hours > 0 THEN estimated_hours ELSE NULL END, #{average}) " +
+			"* (CASE WHEN is_closed = #{connection.quoted_true} THEN 100 ELSE COALESCE(done_ratio, 0) END)", :joins => :status).to_f
           progress = done / (average * leaves_count)
           p.done_ratio = progress.round
         end
@@ -1240,12 +1377,11 @@
   def self.update_versions(conditions=nil)
     # Only need to update issues with a fixed_version from
     # a different project and that is not systemwide shared
-    Issue.scoped(:conditions => conditions).all(
-      :conditions => "#{Issue.table_name}.fixed_version_id IS NOT NULL" +
+    Issue.includes(:project, :fixed_version).
+      where("#{Issue.table_name}.fixed_version_id IS NOT NULL" +
         " AND #{Issue.table_name}.project_id <> #{Version.table_name}.project_id" +
-        " AND #{Version.table_name}.sharing <> 'system'",
-      :include => [:project, :fixed_version]
-    ).each do |issue|
+        " AND #{Version.table_name}.sharing <> 'system'").
+      where(conditions).each do |issue|
       next if issue.project.nil? || issue.fixed_version.nil?
       unless issue.project.shared_versions.include?(issue.fixed_version)
         issue.init_journal(User.current)
@@ -1303,10 +1439,23 @@
     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
 
@@ -1316,7 +1465,7 @@
     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?)
@@ -1365,6 +1514,24 @@
     end
   end
 
+  def send_notification
+    if Setting.notified_events.include?('issue_added')
+      Mailer.deliver_issue_add(self)
+    end
+  end
+
+  # Stores the previous assignee so we can still have access
+  # to it during after_save callbacks (assigned_to_id_was is reset)
+  def set_assigned_to_was
+    @previous_assigned_to_id = assigned_to_id_was
+  end
+
+  # Clears the previous assignee at the end of after_save callbacks
+  def clear_assigned_to_was
+    @assigned_to_was = nil
+    @previous_assigned_to_id = nil
+  end
+
   # Query generator for selecting groups of issue counts for a project
   # based on specific criteria
   #