From a84cd7f506dfd32b9c2489369ae54828abacf2a2 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 10 Aug 2016 12:29:57 +0100 Subject: [PATCH] [PASSED QA] Bug 17251: Koha::AuthorisedValues - Remove GetKohaAuthorisedValuesMapping This subroutine was only used once in GetItemsForInventory. It can be replaced with a quite simple search on AV join on authorised_value_categories and marc_subfield_structures tables. Note that the "interface" parameter was always set to "staff" and was useless. Test plan: Play with the inventory and confirm that the AV descriptions are correctly displayed. The tests in t/db_dependent/Items/GetItemsForInventory.t cover this change and should still pass. Signed-off-by: Claire Gravely Signed-off-by: Martin Renvoize --- C4/Items.pm | 18 ++++++-- C4/Koha.pm | 46 +----------------- Koha/AuthorisedValues.pm | 6 ++- t/db_dependent/Items/GetItemsForInventory.t | 3 +- .../Koha/GetKohaAuthorisedValuesMapping.t | 54 ---------------------- tools/inventory.pl | 2 - 6 files changed, 19 insertions(+), 110 deletions(-) delete mode 100755 t/db_dependent/Koha/GetKohaAuthorisedValuesMapping.t diff --git a/C4/Items.pm b/C4/Items.pm index 3ae5eca..7bd21c1 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -1047,7 +1047,6 @@ sub GetLostItems { offset => $offset, size => $size, statushash => $statushash, - interface => $interface, } ); Retrieve a list of title/authors/barcode/callnumber, for biblio inventory. @@ -1078,7 +1077,6 @@ sub GetItemsForInventory { 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 ); @@ -1158,9 +1156,19 @@ sub GetItemsForInventory { $sth->execute( @bind_params ); my ($iTotalRecords) = $sth->fetchrow_array(); - my $avmapping = C4::Koha::GetKohaAuthorisedValuesMapping( { - interface => $interface - } ); + my @avs = Koha::AuthorisedValues->search( + { 'marc_subfield_structures.kohafield' => { '>' => '' }, + 'me.authorised_value' => { '>' => '' }, + }, + { join => { category => 'marc_subfield_structures' }, + distinct => ['marc_subfield_structures.kohafield, me.category, frameworkcode, me.authorised_value'], + '+select' => [ 'marc_subfield_structures.kohafield', 'marc_subfield_structures.frameworkcode', 'me.authorised_value', 'me.lib' ], + '+as' => [ 'kohafield', 'frameworkcode', 'authorised_value', 'lib' ], + } + ); + + my $avmapping = { map { $_->get_column('kohafield') . ',' . $_->get_column('frameworkcode') . ',' . $_->get_column('authorised_value') => $_->get_column('lib') } @avs }; + foreach my $row (@$tmpresults) { # Auth values diff --git a/C4/Koha.pm b/C4/Koha.pm index fa8245f..42e868e 100644 --- a/C4/Koha.pm +++ b/C4/Koha.pm @@ -26,6 +26,7 @@ use strict; use C4::Context; use Koha::Caches; use Koha::DateUtils qw(dt_from_string); +use Koha::AuthorisedValues; use Koha::Libraries; use Koha::MarcSubfieldStructures; use DateTime::Format::MySQL; @@ -54,7 +55,6 @@ BEGIN { &GetAuthorisedValues &GetAuthorisedValueCategories &GetKohaAuthorisedValues - &GetKohaAuthorisedValuesMapping &GetAuthorisedValueByCode &GetNormalizedUPC &GetNormalizedISBN @@ -1014,50 +1014,6 @@ sub GetKohaAuthorisedValues { return $values; } -=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/Koha/AuthorisedValues.pm b/Koha/AuthorisedValues.pm index 4474030..c4484cb 100644 --- a/Koha/AuthorisedValues.pm +++ b/Koha/AuthorisedValues.pm @@ -45,7 +45,7 @@ my @objects = Koha::AuthorisedValues->search($params); =cut sub search { - my ( $self, $params ) = @_; + my ( $self, $params, $attributes ) = @_; my $branchcode = $params->{branchcode}; delete( $params->{branchcode} ); @@ -60,7 +60,9 @@ sub search { } : {}; my $join = $branchcode ? { join => 'authorised_values_branches' } : {}; - return $self->SUPER::search( { %$params, %$or, }, $join ); + $attributes //= {}; + $attributes = { %$attributes, %$join }; + return $self->SUPER::search( { %$params, %$or, }, $attributes ); } sub search_by_marc_field { diff --git a/t/db_dependent/Items/GetItemsForInventory.t b/t/db_dependent/Items/GetItemsForInventory.t index dfb2175..452b15e 100755 --- a/t/db_dependent/Items/GetItemsForInventory.t +++ b/t/db_dependent/Items/GetItemsForInventory.t @@ -38,7 +38,7 @@ $dbh->{AutoCommit} = 0; $dbh->{RaiseError} = 1; my ($oldResults, $oldCount) = OldWay($dbh); -my ($newResults, $newCount) = GetItemsForInventory( { interface => 'staff' } ); +my ($newResults, $newCount) = GetItemsForInventory; is_deeply($newResults,$oldResults,"Inventory results unchanged."); @@ -58,7 +58,6 @@ sub OldWay { my $offset = ''; my $size = ''; my $statushash = ''; - my $interface = ''; my ( @bind_params, @where_strings ); diff --git a/t/db_dependent/Koha/GetKohaAuthorisedValuesMapping.t b/t/db_dependent/Koha/GetKohaAuthorisedValuesMapping.t deleted file mode 100755 index 2cdb3b6..0000000 --- a/t/db_dependent/Koha/GetKohaAuthorisedValuesMapping.t +++ /dev/null @@ -1,54 +0,0 @@ -#!/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 ef59268..3372a32 100755 --- a/tools/inventory.pl +++ b/tools/inventory.pl @@ -246,7 +246,6 @@ if ( $markseen or $op ) { 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) @@ -262,7 +261,6 @@ if ( $markseen or $op ) { offset => 0, size => undef, statushash => undef, - interface => 'staff', } ); } -- 2.1.4