Use cursor based pagination for endpoints that support it #614

sircpl · 2023-08-30T21:52:58Z

Use CBP for all endpoints that support it.

This PR does not include any of the newly added endpoints in #608. New endpoints supporting CBP will be added after this PR is merged (since this PR contains the cbp method).

aheritier

LGTM but I cannot test them. As said in the
Last PR, even if we don't implement 100% of zendesk APIs it might be great to start to provide a stable API and to have a formal versioning.

(Especially if we can get some official support from Zendesk like here)

cryptomail · 2023-08-31T18:08:17Z

@PierreBtz 🌞 Looking for administrative review 🌞

PierreBtz · 2023-09-01T15:15:30Z

Sorry I didn't have time to progress on this. I'll progress on the review and testing next week.

PierreBtz

The changes look good to me, but there is a big issue in that the Rest API documentation hasn't been properly updated for me to validate the changes (except by trial and error...).

The doc clearly states that CBP works in two different ways (Paginating with the next link and Paginating with the after cursor), and that we should refer to each individual endpoint documentation to know what is supported, but, except if I missed something, there are a large number of endpoints that either do not specify anything, or show examples using the old OBP.

This makes it hard to entirely review this change, and be confident things won't break in the future.

This has nothing to do with your PR per-se, thanks a lot for the work you put into this, but I think we should clarify and have the documentation updated.
I can take care of this through the support but maybe things will go faster if you ask internally?

PierreBtz · 2023-09-04T08:31:54Z

src/main/java/org/zendesk/client/v2/Zendesk.java

