Bugzilla – Attachment 103168 Details for
Bug 8132
Batch delete tool deletes items with holds on them
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 8132: Delete the items in a transaction and rollback if something wrong
Bug-8132-Delete-the-items-in-a-transaction-and-rol.patch (text/plain), 15.97 KB, created by
Victor Grousset/tuxayo
on 2020-04-17 14:58:56 UTC
(
hide
)
Description:
Bug 8132: Delete the items in a transaction and rollback if something wrong
Filename:
MIME Type:
Creator:
Victor Grousset/tuxayo
Created:
2020-04-17 14:58:56 UTC
Size:
15.97 KB
patch
obsolete
>From 1c8e622d275fe21c259afcbe5b9d5b4820c8ee6f Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >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. > >Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com> >Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> >--- > .../prog/en/modules/tools/batchMod-del.tt | 23 +- > tools/batchMod.pl | 227 ++++++++++-------- > 2 files changed, 152 insertions(+), 98 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 8701d9e2c6..38e946b38a 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 %] > > <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&biblionumber=[% not_deleted_loo.biblionumber | uri %]&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 a7e3188ef0..91ede79b1e 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; >@@ -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 >+ } > } > } > # >-- >2.26.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 8132
:
95936
|
95937
|
95938
|
95939
|
95997
|
96147
|
96148
|
96149
|
96150
|
98694
|
98695
|
98696
|
98697
|
101404
|
101405
|
101406
|
101407
|
103147
|
103148
|
103149
|
103150
|
103166
|
103167
|
103168
|
103169
|
104567
|
104568
|
104569
|
104570
|
104571
|
104682