Attachment 109333 Details for Bug 22417 – Bug 22417: Fix the batch authority tool

[patch] Bug 22417: Fix the batch authority tool

Bug-22417-Fix-the-batch-authority-tool.patch (text/plain), 16.49 KB, created by Marcel de Rooy on 2020-08-31 08:26:19 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2020-08-31 08:26:19 UTC

Size: 16.49 KB

patch

obsolete

>From 5658a27c6c8e0f8bb400179f0ddfbbe403ea5d66 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Thu, 20 Feb 2020 16:03:15 +0100
>Subject: [PATCH] Bug 22417: Fix the batch authority tool
>Content-Type: text/plain; charset=utf-8
>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>
>Signed-off-by: David Cook <dcook@prosentient.com.au>
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>---
> 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 ++++++++++++++++++++--
> .../en/modules/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 @@
>                             </div>
>                         [% END %]
>                     [% END %]
>-                [% CASE %][% job.type | html %]
>+                [% CASE 'batch_authority_record_modification' %]
>+                    [% SET report = job.report %]
>+                    [% IF report %]
>+                        [% IF report.total_records == report.total_success %]
>+                            <div class="dialog message">
>+                                All records have successfully been modified! <a href="/cgi-bin/koha/tools/batch_record_modification.pl" title="New batch record modification">New batch record modification</a>
>+                            </div>
>+                        [% ELSE %]
>+                            <div class="dialog message">
>+                                [% 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 %]
>+                                <a href="/cgi-bin/koha/tools/batch_record_modification.pl" title="New batch record modification">New batch record modification</a>
>+                            </div>
>+                        [% END %]
>+                    [% END %]
>+                [% CASE %]Job type "[% job.type | html %]" not handled in the template
>                 [% END %]
>             </li>
>             <li><label for="job_data">Detailed messages: </label>
>@@ -77,7 +92,7 @@
>                             [% END %]
>                             [% SWITCH m.code %]
>                             [% CASE 'biblio_not_modified' %]
>-                                Bibliographic record <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% m.biblionumber | uri %]">[% m.biblionumber | html %]</a> has not been modified. An error occurred on modifying it.
>+                                Bibliographic record <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% m.biblionumber | uri %]">[% m.biblionumber | html %]</a> has not been modified. An error occurred on modifying it.[% IF m.error %] ([% m.error %])[% END %].
>                             [% CASE 'biblio_modified' %]
>                                 Bibliographic record <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% m.biblionumber | uri %]">[% m.biblionumber | html %]</a> has successfully been modified.
>                                 <h3>Next steps</h3>
>@@ -98,7 +113,25 @@
>                             [% END %]
>                         </div>
>                     [% END %]
>-                [% CASE %][% job.type | html %]
>+                [% CASE 'batch_authority_record_modification' %]
>+                    [% FOR m IN job.messages %]
>+                        <div class="dialog message">
>+                            [% IF m.type == 'success' %]
>+                                <i class="fa fa-check success"></i>
>+                            [% ELSIF m.type == 'warning' %]
>+                                <i class="fa fa-warning warn"></i>
>+                            [% ELSIF m.type == 'error' %]
>+                                <i class="fa fa-exclamation error"></i>
>+                            [% END %]
>+                            [% SWITCH m.code %]
>+                            [% CASE 'authority_not_modified' %]
>+                                Authority record <a href="/cgi-bin/koha/authorities/detail.pl?authid=[% message.authid | uri %]">[% message.authid | html %]</a> has not been modified. An error occurred on modifying it[% IF m.error %] ([% m.error %])[% END %].
>+                            [% CASE 'authority_modified' %]
>+                                Authority record <a href="/cgi-bin/koha/authorities/detail.pl?authid=[% message.authid | uri %]">[% message.authid | html %]</a> has successfully been modified..
>+                            [% END %]
>+                        </div>
>+                    [% END %]
>+                [% CASE %]Job type "[% job.type | html %]" not handled in the template
>                 [% END %]
>             </li>
>         </ol>
>@@ -143,6 +176,7 @@
>                     <td>
>                         [% 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 %]
>                     </td>
>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 1c09747848..973ec4185e 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 <a href="/cgi-bin/koha/authorities/detail.pl?authid=[% message.authid | uri %]">[% message.authid | html %]</a> has not been modified. An error occurred on modifying it.
>-                                [% ELSIF message.code == 'authority_modified' %]
>-                                    Bibliographic record <a href="/cgi-bin/koha/authorities/detail.pl?authid=[% message.authid | uri %]">[% message.authid | html %]</a> has successfully been modified.
>                                 [% ELSIF message.code == 'cannot_enqueue_job' %]
>                                     Cannot enqueue this job.
>                                 [% END %]
>@@ -249,9 +245,9 @@
>                         [% END %]
>                      [% ELSIF view == 'enqueued' %]
>                         <div class="dialog message">
>-                            The job has been enqueued! It will be processed as soon as possible (FIXME - well, it could depend on a config?)
>-                            <a href="/cgi-bin/koha/admin/background_jobs.pl?op=view&id=[% job_id | uri %]" title="View detail of the enqueued job">View detail of the enqueued job</a>
>-                            | <a href="/cgi-bin/koha/tools/batch_record_modification.pl" title="New batch record modification">New batch record modification</a>
>+                            <p>The job has been enqueued! It will be processed as soon as possible (FIXME - well, it could depend on a config?)</p>
>+                            <p><a href="/cgi-bin/koha/admin/background_jobs.pl?op=view&id=[% job_id | uri %]" title="View detail of the enqueued job">View detail of the enqueued job</a>
>+                            | <a href="/cgi-bin/koha/tools/batch_record_modification.pl" title="New batch record modification">New batch record modification</a></p>
>                         </div>
>                     [% ELSE %]
>                         <div class="dialog alert">
>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.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 22417: 105542 | 105543 | 107385 | 107386 | 107387 | 107388 | 107389 | 107390 | 107391 | 107392 | 107393 | 107394 | 107395 | 107396 | 107397 | 107398 | 107399 | 107400 | 107401 | 107402 | 107403 | 107404 | 107405 | 107406 | 107481 | 107482 | 107682 | 108023 | 108024 | 108025 | 108026 | 108027 | 108028 | 108029 | 108030 | 108031 | 108032 | 108033 | 108034 | 108035 | 108036 | 108037 | 108038 | 108039 | 108040 | 108041 | 108042 | 108043 | 108044 | 108045 | 108046 | 108047 | 109270 | 109271 | 109319 | 109320 | 109321 | 109322 | 109323 | 109324 | 109325 | 109326 | 109327 | 109328 | 109329 | 109330 | 109331 | 109332 | 109333 | 109334 | 109335 | 109336 | 109337 | 109338 | 109339 | 109340 | 109341 | 109342 | 109343 | 109344 | 109345 | 109350 | 109351 | 109355 | 109390 | 109391 | 109392 | 109393 | 109394 | 109395 | 109396 | 109397 | 109398 | 109399 | 109400 | 109401 | 109402 | 109403 | 109404 | 109405 | 109406 | 109407 | 109408 | 109409 | 109410 | 109411 | 109412 | 109413 | 109414 | 109415 | 109416 | 109417 | 109418 | 109474 | 109475 | 109476 | 109477 | 110150 | 111091 | 111092 | 111093 | 111094 | 111095 | 111096 | 111097 | 111098 | 111099 | 111100 | 111101 | 111102 | 111103 | 111104 | 111105 | 111106 | 111107 | 111108 | 111109 | 111110 | 111111 | 111112 | 111113 | 111114 | 111115 | 111116 | 111117 | 111118 | 111119 | 111120 | 111121 | 111238 | 111239 | 111245