Koha-community
/
Koha
13
5
Fork 0
Code Issues Releases Wiki Activity
Browse Source

Bug 25690: Remove double usage of 'Reserved' return value 

The patch "Bug 19116: Hold not set to waiting after transfer" added a
new meaning to 'Reserved' return value of C4::Reserves::CheckReserves
function. Let's remove double usage and have separate Transferred
return value so we can differentiate between attached and non-attached
holds. This will come useful in future refactorings.

This patch does no changes to the logic except in the
/cgi-bin/koha/circ/branchtransfers.pl and circulation.pl we now give
similarly to waiting state notice about hold being transferred.

To test:
   1) Apply this patch
   2) Create a new item level hold so that pickup library is different
   than where the item is currently. Then return the item so that hold
   is being attached and transferred.
   3) Go to branchtransfers.pl and try to create a new transfer: it
   should prompt you with message "Item is attached to a hold and
   being transferred for XXX" and provide you with option to cancel
   the hold or to ignore the transfer.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
21.05.x
Joonas Kylmälä 3 years ago
committed by Jonathan Druart
parent
979b9c4678
commit
fbef547832
10 changed files with 55 additions and 26 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 15
      C4/Circulation.pm
  2. 14
      C4/Reserves.pm
  3. 2
      C4/SIP/ILS/Transaction/Checkout.pm
  4. 7
      circ/branchtransfers.pl
  5. 2
      circ/circulation.pl
  6. 7
      koha-tmpl/intranet-tmpl/prog/en/modules/circ/branchtransfers.tt
  7. 25
      koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
  8. 5
      koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation_batch_checkouts.tt
  9. 2
      t/db_dependent/Circulation.t
  10. 2
      t/db_dependent/Reserves.t

15
C4/Circulation.pm
View File

@ -720,6 +720,10 @@ issued to someone else.
reserved for someone else.
=head3 TRANSFERRED
reserved and being transferred for someone else.
=head3 INVALID_DATE
sticky due date is invalid or due date in the past
@ -1132,6 +1136,17 @@ sub CanBookBeIssued {
$needsconfirmation{'resreservedate'} = $res->{reservedate};
$needsconfirmation{'reserve_id'} = $res->{reserve_id};
}
elsif ( $restype eq "Transferred" ) {
# The item is determined hold being transferred for someone else.
$needsconfirmation{TRANSFERRED} = 1;
$needsconfirmation{'resfirstname'} = $patron->firstname;
$needsconfirmation{'ressurname'} = $patron->surname;
$needsconfirmation{'rescardnumber'} = $patron->cardnumber;
$needsconfirmation{'resborrowernumber'} = $patron->borrowernumber;
$needsconfirmation{'resbranchcode'} = $patron->branchcode;
$needsconfirmation{'resreservedate'} = $res->{reservedate};
$needsconfirmation{'reserve_id'} = $res->{reserve_id};
}
}
}
}

14
C4/Reserves.pm
View File

