Bug 18458: Fix subfields ordering when Merging authority records

While doing a merge, if a subfield(s) precedes the controlled subfields
(like $i before $a in 7XX, which comes before $a -- rare but will
become more and more usual) after merging will be moved to the end.
This is not right.

The patch (with AuthorityMergeMode == loose) make merge consider the
subfields order: all subfields which originally were found before
the first controlled subfield (e.g. $i before $a in 7XX / MARC 21)
will remain in the front, the rest of not controlled subfields that
should remain in the field will come after the subfields copied
from authority rec.

As a bonus, $9 will be placed at the end.

To test:
0) Have AuthorityMergeMode == loose;
1) Have some field in bibliorecord, controlled by an authority, with
extra subfield(s) (i.e. not present in authority rec.) placed at the
beginning of the field;
2) Open (not necessarily edit) and save the connected authority;
3) See that the extra subfieds were moved to the end of the field
   (and $9 is in the front);
4) Apply the patch;
5) Reorder subfields in biblio field;
6) Open (not necessarily edit) and save the connected authority;
7) See that the order has been conserved, additionally $9 the last
   subfield in the field.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended:
Moved field creation to its original place. Changed $9 handling. Simplified the following add_subfields for loop. Edited comments.
Restored the append_fields_ordered call (see comment6).

With this patch, the Merge.t test now passes.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

diff --git a/C4/AuthoritiesMarc.pm b/C4/AuthoritiesMarc.pm
index 6dbe516..165ef36 100644 (file)
--- a/C4/AuthoritiesMarc.pm
+++ b/C4/AuthoritiesMarc.pm
@@ -1447,8 +1447,6 @@ sub merge {
         # We only need it in loose merge mode; replaces the former $exclude
         ? {}
         : { map { ( $_->[0], 1 ); } ( @record_from, @record_to ) };
-    # And we need to add $9 in order not to duplicate
-    $skip_subfields->{9} = 1 if !$overwrite;
 
     my $counteditedbiblio = 0;
     foreach my $biblionumber ( @biblionumbers ) {
@@ -1477,18 +1475,35 @@ sub merge {
                     $newtag,
                     $field->indicator(1),
                     $field->indicator(2),
-                    "9" => $mergeto,
+                    9 => $mergeto, # Needed to create field, will be moved
                 );
-                foreach my $subfield ( grep { $_->[0] ne '9' } @record_to ) {
-                    $field_to->add_subfields( $subfield->[0] => $subfield->[1] );
-                }
+                my ( @prefix, @postfix );
                 if ( !$overwrite ) {
                     # add subfields back in loose mode, check skip_subfields
+                    # The first extra subfields will be in front of the
+                    # controlled block, the rest at the end.
+                    my $prefix_flag = 1;
                     foreach my $subfield ( $field->subfields ) {
-                        next if $skip_subfields->{ $subfield->[0] };
-                        $field_to->add_subfields( $subfield->[0], $subfield->[1] );
+                        next if $subfield->[0] eq '9'; # skip but leave flag
+                        if ( $skip_subfields->{ $subfield->[0] } ) {
+                            # This marks the beginning of the controlled block
+                            $prefix_flag = 0;
+                            next;
+                        }
+                        if ($prefix_flag) {
+                            push @prefix, [ $subfield->[0], $subfield->[1] ];
+                        } else {
+                            push @postfix, [ $subfield->[0], $subfield->[1] ];
+                        }
                     }
                 }
+                foreach my $subfield ( @prefix, @record_to, @postfix ) {
+                    $field_to->add_subfields($subfield->[0] => $subfield->[1]);
+                }
+                # Move $9 to the end
+                $field_to->delete_subfield( code => '9' );
+                $field_to->add_subfields( 9 => $mergeto );
+
                 if ($tags_new && @$tags_new) {
                     $marcrecord->delete_field($field);
                     append_fields_ordered( $marcrecord, $field_to );