From dfca3f9c74021beabdde62773a5cf49295753933 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Tue, 17 Jan 2017 10:09:40 +0100 Subject: [PATCH] Bug 17913: Remove possible duplicates in strict merge mode Since strict mode does not allow additional subfields that would make identical fields linked to the same authority different, there is no need to keep them while merging. We achieve this goal by simply: [1] Count the number of same fields linked to mergefrom in strict mode to eliminate duplicates. [2] Replaces the if-statement on auth_number by a next. (Tidy follows.) Test plan: Run t/db_dependent/Authorities/Merge.t Signed-off-by: Marcel de Rooy Signed-off-by: Josef Moravec Signed-off-by: Julian Maurice --- C4/AuthoritiesMarc.pm | 11 +++++++++-- t/db_dependent/Authorities/Merge.t | 9 ++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/C4/AuthoritiesMarc.pm b/C4/AuthoritiesMarc.pm index 72d64f2d85..7517419fa2 100644 --- a/C4/AuthoritiesMarc.pm +++ b/C4/AuthoritiesMarc.pm @@ -1477,14 +1477,22 @@ sub merge { my $update = 0; foreach my $tagfield (@$tags_using_authtype){ # warn "tagfield : $tagfield "; + my $countfrom = 0; # used in strict mode to remove duplicates foreach my $field ($marcrecord->field($tagfield)){ # biblio is linked to authority with $9 subfield containing authid my $auth_number=$field->subfield("9"); my $tag=$field->tag(); + next if !defined($auth_number) || $auth_number ne $mergefrom; + $countfrom++; + if( $overwrite && $countfrom > 1 ) { + # remove this duplicate in strict mode + $marcrecord->delete_field( $field ); + $update = 1; + next; + } my $newtag = $tags_new ? _merge_newtag( $tag, $tags_new ) : $tag; - if ($auth_number==$mergefrom) { my $field_to = MARC::Field->new( $newtag, $field->indicator(1), @@ -1508,7 +1516,6 @@ sub merge { $field->replace_with($field_to); } $update=1; - } }#for each tag }#foreach tagfield my ($bibliotag,$bibliosubf) = GetMarcFromKohaField("biblio.biblionumber","") ; diff --git a/t/db_dependent/Authorities/Merge.t b/t/db_dependent/Authorities/Merge.t index dce2530778..2bed211278 100755 --- a/t/db_dependent/Authorities/Merge.t +++ b/t/db_dependent/Authorities/Merge.t @@ -82,7 +82,7 @@ subtest 'Test merge A1 to A2 (within same authtype)' => sub { subtest 'Test merge A1 to modified A1' => sub { # Tests originate from bug 11700 - plan tests => 9; + plan tests => 11; # Simulate modifying an authority from auth1old to auth1new my $auth1old = MARC::Record->new; @@ -95,6 +95,8 @@ subtest 'Test merge A1 to modified A1' => sub { my $MARC1 = MARC::Record->new; $MARC1->append_fields( MARC::Field->new( '109', '', '', 'a' => 'Bruce Wayne', 'b' => '2014', '9' => $authid1 )); $MARC1->append_fields( MARC::Field->new( '245', '', '', 'a' => 'From the depths' )); + $MARC1->append_fields( MARC::Field->new( '609', '', '', 'a' => 'Bruce Lee', 'b' => 'Should be cleared too', '9' => $authid1 )); + $MARC1->append_fields( MARC::Field->new( '609', '', '', 'a' => 'Bruce Lee', 'c' => 'This is a duplicate to be removed in strict mode', '9' => $authid1 )); my $MARC2 = MARC::Record->new; $MARC2->append_fields( MARC::Field->new( '109', '', '', 'a' => 'Batman', '9' => $authid1 )); $MARC2->append_fields( MARC::Field->new( '245', '', '', 'a' => 'All the way to heaven' )); @@ -130,6 +132,11 @@ subtest 'Test merge A1 to modified A1' => sub { $rv = C4::AuthoritiesMarc::merge( $authid1, $auth1old, $authid1, $auth1new ); $biblio1 = GetMarcBiblio($biblionumber1); is( $biblio1->field(109)->subfield('b'), undef, 'Subfield overwritten in strict mode' ); + is( $biblio1->fields, scalar( $MARC1->fields ) - 1, 'strict mode should remove a duplicate 609' ); + is( $biblio1->field(609)->subfields, + scalar($auth1new->field('109')->subfields) + 1, + 'Check number of subfields in strict mode for the remaining 609' ); + # Note: the +1 comes from the added subfield $9 in the biblio t::lib::Mocks::mock_preference('AuthorityMergeMode', 'loose'); }; -- 2.11.0