From 12c9f3afca7582c6581dbc4d11f8a85f68e0b70a Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 3 Dec 2019 10:13:38 +0100 Subject: [PATCH] Bug 8132: Delete the items in a transaction and rollback if something wrong The last_item_for_hold case cannot be guessed (easily), and so we are going to delete the items in a transaction. If something wrong happened we rollback and display a list of items that caused the rollback. --- .../prog/en/modules/tools/batchMod-del.tt | 23 +++- tools/batchMod.pl | 141 +++++++++++++-------- 2 files changed, 111 insertions(+), 53 deletions(-) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batchMod-del.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batchMod-del.tt index 213921538d..366455af13 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batchMod-del.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batchMod-del.tt @@ -120,7 +120,8 @@ [% CASE "not_same_branch" %][% SET cannot_delete_reason = t("Item does not belongs to your library") %] [% CASE "book_reserved" %][% SET cannot_delete_reason = t("Item has a waiting hold") %] [% CASE "linked_analytics" %][% SET cannot_delete_reason = t("Item has linked analytics") %] - [% CASE %][% SET cannot_delete_reason = t("Unknown reason") %] + [% CASE "last_item_for_hold" %][% SET cannot_delete_reason = t("Last item for bibliographic record wich biblio-level hold on it") %] + [% CASE %][% SET cannot_delete_reason = t("Unknown reason") _ '(' _ can_be_deleted _ ')' %] [% END %] @@ -210,7 +211,12 @@ [% END %] [% IF ( action ) %] -
+ [% IF deletion_failed %] +
+ At least one item blocked the deletion. The operation rolled back and nothing happened! +
+ [% ELSE %] +

[% deleted_items | html %] item(s) deleted.

[% IF delete_records %]

[% deleted_records | html %] record(s) deleted.

