From b2148282bf5ee3555c73e8bdfb2bbc3119242e68 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@biblibre.com>
Date: Tue, 21 Apr 2015 16:02:01 +0200
Subject: [PATCH] [PASSED QA] Bug 10938: Item columns displayed in random order

On the MARC detail page, the columns are displayed in a random order.
This means that you can open 2 different records and see the columns
displayed in a different order.

Test plan:
Go on different MARC detail view and confirm that 1/ all fields are
present and 2/ they always are displayed in the same order

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
1/ & 2/ correct
No errors

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
---
 catalogue/MARCdetail.pl                            | 54 ++++++++--------------
 .../prog/en/modules/catalogue/MARCdetail.tt        | 12 ++---
 2 files changed, 26 insertions(+), 40 deletions(-)

diff --git a/catalogue/MARCdetail.pl b/catalogue/MARCdetail.pl
index e8f6949..3462b7d 100755
--- a/catalogue/MARCdetail.pl
+++ b/catalogue/MARCdetail.pl
@@ -59,6 +59,7 @@ use C4::Members; # to use GetMember
 use C4::Serials;    #uses getsubscriptionsfrombiblionumber GetSubscriptionsFromBiblionumber
 use C4::Search;		# enabled_staff_search_views
 
+use List::MoreUtils qw( uniq );
 
 my $query        = new CGI;
 my $dbh          = C4::Context->dbh;
@@ -259,55 +260,39 @@ for ( my $tabloop = 0 ; $tabloop <= 10 ; $tabloop++ ) {
 my @fields = $record->fields();
 my %witness
   ; #---- stores the list of subfields used at least once, with the "meaning" of the code
-my @big_array;
+my @item_subfield_codes;
+my @item_loop;
 my $norequests = 1;
 foreach my $field (@fields) {
     next if ( $field->tag() < 10 );
     my @subf = $field->subfields;
-    my %this_row;
+    my $item;
 
     # loop through each subfield
     for my $i ( 0 .. $#subf ) {
         next if ( $tagslib->{ $field->tag() }->{ $subf[$i][0] }->{tab} ne 10 );
         next if ( $tagslib->{ $field->tag() }->{ $subf[$i][0] }->{hidden} =~ /-7|-4|-3|-2|2|3|5|8/);
+        push @item_subfield_codes, $subf[$i][0];
         $witness{ $subf[$i][0] } =
         $tagslib->{ $field->tag() }->{ $subf[$i][0] }->{lib};
-        $this_row{ $subf[$i][0] } = GetAuthorisedValueDesc( $field->tag(),
+        $item->{ $subf[$i][0] } = GetAuthorisedValueDesc( $field->tag(),
                         $subf[$i][0], $subf[$i][1], '', $tagslib) || $subf[$i][1];
         $norequests = 0 if $subf[$i][1] ==0 and $tagslib->{ $field->tag() }->{ $subf[$i][0] }->{kohafield} eq 'items.notforloan';
     }
-    if (%this_row) {
-        push( @big_array, \%this_row );
-    }
+    push @item_loop, $item if $item;
 }
 
 my ($holdingbrtagf,$holdingbrtagsubf) = &GetMarcFromKohaField("items.holdingbranch",$frameworkcode);
-@big_array = sort {$a->{$holdingbrtagsubf} cmp $b->{$holdingbrtagsubf}} @big_array;
-
-#fill big_row with missing datas
-foreach my $subfield_code ( keys(%witness) ) {
-    for ( my $i = 0 ; $i <= $#big_array ; $i++ ) {
-        $big_array[$i]{$subfield_code} = "&nbsp;"
-          unless ( $big_array[$i]{$subfield_code} );
-    }
-}
-
-# now, construct template !
-my @item_value_loop;
-my @header_value_loop;
-for ( my $i = 0 ; $i <= $#big_array ; $i++ ) {
-    my $items_data;
-    foreach my $subfield_code ( keys(%witness) ) {
-        $items_data .= "<td>" . $big_array[$i]{$subfield_code} . "</td>";
+@item_loop = sort {$a->{$holdingbrtagsubf} cmp $b->{$holdingbrtagsubf}} @item_loop;
+
+@item_subfield_codes = uniq @item_subfield_codes;
+# fill item info
+my @item_header_loop;
+for my $subfield_code ( @item_subfield_codes ) {
+    push @item_header_loop, $witness{$subfield_code};
+    for my $item_data ( @item_loop ) {
+        $item_data->{$subfield_code} ||= "&nbsp;"
     }
-    my %row_data;
-    $row_data{item_value} = $items_data;
-    push( @item_value_loop, \%row_data );
-}
-foreach my $subfield_code ( keys(%witness) ) {
-    my %header_value;
-    $header_value{header_value} = $witness{$subfield_code};
-    push( @header_value_loop, \%header_value );
 }
 
 my $subscriptionscount = CountSubscriptionFromBiblionumber($biblionumber);
@@ -322,9 +307,10 @@ if ($subscriptionscount) {
 }
 
 $template->param (
-    norequests              => $norequests, 
-    item_loop               => \@item_value_loop,
-    item_header_loop        => \@header_value_loop,
+    norequests              => $norequests,
+    item_loop               => \@item_loop,
+    item_header_loop        => \@item_header_loop,
+    item_subfield_codes     => \@item_subfield_codes,
     biblionumber            => $biblionumber,
     popup                   => $popup,
     hide_marc               => C4::Context->preference('hide_marc'),
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/MARCdetail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/MARCdetail.tt
index 698c960..4c0c115 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/MARCdetail.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/MARCdetail.tt
@@ -167,15 +167,15 @@ function Changefwk(FwkList) {
     <div id="tab10XX">
         <table>
                 <tr>
-                    [% FOREACH item_header_loo IN item_header_loop %]
-                        <th>
-                            [% item_header_loo.header_value %]
-                        </th>
+                    [% FOREACH header IN item_header_loop %]
+                        <th>[% header %]</th>
                     [% END %]
                 </tr>
-                [% FOREACH item_loo IN item_loop %]
+                [% FOREACH item IN item_loop %]
                     <tr>
-                        [% item_loo.item_value %]
+                        [% FOREACH sf_code IN item_subfield_codes %]
+                            <td>[% item.$sf_code %]</td>
+                        [% END %]
                     </tr>
                 [% END %]
         </table>
-- 
1.9.1