From 7c7f5998aa1266e92a1583dc9a38948aed867bfc Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 11 Oct 2021 16:26:12 +0200 Subject: [PATCH] Bug 28445: Don't surround the whole batch with a transaction It makes more sense to commit when an item has successfully been modified and so move the transaction inside the loop. It also fixes the progress of the whole job. --- Koha/BackgroundJob/BatchUpdateItem.pm | 36 +++--- Koha/Items.pm | 156 +++++++++++++------------- 2 files changed, 96 insertions(+), 96 deletions(-) diff --git a/Koha/BackgroundJob/BatchUpdateItem.pm b/Koha/BackgroundJob/BatchUpdateItem.pm index 1dbc9226826..8336b16e23e 100644 --- a/Koha/BackgroundJob/BatchUpdateItem.pm +++ b/Koha/BackgroundJob/BatchUpdateItem.pm @@ -111,27 +111,22 @@ sub process { }; try { - my $schema = Koha::Database->new->schema; - $schema->txn_do( - sub { - my ($results) = - Koha::Items->search( { itemnumber => \@record_ids } ) - ->batch_update( - { - regex_mod => $regex_mod, - new_values => $new_values, - exclude_from_local_holds_priority => - $exclude_from_local_holds_priority, - callback => sub { - my ($progress) = @_; - $job->progress($progress)->store; - }, - } - ); - $report->{modified_itemnumbers} = $results->{modified_itemnumbers}; - $report->{modified_fields} = $results->{modified_fields}; + my ($results) = + Koha::Items->search( { itemnumber => \@record_ids } ) + ->batch_update( + { + regex_mod => $regex_mod, + new_values => $new_values, + exclude_from_local_holds_priority => + $exclude_from_local_holds_priority, + callback => sub { + my ($progress) = @_; + $job->progress($progress)->store; + }, } - ); + ); + $report->{modified_itemnumbers} = $results->{modified_itemnumbers}; + $report->{modified_fields} = $results->{modified_fields}; } catch { warn $_; @@ -139,6 +134,7 @@ sub process { if ( $_ =~ /Rollback failed/ ); # Rollback failed }; + $job->discard_changes; my $job_data = decode_json encode_utf8 $job->data; $job_data->{report} = $report; diff --git a/Koha/Items.pm b/Koha/Items.pm index 8d5a01cdc76..7349bc4f387 100644 --- a/Koha/Items.pm +++ b/Koha/Items.pm @@ -20,6 +20,7 @@ package Koha::Items; use Modern::Perl; use Array::Utils qw( array_minus ); use List::MoreUtils qw( uniq ); +use Try::Tiny; use C4::Context; use C4::Biblio qw( GetMarcStructure GetMarcFromKohaField ); @@ -242,98 +243,101 @@ sub batch_update { my (@modified_itemnumbers, $modified_fields); my $i; + my $schema = Koha::Database->new->schema; while ( my $item = $self->next ) { - my $modified_holds_priority = 0; - if ( defined $exclude_from_local_holds_priority ) { - if(!defined $item->exclude_from_local_holds_priority || $item->exclude_from_local_holds_priority != $exclude_from_local_holds_priority) { - $item->exclude_from_local_holds_priority($exclude_from_local_holds_priority)->store; - $modified_holds_priority = 1; + try {$schema->txn_do(sub { + my $modified_holds_priority = 0; + if ( defined $exclude_from_local_holds_priority ) { + if(!defined $item->exclude_from_local_holds_priority || $item->exclude_from_local_holds_priority != $exclude_from_local_holds_priority) { + $item->exclude_from_local_holds_priority($exclude_from_local_holds_priority)->store; + $modified_holds_priority = 1; + } } - } - - my $modified = 0; - my $new_values = {%$new_values}; # Don't modify the original - - my $old_values = $item->unblessed; - if ( $item->more_subfields_xml ) { - $old_values = { - %$old_values, - %{$item->additional_attributes->to_hashref}, - }; - } - for my $attr ( keys %$regex_mod ) { - my $old_value = $old_values->{$attr}; + my $modified = 0; + my $new_values = {%$new_values}; # Don't modify the original - next unless $old_value; + my $old_values = $item->unblessed; + if ( $item->more_subfields_xml ) { + $old_values = { + %$old_values, + %{$item->additional_attributes->to_hashref}, + }; + } - my $value = apply_regex( - { - %{ $regex_mod->{$attr} }, - value => $old_value, - } - ); + for my $attr ( keys %$regex_mod ) { + my $old_value = $old_values->{$attr}; - $new_values->{$attr} = $value; - } + next unless $old_value; - for my $attribute ( keys %$new_values ) { - next if $attribute eq 'more_subfields_xml'; # Already counted before + my $value = apply_regex( + { + %{ $regex_mod->{$attr} }, + value => $old_value, + } + ); - my $old = $old_values->{$attribute}; - my $new = $new_values->{$attribute}; - $modified++ - if ( defined $old xor defined $new ) - || ( defined $old && defined $new && $new ne $old ); - } + $new_values->{$attr} = $value; + } - { # Dealing with more_subfields_xml - - my $frameworkcode = $item->biblio->frameworkcode; - my $tagslib = C4::Biblio::GetMarcStructure( 1, $frameworkcode, { unsafe => 1 }); - my ( $itemtag, $itemsubfield ) = C4::Biblio::GetMarcFromKohaField( "items.itemnumber" ); - - my @more_subfield_tags = map { - ( - ref($_) - && %$_ - && !$_->{kohafield} # Get subfields that are not mapped - ) - ? $_->{tagsubfield} - : () - } values %{ $tagslib->{$itemtag} }; - - my $more_subfields_xml = Koha::Item::Attributes->new( - { - map { - exists $new_values->{$_} ? ( $_ => $new_values->{$_} ) - : exists $old_values->{$_} - ? ( $_ => $old_values->{$_} ) - : () - } @more_subfield_tags - } - )->to_marcxml($frameworkcode); + for my $attribute ( keys %$new_values ) { + next if $attribute eq 'more_subfields_xml'; # Already counted before - $new_values->{more_subfields_xml} = $more_subfields_xml; + my $old = $old_values->{$attribute}; + my $new = $new_values->{$attribute}; + $modified++ + if ( defined $old xor defined $new ) + || ( defined $old && defined $new && $new ne $old ); + } - delete $new_values->{$_} for @more_subfield_tags; # Clean the hash + { # Dealing with more_subfields_xml + + my $frameworkcode = $item->biblio->frameworkcode; + my $tagslib = C4::Biblio::GetMarcStructure( 1, $frameworkcode, { unsafe => 1 }); + my ( $itemtag, $itemsubfield ) = C4::Biblio::GetMarcFromKohaField( "items.itemnumber" ); + + my @more_subfield_tags = map { + ( + ref($_) + && %$_ + && !$_->{kohafield} # Get subfields that are not mapped + ) + ? $_->{tagsubfield} + : () + } values %{ $tagslib->{$itemtag} }; + + my $more_subfields_xml = Koha::Item::Attributes->new( + { + map { + exists $new_values->{$_} ? ( $_ => $new_values->{$_} ) + : exists $old_values->{$_} + ? ( $_ => $old_values->{$_} ) + : () + } @more_subfield_tags + } + )->to_marcxml($frameworkcode); + + $new_values->{more_subfields_xml} = $more_subfields_xml; + + delete $new_values->{$_} for @more_subfield_tags; # Clean the hash - } + } - if ( $modified ) { - my $itemlost_pre = $item->itemlost; - $item->set($new_values)->store({skip_record_index => 1}); + if ( $modified ) { + my $itemlost_pre = $item->itemlost; + $item->set($new_values)->store({skip_record_index => 1}); - LostItem( - $item->itemnumber, 'batchmod', undef, - { skip_record_index => 1 } - ) if $item->itemlost - and not $itemlost_pre; + LostItem( + $item->itemnumber, 'batchmod', undef, + { skip_record_index => 1 } + ) if $item->itemlost + and not $itemlost_pre; - push @modified_itemnumbers, $item->itemnumber if $modified || $modified_holds_priority; - $modified_fields += $modified + $modified_holds_priority; - } + push @modified_itemnumbers, $item->itemnumber if $modified || $modified_holds_priority; + $modified_fields += $modified + $modified_holds_priority; + } + })}; if ( $callback ) { $callback->(++$i); -- 2.25.1