From f0086f9b822c396d7cf26dde8d497dd03f534630 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 15 Mar 2017 17:47:13 -0300 Subject: [PATCH] Bug 18279: Remove C4::Items::GetLostItems MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 https://bugs.koha-community.org/show_bug.cgi?id=18079 --- C4/Items.pm | 63 ---------------------- .../prog/en/modules/reports/itemslost.tt | 36 +++++++------ reports/itemslost.pl | 50 +++++++++-------- 3 files changed, 48 insertions(+), 101 deletions(-) diff --git a/C4/Items.pm b/C4/Items.pm index da3c372..fce23ca 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -778,69 +778,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 c6f9661..c0bed35 100644 --- 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' %] Koha › Reports › Lost items [% INCLUDE 'doc-head-close.inc' %] @@ -42,14 +44,14 @@ [% IF ( get_items ) %]
- [% IF ( total ) %] - [% total %] lost items found + [% IF items.count%] + [% items.count %] lost items found [% ELSE %] No lost items found [% END %]
- [% IF itemsloop %] + [% IF items.count %] @@ -69,25 +71,25 @@ - [% FOREACH itemsloo IN itemsloop %] + [% FOREACH item IN items %] - - + + - - - - - - - - - + + + + + + + + + [% END %] diff --git a/reports/itemslost.pl b/reports/itemslost.pl index 00f2b99..cd97b07 100755 --- a/reports/itemslost.pl +++ b/reports/itemslost.pl @@ -51,32 +51,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 -- 2.1.4
- [% itemsloo.title |html %] + [% item.biblio.title |html %] [% itemsloo.author %][% itemsloo.lib %][% item.biblio.author %][% AuthorisedValues.GetByCode( 'LOST', item.itemlost ) %] - [% itemsloo.barcode %] + [% item.barcode %] [% itemsloo.itemcallnumber %][% itemsloo.datelastseen %][% itemsloo.price %][% itemsloo.replacementprice %][% Branches.GetName(itemsloo.homebranch) %][% IF ( itemsloo.itype_level ) %][% itemsloo.itype %][% ELSE %][% itemsloo.itemtype %][% END %][% Branches.GetName(itemsloo.holdingbranch) %][% itemsloo.location %][% itemsloo.itemnotes %][% item.itemcallnumber %][% item.datelastseen | $KohaDates %][% item.price %][% item.replacementprice %][% Branches.GetName(item.homebranch) %][% item.effective_itemtype %][% Branches.GetName(item.holdingbranch) %][% item.location %][% item.itemnotes %]