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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm
index f38365c..049ddcf 100644 (file)
--- 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 290abad..346f23e 100755 (executable)
--- 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 13dbc00..b1288ec 100644 (file)
--- 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;
+};