Bug 17908 - Authority merge omnibus
Summary: Authority merge omnibus
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: MARC Authority data support (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
: 3343 13760 14583 16802 18106 (view as bug list)
Depends on: 5572 8579 9988 11315 11700 14026 14769 16018 17380 17909 17913 17950 17980 18014 18070 18071 18149
Blocks:
  Show dependency treegraph
 
Reported: 2017-01-14 10:49 UTC by Marcel de Rooy
Modified: 2018-05-15 12:15 UTC (History)
20 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2017-01-14 10:49:15 UTC
In order to coordinate some developments on this subject, I will add some bug reports here and *try* to plan getting them further.
Comment 1 Marcel de Rooy 2017-01-14 11:09:36 UTC
*** Bug 3343 has been marked as a duplicate of this bug. ***
Comment 2 Marcel de Rooy 2017-01-14 12:01:20 UTC
*** Bug 16802 has been marked as a duplicate of this bug. ***
Comment 3 Marcel de Rooy 2017-01-14 12:27:14 UTC
*** Bug 14583 has been marked as a duplicate of this bug. ***
Comment 4 Marcel de Rooy 2017-01-14 12:45:52 UTC
*** Bug 13760 has been marked as a duplicate of this bug. ***
Comment 5 Marcel de Rooy 2017-01-14 13:08:29 UTC
This subject is touched in many reports and we have a lot of duplicated code. The solutions under 5572/11315/11700 are more comprehensive and make merge behavior pref controlled. Somehow we need to *merge* the best of code from 11315, 11700 and 5572 in order to get further.
I would suggest to copy tests from 11315/11700 and my own extended test originally aimed for 9988 to report 17909 as a starting point to verify the current merge routine.
On that we should build the core changes from 5572/11315/11700. (Include the the $field->replace fix from 5572.) My suggestion for the pref name is AuthorityMergeMode btw.

I checked the following reports and propose the following order to get some progress:

[CLOSED] 03343 Duplicates
[CLOSED] 05821 Enable cron job
[CLOSED] 16802 Clearing subfields
[CLOSED] 12791 dontmerge is confusing
[CLOSED] 14583 Deleting subfields
[CLOSED] 13760 What about two 700s linking to the same authority

[START] 17909 Add tests for authority merge
[PHASE2] 05572 Improve C4::AuthoritiesMarc->merge() + syspref AuthCleanBiblios
[PHASE2] 11315 Subfield deletion (overwriteSubfieldsOnMerge)
[PHASE2] 11700 C4::AuthoritiesMarc::merge enhancements/fixes 
[PHASE3] 14769 Correct indicators in merge
[PHASE4/BLOCKED] 16018 Not properly merged (authorities/merge.pl) 
[PHASE5] 09988 AuthorityMergeLimit (instead of dontmerge) 
[PHASE6] 14026 Deleting an authority

Did I forget a report, or would you suggest a different order, please tell.
Comment 6 Josef Moravec 2017-01-14 19:40:49 UTC
(In reply to Marcel de Rooy from comment #5)

Sounds reasonable, I'll be happy to test and sign off. Thanks for your attention!
Comment 7 Katrin Fischer 2017-01-15 11:11:00 UTC
Commented on bug 13760 about multiple links to same authority in one record.
Comment 8 Marcel de Rooy 2017-01-16 12:09:30 UTC
Stage 1 is ready to go. Bug 17909 is ready for signoff. Merges tests from 11315 and 11700.
Hope to get stage 2 further later this week.
Comment 9 Marcel de Rooy 2017-01-17 08:07:19 UTC
A closer look at bug 14769 revealed that we should move it further in the line of work. We actually need framework default indicators first to address merges crossing the authtype border correctly.
Comment 10 Marcel de Rooy 2017-01-17 11:56:20 UTC
Stage 2 is ready for signoff too. Look at bug 17913. The patches allow you to read all specific fixes easily. Some ideas going back to 2011 revived.
We still need to look at:

[PHASE3/BLOCKED] 16018 Not properly merged (authorities/merge.pl) 
[PHASE4] 09988 AuthorityMergeLimit (instead of dontmerge) 
[PHASE5] 14026 Deleting an authority
[PHASE6/DEPENDS ON 9701] 14769 Correct indicators in merge
In phase4 some things might be Koha-Objectified.

Before moving further, I need your support in test and QA. As soon as we hit the PQA queue, I plan to continue. Thanks !
Comment 11 Marcel de Rooy 2017-01-20 13:15:45 UTC
Bug 17909 and 17913 moved to PQA now with thanks to Josef and Julian!
I added a intermediate report for the Merge.t test after a quick fix for UNIMARC field 100. That is bug 17950.
Comment 12 Marcel de Rooy 2017-01-24 14:05:18 UTC
Stage 3 is on its way. Can we get these three reports further this week or so?
BUG 17950: Tiny improvements on Merge.t
BUG 17980: Fixing ModZebra calls in AuthoritiesMarc
BUG 16018: Merge.pl does not update biblios linked to the old authority
The third one is the most interesting fix in this stage, originating from Remi Marchand. Thanks!

What remains is:
[PHASE4] 09988 AuthorityMergeLimit (instead of dontmerge) 
[PHASE5] 14026 Deleting an authority
[PHASE6/DEPENDS ON 9701] 14769 Correct indicators in merge
In phase4 some things might be Koha-Objectified.
We might need one or two intermediate reports in stage 4 to pave the way.
Comment 13 Marcel de Rooy 2017-01-31 09:12:10 UTC
I added report 18014 to solve the problem of reusing authid's after deleting the last record. (In that regard also adding 12722 for adjusting bulkmarcimport and 8579 for a maintenance script.)

Status in the current stage is now:
NSO Bug 17950: Tiny improvements on Merge.t
NSO Bug 17980: Fixing ModZebra calls in AuthoritiesMarc
NSO Bug 18014: Fix AddAuthority
SO Bug 16018: Merge.pl does not update biblios linked to the old authority

So please help by adding your signoff on those three OR QAing the last one !
Comment 14 Marcel de Rooy 2017-02-07 14:00:24 UTC
Progress update:
We have some reports waiting in PQA queue. Thanks Mark, Julian, Mehdi and Nick.
Still open in this stage are:
[NSO] Bug 17980: Fix ModZebra call in AuthoritiesMarc
[NSO] Bug 18070: Support clean removal of authorities
After these two, the stage should be ready for bug 9988 and friends.
Comment 15 Marcel de Rooy 2017-02-23 09:17:48 UTC
*** Bug 18106 has been marked as a duplicate of this bug. ***
Comment 16 Marcel de Rooy 2017-02-23 13:04:06 UTC
Update:
Bug 18070 made its way to PQA (thx Josef and Julian).
Submitted bug 9988 now. It is the enhancement that replaces dontmerge by a limit. This sounds easier than it was. The Zebra code in merge is removed. The cron job has been refactored, and is no longer optional. It also supports merges from one authority type to another (with a table revision). Should be the last patch in this omnibus that makes fundamental changes to merge, except for a small expected change of the field indicators.
So: Please test !

What should still follow?
Housekeeping: Bug 18149 Replace CountUsage calls (finish a refactoring started in 9988)
Bug 18071 Maintenance script
Bug 14769 Indicators
Bug 12722 Bulkmarcimport ??
Comment 17 Marcel de Rooy 2017-03-07 09:17:33 UTC
Progress update:
Bug 18070 has been pushed. The next step is to refine the merge functionality in bug 9988. It is waiting for your signoff. Please help to get this further. Thanks!
Comment 18 Marcel de Rooy 2017-06-30 06:56:54 UTC
Long time no see?
Bug 9988 got pushed into 17.05 in the meantime.
I submitted two new reports now:

18071 Maintenance script update_authorities: renumber, delete or merge auth records on the command line
18149 Replace CountUsage calls by Koha Object calls

Have fun !
Comment 19 Marcel de Rooy 2017-07-05 12:26:24 UTC
Although several authority issues on bug 17380 are outside the scope of this omnibus, I will include a patch on 17380 that still adds a refinement of sub merge. Therefore adding this report to the dependency list too.
Comment 20 Marcel de Rooy 2018-02-08 13:53:08 UTC
One of the last steps in this omnibus is bug 14769. Ready for some attention from your part :) Be welcome!
About biblio indicators controlled by authorities.
Comment 21 Marcel de Rooy 2018-05-15 12:14:47 UTC
I am removing 12722 from the list here and close this omnibus now. Bug 14769 got pushed to 18.05; I consider it the last of this series.