Skip to content

[WIP] [PoC] Fix hasPreviouPage & hasNextPage in GraphQL pageInfo

Mehmet Emin INAC requested to merge fix_page_info_on_graphql_api into master

What does this MR do?

The values hasPreviousPage and hasNextPage are sometimes not correctly set on our GraphQL API. Problem with the hasPreviousPage attribute happens when the client paginates forward and the problem with hasNextPage shows up when the client paginates backward.

This is a PoC MR to collect feedback from maintainers. With this change, we can also eliminate the extra count query fired by graphql-ruby gem to check if there is a next/previous page.

Since this is just a proof of concept, I didn't want to spend too much time on code quality and covering all the cases and focused only on setting the hasPreviousPage attribute correctly when the client paginates forward.

How does this solve the problem?

Instead of querying the requested number of items from database, we can query 2 more entries; the entry before the lower bound and the entry after the upper bound. And based on returned items there are 3 cases;

  1. We got all the items, which means there is a previous page and next page
  2. We got 1 item less and the missing item is what we had in the cursor which means there is no previous page anymore
  3. We got 1 item less and the missing item is not what we had in the cursor which means where is no next page
  4. We got 2 items less which means there is neither previous page nor next page

Here is a diagram to visualize the logic; Blank_Diagram__1_

/related to #208301 (closed), #216498 (closed).

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team

Merge request reports