@ -853,14 +853,12 @@ sub CheckReserves {
my $priority = 10000000;
foreach my $res (@reserves) {
if ( $res->{'itemnumber'} && $res->{'itemnumber'} == $itemnumber && $res->{'priority'} == 0) {
if ($res->{'found'} eq 'W') {
return ( "Waiting", $res, \@reserves ); # Found it, it is waiting
} elsif ($res->{'found'} eq 'P') {
return ( "Processing", $res, \@reserves ); # Found determinated hold, e. g. the transferred one
} else {
return ( "Reserved", $res, \@reserves ); # Found determinated hold, e. g. the transferred one
}
if ($res->{'found'} eq 'W') {
return ( "Waiting", $res, \@reserves ); # Found it, it is waiting
} elsif ($res->{'found'} eq 'P') {
return ( "Processing", $res, \@reserves ); # Found determinated hold, e. g. the transferred one
} elsif ($res->{'found'} eq 'T') {
return ( "Transferred", $res, \@reserves ); # Found determinated hold, e. g. the transferred one
} else {
my $patron;
my $item;

2
C4/SIP/ILS/Transaction/Checkout.pm
View File

@ -71,7 +71,7 @@ sub do_checkout {
foreach my $confirmation (keys %{$needsconfirmation}) {
if ($confirmation eq 'RENEW_ISSUE'){
$self->screen_msg("Item already checked out to you: renewing item.");
} elsif ($confirmation eq 'RESERVED' or $confirmation eq 'RESERVE_WAITING') {
} elsif ($confirmation eq 'RESERVED' or $confirmation eq 'RESERVE_WAITING' or $confirmation eq 'TRANSFERRED') {
my $x = $self->{item}->available($patron->borrowernumber);
if ($x) {
$self->screen_msg("Item was reserved for you.");

7
circ/branchtransfers.pl
View File

@ -73,6 +73,7 @@ my $messages;
my $found;
my $reserved;
my $waiting;
my $hold_transferred;
my $reqmessage;
my $cancelled;
my $setwaiting;
@ -176,6 +177,11 @@ if ($found) {
if ( $res->{'ResFound'} eq "Waiting" ) {
$waiting = 1;
}
if ( $res->{'ResFound'} eq "Transferred" ) {
$hold_transferred = 1;
}
elsif ( $res->{'ResFound'} eq "Reserved" ) {
$reserved = 1;
$biblionumber = $res->{'biblionumber'};
@ -218,6 +224,7 @@ $template->param(
found => $found,
reserved => $reserved,
waiting => $waiting,
transferred => $hold_transferred,
borrowernumber => $borrowernumber,
itemnumber => $itemnumber,
barcode => $barcode,

2
circ/circulation.pl
View File

@ -422,7 +422,7 @@ if (@$barcodes) {
}
}
if ($question->{RESERVE_WAITING} or $question->{RESERVED}){
if ($question->{RESERVE_WAITING} or $question->{RESERVED} or $question->{TRANSFERRED}){
$template->param(
reserveborrowernumber => $question->{'resborrowernumber'},
reserve_id => $question->{reserve_id},

7
koha-tmpl/intranet-tmpl/prog/en/modules/circ/branchtransfers.tt
View File

@ -31,11 +31,14 @@
[% IF ( waiting ) %]
Item is marked waiting at [% branchname | html %] for [% name | html %] (<a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber | uri %]">[% borrowernumber | html %]</a>).
[% END %]
[% IF ( transferred ) %]
Item is attached to a hold and being transferred for [% name | html %] (<a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber | uri %]">[% borrowernumber | html %]</a>).
[% END %]
</caption>
<tr>
<th>
[% IF ( reserved ) %]Set reserve to waiting and transfer book to [% branchname | html %]: [% END %]
[% IF ( waiting ) %]Cancel reservation and then attempt transfer: [% END %]
[% IF ( waiting or transferred ) %]Cancel reservation and then attempt transfer: [% END %]
</th>
<td>
<form method="post" name="mainform" id="mainform" action="branchtransfers.pl">
@ -47,7 +50,7 @@
<input type="hidden" name="itemnumber" value="[% itemnumber | html %]" />
<input type="hidden" name="borrowernumber" value="[% borrowernumber | html %]" />
<input type="hidden" name="tobranchcd" value="[% tobranchcd | html %]" />
[% IF ( waiting ) %]
[% IF ( waiting or transferred ) %]
<input type="hidden" name="barcode" value="[% barcode | html %]" />
<input type="hidden" name="request" value="KillWaiting" />
<input type="submit" value="Cancel" />

25
koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
View File

@ -122,6 +122,10 @@
<li>Item <em>[% getTitleMessageIteminfo | html %]</em> ([% getBarcodeMessageIteminfo | html %]) has been waiting for <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% resborrowernumber | uri %]">[% resfirstname | html %] [% ressurname | html %]</a> ([% rescardnumber | html %]) at [% Branches.GetName( resbranchcode ) | html %] since [% reswaitingdate | $KohaDates %]</li>
[% END %]
[% IF ( TRANSFERRED ) %]
<li>Item <em>[% getTitleMessageIteminfo | html %]</em> ([% getBarcodeMessageIteminfo | html %]) is on hold for <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% resborrowernumber | uri %]">[% resfirstname | html %] [% ressurname | html %]</a> ([% rescardnumber | html %]) and being transferred to [% Branches.GetName( resbranchcode ) | html %]</li>
[% END %]
[% IF ( RESERVED ) %]
<li>Item <em>[% getTitleMessageIteminfo | html %]</em> ([% getBarcodeMessageIteminfo | html %]) has been on hold for <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% resborrowernumber | uri %]">[% resfirstname | html %] [% ressurname | html %]</a> ([% rescardnumber | html %]) at [% Branches.GetName( resbranchcode ) | html %] since [% resreservedate | $KohaDates %]</li>
[% END %]
@ -237,6 +241,15 @@
</p>
[% END %]
[% IF ( TRANSFERRED ) %]
<p>
<label for="cancelreserve">Cancel hold</label>
<input type="radio" value="cancel" name="cancelreserve" id="cancelreserve" /><br />
<label for="revertreserve">Revert hold transfer status</label>
<input type="radio" value="revert" name="cancelreserve" id="revertreserve" checked="checked"/>
</p>
[% END %]
<input type="hidden" name="barcode" value="[% barcode | html %]" />
<input type="hidden" name="borrowernumber" value="[% patron.borrowernumber | html %]" />
<input type="hidden" name="issueconfirmed" value="1" />
@ -267,17 +280,7 @@
</form>
[% END # /IF CAN_user_circulate_force_checkout or HIGHHOLDS %]
[% IF ( RESERVED ) %]
<form method="get" action="/cgi-bin/koha/circ/circulation.pl">
<input type="hidden" name="restoreduedatespec" />
<input type="hidden" name="borrowernumber" value="[% patron.borrowernumber | html %]" />
<input type="hidden" name="duedatespec" value="[% duedatespec | html %]" />
<input type="hidden" name="stickyduedate" value="[% stickyduedate | html %]" />
<button class="print" type="submit" onclick="Dopop('hold-transfer-slip.pl?reserve_id=[% reserve_id | uri %]');this.form.submit();"><i class="fa fa-print"></i> Don't check out and print slip (P)</button>
</form>
[% END %]
[% IF ( RESERVE_WAITING ) %]
[% IF ( RESERVED or RESERVE_WAITING or TRANSFERRED ) %]
<form method="get" action="/cgi-bin/koha/circ/circulation.pl">
<input type="hidden" name="restoreduedatespec" />
<input type="hidden" name="borrowernumber" value="[% patron.borrowernumber | html %]" />

5
koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation_batch_checkouts.tt
View File

@ -147,6 +147,9 @@
[% IF checkout_info.RESERVE_WAITING %]
<li><i class="fa fa-li fa-warning"></i>This item is waiting for another patron.</li>
[% END %]
[% IF checkout_info.TRANSFERRED %]
<li><i class="fa fa-li fa-warning"></i>This item is on hold and being transferred to another patron.</li>
[% END %]
[% IF checkout_info.ISSUED_TO_ANOTHER %]
<li><i class="fa fa-li fa-warning"></i>This item is checked out to another patron.
[% IF CAN_user_circulate_force_checkout %]
@ -187,7 +190,7 @@
</script>
[% END %]
[% IF NOT checkout_info.IMPOSSIBLE && ( CAN_user_circulate_force_checkout || checkout_info.HIGHHOLDS ) && ( checkout_info.RESERVED || checkout_info.RESERVE_WAITING ) %] <!-- arbitrary choice, revert the reserve is not possible-->
[% IF NOT checkout_info.IMPOSSIBLE && ( CAN_user_circulate_force_checkout || checkout_info.HIGHHOLDS ) && ( checkout_info.RESERVED || checkout_info.RESERVE_WAITING || checkout_info.TRANSFERRED ) %] <!-- arbitrary choice, revert the reserve is not possible-->
<li><i class="fa fa-li fa-warning"></i>This item is on hold for another patron. The hold will be overridden, but not cancelled.</li>
[% ELSIF checkout_info.RESERVED %]
<li><i class="fa fa-li fa-warning"></i>This item is on hold for another patron.</li>

2
t/db_dependent/Circulation.t
View File

@ -3364,7 +3364,7 @@ subtest 'Set waiting flag' => sub {
is( $hold->found, 'T', 'Hold is in transit' );
my ( $status ) = CheckReserves($item->itemnumber);
is( $status, 'Reserved', 'Hold is not waiting yet');
is( $status, 'Transferred', 'Hold is not waiting yet');
set_userenv( $library_2 );
$do_transfer = 0;

2
t/db_dependent/Reserves.t
View File

@ -1147,7 +1147,7 @@ subtest 'CheckReserves additional test' => sub {
my ( $status, $matched_reserve, $possible_reserves ) =
CheckReserves( $item->itemnumber );
is( $status, 'Reserved', "We found a reserve" );
is( $status, 'Transferred', "We found a reserve" );
is( $matched_reserve->{reserve_id},
$reserve1->reserve_id, "We got the Transit reserve" );
is( scalar @$possible_reserves, 2, 'We do get both reserves' );

Write Preview
Loading…
Cancel
Save