From 45eee50fcfa08f7fb5b9966783d8ea02441246ed Mon Sep 17 00:00:00 2001 From: Rafal Kopaczka Date: Fri, 30 May 2014 15:15:37 +0200 Subject: [PATCH] Bug 11651: (QA Follow-up) Add possibility to print holds from holds queue. Added unit tests, for new and modified subroutines. Fixed minor qa suggestions. Koha-qa.pl - pass. --- C4/HoldsQueue.pm | 35 ++++++------- C4/Reserves.pm | 13 +++-- circ/hold-pull-print.pl | 42 +++++++++------- circ/view_holdsqueue.pl | 5 +- .../prog/en/modules/circ/printslip.tt | 3 ++ .../prog/en/modules/circ/view_holdsqueue.tt | 52 +++++++++++--------- t/db_dependent/HoldsQueue.t | 26 +++++++++- t/db_dependent/Reserves.t | 8 ++- 8 files changed, 112 insertions(+), 72 deletions(-) diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm index 200cf53..ff1df23 100755 --- a/C4/HoldsQueue.pm +++ b/C4/HoldsQueue.pm @@ -133,7 +133,7 @@ sub GetHoldsQueueItems { $query .=" WHERE tmp_holdsqueue.holdingbranch = ?"; push @bind_params, $branchlimit; } - if (C4::Context->preference('printSlipFromHoldsQueue')){ + if (C4::Context->preference('printSlipFromHoldsQueue') && defined $printstatus){ $query .= $branchlimit ? " AND" : " WHERE"; $query .= " tmp_holdsqueue.print_status = ?"; push @bind_params, $printstatus; @@ -657,14 +657,12 @@ sub least_cost_branch { sub MarkHoldPrinted { my ( $reserve_id ) = @_; my $dbh = C4::Context->dbh; - my $strsth="UPDATE reserves INNER JOIN tmp_holdsqueue USING (reserve_id) - SET reserves.print_status = 1 , tmp_holdsqueue.print_status = 1 + my $strsth="UPDATE reserves LEFT JOIN tmp_holdsqueue USING (reserve_id) + SET reserves.print_status = '1' , tmp_holdsqueue.print_status = '1' WHERE reserve_id = ? - "; + "; my $sth = $dbh->prepare($strsth); - $sth->execute($reserve_id); - - return 1; + return $sth->execute($reserve_id); } =head2 GetItemNumberFromTmpHold @@ -672,20 +670,19 @@ sub MarkHoldPrinted { Returns itemnumber associated with hold. Item number is taken from tmp_holdsqueue table, which is populated by bulid_holds_queue.pl script. - =cut -sub GetItemNumberFromTmpHold{ - my ( $reserve_id ) = @_; - my $dbh = C4::Context->dbh; - my $strsth="SELECT itemnumber - FROM tmp_holdsqueue - WHERE reserve_id = ? - "; - my $sth = $dbh->prepare($strsth); - $sth->execute($reserve_id); +sub GetItemNumberFromTmpHold { + my ( $reserve_id ) = @_; + my $dbh = C4::Context->dbh; + my $strsth="SELECT itemnumber + FROM tmp_holdsqueue + WHERE reserve_id = ? + "; + my $sth = $dbh->prepare($strsth); + $sth->execute($reserve_id); - my $data = $sth->fetchrow; - return $data; + my $data = $sth->fetchrow; + return $data; } 1; diff --git a/C4/Reserves.pm b/C4/Reserves.pm index e382539..c260b91 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -2292,27 +2292,30 @@ sub ReserveSlip { } =head2 HoldSlip - HoldSlip($branch, $reserve_id, $itemnumber) + HoldSlip( { branch => $branch, + reserve_id => $reserve_id, + itemnumber => $itemnumber, + } ) Returns letter hash ( see C4::Letters::GetPreparedLetter ) or undef =cut sub HoldSlip { - my ($branch, $reserve_id, $itemnumber) = @_; + my $params = shift @_; - my $reserve = GetReserveInfo($reserve_id) or return; + my $reserve = GetReserveInfo($params->{reserve_id}) or return; return C4::Letters::GetPreparedLetter ( module => 'circulation', letter_code => 'RESERVESLIP', - branchcode => $branch, + branchcode => $params->{branch}, tables => { 'reserves' => $reserve, 'branches' => $reserve->{branchcode}, 'borrowers' => $reserve->{borrowernumber}, 'biblio' => $reserve->{biblionumber}, - 'items' => $itemnumber, + 'items' => $params->{itemnumber}, }, ); } diff --git a/circ/hold-pull-print.pl b/circ/hold-pull-print.pl index 106b479..7099652 100755 --- a/circ/hold-pull-print.pl +++ b/circ/hold-pull-print.pl @@ -2,21 +2,20 @@ # 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 2 of the License, or (at your option) any later -# version. +# 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. +# 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, write to the Free Software Foundation, Inc., -# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . -use strict; -#use warnings; #FIXME - Bug 2505 +use Modern::Perl; use C4::Context; use C4::Output; use CGI; @@ -38,7 +37,7 @@ my @reservations = $input->param('reserve_id'); my ( $template, $loggedinuser, $cookie ) = get_template_and_user( { - template_name => "circ/printslip.tmpl", + template_name => "circ/printslip.tt", query => $input, type => "intranet", authnotrequired => 0, @@ -49,24 +48,29 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user( my $userenv = C4::Context->userenv; my $branch = $session->param('branch') || $userenv->{branch} || ''; -my ($slip, $is_html); +my ($slip, $is_html, @errors); foreach my $reserve_id (@reservations){ my $itemnumber = GetItemNumberFromTmpHold($reserve_id); - if ( my $letter = HoldSlip ($branch, $reserve_id, $itemnumber) ) { + my $slip_params = ({ + branch => $branch, + reserve_id => $reserve_id, + itemnumber => $itemnumber, + }); + if ( my $letter = HoldSlip ($slip_params) ) { $slip .= $letter->{content}; $is_html = $letter->{is_html}; + MarkHoldPrinted($reserve_id); + } else { + push @errors, $reserve_id; } - else { - $slip .= "Reserve not found"; - } - MarkHoldPrinted($reserve_id); } $template->param( slip => $slip, plain => !$is_html, + errors => \@errors, title => "Koha -- Circulation: Hold print", stylesheet => C4::Context->preference("SlipCSS"), ); diff --git a/circ/view_holdsqueue.pl b/circ/view_holdsqueue.pl index 3e3c5a5..5df4b21 100755 --- a/circ/view_holdsqueue.pl +++ b/circ/view_holdsqueue.pl @@ -4,7 +4,7 @@ # # 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 2 of the License, or (at your option) any later +# 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 @@ -22,8 +22,7 @@ This script displays items in the tmp_holdsqueue table =cut -use strict; -use warnings; +use Modern::Perl; use CGI; use C4::Auth; use C4::Output; diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/printslip.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/printslip.tt index b14ed28..5064125 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/printslip.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/printslip.tt @@ -27,5 +27,8 @@ [% ELSE %] [% IF ( slip ) %][% slip %][% ELSE %]No hold found[% END %] [% END %] +[% FOREACH error IN errors %] + No hold found. (id: [% error %] ) +[% END %] [% INCLUDE 'intranet-bottom.inc' %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/view_holdsqueue.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/view_holdsqueue.tt index ce763d8..1a03f06 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/view_holdsqueue.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/view_holdsqueue.tt @@ -12,39 +12,41 @@ [% INCLUDE 'cat-search.inc' %] [% INCLUDE 'datatables.inc' %] + @@ -73,8 +75,8 @@ function clearAll () { [% IF ( printenable ) %]

- Select all | - Clear all + Select all | + Clear all

@@ -90,11 +92,13 @@ function clearAll () {
[% END %] [% IF ( itemsloop ) %] -
+ [% IF ( printenable ) %] + + [% END %] - [% IF (printenable) %][% END %] + [% IF (printenable) %][% END %] @@ -111,7 +115,7 @@ function clearAll () { [% FOREACH itemsloo IN itemsloop %] [% IF ( printenable ) %] - + [% END %]
Check Title Collection Item type

[% itemsloo.title |html %] [% IF ( itemsloo.subtitle ) %][% itemsloo.subtitle %][% END %]

[% itemsloo.author %] diff --git a/t/db_dependent/HoldsQueue.t b/t/db_dependent/HoldsQueue.t index 0a96143..bec0ec4 100755 --- a/t/db_dependent/HoldsQueue.t +++ b/t/db_dependent/HoldsQueue.t @@ -12,7 +12,7 @@ use C4::Context; use Data::Dumper; -use Test::More tests => 21; +use Test::More tests => 28; use C4::Branch; @@ -281,6 +281,14 @@ $sth->execute( $borrower3->{borrowernumber}, $biblionumber, $branchcodes[0], 3 ) #warn "RESERVES" . Data::Dumper::Dumper( $dbh->selectall_arrayref("SELECT * FROM reserves", { Slice => {} }) ); #warn "ITEMS: " . Data::Dumper::Dumper( $dbh->selectall_arrayref("SELECT * FROM items WHERE biblionumber = $biblionumber", { Slice => {} }) ); +my $reserve_id = $dbh->selectcol_arrayref("SELECT reserve_id FROM reserves"); +my $print_status = $dbh->selectcol_arrayref("SELECT print_status FROM reserves WHERE reserve_id = ?", undef, $reserve_id->[0]); +is( $print_status->[0], "0", "Default print status in reserves for new reserve set as 0"); +C4::HoldsQueue::MarkHoldPrinted($reserve_id->[0]); +$print_status = $dbh->selectcol_arrayref("SELECT print_status FROM reserves WHERE reserve_id = ?", undef, $reserve_id->[0]); +is( $print_status->[0], "1", "Hold status in reserves correctly marked as printed"); +C4::HoldsQueue::MarkHoldPrinted($reserve_id->[1]); + my $holds_queue; $dbh->do("DELETE FROM default_circ_rules"); @@ -298,6 +306,22 @@ $holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice ok( @$holds_queue == 3, "Holds queue filling correct number for holds for default holds policy 'from any library'" ); #warn "HOLDS QUEUE: " . Data::Dumper::Dumper( $holds_queue ); +my $print_status_tmp = $dbh->selectcol_arrayref("SELECT print_status FROM reserves WHERE reserve_id = ?", undef, $reserve_id->[1]); +is( $print_status_tmp->[0], "1", "Hold status printed moved to tmp_holdsqueue from reserves"); +$print_status_tmp = $dbh->selectcol_arrayref("SELECT print_status FROM reserves WHERE reserve_id = ?", undef, $reserve_id->[2]); +is( $print_status_tmp->[0], "0", "Default print status in tmp_holdsqueue moved from reserves"); +C4::HoldsQueue::MarkHoldPrinted($reserve_id->[2]); +$print_status_tmp = $dbh->selectcol_arrayref("SELECT print_status FROM reserves WHERE reserve_id = ?", undef, $reserve_id->[2]); +is( $print_status_tmp->[0], "1", "Hold status in tmp_holdsqueue correctly marked as printed"); + +my $select_itnum = $dbh->selectcol_arrayref("SELECT itemnumber FROM tmp_holdsqueue WHERE reserve_id = ?", undef, $reserve_id->[0]); +my $get_itnum = C4::HoldsQueue::GetItemNumberFromTmpHold($reserve_id->[0]); +is( $select_itnum->[0], $get_itnum, "Correct itemnumber selected from tmp_holdsqueue"); + +C4::Context->set_preference('printSlipFromHoldsQueue', 1); +my $getholds_queue = C4::HoldsQueue::GetHoldsQueueItems(undef, 1); +is( @$getholds_queue, "3", "Get printed holds from tmp_holdsqueue"); + # Cleanup $dbh->rollback; diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index 3ee2832..2c59cbf 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 50; +use Test::More tests => 51; use MARC::Record; use DateTime::Duration; @@ -470,6 +470,12 @@ $cancancel = CanReserveBeCanceledFromOpac($canres->{reserve_id}, $requesters{'CP is($cancancel, 0, 'Reserve in waiting status cant be canceled'); # End of tests for bug 12876 +@results = GetReservesFromItemnumber($itemnumber); +my $hold_slip = C4::Reserves::HoldSlip({ + branch => 'CPL', + reserve_id => $results[3], + itemnumber => $itemnumber }); +ok(defined($hold_slip), 'can successfully generate hold slip'); $dbh->rollback; -- 1.7.10.4