From d864c5640a3e3b02cbb64bde034935fa746a09ce Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 12 Feb 2020 10:02:56 +0100 Subject: [PATCH] Bug 24612: Make hold-transfer-slip take reserve_id To make sure we are going to display the correct hold's info we need to pass the reserve_id. == Test plan == 1. Add some content to HOLD_SLIP notice, e.g.

[% branch.branchname %]

[% biblio.author %]
[% biblio.title %]
[% item.barcode %] 2. Add 2 holds for 1 patron to a single record 3. Check the reserve IDs in the reserves table - on a clean sandbox, they will be 1 and 2 4. Check in one of the items from the record and print the slip 5. Note that the reserve ID on the slip is 2 and the expiration date is blank 6. Repeated check ins do not change this 7. Check in a second item from the record 8. Note that the reserve ID for this hold is also 2, but this time the expiration date is filled in 9. Check in the first item again - the reserve ID stays as 2, but this time the expiration date is filled in 10. Apply patch 11. cancel the holds to come back to a clean state (and maybe ensure items aren't in transit) 12. redo the test and see the following differences 13. 1st checkin: 1. expiration date ok 2. the reserve ID is the one of the first hold 14. 2nd checkin: 1. expiration date ok 2. the reserve ID is the one of the second hold Signed-off-by: Victor Grousset/tuxayo Signed-off-by: Katrin Fischer Signed-off-by: Martin Renvoize --- C4/Circulation.pm | 2 ++ C4/Reserves.pm | 36 ++++--------------- circ/circulation.pl | 3 +- circ/hold-transfer-slip.pl | 10 ++---- circ/returns.pl | 11 ++---- .../prog/en/modules/circ/circulation.tt | 4 +-- .../prog/en/modules/circ/returns.tt | 4 +-- 7 files changed, 18 insertions(+), 52 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index e439250933..2e40721e30 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1038,6 +1038,7 @@ sub CanBookBeIssued { $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber; $needsconfirmation{'resbranchcode'} = $res->{branchcode}; $needsconfirmation{'reswaitingdate'} = $res->{'waitingdate'}; + $needsconfirmation{'reserve_id'} = $res->{reserve_id}; } elsif ( $restype eq "Reserved" ) { # The item is on reserve for someone else. @@ -1048,6 +1049,7 @@ sub CanBookBeIssued { $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 9f8d31637a..d328773b01 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -604,7 +604,7 @@ sub GetOtherReserves { ModReserveStatus($itemnumber,'W'); } - $nextreservinfo = $checkreserves->{'borrowernumber'}; + $nextreservinfo = $checkreserves; } return ( $messages, $nextreservinfo ); @@ -1191,7 +1191,7 @@ sub ModReserveCancelAll { #step 2 launch the subroutine of the others reserves ( $messages, $nextreservinfo ) = GetOtherReserves($itemnumber); - return ( $messages, $nextreservinfo ); + return ( $messages, $nextreservinfo->{borrowernumber} ); } =head2 ModReserveMinusPriority @@ -2055,36 +2055,12 @@ available within the slip: sub ReserveSlip { my ($args) = @_; my $branchcode = $args->{branchcode}; - my $borrowernumber = $args->{borrowernumber}; - my $biblionumber = $args->{biblionumber}; - my $itemnumber = $args->{itemnumber}; - my $barcode = $args->{barcode}; - - - my $patron = Koha::Patrons->find($borrowernumber); - - my $hold; - if ($itemnumber || $barcode ) { - $itemnumber ||= Koha::Items->find( { barcode => $barcode } )->itemnumber; - - $hold = Koha::Holds->search( - { - biblionumber => $biblionumber, - borrowernumber => $borrowernumber, - itemnumber => $itemnumber - } - )->next; - } - else { - $hold = Koha::Holds->search( - { - biblionumber => $biblionumber, - borrowernumber => $borrowernumber - } - )->next; - } + my $reserve_id = $args->{reserve_id}; + my $hold = Koha::Holds->find($reserve_id); return unless $hold; + + my $patron = $hold->borrower; my $reserve = $hold->unblessed; return C4::Letters::GetPreparedLetter ( diff --git a/circ/circulation.pl b/circ/circulation.pl index 81c3bcdf56..5ec80c8f15 100755 --- a/circ/circulation.pl +++ b/circ/circulation.pl @@ -419,7 +419,8 @@ if (@$barcodes) { if ($question->{RESERVE_WAITING} or $question->{RESERVED}){ $template->param( - reserveborrowernumber => $question->{'resborrowernumber'} + reserveborrowernumber => $question->{'resborrowernumber'}, + reserve_id => $question->{reserve_id}, ); } diff --git a/circ/hold-transfer-slip.pl b/circ/hold-transfer-slip.pl index 328f4811f3..a0cb0ef1ea 100755 --- a/circ/hold-transfer-slip.pl +++ b/circ/hold-transfer-slip.pl @@ -35,10 +35,7 @@ my $input = new CGI; my $sessionID = $input->cookie("CGISESSID"); my $session = get_session($sessionID); -my $biblionumber = $input->param('biblionumber'); -my $borrowernumber = $input->param('borrowernumber'); -my $itemnumber = $input->param('itemnumber'); -my $barcode = $input->param('barcode'); +my $reserve_id = $input->param('reserve_id'); my ( $template, $loggedinuser, $cookie ) = get_template_and_user( { @@ -55,10 +52,7 @@ my $userenv = C4::Context->userenv; my ($slip, $is_html); if ( my $letter = ReserveSlip ({ branchcode => $session->param('branch') || $userenv->{branch}, - borrowernumber => $borrowernumber, - biblionumber => $biblionumber, - itemnumber => $itemnumber, - barcode => $barcode, + reserve_id => $reserve_id, }) ) { $slip = $letter->{content}; $is_html = $letter->{is_html}; diff --git a/circ/returns.pl b/circ/returns.pl index 8044262551..e1bce3c3f1 100755 --- a/circ/returns.pl +++ b/circ/returns.pl @@ -77,9 +77,7 @@ my $session = get_session($sessionID); if ( $query->param('print_slip') ) { $template->param( print_slip => 1, - borrowernumber => scalar $query->param('borrowernumber'), # FIXME We should send a Koha::Patron and raise an error if not exist. - biblionumber => scalar $query->param('biblionumber'), - itemnumber => scalar $query->param('itemnumber'), + reserve_id => scalar $query->param('reserve_id'), ); } @@ -419,15 +417,10 @@ if ( $messages->{'ResFound'}) { my $diffBranchSend = !$branchCheck ? $reserve->{branchcode} : undef; ModReserveAffect( $reserve->{itemnumber}, $reserve->{borrowernumber}, $diffBranchSend, $reserve->{reserve_id} ); my ( $messages, $nextreservinfo ) = GetOtherReserves($reserve->{itemnumber}); - - my $patron = Koha::Patrons->find( $nextreservinfo ); - $template->param( hold_auto_filled => 1, print_slip => C4::Context->preference('HoldsAutoFillPrintSlip'), - patron => $patron, - borrowernumber => $patron->id, - biblionumber => $biblio->id, + reserve_id => $nextreservinfo->{reserve_id}, ); if ( $messages->{'transfert'} ) { 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 b8ec599508..e9534ead52 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt @@ -263,7 +263,7 @@ - + [% END %] @@ -273,7 +273,7 @@ - + [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt index bcde1e90cb..43515dac30 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt @@ -929,11 +929,11 @@ $(".print-slip").on('click', function(e) { e.preventDefault(); - Dopop('hold-transfer-slip.pl?borrowernumber=[% patron.borrowernumber | uri %]&biblionumber=[% biblionumber | uri %]'); + Dopop('hold-transfer-slip.pl?reserve_id=[% reserve_id | uri %]'); }); [% IF print_slip %] - Dopop('hold-transfer-slip.pl?borrowernumber=[% borrowernumber | uri %]&biblionumber=[% biblionumber | uri %]&itemnumber=[% itemnumber | uri %]'); + Dopop('hold-transfer-slip.pl?reserve_id=[% reserve_id | uri %]'); [% END %] var columns_settings = [% ColumnsSettings.GetColumns( 'circ', 'returns', 'checkedintable', 'json' ) | $raw %] -- 2.39.5