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 <[email protected]> Signed-off-by: Mark Tompsett <[email protected]> Signed-off-by: Kyle M Hall <[email protected]>
sumodgeorge · Sep 8, 2016 · ea27569 · ea27569
1 parent cadf5ae
commit ea27569
Show file tree

Hide file tree

Showing 10 changed files with 42 additions and 63 deletions.
diff --git a/catalogue/ISBDdetail.pl b/catalogue/ISBDdetail.pl
@@ -33,8 +33,7 @@ =head1 FUNCTIONS
 
 =cut
 
-use strict;
-#use warnings; FIXME - Bug 2505
+use Modern::Perl;
 
 use HTML::Entities;
 use C4::Auth;
@@ -51,9 +50,6 @@ =head1 FUNCTIONS
 use Koha::RecordProcessor;
 
 
-#---- Internal function
-
-
 my $query = new CGI;
 my $dbh = C4::Context->dbh;
 
@@ -80,15 +76,14 @@ =head1 FUNCTIONS
        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
@@ -92,14 +92,13 @@ =head1 FUNCTIONS
     }
 }
 
-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
@@ -85,14 +85,13 @@ =head1 DESCRIPTION
 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
@@ -15,9 +15,8 @@
 # You should have received a copy of the GNU General Public License
 # along with Koha; if not, see <http://www.gnu.org/licenses>.
 
+use Modern::Perl;
 
-use strict;
-use warnings;
 use CGI qw ( -utf8 );
 use C4::Koha;
 use C4::Biblio;
@@ -64,9 +63,8 @@
 
     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
@@ -84,14 +84,13 @@ BEGIN
     }
 }
 
-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
@@ -68,10 +68,8 @@
         });
         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
@@ -62,7 +62,7 @@
 
 
         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 @@
             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
@@ -35,22 +35,19 @@
 $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;
 }
 
 # 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 @@
     $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
@@ -18,6 +18,7 @@
 # along with Koha; if not, see <http://www.gnu.org/licenses>.
 
 use Modern::Perl;
+
 use CGI qw ( -utf8 );
 use C4::Auth;
 use C4::Biblio;
@@ -259,9 +260,8 @@
             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
@@ -39,34 +39,29 @@
 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";