Refs #4798 - Fixing the subscription API

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

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

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;
         };

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');
 

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() {

test/controllers/api/v2/subscriptions_controller_test.rb

@@ -88,17 +88,15 @@ 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'
   end
 
   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'