From b034a924b5d2282c1e7a58c7df0b37aac5dc7852 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Tue, 17 Jan 2017 10:09:40 +0100 Subject: [PATCH] [SIGNED-OFF] 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 --- 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 72d64f2..7517419 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 0ff66a3..e3d08f4 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' )); @@ -128,6 +130,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.1.4