From 84c48f7c1eb313770cec356313b230201faa62fb Mon Sep 17 00:00:00 2001
From: David Gustafsson <glasklas@gmail.com>
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.

This change enables MARC Overlay rules to be enabled for bulkmarkcimport.pl as using
the defer_marc option would previously effectively bypass any defined rules.

To test
1) Apply patch 29440 (which this depends on), but no not yet apply this
   patch
2) Enable MARC overlay rules and add a rule (with source => "*" or
   'bulkmarcimport') to protect some field.
3) Import a biblio using the bulkmarcimport script. Overlay rules will
   only be applied on updates so a match condition need to be supplied
   (for example -match "control-number,001")
4) Edit the imported biblio and change the value of the protected field
5) Run the import again and verify the the field that should have been
   protected has been overwritten
6) Apply the patch and repeat steps 4-5, verify that the field was
   protected and has not been overwritten
---
 C4/Biblio.pm                           | 17 +----------------
 misc/migration_tools/bulkmarcimport.pl | 13 +------------
 2 files changed, 2 insertions(+), 28 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<MARC::Record> 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<defer_marc_save>,
-which if present and mapped to a true value, causes C<AddBiblio>
-to omit the call to save the MARC in C<biblio_metadata.metadata>
-This option is provided B<only>
-for the use of scripts such as C<bulkmarcimport.pl> 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<ModBiblioMarc> 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 5ab289f88a..0f1f883cb4 100755
--- a/misc/migration_tools/bulkmarcimport.pl
+++ b/misc/migration_tools/bulkmarcimport.pl
@@ -548,7 +548,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,13 +570,6 @@ RECORD: foreach my $record (@{$marc_records}) {
                 $record_has_added_items = @{$itemnumbers_ref};
 
                 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);
                 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);
@@ -624,10 +617,6 @@ RECORD: foreach my $record (@{$marc_records}) {
                         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 {
-- 
2.34.0