[Koha-patches] [PATCH] Bug 2094: cleanup of lost items report

[Andrew Moore]

Just some code cleanup of C4::Items::GetLostItems and reports/itemslist.pl.
Some whitespace changes, perltidy, and documentation improvements.
Also, I rewrote and SQL statement to try to reduce possibilities of SQL injection attacks.

No documentation or functional changes necessary with this patch.
---
 C4/Items.pm          |   42 +++++++++++++++++++++++++++---------------
 reports/itemslost.pl |   18 ++++--------------
 2 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/C4/Items.pm b/C4/Items.pm
index 685d85e..fb285db 100644
--- a/C4/Items.pm
+++ b/C4/Items.pm
@@ -841,28 +841,35 @@ sub GetItemLocation {
 
 =over 4
 
-$items = GetLostItems($where,$orderby);
+$items = GetLostItems( $where, $orderby );
 
 =back
 
-This function get the items lost into C<$items>.
+This function gets a list of lost items.
 
 =over 2
 
 =item input:
+
 C<$where> is a hashref. it containts a field of the items table as key
-and the value to match as value.
-C<$orderby> is a field of the items table.
+and the value to match as value. For example:
+
+{ barcode    => 'abc123',
+  homebranch => 'CPL',    }
+
+C<$orderby> is a field of the items table by which the resultset
+should be orderd.
 
 =item return:
-C<$items> is a reference to an array full of hasref which keys are items' table column.
+
+C<$items> is a reference to an array full of hashrefs with columns
+from the "items" table as keys.
 
 =item usage in the perl script:
 
-my %where;
-$where{barcode} = 0001548;
-my $items = GetLostItems( \%where, "homebranch" );
-$template->param(itemsloop => $items);
+my $where = { barcode => '0001548' };
+my $items = GetLostItems( $where, "homebranch" );
+$template->param( itemsloop => $items );
 
 =back
 
@@ -885,18 +892,23 @@ sub GetLostItems {
          	AND itemlost <> 0
           
     ";
+    my @query_parameters;
     foreach my $key (keys %$where) {
-        $query .= " AND " . $key . " LIKE '%" . $where->{$key} . "%'";
+        $query .= " AND $key LIKE ?";
+        push @query_parameters, "%$where->{$key}%";
+    }
+    if ( defined $orderby ) {
+        $query .= ' ORDER BY ?';
+        push @query_parameters, $orderby;
     }
-    $query .= " ORDER BY ".$orderby." " if defined $orderby;
 
     my $sth = $dbh->prepare($query);
-    $sth->execute;
-    my @items;
+    $sth->execute( @query_parameters );
+    my $items;
     while ( my $row = $sth->fetchrow_hashref ){
-        push @items, $row;
+        push @$items, $row;
     }
-    return \@items;
+    return $items;
 }
 
 =head2 GetItemsForInventory
diff --git a/reports/itemslost.pl b/reports/itemslost.pl
index 07b6e3c..9d46f22 100755
--- a/reports/itemslost.pl
+++ b/reports/itemslost.pl
@@ -61,22 +61,12 @@ if ( $get_items ) {
 
     my $items = GetLostItems( \%where, $orderbyfilter ); 
     $template->param(
-        total     => scalar @$items,
-        itemsloop => $items,
-		  get_items => $get_items
-    );
+                     total     => scalar @$items,
+                     itemsloop => $items,
+                     get_items => $get_items
+                 );
 }
 
-# Get the Lost colletion codes
-#my $fw = GetFrameworkCode($biblionumber);
-#$item = GetAuthorisedValues(GetAuthValCode('items.itemlost',$fw),$item->{itemlost}) if GetAuthValCode('items.itemlost',$fw);
-#if ($item->{damaged}) {
-#    $item->{itemdamagedloop}= GetAuthorisedValues(GetAuthValCode('items.damaged',$fw),$item->{damaged}) if GetAuthValCode('items.damaged',$fw);
-#}
-#get collection code description, too
-#my $ccodes = GetAuthorisedValueDesc('','',   'ccode' ,'','','ccode');
-
-
 # getting all branches.
 my $branches = GetBranches;
 my $branch   = C4::Context->userenv->{"branchname"};
-- 
1.5.5.rc0.16.g02b00