Bug 18071

Summary: Add new script update_authorities.pl
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: MARC Authority data supportAssignee: Marcel de Rooy <m.de.rooy>
Status: CLOSED FIXED QA Contact: Nick Clemens <nick>
Severity: enhancement    
Priority: P5 - low CC: fridolin.somers, jcamins, jonathan.druart, katrin.fischer, marjorie.barry-vila, martin.renvoize, nick
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This patch adds a script to perform various authority related maintenance tasks. This version supports deleting an authority record and updating all linked biblio records. Furthermore it supports merging authority records with one reference record, and updating all linked biblio records. It also allows you to force a renumber, i.e. save the authid into field 001.
Version(s) released in:
Bug Depends on: 18070    
Bug Blocks: 17908    
Attachments: Bug 18070: Add script for clean removal of authorities
Bug 18071: Add authorities maintenance script
Bug 18071: Add authorities maintenance script
Bug 18071: Add authorities maintenance script
Bug 18071: (QA follow-up) Add commit parameter, add warning
Bug 18071: Add authorities maintenance script
Bug 18071: (QA follow-up) Add commit parameter, add warning
Bug 18071: (QA Followup) - Add additional feedback and expand commit option
Bug 18071: (QA Followup) - Add additional feedback and expand commit option
Bug 18071: Improve parameters handling
Bug 18071: (QA follow-up) Rename commit to confirm parameter

Description Marcel de Rooy 2017-02-07 12:57:23 UTC

    
Comment 1 Marcel de Rooy 2017-02-15 12:52:06 UTC Comment hidden (obsolete)
Comment 2 Marcel de Rooy 2017-02-15 12:52:40 UTC Comment hidden (obsolete)
Comment 3 Marcel de Rooy 2017-02-23 13:05:46 UTC Comment hidden (obsolete)
Comment 4 Marcel de Rooy 2017-02-23 13:26:00 UTC Comment hidden (obsolete)
Comment 5 Marcel de Rooy 2017-06-30 06:29:49 UTC Comment hidden (obsolete)
Comment 6 Marcel de Rooy 2017-06-30 06:50:45 UTC
Created attachment 64744 [details] [review]
Bug 18071: Add authorities maintenance script

With bug 9988 the manual merge option of merge_authorities was removed.
(Note that it did not work any more too.) This patch reintroduces this
functionality on the command line.

This maintenance script now allows you to force renumbering field 001 for
selected authid's, to delete authid's including the removal of references
in biblio records, as well as merging several authid's into one reference
record.

Test plan:
[1] Test the -renumber parameter. Field 001 and 005 should be updated.
[2] Test the -delete parameter. Check if a linked biblio does no longer
    contain a reference to the deleted authority.
[3] Test the -merge parameter.
    Create two PERSO_NAME records (say A,B) and attach biblios to them.
    Pick a CORPO_NAME record as reference record C.
    Now pass -merge -reference C -authid A,B
    Verify that A and B are gone, and the records link to C now.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Katrin Fischer 2017-07-09 17:47:46 UTC
Hi Marcel,

reading through the test report:

 Verify that A and B are gone, and the records link to C now.

