From 7a198a57ea76d37e01957fec0148f62811aa804a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joonas=20Kylm=C3=A4l=C3=A4?= 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 -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 --item -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 ) %] + [% ELSIF (hold.inprocessing) %] + [% ELSE %] [% END %] @@ -118,13 +120,11 @@ [% IF ( hold.found ) %] [% IF ( hold.atdestination ) %] - [% IF ( hold.found ) %] Item waiting at [% hold.wbrname | html %] since [% hold.waiting_date | $KohaDates %] - [% ELSE %] - Waiting to be pulled - [% END %] - [% ELSE %] + [% ELSIF (hold.intransit) %] Item being transferred to [% hold.wbrname | html %] + [% ELSIF (hold.inprocessing) %] + Item being processed at [% hold.wbrname | 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 @@ [% 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 [% Branches.GetName( transfer.frombranch ) | html %] 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