Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected Results While Retrieving Terms via Cursor-Based Pagination, Sorted by Term Name, When Multiple Terms Have Identical Names #2926

Closed
2 of 3 tasks
mohjak opened this issue Sep 7, 2023 · 4 comments · Fixed by #2930
Labels
Status: In Progress Type: Bug Something isn't working

Comments

@mohjak
Copy link

mohjak commented Sep 7, 2023

Description

Context:

I am encountering unexpected behavior when querying for terms with identical names using cursor-based pagination.

Data Setup:

Here are the terms that serve as children to the term with term_id: 39.

term_id name
62 Child Category 01
63 Child Category 02
64 Child Category 03
65 Child Category 04
66 Child Category 05
199 Child 1
200 Child 1
201 Child 1
202 Child 1

Initial GraphQL Query:

query terms {
  categories(first: 2, after: "", where: {
    parent: 39
    order: ASC
    orderby: NAME
  }) {
    pageInfo {
      endCursor
    }
    nodes {
      name
      databaseId
      slug      
    }
  }
}

Initial SQL Query:

SELECT t.term_id
FROM wp_terms AS t
INNER JOIN wp_term_taxonomy AS tt ON t.term_id = tt.term_id
WHERE tt.taxonomy IN ('category') AND tt.parent = '39'
ORDER BY t.name ASC
LIMIT 0, 3

Expected and Actual Outcome:

The executed GraphQL query returns the following JSON output, which is in line with expectations:

{
  "data": {
    "categories": {
      "pageInfo": {
        "endCursor": "YXJyYXljb25uZWN0aW9uOjIwMQ=="
      },
      "nodes": [
        {
          "name": "Child 1",
          "databaseId": 200,
          "slug": "child-1-2"
        },
        {
          "name": "Child 1",
          "databaseId": 201,
          "slug": "child-1-3"
        }
      ]
    }
  }
}

Follow-up GraphQL Query:

I then executed another query, using the endCursor obtained from the first query:

query terms {
  categories(first: 2, after: "YXJyYXljb25uZWN0aW9uOjIwMQ==", where: {
    parent: 39
    order: ASC
    orderby: NAME
  }) {
    pageInfo {
      endCursor
    }
    nodes {
      name
      databaseId
      slug      
    }
  }
}

Follow-up SQL Query:

SELECT t.term_id
FROM wp_terms AS t
INNER JOIN wp_term_taxonomy AS tt ON t.term_id = tt.term_id
WHERE tt.taxonomy IN ('category') 
  AND tt.parent = '39'
  AND t.name > "Child 1"
ORDER BY t.name ASC
LIMIT 0, 3

Unexpected Outcome:

Contrary to expectations, the following data was returned:

{
  "data": {
    "categories": {
      "pageInfo": {
        "endCursor": "YXJyYXljb25uZWN0aW9uOjYz"
      },
      "nodes": [
        {
          "name": "Child Category 01",
          "databaseId": 62,
          "slug": "child-category-01"
        },
        {
          "name": "Child Category 02",
          "databaseId": 63,
          "slug": "child-category-02"
        }
      ]
    }
  }
}

I expected to see term_id: 201, term_id: 202 in the next cursor pagination as I presumed that if the names are identical, the pagination should consider using the next term_id.

Code Reference:

Upon inspecting the code here, it seems that the term_id is only used for ordering if there are no other specified order criteria.

Suggested Fix:

I propose that term_id should act as a secondary sorting parameter when the names are identical, ensuring consistent cursor-based pagination.

Steps to reproduce

You can replicate this issue at Test Terms Cursor with the following credentials:

  • Basic Auth:
    • Username: pattern
    • Password: unusual
  • Admin:
    • Username: admin
    • Password: password

Additional context

No response

WPGraphQL Version

1.16.0

WordPress Version

6.3.1

PHP Version

8.1.9

Additional enviornment details

No response

Please confirm that you have searched existing issues in the repo.

  • Yes

Please confirm that you have disabled ALL plugins except for WPGraphQL.

  • Yes
  • My issue is with compatibility with a specific WordPress plugin, and I have listed all my installed plugins (and version info) above.
@mohjak mohjak changed the title Unexpected Results When Getting Terms Using Identical Term Names Unexpected Results While Retrieving Terms via Cursor-Based Pagination, Sorted by Term Name, When Multiple Terms Have Identical Names Sep 7, 2023
@jasonbahl jasonbahl added the Needs: Reproduction This issue needs to be reproduced independently. label Sep 12, 2023
@mohjak
Copy link
Author

mohjak commented Sep 13, 2023

@jasonbahl, I have re-enabled the link for reproduction of the issue. It is a localwp wp that connects online. This needs my machine to be up. Could you please that you can connect to it from your end?
I have prepared a code that resolves this issue, but When I run all unit tests after applying the change there are two tests failed. my change requires to stop the following block of code:
https://github.com/wp-graphql/wp-graphql/blob/9b4384e3fe0e5f9ac81279deff0cdb37039c88fa/src/Data/Cursor/TermObjectCursor.php#L106C1-L115C1
I can't find a solution for it till now.
Maybe I should create the pull request to explain more clearly about the change, what do you think?

@jasonbahl
Copy link
Collaborator

@mohjak I was able to login to your environment.

I think you might have some debug code active though, as I'm seeing the following response when executing a query:

CleanShot 2023-09-13 at 09 32 29

@jasonbahl
Copy link
Collaborator

jasonbahl commented Sep 13, 2023

Maybe I should create the pull request to explain more clearly about the change, what do you think?

@mohjak yes, open a PR and we can go from there! 🙏🏻

By the way, thanks so much for the detailed issue, steps to reproduce and the effort put into resolving it. Much appreciated! 🙌🏻

@kidunot89 recently worked on this PR (#2882) fixing similar issues with pagination for queries for posts, so we can likely take some of the work he did for that and apply it to Term cursors as well!

@mohjak
Copy link
Author

mohjak commented Sep 14, 2023

@jasonbahl you are right I am getting the same result with public users, I was executing queries with a logged in user, since I am debugging and review SQL queries. I didn't figure this out. Nothing special, I've just created an empty wp with wp-graphql and the same result appears when I view the website from a public user. I think the problem is related to localwp online service because the issue is not appearing when I am viewing from the localhost url. Anyways, please if you want to reproduce the issue execute queries for a logged in user.

mohjak pushed a commit to mohjak/wp-graphql that referenced this issue Sep 14, 2023
wp-graphql#2926

Terms cursor now handles identical names accurately with asc/desc and after/before.
@justlevine justlevine added Type: Bug Something isn't working Status: In Progress and removed Needs: Reproduction This issue needs to be reproduced independently. labels Sep 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: In Progress Type: Bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants