[PATCH conductor] BZ790861, BZ788103 Remove only catalog entry when removing deployable from catalog detail page

jtomasek at redhat.com jtomasek at redhat.com
Thu Feb 23 14:17:19 UTC 2012


From: Jiri Tomasek <jtomasek at redhat.com>

When removing deployables from catalog show page, only catalog entry is removed. If there are no more catalog references to this deployable, deployable gets deleted in catalog_entry after destroy hook.
Catalog_entry removal is done in deployables multi destroy. If necessary deployable is removed in catalog_entry after_destroy hook.

When we have separate deployables list done (an we support deployables without assigned catalogs), the table in catalog show page should list catalog entries and the form should submit to catalog_entries multi_destroy.
Deployables removal should be done separatelly from deployables list or deployable show/edit pages.

BZ788103 is also fixed with this patch
---
 src/app/controllers/deployables_controller.rb      |    6 +++-
 src/app/models/catalog_entry.rb                    |    6 ++--
 src/app/models/deployable.rb                       |    2 +-
 src/app/views/catalogs/show.html.haml              |    2 +-
 src/config/locales/en.yml                          |   14 ++++++------
 .../controllers/deployables_controller_spec.rb     |   23 +++++++++++++------
 6 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/src/app/controllers/deployables_controller.rb b/src/app/controllers/deployables_controller.rb
index e37bc4a..1e1477f 100644
--- a/src/app/controllers/deployables_controller.rb
+++ b/src/app/controllers/deployables_controller.rb
@@ -188,10 +188,13 @@ class DeployablesController < ApplicationController
     deleted = []
     not_deleted = []
     not_deleted_perms = []
+
+    @catalog = Catalog.find(params[:catalog_id])
+
     if params[:deployables_selected]
       Deployable.find(params[:deployables_selected]).to_a.each do |d|
         if check_privilege(Privilege::MODIFY, d)
-          if d.destroy
+          if d.catalog_entries.where(:catalog_id => @catalog.id).first.destroy
             deleted << d.name
           else
             not_deleted << d.name
@@ -211,7 +214,6 @@ class DeployablesController < ApplicationController
       flash[:error] = t("deployables.flash.error.not_selected")
     end
 
-    @catalog = Catalog.find(params[:catalog_id]) if params[:catalog_id].present?
     if @catalog.present?
       redirect_to catalog_path(@catalog)
     else
diff --git a/src/app/models/catalog_entry.rb b/src/app/models/catalog_entry.rb
index 8f6b15c..857d013 100644
--- a/src/app/models/catalog_entry.rb
+++ b/src/app/models/catalog_entry.rb
@@ -35,10 +35,10 @@ class CatalogEntry < ActiveRecord::Base
 
   validates_uniqueness_of :catalog_id, :scope => [:deployable_id]
 
-  before_destroy :check_deployable_references
+  after_destroy :destroy_deployable_if_last
 
-  def check_deployable_references
-    return deployable.catalogs.count > 1
+  def destroy_deployable_if_last
+    deployable.destroy if deployable.catalog_entries.blank?
   end
 
   # This probably goes away once we separate catalog entry creation from deployables
diff --git a/src/app/models/deployable.rb b/src/app/models/deployable.rb
index c77f094..d7ce439 100644
--- a/src/app/models/deployable.rb
+++ b/src/app/models/deployable.rb
@@ -32,7 +32,7 @@ class Deployable < ActiveRecord::Base
   has_many :permissions, :as => :permission_object, :dependent => :destroy,
            :include => [:role],
            :order => "permissions.id ASC"
-  has_many :catalog_entries, :dependent => :delete_all
+  has_many :catalog_entries, :dependent => :destroy
   has_many :catalogs, :through => :catalog_entries
 
   belongs_to :owner, :class_name => "User", :foreign_key => "owner_id"
diff --git a/src/app/views/catalogs/show.html.haml b/src/app/views/catalogs/show.html.haml
index 0ee2fe7..f925907 100644
--- a/src/app/views/catalogs/show.html.haml
+++ b/src/app/views/catalogs/show.html.haml
@@ -22,7 +22,7 @@
 
   .content
     - content_for :form_header do
