NOTE: first you have to apply the #1098 patch (validate pools deletion) as it relies on the `destroyable?` method for instances.
From: Tomas Sedovic tsedovic@redhat.com
--- .../image_factory/deployables_controller.rb | 18 +++++++++++++++++- src/app/models/deployable.rb | 6 ++++++ src/app/models/deployment.rb | 5 +++++ src/config/locales/en.yml | 10 +++++++++- src/config/navigation.rb | 2 +- 5 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/src/app/controllers/image_factory/deployables_controller.rb b/src/app/controllers/image_factory/deployables_controller.rb index 5b8210c..917d0d6 100644 --- a/src/app/controllers/image_factory/deployables_controller.rb +++ b/src/app/controllers/image_factory/deployables_controller.rb @@ -50,7 +50,23 @@ class ImageFactory::DeployablesController < ApplicationController end
def multi_destroy - Deployable.destroy(params[:deployables_selected]) + destroyed = [] + failed = [] + Deployable.find(params[:deployables_selected]).each do |deployable| + if check_privilege(Privilege::MODIFY, deployable) and deployable.destroyable? + deployable.destroy + destroyed << deployable.name + else + failed << deployable.name + end + end + + unless destroyed.empty? + flash[:notice] = t('deployables.index.deleted', :count => destroyed.length, :list => destroyed.join(', ')) + end + unless failed.empty? + flash[:error] = t('deployables.index.not_deleted', :count => failed.length, :list => failed.join(', ')) + end redirect_to image_factory_deployables_url end
diff --git a/src/app/models/deployable.rb b/src/app/models/deployable.rb index a360c34..f3d954b 100644 --- a/src/app/models/deployable.rb +++ b/src/app/models/deployable.rb @@ -63,6 +63,8 @@ class Deployable < ActiveRecord::Base validates_uniqueness_of :name validates_length_of :name, :maximum => 255
+ before_destroy :destroyable? + def self.default_privilege_target_type Template end @@ -77,4 +79,8 @@ class Deployable < ActiveRecord::Base id ? Deployable.find(id) : Deployable.new end
+ def destroyable? + deployments.all? {|d| d.destroyable? } + end + end diff --git a/src/app/models/deployment.rb b/src/app/models/deployment.rb index fedad45..02324f8 100644 --- a/src/app/models/deployment.rb +++ b/src/app/models/deployment.rb @@ -61,6 +61,8 @@ class Deployment < ActiveRecord::Base validates_uniqueness_of :name, :scope => :pool_id validates_length_of :name, :maximum => 1024
+ before_destroy :destroyable? + SEARCHABLE_COLUMNS = %w(name)
def object_list @@ -96,5 +98,8 @@ class Deployment < ActiveRecord::Base return get_action_list.include?(action) ? true : false end
+ def destroyable? + instances.all? {|i| i.destroyable? } + end
end diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml index 677af3b..3f8b566 100644 --- a/src/config/locales/en.yml +++ b/src/config/locales/en.yml @@ -37,7 +37,6 @@ en: audit_report: Audit / Report assistance_requests: Assistance Requests define: Define - deployables: Deployables image_imports: Image Imports basic_template: Template Builder Basic Workflow browse_packages: Browse Packages @@ -96,6 +95,15 @@ en: index: deleted: "Deleted the following Pool Families: %{list}." not_deleted: "Could not delete the following Pool Families: %{list}." + deployables: + index: + deployables: Deployables + deleted: + one: "Deployable %{list} was deleted." + other: "Deployables %{list} were deleted." + not_deleted: + one: "Deployable %{list} was not deleted. There are deployments associated with it." + other: "Deployables %{list} were not deleted. They have deployments associated with them." setting: Settings name: Name admin: diff --git a/src/config/navigation.rb b/src/config/navigation.rb index 78a7862..21e1236 100644 --- a/src/config/navigation.rb +++ b/src/config/navigation.rb @@ -9,7 +9,7 @@ SimpleNavigation::Configuration.run do |navigation| first_level.item :image_factory, t(:image_factory), image_factory_templates_path, :highlights_on => //image_factory/ do |second_level| second_level.item :templates, t(:templates), image_factory_templates_path second_level.item :assemblies, t(:assemblies), image_factory_assemblies_path - second_level.item :deployables, t(:deployables), image_factory_deployables_path + second_level.item :deployables, t('deployables.index.deployables'), image_factory_deployables_path second_level.item :image_imports, t(:image_imports), new_image_factory_image_import_path end first_level.item :administration, t(:administration), admin_users_path, :highlights_on => //admin/ do |second_level|
From: Tomas Sedovic tsedovic@redhat.com
--- src/spec/models/deployable_spec.rb | 31 +++++++++++++++++++++++++++++++ 1 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/src/spec/models/deployable_spec.rb b/src/spec/models/deployable_spec.rb index af1fc9f..aec4518 100644 --- a/src/spec/models/deployable_spec.rb +++ b/src/spec/models/deployable_spec.rb @@ -22,4 +22,35 @@ describe Deployable do d.assemblies.size.should eql(1) end
+ it "should not be destroyable when it has running instances" do + deployable = Factory.create(:deployable) + deployment = Factory.create(:deployment, :deployable_id => deployable.id) + assembly = Factory.create(:assembly) + + instance = Factory.create(:instance, :deployment_id => deployment.id, :assembly_id => assembly.id, :template_id => nil) + Deployable.find(deployable).should_not be_destroyable + + instance.state = Instance::STATE_STOPPED + instance.save! + Deployable.find(deployable).should be_destroyable + end + + it "should not be destroyable when it has stopped stateful instances" do + deployable = Factory.build(:deployable) + deployment = Factory.build(:deployment, :deployable_id => deployable.id) + deployable.deployments << deployment + assembly = Factory.build(:assembly) + + instance = Factory.build(:instance, :deployment_id => deployment.id, :assembly_id => assembly.id, :template_id => nil) + instance.stub!(:restartable?).and_return(true) + deployment.instances << instance + deployable.should_not be_destroyable + + instance.state = Instance::STATE_STOPPED + deployable.should_not be_destroyable + + instance.stub!(:restartable?).and_return(false) + deployable.should be_destroyable + end + end
On Mon, Apr 18, 2011 at 05:06:43PM +0200, tsedovic@redhat.com wrote:
NOTE: first you have to apply the #1098 patch (validate pools deletion) as it relies on the `destroyable?` method for instances.
ACK to this set. All tests pass, works as described, and the bug in the previous set is fixed. Thanks.
-- Matt
aeolus-devel@lists.fedorahosted.org