Bug 19278 - Add a configurable default page size for endpoints
Summary: Add a configurable default page size for endpoints
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact:
URL:
Keywords:
Depends on: 19369
Blocks: 19410
  Show dependency treegraph
 
Reported: 2017-09-08 14:19 UTC by Tomás Cohen Arazi
Modified: 2018-12-03 20:03 UTC (History)
6 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 19278: Add RESTdefaultPageSize syspref (3.11 KB, patch)
2017-09-08 15:32 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19278: Unit tests (3.35 KB, patch)
2017-09-08 15:32 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19278: Make the pagination plugin use RESTdefaultPageSize (1.67 KB, patch)
2017-09-08 15:32 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19278: Mention RESTdefaultPageSize in POD (954 bytes, patch)
2017-09-22 22:17 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19278: Add RESTdefaultPageSize syspref (3.09 KB, patch)
2017-10-04 13:46 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19278: Unit tests (3.40 KB, patch)
2017-10-04 13:46 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19278: Make the pagination plugin use RESTdefaultPageSize (1.67 KB, patch)
2017-10-04 13:46 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19278: Mention RESTdefaultPageSize in POD (947 bytes, patch)
2017-10-04 13:47 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19278: Add RESTdefaultPageSize syspref (3.11 KB, patch)
2017-10-06 12:13 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19278: Unit tests (3.40 KB, patch)
2017-10-06 12:14 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19278: Make the pagination plugin use RESTdefaultPageSize (1.67 KB, patch)
2017-10-06 12:14 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19278: Mention RESTdefaultPageSize in POD (954 bytes, patch)
2017-10-06 12:14 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19278: Add RESTdefaultPageSize syspref (3.14 KB, patch)
2017-10-07 12:53 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 19278: Unit tests (3.44 KB, patch)
2017-10-07 12:53 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 19278: Make the pagination plugin use RESTdefaultPageSize (1.72 KB, patch)
2017-10-07 12:53 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 19278: Mention RESTdefaultPageSize in POD (998 bytes, patch)
2017-10-07 12:53 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 19278: Add RESTdefaultPageSize syspref (3.19 KB, patch)
2017-11-27 11:37 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19278: Unit tests (3.50 KB, patch)
2017-11-27 11:37 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19278: Make the pagination plugin use RESTdefaultPageSize (1.77 KB, patch)
2017-11-27 11:37 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19278: Mention RESTdefaultPageSize in POD (1.03 KB, patch)
2017-11-27 11:37 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19278: (follow-up) Make the pagination plugin use RESTdefaultPageSize (1.16 KB, patch)
2017-12-11 20:43 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2017-09-08 14:19:18 UTC
Bug 19196 introduces a handy pagination handling plugin that would benefit from a syspref where a default page size can be specified.
Comment 1 Tomás Cohen Arazi 2017-09-08 15:32:30 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi 2017-09-08 15:32:37 UTC Comment hidden (obsolete)
Comment 3 Tomás Cohen Arazi 2017-09-08 15:32:44 UTC Comment hidden (obsolete)
Comment 4 Tomás Cohen Arazi 2017-09-22 22:17:52 UTC Comment hidden (obsolete)
Comment 5 Alex Buckley 2017-10-04 02:52:31 UTC
Hi Tomas

Sorry but when applying your patch I got the following merge error, (note: I did apply patch 19196 beforehand):

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 19278: Add RESTdefaultPageSize syspref
Applying: Bug 19278: Unit tests
Using index info to reconstruct a base tree...
M	t/Koha/REST/Plugin/Pagination.t
Falling back to patching base and 3-way merge...
Auto-merging t/Koha/REST/Plugin/Pagination.t
Applying: Bug 19278: Make the pagination plugin use RESTdefaultPageSize
fatal: sha1 information is lacking or useless (Koha/REST/Plugin/Pagination.pm).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 19278: Make the pagination plugin use RESTdefaultPageSize
The copy of the patch that failed is found in:
   /home/vagrant/kohaclone/.git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-19278-Make-the-pagination-plugin-use-RESTdefau-NPNcVH.patch
