Bug 30308 - bulkmarcimport.pl broken by OAI-PMH:AutoUpdateSets(EmbedItemData)
Summary: bulkmarcimport.pl broken by OAI-PMH:AutoUpdateSets(EmbedItemData)
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: Frédéric Demians
QA Contact: Kyle M Hall
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-03-17 10:46 UTC by Magnus Enger
Modified: 2023-06-08 22:26 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00,22.05.05,21.11.12, 21.05.19


Attachments
Bug 30308 - Fix bulkmarcimport.pl broken by OAI-PMH:AutoUpdateSets(EmbedItemData) (757 bytes, patch)
2022-07-11 10:09 UTC, Frédéric Demians
Details | Diff | Splinter Review
Bug 30308 - Fix bulkmarcimport.pl broken by OAI-PMH:AutoUpdateSets(EmbedItemData) (1.62 KB, patch)
2022-07-12 13:47 UTC, Frédéric Demians
Details | Diff | Splinter Review
Bug 30308 - Fix bulkmarcimport.pl broken by OAI-PMH:AutoUpdateSets(EmbedItemData) (1.66 KB, patch)
2022-07-14 14:23 UTC, David Nind
Details | Diff | Splinter Review
Bug 30308: Fix bulkmarcimport.pl broken by OAI-PMH:AutoUpdateSets(EmbedItemData) (1.65 KB, patch)
2022-07-29 11:41 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Magnus Enger 2022-03-17 10:46:17 UTC
I was trying to import records and items with bulkmarcimport.pl. The records were added, but no items were saved to the DB. Turning off OAI-PMH:AutoUpdateSets and OAI-PMH:AutoUpdateSetsEmbedItemData fixed the problem. I think this is what happens:

1. bulkmarcimport.pl calls C4::Biblio::AddBiblio on line 461:

eval { ( $biblionumber, $biblioitemnumber ) = AddBiblio( $record, '', { defer_marc_save => 1 } ) };

https://git.koha-community.org/Koha-community/Koha/src/branch/master/misc/migration_tools/bulkmarcimport.pl#L461

2. C4::Biblio::AddBiblio has this code:

            # update OAI-PMH sets
            if(C4::Context->preference("OAI-PMH:AutoUpdateSets")) {
                C4::OAI::Sets::UpdateOAISetsBiblio($biblionumber, $record);
            }

https://git.koha-community.org/Koha-community/Koha/src/branch/master/C4/Biblio.pm#L296

3. C4::OAI::Sets::UpdateOAISetsBiblio does this:

    if (C4::Context->preference('OAI-PMH:AutoUpdateSetsEmbedItemData')) {
        C4::Biblio::EmbedItemsInMarcBiblio({
            marc_record  => $record,
            biblionumber => $biblionumber
        });
    }

https://git.koha-community.org/Koha-community/Koha/src/branch/master/C4/OAI/Sets.pm#L612

4. C4::Biblio::EmbedItemsInMarcBiblio does:

    my ($params) = @_;
    $marc = $params->{marc_record};

    ...

    _strip_item_fields($marc, $frameworkcode);

    # ... and embed the current items
    my $dbh = C4::Context->dbh;
    my $sth = $dbh->prepare("SELECT itemnumber FROM items WHERE biblionumber = ?");

https://git.koha-community.org/Koha-community/Koha/src/branch/master/C4/Biblio.pm#L2501

The end result is that item data is stripped from the record that is about to be imported, and no item data is added from the DB, since they are not there yet. Not sure how to fix this.
Comment 1 David Cook 2022-03-18 00:04:35 UTC
This stood out to me as I have some plugin code in C4::Biblio::AddBiblio which fails due to bulkmarcimport.pl using defer_marc_save.

