Bug 9988: Few subtle changes for postponed merge
The fails in the previous test showed that we need the first three
changes here. Some final polishing in points 4 to 6.
[1] Sub merge: Refine the condition for initializing $tags_new.
A postponed 'modify'-merge (A to B) makes that $authtypefrom is not
defined when running merge later. When crossing the type boundary, we
need a new field too.
[2] Sub merge: Add condition for an empty @record_to array.
This indicates also that a field should be removed, since we should
otherwise only add a $9 subfield.
[3] Sub merge: Adjust initializing @record_from.
This change is tested by verifying a cleared subfield in the test.
[4] DelAuthority: Adding a skipmerge parameter to allow the call from
authorities/merge.pl to skip an unneeded merge.
This also prevents that the 'delete-merge' would precede the
'modify-merge' under a hypothetical race condition.
[5] DelAuthority: There is actually no need to call GetAuthority.
The subfields of the old record are not relevant in this case.
[6] Added a few POD lines to merge.
[7] Removed a trailing space in a comment line in merge.
Test plan:
[1] Run t/db_dependent/Authorities/Merge.t.
The last subtest should no longer fail now.
[2] See test plan of next patch.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This commit is contained in:
Marcel de Rooy
Kyle M Hall
parent
7d53670279
commit
d49e8bd5e2
|
|
@ -1,4 +1,5 @@
|
|||
package C4::AuthoritiesMarc;
|
||||
|
||||
# Copyright 2000-2002 Katipo Communications
|
||||
#
|
||||
# This file is part of Koha.
|
||||
|
|
@ -683,17 +684,20 @@ sub AddAuthority {
|
|||
|
||||
=head2 DelAuthority
|
||||
|
||||
DelAuthority({ authid => $authid });
|
||||
DelAuthority({ authid => $authid, [ skip_merge => 1 ] });
|
||||
|
||||
Deletes $authid and calls merge to cleanup in linked biblio records
|
||||
Deletes $authid and calls merge to cleanup linked biblio records.
|
||||
Parameter skip_merge is used in authorities/merge.pl. You should normally not
|
||||
use it.
|
||||
|
||||
=cut
|
||||
|
||||
sub DelAuthority {
|
||||
my ( $params ) = @_;
|
||||
my $authid = $params->{authid} || return;
|
||||
my $dbh=C4::Context->dbh;
|
||||
merge({ mergefrom => $authid, MARCfrom => GetAuthority($authid) });
|
||||
my $skip_merge = $params->{skip_merge};
|
||||
my $dbh = C4::Context->dbh;
|
||||
merge({ mergefrom => $authid }) if !$skip_merge;
|
||||
$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);
|
||||
|
|
@ -1370,19 +1374,24 @@ sub AddAuthorityTrees{
|
|||
=head2 merge
|
||||
|
||||
$count = merge({
|
||||
mergefrom => mergefrom,
|
||||
MARCfrom => $MARCfrom,
|
||||
mergefrom => $mergefrom,
|
||||
[ MARCfrom => $MARCfrom, ]
|
||||
[ mergeto => $mergeto, ]
|
||||
[ MARCto => $MARCto, ]
|
||||
[ biblionumbers => [ $a, $b, $c ], ]
|
||||
[ override_limit => 1, ]
|
||||
});
|
||||
|
||||
Merge biblios linked to authority $mergefrom.
|
||||
Merge biblios linked to authority $mergefrom (mandatory parameter).
|
||||
If $mergeto equals mergefrom, the linked biblio field is updated.
|
||||
If $mergeto is different, the biblio field will be linked to $mergeto.
|
||||
If $mergeto is missing, the biblio field is deleted.
|
||||
|
||||
MARCfrom is used to determine if a cleared subfield in the authority record
|
||||
should be removed from a biblio. MARCto is used to populate the biblio
|
||||
record with the updated values; if you do not pass it, the biblio field
|
||||
will be deleted (same as missing mergeto).
|
||||
|
||||
Normally all biblio records linked to $mergefrom, will be considered. But
|
||||
you can pass specific numbers via the biblionumbers parameter.
|
||||
|
||||
|
|
@ -1435,16 +1444,22 @@ sub merge {
|
|||
my @record_to;
|
||||
@record_to = $MARCto->field($auth_tag_to_report_to)->subfields() if $auth_tag_to_report_to && $MARCto && $MARCto->field($auth_tag_to_report_to);
|
||||
my @record_from;
|
||||
@record_from = $MARCfrom->field($auth_tag_to_report_from)->subfields() if $auth_tag_to_report_from && $MARCfrom && $MARCfrom->field($auth_tag_to_report_from);
|
||||
if( !$authfrom && $MARCfrom && $MARCfrom->field('1..','2..') ) {
|
||||
# postponed merge, authfrom was deleted and MARCfrom only contains the old reporting tag (and possibly a 100 for UNIMARC)
|
||||
# 2XX is for UNIMARC; we use -1 in order to skip 100 in UNIMARC; this will not impact MARC21, since there is only one tag
|
||||
@record_from = ( $MARCfrom->field('1..','2..') )[-1]->subfields;
|
||||
} elsif( $auth_tag_to_report_from && $MARCfrom && $MARCfrom->field($auth_tag_to_report_from) ) {
|
||||
@record_from = $MARCfrom->field($auth_tag_to_report_from)->subfields;
|
||||
}
|
||||
|
||||
# Get All candidate Tags for the change
|
||||
# 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;
|
||||
if( $authtypefrom && $authtypeto && $authtypeto->authtypecode ne $authtypefrom->authtypecode ) {
|
||||
if( $authtypeto && ( !$authtypefrom || $authtypeto->authtypecode ne $authtypefrom->authtypecode )) {
|
||||
$tags_new = $dbh->selectcol_arrayref( $sql, undef, ( $authtypeto->authtypecode ));
|
||||
}
|
||||
|
||||
|
|
@ -1470,9 +1485,10 @@ sub merge {
|
|||
my $tag = $field->tag();
|
||||
next if !defined($auth_number) || $auth_number ne $mergefrom;
|
||||
$countfrom++;
|
||||
if ( !$mergeto || ( $overwrite && $countfrom > 1 ) ) {
|
||||
# if mergeto is missing, this indicates a delete
|
||||
# Or: remove this duplicate in strict mode
|
||||
if ( !$mergeto || !@record_to ||
|
||||
( $overwrite && $countfrom > 1 ) ) {
|
||||
# !mergeto or !record_to indicates a delete
|
||||
# Other condition: remove this duplicate in strict mode
|
||||
$marcrecord->delete_field($field);
|
||||
$update = 1;
|
||||
next;
|
||||
|
|
|
|||
|
|
@ -65,8 +65,9 @@ if ($merge) {
|
|||
my $MARCfrom = GetAuthority( $recordid2 );
|
||||
merge({ mergefrom => $recordid2, MARCfrom => $MARCfrom, mergeto => $recordid1, MARCto => $record });
|
||||
|
||||
# Deleting the other record
|
||||
DelAuthority({ authid => $recordid2 });
|
||||
# Delete the other record. Do not merge. It is unneeded and could under
|
||||
# special circumstances have unwanted side-effects.
|
||||
DelAuthority({ authid => $recordid2, skip_merge => 1 });
|
||||
|
||||
# Parameters
|
||||
$template->param(
|
||||
|
|
|
|||
|
|
@ -323,7 +323,7 @@ subtest "Test some specific postponed merge issues" => sub {
|
|||
# The modify merge would be useless after that.
|
||||
@linkedrecords = ( $biblionumber );
|
||||
my $restored_mocks = set_mocks();
|
||||
DelAuthority({ authid => $id }); # delete A
|
||||
DelAuthority({ authid => $id, skip_merge => 1 }); # delete A
|
||||
$restored_mocks->{auth_mod}->unmock_all;
|
||||
$biblio = C4::Biblio::GetMarcBiblio( $biblionumber );
|
||||
is( $biblio->subfield('109', '9'), $id, 'If the 109 is no longer present, another modify merge would not bring it back' );
|
||||
|
|
|
|||
Loading…
Reference in New Issue