From ea275693348b7d7e7a230de1cdbd3cf3a0ecb826 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 22 Aug 2016 11:00:41 -0300 Subject: [PATCH] Bug 11592: (QA followup) Simplify code Koha::RecordProcessor and the defined filters are supposed to bring us joy and happiness. Let's keep the code compact, simple and clean. This patch removes record cloning all over the place. Signed-off-by: Tomas Cohen Arazi Signed-off-by: Mark Tompsett Signed-off-by: Kyle M Hall --- catalogue/ISBDdetail.pl | 11 +++-------- opac/opac-ISBDdetail.pl | 7 +++---- opac/opac-MARCdetail.pl | 7 +++---- opac/opac-basket.pl | 8 +++----- opac/opac-detail.pl | 7 +++---- opac/opac-downloadcart.pl | 6 ++---- opac/opac-downloadshelf.pl | 7 +++---- opac/opac-export.pl | 17 +++++++---------- opac/opac-shelves.pl | 6 +++--- opac/opac-showmarc.pl | 29 ++++++++++++----------------- 10 files changed, 42 insertions(+), 63 deletions(-) diff --git a/catalogue/ISBDdetail.pl b/catalogue/ISBDdetail.pl index 02c06a9f3b..cb5081f131 100755 --- a/catalogue/ISBDdetail.pl +++ b/catalogue/ISBDdetail.pl @@ -33,8 +33,7 @@ This script needs a biblionumber as parameter =cut -use strict; -#use warnings; FIXME - Bug 2505 +use Modern::Perl; use HTML::Entities; use C4::Auth; @@ -51,9 +50,6 @@ use C4::Acquisition qw(GetOrdersByBiblionumber); use Koha::RecordProcessor; -#---- Internal function - - my $query = new CGI; my $dbh = C4::Context->dbh; @@ -80,15 +76,14 @@ if ( not defined $biblionumber ) { exit; } -my $record_unfiltered = GetMarcBiblio($biblionumber,1); +my $record = 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); +$record_processor->process($record); if ( not defined $record ) { # biblionumber invalid -> report and exit diff --git a/opac/opac-ISBDdetail.pl b/opac/opac-ISBDdetail.pl index d77f473c62..3dc15df31c 100755 --- a/opac/opac-ISBDdetail.pl +++ b/opac/opac-ISBDdetail.pl @@ -92,14 +92,13 @@ if (scalar @items >= 1) { } } -my $record_unfiltered = GetMarcBiblio($biblionumber,1); -if ( ! $record_unfiltered ) { +my $record = GetMarcBiblio($biblionumber,1); +if ( ! $record ) { 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); +$record_processor->process($record); # some useful variables for enhanced content; # in each case, we're grabbing the first value we find in diff --git a/opac/opac-MARCdetail.pl b/opac/opac-MARCdetail.pl index fce1dc880c..b16538ae54 100755 --- a/opac/opac-MARCdetail.pl +++ b/opac/opac-MARCdetail.pl @@ -85,14 +85,13 @@ my $tagslib = &GetMarcStructure( 0, $itemtype ); my ($tag_itemnumber,$subtag_itemnumber) = &GetMarcFromKohaField('items.itemnumber',$itemtype); my $biblio = GetBiblioData($biblionumber); $biblionumber = $biblio->{biblionumber}; -my $record_unfiltered = GetMarcBiblio($biblionumber, 1); -if ( ! $record_unfiltered ) { +my $record = GetMarcBiblio($biblionumber, 1); +if ( ! $record ) { 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); +$record_processor->process($record); # open template my ( $template, $loggedinuser, $cookie ) = get_template_and_user( diff --git a/opac/opac-basket.pl b/opac/opac-basket.pl index 43e1d5f535..4fd2108c25 100755 --- a/opac/opac-basket.pl +++ b/opac/opac-basket.pl @@ -15,9 +15,8 @@ # You should have received a copy of the GNU General Public License # along with Koha; if not, see . +use Modern::Perl; -use strict; -use warnings; use CGI qw ( -utf8 ); use C4::Koha; use C4::Biblio; @@ -64,9 +63,8 @@ foreach my $biblionumber ( @bibs ) { my $dat = &GetBiblioData($biblionumber); next unless $dat; - my $record_unfiltered = &GetMarcBiblio($biblionumber); - my $record_filtered = $record_unfiltered->clone(); - my $record = $record_processor->process($record_filtered); + my $record = &GetMarcBiblio($biblionumber); + $record_processor->process($record); next unless $record; my $marcnotesarray = GetMarcNotes( $record, $marcflavour ); my $marcauthorsarray = GetMarcAuthors( $record, $marcflavour ); diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl index b2046a810e..46abc6f12e 100755 --- a/opac/opac-detail.pl +++ b/opac/opac-detail.pl @@ -84,14 +84,13 @@ if (scalar @all_items >= 1) { } } -my $record_unfiltered = GetMarcBiblio($biblionumber); -if ( ! $record_unfiltered ) { +my $record = GetMarcBiblio($biblionumber); +if ( ! $record ) { print $query->redirect("/cgi-bin/koha/errors/404.pl"); # escape early exit; } my $record_processor = Koha::RecordProcessor->new({ filters => 'ViewPolicy' }); -my $record_filtered = $record_unfiltered->clone(); -my $record = $record_processor->process($record_filtered); +$record_processor->process($record); # redirect if opacsuppression is enabled and biblio is suppressed if (C4::Context->preference('OpacSuppression')) { diff --git a/opac/opac-downloadcart.pl b/opac/opac-downloadcart.pl index c3217f03fb..1cf0de01ec 100755 --- a/opac/opac-downloadcart.pl +++ b/opac/opac-downloadcart.pl @@ -68,10 +68,8 @@ if ($bib_list && $format) { }); foreach my $biblio (@bibs) { - my $record_unfiltered = GetMarcBiblio($biblio, 1); - my $record_filtered = $record_unfiltered->clone(); - my $record = - $record_processor->process($record_filtered); + my $record = GetMarcBiblio($biblio, 1); + $record_processor->process($record); next unless $record; diff --git a/opac/opac-downloadshelf.pl b/opac/opac-downloadshelf.pl index 0218dad194..9f98d91b33 100755 --- a/opac/opac-downloadshelf.pl +++ b/opac/opac-downloadshelf.pl @@ -62,7 +62,7 @@ if ( $shelf and $shelf->can_be_viewed( $borrowernumber ) ) { my $contents = $shelf->get_contents; - my $marcflavour = C4::Context->preference('marcflavour'); + my $marcflavour = C4::Context->preference('marcflavour'); my $output; my $extension; my $type; @@ -82,9 +82,8 @@ if ( $shelf and $shelf->can_be_viewed( $borrowernumber ) ) { while ( my $content = $contents->next ) { my $biblionumber = $content->biblionumber->biblionumber; - my $record_unfiltered = GetMarcBiblio($biblionumber, 1); - my $record_filtered = $record_unfiltered->clone(); - my $record = $record_processor->process($record_filtered); + my $record = GetMarcBiblio($biblionumber, 1); + $record_processor->process($record); next unless $record; if ($format eq 'iso2709') { diff --git a/opac/opac-export.pl b/opac/opac-export.pl index 9429d95b8d..d5f4c4f61f 100755 --- a/opac/opac-export.pl +++ b/opac/opac-export.pl @@ -35,9 +35,11 @@ my $biblionumber = $query->param("bib")||0; $biblionumber = int($biblionumber); my $error = q{}; -my $marc_unfiltered; -$marc_unfiltered = GetMarcBiblio($biblionumber, 1) if $biblionumber; -if(!$marc_unfiltered) { +my $include_items = ($format =~ /bibtex/) ? 0 : 1; +my $marc = GetMarcBiblio($biblionumber, $include_items) + if $biblionumber; + +if(!$marc) { print $query->redirect("/cgi-bin/koha/errors/404.pl"); exit; } @@ -45,12 +47,7 @@ if(!$marc_unfiltered) { # ASSERT: There is a biblionumber, because GetMarcBiblio returned something. my $record_processor = Koha::RecordProcessor->new({ filters => 'ViewPolicy' }); -my $marc_filtered = $marc_unfiltered->clone(); -my $marc = $record_processor->process($marc_filtered); - -my $marc_noitems_unfiltered = GetMarcBiblio($biblionumber); -my $marc_noitems_filtered = $marc_noitems_unfiltered->clone(); -my $marc_noitems = $record_processor->process($marc_noitems_filtered); +$record_processor->process($marc); if ($format =~ /endnote/) { $marc = marc2endnote($marc); @@ -69,7 +66,7 @@ elsif ($format =~ /ris/) { $format = 'ris'; } elsif ($format =~ /bibtex/) { - $marc = marc2bibtex($marc_noitems,$biblionumber); + $marc = marc2bibtex($marc,$biblionumber); $format = 'bibtex'; } elsif ($format =~ /dc$/) { diff --git a/opac/opac-shelves.pl b/opac/opac-shelves.pl index bb6531a409..67c7313ebc 100755 --- a/opac/opac-shelves.pl +++ b/opac/opac-shelves.pl @@ -18,6 +18,7 @@ # along with Koha; if not, see . use Modern::Perl; + use CGI qw ( -utf8 ); use C4::Auth; use C4::Biblio; @@ -259,9 +260,8 @@ if ( $op eq 'view' ) { while ( my $content = $contents->next ) { my $biblionumber = $content->biblionumber->biblionumber; my $this_item = GetBiblioData($biblionumber); - my $record_unfiltered = GetMarcBiblio($biblionumber); - my $record_filtered = $record_unfiltered->clone(); - my $record = $record_processor->process($record_filtered); + my $record = GetMarcBiblio($biblionumber); + $record_processor->process($record); if ( $xslfile ) { $this_item->{XSLTBloc} = XSLTParse4Display( $biblionumber, $record, "OPACXSLTListsDisplay", diff --git a/opac/opac-showmarc.pl b/opac/opac-showmarc.pl index f02ab7cf69..18be49e301 100755 --- a/opac/opac-showmarc.pl +++ b/opac/opac-showmarc.pl @@ -39,34 +39,29 @@ $biblionumber = int($biblionumber); my $importid= $input->param('importid'); my $view= $input->param('viewas') || 'marc'; -my $record_unfiltered; +my $record_processor = Koha::RecordProcessor->new({ filters => 'ViewPolicy' }); + +my $record; if ($importid) { my ($marc) = GetImportRecordMarc($importid); - $record_unfiltered = MARC::Record->new_from_usmarc($marc); + $record = MARC::Record->new_from_usmarc($marc); } else { - $record_unfiltered = GetMarcBiblio($biblionumber); + $record = GetMarcBiblio($biblionumber); + my $frameworkcode = GetFrameworkCode($biblionumber); + $record_processor->options({ frameworkcode => $frameworkcode}); } -if(!ref $record_unfiltered) { + +if(!ref $record) { print $input->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); +$record_processor->process($record); if ($view eq 'card' || $view eq 'html') { - # FIXME: GetXmlBiblio needs filtering later. - my $xml = $importid ? $record->as_xml(): GetXmlBiblio($biblionumber); - if (!$importid && $view eq 'html') { - my $unfiltered_record = MARC::Record->new_from_xml($xml); - my $frameworkcode = GetFrameworkCode($biblionumber); - $record_processor->options({ frameworkcode => $frameworkcode}); - my $filtered_record = $record_processor->process($unfiltered_record); - $xml = $filtered_record->as_xml(); - } - my $xsl = $view eq 'card' ? 'compact.xsl' : 'plainMARC.xsl'; + my $xml = $record->as_xml; + my $xsl = $view eq 'card' ? 'compact.xsl' : 'plainMARC.xsl'; my $htdocs = C4::Context->config('opachtdocs'); my ($theme, $lang) = C4::Templates::themelanguage($htdocs, $xsl, 'opac', $input); $xsl = "$htdocs/$theme/$lang/xslt/$xsl"; -- 2.39.5