From 7c51162957d6268174a5925858cd07ed78eb1123 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Fri, 12 May 2017 10:59:11 +0200 Subject: [PATCH] Bug 18400: [QA Follow-up] Move sort outside the loop Content-Type: text/plain; charset=utf-8 GetItemTypesCategorized can return descriptions that are still undef since Authorized values does not enforce a description in lib and lib_opac. When I add one ITEMTYPECAT auth value without descriptions, I can still generate the string comparison warnings on the itemtypes sort. In order to prevent the warning, we should add an empty string in the assignment on line 229. We do not need to copy the itemtypes hash if we move the sort outside the @advanced_search_types foreach. There is no need to sort it more than once. Note that I did not see any reasons btw for corruption of the structure inside this loop. Note: If we use ITEMTYPECAT without descriptions, we should probably leave them out. No need to show a checkbox without description on Advanced Search, but I would recommend to solve that on its own report. The whole ITEMTYPECAT functionality has imo not been designed properly. Signed-off-by: Marcel de Rooy --- opac/opac-search.pl | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/opac/opac-search.pl b/opac/opac-search.pl index 3d034ab..7b10ac3 100755 --- a/opac/opac-search.pl +++ b/opac/opac-search.pl @@ -224,13 +224,11 @@ $template->param(search_languages_loop => $languages_limit_loop,); my $itemtypes = GetItemTypesCategorized; # add translated_description to itemtypes foreach my $itemtype ( keys %{$itemtypes} ) { - # Itemtypes search categories don't have (yet) translated descriptions, they are auth values + # Itemtypes search categories don't have (yet) translated descriptions, they are auth values (and could still have no descriptions too BZ 18400) my $translated_description = getitemtypeinfo( $itemtype, 'opac' )->{translated_description}; - $itemtypes->{$itemtype}->{translated_description} = - ( $translated_description ) ? $translated_description : $itemtypes->{$itemtype}->{description}; + $itemtypes->{$itemtype}->{translated_description} = $translated_description || $itemtypes->{$itemtype}->{description} || q{}; } -my $itemtypes_copy = { %$itemtypes }; #Sometime itemtypes can be corrupted in advanced_srch_type loop - #Making a copy ensure it is clean + # the index parameter is different for item-level itemtypes my $itype_or_itemtype = (C4::Context->preference("item-level_itypes"))?'itype':'itemtype'; my @advancedsearchesloop; @@ -250,13 +248,13 @@ if ( $yaml =~ /\S/ ) { } } +my @sorted_itemtypes = sort { $itemtypes->{$a}->{translated_description} cmp $itemtypes->{$b}->{translated_description} } keys %$itemtypes; foreach my $advanced_srch_type (@advanced_search_types) { $advanced_srch_type =~ s/^\s*//; $advanced_srch_type =~ s/\s*$//; if ($advanced_srch_type eq 'itemtypes') { # itemtype is a special case, since it's not defined in authorized values my @itypesloop; - my @sorted_itemtypes = ( sort {$itemtypes->{$a}->{translated_description} cmp $itemtypes->{$b}->{translated_description} } keys %$itemtypes_copy ); foreach my $thisitemtype ( @sorted_itemtypes ) { next if $hidingrules->{itype} && any { $_ eq $thisitemtype } @{$hidingrules->{itype}}; next if $hidingrules->{itemtype} && any { $_ eq $thisitemtype } @{$hidingrules->{itemtype}}; -- 2.1.4