Comment 6 Tomás Cohen Arazi 2017-10-04 13:46:45 UTC Comment hidden (obsolete)
Comment 7 Tomás Cohen Arazi 2017-10-04 13:46:51 UTC Comment hidden (obsolete)
Comment 8 Tomás Cohen Arazi 2017-10-04 13:46:57 UTC Comment hidden (obsolete)
Comment 9 Tomás Cohen Arazi 2017-10-04 13:47:05 UTC Comment hidden (obsolete)
Comment 10 Alex Buckley 2017-10-04 19:41:04 UTC
Hi Tomas

Can you please rebase this patch against master because when I applied the patches attached to this bug report (after applying the dependencies) I got the following merge error:

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 19278: Add RESTdefaultPageSize syspref
Applying: Bug 19278: Unit tests
fatal: sha1 information is lacking or useless (t/Koha/REST/Plugin/Pagination.t).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 19278: Unit tests
The copy of the patch that failed is found in:
   /home/vagrant/kohaclone/.git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-19278-Unit-tests-3gVPjM.patch
Comment 11 Tomás Cohen Arazi 2017-10-05 18:55:06 UTC
I changes the dependencies because I fixed it on a working branch with other patches.
Comment 12 Alex Buckley 2017-10-05 21:28:04 UTC
Hi Tomas

It is still failing when applying the patches (specifically it is failing when applying the Unit Tests patch). I've applied the  dependencies listed on the patch as of 5 minutes ago. Please let me know if I am missing something?

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 19278: Add RESTdefaultPageSize syspref
Applying: Bug 19278: Unit tests
fatal: sha1 information is lacking or useless (t/Koha/REST/Plugin/Pagination.t).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 19278: Unit tests
The copy of the patch that failed is found in:
   /home/vagrant/kohaclone/.git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-19278-Unit-tests-XOrn2r.patch
Comment 13 Tomás Cohen Arazi 2017-10-06 12:13:59 UTC
Created attachment 67700 [details] [review]
Bug 19278: Add RESTdefaultPageSize syspref
Comment 14 Tomás Cohen Arazi 2017-10-06 12:14:05 UTC
Created attachment 67701 [details] [review]
Bug 19278: Unit tests

This patch introduces tests for a behaviour change in
Koha::REST::Plugin::Pagination.

To test:
- Run:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ prove t/Koha/REST/Plugin/Pagination.t
=> FAIL: Tests should fail without the followup patch applied

Sponsored-by: Camden County
Sponsored-by: Bywater Solutions
Comment 15 Tomás Cohen Arazi 2017-10-06 12:14:11 UTC
Created attachment 67702 [details] [review]
Bug 19278: Make the pagination plugin use RESTdefaultPageSize

This bug introduces a new syspref RESTdefaultPageSize that is used on
the Koha::REST::Plugin::Pagination plugin to default to a valid value
for the page size if the param is not present on the query.

This patch also considers the situation in which no 'page' param is
passed, and no header should be added.

To test:
- Run:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ prove t/Koha/REST/Plugin/Pagination.t
=> FAIL: Tests fail because the plugin doesn't behave as the tests
expect.
- Apply this patch
- Run:
 k$ prove t/Koha/REST/Plugin/Pagination.t
=> SUCCESS: Tests pass!
- Sign off :-D

Sponsored-by: Camden County
Sponsored-by: ByWater Solutions
Comment 16 Tomás Cohen Arazi 2017-10-06 12:14:19 UTC
Created attachment 67703 [details] [review]
Bug 19278: Mention RESTdefaultPageSize in POD

This patch adds information about RESTdefaultPageSize usage
when the per_page parameter is absent on the query.
Comment 17 Tomás Cohen Arazi 2017-10-06 12:17:41 UTC
Alex, I'm not sure what's going on :-D My guess is that my Git version might be newer and smarter than yours (the merging algorithm). I've re-submitted just in case the commits are differently built.

$ git --version
git version 2.14.2

This are the deps:
https://bugs.koha-community.org/bugzilla3/showdependencygraph.cgi?id=19278

