Merge pull request #12289 from imtayadeway/refactor/api/req-subcollec…

…tion-predicate Extract RequestAdapter#subcollection?
ManageIQ · Jan 6, 2017 · 94e95ba · 94e95ba
2 parents a1575a6 + 36f4264
commit 94e95ba
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 14 deletions.
diff --git a/app/controllers/api/base_controller/generic.rb b/app/controllers/api/base_controller/generic.rb
@@ -6,23 +6,23 @@ module Generic
       #
 
       def show
-        if @req.subcollection
-          render_collection_type @req.subcollection.to_sym, @req.s_id, true
+        if @req.subcollection?
+          render_collection_type @req.subcollection.to_sym, @req.s_id
         else
           render_collection_type @req.collection.to_sym, @req.c_id
         end
       end
 
       def update
-        if @req.subcollection
-          render_normal_update @req.collection.to_sym, update_collection(@req.subcollection.to_sym, @req.s_id, true)
+        if @req.subcollection?
+          render_normal_update @req.collection.to_sym, update_collection(@req.subcollection.to_sym, @req.s_id)
         else
           render_normal_update @req.collection.to_sym, update_collection(@req.collection.to_sym, @req.c_id)
         end
       end
 
       def destroy
-        if @req.subcollection
+        if @req.subcollection?
           delete_subcollection_resource @req.subcollection.to_sym, @req.s_id
         else
           delete_resource(@req.collection.to_sym, @req.c_id)

diff --git a/app/controllers/api/base_controller/manager.rb b/app/controllers/api/base_controller/manager.rb
@@ -1,22 +1,22 @@
 module Api
   class BaseController
     module Manager
-      def update_collection(type, id, is_subcollection = false)
+      def update_collection(type, id)
         if @req.method == :put || @req.method == :patch
           raise BadRequestError,
                 "Must specify a resource id for the #{@req.method} HTTP method" if id.blank?
           return send("#{@req.method}_resource", type, id)
         end
 
         action = @req.action
-        target = target_resource_method(is_subcollection, type, action)
+        target = target_resource_method(type, action)
         raise BadRequestError,
               "Unimplemented Action #{action} for #{type} resources" unless respond_to?(target)
 
         if id
-          get_and_update_one_collection(is_subcollection, target, type, id)
+          get_and_update_one_collection(@req.subcollection?, target, type, id)
         else
-          get_and_update_multiple_collections(is_subcollection, target, type)
+          get_and_update_multiple_collections(@req.subcollection?, target, type)
         end
       end
 
@@ -82,8 +82,8 @@ def delete_subcollection_resource(type, id = nil)
 
       private
 
-      def target_resource_method(is_subcollection, type, action)
-        if is_subcollection
+      def target_resource_method(type, action)
+        if @req.subcollection?
           "#{type}_#{action}_resource"
         else
           target = "#{action}_resource"

diff --git a/app/controllers/api/base_controller/parser/request_adapter.rb b/app/controllers/api/base_controller/parser/request_adapter.rb
@@ -68,6 +68,10 @@ def s_id
           cid?(id) ? from_cid(id) : id
         end
 
+        def subcollection?
+          !!subcollection
+        end
+
         def expand?(what)
           expand_requested.include?(what.to_s)
         end

diff --git a/app/controllers/api/base_controller/renderer.rb b/app/controllers/api/base_controller/renderer.rb
@@ -4,16 +4,16 @@ module Renderer
       #
       # Helper proc for rendering a collection of type specified.
       #
-      def render_collection_type(type, id, is_subcollection = false)
+      def render_collection_type(type, id)
         klass = collection_class(type)
-        opts  = {:name => type.to_s, :is_subcollection => is_subcollection, :expand_actions => true}
+        opts  = {:name => type.to_s, :is_subcollection => @req.subcollection?, :expand_actions => true}
         if id
           render_resource type, resource_search(id, type, klass), opts
         else
           opts[:count]            = klass.count
           opts[:expand_resources] = @req.expand?(:resources)
 
-          res = collection_search(is_subcollection, type, klass)
+          res = collection_search(@req.subcollection?, type, klass)
 
           opts[:subcount] = res.length
 

diff --git a/spec/requests/api/service_catalogs_spec.rb b/spec/requests/api/service_catalogs_spec.rb
@@ -159,6 +159,21 @@ def sc_templates_url(id, st_id = nil)
 
       run_post(service_catalogs_url(sc.id), gen_request(:edit, "description" => "updated sc description"))
 
+      expected = {
+        "service_templates" => a_hash_including(
+          "actions" => a_collection_containing_exactly(
+            a_hash_including(
+              "name"   => "assign",
+              "method" => "post",
+            ),
+            a_hash_including(
+              "name"   => "unassign",
+              "method" => "post",
+            )
+          )
+        )
+      }
+      expect(response.parsed_body).to include(expected)
       expect_single_resource_query("id" => sc.id, "name" => "sc", "description" => "updated sc description")
       expect(sc.reload.description).to eq("updated sc description")
     end