|
Description
Janusz Kaczmarek
2024-05-07 08:35:17 UTC
Created attachment 166828 [details] [review] Bug 36798: Add 'SearchCancelAndInvalidISBNandISSN' system preference This patch adds a new system preference SearchCancelAndInvalidISBNandISSN: whether to search for cancelled / invalid forms of ISBN/ISSN when performing ISBN/ISSN search. (By default, with ES, only valid forms, i.e. 020 $a / 022 $a are considered). Created attachment 166829 [details] [review] Bug 36798: Add ability to search across all ISBNs using the ISBN-search With the Bug 36574 we added a new ES search field isbn-all which makes it possible to find records with the content of the subfield 020 $z (invalid/cancelled ISBN) in general search (by keyword). But still, when performing a search with ISBN-search these records will not be found. Since some libraries deliberately want to find records with only 020 $a with ISBN-search, a system preference would be needed here to be able to customize the ISBN search. The same for ISSNs, with one system preference. Test plan: ========== (ISBN) ------ 1. Have the ktd with with ES enabled. 2. With standard ktd test data, try to perform a ISBN search from Advanced search for 9780141930848. You should get no results. In the general search input field (on the main page) try a search nb:9780141930848 and/or isbn:9780141930848 - you should get no results. 3. Apply the patch, restart_all. 4. Repeat p. 2 -- you should still get no results. 5. Enable SearchCancelAndInvalidISBNandISSN syspref. 6. Repeat p. 2 -- now you should get one record (having 9780141930848 in 020 $z). (ISSN) ------ 0. Return to main, delete the bug branch, reset_all == start from fresh. 1. Have the ktd with with ES enabled. 2. With standard ktd test data, try to perform a ISSN search from Advanced search for 1089-6891. You should get no results. In the general search input field (on the main page) try a search ns:1089-6891 and/or issn:1089-6891 - you should get no results. 3. Apply the patch, restart_all. If you have had to apply also the bug 36727, reindex ES with: koha-elasticsearch --rebuild -r -d kohadev (there is a modification of mappings.yaml) 4. Repeat p. 2 -- you should still get no results. 5. Enable SearchCancelAndInvalidISBNandISSN syspref. 6. Repeat p. 2 -- now you should get one record (having 1089-6891 in 022 $y). Sponsored-by: Ignatianum University in Cracow Created attachment 166830 [details] [review] Bug 36798: Unit tests Added unit tests. Created attachment 166836 [details] [review] Bug 36798: Add 'SearchCancelAndInvalidISBNandISSN' system preference This patch adds a new system preference SearchCancelAndInvalidISBNandISSN: whether to search for cancelled / invalid forms of ISBN/ISSN when performing ISBN/ISSN search. (By default, with ES, only valid forms, i.e. 020 $a / 022 $a are considered). Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Created attachment 166837 [details] [review] Bug 36798: Add ability to search across all ISBNs using the ISBN-search With the Bug 36574 we added a new ES search field isbn-all which makes it possible to find records with the content of the subfield 020 $z (invalid/cancelled ISBN) in general search (by keyword). But still, when performing a search with ISBN-search these records will not be found. Since some libraries deliberately want to find records with only 020 $a with ISBN-search, a system preference would be needed here to be able to customize the ISBN search. The same for ISSNs, with one system preference. Test plan: ========== (ISBN) ------ 1. Have the ktd with with ES enabled. 2. With standard ktd test data, try to perform a ISBN search from Advanced search for 9780141930848. You should get no results. In the general search input field (on the main page) try a search nb:9780141930848 and/or isbn:9780141930848 - you should get no results. 3. Apply the patch, restart_all. 4. Repeat p. 2 -- you should still get no results. 5. Enable SearchCancelAndInvalidISBNandISSN syspref. 6. Repeat p. 2 -- now you should get one record (having 9780141930848 in 020 $z). (ISSN) ------ 0. Return to main, delete the bug branch, reset_all == start from fresh. 1. Have the ktd with with ES enabled. 2. With standard ktd test data, try to perform a ISSN search from Advanced search for 1089-6891. You should get no results. In the general search input field (on the main page) try a search ns:1089-6891 and/or issn:1089-6891 - you should get no results. 3. Apply the patch, restart_all. If you have had to apply also the bug 36727, reindex ES with: koha-elasticsearch --rebuild -r -d kohadev (there is a modification of mappings.yaml) 4. Repeat p. 2 -- you should still get no results. 5. Enable SearchCancelAndInvalidISBNandISSN syspref. 6. Repeat p. 2 -- now you should get one record (having 1089-6891 in 022 $y). Sponsored-by: Ignatianum University in Cracow Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Created attachment 166838 [details] [review] Bug 36798: Unit tests Added unit tests. Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Sorry Janusz, this no longer applies cleanly. Good new, seems like a simple rebase of searching.pref. Created attachment 171906 [details] [review] Bug 36798: Add 'SearchCancelledAndInvalidISBNandISSN' system preference This patch adds a new system preference SearchCancelledAndInvalidISBNandISSN: whether to search for cancelled / invalid forms of ISBN/ISSN when performing ISBN/ISSN search. (By default, with ES, only valid forms, i.e. 020 $a / 022 $a are considered). Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Created attachment 171907 [details] [review] Bug 36798: Add ability to search across all ISBNs using the ISBN-search With the Bug 36574 we added a new ES search field isbn-all which makes it possible to find records with the content of the subfield 020 $z (invalid/cancelled ISBN) in general search (by keyword). But still, when performing a search with ISBN-search these records will not be found. Since some libraries deliberately want to find records with only 020 $a with ISBN-search, a system preference would be needed here to be able to customize the ISBN search. The same for ISSNs, with one system preference. Test plan: ========== (ISBN) ------ 1. Have the ktd with with ES enabled. 2. With standard ktd test data, try to perform a ISBN search from Advanced search for 9780141930848. You should get no results. In the general search input field (on the main page) try a search nb:9780141930848 and/or isbn:9780141930848 - you should get no results. 3. Apply the patch, restart_all. 4. Repeat p. 2 -- you should still get no results. 5. Enable SearchCancelledAndInvalidISBNandISSN syspref. 6. Repeat p. 2 -- now you should get one record (having 9780141930848 in 020 $z). (ISSN) ------ 0. Return to main, delete the bug branch, reset_all == start from fresh. 1. Have the ktd with with ES enabled. 2. With standard ktd test data, try to perform a ISSN search from Advanced search for 1089-6891. You should get no results. In the general search input field (on the main page) try a search ns:1089-6891 and/or issn:1089-6891 - you should get no results. 3. Apply the patch, restart_all. If you have had to apply also the bug 36727, reindex ES with: koha-elasticsearch --rebuild -r -d kohadev (there is a modification of mappings.yaml) 4. Repeat p. 2 -- you should still get no results. 5. Enable SearchCancelledAndInvalidISBNandISSN syspref. 6. Repeat p. 2 -- now you should get one record (having 1089-6891 in 022 $y). Sponsored-by: Ignatianum University in Cracow Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Created attachment 171908 [details] [review] Bug 36798: Unit tests Added unit tests. Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> (In reply to Lucas Gass from comment #7) > Sorry Janusz, this no longer applies cleanly. Good new, seems like a simple > rebase of searching.pref. Thanks, Lucas. Sure--it has been four months since the patch was ready, so no wonder... Rebased + tiny spelling correction. Created attachment 171935 [details] [review] Bug 36798: Add 'SearchCancelledAndInvalidISBNandISSN' system preference This patch adds a new system preference SearchCancelledAndInvalidISBNandISSN: whether to search for cancelled / invalid forms of ISBN/ISSN when performing ISBN/ISSN search. (By default, with ES, only valid forms, i.e. 020 $a / 022 $a are considered). Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Created attachment 171936 [details] [review] Bug 36798: Add ability to search across all ISBNs using the ISBN-search With the Bug 36574 we added a new ES search field isbn-all which makes it possible to find records with the content of the subfield 020 $z (invalid/cancelled ISBN) in general search (by keyword). But still, when performing a search with ISBN-search these records will not be found. Since some libraries deliberately want to find records with only 020 $a with ISBN-search, a system preference would be needed here to be able to customize the ISBN search. The same for ISSNs, with one system preference. Test plan: ========== (ISBN) ------ 1. Have the ktd with with ES enabled. 2. With standard ktd test data, try to perform a ISBN search from Advanced search for 9780141930848. You should get no results. In the general search input field (on the main page) try a search nb:9780141930848 and/or isbn:9780141930848 - you should get no results. 3. Apply the patch, restart_all. 4. Repeat p. 2 -- you should still get no results. 5. Enable SearchCancelledAndInvalidISBNandISSN syspref. 6. Repeat p. 2 -- now you should get one record (having 9780141930848 in 020 $z). (ISSN) ------ 0. Return to main, delete the bug branch, reset_all == start from fresh. 1. Have the ktd with with ES enabled. 2. With standard ktd test data, try to perform a ISSN search from Advanced search for 1089-6891. You should get no results. In the general search input field (on the main page) try a search ns:1089-6891 and/or issn:1089-6891 - you should get no results. 3. Apply the patch, restart_all. If you have had to apply also the bug 36727, reindex ES with: koha-elasticsearch --rebuild -r -d kohadev (there is a modification of mappings.yaml) 4. Repeat p. 2 -- you should still get no results. 5. Enable SearchCancelledAndInvalidISBNandISSN syspref. 6. Repeat p. 2 -- now you should get one record (having 1089-6891 in 022 $y). Sponsored-by: Ignatianum University in Cracow Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Created attachment 171937 [details] [review] Bug 36798: Unit tests Added unit tests. Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Pushed for 24.11! Well done everyone, thank you! I have to admit when we talked about this at the hackfest, I was thinking about a slightly different solution, more in line with how the system preferences *NumbersPreferPhrase work. They work on the GUI level, changing the index used in the drop-downs and search options. With this patch, whenever I use the index "isbn" in "Expert search" it will also be turned into isbn-all, if I understand how this works correctly. Can you confirm? (In reply to Katrin Fischer from comment #16) > I have to admit when we talked about this at the hackfest, I was thinking > about a slightly different solution, more in line with how the system > preferences *NumbersPreferPhrase work. They work on the GUI level, changing > the index used in the drop-downs and search options. > > With this patch, whenever I use the index "isbn" in "Expert search" it will > also be turned into isbn-all, if I understand how this works correctly. Can > you confirm? *ping* (In reply to Katrin Fischer from comment #17) > (In reply to Katrin Fischer from comment #16) > > I have to admit when we talked about this at the hackfest, I was thinking > > about a slightly different solution, more in line with how the system > > preferences *NumbersPreferPhrase work. They work on the GUI level, changing > > the index used in the drop-downs and search options. > > > > With this patch, whenever I use the index "isbn" in "Expert search" it will > > also be turned into isbn-all, if I understand how this works correctly. Can > > you confirm? > > *ping* I am sorry I missed this. I am a bit lost. My intention and understanding is that the patch works exactly as you described for *NumbersPreferPhrase. If SearchCancelledAndInvalidISBNandISSN is on, the patch adds '-all' to isbn/issn search-field-name (cf. corrections in QueryBuilder.pm), thus changing the search-field (in Zebra-terms you would say 'index', but since it is only for ES 'search-field' is better ; 'index' is completely different thing in ES). Are you experiencing different behavior? (In reply to Janusz Kaczmarek from comment #18) > (In reply to Katrin Fischer from comment #17) > > (In reply to Katrin Fischer from comment #16) > > > I have to admit when we talked about this at the hackfest, I was thinking > > > about a slightly different solution, more in line with how the system > > > preferences *NumbersPreferPhrase work. They work on the GUI level, changing > > > the index used in the drop-downs and search options. > > > > > > With this patch, whenever I use the index "isbn" in "Expert search" it will > > > also be turned into isbn-all, if I understand how this works correctly. Can > > > you confirm? > > > > *ping* > > I am sorry I missed this. > > I am a bit lost. My intention and understanding is that the patch works > exactly as you described for *NumbersPreferPhrase. If > SearchCancelledAndInvalidISBNandISSN is on, the patch adds '-all' to > isbn/issn search-field-name (cf. corrections in QueryBuilder.pm), thus > changing the search-field (in Zebra-terms you would say 'index', but since > it is only for ES 'search-field' is better ; 'index' is completely different > thing in ES). > > Are you experiencing different behavior? It works, but the implementation is very different. With *NumbersPreferPhrase we only change templates and use another index when a specific search option is selected. Your patch works on a different level. :) But it's ok, we leave it like that. (In reply to Janusz Kaczmarek from comment #18) > Are you experiencing different behavior? I have checked it (in both interfaces). It works as described for ISBN and ISSN. At the same time 'Standard number' searches always across all (also cancelled) ISBN/ISSN. But this is another story--this was Bug 36574 (for ISBN). (In reply to Katrin Fischer from comment #19) > It works, but the implementation is very different. With > *NumbersPreferPhrase we only change templates and use another index when a > specific search option is selected. Your patch works on a different level. > :) But it's ok, we leave it like that. Yes, but the situation here is different. In Zebra since always both valid and invalid ISBN was covered with ISBN (@attr 1=7) index. In ES, initially, I wanted to have the same, to have ES imitate the 'classic' Zebra behavior--the implementation would be simpler (just a correction to mappings.yaml). But then there was this discussion at Hackfest ("some libraries deliberately want to find records with only 020 $a with ISBN-search"): it was about ES, nobody wanted to touch Zebra. Hence this implementation. |