From ff3bfd04d38ee25f487fd38428606aeaeac67aa7 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 20 Feb 2020 16:03:15 +0100 Subject: [PATCH] Bug 22417: Fix the batch authority tool Signed-off-by: Tomas Cohen Arazi --- Koha/BackgroundJob.pm | 21 ++++++++-- Koha/BackgroundJob/BatchUpdateAuthority.pm | 32 ++++++++------- Koha/BackgroundJob/BatchUpdateBiblio.pm | 13 +++++- Koha/BackgroundJobs.pm | 1 + .../prog/en/modules/admin/background_jobs.tt | 40 +++++++++++++++++-- .../tools/batch_record_modification.tt | 10 ++--- misc/background_jobs_worker.pl | 12 +++--- tools/batch_record_modification.pl | 19 +++++---- 8 files changed, 106 insertions(+), 42 deletions(-) diff --git a/Koha/BackgroundJob.pm b/Koha/BackgroundJob.pm index caf4d300f5..b905f09b8a 100644 --- a/Koha/BackgroundJob.pm +++ b/Koha/BackgroundJob.pm @@ -7,7 +7,9 @@ use Net::Stomp; use C4::Context; use Koha::DateUtils qw( dt_from_string ); -use Koha::BackgroundJobs; +use Koha::Exceptions; +use Koha::BackgroundJob::BatchUpdateBiblio; +use Koha::BackgroundJob::BatchUpdateAuthority; use Scalar::Util qw( weaken ); @@ -23,7 +25,7 @@ sub connect { sub enqueue { my ( $self, $params ) = @_; - my $job_type = $params->{job_type}; + my $job_type = $self->job_type; my $job_size = $params->{job_size}; my $job_args = $params->{job_args}; @@ -49,14 +51,25 @@ sub enqueue { my $conn = $self->connect; $conn->send_with_receipt( { destination => $job_type, body => $json_args } ) - or Koha::Exception->throw('Job has not been enqueued'); + or Koha::Exceptions::Exception->throw('Job has not been enqueued'); } ); return $job_id; } -sub process { croak "This method must be subclassed" } +sub process { + my ( $self, $args ) = @_; + + my $job_type = $self->type; + return $job_type eq 'batch_biblio_record_modification' + ? Koha::BackgroundJob::BatchUpdateBiblio->process($args) + : $job_type eq 'batch_authority_record_modification' + ? Koha::BackgroundJob::BatchUpdateAuthority->process($args) + : Koha::Exceptions::Exception->throw('->process called without valid job_type'); +} + +sub job_type { croak "This method must be subclassed" } sub messages { my ( $self ) = @_; diff --git a/Koha/BackgroundJob/BatchUpdateAuthority.pm b/Koha/BackgroundJob/BatchUpdateAuthority.pm index 682501a261..ed634d251d 100644 --- a/Koha/BackgroundJob/BatchUpdateAuthority.pm +++ b/Koha/BackgroundJob/BatchUpdateAuthority.pm @@ -1,22 +1,29 @@ package Koha::BackgroundJob::BatchUpdateAuthority; use Modern::Perl; +use JSON qw( encode_json decode_json ); + +use C4::MarcModificationTemplates; +use C4::AuthoritiesMarc; use Koha::BackgroundJobs; use Koha::DateUtils qw( dt_from_string ); -use JSON qw( encode_json decode_json ); +use Koha::MetadataRecord::Authority; use base 'Koha::BackgroundJob'; -our $channel; -sub process { - my ( $self, $args, $channel ) = @_; - - my $job_type = $args->{job_type}; +sub job_type { + return 'batch_authority_record_modification'; +} - return unless exists $args->{job_id}; +sub process { + my ( $self, $args ) = @_; my $job = Koha::BackgroundJobs->find( $args->{job_id} ); + if ( !exists $args->{job_id} || !$job || $job->status eq 'cancelled' ) { + return; + } + my $job_progress = 0; $job->started_on(dt_from_string) ->progress($job_progress) @@ -28,14 +35,13 @@ sub process { my @record_ids = @{ $args->{record_ids} }; my $report = { - total_records => 0, + total_records => scalar @record_ids, total_success => 0, }; my @messages; my $dbh = C4::Context->dbh; $dbh->{RaiseError} = 1; RECORD_IDS: for my $record_id ( sort { $a <=> $b } @record_ids ) { - $report->{total_records}++; next unless $record_id; # Authorities my $authid = $record_id; @@ -68,11 +74,10 @@ sub process { $job_data->{report} = $report; $job->ended_on(dt_from_string) - ->status('finished') - ->data(encode_json $job_data) - ->store; + ->data(encode_json $job_data); + $job->status('finished') if $job->status ne 'cancelled'; + $job->store; - $channel->ack(); # FIXME Is that ok even on failure? } sub enqueue { @@ -88,7 +93,6 @@ sub enqueue { my @record_ids = @{ $args->{record_ids} }; $self->SUPER::enqueue({ - job_type => 'batch_record_modification', job_size => scalar @record_ids, job_args => {mmtid => $mmtid, record_type => $record_type, record_ids => \@record_ids,} }); diff --git a/Koha/BackgroundJob/BatchUpdateBiblio.pm b/Koha/BackgroundJob/BatchUpdateBiblio.pm index 09c53fa185..16f2e218e9 100644 --- a/Koha/BackgroundJob/BatchUpdateBiblio.pm +++ b/Koha/BackgroundJob/BatchUpdateBiblio.pm @@ -1,12 +1,19 @@ package Koha::BackgroundJob::BatchUpdateBiblio; use Modern::Perl; +use JSON qw( encode_json decode_json ); + use Koha::BackgroundJobs; use Koha::DateUtils qw( dt_from_string ); -use JSON qw( encode_json decode_json ); +use C4::Biblio; +use C4::MarcModificationTemplates; use base 'Koha::BackgroundJob'; +sub job_type { + return 'batch_biblio_record_modification'; +} + sub process { my ( $self, $args ) = @_; @@ -18,6 +25,9 @@ sub process { 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 process the same records + my $job_progress = 0; $job->started_on(dt_from_string) ->progress($job_progress) @@ -89,7 +99,6 @@ sub enqueue { my @record_ids = @{ $args->{record_ids} }; $self->SUPER::enqueue({ - job_type => 'batch_biblio_record_modification', # FIXME Must be a global const job_size => scalar @record_ids, job_args => {mmtid => $mmtid, record_type => $record_type, record_ids => \@record_ids,} }); diff --git a/Koha/BackgroundJobs.pm b/Koha/BackgroundJobs.pm index 6e44990c9e..ebf5d7e41d 100644 --- a/Koha/BackgroundJobs.pm +++ b/Koha/BackgroundJobs.pm @@ -2,6 +2,7 @@ package Koha::BackgroundJobs; use Modern::Perl; use base qw(Koha::Objects); +use Koha::BackgroundJob; sub _type { return 'BackgroundJob'; 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 2b04c0a844..8be94aa65b 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 @@ -60,7 +60,22 @@ [% END %] [% END %] - [% CASE %][% job.type | html %] + [% CASE 'batch_authority_record_modification' %] + [% SET report = job.report %] + [% IF report %] + [% IF report.total_records == report.total_success %] +
+ All records have successfully been modified! New batch record modification +
+ [% ELSE %] +
+ [% report.total_success | html %] / [% report.total_records | html %] records have successfully been modified. Some errors occurred. + [% IF job.status == 'cancelled' %]The job has been cancelled before it finished.[% END %] + New batch record modification +
+ [% END %] + [% END %] + [% CASE %]Job type "[% job.type | html %]" not handled in the template [% END %]
  • @@ -77,7 +92,7 @@ [% END %] [% SWITCH m.code %] [% CASE 'biblio_not_modified' %] - Bibliographic record [% m.biblionumber | html %] has not been modified. An error occurred on modifying it. + Bibliographic record [% m.biblionumber | html %] has not been modified. An error occurred on modifying it.[% IF m.error %] ([% m.error %])[% END %]. [% CASE 'biblio_modified' %] Bibliographic record [% m.biblionumber | html %] has successfully been modified.

    Next steps

    @@ -98,7 +113,25 @@ [% END %] [% END %] - [% CASE %][% job.type | html %] + [% CASE 'batch_authority_record_modification' %] + [% FOR m IN job.messages %] +
    + [% IF m.type == 'success' %] + + [% ELSIF m.type == 'warning' %] + + [% ELSIF m.type == 'error' %] + + [% END %] + [% SWITCH m.code %] + [% CASE 'authority_not_modified' %] + Authority record [% message.authid | html %] has not been modified. An error occurred on modifying it[% IF m.error %] ([% m.error %])[% END %]. + [% CASE 'authority_modified' %] + Authority record [% message.authid | html %] has successfully been modified.. + [% END %] +
    + [% END %] + [% CASE %]Job type "[% job.type | html %]" not handled in the template [% END %]
    • @@ -143,6 +176,7 @@ [% SWITCH job.type %] [% CASE 'batch_biblio_record_modification' %]Batch bibliographic record modification + [% CASE 'batch_authority_record_modification' %]Batch authority record modification [% CASE %][% job.type | html %] [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_record_modification.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_record_modification.tt index 499f7cc380..47aad031af 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_record_modification.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_record_modification.tt @@ -42,10 +42,6 @@ Bibliographic record [% message.biblionumber | html %] does not exist in the database. [% ELSIF message.code == 'authority_not_exists' %] Authority record [% message.authid | html %] does not exist in the database. - [% ELSIF message.code == 'authority_not_modified' %] - Authority record [% message.authid | html %] has not been modified. An error occurred on modifying it. - [% ELSIF message.code == 'authority_modified' %] - Bibliographic record [% message.authid | html %] has successfully been modified. [% ELSIF message.code == 'cannot_enqueue_job' %] Cannot enqueue this job. [% END %] @@ -249,9 +245,9 @@ [% END %] [% ELSIF view == 'enqueued' %]
    - The job has been enqueued! It will be processed as soon as possible (FIXME - well, it could depend on a config?) - View detail of the enqueued job - | New batch record modification +

    The job has been enqueued! It will be processed as soon as possible (FIXME - well, it could depend on a config?)

    +

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

    [% ELSE %]
    diff --git a/misc/background_jobs_worker.pl b/misc/background_jobs_worker.pl index 6bc7335453..8ce187a6b0 100755 --- a/misc/background_jobs_worker.pl +++ b/misc/background_jobs_worker.pl @@ -18,14 +18,15 @@ use Modern::Perl; use JSON qw( encode_json decode_json ); -use Koha::BackgroundJob::BatchUpdateBiblio; -use Koha::BackgroundJob; +use Koha::BackgroundJobs; my $conn = Koha::BackgroundJob->connect; -my $job_type = 'batch_biblio_record_modification'; +my @job_types = qw( batch_biblio_record_modification batch_authority_record_modification ); -$conn->subscribe({ destination => $job_type, ack => 'client' }); +for my $job_type ( @job_types ) { + $conn->subscribe({ destination => $job_type, ack => 'client' }); +} while (1) { my $frame = $conn->receive_frame; if ( !defined $frame ) { @@ -36,7 +37,8 @@ while (1) { my $body = $frame->body; my $args = decode_json($body); - my $success = Koha::BackgroundJob::BatchUpdateBiblio->process( $args ); + my $job = Koha::BackgroundJobs->find($args->{job_id}); + my $success = $job->process( $args ); $conn->ack( { frame => $frame } ); # FIXME depending on $success? } diff --git a/tools/batch_record_modification.pl b/tools/batch_record_modification.pl index 5b645993ec..f4794c58e7 100755 --- a/tools/batch_record_modification.pl +++ b/tools/batch_record_modification.pl @@ -33,6 +33,7 @@ use C4::MarcModificationTemplates qw( GetModificationTemplateActions GetModifica use Koha::Biblios; use Koha::BackgroundJob::BatchUpdateBiblio; +use Koha::BackgroundJob::BatchUpdateAuthority; use Koha::MetadataRecord::Authority; use Koha::Virtualshelves; @@ -152,13 +153,17 @@ if ( $op eq 'form' ) { my @record_ids = $input->multi_param('record_id'); try { - my $job_id = Koha::BackgroundJob::BatchUpdateBiblio->new->enqueue( - { - mmtid => $mmtid, - record_type => $recordtype, - record_ids => \@record_ids, - } - ); + my $params = { + mmtid => $mmtid, + record_type => $recordtype, + record_ids => \@record_ids, + }; + + my $job_id = + $recordtype eq 'biblio' + ? Koha::BackgroundJob::BatchUpdateBiblio->new->enqueue($params) + : Koha::BackgroundJob::BatchUpdateAuthority->new->enqueue($params); + $template->param( view => 'enqueued', job_id => $job_id, -- 2.20.1