From 8fae625c6b4d6d54829f83b7ec1e4dbb8af20388 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 | 4 +-- C4/Items.pm | 13 ++-------- ...nItems-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 a0af92453e..9d54dfd996 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -2661,9 +2661,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 797bda6244..680e04b8d5 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 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 a0d1132d25..29c207b80b 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -368,7 +368,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 27d1f2ce5c..d4f9e3f89d 100755 --- a/opac/opac-search.pl +++ b/opac/opac-search.pl @@ -244,17 +244,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 af7f7966af..991bec4e2a 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"); @@ -671,8 +669,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({ @@ -694,8 +691,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.17.1