Bug 18279: Remove C4::Items::GetLostItems

The JOIN done by this subroutine are not always useful (depending on
item-level_itypes). They also search with LIKE when it is not needed.

Since we have now Koha::Items, we can replace this subroutine with a
call to Koha::Items->search with the correct parameters.

A change in previous behaviours can happen: If a items.itemlost contains
a value that is not defined as a LOST authorised value, the item will
not be displayed. I think it's the expected behaviour, even if it should
not happen in correctly configured installations.

Test plan:
To test with item-level_itypes set to item and biblio:
List the lost items you have on your system, using the different
filters available.
The result table should contain the correct item's info.

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

diff --git a/C4/Items.pm b/C4/Items.pm
index 1d3db7a..f376c7c 100644 (file)
--- a/C4/Items.pm
+++ b/C4/Items.pm
@@ -774,69 +774,6 @@ has copy-and-paste work.
 
 =cut
 
-=head2 GetLostItems
-
-  $items = GetLostItems( $where );
-
-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. For example:
-
-{ barcode    => 'abc123',
-  homebranch => 'CPL',    }
-
-=item return:
-
-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 = { barcode => '0001548' };
-  my $items = GetLostItems( $where );
-  $template->param( itemsloop => $items );
-
-=back
-
-=cut
-
-sub GetLostItems {
-    # Getting input args.
-    my $where   = shift;
-    my $dbh     = C4::Context->dbh;
-
-    my $query   = "
-        SELECT title, author, lib, itemlost, authorised_value, barcode, datelastseen, price, replacementprice, homebranch,
-               itype, itemtype, holdingbranch, location, itemnotes, items.biblionumber as biblionumber, itemcallnumber
-        FROM   items
-            LEFT JOIN biblio ON (items.biblionumber = biblio.biblionumber)
-            LEFT JOIN biblioitems ON (items.biblionumber = biblioitems.biblionumber)
-            LEFT JOIN authorised_values ON (items.itemlost = authorised_values.authorised_value)
-        WHERE
-               authorised_values.category = 'LOST'
-               AND itemlost IS NOT NULL
-               AND itemlost <> 0
-    ";
-    my @query_parameters;
-    foreach my $key (keys %$where) {
-        $query .= " AND $key LIKE ?";
-        push @query_parameters, "%$where->{$key}%";
-    }
-
-    my $sth = $dbh->prepare($query);
-    $sth->execute( @query_parameters );
-    my $items = [];
-    while ( my $row = $sth->fetchrow_hashref ){
-        push @$items, $row;
-    }
-    return $items;
-}
-
 =head2 GetItemsForInventory
 
 ($itemlist, $iTotalRecords) = GetItemsForInventory( {

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/itemslost.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/itemslost.tt
index e02dccd..e3b8e26 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/itemslost.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/itemslost.tt
@@ -1,5 +1,7 @@
+[% USE AuthorisedValues %]
 [% USE Branches %]
 [% USE ColumnsSettings %]
+[% USE KohaDates %]
 [% INCLUDE 'doc-head-open.inc' %]
 <title>Koha &rsaquo; Reports &rsaquo; Lost items</title>
 [% INCLUDE 'doc-head-close.inc' %]
@@ -42,14 +44,14 @@
 [% IF ( get_items ) %]
 
 <div class="results">
-    [% IF ( total ) %]
-        [% total %] lost items found
+    [% IF items.count%]
+        [% items.count %] lost items found
     [% ELSE %]
         No lost items found
     [% END %]
 </div>
 
-    [% IF itemsloop %]
+    [% IF items.count %]
         <table id="lostitems-table">
             <thead>
                 <tr>
@@ -69,25 +71,25 @@
                 </tr>
             </thead>
             <tbody>
-                [% FOREACH itemsloo IN itemsloop %]
+                [% FOREACH item IN items %]
                     <tr>
                         <td>
-                            <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% itemsloo.biblionumber %]" title="[% itemsloo.itemnotes %]">[% itemsloo.title |html %]</a>
+                            <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% item.biblionumber %]" title="[% item.itemnotes %]">[% item.biblio.title |html %]</a>
                         </td>
-                        <td>[% itemsloo.author %]</td>
-                        <td>[% itemsloo.lib %]</td>
+                        <td>[% item.biblio.author %]</td>
+                        <td>[% AuthorisedValues.GetByCode( 'LOST', item.itemlost ) %]</td>
                         <td>
