Browse Source

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

Signed-off-by: Timothy Alexis Vass <timothy_alexis.vass@ub.lu.se>
Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
20.11.x
Joonas Kylmälä 1 year ago
committed by Jonathan Druart
parent
commit
0e1d291b14
  1. 25
      C4/Reserves.pm
  2. 2
      C4/RotatingCollections.pm
  3. 24
      C4/SIP/ILS/Item.pm
  4. 6
      C4/SIP/ILS/Transaction/Checkout.pm
  5. 35
      Koha/Hold.pm
  6. 26
      circ/returns.pl
  7. 8
      installer/data/mysql/atomicupdate/bug_12556.perl
  8. 1
      installer/data/mysql/mandatory/sysprefs.sql
  9. 10
      koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
  10. 6
      koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
  11. 2
      koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt
  12. 4
      koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc
  13. 1
      reserve/request.pl
  14. 20
      t/db_dependent/Hold.t

25
C4/Reserves.pm

@ -714,6 +714,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';
}
@ -828,7 +829,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 {
@ -1159,7 +1162,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({
@ -1169,9 +1178,7 @@ sub ModReserveAffect {
while( my $transfer = $transfers->next ){
$transfer->datearrived( dt_from_string() )->store;
};
} else {
$hold->set_transfer();
}
}
_FixPriority( { biblionumber => $biblionumber } );
@ -1550,7 +1557,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 );
@ -1562,7 +1569,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);
@ -1977,7 +1984,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(
@ -2147,7 +2154,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 <= ? )';
}

2
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;

24
C4/SIP/ILS/Item.pm

