Bug 12409: Fix fields order on exporting to bibtex
authorJonathan Druart <jonathan.druart@biblibre.com>
Thu, 12 Jun 2014 09:06:58 +0000 (11:06 +0200)
committerTomas Cohen Arazi <tomascohen@gmail.com>
Mon, 7 Jul 2014 13:13:02 +0000 (10:13 -0300)
The C4::Record::marc2bibtex subroutine supposes hashref keys are sorted
which is wrong with perl 5.18

Note that the t/db_dependent/Record.t fails without this patch.

Test plan (for perl >= 5.18 only):
1/ prove t/db_dependent/Record.t should return green
2/ Try to export a record to bibtex and verify the order is correct
(should be author, title, year, published, etc.).

http://bugs.koha-community.org/show_bug.cgi?id=12409

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Wrong Bug number on heading.
Work as described, test pass, no koha-qa errors.

The problem is to think that a hash returns
keys in a particular order, that's not true
and no matter which perl version.

Code as was left is... misleading.
Comments talks about a hash, which is no more.
On array asignment "a => b" is equivalent to "a, b",
but the former is usually used on hashes, so a
replacement of '=>' by ',' could clarify what are we storing.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Fixing the comments in a follow-up patch.
Tests pass now without problems and records export ok.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
C4/Record.pm

index e291486..cd56858 100644 (file)
@@ -661,54 +661,59 @@ sub marc2bibtex {
     $author = join ' and ', @texauthors;
 
     # Defining the conversion hash according to the marcflavour
-    my %bh;
+    my @bh;
     if ( $marcflavour eq "UNIMARC" ) {
-       
-       # FIXME, TODO : handle repeatable fields
-       # TODO : handle more types of documents
-
-       # Unimarc to bibtex hash
-       %bh = (
-
-           # Mandatory
-           author    => $author,
-           title     => $record->subfield("200", "a") || "",
-           editor    => $record->subfield("210", "g") || "",
-           publisher => $record->subfield("210", "c") || "",
-           year      => $record->subfield("210", "d") || $record->subfield("210", "h") || "",
-
-           # Optional
-           volume  =>  $record->subfield("200", "v") || "",
-           series  =>  $record->subfield("225", "a") || "",
-           address =>  $record->subfield("210", "a") || "",
-           edition =>  $record->subfield("205", "a") || "",
-           note    =>  $record->subfield("300", "a") || "",
-           url     =>  $record->subfield("856", "u") || ""
-       );
+
+        # FIXME, TODO : handle repeatable fields
+        # TODO : handle more types of documents
+
+        # Unimarc to bibtex hash
+        @bh = (
+
+            # Mandatory
+            author    => $author,
+            title     => $record->subfield("200", "a") || "",
+            editor    => $record->subfield("210", "g") || "",
+            publisher => $record->subfield("210", "c") || "",
+            year      => $record->subfield("210", "d") || $record->subfield("210", "h") || "",
+
+            # Optional
+            volume  =>  $record->subfield("200", "v") || "",
+            series  =>  $record->subfield("225", "a") || "",
+            address =>  $record->subfield("210", "a") || "",
+            edition =>  $record->subfield("205", "a") || "",
+            note    =>  $record->subfield("300", "a") || "",
+            url     =>  $record->subfield("856", "u") || ""
+        );
     } else {
 
-       # Marc21 to bibtex hash
-       %bh = (
-
-           # Mandatory
-           author    => $author,
-           title     => $record->subfield("245", "a") || "",
-           editor    => $record->subfield("260", "f") || "",
-        publisher => $record->subfield("264", "b") || $record->subfield("260", "b") || "",
-        year      => $record->subfield("264", "c") || $record->subfield("260", "c") || $record->subfield("260", "g") || "",
-
-           # Optional
-           # unimarc to marc21 specification says not to convert 200$v to marc21
-           series  =>  $record->subfield("490", "a") || "",
-        address =>  $record->subfield("264", "a") || $record->subfield("260", "a") || "",
-           edition =>  $record->subfield("250", "a") || "",
-           note    =>  $record->subfield("500", "a") || "",
-           url     =>  $record->subfield("856", "u") || ""
-       );
+        # Marc21 to bibtex hash
+        @bh = (
+
+            # Mandatory
+            author    => $author,
+            title     => $record->subfield("245", "a") || "",
+            editor    => $record->subfield("260", "f") || "",
+            publisher => $record->subfield("264", "b") || $record->subfield("260", "b") || "",
+            year      => $record->subfield("264", "c") || $record->subfield("260", "c") || $record->subfield("260", "g") || "",
+
+            # Optional
+            # unimarc to marc21 specification says not to convert 200$v to marc21
+            series  =>  $record->subfield("490", "a") || "",
+            address =>  $record->subfield("264", "a") || $record->subfield("260", "a") || "",
+            edition =>  $record->subfield("250", "a") || "",
+            note    =>  $record->subfield("500", "a") || "",
+            url     =>  $record->subfield("856", "u") || ""
+        );
     }
 
     $tex .= "\@book{";
-    $tex .= join(",\n", $id, map { $bh{$_} ? qq(\t$_ = {$bh{$_}}) : () } keys %bh);
+    my @elt;
+    for ( my $i = 0 ; $i < scalar( @bh ) ; $i = $i + 2 ) {
+        next unless $bh[$i+1];
+        push @elt, qq|\t$bh[$i] = {$bh[$i+1]}|;
+    }
+    $tex .= join(",\n", $id, @elt);
     $tex .= "\n}\n";
 
     return $tex;