I imagine this wouldn't be an issue if we just got rid of the frustrating defer_marc_save via Bug 25539.
Comment 2 David Cook 2022-03-18 00:05:44 UTC
Oh actually on another review... it's totally unrelated. Don't mind me. I'm just really biased against defer_marc_save 😅
Comment 3 Fridolin Somers 2022-05-10 19:08:23 UTC
We should clone MARC::Record in UpdateOAISetsBiblio ?
Comment 4 Frédéric Demians 2022-07-11 10:09:29 UTC
Created attachment 137502 [details] [review]
Bug 30308 - Fix bulkmarcimport.pl broken by OAI-PMH:AutoUpdateSets(EmbedItemData)
Comment 5 Frédéric Demians 2022-07-11 10:11:09 UTC
(In reply to Fridolin Somers from comment #3)
> We should clone MARC::Record in UpdateOAISetsBiblio ?

Yes! It solves the issue.
Comment 6 Katrin Fischer 2022-07-11 14:08:05 UTC
Shouldn't this be Needs Signoff, Frederic?
Comment 7 David Nind 2022-07-11 23:19:54 UTC
Could we get a test plan for this?

Thanks!
Comment 8 Frédéric Demians 2022-07-12 13:47:25 UTC
Created attachment 137630 [details] [review]
Bug 30308 - Fix bulkmarcimport.pl broken by OAI-PMH:AutoUpdateSets(EmbedItemData)

Test plan:

(A) Reproduce the bug:

A1 In Administration > System preferences > Web services:
   - Enable OAI-PMH
   - Enable OAI-PMH:AutoUpdateSets
   - Enable OAI-PMH:AutoUpdateSetsEmbedItemData

A2 In Tools > Export data > Export bibliographic records, export 1 (one)
   biblio record. Be sure that this biblio record has at least one item.
   Don't tick "Don't export items".

A3 Delete the exported biblio record, and its items.

A4 In command line, on the server load the exported file:
   ./bulkmarcimport.pl -b -v -file /path/to/koha.mrc

A5 Retrieve the biblio record in Koha. Note the absence of the item(s). This is
   the bug.

(B) Apply the patch

B1 Delete the record (without item) loaded at A4.

B2 In command line, on the server load the exported file:
   ./bulkmarcimport.pl -b -v -file /path/to/koha.mrc

B2 Retrieve the biblio record in Koha. Note the presence of the item(s).
Comment 9 David Nind 2022-07-14 14:23:37 UTC
Created attachment 137721 [details] [review]
Bug 30308 - Fix bulkmarcimport.pl broken by OAI-PMH:AutoUpdateSets(EmbedItemData)

Test plan:

(A) Reproduce the bug:

A1 In Administration > System preferences > Web services:
   - Enable OAI-PMH
   - Enable OAI-PMH:AutoUpdateSets
   - Enable OAI-PMH:AutoUpdateSetsEmbedItemData

A2 In Tools > Export data > Export bibliographic records, export 1 (one)
   biblio record. Be sure that this biblio record has at least one item.
   Don't tick "Don't export items".

A3 Delete the exported biblio record, and its items.

A4 In command line, on the server load the exported file:
   ./bulkmarcimport.pl -b -v -file /path/to/koha.mrc

A5 Retrieve the biblio record in Koha. Note the absence of the item(s). This is
   the bug.

(B) Apply the patch

B1 Delete the record (without item) loaded at A4.

B2 In command line, on the server load the exported file:
   ./bulkmarcimport.pl -b -v -file /path/to/koha.mrc

B2 Retrieve the biblio record in Koha. Note the presence of the item(s).

Signed-off-by: David Nind <david@davidnind.com>
Comment 10 David Nind 2022-07-14 14:26:03 UTC
Thanks Frédéric for the test plan!
Comment 11 Katrin Fischer 2022-07-29 08:21:22 UTC
Frederic, please don't forget to assign yourself!
Comment 12 Kyle M Hall 2022-07-29 11:41:56 UTC
Created attachment 138301 [details] [review]
Bug 30308: Fix bulkmarcimport.pl broken by OAI-PMH:AutoUpdateSets(EmbedItemData)

Test plan:

(A) Reproduce the bug:

A1 In Administration > System preferences > Web services:
   - Enable OAI-PMH
   - Enable OAI-PMH:AutoUpdateSets
   - Enable OAI-PMH:AutoUpdateSetsEmbedItemData

A2 In Tools > Export data > Export bibliographic records, export 1 (one)
   biblio record. Be sure that this biblio record has at least one item.
   Don't tick "Don't export items".

A3 Delete the exported biblio record, and its items.

A4 In command line, on the server load the exported file:
   ./bulkmarcimport.pl -b -v -file /path/to/koha.mrc

A5 Retrieve the biblio record in Koha. Note the absence of the item(s). This is
   the bug.

(B) Apply the patch

B1 Delete the record (without item) loaded at A4.

B2 In command line, on the server load the exported file:
   ./bulkmarcimport.pl -b -v -file /path/to/koha.mrc

B2 Retrieve the biblio record in Koha. Note the presence of the item(s).

Signed-off-by: David Nind <david@davidnind.com>
Comment 13 Tomás Cohen Arazi 2022-07-29 18:51:22 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 14 Katrin Fischer 2022-07-29 20:31:27 UTC
Don't forget to sign, Kyle :) Thx for QA!
Comment 15 Fridolin Somers 2022-09-02 07:47:07 UTC
Please backport to old branches
Comment 16 Lucas Gass 2022-09-02 17:05:29 UTC
Backported to 22.05.x for 22.05.05
Comment 17 Arthur Suzuki 2022-09-14 12:58:29 UTC
Thanks!

Pushed to 21.11 for 21.11.12
Comment 18 Victor Grousset/tuxayo 2022-09-19 21:50:31 UTC
Backported: Pushed to 21.05.x branch for 21.05.19

nothing to document