From 88f6b2f943e923e9aa11ebe5d81d40fa34b94c49 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Wed, 13 Jun 2018 15:47:37 +0300 Subject: [PATCH] Bug 20930: First example - Change OpacHiddenItems to type YAML To test: 1. prove t/db_dependent/Items.t 2. Check that desired items are still hidden in OPAC by using OpacHiddenItems as documented here: https://wiki.koha-community.org/wiki/OpacHiddenItems --- C4/Biblio.pm | 2 +- C4/Items.pm | 13 ++---------- ...HiddenItems-system-preference-type-to-YAML.perl | 8 ++++++++ installer/data/mysql/sysprefs.sql | 2 +- opac/opac-search.pl | 13 ++---------- t/db_dependent/Items.t | 23 ++++++++++------------ 6 files changed, 24 insertions(+), 37 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/Bug_20930-Set-OpacHiddenItems-system-preference-type-to-YAML.perl diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 0c5a917..c3f167a 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -2803,7 +2803,7 @@ sub EmbedItemsInMarcBiblio { my ( $itemtag, $itemsubfield ) = GetMarcFromKohaField( "items.itemnumber", $frameworkcode ); my @items; my $opachiddenitems = $opac - && ( C4::Context->preference('OpacHiddenItems') !~ /^\s*$/ ); + && ( C4::Context->preference('OpacHiddenItems') ); require C4::Items; while ( my ($itemnumber) = $sth->fetchrow_array ) { next if @$itemnumbers and not grep { $_ == $itemnumber } @$itemnumbers; diff --git a/C4/Items.pm b/C4/Items.pm index bf464c3..e644af2 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -1409,17 +1409,8 @@ sub GetHiddenItemnumbers { my (@items) = @_; my @resultitems; - my $yaml = C4::Context->preference('OpacHiddenItems'); - return () if (! $yaml =~ /\S/ ); - $yaml = "$yaml\n\n"; # YAML is anal on ending \n. Surplus does not hurt - my $hidingrules; - eval { - $hidingrules = YAML::Load($yaml); - }; - if ($@) { - warn "Unable to parse OpacHiddenItems syspref : $@"; - return (); - } + my $hidingrules = C4::Context->preference('OpacHiddenItems'); + return () if !$hidingrules or ref($hidingrules) ne 'HASH'; my $dbh = C4::Context->dbh; # For each item diff --git a/installer/data/mysql/atomicupdate/Bug_20930-Set-OpacHiddenItems-system-preference-type-to-YAML.perl b/installer/data/mysql/atomicupdate/Bug_20930-Set-OpacHiddenItems-system-preference-type-to-YAML.perl new file mode 100644 index 0000000..b5472aa --- /dev/null +++ b/installer/data/mysql/atomicupdate/Bug_20930-Set-OpacHiddenItems-system-preference-type-to-YAML.perl @@ -0,0 +1,8 @@ +$DBversion = 'XXX'; # will be replaced by the RM +if( CheckVersion( $DBversion ) ) { + $dbh->do( "UPDATE systempreferences set type='YAML' WHERE variable='OpacHiddenItems'" ); + + # Always end with this (adjust the bug info) + SetVersion( $DBversion ); + print "Upgrade to $DBversion done (Bug 20930: Set OpacHiddenItems system preference type to YAML)\n"; +} diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index 2ea76a4..79df940 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -339,7 +339,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('OPACFinesTab','1','','If OFF the patron fines tab in the OPAC is disabled.','YesNo'), ('OPACFRBRizeEditions','0','','If ON, the OPAC will query one or more ISBN web services for associated ISBNs and display an Editions tab on the details pages','YesNo'), ('opacheader','','70|10','Add HTML to be included as a custom header in the OPAC','Textarea'), -('OpacHiddenItems','','','This syspref allows to define custom rules for hiding specific items at opac. See docs/opac/OpacHiddenItems.txt for more informations.','Textarea'), +('OpacHiddenItems','','','This syspref allows to define custom rules for hiding specific items at opac. See docs/opac/OpacHiddenItems.txt for more informations.','YAML'), ('OpacHighlightedWords','1','','If Set, then queried words are higlighted in OPAC','YesNo'), ('OPACHoldingsDefaultSortField','first_column','first_column|homebranch|holdingbranch','Default sort field for the holdings table at the OPAC','choice'), ('OPACHoldsIfAvailableAtPickup','1','','Allow to pickup up holds at libraries where the item is available','YesNo'), diff --git a/opac/opac-search.pl b/opac/opac-search.pl index 1d7615b..3d6a78b 100755 --- a/opac/opac-search.pl +++ b/opac/opac-search.pl @@ -240,17 +240,8 @@ my $cnt; my $advanced_search_types = C4::Context->preference("AdvancedSearchTypes") || "itemtypes"; my @advanced_search_types = split(/\|/, $advanced_search_types); -my $hidingrules = {}; -my $yaml = C4::Context->preference('OpacHiddenItems'); -if ( $yaml =~ /\S/ ) { - $yaml = "$yaml\n\n"; # YAML expects trailing newline. Surplus does not hurt. - eval { - $hidingrules = YAML::Load($yaml); - }; - if ($@) { - warn "Unable to parse OpacHiddenItems syspref : $@"; - } -} +my $hidingrules = C4::Context->preference('OpacHiddenItems'); +$hidingrules = {} if !$hidingrules or ref($hidingrules) ne 'HASH'; my @sorted_itemtypes = sort { $itemtypes->{$a}->{translated_description} cmp $itemtypes->{$b}->{translated_description} } keys %$itemtypes; foreach my $advanced_srch_type (@advanced_search_types) { diff --git a/t/db_dependent/Items.t b/t/db_dependent/Items.t index ef09873..dd3a7a7 100755 --- a/t/db_dependent/Items.t +++ b/t/db_dependent/Items.t @@ -200,31 +200,29 @@ subtest 'GetHiddenItemnumbers tests' => sub { "Hidden items list undef if OpacHiddenItems empty"); # Blank spaces - t::lib::Mocks::mock_preference('OpacHiddenItems',' '); + t::lib::Mocks::mock_preference('OpacHiddenItems', {}); ok( scalar GetHiddenItemnumbers( @items ) == 0, "Hidden items list empty if OpacHiddenItems only contains blanks"); # One variable / value - $opachiddenitems = " - withdrawn: [1]"; + $opachiddenitems = { withdrawn => [1] }; t::lib::Mocks::mock_preference( 'OpacHiddenItems', $opachiddenitems ); @hidden = GetHiddenItemnumbers( @items ); ok( scalar @hidden == 1, "Only one hidden item"); is( $hidden[0], $item1_itemnumber, "withdrawn=1 is hidden"); # One variable, two values - $opachiddenitems = " - withdrawn: [1,0]"; + $opachiddenitems = { withdrawn => [1,0] }; t::lib::Mocks::mock_preference( 'OpacHiddenItems', $opachiddenitems ); @hidden = GetHiddenItemnumbers( @items ); ok( scalar @hidden == 2, "Two items hidden"); is_deeply( \@hidden, \@itemnumbers, "withdrawn=1 and withdrawn=0 hidden"); # Two variables, a value each - $opachiddenitems = " - withdrawn: [1] - homebranch: [$library2->{branchcode}] - "; + $opachiddenitems = { + withdrawn => [1], + homebranch => [$library2->{branchcode}] + }; t::lib::Mocks::mock_preference( 'OpacHiddenItems', $opachiddenitems ); @hidden = GetHiddenItemnumbers( @items ); ok( scalar @hidden == 2, "Two items hidden"); @@ -616,8 +614,7 @@ subtest 'C4::Biblio::EmbedItemsInMarcBiblio' => sub { @items = $record->field($itemfield); is( scalar @items, $number_of_items, 'Should return all items for opac' ); - my $opachiddenitems = " - homebranch: ['$library1->{branchcode}']"; + my $opachiddenitems = { homebranch => [$library1->{branchcode}] }; t::lib::Mocks::mock_preference( 'OpacHiddenItems', $opachiddenitems ); C4::Biblio::EmbedItemsInMarcBiblio( $record, $biblionumber ); @@ -634,8 +631,8 @@ subtest 'C4::Biblio::EmbedItemsInMarcBiblio' => sub { 'For OPAC, the pref OpacHiddenItems should have been take into account. Only items with homebranch ne CPL should have been embedded' ); - $opachiddenitems = " - homebranch: ['$library1->{branchcode}', '$library2->{branchcode}']"; + $opachiddenitems = { + homebranch => [$library1->{branchcode}, $library2->{branchcode}] }; t::lib::Mocks::mock_preference( 'OpacHiddenItems', $opachiddenitems ); C4::Biblio::EmbedItemsInMarcBiblio( $record, $biblionumber, undef, 1 ); @items = $record->field($itemfield); -- 2.7.4