From bdab6bc575479b9e0060bd26d2702de0eacbafe2 Mon Sep 17 00:00:00 2001 From: Matt Blenkinsop 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 --- 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; # 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)