From 3665b9efd29ca33c9039ef9b9f2564ea5382d0bb Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Tue, 7 Oct 2025 17:51:53 +0100 Subject: [PATCH] Bug 32950: (follow-up) Fix copy_field regression for cross-field operations The bug 32950 fix introduced a regression that broke copy_field operations when copying between different fields or when copying a subfield to itself within the same field. The original fix correctly handled moving subfields between different subfields within the same field (e.g., 020$z to 020$a) by using field_exists() to identify which fields have the source subfield. However, this logic was applied too broadly: 1. Cross-field operations (e.g., 650$a to 651$a): The fix passed source field_numbers to _update_subfield for the target field, causing it to look for non-existent target field positions. This resulted in insert_fields_ordered creating new fields out of order. 2. Same-field, same-subfield operations (e.g., 952$d to 952$d): The fix prevented copying to fields that didn't have the source subfield, but the original behavior was correct for duplicating a subfield to itself. This patch restricts the bug 32950 logic to only apply when: - Source and target fields are the same AND - Source and target subfields are different Additionally, it ensures field_numbers are not passed to _update_subfield when copying between different fields. Test plan: 1. Run prove t/SimpleMARC.t 2. All tests should pass, including: - Cross-field copy operations (650$a to 651$a) - Same-field, different-subfield operations (020$z to 020$a) - Same-field, same-subfield operations (952$d to 952$d) Signed-off-by: Martin Renvoize --- Koha/SimpleMARC.pm | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Koha/SimpleMARC.pm b/Koha/SimpleMARC.pm index d03a7fbaf1b..1b188e7b983 100644 --- a/Koha/SimpleMARC.pm +++ b/Koha/SimpleMARC.pm @@ -674,9 +674,10 @@ sub _copy_move_subfield { # Find which source fields actually have the subfield to determine target field numbers my @target_field_numbers; - if ( $fromFieldName eq $toFieldName && @values ) { + if ( $fromFieldName eq $toFieldName && $fromSubfieldName ne $toSubfieldName && @values ) { - # For same-field operations, find fields that have the source subfield + # For same-field operations where subfields differ, find fields that have the source subfield + # This prevents overwriting target subfields in fields that don't have the source subfield @target_field_numbers = @{ field_exists( { record => $record, field => $fromFieldName, subfield => $fromSubfieldName } ) }; if (@$field_numbers) { @@ -694,7 +695,9 @@ sub _copy_move_subfield { subfield => $toSubfieldName, values => \@values, dont_erase => $dont_erase, - field_numbers => @target_field_numbers ? \@target_field_numbers : $field_numbers + field_numbers => @target_field_numbers + ? \@target_field_numbers + : ( $fromFieldName eq $toFieldName ? $field_numbers : [] ) } ); -- 2.51.0