Bug 31185 - Link authorities automatically doesn't detect duplicate authorities
Summary: Link authorities automatically doesn't detect duplicate authorities
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal with 11 votes (vote)
Assignee: Adolfo Rodríguez Taboada
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-07-19 11:03 UTC by Adolfo Rodríguez Taboada
Modified: 2023-07-18 10:56 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.02


Attachments
Detect duplicate authorities using Link authorities automatically (1.40 KB, patch)
2022-07-19 11:13 UTC, Adolfo Rodríguez Taboada
Details | Diff | Splinter Review
Bug 31185 - Link authorities automatically doesn't detect duplicate authorities (1.45 KB, patch)
2022-08-02 18:50 UTC, Rachael
Details | Diff | Splinter Review
Bug 31185: Link authorities automatically doesn't detect duplicate authorities (2.27 KB, patch)
2022-08-27 14:35 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 31185: Unit test (2.86 KB, patch)
2023-05-03 17:21 UTC, Nick Clemens
Details | Diff | Splinter Review
New patch version (2.37 KB, patch)
2023-05-05 14:28 UTC, Adolfo Rodríguez Taboada
Details | Diff | Splinter Review
Bug 31185: Unit test (2.92 KB, patch)
2023-06-21 09:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31185: Link authorities automatically doesn't detect duplicate authorities (2.54 KB, patch)
2023-06-21 09:50 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Adolfo Rodríguez Taboada 2022-07-19 11:03:20 UTC
When cataloging a bibliographic record, the template addbiblio.tt has a code to show that there is more than one local match for the entry point (MULTIPLE_MATCH) but the function LinkBibHeadingsToAuthorities never returns that code. 

This means that when the authority you are looking for is duplicated and you click the "Link authorities automatically", you obtain the following message "No matching authority found." instead of "More than one local match found. Possibly a duplicate authority!" This gives the false impression that the authority doesn't exist.
Comment 1 Adolfo Rodríguez Taboada 2022-07-19 11:13:57 UTC
Created attachment 137860 [details] [review]
Detect duplicate authorities using Link authorities automatically

This patch fixes LinkBibHeadingsToAuthorities so it detects duplicate authorities using "Link authorities automatically"

To test
1 Make sure the system preference AutoCreateAuthorities is turned to don't generate
2 Create a duplicate authority and check that search in Authorities module returns both. For example, two Personal Name (Smith, John)
3 Create a new record and fill the entry point with the same text of the authorities you just created. For example 100$a Smith, John
4 Click "Link authorities automatically" and you will obtain the following message "No matching authority found."
5 Apply patch, restart services
6 Click "Link authorities automatically" and you will obtain the following message "More than one local match found. Possibly a duplicate authority!"
Comment 2 Katrin Fischer 2022-07-24 21:16:25 UTC
Don't forget to switch the status to "Needs Signoff", once you have a patch attached for testing!
Comment 3 Rachael 2022-08-02 18:50:47 UTC
Created attachment 138518 [details] [review]
Bug 31185 - Link authorities automatically doesn't detect duplicate authorities

Signed-off-by: Rachael Laritz <rachael.laritz@inlibro.com>
Comment 4 Katrin Fischer 2022-08-05 11:03:20 UTC
Please don't forget to set Assignee! :)
Comment 5 Katrin Fischer 2022-08-27 14:35:14 UTC
Created attachment 139857 [details] [review]
Bug 31185: Link authorities automatically doesn't detect duplicate authorities

This patch fixes LinkBibHeadingsToAuthorities so it detects duplicate authorities using "Link authorities automatically"

To test
1 Make sure the system preference AutoCreateAuthorities is turned to don't generate
2 Create a duplicate authority and check that search in Authorities module returns both. For example, two Personal Name (Smith, John)
3 Create a new record and fill the entry point with the same text of the authorities you just created. For example 100$a Smith, John
4 Click "Link authorities automatically" and you will obtain the following message "No matching authority found."
5 Apply patch, restart services
6 Click "Link authorities automatically" and you will obtain the following message "More than one local match found. Possibly a duplicate authority!"

Signed-off-by: Rachael Laritz <rachael.laritz@inlibro.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 6 Katrin Fischer 2022-08-27 14:36:47 UTC
I have added the bug description and test plan to the commit message according to our guidelines. Also happy to sign off on this, but we'll need a unit test covering this change for it to pass QA. Please let us know if we can help with that!
Comment 7 Martin Renvoize 2022-09-08 09:15:06 UTC
Yikes, there are hardly any tests for LinkBibHeadingsToAuthorities at the moment :(
Comment 8 Martin Renvoize 2022-09-23 15:44:05 UTC
I'd love to see this one move forward.. but I'm not sure I can pick up adding the tests myself.
Comment 9 Nick Clemens 2023-05-03 17:21:14 UTC
Created attachment 150610 [details] [review]
Bug 31185: Unit test
Comment 10 Nick Clemens 2023-05-03 17:22:32 UTC
I added a unit test, but I discovered that if AutoCreateAuthorities is on, we won't report multiple matches. I think we should? Feel free to adjust test expectations if not, but I suspect it reveals a problem
Comment 11 Adolfo Rodríguez Taboada 2023-05-05 14:28:01 UTC
Created attachment 150762 [details] [review]
New patch version

The test expectations are correct, it should report multiple matches if AutoCreateAuthorities is on as it's an issue must be dealt with no matter the value of that preference.
This is an updated version of the patch that reports multiple matches in both scenarios.
Thanks for the unit test!
Comment 12 Katrin Fischer 2023-06-09 12:04:35 UTC
I am sorry, but this doesn't apply:

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 31185: Unit test
error: sha1 information is lacking or useless (t/db_dependent/Biblio.t).
error: could not build fake ancestor
Patch failed at 0001 Bug 31185: Unit test
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-31185-Unit-test-xmcWMx.patch

Please also make sure to include a bug description and a test plan in the commit message when re-submitting.
Comment 13 Martin Renvoize 2023-06-21 09:50:22 UTC
Created attachment 152507 [details] [review]
Bug 31185: Unit test

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Martin Renvoize 2023-06-21 09:50:26 UTC
Created attachment 152508 [details] [review]
Bug 31185: Link authorities automatically doesn't detect duplicate authorities

Signed-off-by: Rachael Laritz <rachael.laritz@inlibro.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Martin Renvoize 2023-06-21 09:51:51 UTC
All now working as expected and passing the test Nick wrote (thanks for that Nick).

I'm happy to say this seems solid now. I've rebased and am happy to PQA.

Note, you might want to look into using git-bz Adolfo, it really helps with uploading the patches etc.
Comment 16 Tomás Cohen Arazi 2023-06-21 16:20:25 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 17 Martin Renvoize 2023-07-17 12:21:54 UTC
Thanks for all the hard work!

Pushed to 23.05.x for the next release
Comment 18 Pedro Amorim 2023-07-18 10:56:44 UTC
Doesn't apply on 22.11.x. Not backporting.