From d376fbdcde9cc6a01592ba0013ba5f50994fb568 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Thu, 26 May 2022 13:03:10 +0100 Subject: [PATCH] Bug 30822: Clarify that BatchCommitItems is a private function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BatchCommitItems is only being used within this module and isn't mentioned in EXPORT_OK. This patch simply renames it to _batchCommitItems to take the _ standard for private functions and also adds a little hint to the POD of the function to clarify that the caller must trigger a re-index. JK: Amended patch to rename also the function in t/db_dependent/ImportBatch.t and fix typo "commiting" => "commiting" Signed-off-by: Joonas Kylmälä --- C4/ImportBatch.pm | 16 +++++++++------- t/db_dependent/ImportBatch.t | 4 ++-- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/C4/ImportBatch.pm b/C4/ImportBatch.pm index 5603f741fe3..8295c93a287 100644 --- a/C4/ImportBatch.pm +++ b/C4/ImportBatch.pm @@ -603,7 +603,7 @@ sub BatchCommitRecords { my $marc_record = MARC::Record->new_from_usmarc($rowref->{'marc'}); if ($record_type eq 'biblio') { - # remove any item tags - rely on BatchCommitItems + # remove any item tags - rely on _batchCommitItems ($item_tag,$item_subfield) = &GetMarcFromKohaField( "items.itemnumber" ); foreach my $item_field ($marc_record->field($item_tag)) { $marc_record->delete_field($item_field); @@ -624,7 +624,7 @@ sub BatchCommitRecords { push @biblio_ids, $recordid; $query = "UPDATE import_biblios SET matched_biblionumber = ? WHERE import_record_id = ?"; # FIXME call SetMatchedBiblionumber instead if ($item_result eq 'create_new' || $item_result eq 'replace') { - my ($bib_items_added, $bib_items_replaced, $bib_items_errored) = BatchCommitItems($rowref->{'import_record_id'}, $recordid, $item_result, $biblioitemnumber); + my ($bib_items_added, $bib_items_replaced, $bib_items_errored) = _batchCommitItems($rowref->{'import_record_id'}, $recordid, $item_result, $biblioitemnumber); $num_items_added += $bib_items_added; $num_items_replaced += $bib_items_replaced; $num_items_errored += $bib_items_errored; @@ -673,7 +673,7 @@ sub BatchCommitRecords { $query = "UPDATE import_biblios SET matched_biblionumber = ? WHERE import_record_id = ?"; # FIXME call SetMatchedBiblionumber instead if ($item_result eq 'create_new' || $item_result eq 'replace') { - my ($bib_items_added, $bib_items_replaced, $bib_items_errored) = BatchCommitItems($rowref->{'import_record_id'}, $recordid, $item_result); + my ($bib_items_added, $bib_items_replaced, $bib_items_errored) = _batchCommitItems($rowref->{'import_record_id'}, $recordid, $item_result); $num_items_added += $bib_items_added; $num_items_replaced += $bib_items_replaced; $num_items_errored += $bib_items_errored; @@ -696,7 +696,7 @@ sub BatchCommitRecords { $recordid = $record_match; $num_ignored++; if ($record_type eq 'biblio' and defined $recordid and ( $item_result eq 'create_new' || $item_result eq 'replace' ) ) { - my ($bib_items_added, $bib_items_replaced, $bib_items_errored) = BatchCommitItems($rowref->{'import_record_id'}, $recordid, $item_result); + my ($bib_items_added, $bib_items_replaced, $bib_items_errored) = _batchCommitItems($rowref->{'import_record_id'}, $recordid, $item_result); push @biblio_ids, $recordid if $bib_items_added || $bib_items_replaced; $num_items_added += $bib_items_added; $num_items_replaced += $bib_items_replaced; @@ -721,14 +721,16 @@ sub BatchCommitRecords { return ($num_added, $num_updated, $num_items_added, $num_items_replaced, $num_items_errored, $num_ignored); } -=head2 BatchCommitItems +=head2 _batchCommitItems ($num_items_added, $num_items_errored) = - BatchCommitItems($import_record_id, $biblionumber, [$action, $biblioitemnumber]); + _batchCommitItems($import_record_id, $biblionumber, [$action, $biblioitemnumber]); + +Private function for batch committing item changes. We do not trigger a re-index here, that is left to the caller. =cut -sub BatchCommitItems { +sub _batchCommitItems { my ( $import_record_id, $biblionumber, $action, $biblioitemnumber ) = @_; my $dbh = C4::Context->dbh; diff --git a/t/db_dependent/ImportBatch.t b/t/db_dependent/ImportBatch.t index 3100c53e570..9a4ea5f8cee 100755 --- a/t/db_dependent/ImportBatch.t +++ b/t/db_dependent/ImportBatch.t @@ -183,7 +183,7 @@ C4::ImportBatch::DeleteBatch( $id_import_batch3 ); my $import_batch = C4::ImportBatch::GetImportBatch( $id_import_batch3 ); is( $import_batch, undef, "Batch 3 has been deleted"); -subtest "BatchCommitItems" => sub { +subtest "_batchCommitItems" => sub { plan tests => 3; my $exist_item = $builder->build_sample_item; @@ -209,7 +209,7 @@ subtest "BatchCommitItems" => sub { }}); my ( $num_items_added, $num_items_replaced, $num_items_errored ) = - C4::ImportBatch::BatchCommitItems( $import_item->import_record_id, 32, 'always_add',64 ); + C4::ImportBatch::_batchCommitItems( $import_item->import_record_id, 32, 'always_add',64 ); is( $num_items_errored, 1, "Item with duplicate barcode fails when action always_add" ); $import_item->discard_changes(); is( $import_item->status, 'error', "Import item marked as error when duplicate barcode and action always_add"); -- 2.25.1