Skip to content

Bugfix/taxonomy pagination#2410

Merged
jasonbahl merged 3 commits intowp-graphql:developfrom
jasonbahl:bugfix/taxonomy-pagination
Jun 21, 2022
Merged

Bugfix/taxonomy pagination#2410
jasonbahl merged 3 commits intowp-graphql:developfrom
jasonbahl:bugfix/taxonomy-pagination

Conversation

@jasonbahl
Copy link
Copy Markdown
Collaborator

What does this implement/fix? Explain your changes.

This fixes a bug with taxonomy pagination where the end cursor wasn't properly being respected for subsequent paginated requests.

Does this close any currently open issues?

closes #2343

Any other comments?

With the following snippet I've registered a taxonomy for every letter of the alphabet.

So we end up with the following list of taxonomies:

{
  "category": "category",
  "post_tag": "post_tag",
  "post_format": "post_format",
  "A": "A",
  "B": "B",
  "C": "C",
  "D": "D",
  "E": "E",
  "F": "F",
  "G": "G",
  "H": "H",
  "I": "I",
  "J": "J",
  "K": "K",
  "L": "L",
  "M": "M",
  "N": "N",
  "O": "O",
  "P": "P",
  "Q": "Q",
  "R": "R",
  "S": "S",
  "T": "T",
  "U": "U",
  "V": "V",
  "W": "W",
  "X": "X",
  "Y": "Y",
  "Z": "Z"
}

I can then make a query for a set of these taxonomies like so:

query testTaxonomies($first: Int, $after: String, $last: Int, $before: String ) {
  taxonomies(first: $first, last: $last, before: $before, after: $after) {
    pageInfo {
      endCursor
      hasNextPage
      hasPreviousPage
      startCursor
    }
    nodes {
      id
      name
    }
  }
}

with variables:

{
  "first": 5
}

and we should expect to get the first 5 taxonomies in response:

  "category": "category",
  "post_tag": "post_tag",
  "post_format": "post_format",
  "A": "A",
  "B": "B",

We can see this working as expected.

CleanShot 2022-06-20 at 11 14 53@2x

Then, we should be able to take the pageInfo.endCursor value and use it as the input value for the after variable to query the next page.

Like so:

{
  "first": 5,
  "after": "YXJyYXljb25uZWN0aW9uOkI="
}

This is where the bug was happening.

We should expect nodes (the first 5 nodes after node "B"):

  "C": "C",
  "D": "D",
  "E": "E",
  "F": "F",
  "G": "G",

BEFORE

Before this PR, we would get the following output:

{
  "data": {
    "taxonomies": {
      "pageInfo": {
        "endCursor": "YXJyYXljb25uZWN0aW9uOkM=",
        "hasNextPage": true,
        "hasPreviousPage": true,
        "startCursor": "YXJyYXljb25uZWN0aW9uOnBvc3RfdGFn"
      },
      "nodes": [
        {
          "id": "dGF4b25vbXk6cG9zdF90YWc=",
          "name": "post_tag"
        },
        {
          "id": "dGF4b25vbXk6cG9zdF9mb3JtYXQ=",
          "name": "post_format"
        },
        {
          "id": "dGF4b25vbXk6QQ==",
          "name": "A"
        },
        {
          "id": "dGF4b25vbXk6Qg==",
          "name": "B"
        },
        {
          "id": "dGF4b25vbXk6Qw==",
          "name": "C"
        }
      ]
    }
  },
}

This is not what we want!

AFTER

After this PR, the same query returns the following:

{
  "data": {
    "taxonomies": {
      "pageInfo": {
        "endCursor": "YXJyYXljb25uZWN0aW9uOkc=",
        "hasNextPage": true,
        "hasPreviousPage": false,
        "startCursor": "YXJyYXljb25uZWN0aW9uOkM="
      },
      "nodes": [
        {
          "id": "dGF4b25vbXk6Qw==",
          "name": "C"
        },
        {
          "id": "dGF4b25vbXk6RA==",
          "name": "D"
        },
        {
          "id": "dGF4b25vbXk6RQ==",
          "name": "E"
        },
        {
          "id": "dGF4b25vbXk6Rg==",
          "name": "F"
        },
        {
          "id": "dGF4b25vbXk6Rw==",
          "name": "G"
        }
      ]
    }
  },
}

This is what we were expecting! 🥳 🎉

We can take the end cursor (for node "G") and do another request, expecting nodes "H" through "L"

And we see we get the expected results:

{
  "data": {
    "taxonomies": {
      "pageInfo": {
        "endCursor": "YXJyYXljb25uZWN0aW9uOkw=",
        "hasNextPage": true,
        "hasPreviousPage": false,
        "startCursor": "YXJyYXljb25uZWN0aW9uOkg="
      },
      "nodes": [
        {
          "id": "dGF4b25vbXk6SA==",
          "name": "H"
        },
        {
          "id": "dGF4b25vbXk6SQ==",
          "name": "I"
        },
        {
          "id": "dGF4b25vbXk6Sg==",
          "name": "J"
        },
        {
          "id": "dGF4b25vbXk6Sw==",
          "name": "K"
        },
        {
          "id": "dGF4b25vbXk6TA==",
          "name": "L"
        }
      ]
    }
  },
}

@qlty-cloud-legacy
Copy link
Copy Markdown

Code Climate has analyzed commit 49bd6d1 and detected 0 issues on this pull request.

View more on Code Climate.

@coveralls
Copy link
Copy Markdown

coveralls commented Jun 20, 2022

Coverage Status

Coverage increased (+0.1%) to 80.702% when pulling 49bd6d1 on jasonbahl:bugfix/taxonomy-pagination into 16b159f on wp-graphql:develop.

Copy link
Copy Markdown
Collaborator

@justlevine justlevine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good over here 🔥

(For posterity: this works by overloading AbstractConnectionResolver's methods to fix regressions in TaxonomyConnectionResolver only. Once the actual causes are addressed in the parent class - as these same pagination issues affect other connections - , much of this code can probably be removed.)

@jasonbahl jasonbahl merged commit 73d0a34 into wp-graphql:develop Jun 21, 2022
This was referenced Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v1.8.0 causing gatsby-source-wordpress error

3 participants