@@ -, +, @@ - ShouldDisplayOnInterface - ShouldHideMARC - GetFilteredBiblio --------- 1) Back up your DB 2) Go to any OPAC detail page and note the biblio number. 3) Log into the staff client and determine the framework code for that biblio number. 4) The steps should be done with OPACXSLTDetailsDisplay set to blank. 5) Home -> Koha administration -> MARC bibliographic framework -> MARC structure (for the matching framework) 6) On the OPAC detail page, click MARC view 7) In the staff client, for every tag listed in the OPAC -> Subfields -> click the first link Then running through all the tabs, click Advanced constraints and uncheck OPAC visibility. Then click Save Changes 8) Refresh the opac-MARCdetail page in OPAC -- what you hid should be mostly hidden TITLE will still display, even if you hide 245$a! 9) Click Normal view, and all the hidden things are still mostly showing! set to default (or some custom one?). hidden things. before until you... -- Look for ShouldHideMARC and confirm that the perldoc entries are okay. -- 'Visibility values confirmed.' test should pass. -- tests 26 through 41 should pass for UNIMARC and MARC21. It would be good to test this patch on both types of systems. -- LDR and 999$c and 999$d were displaying for me. I realized that I hadn't hidden 999, because the opac-MARCdetail page doesn't display it. LDR is the only known leak. the opac-MARCdetail page. a blank value. including the view plain link, and everything should hidden similarly. -- NOTE: LDR is the only known leak. This patch does not deal with 952 fields. -- Only unhidden fields should display. --- C4/Biblio.pm | 264 +++++++++++++++++++++++++++++++++++++++++++++-- C4/XSLT.pm | 17 +-- opac/opac-ISBDdetail.pl | 5 +- opac/opac-MARCdetail.pl | 3 +- opac/opac-detail.pl | 21 +++- opac/opac-export.pl | 14 ++- opac/opac-showmarc.pl | 13 ++- t/Biblio.t | 27 ++++- t/db_dependent/Biblio.t | 102 +++++++++++++++++- 9 files changed, 439 insertions(+), 27 deletions(-) --- a/C4/Biblio.pm +++ a/C4/Biblio.pm @@ -74,6 +74,9 @@ BEGIN { &GetMarcISBN &GetMarcISSN &GetMarcSubjects + ShouldDisplayOnInterface + ShouldHideMARC + GetFilteredBiblio &GetMarcBiblio &GetMarcAuthors &GetMarcSeries @@ -1259,22 +1262,163 @@ sub GetMarcSubfieldStructureFromKohaField { return $result; } +sub ShouldDisplayOnInterface { + my $display = { + opac => { + 0 => 1, + -1 => 1, + -2 => 1, + -3 => 1, + -4 => 1, + -5 => 1, + -6 => 1, + -7 => 1, + }, + intranet => { + -6 => 1, + -5 => 1, + -1 => 1, + 0 => 1, + 1 => 1, + 4 => 1, + 6 => 1, + 7 => 1, + }, + }; + return $display; +} + +=head2 ShouldHideMARC + +Return a hash reference of whether a field, built from +kohafield and tag, is hidden (1) or not (0) for a given +interface + + my $OpacHideMARC = + GetOpacHideMARC( { + frameworkcode => $frameworkcode, + interface => 'opac', + } ); + + if ($OpacHideMARC->{'stocknumber'}==1) { + print "Hidden!\n"; + } + +C<$OpacHideMARC> is a ref to a hash which contains a series +of key value pairs indicating if that field (key) is +hidden (value == 1) or not (value == 0). + +C<$frameworkcode> is the framework code. + +C<$interface> is the interface. It defaults to 'opac' if +nothing is passed. Valid values include 'opac' or 'intranet'. + +=cut + +sub ShouldHideMARC { + my ($parms) = @_; + my $frameworkcode = $parms->{frameworkcode} // ''; + my $interface = $parms->{interface} // 'opac'; + my $display = ShouldDisplayOnInterface(); + + my $dbh = C4::Context->dbh; + my $sth = $dbh->prepare("SELECT kohafield AS field, tagfield AS tag, hidden FROM marc_subfield_structure WHERE kohafield>'' AND frameworkcode=? ORDER BY field, tagfield;"); + $sth->execute($frameworkcode); + + my %shouldhidemarc; + my $data = $sth->fetchall_hashref( [ qw(field tag) ] ); + foreach my $fullfield (keys %{$data}) { + my @tmpsplit = split(/\./,$fullfield); + my $field = $tmpsplit[-1]; + foreach my $tag (keys %{$data->{$fullfield}}) { + my $show = $display->{$interface}->{ $data->{$fullfield}->{$tag}->{'hidden'} }; + if (!$show) { + $shouldhidemarc{ $field } = 1; + } + elsif ( !exists($shouldhidemarc{ $field }) ) { + $shouldhidemarc{ $field } = 0; + } + } + } + return \%shouldhidemarc; +} + =head2 GetMarcBiblio - my $record = GetMarcBiblio($biblionumber, [$embeditems]); + my $record = GetMarcBiblio( { + biblionumber => $biblionumber, + embeditems => $embeditems, + interface => 'opac', + frameworkcode => '', + debug => 0, + } ); + +Returns MARC::Record containing biblio data identified by the +parameters passed. + +C<$biblionumber>. If no bib exists, returns undef. This is + required. Otherwise, this returns nothing. -Returns MARC::Record representing bib identified by -C<$biblionumber>. If no bib exists, returns undef. C<$embeditems>. If set to true, items data are included. -The MARC record contains biblio data, and items data if $embeditems is set to true. + It is optional, and assumed false if omitted. + +C<$interface>. Valid values include 'opac' or 'intranet'. It is + optional. If not passed, no filtering is done. + +C<$framework>. Any valid framework code. By calling this routine + in the hash style, omitting this parameter is + determined by using the biblionumber. If there is + no framework code defined, regardless of calling + convention, the default one is then used. + +C<$debug>. This optional parameter is not defined in the old + calling style, and so it is defaulted to true. + However, if the new hash parameter style is used, + it defaults to false. This was mostly for testing + purposes. See t/db_dependent/Biblio.t and the + warning_is checks. =cut sub GetMarcBiblio { - my $biblionumber = shift; - my $embeditems = shift || 0; - my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare("SELECT marcxml FROM biblioitems WHERE biblionumber=? "); + my @catch_parameters = @_; + + my $biblionumber; + my $embeditems; + my $interface; + my $frameworkcode; + my $debug; # This is useful for generating warns for tests. + + # No parameters, return nothing. + if (! @catch_parameters) { + return; + } + # New style means filtering. + elsif (scalar @catch_parameters==1 && ref($catch_parameters[0]) eq 'HASH') { + $biblionumber = $catch_parameters[0]->{biblionumber}; + $embeditems = $catch_parameters[0]->{embeditems}; + $interface = $catch_parameters[0]->{interface}; + $frameworkcode = $catch_parameters[0]->{frameworkcode}; + if (!$frameworkcode && $interface) { + $frameworkcode = GetFrameworkCode($biblionumber); + } + $debug = $catch_parameters[0]->{debug} // 0; + } + # Old style means no filtering. + else { + $biblionumber = shift @catch_parameters if @catch_parameters; + $embeditems = shift @catch_parameters if @catch_parameters; + } + if (! $biblionumber ) { + return; + } + $embeditems //= 0; # No items by default. + $interface //= 'unfiltered'; # unfiltered if no interface passed. + $frameworkcode //= ''; # Default framework if undefined. + $debug //= 1; # Give message for old style calls. + + my $dbh = C4::Context->dbh; + my $sth = $dbh->prepare("SELECT marcxml FROM biblioitems WHERE biblionumber=? "); $sth->execute($biblionumber); my $row = $sth->fetchrow_hashref; my $marcxml = StripNonXmlChars( $row->{'marcxml'} ); @@ -1289,12 +1433,114 @@ sub GetMarcBiblio { C4::Biblio::_koha_marc_update_bib_ids($record, '', $biblionumber, $biblionumber); C4::Biblio::EmbedItemsInMarcBiblio($record, $biblionumber) if ($embeditems); + # only 'opac' and 'intranet' keys exist, so 'unfiltered' + # won't change anything. + if ($interface ne 'unfiltered') { + $record = GetFilteredBiblio( { + record => $record, + interface => $interface, + frameworkcode => $frameworkcode, + } ); + warn "INTERNAL FILTERING PROCESSED!" if $debug; + + } + else { + warn "INTERNAL FILTERING NOT PROCESSED!" if $debug; + } return $record; } else { return; } } +=head2 GetFilteredBiblio + +Return a MARC::Record which has been filtered based on the +corresponding marc_subfield_structure records for the given +framework and interface ('opac' or 'intranet') + + my $filtered_record = GetFilteredBiblio( { + record => $record, + frameworkcode => $frameworkcode, + interface => 'opac', + } ); + +C<$record> is a MARC::Record. + +C<$frameworkcode> is the framework code. If this is not passed, +the default framework is used. + +C<$interface> is the interface. Valid values include 'opac' and +'intranet' as defined by the hash in ShowDisplayOnInterface. If +this is not passed, 'opac' is used. + +=cut + +sub GetFilteredBiblio { + my ($parms) = @_; + + my $precord = $parms->{record}; + if (!$precord) { + return $precord; + } + my $record = $precord->clone(); + my $interface = $parms->{interface} // 'opac'; + my $frameworkcode = $parms->{frameworkcode} // ''; + my $display = ShouldDisplayOnInterface(); + + my $marcsubfieldstructure = GetMarcStructure(0,$frameworkcode); + #if ($marcsubfieldstructure->{'000'}->{'@'}->{hidden}>0) { + # LDR field is excluded from $record->fields(). + # if we hide it here, the MARCXML->MARC::Record->MARCXML + # transformation blows up. + #} + foreach my $field ($record->fields()) { + my $tag = $field->tag(); + if ($tag>=10) { + foreach my $subpairs ($field->subfields()) { + my ($subtag,$value) = @$subpairs; + # visibility is a "level" (-7 to +7), default to 0 + my $visibility = $marcsubfieldstructure->{$tag}->{$subtag}->{hidden}; + $visibility //= 0; + my $hidden; + if ($display->{$interface}->{$visibility}) { + $hidden = 0; + } + else { + $hidden = 1; + } + if ($hidden) { + # deleting last subfield doesn't delete field, so + # this detects that case to delete the field. + if (scalar $field->subfields() <= 1) { + $record->delete_fields($field); + } + else { + $field->delete_subfield( code => $subtag ); + } + } + } + } + # tags less than 10 don't have subfields, use @ trick. + else { + # visibility is a "level" (-7 to +7), default to 0 + my $visibility = $marcsubfieldstructure->{$tag}->{'@'}->{hidden}; + $visibility //= 0; + my $hidden; + if ($display->{$interface}->{$visibility}) { + $hidden = 0; + } + else { + $hidden = 1; + } + if ($hidden) { + $record->delete_fields($field); + } + } + } + return $record; +} + =head2 GetXmlBiblio my $marcxml = GetXmlBiblio($biblionumber); @@ -1878,7 +2124,7 @@ sub GetMarcSubjects { push @marcsubjects, { MARCSUBJECT_SUBFIELDS_LOOP => \@subfields_loop, authoritylink => $authoritylink, - }; + } if $authoritylink || @subfields_loop; } return \@marcsubjects; --- a/C4/XSLT.pm +++ a/C4/XSLT.pm @@ -85,12 +85,17 @@ sub transformMARCXML4XSLT { if $av->{ $tag }->{ $letter }; push( @new_subfields, $letter, $value ); } - $field ->replace_with( MARC::Field->new( - $tag, - $field->indicator(1), - $field->indicator(2), - @new_subfields - ) ); + if (@new_subfields) { + $field ->replace_with( MARC::Field->new( + $tag, + $field->indicator(1), + $field->indicator(2), + @new_subfields + ) ); + } + else { + $record->delete_fields($field); + } } } } --- a/opac/opac-ISBDdetail.pl +++ a/opac/opac-ISBDdetail.pl @@ -93,7 +93,10 @@ if (scalar @items >= 1) { } } -my $record = GetMarcBiblio($biblionumber); +my $record = GetMarcBiblio( { + biblionumber => $biblionumber, + interface => 'opac', + } ); if ( ! $record ) { print $query->redirect("/cgi-bin/koha/errors/404.pl"); exit; --- a/opac/opac-MARCdetail.pl +++ a/opac/opac-MARCdetail.pl @@ -98,9 +98,10 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user( } ); +my ($bt_tag,$bt_subtag) = GetMarcFromKohaField('biblio.title',$itemtype); $template->param( bibliotitle => $biblio->{title}, -); +) if $tagslib->{$bt_tag}->{$bt_subtag}->{hidden} <= 0; # get biblionumbers stored in the cart my @cart_list; --- a/opac/opac-detail.pl +++ a/opac/opac-detail.pl @@ -83,7 +83,10 @@ if (scalar @all_items >= 1) { } } -my $record = GetMarcBiblio($biblionumber); +my $record = GetMarcBiblio( { + biblionumber => $biblionumber, + interface => 'opac', + } ); if ( ! $record ) { print $query->redirect("/cgi-bin/koha/errors/404.pl"); # escape early exit; @@ -517,6 +520,10 @@ if ( C4::Context->preference('HighlightOwnItemsOnOPAC') ) { } my $dat = &GetBiblioData($biblionumber); +my $HideMARC = ShouldHideMARC( { + frameworkcode => $dat->{'frameworkcode'}, + interface => 'opac', + } ); my $itemtypes = GetItemTypes(); # imageurl: @@ -676,7 +683,16 @@ my $marcsubjctsarray = GetMarcSubjects($record,$marcflavour); my $marcseriesarray = GetMarcSeries ($record,$marcflavour); my $marcurlsarray = GetMarcUrls ($record,$marcflavour); my $marchostsarray = GetMarcHosts($record,$marcflavour); -my $subtitle = GetRecordValue('subtitle', $record, GetFrameworkCode($biblionumber)); +my ($st_tag,$st_subtag) = GetMarcFromKohaField('bibliosubtitle.subtitle',$dat->{'frameworkcode'}); +my $subtitle; +if ($st_tag && $st_subtag) { + my @subtitles = $record->subfield($st_tag,$st_subtag); + $subtitle = \@subtitles if @subtitles; +} +if ($subtitle && @$subtitle) { + my @subtitles = map { { subfield => $_ } } @$subtitle; + $subtitle = \@subtitles; +} $template->param( MARCNOTES => $marcnotesarray, @@ -726,6 +742,7 @@ if (C4::Context->preference("AlternateHoldingsField") && scalar @items == 0) { } foreach ( keys %{$dat} ) { + next if ( $HideMARC->{$_} ); $template->param( "$_" => defined $dat->{$_} ? $dat->{$_} : '' ); } --- a/opac/opac-export.pl +++ a/opac/opac-export.pl @@ -32,9 +32,17 @@ my $op=$query->param("op")||''; #op=export is currently the only use my $format=$query->param("format")||'utf8'; my $biblionumber = $query->param("bib")||0; $biblionumber = int($biblionumber); -my ($marc, $error)= ('',''); +my ($marc, $marc_noitems, $error)= ('','',''); -$marc = GetMarcBiblio($biblionumber, 1) if $biblionumber; +$marc = GetMarcBiblio( { + biblionumber => $biblionumber, + embeditems => 1, + interface => 'opac', + } ) if $biblionumber; +$marc_noitems = GetMarcBiblio( { + biblionumber => $biblionumber, + interface => 'opac', + } ) if $biblionumber; if(!$marc) { print $query->redirect("/cgi-bin/koha/errors/404.pl"); exit; @@ -56,7 +64,7 @@ elsif ($format =~ /ris/) { $format = 'ris'; } elsif ($format =~ /bibtex/) { - $marc = marc2bibtex(C4::Biblio::GetMarcBiblio($biblionumber),$biblionumber); + $marc = marc2bibtex($marc_noitems,$biblionumber); $format = 'bibtex'; } elsif ($format =~ /dc/) { --- a/opac/opac-showmarc.pl +++ a/opac/opac-showmarc.pl @@ -43,7 +43,10 @@ if ($importid) { $record = MARC::Record->new_from_usmarc($marc); } else { - $record =GetMarcBiblio($biblionumber); + $record =GetMarcBiblio( { + biblionumber => $biblionumber, + interface => 'opac', + } ); } if(!ref $record) { print $input->redirect("/cgi-bin/koha/errors/404.pl"); @@ -52,6 +55,14 @@ if(!ref $record) { if ($view eq 'card' || $view eq 'html') { my $xmlrecord= $importid? $record->as_xml(): GetXmlBiblio($biblionumber); + if (!$importid && $view eq 'html') { + my $filtered_record = MARC::Record->new_from_xml($xmlrecord); + my $frameworkcode = GetFrameworkCode($biblionumber); + $filtered_record = GetFilteredBiblio( { + record => $filtered_record, + frameworkcode => $frameworkcode } ); + $xmlrecord = $filtered_record->as_xml(); + } my $xslfile; my $xslfilename; my $htdocs = C4::Context->config('opachtdocs'); --- a/t/Biblio.t +++ a/t/Biblio.t @@ -3,7 +3,7 @@ use strict; use warnings; -use Test::More tests => 22; +use Test::More tests => 23; BEGIN { use_ok('C4::Biblio'); @@ -12,6 +12,31 @@ BEGIN { # test returns if undef record passed # carp messages appear on stdout +my $display1 = { + opac => { + 0 => 1, + -1 => 1, + -2 => 1, + -3 => 1, + -4 => 1, + -5 => 1, + -6 => 1, + -7 => 1, + }, + intranet => { + -6 => 1, + -5 => 1, + -1 => 1, + 0 => 1, + 1 => 1, + 4 => 1, + 6 => 1, + 7 => 1, + }, + }; +my $display2 = ShouldDisplayOnInterface(); +is_deeply($display2,$display1,'Visibility values confirmed.'); + my @arr = AddBiblio(undef, q{}); my $elements = @arr; --- a/t/db_dependent/Biblio.t +++ a/t/db_dependent/Biblio.t @@ -17,7 +17,8 @@ use Modern::Perl; -use Test::More tests => 3; +use Test::More tests => 7; +use Test::Warn; use Test::MockModule; use MARC::Record; @@ -179,6 +180,88 @@ sub run_tests { "(GetMarcISBN) Corretly retrieves ISBN #". ($i + 1)); } + my $unfiltered_record1; + warning_is { $unfiltered_record1 = GetMarcBiblio( $biblionumber ); } + 'INTERNAL FILTERING NOT PROCESSED!', 'GetMarcBiblio Call1.'; + my $unfiltered_record2; + warning_is { $unfiltered_record2 = GetMarcBiblio( $biblionumber, 0 ); } + 'INTERNAL FILTERING NOT PROCESSED!', 'GetMarcBiblio Call2.'; + my $unfiltered_record3; + warning_is { $unfiltered_record3 = + GetMarcBiblio( { biblionumber => $biblionumber, + debug => 1 } ); } + 'INTERNAL FILTERING NOT PROCESSED!', 'GetMarcBiblio Call3.'; + my $unfiltered_record4; + warning_is { $unfiltered_record4 = + GetMarcBiblio( { biblionumber => $biblionumber, + embeditems => 0, + debug => 1 } ); } + 'INTERNAL FILTERING NOT PROCESSED!', 'GetMarcBiblio Call4.'; + + is_deeply ( $unfiltered_record2, $unfiltered_record1, '(GetMarcBiblio w/o items) old implicitly = old explicitly.'); + is_deeply ( $unfiltered_record3, $unfiltered_record1, '(GetMarcBiblio w/o items) old implicitly = new implicitly.'); + is_deeply ( $unfiltered_record4, $unfiltered_record1, '(GetMarcBiblio w/o items) old implicitly = new explicitly.'); + + warning_is { $unfiltered_record1 = GetMarcBiblio( $biblionumber, 1 ); } + 'INTERNAL FILTERING NOT PROCESSED!', 'GetMarcBiblio Call5.'; + warning_is { $unfiltered_record2 = + GetMarcBiblio( { biblionumber => $biblionumber, + embeditems => 1, + debug => 1 } ); } + 'INTERNAL FILTERING NOT PROCESSED!', 'GetMarcBiblio Call6.'; + + is_deeply ( $unfiltered_record2, $unfiltered_record1, '(GetMarcBiblio with items) old explicitly = new explicitly.'); + + # the frameworkcode for the added biblio was not set, + # so the frameworkcode is '' for Default. + my $title_field = ( $marcflavour eq 'UNIMARC' ) ? '200' : '245'; + my $sth = $dbh->prepare("SELECT hidden FROM marc_subfield_structure WHERE tagfield='$title_field' and tagsubfield='a' and frameworkcode='';"); + $sth->execute(); + my $hidden = $sth->fetchrow; + if (defined($hidden)) { + ok ( defined($hidden), 'There is a title field entry for the ' . $marcflavour . 'Default framework.'); + + $dbh->do("UPDATE marc_subfield_structure SET hidden=4 WHERE tagfield='$title_field' and tagsubfield='a' and frameworkcode='';"); + $sth = $dbh->prepare("SELECT hidden FROM marc_subfield_structure WHERE tagfield='$title_field' and tagsubfield='a' and frameworkcode='';"); + $sth->execute(); + my $new_hidden = $sth->fetchrow; + ok ( defined($new_hidden) && $new_hidden==4, 'Default framework title field hidden value updated.'); + + my $fail_filter = GetFilteredBiblio(); + ok (!defined($fail_filter), 'GetFilteredBiblio properly handles no parameters.'); + + my $filtered_record1 = GetFilteredBiblio( { + record => $unfiltered_record1 } ); + my $filtered_record2 = GetFilteredBiblio( { + record => $unfiltered_record1, + frameworkcode => '' } ); + my @fields1 = $filtered_record1->fields(); + my @fields2 = $filtered_record2->fields(); + ok ( scalar @fields1 == scalar @fields2, 'No framework code matches Default framework results.'); + + $title = $unfiltered_record1->subfield($title_field,'a'); + my $filtered_title = $filtered_record1->subfield($title_field,'a'); + ok ( defined($title) && !defined($filtered_title), + 'GetFilteredBiblio filtered title as expected. ' . + "title: " . ($title ? $title : "UNDEF") . " -- " . + "hidden: " . ($filtered_title ? $filtered_title : "UNDEF") ); + + $sth = $dbh->prepare("UPDATE marc_subfield_structure SET hidden=? WHERE tagfield='$title_field' and tagsubfield='a' AND frameworkcode='';"); + $sth->execute($hidden); + $sth = $dbh->prepare("SELECT hidden FROM marc_subfield_structure WHERE tagfield='$title_field' and tagsubfield='a' and frameworkcode='';"); + $sth->execute(); + $new_hidden = $sth->fetchrow; + ok ( $new_hidden==$hidden, 'Default framework title field hidden value reset.'); + } + else { + ok ( !defined($hidden), 'Skipping tests for MARC field/subfield hiding under ' . $marcflavour . ' Default framework.'); + ok ( !defined($hidden), 'Skipping tests for MARC field/subfield hiding under ' . $marcflavour . ' Default framework.'); + ok ( !defined($hidden), 'Skipping tests for MARC field/subfield hiding under ' . $marcflavour . ' Default framework.'); + ok ( !defined($hidden), 'Skipping tests for MARC field/subfield hiding under ' . $marcflavour . ' Default framework.'); + ok ( !defined($hidden), 'Skipping tests for MARC field/subfield hiding under ' . $marcflavour . ' Default framework.'); + ok ( !defined($hidden), 'Skipping tests for MARC field/subfield hiding under ' . $marcflavour . ' Default framework.'); + } + } sub mock_marcfromkohafield { @@ -240,14 +323,27 @@ sub create_issn_field { return $field; } +my $HideMARC = ShouldHideMARC(); +my $keycount1 = keys %$HideMARC; +ok($keycount1>0,"There are $keycount1 values for the Default framework."); + +$HideMARC = ShouldHideMARC({ frameworkcode => 'YOLO' }); +my $keycount2 = keys %$HideMARC; +ok($keycount2==0,'The YOLO framework does not exist.'); + +$HideMARC = ShouldHideMARC({ frameworkcode => '' }); +my $keycount3 = keys %$HideMARC; +ok($keycount1>0,"There are $keycount3 values for the Default framework."); +ok($keycount1==$keycount3,'The no parameter and empty parameter counts match.'); + subtest 'MARC21' => sub { - plan tests => 25; + plan tests => 41; run_tests('MARC21'); $dbh->rollback; }; subtest 'UNIMARC' => sub { - plan tests => 25; + plan tests => 41; run_tests('UNIMARC'); $dbh->rollback; }; --