Bug 23391: Hide finished ILL requests

Currently, the main table of ILL requests will display all ILL
requests in the database, regardless of their status. For libraries
with active ILL this quickly leads to a lot of requests being displayed,
and the main page of the ILL module taking a long time to load. This
patch proposes to fix this by introducing the ILLHiddenRequestStatuses
syspref, which can take a pipe-separated list of ILL statuses that
will be hidden from view in the ILL module. This means that the
only way to find a hidden request will be through a report.

To test:
- Apply the patch and make sure the atomic database update is run
- Make sure you have a few ILL requests, with at least two different
  statuses
- Check that all requests are still displayed in the main table of
  ILL requests
- Add one of the statuses* you have in your database to the
  ILLHiddenRequestStatuses syspref, reload the ILL module frontpage
  and verify that requests with the given status are not displayed
- Change the syspref to another status and verify requests with
  that status are now hidden
- Change the syspref to hold both statuses, separated by the pipe
  symbol (e.g.: A|B). Verify that no requests with the given
  statuses are now displayed
- Run the ILL REST API tests, e.g.:
  $ sudo koha-shell -c "prove t/db_dependent/api/v1/illrequests.t" kohadev

* = The ILLHiddenRequestStatuses syspref should hold status codes, like
"REQ" and "NEW", not their human readable counterparts.

Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

diff --git a/Koha/REST/V1/Illrequests.pm b/Koha/REST/V1/Illrequests.pm
index a032be4..2274c59 100644 (file)
--- a/Koha/REST/V1/Illrequests.pm
+++ b/Koha/REST/V1/Illrequests.pm
@@ -19,6 +19,7 @@ use Modern::Perl;
 
 use Mojo::Base 'Mojolicious::Controller';
 
+use C4::Context;
 use Koha::Illrequests;
 use Koha::Illrequestattributes;
 use Koha::Libraries;
@@ -54,9 +55,17 @@ sub list {
         delete $args->{embed};
     }
 
+    # Get the pipe-separated string of hidden ILL statuses
+    my $hidden_statuses_string = C4::Context->preference('ILLHiddenRequestStatuses');
+    # Turn into arrayref
+    my $hidden_statuses = [ split /\|/, $hidden_statuses_string ];
+
     # Get all requests
     # If necessary, only get those from a specified patron
     my @requests = Koha::Illrequests->search({
+        $hidden_statuses
+        ? ( status => { 'not in' => $hidden_statuses } )
+        : (),
         $args->{borrowernumber}
         ? ( borrowernumber => $args->{borrowernumber} )
         : ()

diff --git a/installer/data/mysql/atomicupdate/bug23391-hide-finished-ill-requests.perl b/installer/data/mysql/atomicupdate/bug23391-hide-finished-ill-requests.perl
new file mode 100644 (file)
index 0000000..2a7927d
--- /dev/null
+++ b/installer/data/mysql/atomicupdate/bug23391-hide-finished-ill-requests.perl
@@ -0,0 +1,10 @@
+$DBversion = 'XXX';
+if( CheckVersion( $DBversion ) ) {
+    $dbh->do( q{
+            INSERT IGNORE INTO systempreferences (variable,value,options,explanation,type)
+            VALUES ('ILLHiddenRequestStatuses',NULL,NULL,'ILL statuses that are considered finished and should not be displayed in the ILL module','multiple')
+    });
+
+    SetVersion( $DBversion );
+    print "Upgrade to $DBversion done (Bug 23391 - Hide finished ILL requests)\n";
+}

diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql
index b0e3017..e73912d 100644 (file)
--- a/installer/data/mysql/sysprefs.sql
+++ b/installer/data/mysql/sysprefs.sql
@@ -234,6 +234,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `
 ('HTML5MediaYouTube',0,'Embed|Don\'t embed','YouTube links as videos','YesNo'),
 ('IdRef','0','','Disable/enable the IdRef webservice from the OPAC detail page.','YesNo'),
 ('IllCheckAvailability', 0, '', 'If ON, during the ILL request process third party sources will be checked for current availability', 'YesNo'),
+('ILLHiddenRequestStatuses', NULL, NULL, 'ILL statuses that are considered finished and should not be displayed in the ILL module', 'multiple'),
 ('IllLog', 0, '', 'If ON, log information about ILL requests', 'YesNo'),
 ('ILLModule','0','If ON, enables the interlibrary loans module.','','YesNo'),
 ('ILLModuleCopyrightClearance','','70|10','Enter text to enable the copyright clearance stage of request creation. Text will be displayed','Textarea'),

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
index f2379e7..b89fd8f 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
@@ -894,7 +894,12 @@ Circulation:
                   yes: Check
                   no: Don't check
             - external sources for availability during the request process
-    Fines policy:
+        -
+            - "ILL statuses that are considered finished and should not be displayed in the ILL module: "
+            - pref: ILLHiddenRequestStatuses
+              class: multi
+            - (separated with |). If left empty, all ILL requests will be displayed.
+    Fines Policy:
         -
             - pref: finesCalendar
               type: choice

diff --git a/t/db_dependent/api/v1/illrequests.t b/t/db_dependent/api/v1/illrequests.t
index 3ac94fd..269cb4d 100644 (file)
--- a/t/db_dependent/api/v1/illrequests.t
+++ b/t/db_dependent/api/v1/illrequests.t
@@ -40,7 +40,7 @@ my $t              = Test::Mojo->new('Koha::REST::V1');
 
 subtest 'list() tests' => sub {
 
-    plan tests => 24;
+    plan tests => 30;
 
     # Mock ILLBackend (as object)
     my $backend = Test::MockObject->new;
@@ -91,7 +91,8 @@ subtest 'list() tests' => sub {
             value => {
                 backend        => 'Mock',
                 branchcode     => $library->branchcode,
-                borrowernumber => $patron_1->borrowernumber
+                borrowernumber => $patron_1->borrowernumber,
+                status         => 'STATUS1',
             }
         }
     );
@@ -129,7 +130,8 @@ subtest 'list() tests' => sub {
             value => {
                 backend        => 'Mock',
                 branchcode     => $library->branchcode,
-                borrowernumber => $patron_2->borrowernumber
+                borrowernumber => $patron_2->borrowernumber,
+                status         => 'STATUS2',
             }
         }
     );
@@ -162,6 +164,21 @@ subtest 'list() tests' => sub {
     $tx->req->env( { REMOTE_ADDR => $remote_address } );
     $t->request_ok($tx)->status_is(200)->json_is( [ $response2 ] );
 
+    # Test the ILLHiddenRequestStatuses syspref
+    t::lib::Mocks::mock_preference( 'ILLHiddenRequestStatuses', 'STATUS1' );
+    $tx = $t->ua->build_tx( GET => '/api/v1/illrequests' );
+    $tx->req->cookies( { name => 'CGISESSID', value => $session_id } );
+    $tx->req->env( { REMOTE_ADDR => $remote_address } );
+    $t->request_ok($tx)->status_is(200)
+      ->json_is( [ $req2_formatted ] );
+
+    t::lib::Mocks::mock_preference( 'ILLHiddenRequestStatuses', 'STATUS2' );
+    $tx = $t->ua->build_tx( GET => '/api/v1/illrequests' );
+    $tx->req->cookies( { name => 'CGISESSID', value => $session_id } );
+    $tx->req->env( { REMOTE_ADDR => $remote_address } );
+    $t->request_ok($tx)->status_is(200)
+      ->json_is( [ $req_formatted ] );
+
     $schema->storage->txn_rollback;
 };