Thanks!
Comment 18 Lari Taskula 2017-10-07 12:53:12 UTC
Created attachment 67790 [details] [review]
Bug 19278: Add RESTdefaultPageSize syspref

Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Comment 19 Lari Taskula 2017-10-07 12:53:23 UTC
Created attachment 67791 [details] [review]
Bug 19278: Unit tests

This patch introduces tests for a behaviour change in
Koha::REST::Plugin::Pagination.

To test:
- Run:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ prove t/Koha/REST/Plugin/Pagination.t
=> FAIL: Tests should fail without the followup patch applied

Sponsored-by: Camden County
Sponsored-by: Bywater Solutions

Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Comment 20 Lari Taskula 2017-10-07 12:53:38 UTC
Created attachment 67792 [details] [review]
Bug 19278: Make the pagination plugin use RESTdefaultPageSize

This bug introduces a new syspref RESTdefaultPageSize that is used on
the Koha::REST::Plugin::Pagination plugin to default to a valid value
for the page size if the param is not present on the query.

This patch also considers the situation in which no 'page' param is
passed, and no header should be added.

To test:
- Run:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ prove t/Koha/REST/Plugin/Pagination.t
=> FAIL: Tests fail because the plugin doesn't behave as the tests
expect.
- Apply this patch
- Run:
 k$ prove t/Koha/REST/Plugin/Pagination.t
=> SUCCESS: Tests pass!
- Sign off :-D

Sponsored-by: Camden County
Sponsored-by: ByWater Solutions

Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Comment 21 Lari Taskula 2017-10-07 12:53:54 UTC
Created attachment 67793 [details] [review]
Bug 19278: Mention RESTdefaultPageSize in POD

This patch adds information about RESTdefaultPageSize usage
when the per_page parameter is absent on the query.

Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Comment 22 Kyle M Hall 2017-11-27 11:37:24 UTC
Created attachment 69365 [details] [review]
Bug 19278: Add RESTdefaultPageSize syspref

Signed-off-by: Lari Taskula <lari.taskula@jns.fi>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 23 Kyle M Hall 2017-11-27 11:37:31 UTC
Created attachment 69366 [details] [review]
Bug 19278: Unit tests

This patch introduces tests for a behaviour change in
Koha::REST::Plugin::Pagination.

To test:
- Run:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ prove t/Koha/REST/Plugin/Pagination.t
=> FAIL: Tests should fail without the followup patch applied

Sponsored-by: Camden County
Sponsored-by: Bywater Solutions

Signed-off-by: Lari Taskula <lari.taskula@jns.fi>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 24 Kyle M Hall 2017-11-27 11:37:34 UTC
Created attachment 69367 [details] [review]
Bug 19278: Make the pagination plugin use RESTdefaultPageSize

This bug introduces a new syspref RESTdefaultPageSize that is used on
the Koha::REST::Plugin::Pagination plugin to default to a valid value
for the page size if the param is not present on the query.

This patch also considers the situation in which no 'page' param is
passed, and no header should be added.

To test:
- Run:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ prove t/Koha/REST/Plugin/Pagination.t
=> FAIL: Tests fail because the plugin doesn't behave as the tests
expect.
- Apply this patch
- Run:
 k$ prove t/Koha/REST/Plugin/Pagination.t
=> SUCCESS: Tests pass!
- Sign off :-D

Sponsored-by: Camden County
Sponsored-by: ByWater Solutions

Signed-off-by: Lari Taskula <lari.taskula@jns.fi>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 25 Kyle M Hall 2017-11-27 11:37:37 UTC
Created attachment 69368 [details] [review]
Bug 19278: Mention RESTdefaultPageSize in POD

This patch adds information about RESTdefaultPageSize usage
when the per_page parameter is absent on the query.

Signed-off-by: Lari Taskula <lari.taskula@jns.fi>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 26 Jonathan Druart 2017-12-11 20:43:24 UTC
Created attachment 69704 [details] [review]
Bug 19278: (follow-up) Make the pagination plugin use RESTdefaultPageSize

Use 20 as default for RESTdefaultPageSize (just in case)

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 27 Jonathan Druart 2017-12-11 20:47:31 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 28 Nick Clemens 2017-12-20 11:41:45 UTC
Awesome work all! Enhancement, won't be backported for 17.11