From 3fce5d393ed25593f11689fbb03ebdeb35f656ec Mon Sep 17 00:00:00 2001 From: David Davis Date: Mon, 24 Mar 2014 09:46:05 -0400 Subject: [PATCH] Refs #4798 - Fixing the subscription API --- .../api/v2/subscriptions_controller.rb | 46 +++++++++---------- config/routes/api/v2.rb | 2 +- .../subscriptions-helper.service.js | 4 +- ...ystem-add-subscriptions.controller.test.js | 8 ++-- .../system-subscriptions.controller.test.js | 4 +- .../api/v2/subscriptions_controller_test.rb | 6 +-- 6 files changed, 33 insertions(+), 37 deletions(-) diff --git a/app/controllers/katello/api/v2/subscriptions_controller.rb b/app/controllers/katello/api/v2/subscriptions_controller.rb index eb0dc23b65c..39f724d423d 100644 --- a/app/controllers/katello/api/v2/subscriptions_controller.rb +++ b/app/controllers/katello/api/v2/subscriptions_controller.rb @@ -98,23 +98,23 @@ def available api :POST, "/subscriptions/:id", "Add a subscription to a resource" api :POST, "/systems/:system_id/subscriptions", "Add a subscription to a system" api :POST, "/activation_keys/:activation_key_id/subscriptions", "Add a subscription to an activation key" - param :id, String, :desc => "Subscription Pool uuid" - param :system_id, String, :desc => "UUID of the system" - param :activation_key_id, String, :desc => "ID of the activation key" - param :quantity, :number, :desc => "Quantity of this subscriptions to add" - param :subscriptions, Array, :desc => "Array of subscriptions to add", :required => true do - param :subscription, Hash do - param :id, String, :desc => "Subscription Pool uuid", :required => true - param :quantity, :number, :desc => "Quantity of this subscriptions to add", :required => true - end + param :id, String, :desc => "Subscription Pool uuid", :required => false + param :system_id, String, :desc => "UUID of the system", :required => false + param :activation_key_id, String, :desc => "ID of the activation key", :required => false + param :quantity, :number, :desc => "Quantity of this subscriptions to add", :required => false + param :subscriptions, Array, :desc => "Array of subscriptions to add", :required => false do + param :id, String, :desc => "Subscription Pool uuid", :required => true + param :quantity, :number, :desc => "Quantity of this subscriptions to add", :required => true end def create object = @system || @activation_key || @distributor - subscription_params[:subscriptions].each do |subscription| - object.subscribe(subscription[:subscription][:id], subscription[:subscription][:quantity]) - end if subscription_params[:subscriptions] - if subscription_params[:id] && subscription_params[:quantity] - object.subscribe(subscription_params[:id], subscription_params[:quantity]) + + if params[:subscriptions] + params[:subscriptions].each do |subscription| + object.subscribe(subscription[:id], subscription[:quantity]) + end + elsif params[:id] && params.key?(:quantity) + object.subscribe(params[:id], params[:quantity]) end subscriptions = if @system @@ -131,14 +131,18 @@ def create api :DELETE, "/subscriptions/:id", "Unattach a subscription" api :DELETE, "/systems/:system_id/subscriptions/:id", "Unattach a subscription" api :DELETE, "/activation_keys/:activation_key_id/subscriptions/:id", "Unattach a subscription" - param :id, String, :desc => "Subscription ID" + param :id, String, :desc => "Subscription ID", :required => false param :system_id, String, :desc => "UUID of the system" param :activation_key_id, String, :desc => "activation key ID" + param :subscriptions, Array, :desc => "Array of subscriptions to add", :required => false do + param :id, String, :desc => "Subscription Pool uuid", :required => true + end def destroy object = @system || @activation_key || @distributor - if params[:subscription].present? - subscription_params[:subscriptions].each do |subscription| - object.unsubscribe(subscription[:subscription][:id]) + + if params[:subscriptions].present? + params[:subscriptions].each do |subscription| + object.unsubscribe(subscription[:id]) end elsif params[:id] object.unsubscribe(params[:id]) @@ -349,11 +353,5 @@ def activation_key_subscriptions(cp_pools) return subscriptions end - - def subscription_params - params.require(:subscription).permit(:id, :quantity, :subscriptions => [:subscription => [:id, :quantity]]) - params[:subscription] - end - end end diff --git a/config/routes/api/v2.rb b/config/routes/api/v2.rb index 7d47c2427f0..87b79d9072a 100644 --- a/config/routes/api/v2.rb +++ b/config/routes/api/v2.rb @@ -184,7 +184,7 @@ class ActionDispatch::Routing::Mapper api_resources :activation_keys, :only => [:index] api_resources :subscriptions, :only => [:create, :index, :destroy] do collection do - match '/' => 'subscriptions#destroy', :via => :delete + match '/' => 'subscriptions#destroy', :via => :put match '/available' => 'subscriptions#available', :via => :get match '/serials/:serial_id' => 'subscriptions#destroy_by_serial', :via => :delete end diff --git a/engines/bastion/app/assets/javascripts/bastion/subscriptions/subscriptions-helper.service.js b/engines/bastion/app/assets/javascripts/bastion/subscriptions/subscriptions-helper.service.js index ced8d40afbc..b5361cd2c7a 100644 --- a/engines/bastion/app/assets/javascripts/bastion/subscriptions/subscriptions-helper.service.js +++ b/engines/bastion/app/assets/javascripts/bastion/subscriptions/subscriptions-helper.service.js @@ -52,7 +52,7 @@ angular.module('Bastion.subscriptions').service('SubscriptionsHelper', } else { amount = 1; } - selected.push({"subscription": {"id": subscription.id, "quantity": amount}}); + selected.push({"id": subscription.id, "quantity": amount}); }); return selected; }; @@ -62,7 +62,7 @@ angular.module('Bastion.subscriptions').service('SubscriptionsHelper', selected = []; angular.forEach(table.getSelected(), function (subscription) { - selected.push({"subscription": {"id": subscription.id}}); + selected.push({"id": subscription.id}); }); return selected; }; diff --git a/engines/bastion/test/systems/details/system-add-subscriptions.controller.test.js b/engines/bastion/test/systems/details/system-add-subscriptions.controller.test.js index 0caaf78867c..4b9dc5dee7e 100644 --- a/engines/bastion/test/systems/details/system-add-subscriptions.controller.test.js +++ b/engines/bastion/test/systems/details/system-add-subscriptions.controller.test.js @@ -96,7 +96,7 @@ describe('Controller: SystemAddSubscriptionsController', function() { $scope.system = new System({ uuid: 12345, - subscriptions: [{subscription: {id: 1, quantity: 11}}, {subscription: {id: 2, quantity: 22}}] + subscriptions: [{id: 1, quantity: 11}, {id: 2, quantity: 22}] }); })); @@ -107,9 +107,9 @@ describe('Controller: SystemAddSubscriptionsController', function() { it("allows removing system groups from the system", function() { var expected = {uuid: 12345, subscriptions: [ - {subscription: {id: 2, quantity: 0}}, - {subscription: {id: 3, quantity: 1}}, - {subscription: {id: 4, quantity: 1}} + {id: 2, quantity: 0}, + {id: 3, quantity: 1}, + {id: 4, quantity: 1} ]}; spyOn(System, 'addSubscriptions'); diff --git a/engines/bastion/test/systems/details/system-subscriptions.controller.test.js b/engines/bastion/test/systems/details/system-subscriptions.controller.test.js index f3891d60260..967d4cfcfce 100644 --- a/engines/bastion/test/systems/details/system-subscriptions.controller.test.js +++ b/engines/bastion/test/systems/details/system-subscriptions.controller.test.js @@ -82,7 +82,7 @@ describe('Controller: SystemSubscriptionsController', function() { $scope.system = new System({ uuid: 12345, - subscriptions: [{subscription: {id: 1, quantity: 11}}, {subscription: {id: 2, quantity: 22}}] + subscriptions: [{id: 1, quantity: 11}, {id: 2, quantity: 22}] }); })); @@ -92,7 +92,7 @@ describe('Controller: SystemSubscriptionsController', function() { it("allows removing system groups from the system", function() { - var expected = {uuid: 12345, subscriptions: [{subscription:{id: 2}}]}; + var expected = {uuid: 12345, subscriptions: [{id: 2}]}; spyOn(System, 'removeSubscriptions'); $scope.subscriptionsTable.getSelected = function() { diff --git a/test/controllers/api/v2/subscriptions_controller_test.rb b/test/controllers/api/v2/subscriptions_controller_test.rb index de909566566..e9e820b320b 100644 --- a/test/controllers/api/v2/subscriptions_controller_test.rb +++ b/test/controllers/api/v2/subscriptions_controller_test.rb @@ -88,8 +88,7 @@ def test_available def test_create System.any_instance.expects(:subscribe) - post :create, :system_id => @system.uuid, :subscription => { - :subscriptions => [:subscription => {:id => 'redhat', :quantity => 1}]} + post :create, :system_id => @system.uuid, :subscriptions => [{:id => 'redhat', :quantity => 1}] assert_response :success assert_template 'katello/api/v2/subscriptions/index' @@ -97,8 +96,7 @@ def test_create def test_destroy System.any_instance.expects(:unsubscribe) - delete :destroy, :system_id => @system.uuid, :subscription => { - :subscriptions => [:subscription => {:id => 1}]} + delete :destroy, :system_id => @system.uuid, :subscriptions => [:subscription => {:id => 1}] assert_response :success assert_template 'katello/api/v2/subscriptions/index'