Attachment 109140 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), 18.71 KB, created by Joonas Kylmälä on 2020-08-26 09:52:00 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Joonas Kylmälä

Created: 2020-08-26 09:52:00 UTC

Size: 18.71 KB

patch

obsolete

>From f1a487dae83d780f48809a4c83bc77be8c6ca20d 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 ++++++++++++++++++++--
> circ/returns.pl                                    | 26 +++++++---------
> 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 +++++++++++--
> 13 files changed, 108 insertions(+), 35 deletions(-)
> create mode 100644 installer/data/mysql/atomicupdate/bug_12556.perl
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index db01b57cc2..184e78950d 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -1105,6 +1105,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 ec355f9da1..d5a06ace0f 100644
>--- a/C4/Reserves.pm
>+++ b/C4/Reserves.pm
>@@ -712,6 +712,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';
>     }
> 
>@@ -826,7 +827,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 {
>@@ -1155,7 +1158,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({
>@@ -1165,9 +1174,7 @@ sub ModReserveAffect {
>         while( my $transfer = $transfers->next ){
>             $transfer->datearrived( dt_from_string() )->store;
>         };
>-     } else {
>-        $hold->set_transfer();
>-     }
>+    }
> 
> 
>     _FixPriority( { biblionumber => $biblionumber } );
>@@ -1546,7 +1553,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 );
>@@ -1558,7 +1565,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);
>@@ -1970,7 +1977,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(
>@@ -2140,7 +2147,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 c41d5780e9..954c2d419b 100644
>--- a/C4/RotatingCollections.pm
>+++ b/C4/RotatingCollections.pm
>@@ -452,7 +452,7 @@ sub TransferCollection {
>             barcode => $item->{barcode},
>             ignore_reserves => 1,
>             trigger => 'RotatingCollection'
>-        }) unless ( $status eq 'Waiting' || @transfers );
>+        }) unless ( $status eq 'Waiting' || $status eq 'Processing' || @transfers );
>     }
> 
>     return 1;
>diff --git a/Koha/Hold.pm b/Koha/Hold.pm
>index 845028da97..7375a8fdca 100644
>--- a/Koha/Hold.pm
>+++ b/Koha/Hold.pm
>@@ -96,6 +96,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='
>@@ -217,9 +220,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
> 
>@@ -229,6 +246,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
>@@ -257,6 +275,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
>@@ -271,7 +302,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/circ/returns.pl b/circ/returns.pl
>index e1bce3c3f1..69297c6a58 100755
>--- a/circ/returns.pl
>+++ b/circ/returns.pl
>@@ -432,21 +432,17 @@ if ( $messages->{'ResFound'}) {
>                 diffbranch     => 1,
>             );
>         }
>-    }
>-    elsif ( $reserve->{'ResFound'} eq "Waiting" or $reserve->{'ResFound'} eq "Reserved" ) {
>-        if ( $reserve->{'ResFound'} eq "Waiting" ) {
>-            $template->param(
>-                waiting      => $branchCheck ? 1 : undef,
>-            );
>-        } elsif ( $reserve->{'ResFound'} eq "Reserved" ) {
>-            $template->param(
>-                intransit    => $branchCheck ? undef : 1,
>-                transfertodo => $branchCheck ? undef : 1,
>-                reserve_id   => $reserve->{reserve_id},
>-                reserved     => 1,
>-            );
>-        }
>-
>+    } elsif ( $reserve->{'ResFound'} eq "Waiting" ) {
>+        $template->param(
>+            waiting      => $branchCheck ? 1 : undef,
>+        );
>+    } elsif ( $reserve->{'ResFound'} eq "Reserved" || $reserve->{'ResFound'} eq "Processing" ) {
>+        $template->param(
>+            intransit    => $branchCheck ? undef : 1,
>+            transfertodo => $branchCheck ? undef : 1,
>+            reserve_id   => $reserve->{reserve_id},
>+            reserved     => 1,
>+        );
>     } # else { ; }  # error?
> 
>     # same params for Waiting or Reserved
>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 cfb5ba6837..c331af54af 100644
>--- a/installer/data/mysql/sysprefs.sql
>+++ b/installer/data/mysql/sysprefs.sql
>@@ -226,6 +226,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 26791f819d..380a7d7023 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
>+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
>@@ -41,6 +41,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 %]
>@@ -120,13 +122,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 1cdf0e9c0a..7ae275df7b 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
>@@ -545,6 +545,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 0e8ab42403..3587c25615 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 efa256fa53..e2912b1922 100755
>--- a/reserve/request.pl
>+++ b/reserve/request.pl
>@@ -666,6 +666,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