Merge pull request #717 from opf/fix/delete_former_deleted_planning_elements_from_database_migration

[FIX] Delete former deleted planning elements from database migration

app/controllers/api/v2/planning_elements_controller.rb

@@ -56,7 +56,8 @@ module Api
       end
 
       def create
-        @planning_element = planning_element_scope.new(permitted_params.planning_element)
+        @planning_element = @project.work_packages.build
+        @planning_element.update_attributes(permitted_params.planning_element)
 
         # The planning_element inherits from workpackage, which requires an author.
         # Using the current_user also satisfies this demand for API-calls
@@ -89,7 +90,7 @@ module Api
       end
 
       def update
-        @planning_element = planning_element_scope.find(params[:id])
+        @planning_element = WorkPackage.find(params[:id])
         @planning_element.attributes = permitted_params.planning_element
 
         successfully_updated = @planning_element.save
@@ -106,7 +107,7 @@ module Api
       end
 
       def destroy
-        @planning_element = planning_element_scope.find(params[:id])
+        @planning_element = WorkPackage.find(params[:id])
         @planning_element.destroy
 
         respond_to do |format|
@@ -205,7 +206,7 @@ module Api
       end
 
       def current_work_packages(projects)
-        work_packages = WorkPackage.for_projects(projects).without_deleted
+        work_packages = WorkPackage.for_projects(projects)
                                    .includes(:status, :project, :type)
 
         if params[:f]
@@ -223,12 +224,6 @@ module Api
         historical = PlanningComparisonService.compare(projects, at_time, filter)
       end
 
-      # remove this and replace by calls it with calls
-      # to assign_planning_elements once WorkPackages can be created
-      def planning_element_scope
-        @project.work_packages.without_deleted
-      end
-
       # Helpers
       helper_method :include_journals?
 

app/helpers/timelines_helper.rb

@@ -40,7 +40,7 @@ module TimelinesHelper
   end
 
   def parent_id_select_tag(form, planning_element)
-    available_parents = planning_element.project.planning_elements.without_deleted.find(:all, :order => "COALESCE(parent_id, id), parent_id")
+    available_parents = planning_element.project.planning_elements.find(:all, :order => "COALESCE(parent_id, id), parent_id")
     available_parents -= [planning_element]
 
     available_options = available_parents.map do |pe|

app/models/work_package.rb

@@ -71,8 +71,6 @@ class WorkPackage < ActiveRecord::Base
   scope :visible, lambda {|*args| { :include => :project,
                                     :conditions => WorkPackage.visible_condition(args.first ||
                                                                                  User.current) } }
-  scope :without_deleted, :conditions => "#{WorkPackage.quoted_table_name}.deleted_at IS NULL"
-  scope :deleted, :conditions => "#{WorkPackage.quoted_table_name}.deleted_at IS NOT NULL"
 
   scope :in_status, lambda {|*args| where(:status_id => (args.first.respond_to?(:id) ? args.first.id : args.first))}
 
@@ -212,7 +210,7 @@ class WorkPackage < ActiveRecord::Base
                                                     :category_id, :fixed_version_id,
                                                     :planning_element_status_id,
                                                     :author_id, :responsible_id
-  register_on_journal_formatter :datetime,          :start_date, :due_date, :deleted_at
+  register_on_journal_formatter :datetime,          :start_date, :due_date
 
   # By planning element
   register_on_journal_formatter :plaintext,         :subject,
@@ -363,10 +361,6 @@ class WorkPackage < ActiveRecord::Base
     end
   end
 
-  def deleted?
-    !!read_attribute(:deleted_at)
-  end
-
   # Users the work_package can be assigned to
   delegate :assignable_users, :to => :project
 
@@ -727,11 +721,9 @@ class WorkPackage < ActiveRecord::Base
   end
 
   def inherit_dates_from_children
-    active_children = children.without_deleted
-
-    unless active_children.empty?
-      self.start_date = [active_children.minimum(:start_date), active_children.minimum(:due_date)].compact.min
-      self.due_date   = [active_children.maximum(:start_date), active_children.maximum(:due_date)].compact.max
+    unless children.empty?
+      self.start_date = [children.minimum(:start_date), children.minimum(:due_date)].compact.min
+      self.due_date   = [children.maximum(:start_date), children.maximum(:due_date)].compact.max
     end
   end
 

