Bug 23156 - Add pagination to checkouts in ILS-DI GetPatronInfo service
Summary: Add pagination to checkouts in ILS-DI GetPatronInfo service
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Web services (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Julian Maurice
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-06-19 10:23 UTC by Julian Maurice
Modified: 2020-11-30 21:45 UTC (History)
4 users (show)

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


Attachments
Bug 23156: Add pagination to checkouts in ILS-DI GetPatronInfo service (5.88 KB, patch)
2019-06-19 10:23 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 23156: Add pagination to checkouts in ILS-DI GetPatronInfo service (5.95 KB, patch)
2019-06-19 17:32 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 23156: Add pagination to checkouts in ILS-DI GetPatronInfo service (6.02 KB, patch)
2019-06-24 19:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23156: [18.11] Add pagination to checkouts in ILS-DI GetPatronInfo service (5.98 KB, patch)
2019-07-05 09:11 UTC, Arthur Suzuki
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2019-06-19 10:23:16 UTC

    
Comment 1 Julian Maurice 2019-06-19 10:23:48 UTC
Created attachment 90774 [details] [review]
Bug 23156: Add pagination to checkouts in ILS-DI GetPatronInfo service

When patrons have a lot of checkouts, GetPatronInfo can take a lot of
time. This patch introduces two new parameters to allow pagination of
this list of checkouts

Also, fix a warning in C4::ILSDI::Services::GetPatronInfo

Test plan:
1. Go to /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=X&show_loans=1
   where X is a borrowernumber of a patron who has several checkouts
   Verify that all checkouts are listed
2. Add '&loans_per_page=1&loans_page=1' to the URL. Verify that you have
   now only one checkout listed, and that there is a new element
   <total_loans> which contain the total number of checkouts
3. Increase the page number in the URL until you have seen all checkouts
4. prove t/db_dependent/ILSDI_Services.t
Comment 2 Mark Tompsett 2019-06-19 17:32:01 UTC
Created attachment 90812 [details] [review]
Bug 23156: Add pagination to checkouts in ILS-DI GetPatronInfo service

When patrons have a lot of checkouts, GetPatronInfo can take a lot of
time. This patch introduces two new parameters to allow pagination of
this list of checkouts

Also, fix a warning in C4::ILSDI::Services::GetPatronInfo

Test plan:
1. Go to /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=X&show_loans=1
   where X is a borrowernumber of a patron who has several checkouts
   Verify that all checkouts are listed
2. Add '&loans_per_page=1&loans_page=1' to the URL. Verify that you have
   now only one checkout listed, and that there is a new element
   <total_loans> which contain the total number of checkouts
3. Increase the page number in the URL until you have seen all checkouts
4. prove t/db_dependent/ILSDI_Services.t

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 3 Arthur Suzuki 2019-06-19 17:42:39 UTC
Yipee, super great!
I was looking for this since a couple months but unfortunately couldn't find the time nor the expertise to do it myself.
Thanks Julian for the patch!!!
Thanks Mark for signing off :)
Comment 4 Jonathan Druart 2019-06-24 17:20:16 UTC
+            $per_page ||= 10;

Sure about that? It's a change in the behavior. I would default to 'all'.
Comment 5 Julian Maurice 2019-06-24 19:32:45 UTC
(In reply to Jonathan Druart from comment #4)
> +            $per_page ||= 10;
> 
> Sure about that? It's a change in the behavior. I would default to 'all'.

$per_page defaults to 10 only if $page is defined.
If $page and $per_page are both undefined the behavior is to return all checkouts.
Comment 6 Jonathan Druart 2019-06-24 19:54:08 UTC
Indeed Julian!
Comment 7 Jonathan Druart 2019-06-24 19:54:36 UTC
Created attachment 90977 [details] [review]
Bug 23156: Add pagination to checkouts in ILS-DI GetPatronInfo service

When patrons have a lot of checkouts, GetPatronInfo can take a lot of
time. This patch introduces two new parameters to allow pagination of
this list of checkouts

Also, fix a warning in C4::ILSDI::Services::GetPatronInfo

Test plan:
1. Go to /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=X&show_loans=1
   where X is a borrowernumber of a patron who has several checkouts
   Verify that all checkouts are listed
2. Add '&loans_per_page=1&loans_page=1' to the URL. Verify that you have
   now only one checkout listed, and that there is a new element
   <total_loans> which contain the total number of checkouts
3. Increase the page number in the URL until you have seen all checkouts
4. prove t/db_dependent/ILSDI_Services.t

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Jonathan Druart 2019-06-24 19:54:59 UTC
Note that we usually use 20 for default values.
Comment 9 Julian Maurice 2019-06-24 20:16:36 UTC
Thanks for the QA :)

(In reply to Jonathan Druart from comment #8)
> Note that we usually use 20 for default values.
Feel free to change that. I picked '10' without really thinking about it.
Comment 10 Martin Renvoize 2019-06-25 16:20:28 UTC
Nice work!

Pushed to master for 19.11.00
Comment 11 Arthur Suzuki 2019-07-05 09:11:41 UTC
Created attachment 91329 [details] [review]
Bug 23156: [18.11] Add pagination to checkouts in ILS-DI GetPatronInfo service

When patrons have a lot of checkouts, GetPatronInfo can take a lot of
time. This patch introduces two new parameters to allow pagination of
this list of checkouts

Also, fix a warning in C4::ILSDI::Services::GetPatronInfo

Test plan:
1. Go to /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=X&show_loans=1
   where X is a borrowernumber of a patron who has several checkouts
   Verify that all checkouts are listed
2. Add '&loans_per_page=1&loans_page=1' to the URL. Verify that you have
   now only one checkout listed, and that there is a new element
   <total_loans> which contain the total number of checkouts
3. Increase the page number in the URL until you have seen all checkouts
4. prove t/db_dependent/ILSDI_Services.t

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 12 Arthur Suzuki 2019-07-05 09:15:01 UTC
I've backported to 18.11, patch seems to apply fines :)
Can this be pushed to 18.11 as well?
Comment 13 Fridolin Somers 2019-07-25 11:34:48 UTC
Small and smart enhancement.

Pushed to 19.05.x for 19.05.03
Comment 14 Fridolin Somers 2019-07-25 11:36:00 UTC
@Lucas :
I think you can try cherry-picking to 18.11.x, and look at [18.11.x] patch if conflict.
Comment 15 Lucas Gass 2019-07-25 21:18:45 UTC
Thanks Frido,

I backported Arthur's 18.11 patch to 18.11.x for 18.11.09
Comment 16 Arthur Suzuki 2019-07-26 07:27:30 UTC
Thanks Lucas :)
So it will also be released in 18.11.09 then? great :)
(shouldn't you provide your patch and obsolete mine?)
Comment 17 Lucas Gass 2019-07-26 17:38:36 UTC
Arthur,

I backported your patch. 

http://git.koha-community.org/gitweb/?p=koha.git;a=commit;h=f8b7a5f5e144e9539c1c7e7fd59904257b0ed33f

It will be in 18.11.09
Comment 18 Arthur Suzuki 2019-09-03 12:44:08 UTC
Awesome!
Nice work everyone :)