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.

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

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 8701d9e..38e946b 100644 (file)
--- 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 %]
 
                         <td><input type="checkbox" name="itemnumber" value="[% item_loo.itemnumber | html %]" id="row[% item_loo.itemnumber | html %]" disabled="disabled" title="[% cannot_delete_reason %]"/></td>
@@ -210,7 +211,12 @@
     [% END %]
 
 [% IF ( action ) %]
-    <div class="dialog message">
+    [% IF deletion_failed %]
+        <div class="dialog alert">
+            At least one item blocked the deletion. The operation rolled back and nothing happened!
+        </div>
+    [% ELSE %]
+        <div class="dialog message">
             <p>[% deleted_items | html %] item(s) deleted.</p>
             [% IF delete_records %] <p>[% deleted_records | html %] record(s) deleted.</p> [% END %]
             [% IF src == 'CATALOGUING' # from catalogue/detail.pl > Delete items in a batch%]
@@ -225,6 +231,7 @@
                 <a href="/cgi-bin/koha/tools/batchMod.pl?del=1">Return to batch item deletion</a>
             [% END %]
         </div>
+    [% END %]
     [% IF ( not_deleted_items ) %]
     <div style="width:55%;margin:auto;">
         <p>[% not_deleted_items | html %] item(s) could not be deleted: [% FOREACH not_deleted_itemnumber IN not_deleted_itemnumbers %][% not_deleted_itemnumber.itemnumber | html %][% END %]</p>
@@ -242,7 +249,17 @@
             <tr>
                 <td>[% not_deleted_loo.itemnumber | html %]</td>
                 <td>[% IF ( CAN_user_editcatalogue_edit_items ) %]<a href="/cgi-bin/koha/cataloguing/additem.pl?op=edititem&amp;biblionumber=[% not_deleted_loo.biblionumber | uri %]&amp;itemnumber=[% not_deleted_loo.itemnumber | uri %]">[% not_deleted_loo.barcode | html %]</a>[% ELSE %][% not_deleted_loo.barcode | html %][% END %]</td>
-                <td>[% IF ( not_deleted_loo.book_on_loan ) %]Item is checked out[% ELSIF ( not_deleted_loo.book_reserved ) %]Item has a waiting hold[% END %]</td>
+                <td>
+                    [% 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 %]
+                </td>
             </tr>
             [% END %]
         </tbody>

diff --git a/tools/batchMod.pl b/tools/batchMod.pl
index a7e3188..91ede79 100755 (executable)
--- 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;
@@ -181,107 +185,140 @@ 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 (ref($return)) {
-                           $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 ( ref( $return ) ) {
+                                $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'} );
-                        unless ($error) {
-                            $deleted_records++;
-                            if ( $src eq 'CATALOGUING' ) {
-                                # We are coming catalogue/detail.pl, there were items from a single bib record
-                                $template->param( biblio_deleted => 1 );
+                            # 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'} );
+                                    unless ($error) {
+                                        $deleted_records++;
+                                        if ( $src eq 'CATALOGUING' ) {
+                                            # We are coming catalogue/detail.pl, there were items from a single bib record
+                                            $template->param( biblio_deleted => 1 );
+                                        }
+                                    }
+                                }
                             }
                         }
-                    }
-                }
-               } else {
-            if ($values_to_modify || $values_to_blank) {
-                my $localmarcitem = Item2Marc($itemdata);
-                my $modified = 0;
-
-                for ( my $i = 0 ; $i < @tags ; $i++ ) {
-                    my $search = $searches[$i];
-                    next unless $search;
-
-                    my $tag = $tags[$i];
-                    my $subfield = $subfields[$i];
-                    my $replace = $replaces[$i];
-
-                    my $value = $localmarcitem->field( $tag )->subfield( $subfield );
-                    my $old_value = $value;
-
-                    my @available_modifiers = qw( i g );
-                    my $retained_modifiers = q||;
-                    for my $modifier ( split //, $modifiers[$i] ) {
-                        $retained_modifiers .= $modifier
-                            if grep {/$modifier/} @available_modifiers;
-                    }
-                    if ( $retained_modifiers =~ m/^(ig|gi)$/ ) {
-                        $value =~ s/$search/$replace/ig;
-                    }
-                    elsif ( $retained_modifiers eq 'i' ) {
-                        $value =~ s/$search/$replace/i;
-                    }
-                    elsif ( $retained_modifiers eq 'g' ) {
-                        $value =~ s/$search/$replace/g;
-                    }
-                    else {
-                        $value =~ s/$search/$replace/;
-                    }
-
-                    my @fields_to = $localmarcitem->field($tag);
-                    foreach my $field_to_update ( @fields_to ) {
-                        unless ( $old_value eq $value ) {
-                            $modified++;
-                            $field_to_update->update( $subfield => $value );
+                        else {
+                            if ( $values_to_modify || $values_to_blank ) {
+                                my $localmarcitem = Item2Marc($itemdata);
+                                my $modified = 0;
+
+                                for ( my $i = 0 ; $i < @tags ; $i++ ) {
+                                    my $search = $searches[$i];
+                                    next unless $search;
+
+                                    my $tag = $tags[$i];
+                                    my $subfield = $subfields[$i];
+                                    my $replace = $replaces[$i];
+
+                                    my $value = $localmarcitem->field( $tag )->subfield( $subfield );
+                                    my $old_value = $value;
+
+                                    my @available_modifiers = qw( i g );
+                                    my $retained_modifiers = q||;
+                                    for my $modifier ( split //, $modifiers[$i] ) {
+                                        $retained_modifiers .= $modifier
+                                            if grep {/$modifier/} @available_modifiers;
+                                    }
+                                    if ( $retained_modifiers =~ m/^(ig|gi)$/ ) {
+                                        $value =~ s/$search/$replace/ig;
+                                    }
+                                    elsif ( $retained_modifiers eq 'i' ) {
+                                        $value =~ s/$search/$replace/i;
+                                    }
+                                    elsif ( $retained_modifiers eq 'g' ) {
+                                        $value =~ s/$search/$replace/g;
+                                    }
+                                    else {
+                                        $value =~ s/$search/$replace/;
+                                    }
+
+                                    my @fields_to = $localmarcitem->field($tag);
+                                    foreach my $field_to_update ( @fields_to ) {
+                                        unless ( $old_value eq $value ) {
+                                            $modified++;
+                                            $field_to_update->update( $subfield => $value );
+                                        }
+                                    }
+                                }
+
+                                $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,
+                                        }
+                                    );
+                                }
+                            }
                         }
+                        $i++;
+                    }
+                    if (@not_deleted) {
+                        Koha::Exceptions::Exception->throw(
+                            'Some items have not been deleted, rolling back');
                     }
                 }
-
-                $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,
-                    });
-                }
-                   }
-               }
-               $i++;
-       }
+            );
+        }
+        catch {
+            if ( $_->isa('Koha::Exceptions::Exception') ) {
+                $template->param( deletion_failed => 1 );
+            }
+            die "Something terrible has happened!"
+                if ($_ =~ /Rollback failed/); # Rollback failed
+        }
     }
 }
 #