From 681ae8430e60fd0c97ea0652b02ee3daf849ed5d Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Tue, 17 Jan 2017 09:32:44 +0100 Subject: [PATCH] Bug 17913: Do not keep a cleared subfield in loose merge mode If you modify an authority and clear a specific subfield, you expect that merge respects your edit and clears this subfield too in the biblio records. It does in the new strict mode, but it does not yet in the default loose mode. This patch fixes that by adjusting the code around $exclude so that it uses a new hash skip_subfields, built from the reporting tags from the old and the new authority record. This is supported again by some changes in the unit test. 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 Signed-off-by: Kyle M Hall --- C4/AuthoritiesMarc.pm | 21 +++++++++++++-------- t/db_dependent/Authorities/Merge.t | 7 ++++++- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/C4/AuthoritiesMarc.pm b/C4/AuthoritiesMarc.pm index 53be7d2501..72d64f2d85 100644 --- a/C4/AuthoritiesMarc.pm +++ b/C4/AuthoritiesMarc.pm @@ -1467,6 +1467,12 @@ sub merge { # BulkEdit marc records # May be used as a template for a bulkedit field my $overwrite = C4::Context->preference( 'AuthorityMergeMode' ) eq 'strict'; + my $skip_subfields = $overwrite + # This hash contains all subfields from the authority report fields + # Including $MARCfrom as well as $MARCto + # We only need it in loose merge mode; replaces the former $exclude + ? {} + : { map { ( $_->[0], 1 ); } ( @record_from, @record_to ) }; foreach my $marcrecord(@reccache){ my $update = 0; foreach my $tagfield (@$tags_using_authtype){ @@ -1485,17 +1491,16 @@ sub merge { $field->indicator(2), "9" => $mergeto, ); - my $exclude='9'; foreach my $subfield (grep {$_->[0] ne '9'} @record_to) { $field_to->add_subfields($subfield->[0] =>$subfield->[1]); - $exclude.= $subfield->[0]; } - $exclude='['.$exclude.']'; -# add subfields in $field not included in @record_to - my @restore= $overwrite ? () : grep {$_->[0]!~/$exclude/} $field->subfields(); - foreach my $subfield (@restore) { - $field_to->add_subfields($subfield->[0] =>$subfield->[1]); - } + if( !$overwrite ) { + # add subfields back in loose mode, check skip_subfields + foreach my $subfield ( $field->subfields ) { + next if $skip_subfields->{ $subfield->[0] }; + $field_to->add_subfields( $subfield->[0], $subfield->[1] ); + } + } if( $tags_new ) { $marcrecord->delete_field( $field ); append_fields_ordered( $marcrecord, $field_to ); diff --git a/t/db_dependent/Authorities/Merge.t b/t/db_dependent/Authorities/Merge.t index 8ea7817a0e..dce2530778 100755 --- a/t/db_dependent/Authorities/Merge.t +++ b/t/db_dependent/Authorities/Merge.t @@ -137,7 +137,7 @@ subtest 'Test merge A1 to B1 (changing authtype)' => sub { # Tests were aimed for bug 9988, moved to 17909 in adjusted form # Would not encourage this type of merge, but we should test what we offer # The merge routine still needs the fixes on bug 17913 - plan tests => 12; + plan tests => 13; # create two auth recs of different type my $auth1 = MARC::Record->new; @@ -193,6 +193,11 @@ subtest 'Test merge A1 to B1 (changing authtype)' => sub { is( $newbiblio->subfield( '112', 'c' ), $auth2->subfield( '112', 'c' ), 'Check new 112c' ); + # Check 112b; this subfield was cleared when moving from 109 to 112 + # Note that this fix only applies to the current loose mode only + is( $newbiblio->subfield( '112', 'b' ), undef, + 'Merge respects a cleared subfield in loose mode' ); + # Check the original 612 is( ( $newbiblio->field('612') )[0]->subfield( 'a' ), $oldbiblio->subfield( '612', 'a' ), 'Check untouched 612a' ); -- 2.39.5