Bugzilla – Attachment 24657 Details for
Bug 11592
opac detail scripts do not respect MARC tag visibility
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 11592 - opac scripts do not respect marc tag visibility
Bug-11592---opac-scripts-do-not-respect-marc-tag-v.patch (text/plain), 18.57 KB, created by
Mark Tompsett
on 2014-01-24 02:56:23 UTC
(
hide
)
Description:
Bug 11592 - opac scripts do not respect marc tag visibility
Filename:
MIME Type:
Creator:
Mark Tompsett
Created:
2014-01-24 02:56:23 UTC
Size:
18.57 KB
patch
obsolete
>From 2ec06b4cd5226facccd652ff1919fc3d3a71fc5d Mon Sep 17 00:00:00 2001 >From: Mark Tompsett <mtompset@hotmail.com> >Date: Tue, 14 Jan 2014 11:31:36 -0500 >Subject: [PATCH] Bug 11592 - opac scripts do not respect marc tag visibility > >Added two functions to C4/Biblio: GetFilteredOpacBiblio >and GetOpacHideMARC. > >GetFilteredOpacBiblio returns a MARC::Record stripped of all the >fields and subfields which are supposed to be hidden in OPAC. > >GetOpacHideMARC returns a hash of whether a particular key >(eg. title, subtitle, etc.) is hidden. A value of 0 means no, >a value of 1 means hidden. > >Made GetCOinSBiblio function handle hiding of 245$a more >gracefully. Also, modified GetMarcSubjects to not generate an >array entry of empty values. > >Tweaked C4/XSLT.pm to delete a field, if there weren't any >subfields to actually update with. > >Properly hid 245$a, in the case that there is no visibility >for OPAC, for opac-MARCdetail. > >opac-detail now filters the MARC::Record according to the hidden >value in marc_subfield_structure properly. It also corrects a >couple minor issues with a parameter not passed being used in a >concatenation. How the subtitle values calculation works changed. >It is now based on marc_subfield_structure and not fieldmapping. >And, the GetBiblioData hash that generates template variables is >now filtered by the results of a GetOpacHideMARC call. > >The opac-showmarc page now converts the MARCXML record to a >MARC::Record to easily filter using GetFilteredOpacBiblio, which >is then turned back into MARCXML for processing. > >The opac-export script now filters the biblio record to match >what is currently displayed. > >Added tests to t/db_dependent/Biblio.t to test the functionality >of the two functions added into C4::Biblio. > >Added a GetFilteredOpacBiblio call into opac-ISBDdetail page, so >that it should be filtered. > >TEST PLAN >--------- > 1) Backup 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! >10) The steps should be done with OPACXSLTDetailsDisplay > set to default (or some custom one?). >11) Refresh the opac-detail page. Still mostly showing all the > hidden things. >12) Click MARC view, and everything should be hidden... >13) Until you click the 'view plain' link. >14) Apply the patch. >15) Run the Koha QA test tool. >16) Refresh the opac-MARCdetail page. Title should hide now. >17) Click the 'view plain' link. > -- 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. >18) Click Normal view >19) Now all the opac-detail page should hide things just like > the opac-MARCdetail page. >20) In the staff client, change the OPACXSLTDetailsDisplay to > a blank value. >21) Recheck opac-detail and opac-MARC detail pages again, > including the view plain link, and everything should > hidden similarly. > -- NOTE: LDR is the only known leak. >NOTE: 952 fields are treated separately, so I don't believe they > could be hidden in the table using the MARC visibility. > The goal was hiding properly things above the items. >22) And lastly, attempt to Save record in the various formats > using the dropdown and clicking Go. > -- The results should be filtered. >23) Click on ISBD view to ensure nothing broke. >24) Restore DB, because hiding 000,003,005,008,020,040,245,etc. > are not useful across an entire framework. >--- > C4/Biblio.pm | 108 ++++++++++++++++++++++++++++++++++++++++++++++- > C4/XSLT.pm | 17 +++++--- > opac/opac-ISBDdetail.pl | 2 + > opac/opac-MARCdetail.pl | 3 +- > opac/opac-detail.pl | 21 +++++++-- > opac/opac-export.pl | 7 ++- > opac/opac-showmarc.pl | 6 +++ > t/db_dependent/Biblio.t | 56 ++++++++++++++++++++++-- > 8 files changed, 203 insertions(+), 17 deletions(-) > >diff --git a/C4/Biblio.pm b/C4/Biblio.pm >index dc40d68..c371f62 100644 >--- a/C4/Biblio.pm >+++ b/C4/Biblio.pm >@@ -79,6 +79,8 @@ BEGIN { > &GetMarcSeries > &GetMarcHosts > GetMarcUrls >+ &GetOpacHideMARC >+ &GetFilteredOpacBiblio > &GetUsedMarcStructure > &GetXmlBiblio > &GetCOinSBiblio >@@ -1207,6 +1209,107 @@ sub GetUsedMarcStructure { > return $sth->fetchall_arrayref( {} ); > } > >+=head2 GetOpacHideMARC >+ >+Return a hash reference of whether a field, built from >+kohafield and tag, is hidden in OPAC (1) or not (0). >+ >+ my $OpacHideMARC = GetOpacHideMARC($frameworkcode); >+ >+ 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. >+ >+=cut >+ >+sub GetOpacHideMARC { >+ my ( $frameworkcode ) = shift || ''; >+ 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;"); >+ my $rv = $sth->execute($frameworkcode); >+ my %opachidemarc; >+ 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}}) { >+ if ($data->{$fullfield}->{$tag}->{'hidden'}>0) { >+ $opachidemarc{ $field } = 1; >+ } >+ elsif ( !exists($opachidemarc{ $field }) ) { >+ $opachidemarc{ $field } = 0; >+ } >+ } >+ } >+ return \%opachidemarc; >+} >+ >+=head2 GetFilteredOpacBiblio >+ >+Return a MARC::Record which has been filtered based >+on the corresponding marc_subfield_structure records >+for the given framework by excluding hidden>0. >+ >+ my $filtered_record = GetFilteredOpacBiblio($record,$frameworkcode); >+ >+C<$record> is a MARC::Record. >+ >+C<$frameworkcode> is the framework code. >+ >+=cut >+ >+sub GetFilteredOpacBiblio { >+ my ( $record ) = shift; >+ if (!$record) { return $record; } >+ my $filtered_record = $record->clone; >+ >+ my ( $frameworkcode ) = shift || ''; >+ >+ 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 $fields ($filtered_record->fields()) { >+ my $tag = $fields->tag(); >+ if ($tag>=10) { >+ foreach my $subpairs ($fields->subfields()) { >+ my ($subtag,$value) = @$subpairs; >+ my $hidden = $marcsubfieldstructure->{$tag}->{$subtag}->{hidden}; >+ $hidden //= 0; >+ $hidden = ($hidden<=0) ? 0 : 1; >+ if ($hidden) { >+ # deleting last subfield doesn't delete field, so >+ # this detects that case to delete the field. >+ if (scalar $fields->subfields() <= 1) { >+ $filtered_record->delete_fields($fields); >+ } >+ else { >+ $fields->delete_subfield( code => $subtag ); >+ } >+ } >+ } >+ } >+ # tags less than 10 don't have subfields, use @ trick. >+ else { >+ my $hidden = $marcsubfieldstructure->{$tag}->{'@'}->{hidden}; >+ $hidden //= 0; >+ $hidden = ($hidden<=0) ? 0 : 1; >+ if ($hidden) { >+ $filtered_record->delete_fields($fields); >+ } >+ } >+ } >+ >+ return $filtered_record; >+} >+ > =head2 GetMarcFromKohaField > > ($MARCfield,$MARCsubfield)=GetMarcFromKohaField($kohafield,$frameworkcode); >@@ -1427,7 +1530,8 @@ sub GetCOinSBiblio { > $oauthors .= "&rft.au=$au"; > } > } >- $title = "&rft." . $titletype . "title=" . $record->subfield( '245', 'a' ); >+ $title = $record->subfield( '245', 'a' ) || ''; >+ $title = "&rft." . $titletype . "title=" . $title; > $subtitle = $record->subfield( '245', 'b' ) || ''; > $title .= $subtitle; > if ($titletype eq 'a') { >@@ -1873,7 +1977,7 @@ sub GetMarcSubjects { > push @marcsubjects, { > MARCSUBJECT_SUBFIELDS_LOOP => \@subfields_loop, > authoritylink => $authoritylink, >- }; >+ } if $authoritylink || @subfields_loop; > > } > return \@marcsubjects; >diff --git a/C4/XSLT.pm b/C4/XSLT.pm >index d85b048..2a43a41 100644 >--- a/C4/XSLT.pm >+++ b/C4/XSLT.pm >@@ -93,12 +93,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); >+ } > } > } > } >diff --git a/opac/opac-ISBDdetail.pl b/opac/opac-ISBDdetail.pl >index 2579a8b..f632546 100755 >--- a/opac/opac-ISBDdetail.pl >+++ b/opac/opac-ISBDdetail.pl >@@ -100,6 +100,8 @@ if ( ! $record ) { > print $query->redirect("/cgi-bin/koha/errors/404.pl"); > exit; > } >+my $record = GetFilteredOPACBiblio($unfiltered_record,GetFrameworkCode($biblionumber)); >+ > # some useful variables for enhanced content; > # in each case, we're grabbing the first value we find in > # the record and normalizing it >diff --git a/opac/opac-MARCdetail.pl b/opac/opac-MARCdetail.pl >index aaa565d..8a999dd 100755 >--- a/opac/opac-MARCdetail.pl >+++ b/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; >diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl >index 77de12f..63e4a34 100755 >--- a/opac/opac-detail.pl >+++ b/opac/opac-detail.pl >@@ -91,6 +91,10 @@ if ( ! $record ) { > } > $template->param( biblionumber => $biblionumber ); > >+# Filter out the things to hide in OPAC. >+my $frameworkcode = GetFrameworkCode($biblionumber); >+$record = GetFilteredOpacBiblio($record,$frameworkcode); >+ > # get biblionumbers stored in the cart > my @cart_list; > >@@ -377,13 +381,13 @@ if ($session->param('busc')) { > my ($previous, $next, $dataBiblioPaging); > # Previous biblio > if ($paging{'previous'}->{biblionumber}) { >- $previous = 'opac-detail.pl?biblionumber=' . $paging{'previous'}->{biblionumber} . '&query_desc=' . $query->param('query_desc'); >+ $previous = 'opac-detail.pl?biblionumber=' . $paging{'previous'}->{biblionumber} . '&query_desc=' . ($query->param('query_desc') // ''); > $dataBiblioPaging = GetBiblioData($paging{'previous'}->{biblionumber}); > $template->param('previousTitle' => $dataBiblioPaging->{'title'}) if ($dataBiblioPaging); > } > # Next biblio > if ($paging{'next'}->{biblionumber}) { >- $next = 'opac-detail.pl?biblionumber=' . $paging{'next'}->{biblionumber} . '&query_desc=' . $query->param('query_desc'); >+ $next = 'opac-detail.pl?biblionumber=' . $paging{'next'}->{biblionumber} . '&query_desc=' . ($query->param('query_desc') // ''); > $dataBiblioPaging = GetBiblioData($paging{'next'}->{biblionumber}); > $template->param('nextTitle' => $dataBiblioPaging->{'title'}) if ($dataBiblioPaging); > } >@@ -485,6 +489,7 @@ if ( C4::Context->preference('HighlightOwnItemsOnOPAC') ) { > } > > my $dat = &GetBiblioData($biblionumber); >+my $OpacHideMARC = GetOpacHideMARC($dat->{'frameworkcode'}); > > my $itemtypes = GetItemTypes(); > # imageurl: >@@ -646,7 +651,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, >@@ -696,6 +710,7 @@ if (C4::Context->preference("AlternateHoldingsField") && scalar @items == 0) { > } > > foreach ( keys %{$dat} ) { >+ next if ( $OpacHideMARC->{$_} ); > $template->param( "$_" => defined $dat->{$_} ? $dat->{$_} : '' ); > } > >diff --git a/opac/opac-export.pl b/opac/opac-export.pl >index 002c88e..f67f858 100755 >--- a/opac/opac-export.pl >+++ b/opac/opac-export.pl >@@ -35,7 +35,10 @@ my $biblionumber = $query->param("bib")||0; > $biblionumber = int($biblionumber); > my ($marc, $error)= ('',''); > >-$marc = GetMarcBiblio($biblionumber, 1) if $biblionumber; >+my $unfiltered_marc = GetMarcBiblio($biblionumber, 1) if $biblionumber; >+my $frameworkcode = GetFrameworkCode($biblionumber); >+$marc = GetFilteredOpacBiblio($unfiltered_marc,$frameworkcode); >+ > if(!$marc) { > print $query->redirect("/cgi-bin/koha/errors/404.pl"); > exit; >@@ -57,7 +60,7 @@ elsif ($format =~ /ris/) { > $format = 'ris'; > } > elsif ($format =~ /bibtex/) { >- $marc = marc2bibtex(C4::Biblio::GetMarcBiblio($biblionumber),$biblionumber); >+ $marc = marc2bibtex($marc,$biblionumber); > $format = 'bibtex'; > } > elsif ($format =~ /dc/) { >diff --git a/opac/opac-showmarc.pl b/opac/opac-showmarc.pl >index 71fb804..380038b 100755 >--- a/opac/opac-showmarc.pl >+++ b/opac/opac-showmarc.pl >@@ -57,6 +57,12 @@ else { > > 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 = GetFilteredOpacBiblio($filtered_record,$frameworkcode); >+ $xmlrecord = $filtered_record->as_xml(); >+ } > my $xslfile; > my $xslfilename; > my $htdocs = C4::Context->config('opachtdocs'); >diff --git a/t/db_dependent/Biblio.t b/t/db_dependent/Biblio.t >index 4725e53..d50085e 100755 >--- a/t/db_dependent/Biblio.t >+++ b/t/db_dependent/Biblio.t >@@ -3,16 +3,30 @@ > # This Koha test module is a stub! > # Add more tests here!!! > >-use strict; >-use warnings; >-use Test::More tests => 17; >+use Modern::Perl; >+use Test::More tests => 27; > use MARC::Record; > use C4::Biblio; >+use C4::Context; >+use Data::Dumper; > > BEGIN { > use_ok('C4::Biblio'); > } > >+my $OpacHideMARC = GetOpacHideMARC(); >+my $keycount1 = keys %$OpacHideMARC; >+ok($keycount1>0,"There are $keycount1 values for the Default framework."); >+ >+$OpacHideMARC = GetOpacHideMARC('YOLO'); >+my $keycount2 = keys %$OpacHideMARC; >+ok($keycount2==0,'The YOLO framework does not exist.'); >+ >+$OpacHideMARC = GetOpacHideMARC(''); >+my $keycount3 = keys %$OpacHideMARC; >+ok($keycount1>0,"There are $keycount3 values for the Default framework."); >+ok($keycount1==$keycount3,'The no parameter and empty parameter counts match.'); >+ > my $isbn = '0590353403'; > my $title = 'Foundation'; > >@@ -109,5 +123,41 @@ $field->update('123456789X'); > $controlnumber = GetMarcControlnumber( $marc_record, 'MARC21' ); > is( $controlnumber, '123456789X', 'GetMarcControlnumber handles records with 001' ); > >+my $dbh = C4::Context->dbh; >+ >+# the frameworkcode for the added biblio was not set, >+# so the frameworkcode is '' for Default. >+my $sth = $dbh->prepare("SELECT hidden FROM marc_subfield_structure WHERE tagfield='245' and tagsubfield='a' and frameworkcode='';"); >+$sth->execute(); >+my $hidden = $sth->fetchrow; >+ok ( defined($hidden), 'There is a 245$a entry for the Default framework.'); >+ >+$dbh->do("UPDATE marc_subfield_structure SET hidden=4 WHERE tagfield='245' and tagsubfield='a' and frameworkcode='';"); >+$sth = $dbh->prepare("SELECT hidden FROM marc_subfield_structure WHERE tagfield='245' and tagsubfield='a' and frameworkcode='';"); >+$sth->execute(); >+my $new_hidden = $sth->fetchrow; >+ok ( defined($new_hidden) && $new_hidden==4, 'Default framework 245$a hidden value updated.'); >+ >+my $record = GetMarcBiblio($biblionumber); >+my $fail_filter = GetFilteredOpacBiblio(); >+ok (!defined($fail_filter), 'GetFilteredOpacBiblio properly handles no parameters.'); >+ >+my $filtered_record1 = GetFilteredOpacBiblio($record); >+my $filtered_record2 = GetFilteredOpacBiblio($record,''); >+my @fields1 = $filtered_record1->fields(); >+my @fields2 = $filtered_record2->fields(); >+ok ( scalar @fields1 == scalar @fields2, 'No framework code matches Default framework results.'); >+ >+$title = $record->subfield('245','a'); >+my $filtered_title = $filtered_record1->subfield('245','a'); >+ok ( defined($title) && !defined($filtered_title), 'GetFilteredOpacBiblio filtered title as expected.'); >+ >+$sth = $dbh->prepare("UPDATE marc_subfield_structure SET hidden=? WHERE tagfield='245' and tagsubfield='a' AND frameworkcode='';"); >+$sth->execute($hidden); >+$sth = $dbh->prepare("SELECT hidden FROM marc_subfield_structure WHERE tagfield='245' and tagsubfield='a' and frameworkcode='';"); >+$sth->execute(); >+$new_hidden = $sth->fetchrow; >+ok ( $new_hidden==$hidden, 'Default framework 245$a hidden value reset.'); >+ > # clean up after ourselves > DelBiblio($biblionumber); >-- >1.7.9.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 11592
:
24600
|
24601
|
24611
|
24629
|
24654
|
24655
|
24657
|
24658
|
24659
|
24660
|
24661
|
24662
|
24663
|
24664
|
24875
|
24876
|
24952
|
24953
|
28002
|
28003
|
28477
|
28478
|
28479
|
28480
|
28724
|
31356
|
32382
|
32383
|
33356
|
33357
|
33358
|
33689
|
35855
|
35856
|
35857
|
35858
|
47993
|
47996
|
47997
|
48001
|
49288
|
49289
|
49290
|
49291
|
49612
|
49613
|
49614
|
49615
|
50484
|
50485
|
50486
|
50489
|
50542
|
50550
|
50551
|
50552
|
50553
|
50554
|
51313
|
51314
|
51315
|
51316
|
51317
|
51318
|
51326
|
53440
|
53441
|
53442
|
53443
|
53444
|
55040
|
55041
|
55042
|
55043
|
55044
|
55045
|
55046
|
55184
|
55185