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

Pagination - page numbers less than 1 all return page 1 results #2471

Open
jdjoshuadavison opened this issue Nov 15, 2023 · 2 comments
Open
Labels
docs Concerns the documentation

Comments

@jdjoshuadavison
Copy link

Describe the bug

When using pagination, requesting page 0 (or any negative integer page number e.g. page -31) returns page 1.

Expected behavior/Solution

When requesting a page number less than 1, you should not receive page 1.

Option 1: return requested page with empty data array.

myQueryName(first: 10, page: 0) {
    data {
      field1
      field2
      field3
    }
    paginatorInfo {
      count
      currentPage
      lastPage
      firstItem
      lastItem
      perPage
      hasMorePages
      total
    }
  }
{
  "data": {
    "myQueryName": {
      "data": [],
      "paginatorInfo": {
        "count": 0,
        "currentPage": 0,
        "lastPage": 83,
        "firstItem": null,
        "lastItem": null,
        "perPage": 10,
        "hasMorePages": true,
        "total": 83
      }
    }
  }
}

Option 2: return an error

{
  "errors": [
    {
      "message": "Argument 'page' must be greater than 0",
      "locations": [
        {
          ...
        }
      ]
    }
  ]
}

Steps to reproduce

  1. Query a paginated endpoint with argument page: 0 (or any negative integer) ensuring to request paginatorInfo currentPage
  2. Observe that the response says the currentPage is 1 and the data is the first page of results

Output/Logs

N/A

Lighthouse Version

6.14

@spawnia
Copy link
Collaborator

spawnia commented Nov 15, 2023

Given we just wrap Laravel pagination, this seems to be their default behaviour. I would not consider it a bug, and changing it would be a breaking change. I would welcome an update to the documentation though, since it may not be obvious.

@spawnia spawnia added the docs Concerns the documentation label Nov 15, 2023
@jdjoshuadavison
Copy link
Author

jdjoshuadavison commented Nov 15, 2023

Understood, thanks for your quick response. This is not a critical issue, just a minor annoyance. However, I have just raised a similar issue, which we believe is much more significant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Concerns the documentation
Projects
None yet
Development

No branches or pull requests

2 participants