@@ -, +, @@ biblios. Check that the merge was done immediately. Verify that the merge was postponed; a record must be inserted in the need_merge_authorities table. Note: I tested a modification, but the script just needs more attention. --- C4/AuthoritiesMarc.pm | 75 +++++++++++++++++---------------- misc/migration_tools/merge_authority.pl | 19 +++++---- t/db_dependent/Authorities/Merge.t | 45 +++++++++++++++++--- 3 files changed, 86 insertions(+), 53 deletions(-) --- a/C4/AuthoritiesMarc.pm +++ a/C4/AuthoritiesMarc.pm @@ -28,6 +28,7 @@ use C4::Charset; use C4::Log; use Koha::MetadataRecord::Authority; use Koha::Authorities; +use Koha::Authority::MergeRequest; use Koha::Authority::Types; use Koha::Authority; use Koha::SearchEngine; @@ -692,14 +693,7 @@ sub DelAuthority { my ( $params ) = @_; my $authid = $params->{authid} || return; my $dbh=C4::Context->dbh; - - unless( C4::Context->preference('dontmerge') eq '1' ) { - &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 (?,?)"; - $dbh->do( $sqlinsert, undef, $authid, 0 ); - } + merge({ mergefrom => $authid, MARCfrom => GetAuthority($authid) }); $dbh->do( "DELETE FROM auth_header WHERE authid=?", undef, $authid ); logaction( "AUTHORITIES", "DELETE", $authid, "authority" ) if C4::Context->preference("AuthoritiesLog"); ModZebra( $authid, "recordDelete", "authorityserver", undef); @@ -714,27 +708,13 @@ Modifies authority record, optionally updates attached biblios. =cut sub ModAuthority { - my ($authid,$record,$authtypecode)=@_; # deprecated $merge parameter removed - - my $dbh=C4::Context->dbh; - #Now rewrite the $record to table with an add - my $oldrecord=GetAuthority($authid); - $authid=AddAuthority($record,$authid,$authtypecode); - - # If a library thinks that updating all biblios is a long process and wishes - # to leave that to a cron job, use misc/migration_tools/merge_authority.pl. - # In that case set system preference "dontmerge" to 1. Otherwise biblios will - # be updated. - unless(C4::Context->preference('dontmerge') eq '1'){ - &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) ". - "VALUES (?,?)"; - $dbh->do($sqlinsert,undef,($authid,0)); - } - logaction( "AUTHORITIES", "MODIFY", $authid, "authority BEFORE=>" . $oldrecord->as_formatted ) if C4::Context->preference("AuthoritiesLog"); - return $authid; + my ( $authid, $record, $authtypecode ) = @_; + my $oldrecord = GetAuthority($authid); + #Now rewrite the $record to table with an add + $authid = AddAuthority($record, $authid, $authtypecode); + merge({ mergefrom => $authid, MARCfrom => $oldrecord, mergeto => $authid, MARCto => $record }); + logaction( "AUTHORITIES", "MODIFY", $authid, "authority BEFORE=>" . $oldrecord->as_formatted ) if C4::Context->preference("AuthoritiesLog"); + return $authid; } =head2 GetAuthorityXML @@ -1395,6 +1375,7 @@ sub AddAuthorityTrees{ [ mergeto => $mergeto, ] [ MARCto => $MARCto, ] [ biblionumbers => [ $a, $b, $c ], ] + [ override_limit => 1, ] }); Merge biblios linked to authority $mergefrom. @@ -1405,6 +1386,9 @@ If $mergeto is missing, the biblio field is deleted. Normally all biblio records linked to $mergefrom, will be considered. But you can pass specific numbers via the biblionumbers parameter. +The parameter override_limit is used by the cron job to force larger +postponed merges. + Note: Although $mergefrom and $mergeto will normally be of the same authority type, merge also supports moving to another authority type. @@ -1416,20 +1400,35 @@ sub merge { my $MARCfrom = $params->{MARCfrom}; my $mergeto = $params->{mergeto}; my $MARCto = $params->{MARCto}; - my @biblionumbers = $params->{biblionumbers} - # If we do not have biblionumbers, we get all linked biblios - ? @{ $params->{biblionumbers} } - : Koha::Authorities->linked_biblionumbers({ authid => $mergefrom }); + my $override_limit = $params->{override_limit}; + + # If we do not have biblionumbers, we get all linked biblios if the + # number of linked records does not exceed the limit UNLESS we override. + my @biblionumbers; + if( $params->{biblionumbers} ) { + @biblionumbers = @{ $params->{biblionumbers} }; + } elsif( $override_limit ) { + @biblionumbers = Koha::Authorities->linked_biblionumbers({ authid => $mergefrom }); + } else { # now first check number of linked records + my $max = C4::Context->preference('AuthorityMergeLimit') // 0; + my $hits = Koha::Authorities->get_usage_count({ authid => $mergefrom }); + if( $hits > 0 && $hits <= $max ) { + @biblionumbers = Koha::Authorities->linked_biblionumbers({ authid => $mergefrom }); + } elsif( $hits > $max ) { #postpone this merge to the cron job + Koha::Authority::MergeRequest->new({ + authid => $mergefrom, + oldrecord => $MARCfrom, + authid_new => $mergeto, + })->store; + } + } return 0 if !@biblionumbers; - my $counteditedbiblio = 0; - my $dbh = C4::Context->dbh; + # Search authtypes and reporting tags my $authfrom = Koha::Authorities->find($mergefrom); my $authto = Koha::Authorities->find($mergeto); my $authtypefrom = $authfrom ? Koha::Authority::Types->find($authfrom->authtypecode) : undef; my $authtypeto = $authto ? Koha::Authority::Types->find($authto->authtypecode) : undef; - - # search the tag to report my $auth_tag_to_report_from = $authtypefrom ? $authtypefrom->auth_tag_to_report : ''; my $auth_tag_to_report_to = $authtypeto ? $authtypeto->auth_tag_to_report : ''; @@ -1441,6 +1440,7 @@ sub merge { # Get All candidate Tags for the change # (This will reduce the search scope in marc records). # For a deleted authority record, we scan all auth controlled fields + my $dbh = C4::Context->dbh; my $sql = "SELECT DISTINCT tagfield FROM marc_subfield_structure WHERE authtypecode=?"; my $tags_using_authtype = $authtypefrom ? $dbh->selectcol_arrayref( $sql, undef, ( $authtypefrom->authtypecode )) : $dbh->selectcol_arrayref( "SELECT DISTINCT tagfield FROM marc_subfield_structure WHERE authtypecode IS NOT NULL AND authtypecode<>''" ); my $tags_new; @@ -1458,6 +1458,7 @@ sub merge { # And we need to add $9 in order not to duplicate $skip_subfields->{9} = 1 if !$overwrite; + my $counteditedbiblio = 0; foreach my $biblionumber ( @biblionumbers ) { my $marcrecord = GetMarcBiblio( $biblionumber ); next if !$marcrecord; --- a/misc/migration_tools/merge_authority.pl +++ a/misc/migration_tools/merge_authority.pl @@ -15,6 +15,8 @@ use C4::Context; use C4::Search; use C4::Biblio; use C4::AuthoritiesMarc; +use Koha::Authorities; +use Koha::Authority::MergeRequests; use Time::HiRes qw(gettimeofday); use Getopt::Long; @@ -80,16 +82,15 @@ print "Merging\n" unless $noconfirm; if ($batch) { my $authref; $dbh->do("update need_merge_authorities set done=2 where done=0"); #temporary status 2 means: selected for merge - $authref=$dbh->selectall_arrayref("select distinct authid from need_merge_authorities where done=2"); - foreach(@$authref) { - my $authid=$_->[0]; + $authref=$dbh->selectall_arrayref("select id,authid,authid_new from need_merge_authorities where done=2"); + foreach my $row ( @$authref ) { + my $req = Koha::Authority::MergeRequests->find( $row->[0] ); + my $marc = $req ? $req->oldmarc : undef; + my $authid = $row->[1]; + my $authid_new = $row->[2]; print "managing $authid\n" if $verbose; - my $MARCauth = GetAuthority( $authid ); - if( $MARCauth ) { - merge({ mergefrom => $authid, MARCfrom => $MARCauth, mergeto => $authid, MARCto => $MARCauth }); - } else { - merge({ mergefrom => $authid }); # handle a delete - } + # Following merge call handles both modifications and deletes + merge({ mergefrom => $authid, MARCfrom => $marc, mergeto => $authid_new, MARCto => GetAuthority($authid_new), override_limit => 1 }); } $dbh->do("update need_merge_authorities set done=1 where done=2"); #DONE } else { --- a/t/db_dependent/Authorities/Merge.t +++ a/t/db_dependent/Authorities/Merge.t @@ -4,7 +4,7 @@ use Modern::Perl; -use Test::More tests => 5; +use Test::More tests => 6; use Getopt::Long; use MARC::Record; @@ -15,6 +15,7 @@ use t::lib::TestBuilder; use C4::Biblio; use Koha::Authorities; +use Koha::Authority::MergeRequests; use Koha::Database; BEGIN { @@ -34,6 +35,36 @@ our @linkedrecords; my $mocks = set_mocks(); our ( $authtype1, $authtype2 ) = modify_framework(); +subtest 'Test postponed merge feature' => sub { + plan tests => 6; + + # Set limit to zero, and call merge a few times + t::lib::Mocks::mock_preference('AuthorityMergeLimit', 0); + my $auth1 = t::lib::TestBuilder->new->build({ source => 'AuthHeader' }); + my $cnt = Koha::Authority::MergeRequests->count; + merge({ mergefrom => '0' }); + is( Koha::Authority::MergeRequests->count, $cnt, 'No merge request added as expected' ); + merge({ mergefrom => $auth1->{authid} }); + is( Koha::Authority::MergeRequests->count, $cnt, 'No merge request added since we have zero hits' ); + @linkedrecords = ( 1, 2 ); # these biblionumbers do not matter + merge({ mergefrom => $auth1->{authid} }); + is( Koha::Authority::MergeRequests->count, $cnt + 1, 'Merge request added as expected' ); + + # Set limit to two (starting with two records) + t::lib::Mocks::mock_preference('AuthorityMergeLimit', 2); + merge({ mergefrom => $auth1->{authid} }); + is( Koha::Authority::MergeRequests->count, $cnt + 1, 'No merge request added as we do not exceed the limit' ); + @linkedrecords = ( 1, 2, 3 ); # these biblionumbers do not matter + merge({ mergefrom => $auth1->{authid} }); + is( Koha::Authority::MergeRequests->count, $cnt + 2, 'Merge request added as we do exceed the limit again' ); + # Now override + merge({ mergefrom => $auth1->{authid}, override_limit => 1 }); + is( Koha::Authority::MergeRequests->count, $cnt + 2, 'No merge request added as we did override' ); + + # Set merge limit high enough for the other subtests + t::lib::Mocks::mock_preference('AuthorityMergeLimit', 1000); +}; + subtest 'Test merge A1 to A2 (within same authtype)' => sub { # Tests originate from bug 11700 plan tests => 9; @@ -215,9 +246,6 @@ subtest 'Test merge A1 to B1 (changing authtype)' => sub { subtest 'Merging authorities should handle deletes (BZ 18070)' => sub { plan tests => 2; - # For this test we need dontmerge OFF - t::lib::Mocks::mock_preference('dontmerge', '0'); - # Add authority and linked biblio, delete authority my $auth1 = MARC::Record->new; $auth1->append_fields( MARC::Field->new( '109', '', '', 'a' => 'DEL')); @@ -235,7 +263,7 @@ subtest 'Merging authorities should handle deletes (BZ 18070)' => sub { my $marc1 = C4::Biblio::GetMarcBiblio( $biblionumber ); is( $marc1->field('609'), undef, 'Field 609 should be gone too' ); - # Now we simulate the delete as done from the cron job (with dontmerge) + # Now we simulate the delete as done in the cron job # First, restore auth1 and add 609 back in bib1 $auth1 = MARC::Record->new; $auth1->append_fields( MARC::Field->new( '109', '', '', 'a' => 'DEL')); @@ -247,7 +275,7 @@ subtest 'Merging authorities should handle deletes (BZ 18070)' => sub { # Instead of going through DelAuthority, we manually delete the auth # record and call merge afterwards. # This mimics deleting an authority and calling merge later in the - # merge_authority.pl cron job (when dontmerge is enabled). + # merge cron job. # We use the biblionumbers parameter here and unmock linked_biblionumbers. C4::Context->dbh->do( "DELETE FROM auth_header WHERE authid=?", undef, $authid1 ); @linkedrecords = (); @@ -260,10 +288,13 @@ subtest 'Merging authorities should handle deletes (BZ 18070)' => sub { sub set_mocks { # After we removed the Zebra code from merge, we only need to mock - # linked_biblionumbers here. + # get_usage_count and linked_biblionumbers here. my $mocks; $mocks->{auth_mod} = Test::MockModule->new( 'Koha::Authorities' ); + $mocks->{auth_mod}->mock( 'get_usage_count', sub { + return scalar @linkedrecords; + }); $mocks->{auth_mod}->mock( 'linked_biblionumbers', sub { return @linkedrecords; }); --