-  public List<Brand> getBrands() {
-    return complete(submit(req("GET", cnst("/brands.json")), handleList(Brand.class, "brands")));
+  public Iterable<Brand> getBrands() {
+    return new PagedIterable<>(cbp("/brands.json"), handleList(Brand.class, "brands"));


This change doesn't match the current Rest API documentation.

According to it:

a endpoint may or may not support CBP, I need to refer to the endpoint documentation (https://developer.zendesk.com/documentation/api-basics/pagination/paginating-through-lists-using-cursor-pagination/).

refering to the brand endpoint, the only example I see uses OBP (https://developer.zendesk.com/api-reference/ticketing/account-configuration/brands/)

This is of importance, because according to the documentation, depending on the endpoint we need to check the endpoint doc to choose between "Paginating with the after cursor" and "Paginating with the next link".

Usually I would open a ticket with Zendesk support to get clarification and update of the doc if necessary, but in this case I trust you'll have a quicker answer by yourself :)

This endpoint is not in the list provided in https://support.zendesk.com/hc/en-us/articles/5591904358938-New-limits-for-offset-based-pagination#h_01GNCT8Z7DJ7G4TQ3CFFXQ007Z

@PierreBtz The support for Cursor Model is determined based on the endpoint use case and the depth of records we saw in the system for all customers.

We are working on your feedback and currently evaluating the pagination information in the developer docs. We will get the right details added to every endpoint. There is active work in progress now on this outcome. Should be done soon.

cryptomail · 2023-09-04T13:43:16Z

@PierreBtz Here is an article that outlines every endpoint that has CBP (as a choice by passing page[size]=XXX). I will advise internal documentation of the gap and they'll proceed with your suggestions or answer them!.

PierreBtz · 2023-09-05T07:46:43Z

Thanks for the article, it indeed gives a list of endpoints supporting CBP, I'll use that as a reference for this PR, hoping to see the rest api doc updated in the future :)

PierreBtz

We're nearly ready to merge.
I still found a handful of endpoints without any documentation suggesting CBP support (either in the announcement post or the endpoint doc). Once this is sorted out, I think this will be ready.

PierreBtz · 2023-09-05T07:48:06Z

src/main/java/org/zendesk/client/v2/Zendesk.java

-  public List<Brand> getBrands() {
-    return complete(submit(req("GET", cnst("/brands.json")), handleList(Brand.class, "brands")));
+  public Iterable<Brand> getBrands() {
+    return new PagedIterable<>(cbp("/brands.json"), handleList(Brand.class, "brands"));


This endpoint is not in the list provided in https://support.zendesk.com/hc/en-us/articles/5591904358938-New-limits-for-offset-based-pagination#h_01GNCT8Z7DJ7G4TQ3CFFXQ007Z

PierreBtz · 2023-09-05T07:50:09Z

src/main/java/org/zendesk/client/v2/Zendesk.java

-        submit(
-            req("GET", tmpl("/tickets/{id}/incidents.json").set("id", id)),
-            handleList(Ticket.class, "tickets")));
+  public Iterable<Ticket> getTicketIncidents(long id) {


Not in the list provided in https://support.zendesk.com/hc/en-us/articles/5591904358938-New-limits-for-offset-based-pagination#h_01GNCT8Z7DJ7G4TQ3CFFXQ007Z

The endpoint documentation doesn't include CBP either (https://developer.zendesk.com/api-reference/ticketing/tickets/tickets/#list-ticket-incidents)

PierreBtz · 2023-09-05T08:35:58Z

src/main/java/org/zendesk/client/v2/Zendesk.java

@@ -1500,18 +1505,18 @@ public Iterable<AgentRole> getCustomAgentRoles() {

  public Iterable<org.zendesk.client.v2.model.Request> getRequests() {
    return new PagedIterable<>(
-        cnst("/requests.json"), handleList(org.zendesk.client.v2.model.Request.class, "requests"));
+        cbp("/requests.json"), handleList(org.zendesk.client.v2.model.Request.class, "requests"));


Not in the list provided in support.zendesk.com/hc/en-us/articles/5591904358938-New-limits-for-offset-based-pagination#h_01GNCT8Z7DJ7G4TQ3CFFXQ007Z

The endpoint documentation doesn't include CBP either https://developer.zendesk.com/api-reference/ticketing/tickets/ticket-requests/#list-requests

PierreBtz · 2023-09-05T08:36:29Z

src/main/java/org/zendesk/client/v2/Zendesk.java

  }

  public Iterable<org.zendesk.client.v2.model.Request> getOpenRequests() {
    return new PagedIterable<>(
-        cnst("/requests/open.json"),
+        cbp("/requests/open.json"),


PierreBtz · 2023-09-05T08:36:34Z

src/main/java/org/zendesk/client/v2/Zendesk.java

        handleList(org.zendesk.client.v2.model.Request.class, "requests"));
  }

  public Iterable<org.zendesk.client.v2.model.Request> getSolvedRequests() {
    return new PagedIterable<>(
-        cnst("/requests/solved.json"),
+        cbp("/requests/solved.json"),


PierreBtz · 2023-09-05T08:43:17Z

src/main/java/org/zendesk/client/v2/Zendesk.java

-                "GET",
-                tmpl("/users/{user_id}/organization_memberships.json").set("user_id", user_id)),
-            handleList(OrganizationMembership.class, "organization_memberships")));
+  public Iterable<OrganizationMembership> getOrganizationMembershipByUser(long user_id) {


Nice catch, we could probably deprecate this method for later deletion, but outside of the scope of this PR.

PierreBtz · 2023-09-05T08:44:51Z

src/main/java/org/zendesk/client/v2/Zendesk.java

@@ -2583,7 +2580,7 @@ public Iterable<DynamicContentItemVariant> getDynamicContentItemVariants(
      DynamicContentItem item) {
    checkHasId(item);
    return new PagedIterable<>(
-        tmpl("/dynamic_content/items/{id}/variants.json").set("id", item.getId()),
+        cbp("/dynamic_content/items/{id}/variants.json").set("id", item.getId()),


Not in the list provided in support.zendesk.com/hc/en-us/articles/5591904358938-New-limits-for-offset-based-pagination#h_01GNCT8Z7DJ7G4TQ3CFFXQ007Z

The endpoint documentation doesn't include CBP either https://developer.zendesk.com/api-reference/ticketing/ticket-management/dynamic_content/#list-items

PierreBtz · 2023-09-05T08:44:59Z

src/main/java/org/zendesk/client/v2/Zendesk.java

@@ -2537,7 +2534,7 @@ public SatisfactionRating createSatisfactionRating(

  public Iterable<DynamicContentItem> getDynamicContentItems() {
    return new PagedIterable<>(
-        cnst("/dynamic_content/items.json"), handleList(DynamicContentItem.class, "items"));
+        cbp("/dynamic_content/items.json"), handleList(DynamicContentItem.class, "items"));


Not in the list provided in support.zendesk.com/hc/en-us/articles/5591904358938-New-limits-for-offset-based-pagination#h_01GNCT8Z7DJ7G4TQ3CFFXQ007Z

The endpoint documentation doesn't include CBP either developer.zendesk.com/api-reference/ticketing/ticket-management/dynamic_content/#list-items

PierreBtz · 2023-09-05T09:05:39Z

I took the liberty of refreshing your branch onto the latest master commits, and also fix the formatting, so that we have a fresh CI run.

Btw I finally understood the trouble you had with spotless. It turns out the formatting is different between a jdk8 and a jdk11/17... The project is still compatible with Java 8, so you need to format using a jdk8.

cryptomail · 2023-09-05T13:54:30Z

@PierreBtz We're going to address your concerns. Thanks so much for looking all this up.
You're 100% correct that article is incomplete. Just from cursory inspection and execution of the endpoint https://mydomain.zendesk.com/api/v2/dynamic_content/items?page[size]=1 returns CBP as the output protocol.
Again you have pointed out clean deficiencies within the documentation of our API that must be addressed.

PierreBtz · 2023-09-29T14:33:20Z

@cryptomail @sircpl any news on your side, you did a good and important work and I'm a bit sad to see it blocked for some documentation issues.

PierreBtz · 2023-10-19T07:45:40Z

@cryptomail @sircpl I had a quick look and the endpoints are still not officially compatible with CBP. My proposal to get this moving is to partially revert the problematic endpoints and merge the rest of the work. Would that work for you? I can take care of the partial revert.

cryptomail · 2023-10-19T15:14:48Z

@PierreBtz Let me have a talk with product and follow up with their documentation efforts:

PierreBtz

As discussed privately with the Zendesk team, I'm going ahead with the merge in advance of the documentation update.

Use cursor based pagination for endpoints that support it

69f1b4a

sircpl requested review from duemir and PierreBtz as code owners August 30, 2023 21:52

sircpl mentioned this pull request Aug 30, 2023

Sircpl/cbp by default #608

Closed

aheritier approved these changes Aug 31, 2023

View reviewed changes

PierreBtz added the chore label Sep 4, 2023

PierreBtz requested changes Sep 4, 2023

View reviewed changes

PierreBtz requested changes Sep 5, 2023

View reviewed changes

PierreBtz added 2 commits September 5, 2023 10:56

Merge remote-tracking branch 'upstream/HEAD' into sircpl/cbp_only

198c6f3

fix formatting

0f9b6e1

PierreBtz mentioned this pull request Oct 19, 2023

Updated subscriptions methods to use cursor pagination #626

Closed

PierreBtz added 2 commits November 6, 2023 10:07

Merge branch 'master' into sircpl/cbp_only

72210a7

fix merge issue

6509339

PierreBtz approved these changes Nov 6, 2023

View reviewed changes

PierreBtz merged commit 6faf15d into cloudbees-oss:master Nov 6, 2023
3 checks passed

PierreBtz mentioned this pull request Nov 6, 2023

[bug] Revert a test that highlights a bug in the Zendesk Rest API #632

Closed

This was referenced Nov 6, 2023

Fix sorting when using cbp for Triggers; Fix Trigger model to match documented JSON format #633

Closed

Revert changes to getTriggersWithParameters due to CBP bug in API #634

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use cursor based pagination for endpoints that support it #614

Use cursor based pagination for endpoints that support it #614

sircpl commented Aug 30, 2023

aheritier left a comment

cryptomail commented Aug 31, 2023

PierreBtz commented Sep 1, 2023

PierreBtz left a comment

PierreBtz Sep 4, 2023

PierreBtz Sep 5, 2023

snatchblock Sep 6, 2023

cryptomail commented Sep 4, 2023 •

edited

Loading

PierreBtz commented Sep 5, 2023

PierreBtz left a comment

PierreBtz Sep 5, 2023

PierreBtz Sep 5, 2023

PierreBtz Sep 5, 2023

PierreBtz Sep 5, 2023

PierreBtz Sep 5, 2023

PierreBtz Sep 5, 2023

PierreBtz Sep 5, 2023

PierreBtz Sep 5, 2023

PierreBtz commented Sep 5, 2023

cryptomail commented Sep 5, 2023

PierreBtz commented Sep 29, 2023

PierreBtz commented Oct 19, 2023

cryptomail commented Oct 19, 2023

PierreBtz left a comment

Use cursor based pagination for endpoints that support it #614

Use cursor based pagination for endpoints that support it #614

Conversation

sircpl commented Aug 30, 2023

aheritier left a comment

Choose a reason for hiding this comment

cryptomail commented Aug 31, 2023

PierreBtz commented Sep 1, 2023

PierreBtz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cryptomail commented Sep 4, 2023 • edited Loading

PierreBtz commented Sep 5, 2023

PierreBtz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PierreBtz commented Sep 5, 2023

cryptomail commented Sep 5, 2023

PierreBtz commented Sep 29, 2023

PierreBtz commented Oct 19, 2023

cryptomail commented Oct 19, 2023

PierreBtz left a comment

Choose a reason for hiding this comment

cryptomail commented Sep 4, 2023 •

edited

Loading