Bugzilla – Attachment 28003 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), 19.42 KB, created by
Mark Tompsett
on 2014-05-07 03:48:13 UTC
(
hide
)
Description:
Bug 11592 - opac scripts do not respect marc tag visibility
Filename:
MIME Type:
Creator:
Mark Tompsett
Created:
2014-05-07 03:48:13 UTC
Size:
19.42 KB
patch
obsolete
>From 608ddc2fa93e085a7094dd6649cbb894cd15c75c 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. > >Signed-off-by: Robin Sheat <robin@catalyst.net.nz> >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> >--- > C4/Biblio.pm | 108 ++++++++++++++++++++++++++++++++++++++++++++++- > C4/XSLT.pm | 17 +++++--- > opac/opac-ISBDdetail.pl | 6 ++- > opac/opac-MARCdetail.pl | 3 +- > opac/opac-detail.pl | 17 +++++++- > opac/opac-export.pl | 9 ++-- > opac/opac-showmarc.pl | 6 +++ > t/db_dependent/Biblio.t | 64 ++++++++++++++++++++++++++-- > 8 files changed, 212 insertions(+), 18 deletions(-) > >diff --git a/C4/Biblio.pm b/C4/Biblio.pm >index f671828..e9657d2 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') { >@@ -1878,7 +1982,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 242278d..edb826c 100644 >--- a/C4/XSLT.pm >+++ b/C4/XSLT.pm >@@ -92,12 +92,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 4bf42f6..24655bb 100755 >--- a/opac/opac-ISBDdetail.pl >+++ b/opac/opac-ISBDdetail.pl >@@ -93,11 +93,13 @@ if (scalar @items >= 1) { > } > } > >-my $record = GetMarcBiblio($biblionumber); >-if ( ! $record ) { >+my $unfiltered_record = GetMarcBiblio($biblionumber); >+if ( ! $unfiltered_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 e97ac91..ed003e2 100755 >--- a/opac/opac-detail.pl >+++ b/opac/opac-detail.pl >@@ -122,6 +122,10 @@ if (C4::Context->preference('OpacSuppression')) { > > $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; > >@@ -517,6 +521,7 @@ if ( C4::Context->preference('HighlightOwnItemsOnOPAC') ) { > } > > my $dat = &GetBiblioData($biblionumber); >+my $OpacHideMARC = GetOpacHideMARC($dat->{'frameworkcode'}); > > my $itemtypes = GetItemTypes(); > # imageurl: >@@ -676,7 +681,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 +740,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 e707896..dbfd9b9 100755 >--- a/opac/opac-export.pl >+++ b/opac/opac-export.pl >@@ -32,9 +32,12 @@ 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 ($unfiltered_marc, $error, $frameworkcode)= ('','',''); >+ >+$unfiltered_marc = GetMarcBiblio($biblionumber, 1) if $biblionumber; >+$frameworkcode = GetFrameworkCode($biblionumber) if $biblionumber; >+my $marc = GetFilteredOpacBiblio($unfiltered_marc,$frameworkcode); > >-$marc = GetMarcBiblio($biblionumber, 1) if $biblionumber; > if(!$marc) { > print $query->redirect("/cgi-bin/koha/errors/404.pl"); > exit; >@@ -56,7 +59,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 8cce54d..3494c30 100755 >--- a/opac/opac-showmarc.pl >+++ b/opac/opac-showmarc.pl >@@ -60,6 +60,12 @@ 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 = 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 3c0a159..ca869dc 100755 >--- a/t/db_dependent/Biblio.t >+++ b/t/db_dependent/Biblio.t >@@ -17,7 +17,7 @@ > > use Modern::Perl; > >-use Test::More tests => 3; >+use Test::More tests => 7; > use Test::MockModule; > > use MARC::Record; >@@ -32,6 +32,19 @@ my $dbh = C4::Context->dbh; > $dbh->{AutoCommit} = 0; > $dbh->{RaiseError} = 1; > >+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.'); >+ > # Mocking variables > my $context = new Test::MockModule('C4::Context'); > >@@ -153,6 +166,51 @@ sub run_tests { > $controlnumber = GetMarcControlnumber( $marc_record, $marcflavour ); > is( $controlnumber, '123456789X', 'GetMarcControlnumber handles records with 001' ); > >+ # 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 $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($title_field,'a'); >+ my $filtered_title = $filtered_record1->subfield($title_field,'a'); >+ ok ( defined($title) && !defined($filtered_title), 'GetFilteredOpacBiblio filtered title as expected.'); >+ >+ $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.'); >+ } >+ > ## Testing GetMarcISBN > my $record_for_isbn = MARC::Record->new(); > my $isbns = GetMarcISBN( $record_for_isbn, $marcflavour ); >@@ -241,13 +299,13 @@ sub create_issn_field { > } > > subtest 'MARC21' => sub { >- plan tests => 25; >+ plan tests => 31; > run_tests('MARC21'); > $dbh->rollback; > }; > > subtest 'UNIMARC' => sub { >- plan tests => 25; >+ plan tests => 31; > run_tests('UNIMARC'); > $dbh->rollback; > }; >-- >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