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>
This commit is contained in:
Jonathan Druart 2017-03-15 17:47:13 -03:00
parent f466a856a9
commit 8e7718ee65
3 changed files with 47 additions and 100 deletions

View file

@ -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( {

View file

@ -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>

View file

@ -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 $params = {
( $branchfilter ? ( homebranch => $branchfilter ) : () ),
(
$loststatusfilter
? ( itemlost => $loststatusfilter )
: ( itemlost => { '!=' => 0 } )
),
( $barcodefilter ? ( barcode => { like => "%$barcodefilter%" } ) : () ),
};
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 $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