From 6c75710b97ff620ad4db70cdf5017871f5a37507 Mon Sep 17 00:00:00 2001 From: Benjamin Rokseth Date: Mon, 19 Mar 2018 14:41:29 +0100 Subject: [PATCH] Bug 20271: fix remaining tests, add checks and report in atomicupdate --- C4/Circulation.pm | 3 + ...eleteitems_tables_with_their_alive_cousins.perl | 78 ++++++++++++++++++---- t/db_dependent/Acquisition.t | 4 +- t/db_dependent/Items_DelItemCheck.t | 2 +- t/db_dependent/db_structure.t | 3 - t/lib/TestBuilder.pm | 10 +++ 6 files changed, 81 insertions(+), 19 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 3e711cd..1d15101 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1801,6 +1801,9 @@ sub AddReturn { unless ($item) { return ( 0, { BadBarcode => $barcode } ); # no barcode means no item or borrower. bail out. } + if ($item->{deleted_at}) { + return ( 0, { BadBarcode => $barcode } ); + } my $itemnumber = $item->{ itemnumber }; my $itemtype = $item->{itype}; # GetItem called effective_itemtype diff --git a/installer/data/mysql/atomicupdate/bug_20271_-_merge_deletedbiblio_and_deleteitems_tables_with_their_alive_cousins.perl b/installer/data/mysql/atomicupdate/bug_20271_-_merge_deletedbiblio_and_deleteitems_tables_with_their_alive_cousins.perl index 2b493fe..99bc0da 100644 --- a/installer/data/mysql/atomicupdate/bug_20271_-_merge_deletedbiblio_and_deleteitems_tables_with_their_alive_cousins.perl +++ b/installer/data/mysql/atomicupdate/bug_20271_-_merge_deletedbiblio_and_deleteitems_tables_with_their_alive_cousins.perl @@ -1,20 +1,72 @@ $DBversion = 'XXX'; # will be replaced by the RM if( CheckVersion( $DBversion ) ) { - $dbh->do( "ALTER TABLE biblio ADD COLUMN deleted_at datetime DEFAULT NULL" ); - $dbh->do( "ALTER TABLE biblioitems ADD COLUMN deleted_at datetime DEFAULT NULL" ); - $dbh->do( "ALTER TABLE biblio_metadata ADD COLUMN deleted_at datetime DEFAULT NULL" ); - $dbh->do( "ALTER TABLE items ADD COLUMN deleted_at datetime DEFAULT NULL" ); + $dbh->do( "ALTER TABLE biblio ADD COLUMN deleted_at datetime DEFAULT NULL" ) or die $DBI::errstr; + $dbh->do( "ALTER TABLE biblioitems ADD COLUMN deleted_at datetime DEFAULT NULL" ) or die $DBI::errstr; + $dbh->do( "ALTER TABLE biblio_metadata ADD COLUMN deleted_at datetime DEFAULT NULL" ) or die $DBI::errstr; + $dbh->do( "ALTER TABLE items ADD COLUMN deleted_at datetime DEFAULT NULL" ) or die $DBI::errstr; - $dbh->do( "INSERT IGNORE INTO biblio SELECT *, timestamp AS deleted_at FROM deletedbiblio" ); - $dbh->do( "INSERT IGNORE INTO biblioitems SELECT *, timestamp AS deleted_at FROM deletedbiblioitems" ); - $dbh->do( "INSERT IGNORE INTO biblio_metadata SELECT *, timestamp AS deleted_at FROM deletedbiblio_metadata" ); - $dbh->do( "INSERT IGNORE INTO items SELECT *, timestamp AS deleted_at FROM deleteditems WHERE biblionumber IN (SELECT biblionumber FROM biblio)" ); + $dbh->do( "ALTER TABLE deletedbiblio_metadata DROP FOREIGN KEY deletedbiblio_metadata_fk_1" ) or die $DBI::errstr; - my $sth = $dbh->prepare("SELECT * FROM deleteditems WHERE itemnumber NOT IN (SELECT itemnumber FROM items)"); - $sth->execute(); - while ( my $it = $sth->fetchrow_hashref ) { - warn "Following item could not be moved from deleteditems, probably duplicate barcode or never moved: " . - "itemnumber: " . $it->{itemnumber} . " - barcode: ". $it->{barcode} . "\n"; + $dbh->do( "INSERT IGNORE INTO biblio SELECT *, timestamp AS deleted_at FROM deletedbiblio" ) or die $DBI::errstr; + $dbh->do( "INSERT IGNORE INTO biblioitems SELECT *, timestamp AS deleted_at FROM deletedbiblioitems" ) or die $DBI::errstr; + $dbh->do( "INSERT IGNORE INTO biblio_metadata SELECT *, timestamp AS deleted_at FROM deletedbiblio_metadata" ) or die $DBI::errstr; + $dbh->do( "INSERT IGNORE INTO items SELECT *, timestamp AS deleted_at FROM deleteditems" ) or die $DBI::errstr; + + # Check if any rows could not be moved, if so, rename table with underscore for checking, otherwise drop them + { + my $sth = $dbh->prepare("DELETE FROM deleteditems WHERE itemnumber IN (SELECT itemnumber FROM items)"); + $sth->execute(); + $sth = $dbh->prepare("SELECT COUNT(*) AS count FROM deleteditems"); + $sth->execute(); + my $row = $sth->fetchrow_hashref; + if ($row->{count}) { + warn "There were $row->{count} deleteditems that could not be moved, please check '_deleteditems'."; + $dbh->do("RENAME TABLE deleteditems TO _deleteditems"); + } else { + $dbh->do("DROP TABLE deleteditems"); + } + } + + { + my $sth = $dbh->prepare("DELETE FROM deletedbiblio_metadata WHERE biblionumber IN (SELECT biblionumber FROM biblio_metadata)"); + $sth->execute(); + $sth = $dbh->prepare("SELECT COUNT(*) AS count FROM deletedbiblio_metadata"); + $sth->execute(); + my $row = $sth->fetchrow_hashref; + if ($row->{count}) { + warn "There were $row->{count} deletedbiblio_metadata that could not be moved, please check '_deletedbiblio_metadata'."; + $dbh->do("RENAME TABLE deletedbiblio_metadata TO _deletedbiblio_metadata"); + } else { + $dbh->do("DROP TABLE deletedbiblio_metadata"); + } + } + + { + my $sth = $dbh->prepare("DELETE FROM deletedbiblioitems WHERE biblioitemnumber IN (SELECT biblioitemnumber FROM biblioitems)"); + $sth->execute(); + $sth = $dbh->prepare("SELECT COUNT(*) AS count FROM deletedbiblioitems"); + $sth->execute(); + my $row = $sth->fetchrow_hashref; + if ($row->{count}) { + warn "There were $row->{count} deletedbiblioitems that could not be moved, please check '_deletedbiblioitems'."; + $dbh->do("RENAME TABLE deletedbiblioitems TO _deletedbiblioitems"); + } else { + $dbh->do("DROP TABLE deletedbiblioitems"); + } + } + + { + my $sth = $dbh->prepare("DELETE FROM deletedbiblio WHERE biblionumber IN (SELECT biblionumber FROM biblio)"); + $sth->execute(); + $sth = $dbh->prepare("SELECT COUNT(*) AS count FROM deletedbiblio WHERE biblionumber NOT IN (SELECT biblionumber FROM deletedbiblioitems)"); + $sth->execute(); + my $row = $sth->fetchrow_hashref; + if ($row->{count}) { + warn "There were $row->{count} deletedbiblio that could not be moved, please check '_deletedbiblio'."; + $dbh->do("RENAME TABLE deletedbiblio TO _deletedbiblio"); + } else { + $dbh->do("DROP TABLE deletedbiblio"); + } } SetVersion( $DBversion ); diff --git a/t/db_dependent/Acquisition.t b/t/db_dependent/Acquisition.t index 975fb03..c5a36e9 100755 --- a/t/db_dependent/Acquisition.t +++ b/t/db_dependent/Acquisition.t @@ -513,7 +513,7 @@ ok((not defined $error), "DelOrder does not fail"); $order2 = GetOrder($order2->{ordernumber}); ok((defined $order2->{datecancellationprinted}), "order is cancelled"); ok((not defined $order2->{cancellationreason}), "order has no cancellation reason"); -ok((not defined Koha::Biblios->find( $order2->{biblionumber} )), "biblio does not exist anymore"); +ok(Koha::Biblios->find($order2->{biblionumber})->deleted_at , "biblio does not exist anymore"); my $order4 = GetOrder($ordernumbers[3]); $error = DelOrder($order4->{biblionumber}, $order4->{ordernumber}, 1, "foobar"); @@ -521,7 +521,7 @@ ok((not defined $error), "DelOrder does not fail"); $order4 = GetOrder($order4->{ordernumber}); ok((defined $order4->{datecancellationprinted}), "order is cancelled"); ok(($order4->{cancellationreason} eq "foobar"), "order has cancellation reason \"foobar\""); -ok((not defined Koha::Biblios->find( $order4->{biblionumber} )), "biblio does not exist anymore"); +ok(Koha::Biblios->find($order4->{biblionumber})->deleted_at, "biblio does not exist anymore"); my $order5 = GetOrder($ordernumbers[4]); C4::Items::AddItem( { barcode => '0102030405' }, $order5->{biblionumber} ); diff --git a/t/db_dependent/Items_DelItemCheck.t b/t/db_dependent/Items_DelItemCheck.t index 57514d0..0963ebd 100644 --- a/t/db_dependent/Items_DelItemCheck.t +++ b/t/db_dependent/Items_DelItemCheck.t @@ -161,7 +161,7 @@ DelItemCheck( $biblio->{biblionumber}, $item->{itemnumber} ); my $test_item = GetItem( $item->{itemnumber} ); -is( $test_item->{itemnumber}, undef, +isnt( $test_item->{deleted_at}, undef, "DelItemCheck should delete item if ItemSafeToDelete returns true" ); diff --git a/t/db_dependent/db_structure.t b/t/db_dependent/db_structure.t index 41de507..86227fd 100644 --- a/t/db_dependent/db_structure.t +++ b/t/db_dependent/db_structure.t @@ -22,9 +22,6 @@ use Koha::Database; my @modules = ( [ qw( Borrower Deletedborrower ) ], - [ qw( Biblio Deletedbiblio ) ], - [ qw( Biblioitem Deletedbiblioitem ) ], - [ qw( Item Deleteditem ) ], ); my @keys_to_check = qw( size is_nullable data_type accessor datetime_undef_if_invalid default_value ); diff --git a/t/lib/TestBuilder.pm b/t/lib/TestBuilder.pm index 1aa6de9..e098ea1 100644 --- a/t/lib/TestBuilder.pm +++ b/t/lib/TestBuilder.pm @@ -470,8 +470,18 @@ sub _gen_default_values { debarred => undef, borrowernotes => '', }, + Biblio => { + deleted_at => undef, + }, + Biblioitems => { + deleted_at => undef, + }, + BiblioMetadata => { + deleted_at => undef, + }, Item => { more_subfields_xml => undef, + deleted_at => undef, }, Category => { enrolmentfee => 0, -- 2.1.4