From 8764d1365c96b9039b20a2c37e4aec7265bca9ed Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Tue, 21 Feb 2017 08:39:24 +0100 Subject: [PATCH] Bug 9988: Merge should have a parameter hash Content-Type: text/plain; charset=utf-8 We will need a few additional parameters for merge later on. This patch puts the original parameters in a parameter hash. Test plan: [1] Run Merge.t [2] Merge two authorities in the Authorities module Note: I also tested script merge_authority.pl with -b parameter. This script will still need further attention. I manually deleted an authority record, inserted a record in need_merge_authorities, and ran the script with -b while adding -f and -t with an existing authority number. This record number is ignored in batch mode. The biblio record with the deleted authority was correctly cleaned up. Signed-off-by: Marcel de Rooy --- C4/AuthoritiesMarc.pm | 18 ++++++++++++++---- authorities/merge.pl | 2 +- misc/migration_tools/merge_authority.pl | 6 +++--- t/db_dependent/Authorities/Merge.t | 10 +++++----- 4 files changed, 23 insertions(+), 13 deletions(-) diff --git a/C4/AuthoritiesMarc.pm b/C4/AuthoritiesMarc.pm index 2f14c3a..ca132e0 100644 --- a/C4/AuthoritiesMarc.pm +++ b/C4/AuthoritiesMarc.pm @@ -693,7 +693,7 @@ sub DelAuthority { my $dbh=C4::Context->dbh; unless( C4::Context->preference('dontmerge') eq '1' ) { - &merge( $authid, GetAuthority($authid) ); + &merge({ mergefrom => $authid, MARCfrom => GetAuthority($authid) }); } else { # save a record in need_merge_authorities table my $sqlinsert="INSERT INTO need_merge_authorities (authid, done) VALUES (?,?)"; @@ -725,7 +725,7 @@ sub ModAuthority { # In that case set system preference "dontmerge" to 1. Otherwise biblios will # be updated. unless(C4::Context->preference('dontmerge') eq '1'){ - &merge($authid,$oldrecord,$authid,$record); + &merge({ mergefrom => $authid, MARCfrom => $oldrecord, mergeto => $authid, MARCto => $record }); } else { # save a record in need_merge_authorities table my $sqlinsert="INSERT INTO need_merge_authorities (authid, done) ". @@ -1388,7 +1388,12 @@ sub AddAuthorityTrees{ =head2 merge - $count = merge ( mergefrom, $MARCfrom, [$mergeto, $MARCto] ) + $count = merge({ + mergefrom => mergefrom, + MARCfrom => $MARCfrom, + [ mergeto => $mergeto, ] + [ MARCto => $MARCto, ] + }); Merge biblios linked to authority $mergefrom. If $mergeto equals mergefrom, the linked biblio field is updated. @@ -1401,7 +1406,12 @@ authority type, merge also supports moving to another authority type. =cut sub merge { - my ($mergefrom,$MARCfrom,$mergeto,$MARCto) = @_; + my ( $params ) = @_; + my $mergefrom = $params->{mergefrom}; + my $MARCfrom = $params->{MARCfrom}; + my $mergeto = $params->{mergeto}; + my $MARCto = $params->{MARCto}; + return 0 unless $mergefrom > 0; # prevent abuse my ($counteditedbiblio,$countunmodifiedbiblio,$counterrors)=(0,0,0); my $dbh=C4::Context->dbh; diff --git a/authorities/merge.pl b/authorities/merge.pl index 9d00a1d..0c70b8d 100755 --- a/authorities/merge.pl +++ b/authorities/merge.pl @@ -64,7 +64,7 @@ if ($merge) { # Now merge for biblios attached to $recordid2 # We ignore dontmerge now, since recordid2 is deleted my $MARCfrom = GetAuthority( $recordid2 ); - merge( $recordid2, $MARCfrom, $recordid1, $record ); + merge({ mergefrom => $recordid2, MARCfrom => $MARCfrom, mergeto => $recordid1, MARCto => $record }); # Deleting the other record DelAuthority( $recordid2 ); diff --git a/misc/migration_tools/merge_authority.pl b/misc/migration_tools/merge_authority.pl index 061a3bd..dc23e9a 100755 --- a/misc/migration_tools/merge_authority.pl +++ b/misc/migration_tools/merge_authority.pl @@ -86,16 +86,16 @@ if ($batch) { print "managing $authid\n" if $verbose; my $MARCauth = GetAuthority( $authid ); if( $MARCauth ) { - merge( $authid, $MARCauth, $authid, $MARCauth ); + merge({ mergefrom => $authid, MARCfrom => $MARCauth, mergeto => $authid, MARCto => $MARCauth }); } else { - merge( $authid, undef ); # handle a delete + merge({ mergefrom => $authid }); # handle a delete } } $dbh->do("update need_merge_authorities set done=1 where done=2"); #DONE } else { my $MARCfrom = GetAuthority($mergefrom); my $MARCto = GetAuthority($mergeto); - &merge($mergefrom,$MARCfrom,$mergeto,$MARCto); + &merge({ mergefrom => $mergefrom, MARCfrom => $MARCfrom, mergeto => $mergeto, MARCto => $MARCto }); #Could add mergefrom authority to mergeto rejected forms before deletion DelAuthority($mergefrom) if ($mergefrom != $mergeto); } diff --git a/t/db_dependent/Authorities/Merge.t b/t/db_dependent/Authorities/Merge.t index 7ee8cdb..db88d31 100755 --- a/t/db_dependent/Authorities/Merge.t +++ b/t/db_dependent/Authorities/Merge.t @@ -60,7 +60,7 @@ subtest 'Test merge A1 to A2 (within same authtype)' => sub { # Time to merge @zebrarecords = ( $biblio1, $biblio2 ); $index = 0; - my $rv = C4::AuthoritiesMarc::merge( $authid2, $auth2, $authid1, $auth1 ); + my $rv = C4::AuthoritiesMarc::merge({ mergefrom => $authid2, MARCfrom => $auth2, mergeto => $authid1, MARCto => $auth1 }); is( $rv, 1, 'We expect one biblio record (out of two) to be updated' ); # Check the results @@ -105,7 +105,7 @@ subtest 'Test merge A1 to modified A1, test strict mode' => sub { @zebrarecords = ( $MARC1, $MARC2 ); $index = 0; t::lib::Mocks::mock_preference('AuthorityMergeMode', 'loose'); - my $rv = C4::AuthoritiesMarc::merge( $authid1, $auth1old, $authid1, $auth1new ); + my $rv = C4::AuthoritiesMarc::merge({ mergefrom => $authid1, MARCfrom => $auth1old, mergeto => $authid1, MARCto => $auth1new }); is( $rv, 2, 'Both records are updated now' ); #Check the results @@ -125,7 +125,7 @@ subtest 'Test merge A1 to modified A1, test strict mode' => sub { ModBiblio( $MARC1, $biblionumber1, '' ); @zebrarecords = ( $MARC1 ); $index = 0; - $rv = C4::AuthoritiesMarc::merge( $authid1, $auth1old, $authid1, $auth1new ); + $rv = C4::AuthoritiesMarc::merge({ mergefrom => $authid1, MARCfrom => $auth1old, mergeto => $authid1, MARCto => $auth1new }); $biblio1 = GetMarcBiblio($biblionumber1); is( $biblio1->field(109)->subfield('b'), undef, 'Subfield overwritten in strict mode' ); compare_fields( $MARC1, $biblio1, { 609 => 1 }, 'count' ); @@ -172,7 +172,7 @@ subtest 'Test merge A1 to B1 (changing authtype)' => sub { # Time to merge @zebrarecords = ( $marc ); $index = 0; - my $retval = C4::AuthoritiesMarc::merge( $authid1, $auth1, $authid2, $auth2 ); + my $retval = C4::AuthoritiesMarc::merge({ mergefrom => $authid1, MARCfrom => $auth1, mergeto => $authid2, MARCto => $auth2 }); is( $retval, 1, 'We touched only one biblio' ); # Get new marc record for compares @@ -256,7 +256,7 @@ subtest 'Merging authorities should handle deletes (BZ 18070)' => sub { C4::Context->dbh->do( "DELETE FROM auth_header WHERE authid=?", undef, $authid1 ); @zebrarecords = ( $marc1 ); $index = 0; - merge( $authid1, undef ); + merge({ mergefrom => $authid1 }); # Final check $marc1 = C4::Biblio::GetMarcBiblio( $biblionumber ); is( $marc1->field('609'), undef, 'Merge removed the 609 again even after deleting the authority record' ); -- 2.1.4