Merge pull request #5129 from ehelms/fixes-9866

Fixes #9866: Properly remove Pulp sync schedule from products.

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

@@ -120,21 +120,15 @@ def available_products
     param :id, String, :desc => N_("ID of the sync plan"), :required => true
     param :product_ids, Array, :desc => N_("List of product ids to add to the sync plan"), :required => true
     def add_products
-      ids = params[:product_ids]
-      @products  = Product.where(:id => ids).editable
-      @sync_plan.product_ids = (@sync_plan.product_ids + @products.collect { |p| p.id }).uniq
-      sync_task(::Actions::Katello::SyncPlan::UpdateProducts, @sync_plan)
+      sync_task(::Actions::Katello::SyncPlan::AddProducts, @sync_plan, params[:product_ids])
       respond_for_show
     end
 
     api :PUT, "/organizations/:organization_id/sync_plans/:id/remove_products", N_("Remove products from sync plan")
     param :id, String, :desc => N_("ID of the sync plan"), :required => true
     param :product_ids, Array, :desc => N_("List of product ids to remove from the sync plan"), :required => true
     def remove_products
-      ids = params[:product_ids]
-      @products  = Product.where(:id => ids).editable
-      @sync_plan.product_ids = (@sync_plan.product_ids - @products.collect { |p| p.id }).uniq
-      sync_task(::Actions::Katello::SyncPlan::UpdateProducts, @sync_plan)
+      sync_task(::Actions::Katello::SyncPlan::RemoveProducts, @sync_plan, params[:product_ids])
       respond_for_show
     end
 

app/lib/actions/katello/sync_plan/add_products.rb

@@ -0,0 +1,35 @@
+#
+# Copyright 2014 Red Hat, Inc.
+#
+# This software is licensed to you under the GNU General Public
+# License as published by the Free Software Foundation; either version
+# 2 of the License (GPLv2) or (at your option) any later version.
+# There is NO WARRANTY for this software, express or implied,
+# including the implied warranties of MERCHANTABILITY,
+# NON-INFRINGEMENT, or FITNESS FOR A PARTICULAR PURPOSE. You should
+# have received a copy of GPLv2 along with this software; if not, see
+# http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
+
+module Actions
+  module Katello
+    module SyncPlan
+      class AddProducts < Actions::EntryAction
+        def plan(sync_plan, product_ids)
+          action_subject(sync_plan)
+
+          products = ::Katello::Product.where(:id => product_ids).editable
+          sync_plan.product_ids = (sync_plan.product_ids + products.collect { |p| p.id }).uniq
+          sync_plan.save!
+
+          products.each do |product|
+            plan_action(::Actions::Katello::Product::Update, product, :sync_plan_id => sync_plan.id)
+          end
+        end
+
+        def humanized_name
+          _("Add Sync Plan Products")
+        end
+      end
+    end
+  end
+end

app/lib/actions/katello/sync_plan/update_products.rb → app/lib/actions/katello/sync_plan/remove_products.rb

@@ -13,12 +13,16 @@
 module Actions
   module Katello
     module SyncPlan
-      class UpdateProducts < Actions::EntryAction
-        def plan(sync_plan)
+      class RemoveProducts < Actions::EntryAction
+        def plan(sync_plan, product_ids)
           action_subject(sync_plan)
+
+          products = ::Katello::Product.where(:id => product_ids).editable
+          sync_plan.product_ids = (sync_plan.product_ids - products.collect { |p| p.id }).uniq
           sync_plan.save!
-          sync_plan.products.each do |product|
-            plan_action(::Actions::Katello::Product::Update, product, :sync_plan_id => sync_plan.id)
+
+          products.each do |product|
+            plan_action(::Actions::Katello::Product::Update, product, :sync_plan_id => nil)
           end
         end
 

test/actions/katello/sync_plan/add_products_test.rb