@ -108,8 +108,8 @@ sub new {
my $biblio = Koha::Biblios->find( $self->{biblionumber} );
my $holds = $biblio->current_holds->unblessed;
$self->{hold_queue} = $holds;
$self->{hold_shelf} = [( grep { defined $_->{found} and $_->{found} eq 'W' } @{$self->{hold_queue}} )];
$self->{pending_queue} = [( grep {(! defined $_->{found}) or $_->{found} ne 'W' } @{$self->{hold_queue}} )];
$self->{hold_attached} = [( grep { defined $_->{found} and ( $_->{found} eq 'W' or $_->{found} eq 'P' ) } @{$self->{hold_queue}} )];
$self->{pending_queue} = [( grep {(! defined $_->{found}) or ( $_->{found} ne 'W' and $_->{found} ne 'P' ) } @{$self->{hold_queue}} )];
$self->{title} = $biblio->title;
$self->{author} = $biblio->author;
bless $self, $type;
@ -148,8 +148,8 @@ my %fields = (
sub next_hold {
my $self = shift;
# use Data::Dumper; warn "next_hold() hold_shelf: " . Dumper($self->{hold_shelf}); warn "next_hold() pending_queue: " . $self->{pending_queue};
foreach (@{$self->hold_shelf}) { # If this item was taken from the hold shelf, then that reserve still governs
# use Data::Dumper; warn "next_hold() hold_attached: " . Dumper($self->{hold_attached}); warn "next_hold() pending_queue: " . $self->{pending_queue};
foreach (@{$self->hold_attached}) { # If this item was taken from the hold shelf, then that reserve still governs
next unless ($_->{itemnumber} and $_->{itemnumber} == $self->{itemnumber});
return $_;
}
@ -275,7 +275,7 @@ sub sip_circulation_status {
elsif ( $self->{borrowernumber} ) {
return '04'; # charged
}
elsif ( grep { $_->{itemnumber} == $self->{itemnumber} } @{ $self->{hold_shelf} } ) {
elsif ( grep { $_->{itemnumber} == $self->{itemnumber} } @{ $self->{hold_attached} } ) {
return '08'; # waiting on hold shelf
}
else {
@ -314,10 +314,10 @@ sub pending_queue {
(defined $self->{pending_queue}) or return [];
return $self->{pending_queue};
}
sub hold_shelf {
sub hold_attached {
my $self = shift;
(defined $self->{hold_shelf}) or return [];
return $self->{hold_shelf};
(defined $self->{hold_attached}) or return [];
return $self->{hold_attached};
}
sub hold_queue_position {
@ -359,7 +359,7 @@ sub hold_pickup_date {
# AND no pending (i.e. non-W) hold queue
# OR
# 2) not checked out
# AND (not on hold_shelf OR is on hold_shelf for patron)
# AND (not on hold_attached OR is on hold_attached for patron)
#
# What this means is we are consciously allowing the patron to checkout (but not renew) an item that DOES
# have non-W holds on it, but has not been "picked" from the stacks. That is to say, the
@ -371,13 +371,13 @@ sub hold_pickup_date {
sub available {
my ($self, $for_patron) = @_;
my $count = (defined $self->{pending_queue}) ? scalar @{$self->{pending_queue}} : 0;
my $count2 = (defined $self->{hold_shelf} ) ? scalar @{$self->{hold_shelf} } : 0;
$debug and print STDERR "availability check: pending_queue size $count, hold_shelf size $count2\n";
my $count2 = (defined $self->{hold_attached} ) ? scalar @{$self->{hold_attached} } : 0;
$debug and print STDERR "availability check: pending_queue size $count, hold_attached size $count2\n";
if (defined($self->{borrowernumber})) {
($self->{borrowernumber} eq $for_patron) or return 0;
return ($count ? 0 : 1);
} else { # not checked out
($count2) and return $self->barcode_is_borrowernumber($for_patron, $self->{hold_shelf}[0]->{borrowernumber});
($count2) and return $self->barcode_is_borrowernumber($for_patron, $self->{hold_attached}[0]->{borrowernumber});
}
return 0;
}

6
C4/SIP/ILS/Transaction/Checkout.pm

@ -48,7 +48,7 @@ sub new {
sub do_checkout {
my $self = shift;
siplog('LOG_DEBUG', "ILS::Transaction::Checkout performing checkout...");
my $shelf = $self->{item}->hold_shelf;
my $shelf = $self->{item}->hold_attached;
my $barcode = $self->{item}->id;
my $patron = Koha::Patrons->find($self->{patron}->{borrowernumber});
my $overridden_duedate; # usually passed as undef to AddIssue
@ -115,8 +115,8 @@ sub do_checkout {
($_->{itemnumber} eq $itemnumber) or next; # skip it if not this item
($_->{borrowernumber} == $patron->borrowernumber) and last;
# if item was waiting for this patron, we're done. AddIssue takes care of the "W" hold.
$debug and warn "Item is on hold shelf for another patron.";
$self->screen_msg("Item is on hold shelf for another patron.");
$debug and warn "Item is on hold for another patron.";
$self->screen_msg("Item is on hold for another patron.");
$noerror = 0;
}
my ($fee, undef) = GetIssuingCharges($itemnumber, $patron->borrowernumber);

35
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

26
circ/returns.pl

@ -439,21 +439,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

8
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");
}

1
installer/data/mysql/mandatory/sysprefs.sql

@ -228,6 +228,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 interface, 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'),

10
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 <strong> [% hold.wbrname | html %]</strong> <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 <strong> [% hold.wbrname | html %]</strong> <input type="hidden" name="pickup" value="[% hold.wbrcode | html %]" />
[% ELSIF (hold.inprocessing) %]
Item being processed at <strong> [% hold.wbrname | html %]</strong> <input type="hidden" name="pickup" value="[% hold.wbrcode | html %]" />
[% END %]
[% ELSE %]
[% IF Koha.Preference('IndependentBranches') && Branches.all().size == 1 %]

6
koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref

@ -555,6 +555,12 @@ Circulation:
yes: Do
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:

2
koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt

@ -102,6 +102,8 @@
[% END %]
[% ELSIF hold.found == 'W' %]
Waiting
[% ELSIF hold.found == 'P' %]
Processing
[% ELSIF hold.found == 'T' %]
In transit
[% ELSE %]

4
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 %]

1
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 ) {

20
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 );

Loading…
Cancel
Save