From 3d9778b2cd472a520afe4074e09f6c76fa5d7907 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Wed, 17 Jul 2024 10:51:01 +0100 Subject: [PATCH] Bug 37305: Subfield order matters This patch updates the use of a hash to store and then pass subfields to the MARC::Field new method to an array with key/value pairs using fat comma's. This allows us to maintain the integrity check that we're passing string as the first arguament in each pair whilst also allowing us to ensure fields are passed in the order they are found. Sponsored-by: Open Fifth Signed-off-by: Heather Hernandez Signed-off-by: Martin Renvoize Signed-off-by: Lucas Gass --- Koha/Biblio.pm | 105 ++++++++++++++++++++++++++----------------------- 1 file changed, 56 insertions(+), 49 deletions(-) diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm index 0df2b489a7..4f0c0bf1f9 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -1921,7 +1921,7 @@ sub generate_marc_host_field { my $marcflavour = C4::Context->preference('marcflavour'); my $marc_host = $self->metadata->record; - my %sfd; + my @sfd; my $host_field; my $link_field; @@ -1931,19 +1931,20 @@ sub generate_marc_host_field { if ( $host_field = $marc_host->field('100') || $marc_host->field('110') || $marc_host->field('111') ) { my $s = $host_field->as_string('ab'); if ($s) { - $sfd{a} = $s; + push @sfd, ( a => $s ); } } - # Edition - if ( $host_field = $marc_host->field('250') ) { - my $s = $host_field->as_string('ab'); + # Title + if ( $host_field = $marc_host->field('245') ) { + my $s = $host_field->as_string('abnp'); if ($s) { - $sfd{b} = $s; + push @sfd, ( t => $s ); } } # Publication + my $p; my @publication_fields = $marc_host->field('264'); @publication_fields = $marc_host->field('260') unless (@publication_fields); my $index = 0; @@ -1953,32 +1954,33 @@ sub generate_marc_host_field { if ( $index == 0 ) { my $s = $host_field->as_string('abc'); if ($s) { - $sfd{d} = $s; + $p = $s; } $index++; } if ( $host_field->indicator(1) && ( $host_field->indicator(1) eq '3' ) ) { my $s = $host_field->as_string('abc'); if ($s) { - $sfd{d} = $s; + $p = $s; } last; } } + push @sfd, ( d => $p ) if $p; # Uniform title if ( $host_field = $marc_host->field('240') ) { my $s = $host_field->as_string('a'); if ($s) { - $sfd{s} = $s; + push @sfd, ( s => $s ); } } - # Title - if ( $host_field = $marc_host->field('245') ) { - my $s = $host_field->as_string('abnp'); + # Edition + if ( $host_field = $marc_host->field('250') ) { + my $s = $host_field->as_string('ab'); if ($s) { - $sfd{t} = $s; + push @sfd, ( b => $s ); } } @@ -1986,7 +1988,7 @@ sub generate_marc_host_field { if ( $host_field = $marc_host->field('022') ) { my $s = $host_field->as_string('a'); if ($s) { - $sfd{x} = $s; + push @sfd, ( x => $s ); } } @@ -1994,53 +1996,33 @@ sub generate_marc_host_field { if ( $host_field = $marc_host->field('020') ) { my $s = $host_field->as_string('a'); if ($s) { - $sfd{z} = $s; + push @sfd, ( z => $s ); } } if ( C4::Context->preference('UseControlNumber') ) { + my $w; + # Control number if ( $host_field = $marc_host->field('001') ) { - $sfd{w} = $host_field->data(); + $w = $host_field->data(); } # Control number identifier if ( $host_field = $marc_host->field('003') ) { - $sfd{w} = '(' . $host_field->data() . ')' . $sfd{w}; + $w = '(' . $host_field->data() . ')' . $w; } + + push @sfd, ( w => $w ); } - $link_field = MARC::Field->new( 773, '0', ' ', %sfd ); + $link_field = MARC::Field->new( 773, '0', ' ', @sfd ); } elsif ( $marcflavour eq 'UNIMARC' ) { # Author if ( $host_field = $marc_host->field('700') || $marc_host->field('710') || $marc_host->field('720') ) { my $s = $host_field->as_string('ab'); if ($s) { - $sfd{a} = $s; - } - } - - # Place of publication - if ( $host_field = $marc_host->field('210') ) { - my $s = $host_field->as_string('a'); - if ($s) { - $sfd{c} = $s; - } - } - - # Date of publication - if ( $host_field = $marc_host->field('210') ) { - my $s = $host_field->as_string('d'); - if ($s) { - $sfd{d} = $s; - } - } - - # Edition statement - if ( $host_field = $marc_host->field('205') ) { - my $s = $host_field->as_string(); - if ($s) { - $sfd{e} = $s; + push @sfd, ( a => $s ); } } @@ -2048,7 +2030,31 @@ sub generate_marc_host_field { if ( $host_field = $marc_host->field('200') ) { my $s = $host_field->as_string('a'); if ($s) { - $sfd{t} = $s; + push @sfd, ( t => $s ); + } + } + + # Place of publication + if ( $host_field = $marc_host->field('210') ) { + my $s = $host_field->as_string('a'); + if ($s) { + push @sfd, ( c => $s ); + } + } + + # Date of publication + if ( $host_field = $marc_host->field('210') ) { + my $s = $host_field->as_string('d'); + if ($s) { + push @sfd, ( d => $s ); + } + } + + # Edition statement + if ( $host_field = $marc_host->field('205') ) { + my $s = $host_field->as_string(); + if ($s) { + push @sfd, ( e => $s ); } } @@ -2056,7 +2062,7 @@ sub generate_marc_host_field { if ( $host_field = $marc_host->field('856') ) { my $s = $host_field->as_string('u'); if ($s) { - $sfd{u} = $s; + push @sfd, ( u => $s ); } } @@ -2064,7 +2070,7 @@ sub generate_marc_host_field { if ( $host_field = $marc_host->field('011') ) { my $s = $host_field->as_string('a'); if ($s) { - $sfd{x} = $s; + push @sfd, ( x => $s ); } } @@ -2072,13 +2078,14 @@ sub generate_marc_host_field { if ( $host_field = $marc_host->field('010') ) { my $s = $host_field->as_string('a'); if ($s) { - $sfd{y} = $s; + push @sfd, ( y => $s ); } } if ( $host_field = $marc_host->field('001') ) { - $sfd{0} = $host_field->data(); + + push @sfd, ( 0 => $host_field->data() ); } - $link_field = MARC::Field->new( 461, '0', ' ', %sfd ); + $link_field = MARC::Field->new( 461, '0', ' ', @sfd ); } return $link_field;