Attachment 174375 Details for Bug 34355 – Bug 34355: (QA follow-up) Make record staging code DRY

[patch] Bug 34355: (QA follow-up) Make record staging code DRY

Bug-34355-QA-follow-up-Make-record-staging-code-DR.patch (text/plain), 15.18 KB, created by Matt Blenkinsop on 2024-11-11 15:24:39 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Matt Blenkinsop

Created: 2024-11-11 15:24:39 UTC

Size: 15.18 KB

patch

obsolete

>From bdab6bc575479b9e0060bd26d2702de0eacbafe2 Mon Sep 17 00:00:00 2001
>From: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
>Date: Mon, 21 Oct 2024 14:18:36 +0000
>Subject: [PATCH] Bug 34355: (QA follow-up) Make record staging code DRY
>
>Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
>---
> Koha/BackgroundJob/StageMARCForImport.pm | 120 +------------------
> Koha/ImportBatch.pm                      | 146 +++++++++++++++++++++++
> Koha/MarcOrder.pm                        |  77 +-----------
> 3 files changed, 153 insertions(+), 190 deletions(-)
>
>diff --git a/Koha/BackgroundJob/StageMARCForImport.pm b/Koha/BackgroundJob/StageMARCForImport.pm
>index e536ded50bf..cead144cd5d 100644
>--- a/Koha/BackgroundJob/StageMARCForImport.pm
>+++ b/Koha/BackgroundJob/StageMARCForImport.pm
>@@ -66,124 +66,12 @@ sub process {
> 
>     $self->start;
> 
>-    my $record_type                = $args->{record_type};
>-    my $encoding                   = $args->{encoding};
>-    my $format                     = $args->{format};
>-    my $filepath                   = $args->{filepath};
>-    my $filename                   = $args->{filename};
>-    my $marc_modification_template = $args->{marc_modification_template};
>-    my $comments                   = $args->{comments};
>-    my $parse_items                = $args->{parse_items};
>-    my $matcher_id                 = $args->{matcher_id};
>-    my $overlay_action             = $args->{overlay_action};
>-    my $nomatch_action             = $args->{nomatch_action};
>-    my $item_action                = $args->{item_action};
>-    my $vendor_id                  = $args->{vendor_id};
>-    my $basket_id                  = $args->{basket_id};
>-    my $profile_id                 = $args->{profile_id};
>-
>-    my @messages;
>-    my ( $batch_id, $num_valid, $num_items, @import_errors );
>-    my $num_with_matches = 0;
>-    my $checked_matches  = 0;
>-    my $matcher_failed   = 0;
>-    my $matcher_code     = "";
>-
>-    my $schema = Koha::Database->new->schema;
>-    try {
>-        $schema->storage->txn_begin;
>-
>-        my ( $errors, $marcrecords );
>-        if ( $format eq 'MARCXML' ) {
>-            ( $errors, $marcrecords ) =
>-              C4::ImportBatch::RecordsFromMARCXMLFile( $filepath, $encoding );
>-        }
>-        elsif ( $format eq 'ISO2709' ) {
>-            ( $errors, $marcrecords ) =
>-              C4::ImportBatch::RecordsFromISO2709File( $filepath, $record_type,
>-                $encoding );
>-        }
>-        else {    # plugin based
>-            $errors = [];
>-            $marcrecords =
>-              C4::ImportBatch::RecordsFromMarcPlugin( $filepath, $format,
>-                $encoding );
>-        }
>-
>-        $self->size(scalar @$marcrecords)->store;
>-
>-        ( $batch_id, $num_valid, $num_items, @import_errors ) = BatchStageMarcRecords(
>-            $record_type,                $encoding,
>-            $marcrecords,                $filename,
>-            $marc_modification_template, $comments,
>-            '',                          $parse_items,
>-            0,                           50,
>-            sub {
>-                my $job_progress = shift;
>-                if ($matcher_id) {
>-                    $job_progress /= 2;
>-                }
>-                $self->progress( int($job_progress) )->store;
>-            }
>-        );
>-        if( $num_valid ) {
>-            $self->set({ progress => $num_valid, size => $num_valid });
>-        } else { # We must assume that something went wrong here
>-            $self->set({ progress => 0, status => 'failed' });
>-        }
>-
>-        if ($profile_id) {
>-            my $ibatch = Koha::ImportBatches->find($batch_id);
>-            $ibatch->set( { profile_id => $profile_id } )->store;
>-        }
>-
>-        if ($matcher_id) {
>-            my $matcher = C4::Matcher->fetch($matcher_id);
>-            if ( defined $matcher ) {
>-                $checked_matches = 1;
>-                $matcher_code    = $matcher->code();
>-                $num_with_matches =
>-                  BatchFindDuplicates( $batch_id, $matcher, 10, 50,
>-                    sub { my $job_progress = shift; $self->progress( $job_progress )->store } );
>-                SetImportBatchMatcher( $batch_id, $matcher_id );
>-                SetImportBatchOverlayAction( $batch_id, $overlay_action );
>-                SetImportBatchNoMatchAction( $batch_id, $nomatch_action );
>-                SetImportBatchItemAction( $batch_id, $item_action );
>-                $schema->storage->txn_commit;
>-            }
>-            else {
>-                $matcher_failed = 1;
>-                $schema->storage->txn_rollback;
>-            }
>-        } else {
>-            $schema->storage->txn_commit;
>-        }
>-    }
>-    catch {
>-        warn $_;
>-        $schema->storage->txn_rollback;
>-        die "Something terrible has happened!"
>-          if ( $_ =~ /Rollback failed/ );    # TODO Check test: Rollback failed
>-        $self->set({ progress => 0, status => 'failed' });
>-    };
>-
>-    my $report = {
>-        staged          => $num_valid,
>-        matched         => $num_with_matches,
>-        num_items       => $num_items,
>-        import_errors   => scalar(@import_errors),
>-        total           => $num_valid + scalar(@import_errors),
>-        checked_matches => $checked_matches,
>-        matcher_failed  => $matcher_failed,
>-        matcher_code    => $matcher_code,
>-        import_batch_id => $batch_id,
>-        vendor_id       => $vendor_id,
>-        basket_id       => $basket_id,
>-    };
>+    my $import_args = { %$args, job => $self };
>+    my $result = Koha::ImportBatch->new_from_file($import_args);
> 
>     my $data = $self->decoded_data;
>-    $data->{messages} = \@messages;
>-    $data->{report}   = $report;
>+    $data->{messages} = $result->{messages};
>+    $data->{report}   = $result->{report};
> 
>     $self->finish($data);
> }
>diff --git a/Koha/ImportBatch.pm b/Koha/ImportBatch.pm
>index e30389c133c..994ff8b564f 100644
>--- a/Koha/ImportBatch.pm
>+++ b/Koha/ImportBatch.pm
>@@ -19,6 +19,22 @@ package Koha::ImportBatch;
> # <http://www.gnu.org/licenses>
> 
> use Modern::Perl;
>+use Try::Tiny;
>+
>+use Koha::Database;
>+use Koha::ImportBatches;
>+use C4::Matcher;
>+use C4::ImportBatch qw(
>+    RecordsFromMARCXMLFile
>+    RecordsFromISO2709File
>+    RecordsFromMarcPlugin
>+    BatchStageMarcRecords
>+    BatchFindDuplicates
>+    SetImportBatchMatcher
>+    SetImportBatchOverlayAction
>+    SetImportBatchNoMatchAction
>+    SetImportBatchItemAction
>+);
> 
> use base qw(Koha::Object);
> 
>@@ -30,6 +46,136 @@ Koha::ImportBatch - Koha ImportBatch Object class
> 
> =head2 Class Methods
> 
>+=head3 new_from_file
>+
>+Koha::ImportBatch->new_from_file($args);
>+
>+This method is used to create a new Koha::ImportBatch object from a file.
>+If being called from a background job, $args->{job} must be set.
>+
>+=cut
>+
>+sub new_from_file {
>+    my ( $self, $args ) = @_;
>+
>+    my $job                        = $args->{job};
>+    my $record_type                = $args->{record_type};
>+    my $encoding                   = $args->{encoding};
>+    my $format                     = $args->{format};
>+    my $filepath                   = $args->{filepath};
>+    my $filename                   = $args->{filename};
>+    my $marc_modification_template = $args->{marc_modification_template};
>+    my $comments                   = $args->{comments};
>+    my $parse_items                = $args->{parse_items};
>+    my $matcher_id                 = $args->{matcher_id};
>+    my $overlay_action             = $args->{overlay_action};
>+    my $nomatch_action             = $args->{nomatch_action};
>+    my $item_action                = $args->{item_action};
>+    my $vendor_id                  = $args->{vendor_id};
>+    my $basket_id                  = $args->{basket_id};
>+    my $profile_id                 = $args->{profile_id};
>+
>+    my @messages;
>+    my ( $batch_id, $num_valid, $num_items, @import_errors );
>+    my $num_with_matches = 0;
>+    my $checked_matches  = 0;
>+    my $matcher_failed   = 0;
>+    my $matcher_code     = "";
>+
>+    my $schema = Koha::Database->new->schema;
>+    try {
>+        $schema->storage->txn_begin;
>+
>+        my ( $errors, $marcrecords );
>+        if ( $format eq 'MARCXML' ) {
>+            ( $errors, $marcrecords ) = C4::ImportBatch::RecordsFromMARCXMLFile( $filepath, $encoding );
>+        } elsif ( $format eq 'ISO2709' ) {
>+            ( $errors, $marcrecords ) = C4::ImportBatch::RecordsFromISO2709File(
>+                $filepath, $record_type,
>+                $encoding
>+            );
>+        } else {    # plugin based
>+            $errors      = [];
>+            $marcrecords = C4::ImportBatch::RecordsFromMarcPlugin(
>+                $filepath, $format,
>+                $encoding
>+            );
>+        }
>+
>+        $job->size( scalar @$marcrecords )->store if $job;
>+
>+        ( $batch_id, $num_valid, $num_items, @import_errors ) = BatchStageMarcRecords(
>+            $record_type,                $encoding,
>+            $marcrecords,                $filename,
>+            $marc_modification_template, $comments,
>+            '',                          $parse_items,
>+            0,                           50,
>+            sub {
>+                my $job_progress = shift;
>+                if ($matcher_id) {
>+                    $job_progress /= 2;
>+                }
>+                $job->progress( int($job_progress) )->store if $job;
>+            }
>+        );
>+        if ( $num_valid && $job ) {
>+            $job->set( { progress => $num_valid, size => $num_valid } );
>+        } else {    # We must assume that something went wrong here
>+            $job->set( { progress => 0, status => 'failed' } );
>+        }
>+
>+        if ($profile_id) {
>+            my $ibatch = Koha::ImportBatches->find($batch_id);
>+            $ibatch->set( { profile_id => $profile_id } )->store;
>+        }
>+
>+        if ($matcher_id) {
>+            my $matcher = C4::Matcher->fetch($matcher_id);
>+            if ( defined $matcher ) {
>+                $checked_matches  = 1;
>+                $matcher_code     = $matcher->code();
>+                $num_with_matches = BatchFindDuplicates(
>+                    $batch_id, $matcher, 10, 50,
>+                    sub { my $job_progress = shift; $job->progress($job_progress)->store if $job }
>+                );
>+                SetImportBatchMatcher( $batch_id, $matcher_id );
>+                SetImportBatchOverlayAction( $batch_id, $overlay_action );
>+                SetImportBatchNoMatchAction( $batch_id, $nomatch_action );
>+                SetImportBatchItemAction( $batch_id, $item_action );
>+                $schema->storage->txn_commit;
>+            } else {
>+                $matcher_failed = 1;
>+                $schema->storage->txn_rollback;
>+            }
>+        } else {
>+            $schema->storage->txn_commit;
>+        }
>+    } catch {
>+        warn $_;
>+        $schema->storage->txn_rollback;
>+        die "Something terrible has happened!"
>+            if ( $_ =~ /Rollback failed/ );    # TODO Check test: Rollback failed
>+        $job->set( { progress => 0, status => 'failed' } ) if $job;
>+    };
>+
>+    my $report = {
>+        staged          => $num_valid,
>+        matched         => $num_with_matches,
>+        num_items       => $num_items,
>+        import_errors   => scalar(@import_errors),
>+        total           => $num_valid + scalar(@import_errors),
>+        checked_matches => $checked_matches,
>+        matcher_failed  => $matcher_failed,
>+        matcher_code    => $matcher_code,
>+        import_batch_id => $batch_id,
>+        vendor_id       => $vendor_id,
>+        basket_id       => $basket_id,
>+    };
>+
>+    return { report => $report, messages => \@messages };
>+}
>+
>+
> =head3 _type
> 
> =cut
>diff --git a/Koha/MarcOrder.pm b/Koha/MarcOrder.pm
>index 5d90171a800..627b7fd85e6 100644
>--- a/Koha/MarcOrder.pm
>+++ b/Koha/MarcOrder.pm
>@@ -251,87 +251,16 @@ sub _create_basket_for_file {
> 
>     $file->_stage_file($params)
> 
>-    Stages a file directly using parameters from a marc ordering account and without using the background job
>-    This function is a mirror of Koha::BackgroundJob::StageMARCForImport->process but with the background job functionality removed
>+    Stages a file directly using parameters from a marc ordering account
> 
> =cut
> 
> sub _stage_file {
>     my ($args) = @_;
> 
>-    my $record_type                = $args->{record_type};
>-    my $encoding                   = $args->{encoding};
>-    my $format                     = $args->{format};
>-    my $filepath                   = $args->{filepath};
>-    my $filename                   = $args->{filename};
>-    my $marc_modification_template = $args->{marc_modification_template};
>-    my $comments                   = $args->{comments};
>-    my $parse_items                = $args->{parse_items};
>-    my $matcher_id                 = $args->{matcher_id};
>-    my $overlay_action             = $args->{overlay_action};
>-    my $nomatch_action             = $args->{nomatch_action};
>-    my $item_action                = $args->{item_action};
>-
>-    my ( $batch_id, $num_valid, $num_items, @import_errors );
>-    my $num_with_matches = 0;
>-    my $checked_matches  = 0;
>-    my $matcher_failed   = 0;
>-    my $matcher_code     = "";
>-
>-    my $schema = Koha::Database->new->schema;
>-    try {
>-        $schema->storage->txn_begin;
>-
>-        my ( $errors, $marcrecords );
>-        if ( $format eq 'MARCXML' ) {
>-            ( $errors, $marcrecords ) = C4::ImportBatch::RecordsFromMARCXMLFile( $filepath, $encoding );
>-        } elsif ( $format eq 'ISO2709' ) {
>-            ( $errors, $marcrecords ) = C4::ImportBatch::RecordsFromISO2709File(
>-                $filepath, $record_type,
>-                $encoding
>-            );
>-        } else {    # plugin based
>-            $errors      = [];
>-            $marcrecords = C4::ImportBatch::RecordsFromMarcPlugin(
>-                $filepath, $format,
>-                $encoding
>-            );
>-        }
>-
>-        ( $batch_id, $num_valid, $num_items, @import_errors ) = BatchStageMarcRecords(
>-            $record_type,                $encoding,
>-            $marcrecords,                $filename,
>-            $marc_modification_template, $comments,
>-            '',                          $parse_items,
>-            0
>-        );
>-
>-        if ($matcher_id) {
>-            my $matcher = C4::Matcher->fetch($matcher_id);
>-            if ( defined $matcher ) {
>-                $checked_matches  = 1;
>-                $matcher_code     = $matcher->code();
>-                $num_with_matches = BatchFindDuplicates( $batch_id, $matcher, 10 );
>-                SetImportBatchMatcher( $batch_id, $matcher_id );
>-                SetImportBatchOverlayAction( $batch_id, $overlay_action );
>-                SetImportBatchNoMatchAction( $batch_id, $nomatch_action );
>-                SetImportBatchItemAction( $batch_id, $item_action );
>-                $schema->storage->txn_commit;
>-            } else {
>-                $matcher_failed = 1;
>-                $schema->storage->txn_rollback;
>-            }
>-        } else {
>-            $schema->storage->txn_commit;
>-        }
>+    my $result = Koha::ImportBatch->new_from_file($args);
> 
>-        return $batch_id;
>-    } catch {
>-        warn $_;
>-        $schema->storage->txn_rollback;
>-        die "Something terrible has happened!"
>-            if ( $_ =~ /Rollback failed/ );    # TODO Check test: Rollback failed
>-    };
>+    return $result->{report}->{import_batch_id};
> }
> 
> =head3 _get_syspref_mappings
>-- 
>2.39.3 (Apple Git-146)

Actions: View | Diff | Splinter Review

Attachments on bug 34355: 154705 | 154706 | 154707 | 154708 | 154709 | 154710 | 157092 | 157093 | 157094 | 157095 | 157096 | 157097 | 157098 | 157099 | 157100 | 160858 | 160859 | 160860 | 160861 | 160862 | 160863 | 160864 | 160865 | 160866 | 160867 | 160868 | 160869 | 160870 | 160871 | 160872 | 160873 | 160874 | 160875 | 169896 | 169897 | 169898 | 169899 | 169900 | 169901 | 169902 | 169903 | 169904 | 170395 | 170401 | 170402 | 170403 | 170404 | 170405 | 170406 | 170407 | 170408 | 170409 | 173054 | 173055 | 173056 | 173057 | 173058 | 173059 | 173060 | 173061 | 173062 | 173063 | 173064 | 173065 | 173066 | 173709 | 173805 | 173806 | 173809 | 173824 | 173825 | 173993 | 173994 | 173995 | 173996 | 173997 | 173998 | 173999 | 174000 | 174001 | 174002 | 174003 | 174004 | 174005 | 174006 | 174007 | 174008 | 174009 | 174010 | 174362 | 174363 | 174364 | 174365 | 174366 | 174367 | 174368 | 174369 | 174370 | 174371 | 174372 | 174373 | 174374 | 174375 | 174376 | 174377 | 174378 | 174379 | 174380 | 174382 | 174383