Bug 18071 - Add new script update_authorities.pl
Summary: Add new script update_authorities.pl
Status: Pushed to Master
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: Nick Clemens
URL:
Whiteboard:
Keywords: Manual
: 8579 (view as bug list)
Depends on: 18070
Blocks: 17908
  Show dependency treegraph
 
Reported: 2017-02-07 12:57 UTC by Marcel de Rooy
Modified: 2018-01-10 19:55 UTC (History)
6 users (show)

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


Attachments
Bug 18070: Add script for clean removal of authorities (3.30 KB, patch)
2017-02-15 12:52 UTC, Marcel de Rooy
Details | Splinter Review
Bug 18071: Add authorities maintenance script (5.58 KB, patch)
2017-06-30 06:29 UTC, Marcel de Rooy
Details | Splinter Review
Bug 18071: Add authorities maintenance script (5.77 KB, patch)
2017-06-30 06:50 UTC, Marcel de Rooy
Details | Splinter Review
Bug 18071: Add authorities maintenance script (5.79 KB, patch)
2017-09-05 13:57 UTC, Lee Jamison
Details | Splinter Review
Bug 18071: (QA follow-up) Add commit parameter, add warning (3.22 KB, patch)
2017-12-01 11:33 UTC, Marcel de Rooy
Details | Splinter Review
Bug 18071: Add authorities maintenance script (5.84 KB, patch)
2018-01-05 17:43 UTC, Nick Clemens
Details | Splinter Review
Bug 18071: (QA follow-up) Add commit parameter, add warning (3.24 KB, patch)
2018-01-05 17:43 UTC, Nick Clemens
Details | Splinter Review
Bug 18071: (QA Followup) - Add additional feedback and expand commit option (3.93 KB, patch)
2018-01-05 17:43 UTC, Nick Clemens
Details | Splinter Review
Bug 18071: (QA Followup) - Add additional feedback and expand commit option (4.02 KB, patch)
2018-01-08 07:51 UTC, Marcel de Rooy
Details | Splinter Review
Bug 18071: Improve parameters handling (2.25 KB, patch)
2018-01-09 19:56 UTC, Jonathan Druart
Details | Splinter Review
Bug 18071: (QA follow-up) Rename commit to confirm parameter (3.27 KB, patch)
2018-01-10 12:33 UTC, Marcel de Rooy
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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!