From 3b63cab416212eab8ea023c196f282e683d56d64 Mon Sep 17 00:00:00 2001 From: Benjamin Rokseth Date: Thu, 15 Mar 2018 12:57:37 +0100 Subject: [PATCH] Bug 17561: ReserveSlip needs itemnumber for item level holds on same biblio This patch fixes a regression after bug 14695. This patch adds itemnumber and barcode as optional params in ReserveSlip used by hold-transfer-slip.pl to generate HOLD_SLIP. This is for ReserveSlip to be able to generate correct slips when items in multi-item holds are checked in. Test plan: 1) activate a circulation rule with multi-item holds 2) Place two holds on same biblio for patron 3) for debugging, either use browser console to observe POST request and responses or use info from reserves, e.g. reserve_id in the HOLD_SLIP 4) checkin two items from same biblio on pickup branch 5) note that both holds are effectuated, but reserve_id is the same on both slips 6) also note that there is no itemnumber or barcode in the requests from returns.pl 7) Apply this patch 8) repeat 2-4 9) note that reserve_id is now different on the two slips and/or: Run tests: t/db_dependent/Reserves/ReserveSlip.t Signed-off-by: Brendan Gallagher Signed-off-by: Maksim Sen Signed-off-by: Kyle M Hall Signed-off-by: Jonathan Druart Signed-off-by: Nick Clemens Signed-off-by: Martin Renvoize (cherry picked from commit a2b9db00cf468b1712263879f6afeddb3553bf61) Signed-off-by: Fridolin Somers --- C4/Reserves.pm | 27 ++- circ/hold-transfer-slip.pl | 12 +- .../prog/en/modules/circ/circulation.tt | 4 +- t/db_dependent/Reserves/ReserveSlip.t | 165 ++++++++++++++++++ 4 files changed, 198 insertions(+), 10 deletions(-) create mode 100644 t/db_dependent/Reserves/ReserveSlip.t diff --git a/C4/Reserves.pm b/C4/Reserves.pm index bcb5684b51..5b245ee4d2 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -1964,7 +1964,13 @@ sub RevertWaitingStatus { =head2 ReserveSlip - ReserveSlip($branchcode, $borrowernumber, $biblionumber) + ReserveSlip($args => { + branchcode, + borrowernumber, + biblionumber, + [itemnumber], + [barcode], + }) Returns letter hash ( see C4::Letters::GetPreparedLetter ) or undef @@ -1981,19 +1987,28 @@ available within the slip: =cut sub ReserveSlip { - my ($branch, $borrowernumber, $biblionumber) = @_; + my ($args) = @_; + my $patron = Koha::Patrons->find( $args->{borrowernumber} ); -# return unless ( C4::Context->boolean_preference('printreserveslips') ); - my $patron = Koha::Patrons->find( $borrowernumber ); + my $hold; + if ($args->{itemnumber}) { + $hold = Koha::Holds->search({biblionumber => $args->{biblionumber}, borrowernumber => $args->{borrowernumber}, itemnumber => $args->{itemnumber} })->next; + } elsif ($args->{barcode}) { + my $itemnumber = Koha::Items->find({ barcode => $args->{barcode} }); + if ($args->{itemnumber}) { + $hold = Koha::Holds->search({biblionumber => $args->{biblionumber}, borrowernumber => $args->{borrowernumber}, itemnumber => $args->{itemnumber} })->next; + } + } else { + $hold = Koha::Holds->search({biblionumber => $args->{biblionumber}, borrowernumber => $args->{borrowernumber} })->next; + } - my $hold = Koha::Holds->search({biblionumber => $biblionumber, borrowernumber => $borrowernumber })->next; return unless $hold; my $reserve = $hold->unblessed; return C4::Letters::GetPreparedLetter ( module => 'circulation', letter_code => 'HOLD_SLIP', - branchcode => $branch, + branchcode => $args->{branchcode}, lang => $patron->lang, tables => { 'reserves' => $reserve, diff --git a/circ/hold-transfer-slip.pl b/circ/hold-transfer-slip.pl index bd09e21586..a4472e1ebf 100755 --- a/circ/hold-transfer-slip.pl +++ b/circ/hold-transfer-slip.pl @@ -38,9 +38,11 @@ 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 ( $template, $loggedinuser, $cookie ) = get_template_and_user( - { + { template_name => "circ/printslip.tt", query => $input, type => "intranet", @@ -52,7 +54,13 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user( my $userenv = C4::Context->userenv; my ($slip, $is_html); -if ( my $letter = ReserveSlip ($session->param('branch') || $userenv->{branch}, $borrowernumber, $biblionumber) ) { +if ( my $letter = ReserveSlip ({ + branchcode => $session->param('branch') || $userenv->{branch}, + borrowernumber => $borrowernumber, + biblionumber => $biblionumber, + itemnumber => $itemnumber, + barcode => $barcode, +}) ) { $slip = $letter->{content}; $is_html = $letter->{is_html}; } 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 842e479ee9..ae5b146e95 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt @@ -381,7 +381,7 @@ $(document).ready(function() { - + [% END %] @@ -391,7 +391,7 @@ $(document).ready(function() { - + [% END %] diff --git a/t/db_dependent/Reserves/ReserveSlip.t b/t/db_dependent/Reserves/ReserveSlip.t new file mode 100644 index 0000000000..9b28179fcc --- /dev/null +++ b/t/db_dependent/Reserves/ReserveSlip.t @@ -0,0 +1,165 @@ +#!/usr/bin/perl + +# Copyright 2016 Oslo Public Library +# +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use Test::More tests => 5; +use t::lib::TestBuilder; + +use C4::Reserves qw( ReserveSlip ); +use C4::Context; +use Koha::Database; +use Koha::Holds; +my $schema = Koha::Database->new->schema; +$schema->storage->txn_begin; + +my $dbh = C4::Context->dbh; +$dbh->do(q|DELETE FROM letter|); +$dbh->do(q|DELETE FROM reserves|); + +my $builder = t::lib::TestBuilder->new(); +my $library = $builder->build( + { + source => 'Branch', + } +); + +my $patron = $builder->build( + { + source => 'Borrower', + value => { + branchcode => $library->{branchcode}, + }, + } +); + + +my $biblio = $builder->build( + { + source => 'Biblio', + value => { + title => 'Title 1', + }, + } +); + +my $item1 = $builder->build( + { + source => 'Item', + value => { + biblionumber => $biblio->{biblionumber}, + homebranch => $library->{branchcode}, + holdingbranch => $library->{branchcode}, + }, + } +); + +my $item2 = $builder->build( + { + source => 'Item', + value => { + biblionumber => $biblio->{biblionumber}, + homebranch => $library->{branchcode}, + holdingbranch => $library->{branchcode}, + }, + } +); + +my $hold1 = Koha::Hold->new( + { + biblionumber => $biblio->{biblionumber}, + itemnumber => $item1->{itemnumber}, + waitingdate => '2000-01-01', + borrowernumber => $patron->{borrowernumber}, + branchcode => $library->{branchcode}, + } +)->store; + +my $hold2 = Koha::Hold->new( + { + biblionumber => $biblio->{biblionumber}, + itemnumber => $item2->{itemnumber}, + waitingdate => '2000-01-01', + borrowernumber => $patron->{borrowernumber}, + branchcode => $library->{branchcode}, + } +)->store; + +my $letter = $builder->build( + { + source => 'Letter', + value => { + module => 'circulation', + code => 'HOLD_SLIP', + lang => 'default', + branchcode => $library->{branchcode}, + content => 'Hold found for <>: Please pick up <> with barcode <> at <>.', + message_transport_type => 'email', + }, + } +); + +is ( ReserveSlip(), undef, "No hold slip returned if invalid or undef borrowernumber and/or biblionumber" ); +is ( ReserveSlip({ + branchcode => $library->{branchcode}, + borrowernumber => $patron->{borrowernumber}, + biblionumber => $biblio->{biblionumber}, + })->{code}, + 'HOLD_SLIP', "Get a hold slip from library, patron and biblio" ); + +is (ReserveSlip({ + branchcode => $library->{branchcode}, + borrowernumber => $patron->{borrowernumber}, + biblionumber => $biblio->{biblionumber}, + })->{content}, + "Hold found for $patron->{firstname}: Please pick up $biblio->{title} with barcode $item1->{barcode} at $library->{branchcode}.", "Hold slip contains correctly parsed content"); + +is_deeply( + ReserveSlip({ + branchcode => $library->{branchcode}, + borrowernumber => $patron->{borrowernumber}, + biblionumber => $biblio->{biblionumber}, + }), + ReserveSlip({ + branchcode => $library->{branchcode}, + borrowernumber => $patron->{borrowernumber}, + biblionumber => $biblio->{biblionumber}, + itemnumber => $item1->{itemnumber}, + barcode => $item1->{barcode}, + }), + "No item as param generate hold slip from first item in reserves"); + +isnt ( + ReserveSlip({ + branchcode => $library->{branchcode}, + borrowernumber => $patron->{borrowernumber}, + biblionumber => $biblio->{biblionumber}, + })->{content}, + ReserveSlip({ + branchcode => $library->{branchcode}, + borrowernumber => $patron->{borrowernumber}, + biblionumber => $biblio->{biblionumber}, + itemnumber => $item2->{itemnumber}, + barcode => $item2->{barcode}, + })->{content}, + "Item and/or barcode as params return correct pickup item in hold slip"); + +$schema->storage->txn_rollback; + +1; -- 2.39.5