Bug 18014: AddAuthority should respect AUTO_INCREMENT

Instead of using the MAX(authid)+1 logic, AddAuthority should just save
the record and get the new id. The authid column is an autoincrement.

This eliminates problems where a newly assigned authid also refers to a
previously deleted record. (And it will not cause problems when refining
the dontmerge functionality on report 9988.)

Note: ModAuthority also calls AddAuthority to update an existing record; in
that case we should not create a new record even if the record should not
be found any more (which should be exceptional).

This patch also simplifies handling of 001 in the authority record: in all
cases this field is updated now; no need to check its contents.

Test plan:
[1] Run t/db_dependent/AuthoritiesMarc.t
[2] Add a new authority record via the interface

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This commit is contained in:
Marcel de Rooy 2017-01-30 15:19:35 +01:00 committed by Kyle M Hall
parent 24ac8392b3
commit ad38b24308
2 changed files with 20 additions and 34 deletions

View File

@ -663,38 +663,21 @@ sub AddAuthority {
$record->add_fields($auth_type_tag,'','', $auth_type_subfield=>$authtypecode);
}
my $auth_exists=0;
my $oldRecord;
if (!$authid) {
my $sth=$dbh->prepare("select max(authid) from auth_header");
$sth->execute;
($authid)=$sth->fetchrow;
$authid=$authid+1;
##Insert the recordID in MARC record
unless ($record->field('001') && $record->field('001')->data() eq $authid){
$record->delete_field($record->field('001'));
$record->insert_fields_ordered(MARC::Field->new('001',$authid));
# Save record into auth_header, update 001
if (!$authid ) {
# Save a blank record, get authid
$dbh->do( "INSERT INTO auth_header (datecreated,marcxml) values (NOW(),?)", undef, '' );
$authid = $dbh->last_insert_id( undef, undef, 'auth_header', 'authid' );
logaction( "AUTHORITIES", "ADD", $authid, "authority" ) if C4::Context->preference("AuthoritiesLog");
}
} else {
$auth_exists=$dbh->do(qq(select authid from auth_header where authid=?),undef,$authid);
# warn "auth_exists = $auth_exists";
}
if ($auth_exists>0){
$oldRecord=GetAuthority($authid);
$record->add_fields('001',$authid) unless ($record->field('001'));
# warn "\n\n\n enregistrement".$record->as_formatted;
my $sth=$dbh->prepare("update auth_header set authtypecode=?,marc=?,marcxml=? where authid=?");
$sth->execute($authtypecode,$record->as_usmarc,$record->as_xml_record($format),$authid) or die $sth->errstr;
$sth->finish;
}
else {
my $sth=$dbh->prepare("insert into auth_header (authid,datecreated,authtypecode,marc,marcxml) values (?,now(),?,?,?)");
$sth->execute($authid,$authtypecode,$record->as_usmarc,$record->as_xml_record($format));
$sth->finish;
logaction( "AUTHORITIES", "ADD", $authid, "authority" ) if C4::Context->preference("AuthoritiesLog");
}
ModZebra($authid,'specialUpdate',"authorityserver",$oldRecord,$record);
return ($authid);
# Insert/update the recordID in MARC record
$record->delete_field( $record->field('001') );
$record->insert_fields_ordered( MARC::Field->new( '001', $authid ) );
# Update
$dbh->do( "UPDATE auth_header SET authtypecode=?, marc=?, marcxml=? WHERE authid=?", undef, $authtypecode, $record->as_usmarc, $record->as_xml_record($format), $authid ) or die $DBI::errstr;
ModZebra( $authid, 'specialUpdate', 'authorityserver', $record );
return ( $authid );
}

View File

@ -192,15 +192,18 @@ is_deeply(
);
subtest 'AddAuthority should respect AUTO_INCREMENT (BZ 18104)' => sub {
plan tests => 1;
plan tests => 3;
t::lib::Mocks::mock_preference( 'marcflavour', 'MARC21' );
my $record = C4::AuthoritiesMarc::GetAuthority(1);
my $id1 = AddAuthority( $record, undef, 'GEOGR_NAME' );
DelAuthority( $id1 );
my $id2 = AddAuthority( $record, undef, 'GEOGR_NAME' );
is( $id1, $id2, 'FIXME: Got the same id back, let\'s fix that behavior' );
isnt( $id1, $id2, 'Do not return the same id again' );
t::lib::Mocks::mock_preference( 'marcflavour', 'UNIMARC' );
my $id3 = AddAuthority( $record, undef, 'GEOGR_NAME' );
is( $id3 > 0, 1, 'Tested AddAuthority with UNIMARC' );
is( $record->field('001')->data, $id3, 'Check updated 001' );
};
$schema->storage->txn_rollback;