https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |30280 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30280 [Bug 30280] Support authority records with common subject headings from different thesaurus -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 147881 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147881&action=edit Bug 33159: Unit tests -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 Phil Ringnalda <phil@chetcolibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |phil@chetcolibrary.org --- Comment #3 from Phil Ringnalda <phil@chetcolibrary.org> --- I'd like to say this works, because any progress is some progress, but... Without the patch, I set the prefs, remove the $9 from both the author and the subject in bib 3 and save, the subject is relinked to authority 7 With the patch, I set the prefs, remove the $9 from both the author and the subject and save, the subject is linked to a newly created exact duplicate authority, as is the author. Bonus fun (which is probably from bug 33100 being even more broken than we thought, though the patch there doesn't fix it) if I save again the author gets a new authority and a relink to it every time, and if I add the period to the subject which should be there according to punctuation rules, then it gets a relink to a new authority every time too. Even ignoring punctuation issues, it looks like this patch would fix linking to non-subjects, but at the cost of autocreating a new duplicate record for every record. Which means for the non-autocreate case, it would break auto-linking for everything, instead of just for non-subjects. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=32916 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 --- Comment #5 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 147970 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147970&action=edit Bug 33159: Simplify ES handling and fix zebra handling Before this patch we used two indexes for the thesaurus values, we can simply index both needed fields into a single index and just form the search correctly. This patch also ensures we pass the 'thesaurus' vlaue for the heading directly to the query builder - for zebra it goes through, and for ES we convert it to the expected code. This patch also moves the necessary mappings out of the user definable mappings and hardcodes them. There is precedent for this with 'match-heading', it ensures matching works as expected To test: 1 - Follow previous test plan in Zebra and ES -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 ByWater Sandboxes <bws.sandboxes@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #147970|0 |1 is obsolete| | --- Comment #6 from ByWater Sandboxes <bws.sandboxes@gmail.com> --- Created attachment 147980 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147980&action=edit Bug 33159: Simplify ES handling and fix zebra handling Before this patch we used two indexes for the thesaurus values, we can simply index both needed fields into a single index and just form the search correctly. This patch also ensures we pass the 'thesaurus' vlaue for the heading directly to the query builder - for zebra it goes through, and for ES we convert it to the expected code. This patch also moves the necessary mappings out of the user definable mappings and hardcodes them. There is precedent for this with 'match-heading', it ensures matching works as expected To test: 1 - Follow previous test plan in Zebra and ES Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 --- Comment #7 from Phil Ringnalda <phil@chetcolibrary.org> --- Probably just an artifact of how sandboxes are set up, but for Zebra in a sandbox I had to do a full Zebra reindex before name authorities worked correctly. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 Phil Ringnalda <phil@chetcolibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #147882|0 |1 is obsolete| | --- Comment #9 from Phil Ringnalda <phil@chetcolibrary.org> --- Created attachment 147987 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147987&action=edit Bug 33159: Correctly form thesaurus term for non-subject headings Bug 30280 fixed things for subject heading, but didn't take into account that all non-subject headings are considered LCSH - this means we can't really handle different thesauri for controlled headings outside of subjects - maybe a topic for a new bug, but we should fix matching for them now To test: 1 - Set sysprefs: RequireChoosingExistingAuthority - don't require AutoCreateAuthorities - generate CatalogModuleRelink - Do AutoLinkBiblio - Do LinkerModule - first match 2 - Find a record with an author attached to an authority, I used biblionumber 3 "Introduction to Attic Greek" 3 - Edit in advanced editor 4 - Delete the $9 link 5 - Save the record 6 - It generates a new number 7 - Repeat 4-5, it does it again 8 - Apply patch and restart all 9 - Repeat 4-5 10 - It links to the original authority Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 --- Comment #13 from Frank Hansen <frank.hansen@ub.lu.se> --- (In reply to Nick Clemens from comment #12)

