Bug 29654 - Add option to bulkmarimport.pl for matching on original id
Summary: Add option to bulkmarimport.pl for matching on original id
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: David Gustafsson
QA Contact: Testopia
URL:
Keywords:
Depends on: 29440
Blocks:
  Show dependency treegraph
 
Reported: 2021-12-07 23:01 UTC by David Gustafsson
Modified: 2024-01-03 16:40 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 29654 - Add match_record_id option to bulkmarimport.pl (1.74 KB, patch)
2021-12-07 23:09 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 29654 - Add match_record_id option to bulkmarimport.pl (2.60 KB, patch)
2021-12-08 14:04 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 29654: Add match_record_id option to bulkmarimport.pl (2.65 KB, patch)
2021-12-15 16:56 UTC, David Gustafsson
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Gustafsson 2021-12-07 23:01:14 UTC

    
Comment 1 David Gustafsson 2021-12-07 23:07:42 UTC
If I'm not mistaken, currently in  bulkmarkimport.pl, if no match is found for authorities the incoming record is used for matching against an existing Koha authority, this is not done for biblio records though. Instead of this inconsistent behaviour it would be better if there was an option to explicitly set if matching on incoming record ids should be performed or not.
Comment 2 David Gustafsson 2021-12-07 23:09:47 UTC
Created attachment 128335 [details] [review]
Bug 29654 - Add match_record_id option to bulkmarimport.pl
Comment 3 David Gustafsson 2021-12-07 23:11:22 UTC
Not yet ready for review, will later add how to test.
Comment 4 David Gustafsson 2021-12-08 14:04:03 UTC
Created attachment 128365 [details] [review]
Bug 29654 - Add match_record_id option to bulkmarimport.pl

Add -match_record_id option to bulkmarkimport.pl. When enabled
incoming record id will be assumed to match record in Koha.

To test:

1) Before applying patch, export a biblio from staff interace
2) Import using the exported file:
   ./bulkmarcimport.pl -m="MARCXML" -v -b -file bib-xxxxx.marcxml -insert -update -c=MARC21 -l "/tmp/import.log"
3) The import with be inserted and create a duplicate record in Koha.
   Check the import.log or search using the staff interface.
   (could also crash with duplicate key error if contains item)
4) Apply patch
5) Run the import again with the same command as above
6) A new record should been inserted
7) Remove all newly inserted duplicate records (it's important to keep
   the record the export was initially performed on)
6) Run the import again, but add the -match_record_id option
8) The original Koha record should now have been updated
Comment 5 David Gustafsson 2021-12-15 16:56:01 UTC
Created attachment 128575 [details] [review]
Bug 29654: Add match_record_id option to bulkmarimport.pl

Add -match_record_id option to bulkmarkimport.pl. When enabled
incoming record id will be assumed to match record in Koha.

To test:

1) Before applying patch, export a biblio from staff interace
2) Import using the exported file:
   ./bulkmarcimport.pl -m="MARCXML" -v -b -file bib-xxxxx.marcxml -insert -update -c=MARC21 -l "/tmp/import.log"
3) The import with be inserted and create a duplicate record in Koha.
   Check the import.log or search using the staff interface.
   (could also crash with duplicate key error if contains item)
4) Apply patch
5) Run the import again with the same command as above
6) A new record should been inserted
7) Remove all newly inserted duplicate records (it's important to keep
   the record the export was initially performed on)
6) Run the import again, but add the -match_record_id option
8) The original Koha record should now have been updated
Comment 6 Frank Hansen 2022-02-04 13:04:44 UTC
The patch doesn't apply anymore. Rebase please.

Using index info to reconstruct a base tree...
M       misc/migration_tools/bulkmarcimport.pl
Falling back to patching base and 3-way merge...
Auto-merging misc/migration_tools/bulkmarcimport.pl
CONFLICT (content): Merge conflict in misc/migration_tools/bulkmarcimport.pl
error: Failed to merge in the changes.
Patch failed at 0001 Bug 29654: Add match_record_id option to bulkmarimport.pl
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-29654-Add-matchrecordid-option-to-bulkmarimpor-rJcwEW.patch
vagrant@kohadevbox:kohaclone(bug29654|AM 1/1)$
Comment 7 David Gustafsson 2022-03-01 15:15:44 UTC
I must have accidentally added two different iterations of the same patch, now it should work.
Comment 8 Katrin Fischer 2022-06-25 16:28:44 UTC
Sorry, patch doesn't apply (again):

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 29440: Refactor and clean up bulkmarcimport.pl
Using index info to reconstruct a base tree...
M	C4/Biblio.pm
Falling back to patching base and 3-way merge...
Auto-merging C4/Biblio.pm
CONFLICT (content): Merge conflict in C4/Biblio.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 29440: Refactor and clean up bulkmarcimport.pl
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-29440-Refactor-and-clean-up-bulkmarcimportpl-zsN_GN.patch

Cannot apply cleanly patches from bug 29440. Everything will be left dirty.
git bz apply --continue will not continue the process if patches from other bug reports need to be applied.
Comment 9 David Gustafsson 2022-11-07 17:08:07 UTC
Sorry about being slow to reply. Tried applying the patch and applied cleanly for me, realized the conflict was in bug 29440 which since have been rebased.
Comment 10 Michał 2023-12-20 18:39:28 UTC
I'm not sure, as far as I understand the code, that this is the best approach to implementing it. It looks like it still tries to match the record by old logic, and only afterwards swaps out the ID.

I don't think it should perform any matching when we specify the Koha record ID upfront, or at most it should check if it already exists...