-                            <a href="/cgi-bin/koha/catalogue/moredetail.pl?biblionumber=[% itemsloo.biblionumber %]" title="[% itemsloo.itemnotes %]">[% itemsloo.barcode %]</a>
+                            <a href="/cgi-bin/koha/catalogue/moredetail.pl?biblionumber=[% item.biblionumber %]" title="[% item.itemnotes %]">[% item.barcode %]</a>
                         </td>
-                        <td>[% itemsloo.itemcallnumber %]</td>
-                        <td>[% itemsloo.datelastseen %]</td>
-                        <td>[% itemsloo.price %]</td>
-                        <td>[% itemsloo.replacementprice %]</td>
-                        <td>[% Branches.GetName(itemsloo.homebranch) %]</td>
-                        <td>[% IF ( itemsloo.itype_level ) %][% itemsloo.itype %][% ELSE %][% itemsloo.itemtype %][% END %]</td>
-                        <td>[% Branches.GetName(itemsloo.holdingbranch) %]</td>
-                        <td>[% itemsloo.location %]</td>
-                        <td>[% itemsloo.itemnotes %]</td>
+                        <td>[% item.itemcallnumber %]</td>
+                        <td>[% item.datelastseen | $KohaDates %]</td>
+                        <td>[% item.price %]</td>
+                        <td>[% item.replacementprice %]</td>
+                        <td>[% Branches.GetName(item.homebranch) %]</td>
+                        <td>[% item.effective_itemtype %]</td>
+                        <td>[% Branches.GetName(item.holdingbranch) %]</td>
+                        <td>[% item.location %]</td>
+                        <td>[% item.itemnotes %]</td>
                     </tr>
                 [% END %]
             </tbody>

diff --git a/reports/itemslost.pl b/reports/itemslost.pl
index e789098..75bb067 100755 (executable)
--- a/reports/itemslost.pl
+++ b/reports/itemslost.pl
@@ -50,32 +50,40 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
 my $params = $query->Vars;
 my $get_items = $params->{'get_items'};
 
-if ( $get_items ) {
-    my $branchfilter     = $params->{'branchfilter'}    || undef;
-    my $barcodefilter    = $params->{'barcodefilter'}   || undef;
-    my $itemtypesfilter  = $params->{'itemtypesfilter'} || undef;
+if ($get_items) {
+    my $branchfilter     = $params->{'branchfilter'}     || undef;
+    my $barcodefilter    = $params->{'barcodefilter'}    || undef;
+    my $itemtypesfilter  = $params->{'itemtypesfilter'}  || undef;
     my $loststatusfilter = $params->{'loststatusfilter'} || undef;
 
-    my %where;
-    $where{'homebranch'}       = $branchfilter    if defined $branchfilter;
-    $where{'barcode'}          = $barcodefilter   if defined $barcodefilter;
-    $where{'authorised_value'} = $loststatusfilter if defined $loststatusfilter;
-
-    my $itype = C4::Context->preference('item-level_itypes') ? "itype" : "itemtype";
-    $where{$itype}            = $itemtypesfilter if defined $itemtypesfilter;
-
-    my $items = GetLostItems( \%where );
-    foreach my $it (@$items) {
-        $it->{'datelastseen'} = eval { output_pref( { dt => dt_from_string( $it->{'datelastseen'} ), dateonly => 1 }); }
-                   if ( $it->{'datelastseen'} );
+    my $params = {
+        ( $branchfilter ? ( homebranch => $branchfilter ) : () ),
+        (
+            $loststatusfilter
+            ? ( itemlost => $loststatusfilter )
+            : ( itemlost => { '!=' => 0 } )
+        ),
+        ( $barcodefilter ? ( barcode => { like => "%$barcodefilter%" } ) : () ),
+    };
+
+    my $attributes;
+    if ($itemtypesfilter) {
+        if ( C4::Context->preference('item-level_itypes') ) {
+            $params->{itype} = $itemtypesfilter;
+        }
+        else {
+            # We want a join on biblioitems
+            $attributes = { join => 'biblioitem' };
+            $params->{'biblioitem.itemtype'} = $itemtypesfilter;
+        }
     }
 
+    my $items = Koha::Items->search( $params, $attributes );
+
     $template->param(
-                     total       => scalar @$items,
-                     itemsloop   => $items,
-                     get_items   => $get_items,
-                     itype_level => C4::Context->preference('item-level_itypes'),
-                 );
+        items     => $items,
+        get_items => $get_items,
+    );
 }
 
 # getting all itemtypes