Bugzilla – Attachment 60600 Details for
Bug 9988
Leave larger authority merges to merge_authorities cronjob (pref AuthorityMergeLimit)
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 9988: Merge should have a parameter hash
Bug-9988-Merge-should-have-a-parameter-hash.patch (text/plain), 7.45 KB, created by
Marcel de Rooy
on 2017-02-23 12:15:55 UTC
(
hide
)
Description:
Bug 9988: Merge should have a parameter hash
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2017-02-23 12:15:55 UTC
Size:
7.45 KB
patch
obsolete
>From 8764d1365c96b9039b20a2c37e4aec7265bca9ed Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >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 <m.de.rooy@rijksmuseum.nl> >--- > 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 9988
:
59158
|
59159
|
60256
|
60257
|
60258
|
60259
|
60594
|
60595
|
60596
|
60597
|
60598
|
60599
|
60600
|
60601
|
60602
|
60603
|
60604
|
60605
|
60606
|
60607
|
60668
|
60669
|
60670
|
60671
|
60672
|
60673
|
60674
|
60675
|
61306
|
61307
|
61308
|
61309
|
61779
|
61780
|
61781
|
61782
|
61783
|
61784
|
61785
|
61786
|
61787
|
61788
|
61789
|
61790
|
61791
|
61792
|
61890
|
61891
|
61892
|
61893
|
61894
|
61895
|
61897
|
61898
|
61899
|
61900
|
61901
|
61902
|
61903
|
61904
|
61912
|
61913
|
61914
|
61915
|
61916
|
61917
|
61918
|
61919
|
61920
|
61921
|
61922
|
61923
|
61924
|
61925
|
62055
|
62123
|
62124
|
62125
|
62126
|
62127
|
62128
|
62129
|
62130
|
62131
|
62132
|
62133
|
62134
|
62135
|
62136
|
62137