From b732b6ae3998626bc6f383da8016c9d80e0ed486 Mon Sep 17 00:00:00 2001 From: Mark Tompsett Date: Wed, 20 Apr 2016 21:49:41 -0400 Subject: [PATCH] Bug 11592: MARCView and ISBD followup There are still some leaks, but it is not as a result of the filter, but rather a result of poorly written template files. Bug fixing template files is beyond the scope of this set of patches. TEST PLAN --------- 1) Backup your DB 2) run the following SQL on your DB. > UPDATE marc_subfield_structure set hidden=-8; -- this should set EVERYTHING to hidden across the board. 3) In staff client, set OPACXSLTDetailsDisplay to blank 4) In OPAC, view any detail. -- Normal view may mostly leak values still. -- MARC view may leak values. -- ISBD view may leak values. 5) In staff client, set OPACXSLTDetailsDisplay to default 6) In OPAC, view any detail. -- same issues as step 4 -- 'View Plain' may leak too. 7) 'Save record' -> 'Dublin Core' 8) Apply this patch 9) run koha qa test tools -- should be fine 10) prove -v t/db_dependent/Filter_MARC_ViewPolicy.t -- should pass -- this proves Koha/Filter/MARC/ViewPolicy.pm tweaks too 11) In OPAC, view any detail. -- Normal view: -- Material type comes from the LEADER field. -- Lists this is on will still display -- 'Tags from this library' will still display -- Item information in table will still display (THIS IS BEYOND SCOPE) -- MARC view: -- Record number is leaked (THIS IS BEYOND SCOPE) -- 'View plain' leaks LEADER field. -- ISBD view may leak field headings, but not values. (THIS IS BEYOND SCOPE) 12) In staff client, set OPACXSLTDetailsDisplay to blank 13) In OPAC, view any detail. -- same kind of output as step 10 14) 'Save record' -> BIBTEXT -- Should be next to nothing leaked. 15) 'Save record' -> Dublin Core -- Should be the same or less leaked between the two versions. -- (XML FILTERING IS BEYOND SCOPE) 16) In the staff client, go view the same record. -- it should be mostly hidden in ISBD View. 17) run the following SQL on your DB. > UPDATE marc_subfield_structure set hidden=1; -- this should set EVERYTHING to hidden in OPAC, but not the STAFF across the board. 18) Refresh the staff ISBD page -- values should reappear. 19) View the ISBD details in the OPAC -- values should still be hidden. 20) Check out the OPAC Cart and List -- while the intermediate pages may still leak the download links should leak very minimally. -- (CARTS AND LISTS ARE BEYOND SCOPE, THOUGH THE INTRANET ISBD AND SOME CART/LIST STUFF WERE FIXED BECAUSE OF THE GetISBDView REFACTOR) Expectations: Before Patch - all the OPAC Detail pages will display things After Patch - all the OPAC Detail pages will display much less, and hopefully nothing (though there are known limits). the ISBD detail page in the Staff client will be filtered as well based on STAFF settings. The saving/exporting should generate nearly empty files. Signed-off-by: Nick Clemens --- C4/Biblio.pm | 20 ++++++++++++++------ catalogue/ISBDdetail.pl | 33 +++++++++++++++++++++++++++++---- opac/opac-ISBDdetail.pl | 9 +++++++-- opac/opac-MARCdetail.pl | 12 +++++++++--- opac/opac-basket.pl | 7 +++++-- opac/opac-downloadcart.pl | 17 +++++++++++++++-- opac/opac-downloadshelf.pl | 16 +++++++++++++--- opac/opac-export.pl | 7 ++++++- opac/opac-shelves.pl | 9 +++++---- 9 files changed, 103 insertions(+), 27 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 0804f3e..d4726a5 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -917,19 +917,27 @@ sub GetBiblioFromItemNumber { =head2 GetISBDView - $isbd = &GetISBDView($biblionumber); + $isbd = &GetISBDView({ + 'record' => $marc_record, + 'template' => $interface, # opac/intranet + 'framework' => $framework, + }); Return the ISBD view which can be included in opac and intranet =cut sub GetISBDView { - my ( $biblionumber, $template ) = @_; - my $record = GetMarcBiblio($biblionumber, 1); - $template ||= ''; - my $sysprefname = $template eq 'opac' ? 'opacisbd' : 'isbd'; + my ( $params ) = @_; + + # Expecting record WITH items. + my $record = $params->{record}; return unless defined $record; - my $itemtype = &GetFrameworkCode($biblionumber); + + my $template = $params->{template} // q{}; + my $sysprefname = $template eq 'opac' ? 'opacisbd' : 'isbd'; + my $framework = $params->{framework}; + my $itemtype = $framework; my ( $holdingbrtagf, $holdingbrtagsubf ) = &GetMarcFromKohaField( "items.holdingbranch", $itemtype ); my $tagslib = &GetMarcStructure( 1, $itemtype ); diff --git a/catalogue/ISBDdetail.pl b/catalogue/ISBDdetail.pl index 37c739c..55a45cd 100755 --- a/catalogue/ISBDdetail.pl +++ b/catalogue/ISBDdetail.pl @@ -47,6 +47,7 @@ use C4::Members; # to use GetMember use C4::Serials; # CountSubscriptionFromBiblionumber use C4::Search; # enabled_staff_search_views use C4::Acquisition qw(GetOrdersByBiblionumber); +use Koha::RecordProcessor; #---- Internal function @@ -68,16 +69,41 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user( } ); -my $res = GetISBDView($biblionumber, "intranet"); -if ( not defined $res ) { +if ( not defined $biblionumber ) { # biblionumber invalid -> report and exit $template->param( unknownbiblionumber => 1, - biblionumber => $biblionumber + biblionumber => $biblionumber ); output_html_with_http_headers $query, $cookie, $template->output; exit; } +my $record_unfiltered = GetMarcBiblio($biblionumber,1); +my $record_processor = Koha::RecordProcessor->new({ + filters => 'ViewPolicy', + options => { + interface => 'intranet', + }, +}); +my $record_filtered = $record_unfiltered->clone(); +my $record = $record_processor->process($record_filtered); + +if ( not defined $record ) { + # biblionumber invalid -> report and exit + $template->param( unknownbiblionumber => 1, + biblionumber => $biblionumber + ); + output_html_with_http_headers $query, $cookie, $template->output; + exit; +} + +my $framework = GetFrameworkCode( $biblionumber ); +my $res = GetISBDView({ + 'record' => $record, + 'template' => 'intranet', + 'framework' => $framework, +}); + if($query->cookie("holdfor")){ my $holdfor_patron = GetMember('borrowernumber' => $query->cookie("holdfor")); $template->param( @@ -101,7 +127,6 @@ if ($subscriptionsnumber) { subscriptiontitle => $subscriptiontitle, ); } -my $record = GetMarcBiblio($biblionumber); $template->param ( ISBD => $res, diff --git a/opac/opac-ISBDdetail.pl b/opac/opac-ISBDdetail.pl index d956af2..d77f473 100755 --- a/opac/opac-ISBDdetail.pl +++ b/opac/opac-ISBDdetail.pl @@ -92,7 +92,7 @@ if (scalar @items >= 1) { } } -my $record_unfiltered = GetMarcBiblio($biblionumber); +my $record_unfiltered = GetMarcBiblio($biblionumber,1); if ( ! $record_unfiltered ) { print $query->redirect("/cgi-bin/koha/errors/404.pl"); exit; @@ -150,7 +150,12 @@ $template->param( my $norequests = 1; my $allow_onshelf_holds; -my $res = GetISBDView($biblionumber, "opac"); +my $framework = GetFrameworkCode( $biblionumber ); +my $res = GetISBDView({ + 'record' => $record, + 'template' => 'opac', + 'framework' => $framework +}); my $itemtypes = GetItemTypes(); my $borrower = GetMember( 'borrowernumber' => $loggedinuser ); diff --git a/opac/opac-MARCdetail.pl b/opac/opac-MARCdetail.pl index 065b570..fce1dc8 100755 --- a/opac/opac-MARCdetail.pl +++ b/opac/opac-MARCdetail.pl @@ -57,6 +57,7 @@ use C4::Members; use C4::Acquisition; use C4::Koha; use List::MoreUtils qw( any uniq ); +use Koha::RecordProcessor; my $query = new CGI; @@ -84,11 +85,15 @@ my $tagslib = &GetMarcStructure( 0, $itemtype ); my ($tag_itemnumber,$subtag_itemnumber) = &GetMarcFromKohaField('items.itemnumber',$itemtype); my $biblio = GetBiblioData($biblionumber); $biblionumber = $biblio->{biblionumber}; -my $record = GetMarcBiblio($biblionumber, 1); -if ( ! $record ) { +my $record_unfiltered = GetMarcBiblio($biblionumber, 1); +if ( ! $record_unfiltered ) { print $query->redirect("/cgi-bin/koha/errors/404.pl"); exit; } +my $record_processor = Koha::RecordProcessor->new({ filters => 'ViewPolicy' }); +my $record_filtered = $record_unfiltered->clone(); +my $record = $record_processor->process($record_filtered); + # open template my ( $template, $loggedinuser, $cookie ) = get_template_and_user( { @@ -103,7 +108,8 @@ 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; #<=0 is OPAC visible. +) if $tagslib->{$bt_tag}->{$bt_subtag}->{hidden} <= 0 && # <=0 OPAC visible. + $tagslib->{$bt_tag}->{$bt_subtag}->{hidden} > -8; # except -8; # get biblionumbers stored in the cart if(my $cart_list = $query->cookie("bib_list")){ diff --git a/opac/opac-basket.pl b/opac/opac-basket.pl index d41b7a7..43e1d5f 100755 --- a/opac/opac-basket.pl +++ b/opac/opac-basket.pl @@ -26,6 +26,7 @@ use C4::Items; use C4::Circulation; use C4::Auth; use C4::Output; +use Koha::RecordProcessor; my $query = new CGI; @@ -57,13 +58,15 @@ if (C4::Context->preference('TagsEnabled')) { } } - +my $record_processor = Koha::RecordProcessor->new({ filters => 'ViewPolicy' }); foreach my $biblionumber ( @bibs ) { $template->param( biblionumber => $biblionumber ); my $dat = &GetBiblioData($biblionumber); next unless $dat; - my $record = &GetMarcBiblio($biblionumber); + my $record_unfiltered = &GetMarcBiblio($biblionumber); + my $record_filtered = $record_unfiltered->clone(); + my $record = $record_processor->process($record_filtered); next unless $record; my $marcnotesarray = GetMarcNotes( $record, $marcflavour ); my $marcauthorsarray = GetMarcAuthors( $record, $marcflavour ); diff --git a/opac/opac-downloadcart.pl b/opac/opac-downloadcart.pl index c5574e3..506e556 100755 --- a/opac/opac-downloadcart.pl +++ b/opac/opac-downloadcart.pl @@ -30,6 +30,7 @@ use C4::Output; use C4::Record; use C4::Ris; use C4::Csv; +use Koha::RecordProcessor; use utf8; my $query = new CGI; @@ -62,9 +63,16 @@ if ($bib_list && $format) { # Other formats } else { + my $record_processor = Koha::RecordProcessor->new({ + filters => 'ViewPolicy' + }); foreach my $biblio (@bibs) { - my $record = GetMarcBiblio($biblio, 1); + my $record_unfiltered = GetMarcBiblio($biblio, 1); + my $record_filtered = $record_unfiltered->clone(); + my $record = + $record_processor->process($record_filtered); + next unless $record; if ($format eq 'iso2709') { @@ -77,7 +85,12 @@ if ($bib_list && $format) { $output .= marc2bibtex($record, $biblio); } elsif ( $format eq 'isbd' ) { - $output .= GetISBDView($biblio, "opac"); + my $framework = GetFrameworkCode( $biblio ); + $output .= GetISBDView({ + 'record' => $record, + 'template' => 'opac', + 'framework' => $framework, + }); $extension = "txt"; $type = "text/plain"; } diff --git a/opac/opac-downloadshelf.pl b/opac/opac-downloadshelf.pl index 6102082..4711b51 100755 --- a/opac/opac-downloadshelf.pl +++ b/opac/opac-downloadshelf.pl @@ -30,7 +30,7 @@ use C4::Output; use C4::Record; use C4::Ris; use C4::Csv; - +use Koha::RecordProcessor; use Koha::Virtualshelves; use utf8; @@ -77,10 +77,15 @@ if ( $shelf and $shelf->can_be_viewed( $borrowernumber ) ) { $output = marc2csv(\@biblios, $format); # Other formats } else { + my $record_processor = Koha::RecordProcessor->new({ + filters => 'ViewPolicy' + }); while ( my $content = $contents->next ) { my $biblionumber = $content->biblionumber->biblionumber; - my $record = GetMarcBiblio($biblionumber, 1); + my $record_unfiltered = GetMarcBiblio($biblionumber, 1); + my $record_filtered = $record_unfiltered->clone(); + my $record = $record_processor->process($record_filtered); next unless $record; if ($format eq 'iso2709') { @@ -93,7 +98,12 @@ if ( $shelf and $shelf->can_be_viewed( $borrowernumber ) ) { $output .= marc2bibtex($record, $biblionumber); } elsif ( $format eq 'isbd' ) { - $output .= GetISBDView($biblionumber, "opac"); + my $framework = GetFrameworkCode( $biblionumber ); + $output .= GetISBDView({ + 'record' => $record, + 'template' => 'opac', + 'framework' => $framework, + }); $extension = "txt"; $type = "text/plain"; } diff --git a/opac/opac-export.pl b/opac/opac-export.pl index 0eb0dba..77ec11d 100755 --- a/opac/opac-export.pl +++ b/opac/opac-export.pl @@ -93,7 +93,12 @@ elsif ($format =~ /marcstd/) { $format = 'marcstd'; } elsif ( $format =~ /isbd/ ) { - $marc = GetISBDView($biblionumber, "opac"); + my $framework = GetFrameworkCode( $biblionumber ); + $marc = GetISBDView({ + 'record' => $marc, + 'template' => 'opac', + 'framework' => $framework, + }); $format = 'isbd'; } else { diff --git a/opac/opac-shelves.pl b/opac/opac-shelves.pl index 6e5052d..bad1549 100755 --- a/opac/opac-shelves.pl +++ b/opac/opac-shelves.pl @@ -28,6 +28,7 @@ use C4::Output; use C4::Tags qw( get_tags ); use C4::XSLT; use Koha::Virtualshelves; +use Koha::RecordProcessor; my $query = new CGI; @@ -252,11 +253,13 @@ if ( $op eq 'view' ) { my $lang = $xslfile ? C4::Languages::getlanguage() : undef; my $sysxml = $xslfile ? C4::XSLT::get_xslt_sysprefs() : undef; + my $record_processor = Koha::RecordProcessor->new({ filters => 'ViewPolicy' }); my @items; while ( my $content = $contents->next ) { my $biblionumber = $content->biblionumber->biblionumber; - my $this_item = GetBiblioData($biblionumber); - my $record = GetMarcBiblio($biblionumber); + my $record_unfiltered = GetMarcBiblio($biblionumber); + my $record_filtered = $record_unfiltered->clone(); + my $record = $record_processor->process($record_filtered); if ( $xslfile ) { $this_item->{XSLTBloc} = XSLTParse4Display( $biblionumber, $record, "OPACXSLTResultsDisplay", @@ -265,8 +268,6 @@ if ( $op eq 'view' ) { my $marcflavour = C4::Context->preference("marcflavour"); my $itemtypeinfo = getitemtypeinfo( $content->biblionumber->biblioitems->first->itemtype, 'opac' ); - $this_item->{imageurl} = $itemtypeinfo->{imageurl}; - $this_item->{description} = $itemtypeinfo->{description}; $this_item->{notforloan} = $itemtypeinfo->{notforloan}; $this_item->{'coins'} = GetCOinSBiblio($record); $this_item->{'subtitle'} = GetRecordValue( 'subtitle', $record, GetFrameworkCode( $biblionumber ) ); -- 2.1.4