From 17ae9b230d2c85b44e4f6dfac90dc9769c0c4f35 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 23 Sep 2014 13:02:51 +0200 Subject: [PATCH] Bug 12980: GetHistory does useless processing 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 Signed-off-by: Kyle M Hall Signed-off-by: Tomas Cohen Arazi Signed-off-by: scourret@gmail.com <2> http://bugs.koha-community.org/show_bug.cgi?id=12404 --- C4/Acquisition.pm | 19 +++---------------- acqui/histsearch.pl | 7 ++----- catalogue/detail.pl | 2 +- t/db_dependent/Acquisition.t | 4 ++-- 4 files changed, 8 insertions(+), 24 deletions(-) diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm index f9e8c49..ecf96fe 100644 --- a/C4/Acquisition.pm +++ b/C4/Acquisition.pm @@ -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 diff --git a/acqui/histsearch.pl b/acqui/histsearch.pl index b474ac1..376681f 100755 --- a/acqui/histsearch.pl +++ b/acqui/histsearch.pl @@ -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, diff --git a/catalogue/detail.pl b/catalogue/detail.pl index 967d71d..f33e263 100755 --- a/catalogue/detail.pl +++ b/catalogue/detail.pl @@ -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, ); diff --git a/t/db_dependent/Acquisition.t b/t/db_dependent/Acquisition.t index 27da4e0..9211eed 100755 --- a/t/db_dependent/Acquisition.t +++ b/t/db_dependent/Acquisition.t @@ -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( -- 1.7.2.5