From d2ff04c27fc12db439ece4d06363cf8206549889 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Tue, 23 Sep 2025 10:49:15 +0100 Subject: [PATCH] Bug 32950: Fix moving subfield losing values for repeatable fields Moving one subfield to another was failing for repeatable fields where the field to be moved doesn't exist in the first instance. When moving subfields within the same field type (e.g. 020$z to 020$a), existing subfield values in fields that didn't contain the source subfield were being overwritten with values from other fields. This patch fixes the issue by determining which fields actually contain the source subfield and only updating those corresponding target fields, preserving existing values in fields that don't have the source subfield. The fix adds logic to _copy_move_subfield to: 1. Use field_exists() to identify which fields have the source subfield 2. Pass these field numbers to _update_subfield for targeted updates 3. Preserve all existing functionality while fixing the data loss bug Test plan: 1. Create a MARC modification template to move 020$z to 020$a 2. Create a record with multiple 020 fields: - Some with existing $a values - Some with $z values to be moved 3. Apply the template 4. Verify existing $a values are preserved 5. Verify $z values are moved to $a only in appropriate fields 6. Verify $z subfields are removed after the move --- Koha/SimpleMARC.pm | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/Koha/SimpleMARC.pm b/Koha/SimpleMARC.pm index d8e7f9b6ccd..87625de6d2c 100644 --- a/Koha/SimpleMARC.pm +++ b/Koha/SimpleMARC.pm @@ -671,10 +671,30 @@ sub _copy_move_subfield { } _modify_values( { values => \@values, regex => $regex } ); my $dont_erase = $action eq 'copy' ? 1 : 0; + + # Find which source fields actually have the subfield to determine target field numbers + my @target_field_numbers; + if ( $fromFieldName eq $toFieldName && @values ) { + + # For same-field operations, find fields that have the source subfield + @target_field_numbers = + @{ field_exists( { record => $record, field => $fromFieldName, subfield => $fromSubfieldName } ) }; + if (@$field_numbers) { + + # If specific field numbers were requested, intersect them with fields that have the subfield + my %requested = map { $_ => 1 } @$field_numbers; + @target_field_numbers = grep { $requested{$_} } @target_field_numbers; + } + } + _update_subfield( { - record => $record, field => $toFieldName, subfield => $toSubfieldName, values => \@values, - dont_erase => $dont_erase + record => $record, + field => $toFieldName, + subfield => $toSubfieldName, + values => \@values, + dont_erase => $dont_erase, + field_numbers => @target_field_numbers ? \@target_field_numbers : $field_numbers } ); -- 2.51.0