From 40096821f38c122dfb35b64093da08f774c479fd 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 (cherry picked from commit d864c5640a3e3b02cbb64bde034935fa746a09ce) Signed-off-by: Aleisha Amohia --- 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 09d55f2062..6b6d6325d8 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1036,6 +1036,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. @@ -1046,6 +1047,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 75235ee670..2a9296f76f 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -567,7 +567,7 @@ sub GetOtherReserves { ModReserveStatus($itemnumber,'W'); } - $nextreservinfo = $checkreserves->{'borrowernumber'}; + $nextreservinfo = $checkreserves; } return ( $messages, $nextreservinfo ); @@ -1153,7 +1153,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 @@ -2004,36 +2004,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 b7d9782ef1..e854277f9a 100755 --- a/circ/circulation.pl +++ b/circ/circulation.pl @@ -443,7 +443,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 903a646d04..557fadc274 100755 --- a/circ/returns.pl +++ b/circ/returns.pl @@ -80,9 +80,7 @@ if ($session->param('branch') eq 'NO_LIBRARY_SET'){ 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'), ); } @@ -425,15 +423,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 2624ec9a61..bdf6ed3297 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt @@ -262,7 +262,7 @@ - + [% END %] @@ -272,7 +272,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 13dac5f2b2..184ae004d7 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt @@ -936,11 +936,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