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
Created attachment 79600 [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 FIXME: This needs unit tests 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
> FIXME: This needs unit tests Does that explain the NEW status ?
(In reply to Marcel de Rooy from comment #2) > > FIXME: This needs unit tests > > Does that explain the NEW status ? Indeed, I will try to get to this, but anyone is welcome to take it on
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
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
Nice job! This patch applies and works fine for me.
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.
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>
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>
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>
Created attachment 90078 [details] [review] Bug 21450: (QA follow-up) remove a couple of unused variables to make QA script happy
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>
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>
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>
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 :)
Awesome work all! Pushed to master for 19.05
(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
Pushed to 18.11.x for 18.11.06