From 570c0405b4074555df0fc129ec860991e9680bac Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Wed, 19 Jun 2019 12:07:58 +0200 Subject: [PATCH] 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 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 Signed-off-by: Jonathan Druart Signed-off-by: Martin Renvoize (cherry picked from commit 2351693b61db83e0405157fd75951abee8df0116) Signed-off-by: Fridolin Somers --- C4/ILSDI/Services.pm | 17 +++++++++- opac/ilsdi.pl | 2 +- t/db_dependent/ILSDI_Services.t | 55 ++++++++++++++++++++++++++++++++- 3 files changed, 71 insertions(+), 3 deletions(-) diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm index f38365c732..049ddcf600 100644 --- a/C4/ILSDI/Services.pm +++ b/C4/ILSDI/Services.pm @@ -478,7 +478,21 @@ sub GetPatronInfo { # Issues management if ( $cgi->param('show_loans') && $cgi->param('show_loans') eq "1" ) { + my $per_page = $cgi->param('loans_per_page'); + my $page = $cgi->param('loans_page'); + my $pending_checkouts = $patron->pending_checkouts; + + if ($page || $per_page) { + $page ||= 1; + $per_page ||= 10; + $borrower->{total_loans} = $pending_checkouts->count(); + $pending_checkouts = $pending_checkouts->search(undef, { + rows => $per_page, + page => $page, + }); + } + my @checkouts; while ( my $c = $pending_checkouts->next ) { # FIXME We should only retrieve what is needed in the template @@ -489,7 +503,8 @@ sub GetPatronInfo { $borrower->{'loans'}->{'loan'} = \@checkouts; } - if ( $cgi->param('show_attributes') eq "1" ) { + my $show_attributes = $cgi->param('show_attributes'); + if ( $show_attributes && $show_attributes eq "1" ) { my $attrs = GetBorrowerAttributes( $borrowernumber, 1 ); $borrower->{'attributes'} = $attrs; } diff --git a/opac/ilsdi.pl b/opac/ilsdi.pl index 290abad758..346f23e4ed 100755 --- a/opac/ilsdi.pl +++ b/opac/ilsdi.pl @@ -104,7 +104,7 @@ my %optional = ( 'GetAuthorityRecords' => ['schema'], 'LookupPatron' => ['id_type'], 'AuthenticatePatron' => [], - 'GetPatronInfo' => [ 'show_contact', 'show_fines', 'show_holds', 'show_loans', 'show_attributes' ], + 'GetPatronInfo' => [ 'show_contact', 'show_fines', 'show_holds', 'show_loans', 'loans_per_page', 'loans_page', 'show_attributes' ], 'GetPatronStatus' => [], 'GetServices' => [], 'RenewLoan' => ['desired_due_date'], diff --git a/t/db_dependent/ILSDI_Services.t b/t/db_dependent/ILSDI_Services.t index 13dbc0089c..b1288ec469 100644 --- a/t/db_dependent/ILSDI_Services.t +++ b/t/db_dependent/ILSDI_Services.t @@ -19,12 +19,13 @@ use Modern::Perl; use CGI qw ( -utf8 ); -use Test::More tests => 8; +use Test::More tests => 9; use Test::MockModule; use t::lib::Mocks; use t::lib::TestBuilder; use C4::Items qw( ModItemTransfer ); +use C4::Circulation; use Koha::AuthUtils; @@ -631,3 +632,55 @@ subtest 'RenewHold' => sub { $schema->storage->txn_rollback; }; + +subtest 'GetPatronInfo paginated loans' => sub { + plan tests => 7; + + $schema->storage->txn_begin; + + my $library = $builder->build_object({ + class => 'Koha::Libraries', + }); + + my $item1 = $builder->build_sample_item({ library => $library->branchcode }); + my $item2 = $builder->build_sample_item({ library => $library->branchcode }); + my $item3 = $builder->build_sample_item({ library => $library->branchcode }); + my $patron = $builder->build_object({ + class => 'Koha::Patrons', + value => { + branchcode => $library->branchcode, + }, + }); + my $module = new Test::MockModule('C4::Context'); + $module->mock('userenv', sub { { branch => $library->branchcode } }); + my $date_due = DateTime->now->add(weeks => 2); + my $issue1 = C4::Circulation::AddIssue($patron->unblessed, $item1->barcode, $date_due); + my $date_due1 = Koha::DateUtils::dt_from_string( $issue1->date_due ); + my $issue2 = C4::Circulation::AddIssue($patron->unblessed, $item2->barcode, $date_due); + my $date_due2 = Koha::DateUtils::dt_from_string( $issue2->date_due ); + my $issue3 = C4::Circulation::AddIssue($patron->unblessed, $item3->barcode, $date_due); + my $date_due3 = Koha::DateUtils::dt_from_string( $issue3->date_due ); + + my $cgi = new CGI; + + $cgi->param( 'service', 'GetPatronInfo' ); + $cgi->param( 'patron_id', $patron->borrowernumber ); + $cgi->param( 'show_loans', '1' ); + $cgi->param( 'loans_per_page', '2' ); + $cgi->param( 'loans_page', '1' ); + my $reply = C4::ILSDI::Services::GetPatronInfo($cgi); + + is($reply->{total_loans}, 3, 'total_loans == 3'); + is(scalar @{ $reply->{loans}->{loan} }, 2, 'GetPatronInfo returned only 2 loans'); + is($reply->{loans}->{loan}->[0]->{itemnumber}, $item3->itemnumber); + is($reply->{loans}->{loan}->[1]->{itemnumber}, $item2->itemnumber); + + $cgi->param( 'loans_page', '2' ); + $reply = C4::ILSDI::Services::GetPatronInfo($cgi); + + is($reply->{total_loans}, 3, 'total_loans == 3'); + is(scalar @{ $reply->{loans}->{loan} }, 1, 'GetPatronInfo returned only 1 loan'); + is($reply->{loans}->{loan}->[0]->{itemnumber}, $item1->itemnumber); + + $schema->storage->txn_rollback; +}; -- 2.39.5