From a9eb4005811882da0eb5e20b52861a3c85556dff Mon Sep 17 00:00:00 2001 From: Olli-Antti Kivilahti Date: Thu, 19 May 2016 17:12:06 +0300 Subject: [PATCH] Bug 16556 - KohaToMarcMapped columns sharing same field with biblio(item)number are removed. REPLICATE ISSUE: 1. Map biblio.frameworkcode to 999$b 2. Map biblio.biblionumber to 999$c 3. Add a record with something in 999$b 4. 999$b is removed by C4::Biblio::AddBiblio() After this patch, the field used by biblio.biblionumber or biblioitems.biblioitemnumber is not removed and created anew, thus dropping all existing additions. There is no point in dropping the field in any case, since we can just replace the existing subfields in-place with no need to recreate the whole field. UNIT TESTS INCLUDED Signed-off-by: Chris Cormack Signed-off-by: Nick Clemens Signed-off-by: Kyle M Hall --- C4/Biblio.pm | 91 +++++++++++++++++++++++++++------------------------- t/Biblio2.t | 55 +++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 44 deletions(-) create mode 100644 t/Biblio2.t diff --git a/C4/Biblio.pm b/C4/Biblio.pm index a1f70b8f3f..25bf496e61 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -2226,6 +2226,45 @@ sub GetMarcHosts { return $marchostsarray; } +=head2 UpsertMarcSubfield + + my $record = C4::Biblio::UpsertMarcSubfield($MARC::Record, $fieldTag, $subfieldCode, $subfieldContent); + +=cut + +sub UpsertMarcSubfield { + my ($record, $tag, $code, $content) = @_; + my $f = $record->field($tag); + + if ($f) { + $f->update( $code => $content ); + } + else { + my $f = MARC::Field->new( $tag, '', '', $code => $content); + $record->insert_fields_ordered( $f ); + } +} + +=head2 UpsertMarcControlField + + my $record = C4::Biblio::UpsertMarcControlField($MARC::Record, $fieldTag, $content); + +=cut + +sub UpsertMarcControlField { + my ($record, $tag, $content) = @_; + die "UpsertMarcControlField() \$tag '$tag' is not a control field\n" unless 0+$tag < 10; + my $f = $record->field($tag); + + if ($f) { + $f->update( $content ); + } + else { + my $f = MARC::Field->new($tag, $content); + $record->insert_fields_ordered( $f ); + } +} + =head2 GetFrameworkCode $frameworkcode = GetFrameworkCode( $biblionumber ) @@ -3050,56 +3089,20 @@ the MARC XML. sub _koha_marc_update_bib_ids { my ( $record, $frameworkcode, $biblionumber, $biblioitemnumber ) = @_; - # we must add bibnum and bibitemnum in MARC::Record... - # we build the new field with biblionumber and biblioitemnumber - # we drop the original field - # we add the new builded field. my ( $biblio_tag, $biblio_subfield ) = GetMarcFromKohaField( "biblio.biblionumber", $frameworkcode ); die qq{No biblionumber tag for framework "$frameworkcode"} unless $biblio_tag; my ( $biblioitem_tag, $biblioitem_subfield ) = GetMarcFromKohaField( "biblioitems.biblioitemnumber", $frameworkcode ); die qq{No biblioitemnumber tag for framework "$frameworkcode"} unless $biblioitem_tag; - if ( $biblio_tag == $biblioitem_tag ) { - - # biblionumber & biblioitemnumber are in the same field (can't be <10 as fields <10 have only 1 value) - my $new_field = MARC::Field->new( - $biblio_tag, '', '', - "$biblio_subfield" => $biblionumber, - "$biblioitem_subfield" => $biblioitemnumber - ); - - # drop old field and create new one... - my $old_field = $record->field($biblio_tag); - $record->delete_field($old_field) if $old_field; - $record->insert_fields_ordered($new_field); + if ( $biblio_tag < 10 ) { + C4::Biblio::UpsertMarcControlField( $record, $biblio_tag, $biblionumber ); } else { - - # biblionumber & biblioitemnumber are in different fields - - # deal with biblionumber - my ( $new_field, $old_field ); - if ( $biblio_tag < 10 ) { - $new_field = MARC::Field->new( $biblio_tag, $biblionumber ); - } else { - $new_field = MARC::Field->new( $biblio_tag, '', '', "$biblio_subfield" => $biblionumber ); - } - - # drop old field and create new one... - $old_field = $record->field($biblio_tag); - $record->delete_field($old_field) if $old_field; - $record->insert_fields_ordered($new_field); - - # deal with biblioitemnumber - if ( $biblioitem_tag < 10 ) { - $new_field = MARC::Field->new( $biblioitem_tag, $biblioitemnumber, ); - } else { - $new_field = MARC::Field->new( $biblioitem_tag, '', '', "$biblioitem_subfield" => $biblioitemnumber, ); - } - - # drop old field and create new one... - $old_field = $record->field($biblioitem_tag); - $record->delete_field($old_field) if $old_field; - $record->insert_fields_ordered($new_field); + C4::Biblio::UpsertMarcSubfield($record, $biblio_tag, $biblio_subfield, $biblionumber); + } + if ( $biblioitem_tag < 10 ) { + C4::Biblio::UpsertMarcControlField( $record, $biblioitem_tag, $biblioitemnumber ); + } else { + C4::Biblio::UpsertMarcSubfield($record, $biblioitem_tag, $biblioitem_subfield, $biblioitemnumber); } } diff --git a/t/Biblio2.t b/t/Biblio2.t new file mode 100644 index 0000000000..3cb0cb051c --- /dev/null +++ b/t/Biblio2.t @@ -0,0 +1,55 @@ +#!/usr/bin/perl + +use Modern::Perl; +use Test::More; +use Test::MockModule; + +use MARC::Record; + +use C4::Biblio; + +subtest "_koha_marc_update_bib_ids basic Field", \&_koha_marc_update_bib_ids_simple; +sub _koha_marc_update_bib_ids_simple { + my $module = Test::MockModule->new('C4::Biblio'); + $module->mock('GetMarcFromKohaField', sub { + my ($source) = @_; + return ('999','c') if $source eq 'biblio.biblionumber'; + return ('999','d') if $source eq 'biblioitems.biblioitemnumber'; + } + ); + + my $r = MARC::Record->new(); + C4::Biblio::_koha_marc_update_bib_ids($r, undef, 10, 20); + is($r->subfield('999', 'c'), 10, 'Biblionumber'); + is($r->subfield('999', 'd'), 20, 'Biblioitemnumber'); + + C4::Biblio::_koha_marc_update_bib_ids($r, undef, 10, 20); + my @f = $r->field('999'); + is(scalar(@f), 1, 'Field not duplicated'); + is($r->subfield('999', 'c'), 10, 'Biblionumber'); + is($r->subfield('999', 'd'), 20, 'Biblioitemnumber'); +} + +subtest "_koha_marc_update_bib_ids ControlField", \&_koha_marc_update_bib_ids_control; +sub _koha_marc_update_bib_ids_control { + my $module = Test::MockModule->new('C4::Biblio'); + $module->mock('GetMarcFromKohaField', sub { + my ($source) = @_; + return ('001',undef) if $source eq 'biblio.biblionumber'; + return ('004',undef) if $source eq 'biblioitems.biblioitemnumber'; + } + ); + + my $r = MARC::Record->new(); + C4::Biblio::_koha_marc_update_bib_ids($r, undef, 10, 20); + is($r->field('001')->data(), 10, 'Biblionumber to control field'); + is($r->field('004')->data(), 20, 'Biblioitemnumber to control field'); + + C4::Biblio::_koha_marc_update_bib_ids($r, undef, 10, 20); + my @f = $r->field('001'); + is(scalar(@f), 1, 'Control field not duplicated'); + is($r->field('001')->data(), 10, 'Biblionumber to control field'); + is($r->field('004')->data(), 20, 'Biblioitemnumber to control field'); +} + +done_testing(); -- 2.39.5