so you would merge to records of one type into another - would that not potentially cause a lot of trouble? You'd potentially end up with 100/700 fields linked to a corporate record as the fields are not corrected with the merge, right?
Comment 8 Marcel de Rooy 2017-07-10 06:28:20 UTC
(In reply to Katrin Fischer from comment #7)
> Hi Marcel,
> 
> reading through the test report:
> 
>  Verify that A and B are gone, and the records link to C now.
> 
> so you would merge to records of one type into another - would that not
> potentially cause a lot of trouble? You'd potentially end up with 100/700
> fields linked to a corporate record as the fields are not corrected with the
> merge, right?

No, not right :)
100 => 110 and 700 => 710
Merge got smarter in the meantime..
Comment 9 Katrin Fischer 2017-07-10 08:06:37 UTC
Oooh, nice. Could I recruit you to do a write up of the merge functionality for the manual? I think this is something a lot of people would be interested in and the information is a bit hard to get from a lot of bug reports.
Comment 10 Marcel de Rooy 2017-07-10 08:37:58 UTC
(In reply to Katrin Fischer from comment #9)
> Oooh, nice. Could I recruit you to do a write up of the merge functionality
> for the manual? I think this is something a lot of people would be
> interested in and the information is a bit hard to get from a lot of bug
> reports.

There is not much more to say about it. Merge supports merging across authority type borders. MARC tags in biblio records will be adjusted accordingly.
Your example of PERSO/CORPO with 100-700/110-710 would be fine.
Comment 11 Lee Jamison 2017-09-05 13:57:17 UTC
Created attachment 66854 [details] [review]
Bug 18071: Add authorities maintenance script

With bug 9988 the manual merge option of merge_authorities was removed.
(Note that it did not work any more too.) This patch reintroduces this
functionality on the command line.

This maintenance script now allows you to force renumbering field 001 for
selected authid's, to delete authid's including the removal of references
in biblio records, as well as merging several authid's into one reference
record.

Test plan:
[1] Test the -renumber parameter. Field 001 and 005 should be updated.
[2] Test the -delete parameter. Check if a linked biblio does no longer
    contain a reference to the deleted authority.
[3] Test the -merge parameter.
    Create two PERSO_NAME records (say A,B) and attach biblios to them.
    Pick a CORPO_NAME record as reference record C.
    Now pass -merge -reference C -authid A,B
    Verify that A and B are gone, and the records link to C now.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Comment 12 Nick Clemens 2017-10-13 10:44:11 UTC
Blocker:
1 - There should be a commit option
2 - If a given record is not found we should let the user know:
given: perl misc/maintenance/update_authorities.pl --authid 1, 2 --merge --reference 3 -v
result: 1 is merged, ' 2' is not found and ignored, no error

Suggestions:
Verbose should give more info:
-when merging list the reference that was used
 --would be nice to see the heading of the records if possible
-when renumbering would be nice to see the values that were changed
-when merging or deleting would be nice to get the records that were updated, or a count
-when deleting a record we should let the user know if bibs are not modified (because of AuthorityMergeLimit syspref) - I thought things were not working until I dug in to code

I think all the verbosity would have to add error returns to the modules, so I understand if it is out of the scope of this bug, but want to put the idea out there at least
Comment 13 Marcel de Rooy 2017-10-16 06:58:45 UTC
Will address these points some time later.
Comment 14 Marcel de Rooy 2017-12-01 11:33:07 UTC
Created attachment 69428 [details] [review]
Bug 18071: (QA follow-up) Add commit parameter, add warning

Addressing points mentioned in comment12:
[1] Commit parameter.
[2] Warning if authid does not exist for -merge.

Test plan:
[1] Run update_authorities.pl -authid X -merge -ref Y -c
    where X does not exist in your db and Y does.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 15 Marcel de Rooy 2017-12-01 11:34:13 UTC
(In reply to Nick Clemens from comment #12)
> Blocker:
> 1 - There should be a commit option
> 2 - If a given record is not found we should let the user know:
> given: perl misc/maintenance/update_authorities.pl --authid 1, 2 --merge
> --reference 3 -v
> result: 1 is merged, ' 2' is not found and ignored, no error

Fixed

> Suggestions:
> Verbose should give more info:
> -when merging list the reference that was used
>  --would be nice to see the heading of the records if possible
> -when renumbering would be nice to see the values that were changed
> -when merging or deleting would be nice to get the records that were
> updated, or a count
> -when deleting a record we should let the user know if bibs are not modified
> (because of AuthorityMergeLimit syspref) - I thought things were not working
> until I dug in to code
> 
> I think all the verbosity would have to add error returns to the modules, so
> I understand if it is out of the scope of this bug, but want to put the idea
> out there at least

Thx for feedback. We should put them on a new report.

He's yours again Nick.
Comment 16 Nick Clemens 2018-01-05 17:43:07 UTC
Created attachment 70310 [details] [review]
Bug 18071: Add authorities maintenance script

With bug 9988 the manual merge option of merge_authorities was removed.
(Note that it did not work any more too.) This patch reintroduces this
functionality on the command line.

This maintenance script now allows you to force renumbering field 001 for
selected authid's, to delete authid's including the removal of references
in biblio records, as well as merging several authid's into one reference
record.

Test plan:
[1] Test the -renumber parameter. Field 001 and 005 should be updated.
[2] Test the -delete parameter. Check if a linked biblio does no longer
    contain a reference to the deleted authority.
[3] Test the -merge parameter.
    Create two PERSO_NAME records (say A,B) and attach biblios to them.
    Pick a CORPO_NAME record as reference record C.
    Now pass -merge -reference C -authid A,B
    Verify that A and B are gone, and the records link to C now.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 17 Nick Clemens 2018-01-05 17:43:10 UTC
Created attachment 70311 [details] [review]
Bug 18071: (QA follow-up) Add commit parameter, add warning

Addressing points mentioned in comment12:
[1] Commit parameter.
[2] Warning if authid does not exist for -merge.

Test plan:
[1] Run update_authorities.pl -authid X -merge -ref Y -c
    where X does not exist in your db and Y does.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 18 Nick Clemens 2018-01-05 17:43:14 UTC
Created attachment 70312 [details] [review]
Bug 18071: (QA Followup) - Add additional feedback and expand commit option

To test:
Test as before, verify commit option makes no changes and provides
additional feedback when verbose

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 19 Nick Clemens 2018-01-05 17:44:09 UTC
Added some additional feedback, please verify you are okay with changes Marcel, otherwise looks good.
Comment 20 Marcel de Rooy 2018-01-08 07:51:46 UTC
Created attachment 70320 [details] [review]
Bug 18071: (QA Followup) - Add additional feedback and expand commit option

To test:
Test as before, verify commit option makes no changes and provides
additional feedback when verbose

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 21 Marcel de Rooy 2018-01-08 07:55:11 UTC
(In reply to Nick Clemens from comment #19)
> Added some additional feedback, please verify you are okay with changes
> Marcel, otherwise looks good.

Fine with me. Thx
Comment 22 Jonathan Druart 2018-01-09 19:50:40 UTC
Can we rename commit with confirm? We use commit 3x in misc instead of 8x for confirm.
Comment 23 Jonathan Druart 2018-01-09 19:56:58 UTC
Created attachment 70388 [details] [review]
Bug 18071: Improve parameters handling
Comment 24 Marcel de Rooy 2018-01-10 12:33:55 UTC
Created attachment 70400 [details] [review]
Bug 18071: (QA follow-up) Rename commit to confirm parameter

Trivial edit.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 25 Marcel de Rooy 2018-01-10 12:34:35 UTC
(In reply to Jonathan Druart from comment #22)
> Can we rename commit with confirm? We use commit 3x in misc instead of 8x
> for confirm.

Yes we can.
Comment 26 Jonathan Druart 2018-01-10 19:55:01 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 27 Nick Clemens 2018-01-23 00:51:15 UTC
Awesome work all!

Enhancement, not backported