Bugzilla – Attachment 108711 Details for
Bug 12556
SelfCheck machine starts the hold instantly with an email sent out
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 12556: Add new "in processing" state to holds
Bug-12556-Add-new-in-processing-state-to-holds.patch (text/plain), 17.28 KB, created by
Joonas Kylmälä
on 2020-08-20 07:08:58 UTC
(
hide
)
Description:
Bug 12556: Add new "in processing" state to holds
Filename:
MIME Type:
Creator:
Joonas Kylmälä
Created:
2020-08-20 07:08:58 UTC
Size:
17.28 KB
patch
obsolete
>From 499953d6e77bf379ef5ac66097bdb7e8a81f2ca2 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 7951fbe471..f5175c5ea8 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 2591a4784c..b21c431086 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 { >@@ -1154,7 +1157,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({ >@@ -1164,9 +1173,7 @@ sub ModReserveAffect { > while( my $transfer = $transfers->next ){ > $transfer->datearrived( dt_from_string() )->store; > }; >- } else { >- $hold->set_transfer(); >- } >+ } > > > _FixPriority( { biblionumber => $biblionumber } ); >@@ -1539,7 +1546,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 ); >@@ -1551,7 +1558,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); >@@ -1963,7 +1970,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( >@@ -2133,7 +2140,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 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 0924a4271d..ec97734745 100644 >--- a/installer/data/mysql/sysprefs.sql >+++ b/installer/data/mysql/sysprefs.sql >@@ -223,6 +223,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 1e2c4bf1ba..978e32af9a 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 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 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
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=108711">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
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