Bug 26231 - bulkmarcimport.pl does not import authority if it already has a 001 field
Summary: bulkmarcimport.pl does not import authority if it already has a 001 field
Status: Pushed to oldoldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: MARC Bibliographic record staging/import (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Assignee: David Cook
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-08-18 02:17 UTC by David Cook
Modified: 2020-10-20 21:08 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Manual merge request:
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.05, 19.11.11, 19.05.16


Attachments
Bug 26231: Remove incorrect use of AddAuthority() when 001 present (1.90 KB, patch)
2020-08-18 02:30 UTC, David Cook
Details | Diff | Splinter Review
Bug 26231: Remove incorrect use of AddAuthority() when 001 present (1.97 KB, patch)
2020-09-21 10:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26231: Remove incorrect use of AddAuthority() when 001 present (2.01 KB, patch)
2020-09-25 13:51 UTC, Julian Maurice
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2020-08-18 02:17:07 UTC
I'm trying to import authorities which already have a 001 and bulkmarcimport.pl says that it's worked but it hasn't actually imported anything and no errors have been thrown.

I'm using this command:

koha-shell -c "perl /usr/share/koha/bin/migration_tools/bulkmarcimport.pl -file /home/dcook/auth.mrc -a -fk -d -v -l /tmp/auth.log" <instance>

It appears to be because the value from the 001 is being passed to AddAuthority() which - instead of adding the authority - is trying to update an existing authority.
Comment 1 David Cook 2020-08-18 02:30:44 UTC
Created attachment 108431 [details] [review]
Bug 26231: Remove incorrect use of AddAuthority() when 001 present

This patch removes the use of AddAuthority with a non-empty
authid argument, since that triggers an update rather than an insert.

In practice, the update also fails, but the error isn't raised,
as the database connection doesn't have RaiseError set.

Test plan:
1) Do not apply patch
2) Try to bulkmarcimport an authority file with a 001
3) Observe that the script reports success but no authority is added
4) Apply the path
5) Try to bulkmarcimport an authority file with a 001
6) Observe that the script reports success and the authority is added
Comment 2 Fridolin Somers 2020-09-03 07:10:11 UTC
Looks logical.
Maybe just log that value of 001 will be lost no ?
Comment 3 David Cook 2020-09-04 03:56:37 UTC
(In reply to Fridolin SOMERS from comment #2)
> Looks logical.
> Maybe just log that value of 001 will be lost no ?

Possibly although that's already the case now?
Comment 4 Katrin Fischer 2020-09-15 22:02:18 UTC
I think Bug 12722 - bulkmarcimport.pl: Should we move the old 001 to 035 would deal with the issue of the lost 001. 

I am adding Marcel here, if I IIRC this is related to work he did a while ago. (Bug 18014)
Comment 5 Kyle M Hall 2020-09-21 10:49:14 UTC
Created attachment 110448 [details] [review]
Bug 26231: Remove incorrect use of AddAuthority() when 001 present

This patch removes the use of AddAuthority with a non-empty
authid argument, since that triggers an update rather than an insert.

In practice, the update also fails, but the error isn't raised,
as the database connection doesn't have RaiseError set.

Test plan:
1) Do not apply patch
2) Try to bulkmarcimport an authority file with a 001
3) Observe that the script reports success but no authority is added
4) Apply the path
5) Try to bulkmarcimport an authority file with a 001
6) Observe that the script reports success and the authority is added

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Julian Maurice 2020-09-25 13:51:39 UTC
Created attachment 110761 [details] [review]
Bug 26231: Remove incorrect use of AddAuthority() when 001 present

This patch removes the use of AddAuthority with a non-empty
authid argument, since that triggers an update rather than an insert.

In practice, the update also fails, but the error isn't raised,
as the database connection doesn't have RaiseError set.

Test plan:
1) Do not apply patch
2) Try to bulkmarcimport an authority file with a 001
3) Observe that the script reports success but no authority is added
4) Apply the path
5) Try to bulkmarcimport an authority file with a 001
6) Observe that the script reports success and the authority is added

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 7 Jonathan Druart 2020-09-28 09:10:42 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 8 Lucas Gass 2020-10-16 16:13:36 UTC
backported to 20.05.x for 20.05.05
Comment 9 Aleisha Amohia 2020-10-20 01:58:57 UTC
backported to 19.11.x for 19.11.11
Comment 10 Victor Grousset/tuxayo 2020-10-20 21:08:42 UTC
Backported to 19.05.x branch for 19.05.16