Koha-community/Koha
15
7
Fork 0
Code Issues Releases Wiki Activity

Bug 12980: GetHistory does useless processing

Browse source 
GetHistory iterated on the orders to calculate the quantity and price.
These values are never used by the called.
It can be removed.

Test plan:
Verify there is no regression on acqui/histsearch.pl and
catalogue/detail.pl
Actually you just have to check that the total quantity and price are
not displayed on these views.

QA: note that 'count' and 'toggle' are never used in the template.

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
This commit is contained in:
Jonathan Druart 2014-09-23 13:02:51 +02:00 committed by Tomas Cohen Arazi
parent b449b08d91
commit 46e3f8169c
4 changed files with 8 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
C4/Acquisition.pm
View file

@ -2186,7 +2186,7 @@ sub GetLateOrders {
=head3 GetHistory
(\@order_loop, $total_qty, $total_price, $total_qtyreceived) = GetHistory( %params );
\@order_loop = GetHistory( %params );
Retreives some acquisition history information
@ -2226,9 +2226,6 @@ returns:
'quantityreceived' => undef,
'title' => 'The Adventures of Huckleberry Finn'
}
$total_qty is the sum of all of the quantities in $order_loop
$total_price is the cost of each in $order_loop times the quantity
$total_qtyreceived is the sum of all of the quantityreceived entries in $order_loop
=cut
@ -2397,18 +2394,8 @@ sub GetHistory {
}
}
$query .= " ORDER BY id";
my $sth = $dbh->prepare($query);
$sth->execute( @query_params );
my $cnt = 1;
while ( my $line = $sth->fetchrow_hashref ) {
$line->{count} = $cnt++;
$line->{toggle} = 1 if $cnt % 2;
push @order_loop, $line;
$total_qty += ( $line->{quantity} ) ? $line->{quantity} : 0;
$total_qtyreceived += ( $line->{quantityreceived} ) ? $line->{quantityreceived} : 0;
$total_price += ( $line->{quantity} and $line->{ecost} ) ? $line->{quantity} * $line->{ecost} : 0;
}
return \@order_loop, $total_qty, $total_price, $total_qtyreceived;
return $dbh->selectall_arrayref( $query, { Slice => {} }, @query_params );
}
=head2 GetRecentAcqui

7
acqui/histsearch.pl
View file

@ -103,10 +103,10 @@ if ( $d = $input->param('iso') ) {
$to_iso = C4::Dates->new($d)->output('iso');
}
my ( $order_loop, $total_qty, $total_price, $total_qtyreceived );
my $order_loop;
# If we're supplied any value then we do a search. Otherwise we don't.
if ($do_search) {
( $order_loop, $total_qty, $total_price, $total_qtyreceived ) = GetHistory(
$order_loop = GetHistory(
title => $title,
author => $author,
isbn => $isbn,
@ -139,9 +139,6 @@ for my $bp ( @{$budgetperiods} ) {
$template->param(
order_loop => $order_loop,
total_qty => $total_qty,
total_qtyreceived => $total_qtyreceived,
total_price => sprintf( "%.2f", $total_price ),
numresults => $order_loop ? scalar(@$order_loop) : undef,
title => $title,
author => $author,

2
catalogue/detail.pl
View file

@ -168,7 +168,7 @@ foreach my $subscription (@subscriptions) {
# Get acquisition details
if ( C4::Context->preference('AcquisitionDetails') ) {
my ( $orders, $qty, $price, $received ) = C4::Acquisition::GetHistory( biblionumber => $biblionumber, get_canceled_order => 1 );
my $orders = C4::Acquisition::GetHistory( biblionumber => $biblionumber, get_canceled_order => 1 );
$template->param(
orders => $orders,
);

4
t/db_dependent/Acquisition.t
View file

@ -834,9 +834,9 @@ is( $neworder->{'budget_id'}, $budgetid, 'Budget on new order is unchanged' );
is( $neworder->{ordernumber}, $new_ordernumber, 'Split: test ordernumber' );
is( $neworder->{parent_ordernumber}, $ordernumbers[1], 'Split: test parent_ordernumber' );
my ( $orders ) = GetHistory( ordernumber => $ordernumbers[1] );
my $orders = GetHistory( ordernumber => $ordernumbers[1] );
is( scalar( @$orders ), 1, 'GetHistory with a given ordernumber returns 1 order' );
( $orders ) = GetHistory( ordernumber => $ordernumbers[1], search_children_too => 1 );
$orders = GetHistory( ordernumber => $ordernumbers[1], search_children_too => 1 );
is( scalar( @$orders ), 2, 'GetHistory with a given ordernumber and search_children_too set returns 2 orders' );
my $budgetid2 = C4::Budgets::AddBudget(