Bugzilla – Attachment 187185 Details for
Bug 32950
MARC modification template moving subfield can lose values for repeatable fields
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 32950: Fix subfield moving in repeatable MARC fields
Bug-32950-Fix-subfield-moving-in-repeatable-MARC-f.patch (text/plain), 4.81 KB, created by
Martin Renvoize (ashimema)
on 2025-10-01 12:36:36 UTC
(
hide
)
Description:
Bug 32950: Fix subfield moving in repeatable MARC fields
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-10-01 12:36:36 UTC
Size:
4.81 KB
patch
obsolete
>From 09d64e7701e14ec190889f6d29b53aa9f1bd6da9 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Tue, 23 Sep 2025 10:09:31 +0100 >Subject: [PATCH] Bug 32950: Fix subfield moving in repeatable MARC fields > >This patch fixes the issue where moving subfields between repeatable >MARC fields could lose or duplicate values due to incorrect logic in >the _update_subfield function. > >The problem was in the handling of multiple values when there were >more values than target fields. The original code would add all >remaining values to all target fields, causing duplication. > >Changes made: >- Rewrote _update_subfield to handle one-to-one value mapping correctly >- Ensures proper ordering when moving between fields >- Creates new fields in correct position when needed >- Removes excess fields when there are fewer values than existing fields >- Maintains backwards compatibility for existing functionality > >Test plan: >1. Create a MARC record with multiple 020 fields containing both $a and $z subfields >2. Use MARC modification template to move 020$z to 024$a >3. Verify that values are transferred correctly without duplication >4. Verify that the order of values is preserved >5. Run existing SimpleMARC tests to ensure no regression > >Signed-off-by: Bernard Scaife <bernard.scaife@openfifth.co.uk> >--- > Koha/SimpleMARC.pm | 59 +++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 50 insertions(+), 9 deletions(-) > >diff --git a/Koha/SimpleMARC.pm b/Koha/SimpleMARC.pm >index d5c5c791a22..798de7856ee 100644 >--- a/Koha/SimpleMARC.pm >+++ b/Koha/SimpleMARC.pm >@@ -286,23 +286,64 @@ sub _update_subfield { > > if (@fields) { > unless ($dont_erase) { >- @values = ( $values[0] ) x scalar(@fields) >- if @values == 1; >- foreach my $field (@fields) { >- $field->update( "$subfieldName" => $values[ $i++ ] ); >+ >+ # If we have only one value, apply it to all fields >+ if ( @values == 1 ) { >+ foreach my $field (@fields) { >+ $field->update( "$subfieldName" => $values[0] ); >+ } >+ } else { >+ >+ # For multiple values, we need to handle this differently to maintain order >+ # First, update existing fields one-to-one with values >+ my $values_per_field = scalar(@values) / scalar(@fields); >+ my $remainder = scalar(@values) % scalar(@fields); >+ >+ if ( scalar(@values) <= scalar(@fields) ) { >+ >+ # We have fewer or equal values than fields - update sequentially >+ for my $i ( 0 .. $#values ) { >+ $fields[$i]->update( "$subfieldName" => $values[$i] ); >+ } >+ >+ # Remove extra fields if we have more fields than values >+ for my $i ( scalar(@values) .. $#fields ) { >+ $fields[$i]->delete_subfield( code => $subfieldName ); >+ >+ # Remove field entirely if it has no subfields left >+ $record->delete_field( $fields[$i] ) unless $fields[$i]->subfields(); >+ } >+ } else { >+ >+ # We have more values than fields >+ # Update existing fields first >+ for my $i ( 0 .. $#fields ) { >+ $fields[$i]->update( "$subfieldName" => $values[$i] ); >+ } >+ >+ # Create new fields for remaining values, inserting them in the right position >+ for my $i ( scalar(@fields) .. $#values ) { >+ my $new_field = MARC::Field->new( $fieldName, '', '', "$subfieldName" => $values[$i] ); >+ >+ # Insert after the last existing field of this type >+ my $last_field = $fields[-1]; >+ $record->insert_fields_after( $last_field, $new_field ); >+ } >+ } > } >- } >- if ( $i <= scalar(@values) - 1 ) { >+ } else { >+ >+ # dont_erase is true (copy mode), add values to existing fields without erasing > foreach my $field (@fields) { >- foreach my $j ( $i .. scalar(@values) - 1 ) { >- $field->add_subfields( "$subfieldName" => $values[$j] ); >+ foreach my $value (@values) { >+ $field->add_subfields( "$subfieldName" => $value ); > } > } > } > } else { > ## Field does not exist, create it. > foreach my $value (@values) { >- my $field = MARC::Field->new( $fieldName, '', '', "$subfieldName" => $values[ $i++ ] ); >+ my $field = MARC::Field->new( $fieldName, '', '', "$subfieldName" => $value ); > $record->insert_fields_ordered($field); > } > } >-- >2.51.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 32950
:
146586
|
186781
|
186782
|
187184
|
187185
|
187186
|
187187
|
187450
|
187451
|
187452
|
187538