Bug 9072 - Add CheckValidity option to Linker
Summary: Add CheckValidity option to Linker
Status: CLOSED INVALID
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P3 enhancement (vote)
Assignee: Fridolin Somers
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-14 11:09 UTC by Fridolin Somers
Modified: 2015-12-03 22:00 UTC (History)
2 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:


Attachments
Proposed patch (2.58 KB, patch)
2012-11-14 11:12 UTC, Fridolin Somers
Details | Diff | Splinter Review
Proposed patch (main) (5.66 KB, patch)
2012-11-16 08:44 UTC, Fridolin Somers
Details | Diff | Splinter Review
Proposed patch (follow-up 1) (2.22 KB, patch)
2012-11-16 08:45 UTC, Fridolin Somers
Details | Diff | Splinter Review
Proposed patch (follow-up 2) (644 bytes, patch)
2012-11-16 09:18 UTC, Fridolin Somers
Details | Diff | Splinter Review
Proposed patch (main) (6.07 KB, patch)
2012-11-16 15:50 UTC, Fridolin Somers
Details | Diff | Splinter Review
Proposed patch (follow-up 1) (2.69 KB, patch)
2012-11-16 16:05 UTC, Fridolin Somers
Details | Diff | Splinter Review
Proposed patch (follow-up 2) (1.08 KB, patch)
2012-11-16 16:06 UTC, Fridolin Somers
Details | Diff | Splinter Review
Proposed patch (follow-up 3) (670 bytes, patch)
2012-11-16 16:06 UTC, Fridolin Somers
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2012-11-14 11:09:45 UTC
When linking biblio to authorities, matching authorities search is made with 'AuthidAsc' sort order hardcoded.

This patch allows to set sort order in LinkerOptions syspref (HeadingAsc, HeadingDsc, AuthidAsc or AuthidDsc).
Without option, sort order 'AuthidAsc' is used like previsously.

This is useful for FirstMatch or LastMatch linkers where results order is important.


PS :
This solves a problem with some headings.
ie : headings "History" and "History of art".
The linker search for "History" will return both headings.
With linker FistMatch and "HeadingAsc" sort order, the heading "History" will be first and so will match.
Comment 1 Fridolin Somers 2012-11-14 11:12:20 UTC Comment hidden (obsolete)
Comment 2 Fridolin Somers 2012-11-15 11:12:43 UTC
Comment on attachment 13442 [details] [review]
Proposed patch

Forget, patch not work.
Comment 3 Fridolin Somers 2012-11-16 08:39:32 UTC
I changed for a 'CheckValidity' option.
When present in LinkerOptions, the authorities found by linker will be checked for validity by comparing a to z subfields values between authority field and biblio field.

This is based on C4::Biblio->_check_valid_auth_link behavior. I could not use this method because like said in method comment, this method can not be moved and I could not add a dependency to C4::Biblio in C4::Linker::Default.
Comment 4 Fridolin Somers 2012-11-16 08:44:05 UTC Comment hidden (obsolete)
Comment 5 Fridolin Somers 2012-11-16 08:45:10 UTC Comment hidden (obsolete)
Comment 6 Fridolin Somers 2012-11-16 09:18:07 UTC Comment hidden (obsolete)
Comment 7 Fridolin Somers 2012-11-16 09:31:49 UTC
(In reply to comment #6)
> Created attachment 13483 [details] [review]
> Proposed patch (follow-up 2)
> 
> Small additional correction :
> GetAuthorizedHeading was missing in exported methods of C4::AuthoritiesMarc.

This export was present in Bug 7475 (Teach matching rules to handle authorities) but was removed, certainly unwanted, by Bug 8523 (Display auth hierarchies w/all marcflavours).
Comment 8 Fridolin Somers 2012-11-16 15:50:07 UTC
Created attachment 13487 [details] [review]
Proposed patch (main)

I revized patch for performance and for a more clear syntax.
Comment 9 Fridolin Somers 2012-11-16 15:54:46 UTC
(In reply to comment #8)
> Created attachment 13487 [details] [review]
> Proposed patch (main)
> 
> I revised patch for performance and for a more clear syntax.

Patch adding CheckValidity option.
+ Search for linked authorities can return more that 20 results, I set a max of 10000 (in C4::Heading->_search).
+ In C4::AuthoritiesMarc->GetAuthorizedHeading() : a 'k' was missing in a to z list.
Comment 10 Fridolin Somers 2012-11-16 16:05:22 UTC
Created attachment 13488 [details] [review]
Proposed patch (follow-up 1)

Perltidy formatting
Comment 11 Fridolin Somers 2012-11-16 16:06:16 UTC
Created attachment 13489 [details] [review]
Proposed patch (follow-up 2)

Avoid null for string compare in C4::Biblio::_check_valid_auth_link
Comment 12 Fridolin Somers 2012-11-16 16:06:56 UTC
Created attachment 13490 [details] [review]
Proposed patch (follow-up 3)

Missing method export in C4::AuthoritiesMarc
Comment 13 Bernardo Gonzalez Kriegel 2013-02-26 23:02:51 UTC
Hi, which is the test plan?
Comment 14 Owen Leonard 2013-03-19 00:38:50 UTC
(In reply to comment #13)
> Hi, which is the test plan?

I agree. Needs a test plan.
Comment 15 Fridolin Somers 2015-02-24 14:06:25 UTC
Is now invalid