diff --git a/app/controllers/api/v2/planning_elements_controller.rb b/app/controllers/api/v2/planning_elements_controller.rb index e6e1875c74..f07a19896a 100644 --- a/app/controllers/api/v2/planning_elements_controller.rb +++ b/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? diff --git a/app/helpers/timelines_helper.rb b/app/helpers/timelines_helper.rb index 440d0c7de4..99a399f92b 100644 --- a/app/helpers/timelines_helper.rb +++ b/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| diff --git a/app/models/work_package.rb b/app/models/work_package.rb index ccfb7fed2f..4c86ff214b 100644 --- a/app/models/work_package.rb +++ b/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 diff --git a/app/services/planning_comparison_service.rb b/app/services/planning_comparison_service.rb index 40d557f6b1..06382fdc6f 100644 --- a/app/services/planning_comparison_service.rb +++ b/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]) diff --git a/db/migrate/20131202094511_delete_former_deleted_planning_elements.rb b/db/migrate/20131202094511_delete_former_deleted_planning_elements.rb new file mode 100644 index 0000000000..ec5dc56d4d --- /dev/null +++ b/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 diff --git a/doc/CHANGELOG.md b/doc/CHANGELOG.md index 335e5fb14a..0f5415caae 100644 --- a/doc/CHANGELOG.md +++ b/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 diff --git a/spec/models/work_package/work_package_planning_spec.rb b/spec/models/work_package/work_package_planning_spec.rb index 8c69716b76..3d84c33a50 100644 --- a/spec/models/work_package/work_package_planning_spec.rb +++ b/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