From fbef5478325bbc71064f9ecb3c4c50eaed0b223d Mon Sep 17 00:00:00 2001 From: =?utf8?q?Joonas=20Kylm=C3=A4l=C3=A4?= Date: Wed, 17 Feb 2021 14:04:47 +0200 Subject: [PATCH] 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 Signed-off-by: Kyle M Hall Signed-off-by: Jonathan Druart --- C4/Circulation.pm | 15 +++++++++++ C4/Reserves.pm | 14 +++++------ C4/SIP/ILS/Transaction/Checkout.pm | 2 +- circ/branchtransfers.pl | 7 ++++++ circ/circulation.pl | 2 +- .../prog/en/modules/circ/branchtransfers.tt | 7 ++++-- .../prog/en/modules/circ/circulation.tt | 25 +++++++++++-------- .../circ/circulation_batch_checkouts.tt | 5 +++- t/db_dependent/Circulation.t | 2 +- t/db_dependent/Reserves.t | 2 +- 10 files changed, 55 insertions(+), 26 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 2229f3edb0..41627e89fb 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -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}; + } } } } diff --git a/C4/Reserves.pm b/C4/Reserves.pm index d5f90494eb..e2aed57a77 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -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; diff --git a/C4/SIP/ILS/Transaction/Checkout.pm b/C4/SIP/ILS/Transaction/Checkout.pm index e33a3c3f35..251a38f695 100644 --- a/C4/SIP/ILS/Transaction/Checkout.pm +++ b/C4/SIP/ILS/Transaction/Checkout.pm @@ -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."); diff --git a/circ/branchtransfers.pl b/circ/branchtransfers.pl index 14ee57eca1..a63d7453cf 100755 --- a/circ/branchtransfers.pl +++ b/circ/branchtransfers.pl @@ -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, diff --git a/circ/circulation.pl b/circ/circulation.pl index ee3878ca13..9923f42f74 100755 --- a/circ/circulation.pl +++ b/circ/circulation.pl @@ -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}, diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/branchtransfers.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/branchtransfers.tt index b29fac2c09..9b563c86b8 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/branchtransfers.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/branchtransfers.tt @@ -31,11 +31,14 @@ [% IF ( waiting ) %] Item is marked waiting at [% branchname | html %] for [% name | html %] ([% borrowernumber | html %]). [% END %] + [% IF ( transferred ) %] + Item is attached to a hold and being transferred for [% name | html %] ([% borrowernumber | html %]). + [% END %] [% 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 %]
@@ -47,7 +50,7 @@ - [% IF ( waiting ) %] + [% IF ( waiting or transferred ) %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt index a4f7369bb2..f91ef45cf1 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt @@ -122,6 +122,10 @@
  • Item [% getTitleMessageIteminfo | html %] ([% getBarcodeMessageIteminfo | html %]) has been waiting for [% resfirstname | html %] [% ressurname | html %] ([% rescardnumber | html %]) at [% Branches.GetName( resbranchcode ) | html %] since [% reswaitingdate | $KohaDates %]
    • [% END %] + [% IF ( TRANSFERRED ) %] +
  • Item [% getTitleMessageIteminfo | html %] ([% getBarcodeMessageIteminfo | html %]) is on hold for [% resfirstname | html %] [% ressurname | html %] ([% rescardnumber | html %]) and being transferred to [% Branches.GetName( resbranchcode ) | html %]
    • + [% END %] + [% IF ( RESERVED ) %]
  • Item [% getTitleMessageIteminfo | html %] ([% getBarcodeMessageIteminfo | html %]) has been on hold for [% resfirstname | html %] [% ressurname | html %] ([% rescardnumber | html %]) at [% Branches.GetName( resbranchcode ) | html %] since [% resreservedate | $KohaDates %]
    • [% END %] @@ -237,6 +241,15 @@

    [% END %] + [% IF ( TRANSFERRED ) %] +

    + +
    + + +

    + [% END %] + @@ -267,17 +280,7 @@
    [% END # /IF CAN_user_circulate_force_checkout or HIGHHOLDS %] - [% IF ( RESERVED ) %] -
    - - - - - -
    - [% END %] - - [% IF ( RESERVE_WAITING ) %] + [% IF ( RESERVED or RESERVE_WAITING or TRANSFERRED ) %]
    diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation_batch_checkouts.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation_batch_checkouts.tt index 6793b95c91..c21b2e79ae 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation_batch_checkouts.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation_batch_checkouts.tt @@ -147,6 +147,9 @@ [% IF checkout_info.RESERVE_WAITING %]
  • This item is waiting for another patron.
    • [% END %] + [% IF checkout_info.TRANSFERRED %] +
  • This item is on hold and being transferred to another patron.
    • + [% END %] [% IF checkout_info.ISSUED_TO_ANOTHER %]
  • This item is checked out to another patron. [% IF CAN_user_circulate_force_checkout %] @@ -187,7 +190,7 @@ [% END %] - [% IF NOT checkout_info.IMPOSSIBLE && ( CAN_user_circulate_force_checkout || checkout_info.HIGHHOLDS ) && ( checkout_info.RESERVED || checkout_info.RESERVE_WAITING ) %] + [% IF NOT checkout_info.IMPOSSIBLE && ( CAN_user_circulate_force_checkout || checkout_info.HIGHHOLDS ) && ( checkout_info.RESERVED || checkout_info.RESERVE_WAITING || checkout_info.TRANSFERRED ) %]
  • This item is on hold for another patron. The hold will be overridden, but not cancelled.
    • [% ELSIF checkout_info.RESERVED %]
  • This item is on hold for another patron.
    • diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index d40e586e6b..d0bfe6e207 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -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; diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index 2b47ebce5e..7422cb9817 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -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' ); -- 2.39.5