[Koha-bugs] [Bug 24480] Fields added with MARC modifications templates are not added in an ordered way

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24480

--- Comment #28 from Phil Ringnalda <phil at chetcolibrary.org> ---
Comment on attachment 150666
  --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150666
Bug 24480: Unit test

>diff --git a/Koha/SimpleMARC.pm b/Koha/SimpleMARC.pm
>index 0c95ccf804..d3c05647ff 100644
>--- a/Koha/SimpleMARC.pm
>+++ b/Koha/SimpleMARC.pm
>@@ -575,6 +575,13 @@ sub _copy_move_field {
>         @from_fields = map { $_ <= @from_fields ? $from_fields[ $_ - 1 ] : () } @$field_numbers;
>     }
> 
>+    if ( $action eq 'replace' ) {
>+        my @to_fields = $record->field( $toFieldName );
>+        for my $to_field ( @to_fields ) {
>+            $record->delete_field( $to_field );
>+        }
>+    }
>+
>     for my $from_field ( @from_fields ) {
>         my $new_field = $from_field->clone;
>         $new_field->{_tag} = $toFieldName; # Should be replaced by set_tag, introduced by MARC::Field 2.0.4
>@@ -588,12 +595,6 @@ sub _copy_move_field {
>         if ( $action eq 'move' ) {
>             $record->delete_field( $from_field )
>         }
>-        elsif ( $action eq 'replace' ) {
>-            my @to_fields = $record->field( $toFieldName );
>-            if ( @to_fields ) {
>-                $record->delete_field( $to_fields[0] );
>-            }
>-        }
>         $record->insert_grouped_field( $new_field );
>     }
> }

Changing the meaning of a Copy and replace action from "copy n from fields,
replacing the first n to fields" to "copy n from fields, replacing every to
field" may be a reasonable change (I don't have an opinion, since I never use
Copy and replace), but doing so in a patch with the summary "Unit test" seems
very wrong.

-- 
You are receiving this mail because:
You are watching all bug changes.