Bug 30181: (follow-up) Remove redundant queries and parameters

Now $self is actually an instance of the job class, there's no need to
have the job_id parameter passed, or the have the ->process method
re-fetch the object from the database.

This patch cleans things up.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

diff --git a/Koha/BackgroundJob.pm b/Koha/BackgroundJob.pm
index 2a53702..b2d6129 100644 (file)
--- a/Koha/BackgroundJob.pm
+++ b/Koha/BackgroundJob.pm
@@ -155,7 +155,7 @@ sub process {
 
     $args ||= {};
 
-    return $derived_class->process({job_id => $self->id, %$args});
+    return $derived_class->process( $args );
 }
 
 =head3 job_type
@@ -210,7 +210,7 @@ sub additional_report {
 
     my $derived_class = $self->_derived_class;
 
-    return $derived_class->additional_report({job_id => $self->id});
+    return $derived_class->additional_report;
 }
 
 =head3 cancel

diff --git a/Koha/BackgroundJob/BatchCancelHold.pm b/Koha/BackgroundJob/BatchCancelHold.pm
index 5216119..ba85cfc 100644 (file)
--- a/Koha/BackgroundJob/BatchCancelHold.pm
+++ b/Koha/BackgroundJob/BatchCancelHold.pm
@@ -18,7 +18,6 @@ package Koha::BackgroundJob::BatchCancelHold;
 use Modern::Perl;
 use JSON qw( encode_json decode_json );
 
-use Koha::BackgroundJobs;
 use Koha::DateUtils qw( dt_from_string );
 use Koha::Holds;
 use Koha::Patrons;
@@ -55,14 +54,12 @@ Process the modification.
 sub process {
     my ( $self, $args ) = @_;
 
-    my $job = Koha::BackgroundJobs->find( $args->{job_id} );
-
-    if ( !exists $args->{job_id} || !$job || $job->status eq 'cancelled' ) {
+    if ( $self->status eq 'cancelled' ) {
         return;
     }
 
     my $job_progress = 0;
-    $job->started_on(dt_from_string)->progress($job_progress)
+    $self->started_on(dt_from_string)->progress($job_progress)
       ->status('started')->store;
 
     my @hold_ids = @{ $args->{hold_ids} };
@@ -109,16 +106,16 @@ sub process {
               };
             $report->{total_success}++;
         }
-        $job->progress( ++$job_progress )->store;
+        $self->progress( ++$job_progress )->store;
     }
 
-    my $job_data = decode_json $job->data;
+    my $job_data = decode_json $self->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;
+    $self->ended_on(dt_from_string)->data( encode_json $job_data);
+    $self->status('finished') if $self->status ne 'cancelled';
+    $self->store;
 
 }
 
