From 66f416d3e333032bc4783a3a7e2246bc1155ee1b Mon Sep 17 00:00:00 2001
From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Date: Mon, 5 Sep 2022 11:57:52 +0000
Subject: [PATCH] Bug 27421: (QA follow-up) BatchCommitImportRecords needs
 param for skipping commits
Content-Type: text/plain; charset=utf-8

When you submit a background jobs, and it fails, you do not expect
partial results in the database.
Note that when the Background feature would support a partially
completed status, things might change again.

Test plan:
Run t/db_dependent/Koha/BackgroundJobs/StageMARCForImport.t
Note: This serves to verify that it still runs as expected.
We will still test the new parameter further on.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
---
 C4/ImportBatch.pm | 38 +++++++++++++++++---------------------
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/C4/ImportBatch.pm b/C4/ImportBatch.pm
index bd51a5c738..5d9bd62017 100644
--- a/C4/ImportBatch.pm
+++ b/C4/ImportBatch.pm
@@ -509,28 +509,23 @@ sub BatchFindDuplicates {
 
 =head2 BatchCommitRecords
 
-  my ($num_added, $num_updated, $num_items_added, $num_items_replaced, $num_items_errored, $num_ignored) =
-        BatchCommitRecords($batch_id, $framework,
-        $progress_interval, $progress_callback);
+    my ($num_added, $num_updated, $num_items_added, $num_items_replaced, $num_items_errored, $num_ignored) =
+        BatchCommitRecords( $batch_id, $framework, $progress_interval, $progress_callback, $params );
+
+    Parameter skip_intermediate_commit does what is says.
 
 =cut
 
 sub BatchCommitRecords {
-    my $batch_id = shift;
-    my $framework = shift;
+    my ( $batch_id, $framework, $progress_interval, $progress_callback, $params ) = @_;
+    my $skip_intermediate_commit = $params->{skip_intermediate_commit};
+    $progress_interval = 0 unless $progress_interval =~ /^\d+$/ and $progress_interval > 0;
+    $progress_interval = 0 unless ref($progress_callback) eq 'CODE';
 
     my $schema = Koha::Database->schema;
-
-    # optional callback to monitor status 
-    # of job
-    my $progress_interval = 0;
-    my $progress_callback = undef;
-    if ($#_ == 1) {
-        $progress_interval = shift;
-        $progress_callback = shift;
-        $progress_interval = 0 unless $progress_interval =~ /^\d+$/ and $progress_interval > 0;
-        $progress_interval = 0 unless 'CODE' eq ref $progress_callback;
-    }
+    $schema->txn_begin;
+    # NOTE: Moved this transaction to the front of the routine. Note that inside the while loop below
+    # transactions may be committed and started too again. The final commit is close to the end.
 
     my $record_type;
     my $num_added = 0;
@@ -560,7 +555,6 @@ sub BatchCommitRecords {
 
     my $rec_num = 0;
     my @biblio_ids;
-    $schema->txn_begin; # We commit in a transaction
     while (my $rowref = $sth->fetchrow_hashref) {
         $record_type = $rowref->{'record_type'};
 
@@ -568,9 +562,9 @@ sub BatchCommitRecords {
 
         if ($progress_interval and (0 == ($rec_num % $progress_interval))) {
             # report progress and commit
-            $schema->txn_commit;
+            $schema->txn_commit unless $skip_intermediate_commit;
             &$progress_callback( $rec_num );
-            $schema->txn_begin;
+            $schema->txn_begin unless $skip_intermediate_commit;
         }
         if ($rowref->{'status'} eq 'error' or $rowref->{'status'} eq 'imported') {
             $num_ignored++;
@@ -700,8 +694,6 @@ sub BatchCommitRecords {
         &$progress_callback($rec_num);
     }
 
-    $schema->txn_commit; # Commit final records that may not have hit callback threshold
-
     $sth->finish();
 
     if ( @biblio_ids ) {
@@ -710,6 +702,10 @@ sub BatchCommitRecords {
     }
 
     SetImportBatchStatus($batch_id, 'imported');
+
+    # Moved final commit to the end
+    $schema->txn_commit;
+
     return ($num_added, $num_updated, $num_items_added, $num_items_replaced, $num_items_errored, $num_ignored);
 }
 
-- 
2.20.1