[PATCH 0/3 conductor] BZ 801911 Able to delete Cloud with running instances - orphaned instance and zones
9:54 a.m.
This patchset a) prevents a pool family from being destroyed unless its member pools are
also destroyable (no running instances), and b) clarifies the consequences of destroying a
pool family.
Mainn
9:54 a.m.
New subject: [PATCH 1/3] BZ 801911 added/clarified text related to pool family deletion
---
src/config/locales/en.yml | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
index 7dff372..94cc7f4 100644
--- a/src/config/locales/en.yml
+++ b/src/config/locales/en.yml
@@ -603,7 +603,7 @@ en:
pool_families:
pool_families: Pool Families
pool_family: Pool Family
- confirm_delete: "Are you sure you want to delete?"
+ confirm_delete: "Are you sure you want to delete this pool family? All related
pools, catalogs, and deployables will be deleted too!"
pool_family_details: Pool Family Details
provider_accounts_for: Provider Accounts for
choose_provider_accounts: Choose Provider Accounts to enable for this Pool Family
@@ -642,7 +642,8 @@ en:
creation_failed: "Pool family's creation failed."
error:
not_updated: "Pool Family wasn't updated!"
- not_deleted: "Pool Family cannot be deleted!"
+ not_deleted: "Pool Family cannot be deleted! Check to make sure all
instances are stopped."
+ default_pool_family_not_deleted: "The default Pool Family cannot be
deleted!"
more_not_deleted: "Could not delete the following Pool Families:
%{list}."
provider_accounts_not_added: "Could not add these Provider Accounts"
provider_accounts_not_removed: "Could not remove these Provider
Accounts"
--
1.7.6.5
10:36 a.m.
New subject: [PATCH 1/3] BZ 801911 added/clarified text related to pool family deletion
On 03/14/2012 03:54 PM, Tzu-Mainn Chen wrote:
---
src/config/locales/en.yml | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
index 7dff372..94cc7f4 100644
--- a/src/config/locales/en.yml
+++ b/src/config/locales/en.yml
@@ -603,7 +603,7 @@ en:
pool_families:
pool_families: Pool Families
pool_family: Pool Family
- confirm_delete: "Are you sure you want to delete?"
+ confirm_delete: "Are you sure you want to delete this pool family? All related
pools, catalogs, and deployables will be deleted too!"
pool_family_details: Pool Family Details
provider_accounts_for: Provider Accounts for
choose_provider_accounts: Choose Provider Accounts to enable for this Pool Family
@@ -642,7 +642,8 @@ en:
creation_failed: "Pool family's creation failed."
error:
not_updated: "Pool Family wasn't updated!"
- not_deleted: "Pool Family cannot be deleted!"
+ not_deleted: "Pool Family cannot be deleted! Check to make sure all
instances are stopped."
+ default_pool_family_not_deleted: "The default Pool Family cannot be
deleted!"
more_not_deleted: "Could not delete the following Pool Families:
%{list}."
provider_accounts_not_added: "Could not add these Provider Accounts"
provider_accounts_not_removed: "Could not remove these Provider
Accounts"
Conditional ACK - the messages should be added also into product's
override file because "pool family" is being translated.
9:54 a.m.
New subject: [PATCH 2/3] BZ 801911 check that pools are destroyable before destroying a pool family
---
src/app/models/pool_family.rb | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/app/models/pool_family.rb b/src/app/models/pool_family.rb
index 219245d..8f5ea24 100644
--- a/src/app/models/pool_family.rb
+++ b/src/app/models/pool_family.rb
@@ -62,8 +62,8 @@ class PoolFamily < ActiveRecord::Base
end
def destroyable?
- # A PoolFamily is destroyable unless it is the default PoolFamily
- self != PoolFamily.default
+ # A PoolFamily is destroyable if its pools are destroyable and it is not the default
PoolFamily
+ pools.all? {|p| p.destroyable? } && self != PoolFamily.default
end
def all_providers_disabled?
--
1.7.6.5
10:38 a.m.
New subject: [PATCH 2/3] BZ 801911 check that pools are destroyable before destroying a pool family
On 03/14/2012 03:54 PM, Tzu-Mainn Chen wrote:
---
src/app/models/pool_family.rb | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/app/models/pool_family.rb b/src/app/models/pool_family.rb
index 219245d..8f5ea24 100644
--- a/src/app/models/pool_family.rb
+++ b/src/app/models/pool_family.rb
@@ -62,8 +62,8 @@ class PoolFamily< ActiveRecord::Base
end
def destroyable?
- # A PoolFamily is destroyable unless it is the default PoolFamily
- self != PoolFamily.default
+ # A PoolFamily is destroyable if its pools are destroyable and it is not the
default PoolFamily
+ pools.all? {|p| p.destroyable? }&& self != PoolFamily.default
end
def all_providers_disabled?
ACK (it would be nice to add a test for this at least later)
9:55 a.m.
New subject: [PATCH 3/3] BZ 801911 added checks before destroying a pool family
---
src/app/controllers/pool_families_controller.rb | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/app/controllers/pool_families_controller.rb
b/src/app/controllers/pool_families_controller.rb
index 0a4f2d5..ee74d96 100644
--- a/src/app/controllers/pool_families_controller.rb
+++ b/src/app/controllers/pool_families_controller.rb
@@ -95,11 +95,14 @@ class PoolFamiliesController < ApplicationController
def destroy
pool_family = PoolFamily.find(params[:id])
require_privilege(Privilege::MODIFY, pool_family)
- if pool_family.destroy
+ if pool_family == PoolFamily.default
+ flash[:error] =
t("pool_families.flash.error.default_pool_family_not_deleted")
+ elsif pool_family.destroy
flash[:notice] = t "pool_families.flash.notice.deleted"
else
flash[:error] = t "pool_families.flash.error.not_deleted"
end
+
redirect_to pool_families_path
end
--
1.7.6.5
11:02 a.m.
New subject: [PATCH 3/3] BZ 801911 added checks before destroying a pool family
On 03/14/2012 03:55 PM, Tzu-Mainn Chen wrote:
---
src/app/controllers/pool_families_controller.rb | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/app/controllers/pool_families_controller.rb
b/src/app/controllers/pool_families_controller.rb
index 0a4f2d5..ee74d96 100644
--- a/src/app/controllers/pool_families_controller.rb
+++ b/src/app/controllers/pool_families_controller.rb
@@ -95,11 +95,14 @@ class PoolFamiliesController< ApplicationController
def destroy
pool_family = PoolFamily.find(params[:id])
require_privilege(Privilege::MODIFY, pool_family)
- if pool_family.destroy
+ if pool_family == PoolFamily.default
+ flash[:error] =
t("pool_families.flash.error.default_pool_family_not_deleted")
It's better to do all checks in model (this check is already done in
before_destroy callback) and just call errors.add(:base, msg) in the
model. Then in the controller fill in flash[:error] with all object's
errors.
But your solution works fine so go ahead and push it. Similar code is on
more places in conductor's code, so we can optimize it post 1.0.
+ elsif pool_family.destroy
flash[:notice] = t "pool_families.flash.notice.deleted"
else
flash[:error] = t "pool_families.flash.error.not_deleted"
end
+
redirect_to pool_families_path
end
ACK, one note inline
4452
days inactive
4454
days old
aeolus-devel@lists.fedorahosted.org
6 comments
2 participants
participants (2)
-
Jan Provazník -
Tzu-Mainn Chen