@@ -153,8 +150,7 @@ Pass the biblio's title and patron's name
 sub additional_report {
     my ( $self, $args ) = @_;
 
-    my $job = Koha::BackgroundJobs->find( $args->{job_id} );
-    my $messages = $job->messages;
+    my $messages = $self->messages;
     for my $m ( @$messages ) {
         $m->{patron} = Koha::Patrons->find($m->{patron_id});
         $m->{biblio} = Koha::Biblios->find($m->{biblio_id});

diff --git a/Koha/BackgroundJob/BatchDeleteAuthority.pm b/Koha/BackgroundJob/BatchDeleteAuthority.pm
index 99833ab..0f21519 100644 (file)
--- a/Koha/BackgroundJob/BatchDeleteAuthority.pm
+++ b/Koha/BackgroundJob/BatchDeleteAuthority.pm
@@ -3,7 +3,6 @@ package Koha::BackgroundJob::BatchDeleteAuthority;
 use Modern::Perl;
 use JSON qw( encode_json decode_json );
 
-use Koha::BackgroundJobs;
 use Koha::DateUtils qw( dt_from_string );
 use C4::AuthoritiesMarc;
 
@@ -16,11 +15,7 @@ sub job_type {
 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' ) {
+    if ( $self->status eq 'cancelled' ) {
         return;
     }
 
@@ -28,7 +23,7 @@ sub process {
     # Then we will start from scratch and so double delete the same records
 
     my $job_progress = 0;
-    $job->started_on(dt_from_string)
+    $self->started_on(dt_from_string)
         ->progress($job_progress)
         ->status('started')
         ->store;
@@ -44,7 +39,7 @@ sub process {
     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';
+        last if $self->get_from_storage->status eq 'cancelled';
 
         next unless $record_id;
 
@@ -71,17 +66,17 @@ sub process {
             $schema->storage->txn_commit;
         }
 
-        $job->progress( ++$job_progress )->store;
+        $self->progress( ++$job_progress )->store;
     }
 
-    my $job_data = decode_json $job->data;
+    my $job_data = decode_json $self->data;
     $job_data->{messages} = \@messages;
     $job_data->{report} = $report;
 
-    $job->ended_on(dt_from_string)
+    $self->ended_on(dt_from_string)
         ->data(encode_json $job_data);
-    $job->status('finished') if $job->status ne 'cancelled';
-    $job->store;
+    $self->status('finished') if $self->status ne 'cancelled';
+    $self->store;
 }
 
 sub enqueue {

diff --git a/Koha/BackgroundJob/BatchDeleteBiblio.pm b/Koha/BackgroundJob/BatchDeleteBiblio.pm
index 8c94720..15246fe 100644 (file)
--- a/Koha/BackgroundJob/BatchDeleteBiblio.pm
+++ b/Koha/BackgroundJob/BatchDeleteBiblio.pm
@@ -3,7 +3,6 @@ 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;
 
@@ -38,11 +37,7 @@ Process the job.
 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' ) {
+    if ( $self->status eq 'cancelled' ) {
         return;
     }
 
@@ -50,7 +45,7 @@ sub process {
     # Then we will start from scratch and so double delete the same records
 
     my $job_progress = 0;
-    $job->started_on(dt_from_string)
+    $self->started_on(dt_from_string)
         ->progress($job_progress)
         ->status('started')
         ->store;
@@ -66,7 +61,7 @@ sub process {
     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';
+        last if $self->get_from_storage->status eq 'cancelled';
 
         next unless $record_id;
 
@@ -85,7 +80,7 @@ sub process {
                 biblionumber => $biblionumber,
             };
             $schema->storage->txn_rollback;
-            $job->progress( ++$job_progress )->store;
+            $self->progress( ++$job_progress )->store;
             next;
         }
 
@@ -104,7 +99,7 @@ sub process {
                     error => "$@",
                 };
                 $schema->storage->txn_rollback;
-                $job->progress( ++$job_progress )->store;
+                $self->progress( ++$job_progress )->store;
                 next RECORD_IDS;
             }
         }
@@ -122,7 +117,7 @@ sub process {
                     error => @{$deleted->messages}[0]->message,
                 };
                 $schema->storage->txn_rollback;
-                $job->progress( ++$job_progress )->store;
+                $self->progress( ++$job_progress )->store;
                 next RECORD_IDS;
             }
         }
@@ -139,7 +134,7 @@ sub process {
                 error => ($@ ? $@ : $error),
             };
             $schema->storage->txn_rollback;
-            $job->progress( ++$job_progress )->store;
+            $self->progress( ++$job_progress )->store;
             next;
         }
 
@@ -150,17 +145,17 @@ sub process {
         };
         $report->{total_success}++;
         $schema->storage->txn_commit;
-        $job->progress( ++$job_progress )->store;
+        $self->progress( ++$job_progress )->store;
     }
 
-    my $job_data = decode_json $job->data;
+    my $job_data = decode_json $self->data;
     $job_data->{messages} = \@messages;
     $job_data->{report} = $report;
 
-    $job->ended_on(dt_from_string)
+    $self->ended_on(dt_from_string)
         ->data(encode_json $job_data);
-    $job->status('finished') if $job->status ne 'cancelled';
-    $job->store;
+    $self->status('finished') if $self->status ne 'cancelled';
+    $self->store;
 }
 
 =head3 enqueue

diff --git a/Koha/BackgroundJob/BatchDeleteItem.pm b/Koha/BackgroundJob/BatchDeleteItem.pm
index b5147e7..b4bcbfa 100644 (file)
--- a/Koha/BackgroundJob/BatchDeleteItem.pm
+++ b/Koha/BackgroundJob/BatchDeleteItem.pm
@@ -26,7 +26,6 @@ use JSON qw( encode_json decode_json );
 use List::MoreUtils qw( uniq );
 use Try::Tiny;
 
