Attachment 107186 Details for Bug 12556 – Bug 12556: Add new "in processing" state to holds

[patch] Bug 12556: Add new "in processing" state to holds

Bug-12556-Add-new-in-processing-state-to-holds.patch (text/plain), 17.52 KB, created by Joonas Kylmälä on 2020-07-22 15:50:29 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Joonas Kylmälä

Created: 2020-07-22 15:50:29 UTC

Size: 17.52 KB

patch

obsolete

>From 7a198a57ea76d37e01957fec0148f62811aa804a Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Joonas=20Kylm=C3=A4l=C3=A4?= <joonas.kylmala@helsinki.fi>
>Date: Wed, 22 Jul 2020 18:45:36 +0300
>Subject: [PATCH] Bug 12556: Add new "in processing" state to holds
>
>This adds new syspref, HoldsNeedProcessingSIP, which controls whether
>a hold that is related to item will be filled automatically or not. If
>the user has enabled the syspref then instead of fulfilling the hold
>automatically the hold will go to "in processing" state.
>
>To test:
> 1. Checkout a book to patron A
> 2. Place a bib level hold to the book for B
> 3. Patron A returns the book via SIP, to simulate this use:
>        ./misc/sip_cli_emulator.pl -su koha -sp koha -l CPL -a 127.0.0.1 -p 6001 --item <ItemBarcode> -m checkin
> 4. Notice that no notification is generated for Patron B about hold
>    and that the hold status in intranet and opac is "In Processing".
> 5. Notice that patron A (or other patrons) cannot checkout a book
>    that is in processing, because it is considered to be attached to
>    the holdee (similarly to the waiting state):
>        ./misc/sip_cli_emulator.pl -su koha -sp koha -l CPL -a 127.0.0.1 -p 6001 --patron <PatronABarcode> --item <ItemBarcode> -m checkout
>---
> C4/Circulation.pm                                  |  3 ++
> C4/Reserves.pm                                     | 25 ++++++++++------
> C4/RotatingCollections.pm                          |  2 +-
> Koha/Hold.pm                                       | 35 ++++++++++++++++++++--
> installer/data/mysql/atomicupdate/bug_12556.perl   |  8 +++++
> installer/data/mysql/sysprefs.sql                  |  1 +
> .../intranet-tmpl/prog/en/includes/holds_table.inc | 10 +++----
> .../en/modules/admin/preferences/circulation.pref  |  6 ++++
> .../prog/en/modules/members/holdshistory.tt        |  2 ++
> .../bootstrap/en/includes/holds-table.inc          |  4 ++-
> reserve/request.pl                                 |  1 +
> t/db_dependent/Hold.t                              | 20 +++++++++++--
> 12 files changed, 97 insertions(+), 20 deletions(-)
> create mode 100644 installer/data/mysql/atomicupdate/bug_12556.perl
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index 2fe1dcdede..98df1660df 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -1051,6 +1051,9 @@ sub CanBookBeIssued {
>                     $needsconfirmation{'resreservedate'} = $res->{reservedate};
>                     $needsconfirmation{'reserve_id'} = $res->{reserve_id};
>                 }
>+                elsif ( $restype eq "Processing" && C4::Context->interface eq 'sip' ) {
>+                    $issuingimpossible{PROCESSING} = 1;
>+                }
>             }
>         }
>     }
>diff --git a/C4/Reserves.pm b/C4/Reserves.pm
>index 197495a0ca..67a8777870 100644
>--- a/C4/Reserves.pm
>+++ b/C4/Reserves.pm
>@@ -709,6 +709,7 @@ sub GetReserveStatus {
> 
>     if(defined $found) {
>         return 'Waiting'  if $found eq 'W' and $priority == 0;
>+        return 'Processing'  if $found eq 'P';
>         return 'Finished' if $found eq 'F';
>     }
> 
>@@ -823,7 +824,9 @@ sub CheckReserves {
>             if ( $res->{'itemnumber'} && $res->{'itemnumber'} == $itemnumber && $res->{'priority'} == 0) {
>                 if ($res->{'found'} eq 'W') {
>                     return ( "Waiting", $res, \@reserves ); # Found it, it is waiting
>-                } else {
>+                } elsif ($res->{'found'} eq 'P') {
>+                    return ( "Processing", $res, \@reserves ); # Found determinated hold, e. g. the tranferred one
>+                 } else {
>                     return ( "Reserved", $res, \@reserves ); # Found determinated hold, e. g. the tranferred one
>                 }
>             } else {
>@@ -1151,7 +1154,13 @@ sub ModReserveAffect {
> 
>     $hold->itemnumber($itemnumber);
> 
>-    if( !$transferToDo ){
>+    if ($transferToDo) {
>+        $hold->set_transfer();
>+    } elsif (C4::Context->preference('HoldsNeedProcessingSIP')
>+             && C4::Context->interface eq 'sip'
>+             && !$already_on_shelf) {
>+        $hold->set_processing();
>+    } else {
>         $hold->set_waiting();
>         _koha_notify_reserve( $hold->reserve_id ) unless $already_on_shelf;
>         my $transfers = Koha::Item::Transfers->search({
>@@ -1161,9 +1170,7 @@ sub ModReserveAffect {
>         while( my $transfer = $transfers->next ){
>             $transfer->datearrived( dt_from_string() )->store;
>         };
>-     } else {
>-        $hold->set_transfer();
>-     }
>+    }
> 
> 
>     _FixPriority( { biblionumber => $biblionumber } );
>@@ -1536,7 +1543,7 @@ sub _FixPriority {
>             UPDATE reserves
>             SET    priority = 0
>             WHERE reserve_id = ?
>-            AND found IN ('W', 'T')
>+            AND found IN ('W', 'T', 'P')
>         ";
>         my $sth = $dbh->prepare($query);
>         $sth->execute( $reserve_id );
>@@ -1548,7 +1555,7 @@ sub _FixPriority {
>         SELECT reserve_id, borrowernumber, reservedate
>         FROM   reserves
>         WHERE  biblionumber   = ?
>-          AND  ((found <> 'W' AND found <> 'T') OR found IS NULL)
>+          AND  ((found <> 'W' AND found <> 'T' AND found <> 'P') OR found IS NULL)
>         ORDER BY priority ASC
>     ";
>     my $sth = $dbh->prepare($query);
>@@ -1960,7 +1967,7 @@ sub MergeHolds {
> "UPDATE reserves SET priority = ? WHERE biblionumber = ? AND borrowernumber = ?
>         AND reservedate = ? AND (itemnumber = ? or itemnumber is NULL) "
>         );
>-        $sth->execute( $to_biblio, 'W', 'T' );
>+        $sth->execute( $to_biblio, 'W', 'T', 'P' );
>         my $priority = 1;
>         while ( my $reserve = $sth->fetchrow_hashref() ) {
>             $upd_sth->execute(
>@@ -2142,7 +2149,7 @@ sub CalculatePriority {
>         AND   priority > 0
>         AND   (found IS NULL OR found = '')
>     };
>-    #skip found==W or found==T (waiting or transit holds)
>+    #skip found==W or found==T or found==P (waiting, transit or processing holds)
>     if( $resdate ) {
>         $sql.= ' AND ( reservedate <= ? )';
>     }
>diff --git a/C4/RotatingCollections.pm b/C4/RotatingCollections.pm
>index a4477fa9e5..795b7a12b7 100644
>--- a/C4/RotatingCollections.pm
>+++ b/C4/RotatingCollections.pm
>@@ -446,7 +446,7 @@ sub TransferCollection {
>     while ( my $item = $sth->fetchrow_hashref ) {
>         my ($status) = CheckReserves( $item->{itemnumber} );
>         my @transfers = C4::Circulation::GetTransfers( $item->{itemnumber} );
>-        C4::Circulation::transferbook( $colBranchcode, $item->{barcode}, 1, 'RotatingCollection' ) unless ( $status eq 'Waiting' || @transfers );
>+        C4::Circulation::transferbook( $colBranchcode, $item->{barcode}, 1, 'RotatingCollection' ) unless ( $status eq 'Waiting' || $status eq 'Processing' || @transfers );
>     }
> 
>     return 1;
>diff --git a/Koha/Hold.pm b/Koha/Hold.pm
>index 4c57b7cb2f..cfe25baa6a 100644
>--- a/Koha/Hold.pm
>+++ b/Koha/Hold.pm
>@@ -94,6 +94,9 @@ sub suspend_hold {
>         elsif ( $self->is_in_transit ) {
>             Koha::Exceptions::Hold::CannotSuspendFound->throw( status => 'T' );
>         }
>+        elsif ( $self->is_in_processing ) {
>+            Koha::Exceptions::Hold::CannotSuspendFound->throw( status => 'P' );
>+        }
>         else {
>             Koha::Exceptions::Hold::CannotSuspendFound->throw(
>                       'Unhandled data exception on found hold (id='
>@@ -215,9 +218,23 @@ sub set_waiting {
>     return $self;
> }
> 
>+=head3 set_processing
>+
>+=cut
>+
>+sub set_processing {
>+    my ( $self ) = @_;
>+
>+    $self->priority(0);
>+    $self->found('P');
>+    $self->store();
>+
>+    return $self;
>+}
>+
> =head3 is_found
> 
>-Returns true if hold is a waiting or in transit
>+Returns true if hold is waiting, in transit or in processing
> 
> =cut
> 
>@@ -227,6 +244,7 @@ sub is_found {
>     return 0 unless $self->found();
>     return 1 if $self->found() eq 'W';
>     return 1 if $self->found() eq 'T';
>+    return 1 if $self->found() eq 'P';
> }
> 
> =head3 is_waiting
>@@ -255,6 +273,19 @@ sub is_in_transit {
>     return $self->found() eq 'T';
> }
> 
>+=head3 is_in_processing
>+
>+Returns true if hold is a in_processing hold
>+
>+=cut
>+
>+sub is_in_processing {
>+    my ($self) = @_;
>+
>+    return 0 unless $self->found();
>+    return $self->found() eq 'P';
>+}
>+
> =head3 is_cancelable_from_opac
> 
> Returns true if hold is a cancelable hold
>@@ -269,7 +300,7 @@ sub is_cancelable_from_opac {
>     my ($self) = @_;
> 
>     return 1 unless $self->is_found();
>-    return 0; # if ->is_in_transit or if ->is_waiting
>+    return 0; # if ->is_in_transit or if ->is_waiting or ->is_in_processing
> }
> 
> =head3 is_at_destination
>diff --git a/installer/data/mysql/atomicupdate/bug_12556.perl b/installer/data/mysql/atomicupdate/bug_12556.perl
>new file mode 100644
>index 0000000000..06cae68b96
>--- /dev/null
>+++ b/installer/data/mysql/atomicupdate/bug_12556.perl
>@@ -0,0 +1,8 @@
>+$DBversion = 'XXX'; # will be replaced by the RM
>+if( CheckVersion( $DBversion ) ) {
>+    $dbh->do(q{
>+        INSERT IGNORE INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type` ) VALUES
>+        ('HoldsNeedProcessingSIP', '0', NULL, 'Require staff to check-in before hold is set to waiting state', 'YesNo' )
>+    });
>+    NewVersion( $DBversion, 12556, "Add new syspref HoldsNeedProcessingSIP");
>+}
>diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql
>index d67a02b367..019425481f 100644
>--- a/installer/data/mysql/sysprefs.sql
>+++ b/installer/data/mysql/sysprefs.sql
>@@ -224,6 +224,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `
> ('HoldsAutoFill','0',NULL,'If on, librarian will not be asked if hold should be filled, it will be filled automatically','YesNo'),
> ('HoldsAutoFillPrintSlip','0',NULL,'If on, hold slip print dialog will be displayed automatically','YesNo'),
> ('HoldsLog','0',NULL,'If ON, log create/cancel/suspend/resume actions on holds.','YesNo'),
>+('HoldsNeedProcessingSIP', '0', NULL, 'Require staff to check-in before hold is set to waiting state', 'YesNo' ),
> ('HoldsQueueSkipClosed', '0', NULL, 'If enabled, any libraries that are closed when the holds queue is built will be ignored for the purpose of filling holds.', 'YesNo'),
> ('HoldsSplitQueue','nothing','nothing|branch|itemtype|branch_itemtype','In the staff client, split the holds view by the given criteria','Choice'),
> ('HoldsToPullStartDate','2',NULL,'Set the default start date for the Holds to pull list to this many days ago','Integer'),
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
>index 600977a054..95256bcda6 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
>+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
>@@ -39,6 +39,8 @@
>                     [% IF ( hold.found ) %]
>                         [% IF ( hold.intransit ) %]
>                             <option value="T" selected="selected">In transit</option>
>+                        [% ELSIF (hold.inprocessing) %]
>+                            <option value="P" selected="selected">In processing</option>
>                         [% ELSE %]
>                             <option value="W" selected="selected">Waiting</option>
>                         [% END %]
>@@ -118,13 +120,11 @@
>             <td>
>                 [% IF ( hold.found ) %]
>                     [% IF ( hold.atdestination ) %]
>-                        [% IF ( hold.found ) %]
>                             Item waiting at <b> [% hold.wbrname | html %]</b> <input type="hidden" name="pickup" value="[% hold.wbrcode | html %]" /> since [% hold.waiting_date | $KohaDates %]
>-                        [% ELSE %]
>-                            Waiting to be pulled <input type="hidden" name="pickup" value="[% hold.wbrcode | html %]" />
>-                        [% END %]
>-                    [% ELSE %]
>+                    [% ELSIF (hold.intransit) %]
>                         Item being transferred to <b> [% hold.wbrname | html %]</b> <input type="hidden" name="pickup" value="[% hold.wbrcode | html %]" />
>+                    [% ELSIF (hold.inprocessing) %]
>+                        Item being processed at <b> [% hold.wbrname | html %]</b> <input type="hidden" name="pickup" value="[% hold.wbrcode | html %]" />
>                     [% END %]
>                 [% ELSE %]
>                     [% IF Koha.Preference('IndependentBranches') && Branches.all().size == 1 %]
>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 f2379e797e..7dfe5ee44a 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
>@@ -539,6 +539,12 @@ Circulation:
>                   no: "Don't"
>             - automatically fill holds instead of asking the librarian.
>         -
>+            - pref: HoldsNeedProcessingSIP
>+              choices:
>+                  yes: "Don't fulfill"
>+                  no: Fulfill
>+            - holds automatically if matching item is returned via SIP protocol.
>+        -
>             - pref: HoldsAutoFillPrintSlip
>               choices:
>                   yes: Do
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt
>index 5b93bce7b3..8253473c15 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt
>@@ -99,6 +99,8 @@
>               Cancelled
>           [% ELSIF hold.found == 'W' %]
>               Waiting
>+          [% ELSIF hold.found == 'P' %]
>+              Processing
>           [% ELSIF hold.found == 'T' %]
>               In transit
>           [% ELSE %]
>diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc
>index bb24886c5d..ab9b225db4 100644
>--- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc
>+++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc
>@@ -34,7 +34,7 @@
>             <tbody>
>                 [% SET all_holds_waiting = 1 %]
>                 [% FOREACH HOLD IN HOLDS %]
>-                    [% UNLESS ( HOLD.is_waiting || HOLD.is_in_transit ) %]
>+                    [% UNLESS ( HOLD.is_waiting || HOLD.is_in_transit || HOLD.is_in_processing) %]
>                         [% SET all_holds_waiting = 0 %]
>                     [% END %]
>                     [% IF ( HOLD.is_at_destination ) %]
>@@ -114,6 +114,8 @@
>                                     [% SET transfer = HOLD.item.get_transfer %]
>                                     Item in transit from <strong> [% Branches.GetName( transfer.frombranch ) | html %]</strong> since
>                                     [% transfer.datesent | $KohaDates %]
>+                                [% ELSIF ( HOLD.is_in_processing ) %]
>+                                    Item in processing
>                                 [% ELSIF ( HOLD.suspend ) %]
>                                     Suspended [% IF ( HOLD.suspend_until ) %] until [% HOLD.suspend_until | $KohaDates %] [% END %]
>                                 [% ELSE %]
>diff --git a/reserve/request.pl b/reserve/request.pl
>index 61d56fd49d..6ee5c6cc76 100755
>--- a/reserve/request.pl
>+++ b/reserve/request.pl
>@@ -665,6 +665,7 @@ foreach my $biblionumber (@biblionumbers) {
>             $reserve{'wbrname'}       = $res->branch()->branchname();
>             $reserve{'atdestination'} = $res->is_at_destination();
>             $reserve{'found'}     = $res->is_found();
>+            $reserve{'inprocessing'} = $res->is_in_processing();
>             $reserve{'intransit'} = $res->is_in_transit();
>         }
>         elsif ( $res->priority() > 0 ) {
>diff --git a/t/db_dependent/Hold.t b/t/db_dependent/Hold.t
>index d58b0a243c..5b71b64292 100755
>--- a/t/db_dependent/Hold.t
>+++ b/t/db_dependent/Hold.t
>@@ -29,7 +29,7 @@ use Koha::Item;
> use Koha::DateUtils;
> use t::lib::TestBuilder;
> 
>-use Test::More tests => 29;
>+use Test::More tests => 33;
> use Test::Exception;
> use Test::Warn;
> 
>@@ -112,10 +112,15 @@ isnt( $hold->is_waiting, 1, 'The hold is not waiting (T)' );
> is( $hold->is_found, 1, 'The hold is found');
> is( $hold->is_in_transit, 1, 'The hold is in transit' );
> 
>+$hold->found('P');
>+is( $hold->is_found, 1, 'The hold is found');
>+is( $hold->is_in_processing, 1, 'The hold is in processing' );
>+
> $hold->found(q{});
> isnt( $hold->is_waiting, 1, 'The hold is not waiting (W)' );
> is( $hold->is_found, 0, 'The hold is not found' );
> ok( !$hold->is_in_transit, 'The hold is not in transit' );
>+ok( !$hold->is_in_processing, 'The hold is not in processing' );
> 
> # Test method is_cancelable_from_opac
> $hold->found(undef);
>@@ -124,6 +129,8 @@ $hold->found('W');
> is( $hold->is_cancelable_from_opac, 0, "Waiting hold is not cancelable" );
> $hold->found('T');
> is( $hold->is_cancelable_from_opac, 0, "In transit hold is not cancelable" );
>+$hold->found('P');
>+is( $hold->is_cancelable_from_opac, 0, "In processing hold is not cancelable" );
> 
> # Test method is_at_destination
> $hold->found(undef);
>@@ -178,7 +185,7 @@ subtest "delete() tests" => sub {
> 
> subtest 'suspend() tests' => sub {
> 
>-    plan tests => 16;
>+    plan tests => 18;
> 
>     $schema->storage->txn_begin;
> 
>@@ -232,6 +239,15 @@ subtest 'suspend() tests' => sub {
> 
>     is( $@->status, 'T', 'Exception gets the \'status\' parameter set correctly' );
> 
>+    # set hold found=T
>+    $hold->set_processing();
>+    throws_ok
>+        { $hold->suspend_hold; }
>+        'Koha::Exceptions::Hold::CannotSuspendFound',
>+        'Exception is thrown when a found hold is tried to suspend';
>+
>+    is( $@->status, 'P', 'Exception gets the \'status\' parameter set correctly' );
>+
>     my $holds_module = Test::MockModule->new('Koha::Hold');
>     $holds_module->mock( 'is_found', 1 );
> 
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 12556: 107181 | 107182 | 107183 | 107184 | 107185 | 107186 | 108711 | 109140 | 109924 | 109925 | 109926 | 109927 | 109928 | 109929 | 110719 | 110942 | 110943 | 110944 | 110945 | 110946 | 110947 | 110948 | 110949 | 110950 | 110951 | 110952 | 110953 | 110954 | 110955 | 110956 | 110961 | 110962 | 110963 | 110964 | 110965 | 110966 | 111405 | 111406 | 111407 | 111408 | 111409 | 111410 | 111494 | 111495 | 111496 | 111497 | 111498 | 111499 | 111500 | 111509 | 111516 | 111585 | 111610