Bug 21450 - link_bibs_to_authorities.pl is caching searches without the auth type
Summary: link_bibs_to_authorities.pl is caching searches without the auth type
Status: Pushed to Stable
Alias: None
Product: Koha
Classification: Unclassified
Component: MARC Authority data support (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Target Milestone: ---
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-09-28 14:58 UTC by Nick Clemens
Modified: 2019-05-30 20:25 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
19.05.00, 18.11.06


Attachments
Bug 21450: Cache authorities search using term and auth_type (2.61 KB, patch)
2018-09-28 15:02 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 21450: Unit tests (3.94 KB, patch)
2019-01-24 00:40 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 21450: Cache authorities search using term and auth_type (2.76 KB, patch)
2019-01-24 00:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 21450: Cache authorities search using term and auth_type (2.81 KB, patch)
2019-05-14 10:35 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 21450: Unit tests (3.97 KB, patch)
2019-05-24 17:57 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 21450: Cache authorities search using term and auth_type (2.85 KB, patch)
2019-05-24 17:57 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 21450: (QA follow-up) remove a couple of unused variables to make QA script happy (786 bytes, patch)
2019-05-24 17:57 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 21450: Unit tests (4.05 KB, patch)
2019-05-24 20:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21450: Cache authorities search using term and auth_type (2.92 KB, patch)
2019-05-24 20:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21450: (QA follow-up) remove a couple of unused variables to make QA script happy (859 bytes, patch)
2019-05-24 20:35 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2018-09-28 14:58:09 UTC
It seems that the script stores the results of a search term using only the search term - not the authority type, this can be problematic

Image A record containing both a subject and topic term:
650 _0 $aShort stories
655 __ $aShort stories

And there being an authority record for each:
Short stories AUTH_TYPE SU_TOPIC #1708
Short stories AUTH_TYPE GENRE #1709

The linker for the first stores:
$cache->{Short stories} = 1708
and the genre link then reuses this search
Comment 1 Nick Clemens 2018-09-28 15:02:23 UTC Comment hidden (obsolete)
Comment 2 Marcel de Rooy 2018-10-05 09:52:23 UTC Comment hidden (obsolete)
Comment 3 Nick Clemens 2018-10-05 17:23:32 UTC Comment hidden (obsolete)
Comment 4 Nick Clemens 2019-01-24 00:40:56 UTC
Created attachment 84340 [details] [review]
Bug 21450: Unit tests

To test:
1 - Apply just this patch
2 - prove -v t/db_dependent/Linker_Default.t
3 - It fails
4 - Apply second patch
5 - prove -v t/db_dependent/Linker_Default.t
6 - It passes
Comment 5 Nick Clemens 2019-01-24 00:41:00 UTC
Created attachment 84341 [details] [review]
Bug 21450: Cache authorities search using term and auth_type

This is a simple fix to prevent incorrect caching of results when using
link_bibs_to_authorities

To test:
1 - Find or create a record with the same term in a subject and genre
heading
2 - Find or create authority records for the term as a subject and genre
type
3 - Run link bibs to authorities
4 - Verify both tags in the record are linked to the subject heading
5 - Aply patch
6 - Unlink the record and re-run the script (or just re-run)
7 - Each tag should be linked to correct authority
Comment 6 Frank Hansen 2019-05-14 08:58:00 UTC
Nice job! This patch applies and works fine for me.
Comment 7 Katrin Fischer 2019-05-14 09:28:20 UTC
Hi Frank, if you tested this, you can mark it sign off. An easy way to sign-off is using the sandboxes: https://wiki.koha-community.org/wiki/Sandboxes

If you can't use them, it's good to at least switch the status and let us know how you want the sign-off line to look like (name + email) if different from your bugzilla account.
Comment 8 ByWater Sandboxes 2019-05-14 10:35:11 UTC
Created attachment 89732 [details] [review]
Bug 21450: Cache authorities search using term and auth_type

This is a simple fix to prevent incorrect caching of results when using
link_bibs_to_authorities

To test:
1 - Find or create a record with the same term in a subject and genre
heading
2 - Find or create authority records for the term as a subject and genre
type
3 - Run link bibs to authorities
4 - Verify both tags in the record are linked to the subject heading
5 - Aply patch
6 - Unlink the record and re-run the script (or just re-run)
7 - Each tag should be linked to correct authority

Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se>
Comment 9 Liz Rea 2019-05-24 17:57:16 UTC
Created attachment 90076 [details] [review]
Bug 21450: Unit tests

To test:
1 - Apply just this patch
2 - prove -v t/db_dependent/Linker_Default.t
3 - It fails
4 - Apply second patch
5 - prove -v t/db_dependent/Linker_Default.t
6 - It passes

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 10 Liz Rea 2019-05-24 17:57:26 UTC
Created attachment 90077 [details] [review]
Bug 21450: Cache authorities search using term and auth_type

This is a simple fix to prevent incorrect caching of results when using
link_bibs_to_authorities

To test:
1 - Find or create a record with the same term in a subject and genre
heading
2 - Find or create authority records for the term as a subject and genre
type
3 - Run link bibs to authorities
4 - Verify both tags in the record are linked to the subject heading
5 - Aply patch
6 - Unlink the record and re-run the script (or just re-run)
7 - Each tag should be linked to correct authority

Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se>

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 11 Liz Rea 2019-05-24 17:57:29 UTC
Created attachment 90078 [details] [review]
Bug 21450: (QA follow-up) remove a couple of unused variables to make QA script happy
Comment 12 Jonathan Druart 2019-05-24 20:35:18 UTC
Created attachment 90088 [details] [review]
Bug 21450: Unit tests

To test:
1 - Apply just this patch
2 - prove -v t/db_dependent/Linker_Default.t
3 - It fails
4 - Apply second patch
5 - prove -v t/db_dependent/Linker_Default.t
6 - It passes

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Jonathan Druart 2019-05-24 20:35:26 UTC
Created attachment 90089 [details] [review]
Bug 21450: Cache authorities search using term and auth_type

This is a simple fix to prevent incorrect caching of results when using
link_bibs_to_authorities

To test:
1 - Find or create a record with the same term in a subject and genre
heading
2 - Find or create authority records for the term as a subject and genre
type
3 - Run link bibs to authorities
4 - Verify both tags in the record are linked to the subject heading
5 - Aply patch
6 - Unlink the record and re-run the script (or just re-run)
7 - Each tag should be linked to correct authority

Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se>

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 14 Jonathan Druart 2019-05-24 20:35:32 UTC
Created attachment 90090 [details] [review]
Bug 21450: (QA follow-up) remove a couple of unused variables to make QA script happy

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 15 Jonathan Druart 2019-05-24 20:38:37 UTC
Nick, the Copyright of the tests need to be updated before push.

Also I think that $h->{"$a$b"} is more readable than $h->{$a.$b}, but it can be debatable I guess :)
Comment 16 Nick Clemens 2019-05-30 10:33:03 UTC
Awesome work all!

Pushed to master for 19.05
Comment 17 Nick Clemens 2019-05-30 10:33:29 UTC
(In reply to Jonathan Druart from comment #15)
> Nick, the Copyright of the tests need to be updated before push.

Done, removed

> 
> Also I think that $h->{"$a$b"} is more readable than $h->{$a.$b}, but it can
> be debatable I guess :)

file a bug ;-P
Comment 18 Martin Renvoize 2019-05-30 20:25:22 UTC
Pushed to 18.11.x for 18.11.06