app/services/planning_comparison_service.rb

@@ -62,7 +62,6 @@ SQL
                                       .joins(:status)
                                       .joins(:project) # query doesn't provide these joins itself...
                                       .for_projects(projects)
-                                      .without_deleted
 
       query = Query.new
       query.add_filters(filter[:f], filter[:op], filter[:v])

db/migrate/20131202094511_delete_former_deleted_planning_elements.rb

@@ -0,0 +1,72 @@
+#-- copyright
+# OpenProject is a project management system.
+#
+# Copyright (C) 2012-2013 the OpenProject Foundation (OPF)
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# See doc/COPYRIGHT.rdoc for more details.
+#++
+
+require_relative 'migration_utils/utils'
+
+class DeleteFormerDeletedPlanningElements < ActiveRecord::Migration
+  include Migration::Utils
+
+  def up
+    say_with_time_silently "Remove deleted work packages and related journals" do
+      delete_work_packages_marked_as_deleted
+    end
+
+    remove_column :work_packages, :deleted_at
+    remove_column :work_package_journals, :deleted_at
+  end
+
+  def down
+    add_column :work_packages, :deleted_at, :datetime
+    add_column :work_package_journals, :deleted_at, :datetime
+  end
+
+  private
+
+  def delete_work_packages_marked_as_deleted
+    delete_ids_from_table('attachable_journals', 'journal_id', deleted_work_packages_journals_ids)
+    delete_ids_from_table('customizable_journals', 'journal_id', deleted_work_packages_journals_ids)
+    delete_ids_from_table('work_package_journals', 'journal_id', deleted_work_packages_journals_ids)
+    delete_ids_from_table('journals', 'id', deleted_work_packages_journals_ids)
+    delete_ids_from_table('work_packages', 'id', deleted_work_package_ids)
+  end
+
+  def delete_ids_from_table(table, id_column, ids)
+    unless ids.empty?
+      delete <<-SQL
+        DELETE FROM #{table}
+        WHERE #{id_column} IN (#{ids.join(", ")})
+      SQL
+    end
+  end
+
+  def deleted_work_package_ids
+    return @deleted_work_package_ids if @deleted_work_package_ids
+
+    result = select_all <<-SQL
+      SELECT id FROM work_packages WHERE deleted_at IS NOT NULL
+    SQL
+
+    @deleted_work_package_ids = result.collect { |r| r['id'] }
+  end
+
+  def deleted_work_packages_journals_ids
+    return @deleted_work_packages_journals_ids if @deleted_work_packages_journals_ids
+
+    result = select_all <<-SQL
+      SELECT j.id
+      FROM journals AS j
+        JOIN work_packages AS w ON (j.journable_id = w.id AND j.journable_type = 'WorkPackage')
+      WHERE w.deleted_at IS NOT NULL;
+    SQL
+
+    @deleted_work_packages_journals_ids = result.collect { |r| r['id'] }
+  end
+end

doc/CHANGELOG.md

@@ -31,6 +31,7 @@ See doc/COPYRIGHT.rdoc for more details.
 
 * `#3050` Fix: Fix: [Work Package Tracking] Internal error with query from the time before work packages
 * `#3202` Fix: [Bug] Grouping work packages by responsible is broken
+* `#3055` [Work Package Tracking] Former deleted planning elements not handled in any way
 
 ## 3.0.0pre34
 

spec/models/work_package/work_package_planning_spec.rb

@@ -340,7 +340,6 @@ describe WorkPackage do
     it 'should delete the object permanantly when using destroy' do
       @pe1.destroy
 
-      WorkPackage.without_deleted.find_by_id(@pe1.id).should be_nil
       WorkPackage.find_by_id(@pe1.id).should be_nil
     end
 
@@ -354,11 +353,8 @@ describe WorkPackage do
       pe1.destroy
 
       [pe1, pe11, pe12, pe121].each do |pe|
-        WorkPackage.without_deleted.find_by_id(pe.id).should be_nil
         WorkPackage.find_by_id(pe.id).should be_nil
       end
-
-      WorkPackage.without_deleted.find_by_id(pe2.id).should == pe2
     end
   end
 end