From 5194e46d39072aef6cedb94209ac852b9948103b Mon Sep 17 00:00:00 2001 From: Mark Tompsett Date: Fri, 24 Apr 2015 00:40:46 -0400 Subject: [PATCH] [Signed-off] Bug 14057 - A Rough Start MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch is a rough start. I believe it runs exponentially faster, but its equality to the previous version needs to be tested before I clean it up to acceptable standards. Nested hashes of hashes was being a debugging nightmare. Moved the SQL select to C4::Koha. Changed the GetItemsForInventory to have a hashref parameter. Added interface, in case there is a need for 'opac' vs. 'staff'. Added t/db_dependent/Items/GetItemsForInventory.t Added t/db_dependent/Koha/GetKohaAuthorisedValuesMapping.t --- How I tested: - Apply patch - Do some full and some filtered Inventories, behaviour is the same (but faster as without patch) - Run both tests Signed-off-by: Marc Véron --- C4/Items.pm | 41 ++++- C4/Koha.pm | 45 ++++++ t/db_dependent/Items/GetItemsForInventory.t | 156 ++++++++++++++++++++ .../Koha/GetKohaAuthorisedValuesMapping.t | 54 +++++++ tools/inventory.pl | 30 +++- 5 files changed, 316 insertions(+), 10 deletions(-) create mode 100755 t/db_dependent/Items/GetItemsForInventory.t create mode 100755 t/db_dependent/Koha/GetKohaAuthorisedValuesMapping.t diff --git a/C4/Items.pm b/C4/Items.pm index 11ff4ca..d1a866b 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -1024,7 +1024,20 @@ sub GetLostItems { =head2 GetItemsForInventory -($itemlist, $iTotalRecords) = GetItemsForInventory($minlocation, $maxlocation, $location, $itemtype, $ignoreissued, $datelastseen, $branchcode, $offset, $size, $statushash); +($itemlist, $iTotalRecords) = GetItemsForInventory( { + minlocation => $minlocation, + maxlocation => $maxlocation, + location => $location, + itemtype => $itemtype, + ignoreissued => $ignoreissued, + datelastseen => $datelastseen, + branchcode => $branchcode, + branch => $branch, + offset => $offset, + size => $size, + stautshash => $statushash + interface => $interface, +} ); Retrieve a list of title/authors/barcode/callnumber, for biblio inventory. @@ -1042,7 +1055,20 @@ $iTotalRecords is the number of rows that would have been returned without the $ =cut sub GetItemsForInventory { - my ( $minlocation, $maxlocation,$location, $itemtype, $ignoreissued, $datelastseen, $branchcode, $branch, $offset, $size, $statushash ) = @_; + my ( $parameters ) = @_; + my $minlocation = $parameters->{'minlocation'} // ''; + my $maxlocation = $parameters->{'maxlocation'} // ''; + my $location = $parameters->{'location'} // ''; + my $itemtype = $parameters->{'itemtype'} // ''; + my $ignoreissued = $parameters->{'ignoreissued'} // ''; + my $datelastseen = $parameters->{'datelastseen'} // ''; + my $branchcode = $parameters->{'branchcode'} // ''; + my $branch = $parameters->{'branch'} // ''; + my $offset = $parameters->{'offset'} // ''; + my $size = $parameters->{'size'} // ''; + my $statushash = $parameters->{'statushash'} // ''; + my $interface = $parameters->{'interface'} // ''; + my $dbh = C4::Context->dbh; my ( @bind_params, @where_strings ); @@ -1121,16 +1147,15 @@ sub GetItemsForInventory { $sth->execute( @bind_params ); my ($iTotalRecords) = $sth->fetchrow_array(); + my $avmapping = C4::Koha::GetKohaAuthorisedValuesMapping( { + interface => $interface + } ); foreach my $row (@$tmpresults) { # Auth values foreach (keys %$row) { - # If the koha field is mapped to a marc field - my ($f, $sf) = GetMarcFromKohaField("items.$_", $row->{'frameworkcode'}); - if ($f and $sf) { - # We replace the code with it's description - my $authvals = C4::Koha::GetKohaAuthorisedValuesFromField($f, $sf, $row->{'frameworkcode'}); - $row->{$_} = $authvals->{$row->{$_}} if defined $authvals->{$row->{$_}}; + if (defined($avmapping->{"items.$_,".$row->{'frameworkcode'}.",".$row->{$_}})) { + $row->{$_} = $avmapping->{"items.$_,".$row->{'frameworkcode'}.",".$row->{$_}}; } } push @results, $row; diff --git a/C4/Koha.pm b/C4/Koha.pm index 23fbfee..cb0ed5a 100644 --- a/C4/Koha.pm +++ b/C4/Koha.pm @@ -62,6 +62,7 @@ BEGIN { &IsAuthorisedValueCategory &GetKohaAuthorisedValues &GetKohaAuthorisedValuesFromField + &GetKohaAuthorisedValuesMapping &GetKohaAuthorisedValueLib &GetAuthorisedValueByCode &GetKohaImageurlFromAuthorisedValues @@ -1327,6 +1328,50 @@ sub GetKohaAuthorisedValuesFromField { } } +=head2 GetKohaAuthorisedValuesMapping + +Takes a hash as a parameter. The interface key indicates the +description to use in the mapping. + +Returns hashref of: + "{kohafield},{frameworkcode},{authorised_value}" => "{description}" +for all the kohafields, frameworkcodes, and authorised values. + +Returns undef if nothing is found. + +=cut + +sub GetKohaAuthorisedValuesMapping { + my ($parameter) = @_; + my $interface = $parameter->{'interface'} // ''; + + my $query_mapping = q{ +SELECT TA.kohafield,TA.authorised_value AS category, + TA.frameworkcode,TB.authorised_value, + IF(TB.lib_opac>'',TB.lib_opac,TB.lib) AS OPAC, + TB.lib AS Intranet,TB.lib_opac +FROM marc_subfield_structure AS TA JOIN + authorised_values as TB ON + TA.authorised_value=TB.category +WHERE TA.kohafield>'' AND TA.authorised_value>''; + }; + my $dbh = C4::Context->dbh; + my $sth = $dbh->prepare($query_mapping); + $sth->execute(); + my $avmapping; + if ($interface eq 'opac') { + while (my $row = $sth->fetchrow_hashref) { + $avmapping->{$row->{kohafield}.",".$row->{frameworkcode}.",".$row->{authorised_value}} = $row->{OPAC}; + } + } + else { + while (my $row = $sth->fetchrow_hashref) { + $avmapping->{$row->{kohafield}.",".$row->{frameworkcode}.",".$row->{authorised_value}} = $row->{Intranet}; + } + } + return $avmapping; +} + =head2 xml_escape my $escaped_string = C4::Koha::xml_escape($string); diff --git a/t/db_dependent/Items/GetItemsForInventory.t b/t/db_dependent/Items/GetItemsForInventory.t new file mode 100755 index 0000000..3a0a4d3 --- /dev/null +++ b/t/db_dependent/Items/GetItemsForInventory.t @@ -0,0 +1,156 @@ +#!/usr/bin/perl +# +# This file is part of Koha. +# +# Copyright (c) 2015 Mark Tompsett +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; +use Test::More tests => 6; + +$| = 1; + +BEGIN { + use_ok('C4::Context'); + use_ok('C4::Items'); + use_ok('C4::Biblio'); + use_ok('C4::Koha'); +} + +can_ok('C4::Items','GetItemsForInventory'); + +my $dbh = C4::Context->dbh; +$dbh->{AutoCommit} = 0; +$dbh->{RaiseError} = 1; + +diag("This could take a while for large datasets..."); + +my ($oldResults, $oldCount) = OldWay($dbh); +my ($newResults, $newCount) = GetItemsForInventory( { interface => 'staff' } ); + +is_deeply($newResults,$oldResults,"Inventory results unchanged."); + +$dbh->rollback; + +sub OldWay { + my ($tdbh) = @_; + my $ldbh = $tdbh; + my $minlocation = ''; + my $maxlocation = ''; + my $location = ''; + my $itemtype = ''; + my $ignoreissued = ''; + my $datelastseen = ''; + my $branchcode = ''; + my $branch = ''; + my $offset = ''; + my $size = ''; + my $statushash = ''; + my $interface = ''; + + my ( @bind_params, @where_strings ); + + my $select_columns = q{ + SELECT items.itemnumber, barcode, itemcallnumber, title, author, biblio.biblionumber, biblio.frameworkcode, datelastseen, homebranch, location, notforloan, damaged, itemlost, withdrawn, stocknumber + }; + my $select_count = q{SELECT COUNT(*)}; + my $query = q{ + FROM items + LEFT JOIN biblio ON items.biblionumber = biblio.biblionumber + LEFT JOIN biblioitems on items.biblionumber = biblioitems.biblionumber + }; + if ($statushash){ + for my $authvfield (keys %$statushash){ + if ( scalar @{$statushash->{$authvfield}} > 0 ){ + my $joinedvals = join ',', @{$statushash->{$authvfield}}; + push @where_strings, "$authvfield in (" . $joinedvals . ")"; + } + } + } + + if ($minlocation) { + push @where_strings, 'itemcallnumber >= ?'; + push @bind_params, $minlocation; + } + + if ($maxlocation) { + push @where_strings, 'itemcallnumber <= ?'; + push @bind_params, $maxlocation; + } + + if ($datelastseen) { + $datelastseen = format_date_in_iso($datelastseen); + push @where_strings, '(datelastseen < ? OR datelastseen IS NULL)'; + push @bind_params, $datelastseen; + } + + if ( $location ) { + push @where_strings, 'items.location = ?'; + push @bind_params, $location; + } + + if ( $branchcode ) { + if($branch eq "homebranch"){ + push @where_strings, 'items.homebranch = ?'; + }else{ + push @where_strings, 'items.holdingbranch = ?'; + } + push @bind_params, $branchcode; + } + + if ( $itemtype ) { + push @where_strings, 'biblioitems.itemtype = ?'; + push @bind_params, $itemtype; + } + + if ( $ignoreissued) { + $query .= "LEFT JOIN issues ON items.itemnumber = issues.itemnumber "; + push @where_strings, 'issues.date_due IS NULL'; + } + + if ( @where_strings ) { + $query .= 'WHERE '; + $query .= join ' AND ', @where_strings; + } + $query .= ' ORDER BY items.cn_sort, itemcallnumber, title'; + my $count_query = $select_count . $query; + $query .= " LIMIT $offset, $size" if ($offset and $size); + $query = $select_columns . $query; + my $sth = $ldbh->prepare($query); + $sth->execute( @bind_params ); + + my @results = (); + my $tmpresults = $sth->fetchall_arrayref({}); + $sth = $ldbh->prepare( $count_query ); + $sth->execute( @bind_params ); + my ($iTotalRecords) = $sth->fetchrow_array(); + + foreach my $row (@$tmpresults) { + + # Auth values + foreach my $field (sort keys %$row) { + # If the koha field is mapped to a marc field + my ($f, $sf) = C4::Biblio::GetMarcFromKohaField("items.$field", $row->{'frameworkcode'}); + if (defined($f) and defined($sf)) { + # We replace the code with it's description + my $authvals = C4::Koha::GetKohaAuthorisedValuesFromField($f, $sf, $row->{'frameworkcode'}); + $row->{$field} = $authvals->{$row->{$field}} if defined $authvals && defined $row->{$field} && defined $authvals->{$row->{$field}}; + } + } + push @results, $row; + } + + return (\@results, $iTotalRecords); +} diff --git a/t/db_dependent/Koha/GetKohaAuthorisedValuesMapping.t b/t/db_dependent/Koha/GetKohaAuthorisedValuesMapping.t new file mode 100755 index 0000000..2cdb3b6 --- /dev/null +++ b/t/db_dependent/Koha/GetKohaAuthorisedValuesMapping.t @@ -0,0 +1,54 @@ +#!/usr/bin/perl +# +# This file is part of Koha. +# +# Copyright (c) 2015 Mark Tompsett +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use C4::Context; + +use Test::More tests => 8; + +BEGIN { + use_ok('C4::Context'); + use_ok('C4::Koha'); +} + +can_ok('C4::Koha','GetKohaAuthorisedValuesMapping'); + +my $avMappingOPAC; +my $avMappingStaff; +my $avMappingUndef1; +my $avMappingUndef2; +my $avMappingUndef3; +SKIP: { + my $dbh = C4::Context->dbh; + my $count = $dbh->selectrow_arrayref("SELECT COUNT(*) FROM marc_subfield_structure WHERE kohafield LIKE 'item%';"); + skip "Lacking item mappings in marc_subfield_structure",5 unless ($count && $count->[0]>0); + $count = $dbh->selectrow_arrayref("SELECT COUNT(*) FROM authorised_values;"); + skip "Lacking authorised_values",5 unless ($count && $count->[0]>0); + $avMappingOPAC = GetKohaAuthorisedValuesMapping( { interface => 'opac' }); + $avMappingStaff = GetKohaAuthorisedValuesMapping( { interface => 'staff' }); + $avMappingUndef1 = GetKohaAuthorisedValuesMapping( { interface => undef }); + $avMappingUndef2 = GetKohaAuthorisedValuesMapping( { } ); + $avMappingUndef3 = GetKohaAuthorisedValuesMapping(); + is_deeply($avMappingUndef1,$avMappingStaff,"Undefined interface = Staff test 1"); + is_deeply($avMappingUndef2,$avMappingStaff,"Undefined interface = Staff test 2"); + is_deeply($avMappingUndef3,$avMappingStaff,"Undefined interface = Staff test 3"); + isnt($avMappingOPAC ,undef,"OPAC has a mapping"); + isnt($avMappingStaff,undef,"Staff has a mapping"); +} diff --git a/tools/inventory.pl b/tools/inventory.pl index d463d19..015d0be 100755 --- a/tools/inventory.pl +++ b/tools/inventory.pl @@ -234,10 +234,36 @@ if ( $markseen or $op ) { # We use datelastseen only when comparing the results to the barcode file. my $paramdatelastseen = ($compareinv2barcd) ? $datelastseen : ''; - ($inventorylist, $totalrecords) = GetItemsForInventory($minlocation, $maxlocation, $location, $itemtype, $ignoreissued, $paramdatelastseen, $branchcode, $branch, 0, undef, $staton); + ($inventorylist, $totalrecords) = GetItemsForInventory( { + minlocation => $minlocation, + maxlocation => $maxlocation, + location => $location, + itemtype => $itemtype, + ignoreissued => $ignoreissued, + datelastseen => $paramdatelastseen, + branchcode => $branchcode, + branch => $branch, + offset => 0, + size => undef, + statushash => $staton, + interface => 'staff', + } ); # For the items that may be marked as "wrong place", we only check the location (callnumbers, location and branch) - ($wrongplacelist, $totalrecords) = GetItemsForInventory($minlocation, $maxlocation, $location, undef, undef, undef, $branchcode, $branch, 0, undef, undef); + ($wrongplacelist, $totalrecords) = GetItemsForInventory( { + minlocation => $minlocation, + maxlocation => $maxlocation, + location => $location, + itemtype => undef, + ignoreissued => undef, + datelastseen => undef, + branchcode => $branchcode, + branch => $branch, + offset => 0, + size => undef, + statushash => undef, + interface => 'staff', + } ); } -- 1.7.10.4