From df216f44e52aba5a3b212f340de4130c46deb9dc Mon Sep 17 00:00:00 2001 From: David Gustafsson Date: Fri, 12 Nov 2021 23:16:55 +0100 Subject: [PATCH] Bug 25539: Remove AddBiblio option "defer_marc_save" Items are no longer embedded in the MARCXML and because of this the MARC data does not need to be saved once more after changing record items data. The "defer_marc_save" is no longer needed since bulkmarcimport.pl was the only place this option was utilized in order to resave MARC data after possibly changing items data. There is also a bug bulkmarcimport.pl where the record data is re-saved without stripping items if duplicate items are found and the dedup barcodes option is enabled that is resolved by this change. --- C4/Biblio.pm | 17 +--------- misc/migration_tools/bulkmarcimport.pl | 46 ++++++++++---------------- 2 files changed, 19 insertions(+), 44 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index f057c80207..bc4fc160f0 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -187,31 +187,16 @@ The first argument is a C object containing the bib to add, while the second argument is the desired MARC framework code. -This function also accepts a third, optional argument: a hashref -to additional options. The only defined option is C, -which if present and mapped to a true value, causes C -to omit the call to save the MARC in C -This option is provided B -for the use of scripts such as C that may need -to do some manipulation of the MARC record for item parsing before -saving it and which cannot afford the performance hit of saving -the MARC record twice. Consequently, do not use that option -unless you can guarantee that C will be called. - =cut sub AddBiblio { my $record = shift; my $frameworkcode = shift; my $options = @_ ? shift : undef; - my $defer_marc_save = 0; if (!$record) { carp('AddBiblio called with undefined record'); return; } - if ( defined $options and exists $options->{'defer_marc_save'} and $options->{'defer_marc_save'} ) { - $defer_marc_save = 1; - } my $schema = Koha::Database->schema; my ( $biblionumber, $biblioitemnumber ); @@ -292,7 +277,7 @@ sub AddBiblio { } # now add the record - ModBiblioMarc( $record, $biblionumber ) unless $defer_marc_save; + ModBiblioMarc( $record, $biblionumber ); # update OAI-PMH sets if(C4::Context->preference("OAI-PMH:AutoUpdateSets")) { diff --git a/misc/migration_tools/bulkmarcimport.pl b/misc/migration_tools/bulkmarcimport.pl index 3409e00608..43467a756c 100755 --- a/misc/migration_tools/bulkmarcimport.pl +++ b/misc/migration_tools/bulkmarcimport.pl @@ -502,7 +502,7 @@ RECORD: foreach my $record (@{$marc_records}) { } } else { - my ($biblioitemnumber, $itemnumbers_ref, $errors_ref, $record_id); + my ($biblioitemnumber, $itemnumbers, $errors, $record_id); # check for duplicate, based on ISBN (skip it if we already have found a duplicate with match parameter if (!$matched_record_id && $isbn_check && $isbn) { $sth_isbn->execute($isbn); @@ -551,7 +551,7 @@ RECORD: foreach my $record (@{$marc_records}) { } } elsif ($insert) { - eval { ($record_id, $biblioitemnumber) = AddBiblio($record, $framework, { defer_marc_save => 1 }) }; + eval { ($record_id, $biblioitemnumber) = AddBiblio($record, $framework) }; if ($@) { warn "ERROR: Insert biblio $originalid failed: $@\n"; printlog( { id => $originalid, op => "insert", status => "ERROR" } ) if ($logfile); @@ -570,16 +570,9 @@ RECORD: foreach my $record (@{$marc_records}) { if ($record_id) { $yamlhash->{$originalid} = $record_id if $yamlfile; # TODO: Add option for skipping items? - eval { ($itemnumbers_ref, $errors_ref) = AddItemBatchFromMarc($record, $record_id, $biblioitemnumber, $framework); }; - $record_has_added_items = @{$itemnumbers_ref}; + eval { ($itemnumbers, $errors) = AddItemBatchFromMarc($record, $record_id, $biblioitemnumber, $framework); }; my $error_adding = $@; - # Work on a clone so that if there are real errors, we can maybe - # fix them up later. - my $clone_record = $record->clone(); - C4::Biblio::_strip_item_fields($clone_record, $framework); - # This sets the marc fields if there was an error, and also calls - # defer_marc_save. - ModBiblioMarc($clone_record, $record_id, $modify_biblio_marc_options); + $record_has_added_items = @{$itemnumbers}; if ($error_adding) { warn "ERROR: Adding items to bib $record_id failed: $error_adding"; printlog({ id => $record_id, op => "insert items", status => "ERROR"}) if ($logfile); @@ -590,20 +583,20 @@ RECORD: foreach my $record (@{$marc_records}) { else { printlog({ id => $record_id, op => "insert items", status => "ok" }) if ($logfile); } - if ($dedup_barcode && grep { exists $_->{error_code} && $_->{error_code} eq 'duplicate_barcode' } @$errors_ref) { + if ($dedup_barcode && grep { exists $_->{error_code} && $_->{error_code} eq 'duplicate_barcode' } @{$errors}) { # Find the record called 'barcode' my ($tag, $sub) = C4::Biblio::GetMarcFromKohaField('items.barcode'); # Now remove any items that didn't have a duplicate_barcode error, # erase the barcodes on items that did, and re-add those items. my %dupes; # FIXME: This could cause array out of bounds because shifting down rest of items on array item delete? - foreach my $i (0 .. $#{$errors_ref}) { - my $ref = $errors_ref->[$i]; - if ($ref && ($ref->{error_code} eq 'duplicate_barcode')) { - $dupes{$ref->{item_sequence}} = 1; + foreach my $i (0 .. $#{$errors}) { + my $error = $errors->[$i]; + if ($error && ($error->{error_code} eq 'duplicate_barcode')) { + $dupes{$error->{item_sequence}} = 1; # Delete the error message because we're going to # retry this one. - delete $errors_ref->[$i]; + delete $errors->[$i]; } } my $seq = 0; @@ -621,26 +614,23 @@ RECORD: foreach my $record (@{$marc_records}) { } # Now re-add the record as before, adding errors to the prev list my $more_errors; - eval { ($itemnumbers_ref, $more_errors) = AddItemBatchFromMarc($record, $record_id, $biblioitemnumber, ''); }; - $record_has_added_items ||= @{$itemnumbers_ref}; - if ($@) { - warn "ERROR: Adding items to bib $record_id failed: $@\n"; + eval { ($itemnumbers, $more_errors) = AddItemBatchFromMarc($record, $record_id, $biblioitemnumber, ''); }; + $error_adding = $@; + $record_has_added_items ||= @{$itemnumbers}; + if ($error_adding) { + warn "ERROR: Adding items to bib $record_id failed: $error_adding\n"; printlog({ id => $record_id, op => "insert items", status => "ERROR" }) if ($logfile); # if we failed because of an exception, assume that # the MARC columns in biblioitems were not set. - - # @FIXME: Why do we save here without stripping items? Besides, - # save with stripped items has already been performed - ModBiblioMarc($record, $record_id, $modify_biblio_marc_options); next RECORD; } else { printlog({ id => $record_id, op => "insert", status => "ok" }) if ($logfile); } - push @$errors_ref, @{$more_errors}; + push @{$errors}, @{$more_errors}; } - if (@{$errors_ref}) { - report_item_errors($record_id, $errors_ref); + if (@{$errors}) { + report_item_errors($record_id, $errors); } C4::Biblio::_strip_item_fields($record, $framework); if ($record_has_added_items || $matched_record_id) { -- 2.20.1