From 3b048283c015ddd4e5e41ca0efebfc19fcca2c90 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 Signed-off-by: Michal Denar --- 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 ce0d4fbdb6..1d5a2d9af5 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 862e5e2d90..161c463ddf 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -1201,17 +1201,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 0000000000..b5472aaf91 --- /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 6a68769eda..97c2f30b6c 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 96264e4452..b40a67403e 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 8ac9e7ef22..6ab8102438 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"); @@ -673,8 +671,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({ @@ -696,8 +693,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