[PATCH conductor] BZ 788018 Destroy deployables when deleting its last catalog
Matt Wagner
matt.wagner at redhat.com
Wed Feb 8 15:29:30 UTC 2012
On Wed, Feb 08, 2012 at 03:21:50PM +0100, jtomasek at redhat.com wrote:
> From: Jiri Tomasek <jtomasek at redhat.com>
>
> When a Catalog is deleted, all deployables related only to it are destroyed too.
> ---
> src/app/models/catalog.rb | 8 ++++----
> src/config/locales/en.yml | 2 +-
> src/spec/controllers/catalogs_controller_spec.rb | 13 ++++++-------
> 3 files changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/src/app/models/catalog.rb b/src/app/models/catalog.rb
> index 00d1e7a..3472c59 100644
> --- a/src/app/models/catalog.rb
> +++ b/src/app/models/catalog.rb
> @@ -38,7 +38,7 @@ class Catalog < ActiveRecord::Base
> :include => [:role],
> :order => "permissions.id ASC"
>
> - before_destroy :check_deployable_references
> + before_destroy :destroy_deployables_related_only_to_self
>
> validates_presence_of :pool
> validates_presence_of :name
> @@ -48,9 +48,9 @@ class Catalog < ActiveRecord::Base
> PRESET_FILTERS_OPTIONS = [
> {:title => I18n.t("catalogs.preset_filters.belongs_to_default_pool"), :id => "belongs_to_default_pool", :query => includes(:pool).where("pools.name" => "Default")}
> ]
> - def check_deployable_references
> - return true if deployables.empty?
> - !(deployables.any? {|d| d.catalogs.count == 1})
> +
> + def destroy_deployables_related_only_to_self
> + deployables.each {|d| d.destroy if d.catalogs.count == 1}
> end
>
> private
> diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
> index 048047b..971f0f8 100644
> --- a/src/config/locales/en.yml
> +++ b/src/config/locales/en.yml
> @@ -411,7 +411,7 @@ en:
> owner: Owner
> pending: Pending
> failed: Failed
> - confirm_delete: "Are you sure you want to delete this pool?"
> + confirm_delete: "Are you sure you want to delete this pool? All related catalogs and deployables will be deleted too!"
> overview: Overview
> your_pools: Your Pools
> pool_details: Pool details
> diff --git a/src/spec/controllers/catalogs_controller_spec.rb b/src/spec/controllers/catalogs_controller_spec.rb
> index 50a53bf..8e46aaf 100644
> --- a/src/spec/controllers/catalogs_controller_spec.rb
> +++ b/src/spec/controllers/catalogs_controller_spec.rb
> @@ -33,10 +33,9 @@ describe CatalogsController do
> response.should redirect_to catalogs_path
> end
>
> - it "cannot remove catalog2 because it is the last deployable's reference " do
> - delete :destroy, :id => @catalog.id
> - expect {delete :destroy, :id => @catalog2.id}.not_to change(Catalog, :count).by(-1)
> - response.should redirect_to catalog_path(@catalog2)
> + it "cannot remove deployable related to catalog because it is also related to catalog2" do
> + expect {delete :destroy, :id => @catalog.id}.not_to change(Deployable, :count).by(-1)
I don't think the ".by(-1)" is necessary here -- it shouldn't change at
all.
> + response.should redirect_to catalogs_path
> end
>
> context "given catalog with one deployable, that is exclusive in this catalog" do
> @@ -44,9 +43,9 @@ describe CatalogsController do
> @catalog = Factory :catalog_with_deployable
> end
>
> - it "cannot be deleted when its deployables has no reference to other catalogs" do
> - expect {delete :destroy, :id => @catalog.id}.not_to change(Catalog, :count).by(-1)
> - response.should redirect_to catalog_path(@catalog)
> + it "should delete its deployables if its deployables have no reference to other catalogs" do
> + expect {delete :destroy, :id => @catalog.id}.to change(Deployable, :count).by(-1)
> + response.should redirect_to catalogs_path
> end
> end
> end
> --
> 1.7.7.6
Looks good. ACK, with one small nit above.
-- Matt
More information about the aeolus-devel
mailing list