zendesk / zendesk_api_client_rb

Official Ruby Zendesk API Client
http://developer.zendesk.com/
Apache License 2.0
386 stars 184 forks source link

Offset Based Pagination will be deprecated soon #581

Closed navidemad closed 1 week ago

navidemad commented 4 weeks ago

Context

rails: 7.1.3.4 zendesk_api: 3.1.0 ruby: 3.3.4

Goal

Fetching requested tickets from our customer

Problem

We are having the following warning when we followed the CBP example from: https://github.com/zendesk/zendesk_api_client_rb?tab=readme-ov-file#pagination

Offset Based Pagination will be deprecated soon

Debug

Our customer has 10 tickets. I used per_page(2) to make sure the pagination works fine, we will put 100 once we fixed this issue. I put some log on https://github.com/search?q=repo%3Azendesk%2Fzendesk_api_client_rb%20intentional_obp_request%3F&type=code

@options: #<ZendeskAPI::SilentMash page=nil query="email:flabro@hotmail.com">
@options: #<ZendeskAPI::SilentMash per_page=2>
@options: #<ZendeskAPI::SilentMash page=2 per_page=2>
Offset Based Pagination will be deprecated soon
@options: #<ZendeskAPI::SilentMash page=3 per_page=2>
Offset Based Pagination will be deprecated soon
@options: #<ZendeskAPI::SilentMash page=4 per_page=2>
Offset Based Pagination will be deprecated soon
@options: #<ZendeskAPI::SilentMash page=5 per_page=2>
Offset Based Pagination will be deprecated soon

Code:

# frozen_string_literal: true

class Zendesk::AllRequestedTicketsService
  attr_reader :errors

  def initialize(user_id:)
    @user_id = user_id
    @errors = []
  end

  def success?
    @errors.blank?
  end

  def failure?
    @errors.present?
  end

  # https://github.com/zendesk/zendesk_api_client_rb?tab=readme-ov-file#pagination
  def call
    user = User.find_by(id: @user_id)
    return [] if user.blank?

    zendesk_client = ZendeskAPI::Client.new do |config|
      config.url = ENV.fetch("ZENDESK_URL")
      config.username = ENV.fetch("ZENDESK_USERNAME")
      config.token = ENV.fetch("ZENDESK_TOKEN")
      config.retry = true
    end

    zendesk_user_client = zendesk_client.users.search(query: "email:#{user.email}").first
    return [] if !zendesk_user_client || !zendesk_user_client.respond_to?(:id) || zendesk_user_client.id.blank?

    tickets = Set.new
    current_page = zendesk_user_client.requested_tickets.per_page(2)

    loop do
      current_page.fetch!
      tickets.merge(current_page.map(&:as_json))
      break if current_page.last_page?
      current_page.next
    rescue StandardError => e
      @errors << e.message
      break
    end

    tickets = tickets.to_a
    tickets.sort_by! { |ticket| -ticket["created_at"].to_datetime.to_i }
    tickets
  end
end

Question

Is there something we should change in our code ?

Thanks

fbvilela commented 3 weeks ago

Hi @navidemad , thanks for raising this issue! This endpoint is indeed missing CBP support which is added by https://github.com/zendesk/zendesk_api_client_rb/pull/582 - it's still a DRAFT PR, missing tests for now. One thing you could do while we don't merge it is to grab that change and test it on your own. I also have made a few suggestions on how to simplify your code. Basically you don't need to manually paginate through data (unless you want to).

 # Your `call` method could have the following changes after our fix is in place:
    zendesk_user_client = client.current_user # get the user you need
    tickets = Set.new
    zendesk_user_client.requested_tickets.all!{ |ticket| tickets.add ticket }
    # This will take care of the pagination for you. The default will be 100 per page.
    # No need to set per_page to 2.

    tickets.sort_by{ |ticket| ticket.created_at }.reverse # most recent tickets at the top.
    # notice that this line returns (from the method `call``) the sorted array

Let me know if you have any more questions. thanks again for raising the issue 🙏

navidemad commented 3 weeks ago

@fbvilela

Alright i tested with your suggestions and a simple initializer to have your PR change, and it seems to be working fine :

# https://github.com/zendesk/zendesk_api_client_rb/pull/582
# config/initializers/zendesk_client.rb
Rails.application.config.to_prepare do
  ZendeskAPI::Ticket.singleton_class.prepend(
    Module.new do
      def cbp_path_regexes
        super | [%r{users/\d+/tickets/requested}]
      end
    end,
  )
end
ecoologic commented 1 week ago

Problem solved for the requester, closing.