From 26ccb6a4809154368e02e5c147414fc7a19d31be 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 (cherry picked from commit a9eb4005811882da0eb5e20b52861a3c85556dff) --- 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 3a34d79524..3e38172418 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -2211,6 +2211,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 ) @@ -3035,56 +3074,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