small refactoring

Author: fbvilela

lib/zendesk_api/client.rb

@@ -54,7 +54,8 @@ def method_missing(method, *args, &block)
     end
 
     def respond_to?(method, *args)
-      ((cache = @resource_cache[method]) && cache[:class]) || !resource_class_for(method).nil? || super
+      cache = @resource_cache[method]
+      !!(cache.to_h[:class] || resource_class_for(method) || super)
     end
 
     # Returns the current user (aka me)
@@ -185,8 +186,8 @@ def build_connection
     private
 
     def resource_class_for(method)
-      klass_as_string = ZendeskAPI::Helpers.modulize_string(Inflection.singular(method.to_s.gsub(/\W/, '')))
-      ZendeskAPI::Association.class_from_namespace(klass_as_string)
+      resource_name = ZendeskAPI::Helpers.modulize_string(Inflection.singular(method.to_s.gsub(/\W/, '')))
+      ZendeskAPI::Association.class_from_namespace(resource_name)
     end
 
     def check_url

lib/zendesk_api/collection.rb

@@ -348,33 +348,37 @@ def cbp_response?(body)
       !!(body["meta"] && body["links"])
     end
 
+    def set_cbp_options
+      @options_per_page_was = @options.delete("per_page")
+      # Default to CBP by using the page param as a map
+      @options.page = { size: (@options_per_page_was || DEFAULT_PAGE_SIZE) }
+    end
+
     def cbp_request?
+      # CBP requests look like: `/resources?page[size]=100`
+      # OBP requests look like: `/resources?page=2`
       @options["page"].is_a?(Hash)
     end
 
     def intentional_obp_request?
       Helpers.present?(@options["page"]) && !cbp_request?
     end
 
-    def supports_cursor_based_pagination?
+    def supports_cbp?
       @resource_class.const_defined?(:CBP_ACTIONS) && @resource_class.const_get(:CBP_ACTIONS).any? { |supported_path| path.end_with?(supported_path) }
     end
 
-    def first_of_multiple_pages?
+    def first_cbp_request?
+      # @next_page will be nil when making the first cbp request
       @next_page.nil?
     end
 
-    def modify_request_for_cbp
-      @options_per_page_was = @options.delete("per_page")
-      # Default to CBP by using the page param as a map
-      @options.page = { size: (@options_per_page_was || DEFAULT_PAGE_SIZE) }
-    end
-
     def get_resources(path_query_link)
       if intentional_obp_request?
         warn "Offset Based Pagination will be deprecated soon"
-      elsif supports_cursor_based_pagination? && first_of_multiple_pages?
-        modify_request_for_cbp
+      elsif supports_cbp? && first_cbp_request?
+        # only set cbp options if it's the first request, otherwise the options would be already in place
+        set_cbp_options
       end
       @response = get_response(path_query_link)
 
@@ -391,14 +395,7 @@ def set_page_and_count(body)
       @next_page, @prev_page = page_links(body)
 
       if cbp_response?(body)
-        @options.page = {} unless cbp_request?
-        # the line above means an intentional CBP request where page[size] is passed on the query
-        # this is to cater for CBP responses where we don't specify page[size] but the endpoint
-        # responds CBP by default. i.e  `client.trigger_categories.fetch`
-        @options.page.merge!(
-          after: body["meta"]["after_cursor"],
-          before: body["meta"]["before_cursor"]
-                             )
+        set_cbp_response_options(body)
       elsif @next_page =~ /page=(\d+)/
         @options["page"] = $1.to_i - 1
       elsif @prev_page =~ /page=(\d+)/
@@ -571,5 +568,16 @@ def assert_results(results, body)
       return if results
       raise ZendeskAPI::Error::ClientError, "Expected #{@resource_class.model_key} or 'results' in response keys: #{body.keys.inspect}"
     end
+
+    def set_cbp_response_options(body)
+      @options.page = {} unless cbp_request?
+      # the line above means an intentional CBP request where page[size] is passed on the query
+      # this is to cater for CBP responses where we don't specify page[size] but the endpoint
+      # responds CBP by default. i.e  `client.trigger_categories.fetch`
+      @options.page.merge!(
+        before: body["meta"]["before_cursor"],
+        after: body["meta"]["after_cursor"]
+                           )
+    end
   end
 end

spec/core/collection_spec.rb

@@ -563,7 +563,7 @@ def to_proc
         stub_json_request(:get, %r{test_resources}, json(cbp_response))
       end
 
-      it "should set the next and previous pages and cursors" do
+      it "sets the next and previous pages and cursors" do
         subject.fetch
         expect(subject.instance_variable_get(:@next_page)).to eq("next_page")
         expect(subject.instance_variable_get(:@prev_page)).to eq("previous_page")