(In reply to Frank Hansen from comment #11)

...

(In reply to Nick Clemens from comment #5)

...

Created attachment 147970 [details] [review] [review] [review] Bug 33159: Simplify ES handling and fix zebra handling

It seems to work with Zebra now. But for ES, after resetting the mappings and re-index (recreating) the indexes, linking doesn't work anymore. I notices that both Subject-heading-thesaurus and Subject-heading-thesaurus-conventions index was removed from mappings.yaml. Was that a mistake? I thought you want want a single index for both 008/_11

They are removed from the mappings, and moved into Koha::SearchEngine::Elasticsearch - they are hardcoded to always be present, so it should work after recreating the indexes - I will retest on my end

Ah, I gave it another try and Subject-heading-thesaurus is actually created. I think its ok then. Regards, Frank -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |33100 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33100 [Bug 33100] Authority linking doesn't work for bib headings ending in two or more punctuation characters -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 --- Comment #15 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 148033 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=148033&action=edit Bug 33159: Correctly form thesaurus term for non-subject headings Bug 30280 fixed things for subject heading, but didn't take into account that all non-subject headings are considered LCSH - this means we can't really handle different thesauri for controlled headings outside of subjects - maybe a topic for a new bug, but we should fix matching for them now To test: 1 - Set sysprefs: RequireChoosingExistingAuthority - don't require AutoCreateAuthorities - generate CatalogModuleRelink - Do AutoLinkBiblio - Do LinkerModule - first match 2 - Find a record with an author attached to an authority, I used biblionumber 3 "Introduction to Attic Greek" 3 - Edit in advanced editor 4 - Delete the $9 link 5 - Save the record 6 - It generates a new number 7 - Repeat 4-5, it does it again 8 - Apply patch and restart all 9 - Repeat 4-5 10 - It links to the original authority Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 Frank Hansen <frank.hansen@ub.lu.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #148032|0 |1 is obsolete| | Attachment #148033|0 |1 is obsolete| | Attachment #148034|0 |1 is obsolete| | --- Comment #17 from Frank Hansen <frank.hansen@ub.lu.se> --- Created attachment 148050 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=148050&action=edit Bug 33159: Unit tests Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 --- Comment #19 from Frank Hansen <frank.hansen@ub.lu.se> --- Created attachment 148052 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=148052&action=edit Bug 33159: Simplify ES handling and fix zebra handling Before this patch we used two indexes for the thesaurus values, we can simply index both needed fields into a single index and just form the search correctly. This patch also ensures we pass the 'thesaurus' vlaue for the heading directly to the query builder - for zebra it goes through, and for ES we convert it to the expected code. This patch also moves the necessary mappings out of the user definable mappings and hardcodes them. There is precedent for this with 'match-heading', it ensures matching works as expected To test: 1 - Follow previous test plan in Zebra and ES Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|33100 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33100 [Bug 33100] Authority linking doesn't work for bib headings ending in two or more punctuation characters -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #148050|0 |1 is obsolete| | --- Comment #20 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 148227 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=148227&action=edit Bug 33159: Unit tests Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #148052|0 |1 is obsolete| | --- Comment #22 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 148229 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=148229&action=edit Bug 33159: Simplify ES handling and fix zebra handling Before this patch we used two indexes for the thesaurus values, we can simply index both needed fields into a single index and just form the search correctly. This patch also ensures we pass the 'thesaurus' vlaue for the heading directly to the query builder - for zebra it goes through, and for ES we convert it to the expected code. This patch also moves the necessary mappings out of the user definable mappings and hardcodes them. There is precedent for this with 'match-heading', it ensures matching works as expected To test: 1 - Follow previous test plan in Zebra and ES Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 --- Comment #25 from Janusz Kaczmarek <januszop@gmail.com> --- Here some more details, what and why goes wrong: 100 1 $a Shakespeare, William -- It does not make sense to include in the query (and check) subject-heading-thesaurus, since we are looking for auth for 100, not for 6XX (It would have sense to check 008/14 of the auth. rec., but this is another story) 655 4 $a Literatura światowa --For ind2 == 4 we should look for an auth with 008/11 == ‘|’. In the proposal the search is being performed for subject-heading-thesaurus = 'n', which is wrong (008/11 = n is used in auth when "1XX heading does not conform to subject heading system/thesaurus conventions and is not appropriate for use as a subject added entry in bibliographic records" -- that is non intended for use in 6XX, would have 008/15 = 'b') 655 7 $a Literatura angielska $2 DBN -- It is correct to look for auth subject-heading-thesaurus = 'DBN', but in case of failure the second search should be performed with subject-heading-thesaurus = 'z'. This is because 040 $f is not obligatory for 008/11 = 'z' (according to MARC 21: "A MARC code for the conventions used to formulate the heading MAY be contained in subfield $f (Subject heading/thesaurus conventions) in field 040 (Cataloging Source)". If there is only one thesaurus present in the system, it is legal not to have 040 $f in the auth recs., having $2 in 6XX in biblio. 700 12 $a Shakespeare, William. $t Othello -- Same as for 100 -- no sense of querying for subject-heading-thesaurus. 830 0 $a Great Books for Busy People -- Same as for 100 -- no sense of querying for subject-heading-thesaurus. I have not enough courage to remove the status 'Passed QA' but IMHO the patch should be reviewed again. Otherwise we would have to patch the patch right after its application. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 --- Comment #27 from Phil Ringnalda <phil@chetcolibrary.org> --- *** Bug 31924 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 --- Comment #28 from Janusz Kaczmarek <januszop@gmail.com> --- (In reply to Phil Ringnalda from comment #26)

There is sense in looking for a thesaurus for 100, 700, and 830, because they share authority records with 600 and 630. If you don't look for a thesaurus to decide between two or more Shakespeare, William authority records, then the default linker says "oops, more than one result, no link for you" (and worse yet, first match or last match could decide to link to the authority record for a subject thesaurus that you could later decide you didn't want to use, and when you deleted the authority record you would not only delete the subject headings, you would also delete the author headings).

Phil, thank you for your comment. Maybe what you are saying is true in some contexts (US?), but in others -- not. Here is a real authority record for William Shakespeare from NLPol: https://dbn.bn.org.pl/descriptor-details/a0000001180305. You will not find it in Koha when unnecessarily enriching the query for the main heading with subject-heading-thesaurus = 'a', because it has nothing to do with LCSH. If we really wanted to narrow the query in this case, we should enrich it with Heading-use-main-or-added-entry = 'a'. And yes, this auth record it is shared for the use in 600 (008/15 = 'a'), but the thesaurus is not LCSH but DBN (note, that it is not explicitly declared in 040 $f, which is correct). So even if in some (not all) cases the auth record you are looking for for your main or added entry (1XX/7XX) was shared with 6XX, from the heading field of biblio you cannot state what thesaurus is would be. It is not prudent to assume that it is definitely a LCSH what is made now.

This bug is about making what bug 30280 tried to do actually work, a narrow enough scope that is has made it to Passed QA and is ready to land.

"Bug 30280 shouldn't have mapped ind2=4 to 008/11=n" and "Bug 30280 should have treated 008/11=z with no 040$f as a wildcard authority that matches any value of $2 when no other authority with a match in 040$f exists" are both legitimate other followup bugs (as is "Bug 30280 should have put its new behavior behind a preference that defaults to off"), but they aren't part of making what it tried to do actually work.

IMHO, if we are trying to fix the problems introduced by Bug 30280, it should be done with possibly complete and universal set of test data. Of course one can always prepare test data that pass the test, but in real life the application will fail for the people. Now the linking is not working for some libraries (before 30280 it was not perfect, but was acceptable in most cases), and it will still not work after applying this patch. So what would be the point to introduce a patch that is known to be problematic, to put it mildly... -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 --- Comment #29 from Nick Clemens <nick@bywatersolutions.com> --- I filed bug 33277 - I think we can require that one before pushing, but still work on a new bug, expanding the tests and code there, but agreeing on this base as it solves the problem for some cases. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 --- Comment #31 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 148415 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=148415&action=edit Bug 33159: Correctly form thesaurus term for non-subject headings Bug 30280 fixed things for subject heading, but didn't take into account that all non-subject headings are considered LCSH - this means we can't really handle different thesauri for controlled headings outside of subjects - maybe a topic for a new bug, but we should fix matching for them now To test: 1 - Set sysprefs: RequireChoosingExistingAuthority - don't require AutoCreateAuthorities - generate CatalogModuleRelink - Do AutoLinkBiblio - Do LinkerModule - first match 2 - Find a record with an author attached to an authority, I used biblionumber 3 "Introduction to Attic Greek" 3 - Edit in advanced editor 4 - Delete the $9 link 5 - Save the record 6 - It generates a new number 7 - Repeat 4-5, it does it again 8 - Apply patch and restart all 9 - Repeat 4-5 10 - It links to the original authority Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 --- Comment #33 from Janusz Kaczmarek <januszop@gmail.com> --- (In reply to Nick Clemens from comment #29)

I filed bug 33277 - I think we can require that one before pushing, but still work on a new bug, expanding the tests and code there, but agreeing on this base as it solves the problem for some cases.

Nick, OK, I understand. But we have still remember about two other classes of problems: 1. 6XX _7 $a NNN $2 <thesaurus_code> but no 040 $f in auth rec. This is common and perfectly legal when is planed to have only one thesaurus in the system - like in data from NLPol, e.g.: http://data.bn.org.pl/api/institutions/authorities.marcxml?id=a0000001003606 -- the auth record is to be used in 6XX (note 008/15='a') with 2nd ind. = 7 (note 008/11 = 'z') and the use example: http://data.bn.org.pl/api/networks/bibs.marcxml?id=b0000003061976: <datafield ind1=" " ind2="7" tag="655"> <subfield code="a">Literatura angielska</subfield> <subfield code="2">DBN</subfield> </datafield> 2. 6XX _4 -- we should look for an auth rec. with 008/11 = '|', non 'n' ('n' means it is not, roughly speaking, 'it is not for 6XX', and more precisely: "is not appropriate for use as a subject added entry in bibliographic records"). You can have a look at the code proposed in Bug 32916 (where all the problems have been solved) and adapt it. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 --- Comment #35 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Janusz Kaczmarek from comment #33)

(In reply to Nick Clemens from comment #29)

...

I filed bug 33277 - I think we can require that one before pushing, but still work on a new bug, expanding the tests and code there, but agreeing on this base as it solves the problem for some cases.

Nick, OK, I understand. But we have still remember about two other classes of problems:

1. 6XX _7 $a NNN $2 <thesaurus_code>

but no 040 $f in auth rec. This is common and perfectly legal when is planed to have only one thesaurus in the system - like in data from NLPol, e.g.:

http://data.bn.org.pl/api/institutions/authorities.marcxml?id=a0000001003606 -- the auth record is to be used in 6XX (note 008/15='a') with 2nd ind. = 7 (note 008/11 = 'z')

and the use example: http://data.bn.org.pl/api/networks/bibs.marcxml?id=b0000003061976: <datafield ind1=" " ind2="7" tag="655"> <subfield code="a">Literatura angielska</subfield> <subfield code="2">DBN</subfield> </datafield>

2. 6XX _4 -- we should look for an auth rec. with 008/11 = '|', non 'n' ('n' means it is not, roughly speaking, 'it is not for 6XX', and more precisely: "is not appropriate for use as a subject added entry in bibliographic records").

You can have a look at the code proposed in Bug 32916 (where all the problems have been solved) and adapt it.

I addressed those as well as I could on 33277, please test. The second scenario requires larger work, but didn't work before 30280 either. While I agree on fixing all the problems we see, I think we should push this, work on 33277, and file more bugs if needed. It seems that 30280 has revealed many problems with our linking, and I love getting all the feedback here, but some of these issues predate 30280. Fixing each issue on its own will allow for easier testing, quicker fixes, and less likelihood of new breakages -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |23.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable Version(s)|23.05.00 |23.05.00,22.11.05 released in| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33159 Phil Ringnalda <phil@chetcolibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to stable |RESOLVED -- You are receiving this mail because: You are watching all bug changes.