[% END %] [% IF src == 'CATALOGUING' # from catalogue/detail.pl > Delete items in a batch%] @@ -221,6 +227,7 @@ Return to batch item deletion [% END %]
+ [% END %] [% IF ( not_deleted_items ) %]

[% not_deleted_items | html %] item(s) could not be deleted: [% FOREACH not_deleted_itemnumber IN not_deleted_itemnumbers %][% not_deleted_itemnumber.itemnumber | html %][% END %]

@@ -238,7 +245,17 @@ [% not_deleted_loo.itemnumber | html %] [% IF ( CAN_user_editcatalogue_edit_items ) %][% not_deleted_loo.barcode | html %][% ELSE %][% not_deleted_loo.barcode | html %][% END %] - [% IF ( not_deleted_loo.book_on_loan ) %]Item is checked out[% ELSIF ( not_deleted_loo.book_reserved ) %]Item has a waiting hold[% END %] + + [% SWITCH not_deleted_loo.reason %] + [% CASE "book_on_loan" %][% SET cannot_delete_reason = t("Item is checked out") %] + [% CASE "not_same_branch" %][% SET cannot_delete_reason = t("Item does not belongs to your library") %] + [% CASE "book_reserved" %][% SET cannot_delete_reason = t("Item has a waiting hold") %] + [% CASE "linked_analytics" %][% SET cannot_delete_reason = t("Item has linked analytics") %] + [% CASE "last_item_for_hold" %][% SET cannot_delete_reason = t("Last item for bibliographic record wich biblio-level hold on it") %] + [% CASE %][% SET cannot_delete_reason = t("Unknown reason") _ '(' _ can_be_deleted _ ')' %] + [% END %] + [% cannot_delete_reason %] + [% END %] diff --git a/tools/batchMod.pl b/tools/batchMod.pl index e8980cdbcb..f33aceeeac 100755 --- a/tools/batchMod.pl +++ b/tools/batchMod.pl @@ -20,6 +20,8 @@ use CGI qw ( -utf8 ); use Modern::Perl; +use Try::Tiny; + use C4::Auth; use C4::Output; use C4::Biblio; @@ -34,6 +36,8 @@ use C4::Members; use MARC::File::XML; use List::MoreUtils qw/uniq/; +use Koha::Database; +use Koha::Exceptions::Exception; use Koha::AuthorisedValues; use Koha::Biblios; use Koha::DateUtils; @@ -177,61 +181,98 @@ if ($op eq "action") { } } - # For each item - my $i = 1; - foreach my $itemnumber(@itemnumbers){ - - $job->progress($i) if $runinbackground; - my $item = Koha::Items->find($itemnumber); - next unless $item; # Should have been tested earlier, but just in case... - my $itemdata = $item->unblessed; - if ( $del ){ - my $return = $item->safe_delete; - if ($return == 1) { - $deleted_items++; - } else { - $not_deleted_items++; - push @not_deleted, - { biblionumber => $itemdata->{'biblionumber'}, - itemnumber => $itemdata->{'itemnumber'}, - barcode => $itemdata->{'barcode'}, - title => $itemdata->{'title'}, - $return => 1 - }; - } + try { + my $schema = Koha::Database->new->schema; + $schema->txn_do( + sub { + # For each item + my $i = 1; + foreach my $itemnumber (@itemnumbers) { + $job->progress($i) if $runinbackground; + my $item = Koha::Items->find($itemnumber); + next + unless $item + ; # Should have been tested earlier, but just in case... + my $itemdata = $item->unblessed; + if ($del) { + my $return = $item->safe_delete; + if ( $return eq '1' ) { + $deleted_items++; + } + else { + $not_deleted_items++; + push @not_deleted, + { + biblionumber => $itemdata->{'biblionumber'}, + itemnumber => $itemdata->{'itemnumber'}, + barcode => $itemdata->{'barcode'}, + title => $itemdata->{'title'}, + reason => $return, + }; + } - # If there are no items left, delete the biblio - if ( $del_records ) { - my $itemscount = Koha::Biblios->find( $itemdata->{'biblionumber'} )->items->count; - if ( $itemscount == 0 ) { - my $error = DelBiblio($itemdata->{'biblionumber'}); - $deleted_records++ unless ( $error ); + # If there are no items left, delete the biblio + if ($del_records) { + my $itemscount = + Koha::Biblios->find( + $itemdata->{'biblionumber'} )->items->count; + if ( $itemscount == 0 ) { + my $error = + DelBiblio( $itemdata->{'biblionumber'} ); + $deleted_records++ unless ($error); + } } } - } else { - if ($values_to_modify || $values_to_blank) { - my $localmarcitem = Item2Marc($itemdata); - - my $modified = UpdateMarcWith( $marcitem, $localmarcitem ); - if ( $modified ) { - eval { - if ( my $item = ModItemFromMarc( $localmarcitem, $itemdata->{biblionumber}, $itemnumber ) ) { - LostItem($itemnumber, 'batchmod') if $item->{itemlost} and not $itemdata->{itemlost}; + else { + if ( $values_to_modify || $values_to_blank ) { + my $localmarcitem = Item2Marc($itemdata); + + my $modified = + UpdateMarcWith( $marcitem, $localmarcitem ); + if ($modified) { + eval { + if ( + my $item = ModItemFromMarc( + $localmarcitem, + $itemdata->{biblionumber}, + $itemnumber + ) + ) + { + LostItem( $itemnumber, 'batchmod' ) + if $item->{itemlost} + and not $itemdata->{itemlost}; + } + }; + } + if ($runinbackground) { + $modified_items++ if $modified; + $modified_fields += $modified; + $job->set( + { + modified_items => $modified_items, + modified_fields => $modified_fields, + } + ); + } + } } - }; - } - if ( $runinbackground ) { - $modified_items++ if $modified; - $modified_fields += $modified; - $job->set({ - modified_items => $modified_items, - modified_fields => $modified_fields, - }); + $i++; + } + if (@not_deleted) { + Koha::Exceptions::Exception->throw( + 'Some items have not been deleted, rolling back'); + } } - } - } - $i++; - } + ); + } + catch { + if ( $_->isa('Koha::Exceptions::Exception') ) { + $template->param( deletion_failed => 1 ); + } + die "Something terrible has happened!" + if ($_ =~ /Rollback failed/); # Rollback failed + } } } # -- 2.11.0