Bug 25570 - Listing requests should be paginated by default
Summary: Listing requests should be paginated by default
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 19410 24229
Blocks: 26143
  Show dependency treegraph
 
Reported: 2020-05-21 14:32 UTC by Tomás Cohen Arazi
Modified: 2021-12-13 21:09 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.02, 19.11.08, 19.05.14


Attachments
Bug 25570: Regression tests (2.19 KB, patch)
2020-05-21 21:32 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25570: Paginate results by default in objects->search (1.28 KB, patch)
2020-05-21 21:32 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25570: (follow-up) Fix tests (1.97 KB, patch)
2020-05-21 21:32 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25570: Regression tests (2.26 KB, patch)
2020-05-22 07:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 25570: Paginate results by default in objects->search (1.35 KB, patch)
2020-05-22 07:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 25570: (follow-up) Fix tests (2.04 KB, patch)
2020-05-22 07:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 25570: Regression tests (2.32 KB, patch)
2020-06-19 11:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25570: Paginate results by default in objects->search (1.42 KB, patch)
2020-06-19 11:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25570: (follow-up) Fix tests (2.11 KB, patch)
2020-06-19 11:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25570: Allow "all", -1 (955 bytes, patch)
2020-06-25 08:10 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 2020-05-21 14:32:48 UTC
They were supposed to be paginated by default, but we lost that (or maybe just the idea) at some point.

If pagination information is not passed, it should default to page 1, and page size to  RESTdefaultPageSize.
Comment 1 Tomás Cohen Arazi 2020-05-21 21:32:24 UTC
Created attachment 105229 [details] [review]
Bug 25570: Regression tests

This tests verify that the default behaviour is to paginate the results.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t
=> FAIL: $c->objects->search doesn't paginate results by default
Comment 2 Tomás Cohen Arazi 2020-05-21 21:32:29 UTC
Created attachment 105230 [details] [review]
Bug 25570: Paginate results by default in objects->search

This patch makes the helper paginate the resultsif no pagination
parameters are passed.

Page number defaults to 1, and the page size to the RESTdefaultPageSize
syspref value.

To test:
1. Apply the regression tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t
=> FAIL: Pagination is not enforced by default
3. Apply this patch
4. Repeat 2
=> SUCCESS: Results are paginated
5. Sign off :-D
Comment 3 Tomás Cohen Arazi 2020-05-21 21:32:34 UTC
Created attachment 105231 [details] [review]
Bug 25570: (follow-up) Fix tests

This patch fixes the libraries tests, that expected the old behaviour

To test:
1. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/libraries.t
=> FAIL: Tests fail
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D
Comment 4 Jonathan Druart 2020-05-22 07:59:20 UTC
Created attachment 105248 [details] [review]
Bug 25570: Regression tests

This tests verify that the default behaviour is to paginate the results.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t
=> FAIL: $c->objects->search doesn't paginate results by default

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 5 Jonathan Druart 2020-05-22 07:59:23 UTC
Created attachment 105249 [details] [review]
Bug 25570: Paginate results by default in objects->search

This patch makes the helper paginate the resultsif no pagination
parameters are passed.

Page number defaults to 1, and the page size to the RESTdefaultPageSize
syspref value.

To test:
1. Apply the regression tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t
=> FAIL: Pagination is not enforced by default
3. Apply this patch
4. Repeat 2
=> SUCCESS: Results are paginated
5. Sign off :-D

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 6 Jonathan Druart 2020-05-22 07:59:26 UTC
Created attachment 105250 [details] [review]
Bug 25570: (follow-up) Fix tests

This patch fixes the libraries tests, that expected the old behaviour

To test:
1. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/libraries.t
=> FAIL: Tests fail
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 7 Martin Renvoize 2020-05-22 08:30:17 UTC
I think we should target 20.05.01 for this one.
Comment 8 Martin Renvoize 2020-06-19 11:45:56 UTC
Created attachment 106070 [details] [review]
Bug 25570: Regression tests

This tests verify that the default behaviour is to paginate the results.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t
=> FAIL: $c->objects->search doesn't paginate results by default

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize 2020-06-19 11:45:59 UTC
Created attachment 106071 [details] [review]
Bug 25570: Paginate results by default in objects->search

This patch makes the helper paginate the resultsif no pagination
parameters are passed.

Page number defaults to 1, and the page size to the RESTdefaultPageSize
syspref value.

To test:
1. Apply the regression tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t
=> FAIL: Pagination is not enforced by default
3. Apply this patch
4. Repeat 2
=> SUCCESS: Results are paginated
5. Sign off :-D

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 10 Martin Renvoize 2020-06-19 11:46:02 UTC
Created attachment 106072 [details] [review]
Bug 25570: (follow-up) Fix tests

This patch fixes the libraries tests, that expected the old behaviour

To test:
1. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/libraries.t
=> FAIL: Tests fail
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2020-06-19 11:46:26 UTC
Works as expected, tests pass, qa script passes..

Passing QA
Comment 12 Jonathan Druart 2020-06-24 13:32:52 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 13 Jonathan Druart 2020-06-25 08:09:35 UTC
Tomas, if "all" is selected we get an error:

[2020-06-25 07:58:51.08130] [2024] [debug] Routing to controller "Koha::REST::V1::Libraries" and action "list"                                                                                                     No method pager found for Koha::Libraries Fewer than one entry per page! at /usr/share/perl5/DBIx/Class/ResultSet.pm line 2542.                                                                                    
 at /kohadevbox/koha/Koha/REST/Plugin/Objects.pm line 127.                                                                                                                                                         [2020-06-25 07:58:51.10649] [2024] [debug] 500 Internal Server Error (0.053242s, 18.782/s)    

Caught by Jenkins on Master_D9 run #1375, t/db_dependent/selenium/administration_tasks.t
Comment 14 Jonathan Druart 2020-06-25 08:10:17 UTC
Created attachment 106277 [details] [review]
Bug 25570: Allow "all", -1
Comment 15 Jonathan Druart 2020-06-25 08:14:38 UTC
Is this patch correct? It fixes the problem.

I did not manage to find a good place for the tests. Should it be in t/Koha/REST/Plugin/Pagination.t, t/db_dependent/Koha/REST/Plugin/Objects.t or both?
Comment 16 Lucas Gass 2020-07-10 15:48:06 UTC
backported to 20.05.x for 20.05.02
Comment 17 Aleisha Amohia 2020-07-12 22:49:31 UTC
backported to 19.11.x for 19.11.08
Comment 18 Victor Grousset/tuxayo 2020-07-23 22:13:53 UTC
Backported to 19.05.x branch for 19.05.14
Comment 19 Tomás Cohen Arazi 2021-01-08 13:43:36 UTC
(In reply to Jonathan Druart from comment #13)
> Tomas, if "all" is selected we get an error:
> 
> [2020-06-25 07:58:51.08130] [2024] [debug] Routing to controller
> "Koha::REST::V1::Libraries" and action "list"                               
> No method pager found for Koha::Libraries Fewer than one entry per page! at
> /usr/share/perl5/DBIx/Class/ResultSet.pm line 2542.                         
> 
>  at /kohadevbox/koha/Koha/REST/Plugin/Objects.pm line 127.                  
> [2020-06-25 07:58:51.10649] [2024] [debug] 500 Internal Server Error
> (0.053242s, 18.782/s)    
> 
> Caught by Jenkins on Master_D9 run #1375,
> t/db_dependent/selenium/administration_tasks.t

This was caught in the wild, and I solved it on bug 26143, before noticing this comment. Sorry dude.