-use Koha::BackgroundJobs;
 use Koha::DateUtils qw( dt_from_string );
 use Koha::Items;
 
@@ -75,11 +74,7 @@ The generated report will be:
 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' ) {
+    if ( $self->status eq 'cancelled' ) {
         return;
     }
 
@@ -87,7 +82,7 @@ sub process {
     # 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)
+    $self->started_on(dt_from_string)->progress($job_progress)
       ->status('started')->store;
 
     my @record_ids     = @{ $args->{record_ids} };
@@ -109,7 +104,7 @@ sub process {
                 my (@biblionumbers);
                 for my $record_id ( sort { $a <=> $b } @record_ids ) {
 
-                    last if $job->get_from_storage->status eq 'cancelled';
+                    last if $self->get_from_storage->status eq 'cancelled';
 
                     my $item = Koha::Items->find($record_id) || next;
 
@@ -136,7 +131,7 @@ sub process {
                     push @biblionumbers,       $item->biblionumber;
 
                     $report->{total_success}++;
-                    $job->progress( ++$job_progress )->store;
+                    $self->progress( ++$job_progress )->store;
                 }
 
                 # If there are no items left, delete the biblio
@@ -183,13 +178,13 @@ sub process {
     $report->{not_deleted_itemnumbers} = \@not_deleted_itemnumbers;
     $report->{deleted_biblionumbers}   = \@deleted_biblionumbers;
 
-    my $job_data = decode_json $job->data;
+    my $job_data = decode_json $self->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;
+    $self->ended_on(dt_from_string)->data( encode_json $job_data);
+    $self->status('finished') if $self->status ne 'cancelled';
+    $self->store;
 }
 
 =head3 enqueue

diff --git a/Koha/BackgroundJob/BatchUpdateAuthority.pm b/Koha/BackgroundJob/BatchUpdateAuthority.pm
index 9eacf58..482acf8 100644 (file)
--- a/Koha/BackgroundJob/BatchUpdateAuthority.pm
+++ b/Koha/BackgroundJob/BatchUpdateAuthority.pm
@@ -55,14 +55,12 @@ Process the modification.
 sub process {
     my ( $self, $args ) = @_;
 
-    my $job = Koha::BackgroundJobs->find( $args->{job_id} );
-
-    if ( !exists $args->{job_id} || !$job || $job->status eq 'cancelled' ) {
+    if ( $self->status eq 'cancelled' ) {
         return;
     }
 
     my $job_progress = 0;
-    $job->started_on(dt_from_string)
+    $self->started_on(dt_from_string)
         ->progress($job_progress)
         ->status('started')
         ->store;
@@ -100,17 +98,17 @@ sub process {
             };
             $report->{total_success}++;
         }
-        $job->progress( ++$job_progress )->store;
+        $self->progress( ++$job_progress )->store;
     }
 
-    my $job_data = decode_json $job->data;
+    my $job_data = decode_json $self->data;
     $job_data->{messages} = \@messages;
     $job_data->{report} = $report;
 
-    $job->ended_on(dt_from_string)
+    $self->ended_on(dt_from_string)
         ->data(encode_json $job_data);
-    $job->status('finished') if $job->status ne 'cancelled';
-    $job->store;
+    $self->status('finished') if $self->status ne 'cancelled';
+    $self->store;
 
 }
 

diff --git a/Koha/BackgroundJob/BatchUpdateBiblio.pm b/Koha/BackgroundJob/BatchUpdateBiblio.pm
index 480928c..21e938f 100644 (file)
--- a/Koha/BackgroundJob/BatchUpdateBiblio.pm
+++ b/Koha/BackgroundJob/BatchUpdateBiblio.pm
@@ -18,7 +18,6 @@ package Koha::BackgroundJob::BatchUpdateBiblio;
 use Modern::Perl;
 use JSON qw( decode_json encode_json );
 
-use Koha::BackgroundJobs;
 use Koha::DateUtils qw( dt_from_string );
 use Koha::Virtualshelves;
 
@@ -57,9 +56,7 @@ Process the modification.
 sub process {
     my ( $self, $args ) = @_;
 
-    my $job = Koha::BackgroundJobs->find( $args->{job_id} );
-
-    if ( !exists $args->{job_id} || !$job || $job->status eq 'cancelled' ) {
+    if ( $self->status eq 'cancelled' ) {
         return;
     }
 
@@ -67,7 +64,7 @@ sub process {
     # Then we will start from scratch and so double process the same records
 
     my $job_progress = 0;
-    $job->started_on(dt_from_string)
+    $self->started_on(dt_from_string)
         ->progress($job_progress)
         ->status('started')
         ->store;
@@ -82,7 +79,7 @@ sub process {
     my @messages;
     RECORD_IDS: for my $biblionumber ( sort { $a <=> $b } @record_ids ) {
 
-        last if $job->get_from_storage->status eq 'cancelled';
+        last if $self->get_from_storage->status eq 'cancelled';
 
         next unless $biblionumber;
 
@@ -110,17 +107,17 @@ sub process {
             };
             $report->{total_success}++;
         }
-        $job->progress( ++$job_progress )->store;
+        $self->progress( ++$job_progress )->store;
     }
 
-    my $job_data = decode_json $job->data;
+    my $job_data = decode_json $self->data;
     $job_data->{messages} = \@messages;
     $job_data->{report} = $report;
 
-    $job->ended_on(dt_from_string)
+    $self->ended_on(dt_from_string)
         ->data(encode_json $job_data);
-    $job->status('finished') if $job->status ne 'cancelled';
-    $job->store;
+    $self->status('finished') if $self->status ne 'cancelled';
+    $self->store;
 }
 
 =head3 enqueue

diff --git a/Koha/BackgroundJob/BatchUpdateItem.pm b/Koha/BackgroundJob/BatchUpdateItem.pm
index fb50893..7e7ae38 100644 (file)
--- a/Koha/BackgroundJob/BatchUpdateItem.pm
+++ b/Koha/BackgroundJob/BatchUpdateItem.pm
@@ -27,7 +27,6 @@ use MARC::Field;
 use C4::Biblio;
 use C4::Items;
 
-use Koha::BackgroundJobs;
 use Koha::DateUtils qw( dt_from_string );
 use Koha::SearchEngine::Indexer;
 use Koha::Items;
@@ -85,9 +84,7 @@ regex_mod allows to modify existing subfield's values using a regular expression
 sub process {
     my ( $self, $args ) = @_;
 
-    my $job = Koha::BackgroundJobs->find( $args->{job_id} );
-
-    if ( !exists $args->{job_id} || !$job || $job->status eq 'cancelled' ) {
+    if ( $self->status eq 'cancelled' ) {
         return;
     }
 
@@ -95,7 +92,7 @@ sub process {
     # 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)
+    $self->started_on(dt_from_string)->progress($job_progress)
       ->status('started')->store;
 
     my @record_ids = @{ $args->{record_ids} };
@@ -120,7 +117,7 @@ sub process {
                   $exclude_from_local_holds_priority,
                 callback => sub {
                     my ($progress) = @_;
-                    $job->progress($progress)->store;
+                    $self->progress($progress)->store;
                 },
             }
           );
@@ -133,13 +130,13 @@ sub process {
           if ( $_ =~ /Rollback failed/ );    # Rollback failed
     };
 
-    $job->discard_changes;
-    my $job_data = decode_json encode_utf8 $job->data;
+    $self->discard_changes;
+    my $job_data = decode_json encode_utf8 $self->data;
     $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;
+    $self->ended_on(dt_from_string)->data( encode_json $job_data);
+    $self->status('finished') if $self->status ne 'cancelled';
+    $self->store;
 }
 
 =head3 enqueue
@@ -173,9 +170,7 @@ Sent the infos to generate the table containing the details of the modified item
 sub additional_report {
     my ( $self, $args ) = @_;
 
-    my $job = Koha::BackgroundJobs->find( $args->{job_id} );
-
-    my $itemnumbers = $job->report->{modified_itemnumbers};
+    my $itemnumbers = $self->report->{modified_itemnumbers};
     if ( scalar(@$itemnumbers) > C4::Context->preference('MaxItemsToDisplayForBatchMod') ) {
         return { too_many_items_display => 1 };
     } else {