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 <tomascohen@theke.io>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

diff --git a/catalogue/ISBDdetail.pl b/catalogue/ISBDdetail.pl
index 02c06a9..cb5081f 100755 (executable)
--- 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 d77f473..3dc15df 100755 (executable)
--- 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 fce1dc8..b16538a 100755 (executable)
--- 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 43e1d5f..4fd2108 100755 (executable)
--- 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 @@ 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 b2046a8..46abc6f 100755 (executable)
--- 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 c3217f0..1cf0de0 100755 (executable)
--- 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 0218dad..9f98d91 100755 (executable)
--- 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 9429d95..d5f4c4f 100755 (executable)
--- 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 bb6531a..67c7313 100755 (executable)
--- 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 @@ 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 f02ab7c..18be49e 100755 (executable)
--- 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";