@@ -0,0 +1,40 @@
+#
+# Copyright 2015 Red Hat, Inc.
+#
+# This software is licensed to you under the GNU General Public
+# License as published by the Free Software Foundation; either version
+# 2 of the License (GPLv2) or (at your option) any later version.
+# There is NO WARRANTY for this software, express or implied,
+# including the implied warranties of MERCHANTABILITY,
+# NON-INFRINGEMENT, or FITNESS FOR A PARTICULAR PURPOSE. You should
+# have received a copy of GPLv2 along with this software; if not, see
+# http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
+
+require 'katello_test_helper'
+
+describe ::Actions::Katello::SyncPlan::AddProducts do
+  include Dynflow::Testing
+  include Support::Actions::Fixtures
+  include FactoryGirl::Syntax::Methods
+
+  before :all do
+    @product = katello_products(:fedora)
+    @sync_plan = FactoryGirl.build(
+      'katello_sync_plan',
+      :products => [],
+      :interval => 'daily',
+      :sync_date => Time.now,
+      :organization_id => Organization.first.id
+    )
+  end
+
+  let(:action_class) { ::Actions::Katello::SyncPlan::AddProducts }
+  let(:action) { create_action action_class }
+
+  it 'plans' do
+    action.stubs(:action_subject).with(@sync_plan)
+    plan_action(action, @sync_plan, [@product.id])
+
+    assert_action_planed_with(action, ::Actions::Katello::Product::Update, @product, :sync_plan_id => @sync_plan.id)
+  end
+end

test/actions/katello/sync_plan/remove_products_test.rb

@@ -0,0 +1,40 @@
+#
+# Copyright 2015 Red Hat, Inc.
+#
+# This software is licensed to you under the GNU General Public
+# License as published by the Free Software Foundation; either version
+# 2 of the License (GPLv2) or (at your option) any later version.
+# There is NO WARRANTY for this software, express or implied,
+# including the implied warranties of MERCHANTABILITY,
+# NON-INFRINGEMENT, or FITNESS FOR A PARTICULAR PURPOSE. You should
+# have received a copy of GPLv2 along with this software; if not, see
+# http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
+
+require 'katello_test_helper'
+
+describe ::Actions::Katello::SyncPlan::RemoveProducts do
+  include Dynflow::Testing
+  include Support::Actions::Fixtures
+  include FactoryGirl::Syntax::Methods
+
+  before :all do
+    @product = katello_products(:fedora)
+    @sync_plan = FactoryGirl.build(
+      'katello_sync_plan',
+      :products => [@product],
+      :interval => 'daily',
+      :sync_date => Time.now,
+      :organization_id => Organization.first.id
+    )
+  end
+
+  let(:action_class) { ::Actions::Katello::SyncPlan::RemoveProducts }
+  let(:action) { create_action action_class }
+
+  it 'plans' do
+    action.stubs(:action_subject).with(@sync_plan)
+    plan_action(action, @sync_plan, [@product.id])
+
+    assert_action_planed_with(action, ::Actions::Katello::Product::Update, @product, :sync_plan_id => nil)
+  end
+end

test/controllers/api/v2/sync_plans_controller_test.rb

@@ -148,10 +148,11 @@ def test_available_products_protected
     end
 
     def test_add_products
-      ::ForemanTasks.expects(:sync_task).with(::Actions::Katello::SyncPlan::UpdateProducts, @sync_plan)
+      product_ids = @products.collect { |p| p.id.to_s }
+      ::ForemanTasks.expects(:sync_task).with(::Actions::Katello::SyncPlan::AddProducts, @sync_plan, product_ids)
 
       put :add_products, :id => @sync_plan.id, :organization_id => @organization.id,
-          :product_ids => @products.collect { |p| p.id }
+          :product_ids => product_ids
 
       assert_response :success
       assert_template 'api/v2/sync_plans/show'
@@ -168,8 +169,11 @@ def test_add_products_protected
     end
 
     def test_remove_products
+      product_ids = @products.collect { |p| p.id.to_s }
+      ::ForemanTasks.expects(:sync_task).with(::Actions::Katello::SyncPlan::RemoveProducts, @sync_plan, product_ids)
+
       put :remove_products, :id => @sync_plan.id, :organization_id => @organization.id,
-          :product_ids => @products.collect { |p| p.id }
+          :product_ids => product_ids
 
       assert_response :success
       assert_template 'api/v2/sync_plans/show'