From cb7fe013f6624da3babc4942defcea0d4c4754fc 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 Signed-off-by: Owen Leonard --- C4/Biblio.pm | 4 +--- 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 | 25 ++++++++++------------ 6 files changed, 25 insertions(+), 40 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 ce0d4fbdb6b..1d5a2d9af5d 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -2633,9 +2633,7 @@ sub EmbedItemsInMarcBiblio { # and on this biblionumber # Flag indicating if there is potential hiding. - my $opachiddenitems = $opac - && ( C4::Context->preference('OpacHiddenItems') !~ /^\s*$/ ); - + my $opachiddenitems = $opac && ( 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 b81fb0aa17f..e11137f01d5 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -1205,17 +1205,8 @@ sub GetHiddenItemnumbers { } 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 00000000000..b5472aaf916 --- /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 9fc3d5821ea..de41670ede2 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -380,7 +380,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 the OPAC. See http://wiki.koha-community.org/wiki/OpacHiddenItems for more information.','Textarea'), +('OpacHiddenItems','','','This syspref allows to define custom rules for hiding specific items at the OPAC. See http://wiki.koha-community.org/wiki/OpacHiddenItems for more information.','YAML'), ('OpacHiddenItemsExceptions','',NULL,'List of borrower categories, separated by |, that can see items otherwise hidden by OpacHiddenItems','Textarea'), ('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'), diff --git a/opac/opac-search.pl b/opac/opac-search.pl index ea78ca0b23b..713b2a5c149 100755 --- a/opac/opac-search.pl +++ b/opac/opac-search.pl @@ -247,17 +247,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 bbed0436b6b..7fb84c1d59a 100755 --- a/t/db_dependent/Items.t +++ b/t/db_dependent/Items.t @@ -203,32 +203,30 @@ subtest 'GetHiddenItemnumbers tests' => sub { ok( !defined( GetHiddenItemnumbers( { items => \@items } ) ), "Hidden items list undef if OpacHiddenItems empty"); - # Blank spaces - t::lib::Mocks::mock_preference('OpacHiddenItems',' '); + # Empty ref + t::lib::Mocks::mock_preference('OpacHiddenItems', {}); ok( scalar GetHiddenItemnumbers( { items => \@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 => \@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 => \@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 => \@items } ); ok( scalar @hidden == 2, "Two items hidden"); @@ -675,8 +673,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({ @@ -698,8 +695,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({ marc_record => $record, -- 2.11.0