From acc9dfcf379905d9487e4b481de4f6208d0e0812 Mon Sep 17 00:00:00 2001
From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Date: Wed, 26 Aug 2015 16:35:38 +0200
Subject: [PATCH] Bug 14306: Follow-up for URLs in 555$u
Content-Type: text/plain; charset=utf-8

This patch removes the code for inserting the <a> anchor tags around
URLs in GetMarcNotes (as added originally).
The URLs are placed in separate array elements; the template should take
care of further handling.
The unit test has been adjusted accordingly.

Test plan:
Run the unit test.
---
 C4/Biblio.pm            |   41 ++++++++++++++++++-----------------------
 t/db_dependent/Biblio.t |   10 ++++------
 2 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/C4/Biblio.pm b/C4/Biblio.pm
index be864cd..8e9c9da 100644
--- a/C4/Biblio.pm
+++ b/C4/Biblio.pm
@@ -1746,10 +1746,11 @@ sub GetMarcISSN {
 
 =head2 GetMarcNotes
 
-  $marcnotesarray = GetMarcNotes( $record, $marcflavour );
+    $marcnotesarray = GetMarcNotes( $record, $marcflavour );
 
-Get all notes from the MARC record and returns them in an array.
-The note are stored in different fields depending on MARC flavour
+    Get all notes from the MARC record and returns them in an array.
+    The notes are stored in different fields depending on MARC flavour.
+    MARC21 field 555 gets special attention for the $u subfields.
 
 =cut
 
@@ -1759,34 +1760,28 @@ sub GetMarcNotes {
         carp 'GetMarcNotes called on undefined record';
         return;
     }
-    my $scope;
-    if ( $marcflavour eq "UNIMARC" ) {
-        $scope = '3..';
-    } else {    # assume marc21 if not unimarc
-        $scope = '5..';
-    }
+
+    my $scope = $marcflavour eq "UNIMARC"? '3..': '5..';
     my @marcnotes;
-    my $note = "";
-    my $tag  = "";
-    my $marcnote;
-    my %blacklist = map { $_ => 1 } split(/,/,C4::Context->preference('NotesBlacklist'));
+    my %blacklist = map { $_ => 1 }
+        split( /,/, C4::Context->preference('NotesBlacklist'));
     foreach my $field ( $record->field($scope) ) {
         my $tag = $field->tag();
-        next if $blacklist{$tag};
-
-        my $value = $field->as_string();
+        next if $blacklist{ $tag };
         if( $marcflavour ne 'UNIMARC' && $tag =~ /555/ ) {
-            my @sub= $field->subfield('u');
-            foreach my $s (@sub) {
-                next if $s !~ /^http/;
-                my $i= index( $value, $s);
-                $value= substr( $value,0, $i) . "<a href=\"$s\" target=\"_blank\">$s</a>" . substr( $value, $i + length($s) );
+            # Field 555$u contains URLs
+            # We first push the regular subfields and all $u's separately
+            # Leave further actions to the template
+            push @marcnotes, { marcnote => $field->as_string('abcd') };
+            foreach my $sub ( $field->subfield('u') ) {
+                push @marcnotes, { marcnote => $sub };
             }
+        } else {
+            push @marcnotes, { marcnote => $field->as_string() };
         }
-        push @marcnotes, { marcnote => $value };
     }
     return \@marcnotes;
-}    # end GetMarcNotes
+}
 
 =head2 GetMarcSubjects
 
diff --git a/t/db_dependent/Biblio.t b/t/db_dependent/Biblio.t
index bda1689..0b61fa2 100755
--- a/t/db_dependent/Biblio.t
+++ b/t/db_dependent/Biblio.t
@@ -205,18 +205,16 @@ sub run_tests {
     } else {
         $biblioitemnumbertotest = $updatedrecord->field($biblioitem_tag)->subfield($biblioitem_subfield);
     }
-    is ($newincbiblioitemnumber, $biblioitemnumbertotest);
+    is ($newincbiblioitemnumber, $biblioitemnumbertotest, 'Check newincbiblioitemnumber');
 
     # test for GetMarcNotes
     my $a1= GetMarcNotes( $marc_record, $marcflavour );
     my $field2 = MARC::Field->new( $marcflavour eq 'UNIMARC'? 300: 555, 0, '', a=> 'Some text', u=> 'http://url-1.com', u=> 'nohttp://something_else' );
     $marc_record->append_fields( $field2 );
     my $a2= GetMarcNotes( $marc_record, $marcflavour );
-    my $last= @$a2? $a2->[@$a2-1]->{marcnote}: '';
-    is( @$a2 == @$a1 + 1 && (
-        ( $marcflavour eq 'UNIMARC' && $last eq $field2->as_string() ) ||
-        ( $marcflavour ne 'UNIMARC' && $last =~ /\<a href=/ )),
-        1, 'Test for GetMarcNotes' );
+    is( ( $marcflavour eq 'UNIMARC' && @$a2 == @$a1 + 1 ) ||
+        ( $marcflavour ne 'UNIMARC' && @$a2 == @$a1 + 3 ), 1,
+        'Check the number of returned notes of GetMarcNotes' );
 }
 
 sub mock_marcfromkohafield {
-- 
1.7.10.4