fixes #6516 - various product deletion fixes

* Redhat products & promoted products could be deleted under bulk actions api & ui
* Single product delete api allowed redhat & promoted product deletion
* cp_id was being sent up for bulk actions
* error messages were only including a single errored product
* success and error messages were not actually printing
* products were removed from list even if they were not actually deleted

app/controllers/katello/api/api_controller.rb

@@ -153,7 +153,7 @@ def format_bulk_action_messages(args = {})
     unauthorized = models - authorized
 
     messages[:success] << args.fetch(:success) % authorized.length if authorized.present?
-    messages[:error] << args.fetch(:error) % unauthorized if unauthorized.present?
+    unauthorized.each{|item| messages[:error] << args.fetch(:error) % item }
 
     messages
   end

app/controllers/katello/api/v2/products_bulk_actions_controller.rb

@@ -18,13 +18,13 @@ class Api::V2::ProductsBulkActionsController < Api::V2::ApiController
     api :PUT, "/products/bulk/destroy", N_("Destroy one or more products")
     param :ids, Array, :desc => N_("List of product ids"), :required => true
     def destroy_products
-      deletable_products = @products.deletable
+      deletable_products = @products.deletable#.select{|p| p.user_deletable?}
       deletable_products.each do |prod|
         async_task(::Actions::Katello::Product::Destroy, prod)
       end
 
       messages = format_bulk_action_messages(
-        :success    => _("Successfully removed %s product(s)"),
+        :success    => _("Successfully initiated removal of %s product(s)"),
         :error      => _("You were not allowed to delete %s"),
         :models     => @products,
         :authorized => deletable_products
@@ -76,7 +76,7 @@ def update_sync_plans
 
     def find_products
       params.require(:ids)
-      @products = Product.where(:cp_id => params[:ids])
+      @products = Product.where(:id => params[:ids])
     end
 
   end

app/lib/actions/katello/product/destroy.rb

@@ -15,9 +15,16 @@ module Katello
     module Product
       class Destroy < Actions::EntryAction
 
+        # rubocop:disable MethodLength
         def plan(product)
+
+          unless product.user_deletable?
+            fail _("Cannot delete a Red Hat Products or Products with Repositories published in a Content View")
+          end
+
           no_other_assignment = ::Katello::Product.where(["cp_id = ? AND id != ?", product.cp_id, product.id]).count == 0
           action_subject(product)
+
           sequence do
             concurrence do
               product.repositories.each do |repo|

app/models/katello/product.rb

@@ -148,6 +148,10 @@ def redhat?
     provider.redhat_provider?
   end
 
+  def user_deletable?
+    self.published_content_views.empty? && !self.redhat?
+  end
+
   def custom?
     provider.custom_provider?
   end

engines/bastion/app/assets/javascripts/bastion/products/bulk/products-bulk-action.controller.js

@@ -42,7 +42,7 @@ angular.module('Bastion.products').controller('ProductsBulkActionController',
 
         $scope.getSelectedProductIds = function () {
             var rows = $scope.productTable.getSelected();
-            return _.pluck(rows, 'cp_id');
+            return _.pluck(rows, 'id');
         };
 
         $scope.removeProducts = function () {
@@ -52,12 +52,11 @@ angular.module('Bastion.products').controller('ProductsBulkActionController',
             $scope.actionParams.ids = $scope.getSelectedProductIds();
 
             success = function (data) {
-                angular.forEach($scope.productTable.getSelected(), function (row) {
-                    $scope.removeRow(row.id);
-                });
-
+                $scope.productsNutupane.refresh();
                 $scope.table.selectAll(false);
-                $scope.successMessages = data.displayMessages.success;
+
+                $scope.$parent.successMessages = data.displayMessages.success;
+                $scope.$parent.errorMessages = data.displayMessages.error;
                 $scope.removingProducts = false;
                 $scope.transitionTo('products.index');
             };

engines/bastion/app/assets/javascripts/bastion/products/products.controller.js

@@ -39,9 +39,9 @@ angular.module('Bastion.products').controller('ProductsController',
             'paged':            true
         };
 
-        var nutupane = new Nutupane(Product, params);
-        $scope.productTable = nutupane.table;
-        $scope.removeRow = nutupane.removeRow;
+        $scope.productsNutupane = new Nutupane(Product, params);
+        $scope.productTable = $scope.productsNutupane.table;
+        $scope.removeRow = $scope.productsNutupane.removeRow;
 
         $scope.productTable.closeItem = function () {
             $scope.transitionTo('products.index');
@@ -55,7 +55,7 @@ angular.module('Bastion.products').controller('ProductsController',
             $scope.productDeletionTaskId = undefined;
         };
 
-        $scope.productTable.refresh = nutupane.refresh;
+        $scope.productTable.refresh = $scope.productsNutupane.refresh;
 
         $scope.table = $scope.productTable;
 

engines/bastion/test/products/bulk/products-bulk-action.controller.test.js

@@ -17,7 +17,7 @@ describe('Controller: ProductsBulkActionController', function() {
     beforeEach(module('Bastion.products'));
 
     beforeEach(function() {
-        selected = [{cp_id: 1}, {cp_id: 2}, {cp_id: 3}];
+        selected = [{id: 1}, {id: 2}, {id: 3}];
         ProductBulkAction = {
             removeProducts: function() {
                 var deferred = $q.defer();

test/actions/katello/product_test.rb

@@ -128,6 +128,7 @@ class DestroyTest < TestBase
       end
 
       repo = "fooo"
+      product.expects(:user_deletable?).returns(true)
       ::Katello::Product.any_instance.expects(:repositories).returns([repo])
       product.expects(:destroy!)
 
@@ -144,6 +145,14 @@ class DestroyTest < TestBase
                                 cp_id: product.cp_id, organization_label: product.organization.label)
 
     end
+
+    it 'fails' do
+      product.expects(:user_deletable?).returns(false)
+
+      assert_raises(RuntimeError) do
+        plan_action(action, product)
+      end
+    end
   end
 
 end

test/controllers/api/v2/products_bulk_actions_controller_test.rb

@@ -53,7 +53,7 @@ def test_destroy_products
         test_product.id.must_equal product.id
       end
 
-      put :destroy_products, {:ids => [test_product.cp_id], :organization_id => @organization.id}
+      put :destroy_products, {:ids => [test_product.id], :organization_id => @organization.id}
 
       assert_response :success
     end
@@ -70,7 +70,7 @@ def test_destroy_products_protected
     def test_sync
       Product.any_instance.expects(:sync).times(@products.length).returns([{}])
 
-      put :sync_products, {:ids => @products.collect(&:cp_id), :organization_id => @organization.id}
+      put :sync_products, {:ids => @products.collect(&:id), :organization_id => @organization.id}
 
       assert_response :success
     end
@@ -80,14 +80,14 @@ def test_sync_protected
       denied_perms = [@update_permission, @destroy_permission, @view_permission, @create_permission]
 
       assert_protected_action(:sync_products, allowed_perms, denied_perms) do
-        put :sync_products, {:ids => @products.collect(&:cp_id), :organization_id => @organization.id}
+        put :sync_products, {:ids => @products.collect(&:id), :organization_id => @organization.id}
       end
     end
 
     def test_update_sync_plans
       Product.any_instance.expects(:save!).times(@products.length).returns([{}])
 
-      put :update_sync_plans, {:ids => @products.collect(&:cp_id), :organization_id => @organization.id, :plan_id => 1}
+      put :update_sync_plans, {:ids => @products.collect(&:id), :organization_id => @organization.id, :plan_id => 1}
 
       assert_response :success
     end
@@ -97,7 +97,7 @@ def test_update_sync_plans_protected
       denied_perms = [@sync_permission, @create_permission, @destroy_permission, @view_permission]
 
       assert_protected_action(:update_sync_plans, allowed_perms, denied_perms) do
-        put :update_sync_plans, {:ids => @products.collect(&:cp_id), :organization_id => @organization.id}
+        put :update_sync_plans, {:ids => @products.collect(&:id), :organization_id => @organization.id}
       end
     end
   end

test/models/product_test.rb

@@ -28,12 +28,25 @@ def setup
                        :organization => get_organization,
                        :provider => katello_providers(:anonymous)
                       )
+      @redhat_product = Product.find(katello_products(:redhat))
+      @promoted_product = Product.find(katello_products(:fedora))
     end
 
     def teardown
       @product.destroy if @product
     end
 
+    def test_redhat?
+      assert @redhat_product.redhat?
+      refute @product.redhat?
+    end
+
+    def test_user_deletable?
+      refute @redhat_product.user_deletable?
+      assert @product.user_deletable?
+      refute @promoted_product.user_deletable?
+    end
+
     def test_create
       assert        @product.save
       refute_empty  Product.where(:id => @product.id)