From c93af6e76213ce9f0ae3eecf70dc809c06bfedbb Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 29 Jul 2020 11:17:34 +0200 Subject: [PATCH] Bug 26080: Use the task queue for batch delete biblios This patch takes advantage of the task queue to delegate the batch delete biblios tool. Test plan: Delete bibliographic records using the batch record deletion tool Confirm that the job is now delegated to the task queue and that everything else is working as before --- Koha/BackgroundJob.pm | 3 + Koha/BackgroundJob/BatchDeleteBiblio.pm | 158 ++++++++++++++++++ .../prog/en/modules/admin/background_jobs.tt | 44 +++++ .../en/modules/tools/batch_delete_records.tt | 18 +- misc/background_jobs_worker.pl | 6 +- tools/batch_delete_records.pl | 137 +++------------ 6 files changed, 241 insertions(+), 125 deletions(-) create mode 100644 Koha/BackgroundJob/BatchDeleteBiblio.pm diff --git a/Koha/BackgroundJob.pm b/Koha/BackgroundJob.pm index 09139bdbdd0..47c2cf8aba5 100644 --- a/Koha/BackgroundJob.pm +++ b/Koha/BackgroundJob.pm @@ -26,6 +26,7 @@ use Koha::DateUtils qw( dt_from_string ); use Koha::Exceptions; use Koha::BackgroundJob::BatchUpdateBiblio; use Koha::BackgroundJob::BatchUpdateAuthority; +use Koha::BackgroundJob::BatchDeleteBiblio; use base qw( Koha::Object ); @@ -155,6 +156,8 @@ sub process { ? Koha::BackgroundJob::BatchUpdateBiblio->process($args) : $job_type eq 'batch_authority_record_modification' ? Koha::BackgroundJob::BatchUpdateAuthority->process($args) + : $job_type eq 'batch_biblio_record_deletion' + ? Koha::BackgroundJob::BatchDeleteBiblio->process($args) : Koha::Exceptions::Exception->throw('->process called without valid job_type'); } diff --git a/Koha/BackgroundJob/BatchDeleteBiblio.pm b/Koha/BackgroundJob/BatchDeleteBiblio.pm new file mode 100644 index 00000000000..64984bfe8e4 --- /dev/null +++ b/Koha/BackgroundJob/BatchDeleteBiblio.pm @@ -0,0 +1,158 @@ +package Koha::BackgroundJob::BatchDeleteBiblio; + +use Modern::Perl; +use JSON qw( encode_json decode_json ); + +use Koha::BackgroundJobs; +use Koha::DateUtils qw( dt_from_string ); +use C4::Biblio; + +use base 'Koha::BackgroundJob'; + +sub job_type { + return 'batch_biblio_record_deletion'; +} + +sub process { + my ( $self, $args ) = @_; + + my $job_type = $args->{job_type}; + + my $job = Koha::BackgroundJobs->find( $args->{job_id} ); + + if ( !exists $args->{job_id} || !$job || $job->status eq 'cancelled' ) { + return; + } + + # FIXME If the job has already been started, but started again (worker has been restart for instance) + # Then we will start from scratch and so double delete the same records + + my $job_progress = 0; + $job->started_on(dt_from_string) + ->progress($job_progress) + ->status('started') + ->store; + + my $mmtid = $args->{mmtid}; + my @record_ids = @{ $args->{record_ids} }; + + my $report = { + total_records => scalar @record_ids, + total_success => 0, + }; + my @messages; + my $schema = Koha::Database->new->schema; + RECORD_IDS: for my $record_id ( sort { $a <=> $b } @record_ids ) { + + last if $job->get_from_storage->status eq 'cancelled'; + + next unless $record_id; + + $schema->storage->txn_begin; + + my $biblionumber = $record_id; + # First, checking if issues exist. + # If yes, nothing to do + my $biblio = Koha::Biblios->find( $biblionumber ); + + # TODO Replace with $biblio->get_issues->count + if ( C4::Biblio::CountItemsIssued( $biblionumber ) ) { + push @messages, { + type => 'warning', + code => 'item_issued', + biblionumber => $biblionumber, + }; + $schema->storage->txn_rollback; + $job->progress( ++$job_progress )->store; + next; + } + + # Cancel reserves + my $holds = $biblio->holds; + while ( my $hold = $holds->next ) { + eval{ + $hold->cancel; + }; + if ( $@ ) { + push @messages, { + type => 'error', + code => 'reserve_not_cancelled', + biblionumber => $biblionumber, + reserve_id => $hold->reserve_id, + error => $@, + }; + $schema->storage->txn_rollback; + $job->progress( ++$job_progress )->store; + next RECORD_IDS; + } + } + + # Delete items + my $items = Koha::Items->search({ biblionumber => $biblionumber }); + while ( my $item = $items->next ) { + my $error = $item->safe_delete; + if(ref($error) ne 'Koha::Item'){ + push @messages, { + type => 'error', + code => 'item_not_deleted', + biblionumber => $biblionumber, + itemnumber => $item->itemnumber, + error => $error, + }; + $schema->storage->txn_rollback; + $job->progress( ++$job_progress )->store; + next RECORD_IDS; + } + } + + # Finally, delete the biblio + my $error = eval { + C4::Biblio::DelBiblio( $biblionumber ); + }; + if ( $error or $@ ) { + push @messages, { + type => 'error', + code => 'biblio_not_deleted', + biblionumber => $biblionumber, + error => ($@ ? $@ : $error), + }; + $schema->storage->txn_rollback; + $job->progress( ++$job_progress )->store; + next; + } + + push @messages, { + type => 'success', + code => 'biblio_deleted', + biblionumber => $biblionumber, + }; + $report->{total_success}++; + $schema->storage->txn_commit; + $job->progress( ++$job_progress )->store; + } + + my $job_data = decode_json $job->data; + $job_data->{messages} = \@messages; + $job_data->{report} = $report; + + $job->ended_on(dt_from_string) + ->data(encode_json $job_data); + $job->status('finished') if $job->status ne 'cancelled'; + $job->store; +} + +sub enqueue { + my ( $self, $args) = @_; + + # TODO Raise exception instead + return unless exists $args->{record_ids}; + + my @record_ids = @{ $args->{record_ids} }; + + $self->SUPER::enqueue({ + job_size => scalar @record_ids, + job_args => {record_ids => \@record_ids,} + }); +} + +1; diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/background_jobs.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/background_jobs.tt index 64f2278c20c..c34d759c991 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/background_jobs.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/background_jobs.tt @@ -108,6 +108,23 @@ Administration › Koha [% END %] [% END %] + [% CASE 'batch_biblio_record_deletion' %] + [% SET report = job.report %] + [% IF report %] + [% IF report.total_records == report.total_success %] +
+ All records have been deleted successfully! +
+ [% ELSIF report.total_success == 0 %] +
+ No record has been deleted. An error occurred. +
+ [% ELSE %] +
+ [% report.total_success | html %] / [% report.total_records | html %] records have been deleted successfully but some errors occurred. +
+ [% END %] + [% END %] [% CASE %]Job type "[% job.type | html %]" not handled in the template [% END %] @@ -149,6 +166,33 @@ Administration › Koha [% END %] [% END %] + [% CASE 'batch_biblio_record_deletion' %] + [% FOR m IN job.messages %] +
+ [% IF m.type == 'success' %] + + [% ELSIF m.type == 'warning' %] + + [% ELSIF m.type == 'error' %] + + [% END %] + [% SWITCH m.code %] + [% CASE 'biblio_not_exists' %] + The biblionumber [% m.biblionumber | html %] does not exist in the database. + [% CASE 'item_issued' %] + At least one item is checked out on bibliographic record [% m.biblionumber | html %]. + [% CASE 'reserve_not_cancelled' %] + Bibliographic record [% m.biblionumber | html %] was not deleted. A hold could not be canceled (reserve_id [% m.reserve_id | html %]). + [% CASE 'item_not_deleted' %] + The bibliographic record [% m.biblionumber | html %] was not deleted. An error was encountered when deleting an item (itemnumber [% m.itemnumber | html %]). + [% CASE 'biblio_not_deleted' %] + Bibliographic record [% m.biblionumber | html %] was not deleted. An error occurred. + [% CASE 'biblio_deleted' %] + Bibliographic record [% m.biblionumber | html %] has been deleted successfully. + [% END %] +
+ [% END %] + [% CASE %]Job type "[% job.type | html %]" not handled in the template [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_delete_records.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_delete_records.tt index 14341364b69..0185e30e679 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_delete_records.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_delete_records.tt @@ -43,20 +43,12 @@ The biblionumber [% message.biblionumber | html %] does not exist in the database. [% ELSIF message.code == 'authority_not_exists' %] The authority id [% message.authid | html %] does not exist in the database. - [% ELSIF message.code == 'item_issued' %] - At least one item is checked out on bibliographic record [% message.biblionumber | html %]. - [% ELSIF message.code == 'reserve_not_cancelled' %] - Bibliographic record [% message.biblionumber | html %] was not deleted. A hold could not be canceled (reserve_id [% message.reserve_id | html %]). - [% ELSIF message.code == 'item_not_deleted' %] - The bibliographic record [% message.biblionumber | html %] was not deleted. An error was encountered when deleting an item (itemnumber [% message.itemnumber | html %]). - [% ELSIF message.code == 'biblio_not_deleted' %] - Bibliographic record [% message.biblionumber | html %] was not deleted. An error occurred. [% ELSIF message.code == 'authority_not_deleted' %] Authority record [% message.authid | html %] was not deleted. An error occurred. - [% ELSIF message.code == 'biblio_deleted' %] - Bibliographic record [% message.biblionumber | html %] has been deleted successfully. [% ELSIF message.code == 'authority_deleted' %] Authority [% message.authid | html %] has been deleted successfully. + [% ELSIF message.code == 'cannot_enqueue_job' %] + Cannot enqueue this job. [% END %] [% IF message.error %] (The error was: [% message.error | html %], see the Koha log file for more information). @@ -215,6 +207,12 @@ [% report.total_success | html %] / [% report.total_records | html %] records have been deleted successfully but some errors occurred. [% END %]

New batch record deletion

+ [% ELSIF op == 'enqueued' %] +
+

The job has been enqueued! It will be processed as soon as possible.

+

View detail of the enqueued job + | New batch record deletion

+
[% ELSE %] No action defined for the template. [% END %] diff --git a/misc/background_jobs_worker.pl b/misc/background_jobs_worker.pl index 5d409cdf1b7..e1dbff40b18 100755 --- a/misc/background_jobs_worker.pl +++ b/misc/background_jobs_worker.pl @@ -28,7 +28,11 @@ try { warn sprintf "Cannot connect to the message broker, the jobs will be processed anyway (%s)", $_; }; -my @job_types = qw( batch_biblio_record_modification batch_authority_record_modification ); +my @job_types = qw( + batch_biblio_record_modification + batch_authority_record_modification + batch_biblio_record_deletion +); if ( $conn ) { # FIXME cf note in Koha::BackgroundJob about $namespace diff --git a/tools/batch_delete_records.pl b/tools/batch_delete_records.pl index ea87d8b6c85..59ab4e6bcac 100755 --- a/tools/batch_delete_records.pl +++ b/tools/batch_delete_records.pl @@ -22,6 +22,7 @@ use Modern::Perl; use CGI; use List::MoreUtils qw( uniq ); +use Try::Tiny; use C4::Auth; use C4::Output; @@ -32,6 +33,7 @@ use Koha::Virtualshelves; use Koha::Authorities; use Koha::Biblios; use Koha::Items; +use Koha::BackgroundJob::BatchDeleteBiblio; my $input = CGI->new; my $op = $input->param('op') // q|form|; @@ -124,122 +126,29 @@ if ( $op eq 'form' ) { } elsif ( $op eq 'delete' ) { # We want to delete selected records! my @record_ids = $input->multi_param('record_id'); - my $schema = Koha::Database->new->schema; - my $error; - my $report = { - total_records => 0, - total_success => 0, + try { + my $params = { + record_ids => \@record_ids, + }; + + my $job_id = + $recordtype eq 'biblio' + ? Koha::BackgroundJob::BatchDeleteBiblio->new->enqueue($params) + : Koha::BackgroundJob::BatchDeleteAuthority->new->enqueue($params); + + $template->param( + op => 'enqueued', + job_id => $job_id, + ); + } catch { + push @messages, { + type => 'error', + code => 'cannot_enqueue_job', + error => $_, + }; + $template->param( view => 'errors' ); }; - RECORD_IDS: for my $record_id ( sort { $a <=> $b } @record_ids ) { - $report->{total_records}++; - next unless $record_id; - $schema->storage->txn_begin; - - if ( $recordtype eq 'biblio' ) { - # Biblios - my $biblionumber = $record_id; - # First, checking if issues exist. - # If yes, nothing to do - my $biblio = Koha::Biblios->find( $biblionumber ); - - # TODO Replace with $biblio->get_issues->count - if ( C4::Biblio::CountItemsIssued( $biblionumber ) ) { - push @messages, { - type => 'warning', - code => 'item_issued', - biblionumber => $biblionumber, - }; - $schema->storage->txn_rollback; - next; - } - - # Cancel reserves - my $holds = $biblio->holds; - while ( my $hold = $holds->next ) { - eval{ - $hold->cancel; - }; - if ( $@ ) { - push @messages, { - type => 'error', - code => 'reserve_not_cancelled', - biblionumber => $biblionumber, - reserve_id => $hold->reserve_id, - error => $@, - }; - $schema->storage->txn_rollback; - next RECORD_IDS; - } - } - - # Delete items - my $items = Koha::Items->search({ biblionumber => $biblionumber }); - while ( my $item = $items->next ) { - my $deleted_item = eval { $item->safe_delete }; - if ( !ref($deleted_item) or $@ ) { - push @messages, { - type => 'error', - code => 'item_not_deleted', - biblionumber => $biblionumber, - itemnumber => $item->itemnumber, - error => ($@ ? $@ : $error), - }; - $schema->storage->txn_rollback; - next RECORD_IDS; - } - } - - # Finally, delete the biblio - my $error = eval { - C4::Biblio::DelBiblio( $biblionumber ); - }; - if ( $error or $@ ) { - push @messages, { - type => 'error', - code => 'biblio_not_deleted', - biblionumber => $biblionumber, - error => ($@ ? $@ : $error), - }; - $schema->storage->txn_rollback; - next; - } - - push @messages, { - type => 'success', - code => 'biblio_deleted', - biblionumber => $biblionumber, - }; - $report->{total_success}++; - $schema->storage->txn_commit; - } else { - # Authorities - my $authid = $record_id; - eval { C4::AuthoritiesMarc::DelAuthority({ authid => $authid }) }; - if ( $@ ) { - push @messages, { - type => 'error', - code => 'authority_not_deleted', - authid => $authid, - error => ($@ ? $@ : 0), - }; - $schema->storage->txn_rollback; - next; - } else { - push @messages, { - type => 'success', - code => 'authority_deleted', - authid => $authid, - }; - $report->{total_success}++; - $schema->storage->txn_commit; - } - } - } - $template->param( - op => 'report', - report => $report, - ); } $template->param( -- 2.20.1