Skip to content

Total count and configurable default per_page#31

Merged
stas merged 5 commits intostas:masterfrom
jkorz:master
Dec 12, 2020
Merged

Total count and configurable default per_page#31
stas merged 5 commits intostas:masterfrom
jkorz:master

Conversation

@jkorz
Copy link
Contributor

@jkorz jkorz commented Nov 25, 2020

What is the current behavior?

Default per-page is 30 and set using a constant. Pagination meta does not include total number of records.

What is the new behavior?

Implementers may now configure the default per-page by defining a method. Total number of records is included in the pagination meta.

Checklist

Please make sure the following requirements are complete:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes /
    features)
  • All automated checks pass (CI/CD)

@stas
Copy link
Owner

stas commented Nov 26, 2020

@jkorz there's already a PR related to the totals (stalled, for which I'm ashamed 🙈), please take a look at #29

I like the idea of moving away from the constant, so I'd be happy to merge that part. But if I may suggest, I'd like us to keep the naming similar to what we have now, as in: JSONAPI_PAGE_SIZE to become jsonapi_page_size.

To move forward with this, I suggest we merge #29 and let you rebase your work to cover just the default page size setup.

How does that sound to you?

@jkorz
Copy link
Contributor Author

jkorz commented Nov 27, 2020

@stas sounds good to me

@jkorz
Copy link
Contributor Author

jkorz commented Dec 7, 2020

@stas I made the changes you requested.

@stas
Copy link
Owner

stas commented Dec 7, 2020

@jkorz this is good to go, let's just make sure the CI passes.

@stas stas merged commit 3a685a7 into stas:master Dec 12, 2020
@stas stas mentioned this pull request Jan 28, 2021
3 tasks
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.

2 participants