-      %li= restful_submit_tag t("delete"), "destroy", multi_destroy_catalog_deployables_path(@catalog), 'DELETE', :id => 'delete_button', :class => 'button danger'
+      %li= restful_submit_tag t("remove"), "destroy", multi_destroy_catalog_deployables_path(@catalog), 'DELETE', :id => 'delete_button', :class => 'button danger'
       - if check_privilege(Privilege::CREATE, Deployable)
         %li= link_to t("catalog_entries.new_catalog_entry"), new_catalog_deployable_path(@catalog), :class => 'button', :id => 'new_deployable_button'
 
diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
index 126130c..e0e4e79 100644
--- a/src/config/locales/en.yml
+++ b/src/config/locales/en.yml
@@ -913,12 +913,12 @@ en:
         attribute_not_exist: "Some attribute(s) is missing in XML, please check the file!"
         no_catalog: "No catalogs selected"
         not_deleted:
-          one: "Deployable %{not_deleted} delete failed!"
-          other: "%{count} deployables %{not_deleted} were not deleted!"
+          one: "Deployable %{not_deleted} removal failed!"
+          other: "%{count} deployables %{not_deleted} were not removed!"
         not_deleted_perms:
-          one: "Insufficient permissions to delete deployable %{not_deleted}!"
-          other: "Insufficient permissions to delete %{count} deployables %{not_deleted}!"
-        not_selected: "No deployable was not selected!"
+          one: "Insufficient permissions to remove deployable %{not_deleted}!"
+          other: "Insufficient permissions to remove %{count} deployables %{not_deleted}!"
+        not_selected: "No deployable was selected!"
         no_catalog_exists: "No catalog exists! Please create one."
         no_hwp_exists: "No hardware profile exists! Please create one."
         missing_image: "%{assembly}: Image (UUID: %{uuid}) doesn't exist"
@@ -927,8 +927,8 @@ en:
         failed: "Deployable was not created: %{message}"
       notice:
         deleted:
-          one: "Deployable %{deleted} delete successfully!"
-          other: "%{count} deployables %{deleted} were deleted!"
+          one: "Deployable %{deleted} removed successfully!"
+          other: "%{count} deployables %{deleted} were removed!"
     error:
       attribute_not_exists: invalid
       hwp_not_exists: "Hardware profile %{name} specified in XML doesn't exist."
diff --git a/src/spec/controllers/deployables_controller_spec.rb b/src/spec/controllers/deployables_controller_spec.rb
index 72d89f4..9598b3d 100644
--- a/src/spec/controllers/deployables_controller_spec.rb
+++ b/src/spec/controllers/deployables_controller_spec.rb
@@ -123,6 +123,8 @@ describe DeployablesController do
       before do
         @deployable1 = Factory :deployable, :name => "test_delete"
         @deployable2 = Factory :deployable, :name => "test_delete2"
+        @catalog.deployables << @deployable1
+        @catalog.deployables << @deployable2
       end
 
       it "deletes both deployables and shows flash notice" do
@@ -131,16 +133,23 @@ describe DeployablesController do
       end
 
       it "not delete deployable1 but not deployable2 and shows flash notice and error" do
-        Deployable.any_instance.stub(:destroy).and_return(false)
-        delete :multi_destroy, :deployables_selected => [@deployable1.id, @deployable2.id]
+        CatalogEntry.any_instance.stub(:destroy).and_return(false)
+        delete :multi_destroy, :deployables_selected => [@deployable1.id, @deployable2.id], :catalog_id => @catalog.id
         flash[:error].should_not be_empty
       end
-    end
 
-    context "without params[:deployables_selected]" do
-      it "not delete a deployable and shows flash error" do
-        delete :multi_destroy
-        flash[:error].should_not be_empty
+      it "not delete deployable with multiple catalogs" do
+        @catalog2 = FactoryGirl.create(:catalog)
+        @catalog2.deployables << @deployable1
+        delete :multi_destroy, :deployables_selected => [@deployable1.id], :catalog_id => @catalog.id
+        @deployable1.should_not be_nil
+      end
+
+      context "without params[:deployables_selected]" do
+        it "not delete a deployable and shows flash error" do
+          delete :multi_destroy, :catalog_id => @catalog.id
+          flash[:error].should_not be_empty
+        end
       end
     end
   end
-- 
1.7.7.6




More information about the aeolus-devel mailing list