Change strategy

mattapayne · mattapayne · commit d0f152f6c382 · 2018-05-22T18:27:02.000-07:00
diff --git a/README.md b/README.md
@@ -22,7 +22,15 @@ Or install it yourself as:
 
 ## Usage
 
-TODO: Write usage instructions here
+Currently, the API bindings only expose the following endpoints:
+
+1. [Contacts](https://developers.freshdesk.com/api/#contacts)
+2. [Companies](https://developers.freshdesk.com/api/#companies)
+3. [Contact Fields](https://developers.freshdesk.com/api/#list_all_contact_fields)
+4. [Company Fields](https://developers.freshdesk.com/api/#list_all_company_fields)
+
+Note that while search/filter works for both Companies and Contacts, the API is still not working perfectly.
+For example, filtering Contacts by email fails - likely due to encosing issues on Freshdesk's side.
 
 ## Development
 
diff --git a/lib/freshdesk_api_v2/base.rb b/lib/freshdesk_api_v2/base.rb
@@ -4,43 +4,44 @@ def initialize(http)
       @http = http
     end
 
+    # pagination_options: page: integer > 0, per_page: integer > 0 && <= 100
     def list(pagination_options = {})
-      per_page = pagination_options[:per_page] || Utils::MAX_PAGE_SIZE
-      raise PaginationException, "Max per page is #{Utils::MAX_PAGE_SIZE}" if per_page.to_i > Utils::MAX_PAGE_SIZE
-      first_page, last_page = extract_list_pagination(pagination_options)
-      validate_list_pagination!(first_page, last_page)
-      paginated_get(first_page, last_page, per_page)
+      page = pagination_options[:page]
+      per_page = pagination_options[:per_page]
+      validate_list_pagination!(page, per_page)
+      @http.get("#{endpoint}?page=#{page}&per_page=#{per_page}")
     end
 
     # TODO - Note that queries that by email or things that have special characters to not work yet in
-    # Freshdesk
+    # Freshdesk. This appears to be a bug on their side.
+    # query: An instance of FreshdeskApiV2::SearchArgs
+    # pagination_options: page: integer > 0 && <= 10, per_page: integer > 0 && <= 30
     def search(query, pagination_options = {})
-      raise SearchException, 'You must provide a query' if query.nil?
-      raise SearchException, 'You must provide a query of type FreshdeskApiV2::SearchArgs' unless query.is_a?(FreshdeskApiV2::SearchArgs)
-      raise SearchException, 'You must provide a query' unless query.valid?
-      first_page, last_page = extract_search_pagination(pagination_options)
-      validate_search_pagination!(first_page, last_page)
-      paginated_search(query.to_query, first_page, last_page)
+      validate_search_query!(query)
+      page = pagination_options[:page]
+      per_page = pagination_options[:per_page]
+      validate_search_pagination!(page, per_page)
+      @http.get("#{endpoint}/search/#{endpoint}?page=#{page}&query=#{query}")
     end
 
-    def show(id)
-      get(id)
+    def get(id)
+      @http.get("/#{endpoint}/#{id}")
     end
 
     def create(attributes)
       validate_create_attributes!(attributes)
       attributes = prepare_attributes!(attributes)
-      post(attributes)
+      @http.post(endpoint, attributes)
     end
 
     def update(id, attributes)
       validate_update_attributes!(attributes)
       attributes = prepare_attributes!(attributes)
-      put(id, attributes)
+      @http.put("#{endpoint}/#{id}", attributes)
     end
 
     def destroy(id)
-      delete(id)
+      @http.delete("#{endpoint}/#{id}")
     end
 
     protected
@@ -59,16 +60,19 @@ def validate_update_attributes!(attributes)
         raise UpdateException, 'Please provide attributes' if attributes.nil? || attributes.count == 0
       end
 
-      def validate_list_pagination!(first_page, last_page)
-        raise PaginationException, 'first_page must be a number greater than 0' if first_page.to_i <= 0
-        raise PaginationException, 'last_page must be a number greater than or equal to first_page' if last_page.to_i < first_page.to_i
+      def validate_list_pagination!(page, per_page)
+        raise PaginationException, 'page must be a number greater than 0' if !page.nil? && page.to_i <= 0
+        unless per_page.nil?
+          raise PaginationException, 'per_page must be a number greater than 0' if per_page.to_i <= 0
+          raise PaginationException, "per_page must be a number less than or equal to #{Utils::MAX_LIST_PER_PAGE}" if per_page.to_i > Utils::MAX_LIST_PER_PAGE
+        end
       end
 
-      def validate_search_pagination!(first_page, last_page)
-        raise PaginationException, 'first_page must be a number greater than 0' if first_page.to_i <= 0
-        unless last_page.nil?
-          raise PaginationException, "last_page cannot exceed #{Utils::MAX_SEARCH_PAGES}" if last_page.to_i > Utils::MAX_SEARCH_PAGES
-          raise PaginationException, 'last_page must be a number greater than or equal to first_page' if last_page.to_i < first_page.to_i
+      def validate_search_pagination!(page, per_page)
+        raise PaginationException, 'page must be a number greater than 0' if !page.nil? && page.to_i <= 0
+        unless per_page.nil?
+          raise PaginationException, 'per_page must be a number greater than 0' if per_page.to_i <= 0
+          raise PaginationException, "per_page must be a number less than or equal to #{Utils::MAX_SEARCH_PER_PAGE}" if per_page.to_i > Utils::MAX_SEARCH_PER_PAGE
         end
       end
 
@@ -82,57 +86,10 @@ def prepare_attributes!(attributes)
         clean
       end
 
-    private
-
-      def get(id)
-        response = @http.get("#{api_url}/#{id}")
-        JSON.parse(response.body)
-      end
-
-      def post(attributes)
-        response = @http.post("#{api_url}", attributes)
-        JSON.parse(response.body)
-      end
-
-      def delete(id)
-        response = @http.delete("#{api_url}/#{id}")
-        response.status
-      end
-
-      def put(id, attributes)
-        response = @http.put("#{api_url}/#{id}", attributes)
-        JSON.parse(response.body)
-      end
-
-      def paginated_get(first_page, last_page, per_page)
-        url = "#{api_url}?page=#{first_page}&per_page=#{per_page}"
-        @http.paginated_get(url, last_page)
-      end
-
-      # For example, see: https://developers.freshdesk.com/api/#filter_contacts
-      def paginated_search(query, first_page, last_page)
-        url = "#{base_api_url}/search/#{endpoint}?page=#{first_page}&query=#{query}"
-        @http.paginated_search(url, last_page)
-      end
-
-      def extract_list_pagination(options)
-        first_page = options[:first_page] || Utils::DEFAULT_PAGE
-        last_page = options[:last_page] || Utils::INTEGER_MAX
-        [first_page, last_page]
-      end
-
-      def extract_search_pagination(options)
-        first_page = options[:first_page] || Utils::DEFAULT_PAGE
-        last_page = options[:last_page]
-        [first_page, last_page]
-      end
-
-      def base_api_url
-        "https://#{@http.domain}.freshdesk.com/api/v2"
-      end
-
-      def api_url
-        "#{base_api_url}/#{endpoint}"
+      def validate_search_query!(query)
+        raise SearchException, 'You must provide a query' if query.nil?
+        raise SearchException, 'You must provide a query of type FreshdeskApiV2::SearchArgs' unless query.is_a?(FreshdeskApiV2::SearchArgs)
+        raise SearchException, 'You must provide a query' unless query.valid?
       end
   end
 end
diff --git a/lib/freshdesk_api_v2/client.rb b/lib/freshdesk_api_v2/client.rb
@@ -1,16 +1,14 @@
 module FreshdeskApiV2
   class Client
-    attr_reader :configuration
-
     def initialize(configuration = nil)
       if !configuration.nil?
-        @configuration = Config.new(configuration) if configuration.is_a?(Hash)
-        @configuration = configuration if configuration.is_a?(FreshdeskApiV2::Config)
+        configuration = Config.new(configuration) if configuration.is_a?(Hash)
+        configuration = configuration if configuration.is_a?(FreshdeskApiV2::Config)
       else
-        @configuration = FreshdeskApiV2.configuration
+        configuration = FreshdeskApiV2.configuration
       end
-      @configuration.validate!
-      @http = Http.new(@configuration)
+      configuration.validate!
+      @http = Http.new(configuration)
     end
 
     def contacts
diff --git a/lib/freshdesk_api_v2/http.rb b/lib/freshdesk_api_v2/http.rb
@@ -2,98 +2,35 @@ module FreshdeskApiV2
   class Http
     def initialize(configuration)
       @configuration = configuration
-      @link_parser = Nitlink::Parser.new
     end
 
-    # Freshdesk's search API uses a different pagination mechanism than their
-    # regular pagination mechanism, so this implementation needs to be different.
-    def paginated_search(url, last_page, collection = [])
-      current_page = 1
-      page_count, items = do_search(url)
-      collection += items
-      return collection if page_count == 1
-      loop do
-        current_page += 1
-        # Freshdesk will only return up to a maximum of 10 pages, so
-        # kill the loop if we get that far OR if we hit the last requested page
-        break if current_page > Utils::MAX_SEARCH_PAGES || current_page > last_page
-        url.gsub!("?page=#{current_page - 1}", "?page=#{current_page}")
-        _, items = do_search(url)
-        collection += items
-        break if current_page > page_count
-      end
-      collection
-    end
-
-    # This is Freshdesk's normal pagination. It always returns a link to the next
-    # page in a header called 'link' with a rel of 'next'
-    def paginated_get(url, last_page, collection = [])
-      response = get(url)
-      links = @link_parser.parse(response)
-      collection += JSON.parse(response.body)
-      while !links.nil? && links.by_rel('next')
-        url = links.by_rel('next').target.to_s
-        next_page = next_page(url)
-        if next_page.to_i <= last_page.to_i
-          response = get(url)
-          links = @link_parser.parse(response)
-          collection += JSON.parse(response.body)
-        else
-          links = nil
-        end
-      end
-      collection
-    end
-
-    def get(url)
-      res = construct_http_client(url)
+    def get(path, headers = {})
+      res = construct_http_client(path, default_headers.merge(headers))
       res.get
     end
 
-    def delete(url)
-      res = construct_http_client(url)
+    def delete(path, headers = {})
+      res = construct_http_client(path, default_headers.merge(headers))
       res.delete
     end
 
-    def put(url, attributes)
-      res = construct_http_client(url)
+    def put(path, attributes, headers = {})
+      res = construct_http_client(path, default_headers.merge(headers))
       res.put(body: attributes.to_json)
     end
 
-    def post(url, attributes)
-      res = construct_http_client(url)
+    def post(path, attributes, headers = {})
+      res = construct_http_client(path, default_headers.merge(headers))
       res.post(body: attributes.to_json)
     end
 
-    def domain
-      @configuration.domain
-    end
-
     private
 
-      def do_search(url)
-        response = get(url)
-        payload = JSON.parse(response.body)
-        total = (payload['total'] || 0).to_f
-        if total > Utils::MAX_SEARCH_RESULTS_PER_PAGE
-          page_count = (total / Utils::MAX_SEARCH_RESULTS_PER_PAGE).ceil
-        else
-          page_count = 1
-        end
-        results = payload['results']
-        if results.nil? && response.status == 400
-          error_desc = payload['description']
-          errors = payload['errors']
-          raise InvalidSearchException.new(error_desc, errors)
-        end
-        [page_count, results]
-      end
-
-      def next_page(url)
-        url[Utils::PAGE_REGEX, 1]
+      def base_api_url
+        "https://#{@configuration.domain}.freshdesk.com/api/v2"
       end
 
-      def headers
+      def default_headers
         {
           'Accept' => 'application/json',
           'Content-Type' => 'application/json'
@@ -116,8 +53,8 @@ def username_or_api_key
         end
       end
 
-      def construct_http_client(url)
-        Excon.new(url,
+      def construct_http_client(path, headers)
+        Excon.new("#{base_api_url}/#{path}",
           uri_parser: Addressable::URI,
           headers: headers,
           user: username_or_api_key,
diff --git a/lib/freshdesk_api_v2/list_only_base.rb b/lib/freshdesk_api_v2/list_only_base.rb
@@ -5,8 +5,7 @@ def initialize(http)
     end
 
     def list
-      response = @http.get(api_url)
-      JSON.parse(response.body)
+      @http.get(api_url)
     end
 
     protected
@@ -19,7 +18,7 @@ def endpoint
     private
 
       def base_api_url
-        "https://#{@http.domain}.freshdesk.com/api/v2"
+        "https://#{@http.send(:domain)}.freshdesk.com/api/v2"
       end
 
       def api_url
diff --git a/lib/freshdesk_api_v2/search_args.rb b/lib/freshdesk_api_v2/search_args.rb
@@ -37,5 +37,11 @@ def to_query
       s = @args.join('')
       '"' + s + '"'
     end
+
+    class << self
+      def create(field, value)
+        new().add(field, value)
+      end
+    end
   end
 end
diff --git a/lib/freshdesk_api_v2/utils.rb b/lib/freshdesk_api_v2/utils.rb
@@ -1,11 +1,12 @@
 module FreshdeskApiV2
   module Utils
-    INTEGER_MAX = 2_147_483_647
-    PAGE_REGEX = /.*[\?\&]page=(\d)*.*/
-    MAX_SEARCH_RESULTS_PER_PAGE = 30.0
-    MAX_SEARCH_PAGES = 10
     DEFAULT_PAGE = 1
-    MAX_PAGE_SIZE = 100
-    MAX_PAGE_SIZE_SEARCH = 30
+
+    MAX_LIST_PER_PAGE = 100
+    MIN_LIST_PER_PAGE = 1
+
+    MAX_SEARCH_PAGES = 10
+    MAX_SEARCH_PER_PAGE = 30
+    MIN_SEARCH_PER_PAGE = 1
   end
 end
