Bug 36798 - Add ability to search across all ISBNs using the ISBN-search
Summary: Add ability to search across all ISBNs using the ISBN-search
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Janusz Kaczmarek
QA Contact: Lucas Gass (lukeg)
URL:
Keywords:
Depends on: 36574 36727
Blocks:
  Show dependency treegraph
 
Reported: 2024-05-07 08:35 UTC by Janusz Kaczmarek
Modified: 2024-11-22 22:16 UTC (History)
3 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
The new system preference `SearchCancelledAndInvalidISBNandISSN` allows to include invalid and cancelled ISBNs (MARC21 020/022 $z) in searches for ISBN and ISSN.
Version(s) released in:
24.11.00
Circulation function:


Attachments
Bug 36798: Add 'SearchCancelAndInvalidISBNandISSN' system preference (4.10 KB, patch)
2024-05-16 14:01 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 36798: Add ability to search across all ISBNs using the ISBN-search (3.42 KB, patch)
2024-05-16 14:01 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 36798: Unit tests (2.14 KB, patch)
2024-05-16 14:01 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 36798: Add 'SearchCancelAndInvalidISBNandISSN' system preference (4.15 KB, patch)
2024-05-16 17:52 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 36798: Add ability to search across all ISBNs using the ISBN-search (3.47 KB, patch)
2024-05-16 17:53 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 36798: Unit tests (2.19 KB, patch)
2024-05-16 17:53 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 36798: Add 'SearchCancelledAndInvalidISBNandISSN' system preference (4.16 KB, patch)
2024-09-23 22:57 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 36798: Add ability to search across all ISBNs using the ISBN-search (3.48 KB, patch)
2024-09-23 22:57 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 36798: Unit tests (2.19 KB, patch)
2024-09-23 22:57 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 36798: Add 'SearchCancelledAndInvalidISBNandISSN' system preference (4.22 KB, patch)
2024-09-24 13:51 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36798: Add ability to search across all ISBNs using the ISBN-search (3.54 KB, patch)
2024-09-24 13:51 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36798: Unit tests (2.24 KB, patch)
2024-09-24 13:51 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Janusz Kaczmarek 2024-05-07 08:35:17 UTC
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.
Comment 1 Janusz Kaczmarek 2024-05-16 14:01:27 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).
Comment 2 Janusz Kaczmarek 2024-05-16 14:01:29 UTC
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
Comment 3 Janusz Kaczmarek 2024-05-16 14:01:31 UTC
Created attachment 166830 [details] [review]
Bug 36798: Unit tests

Added unit tests.
Comment 4 Roman Dolny 2024-05-16 17:52:55 UTC
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>
Comment 5 Roman Dolny 2024-05-16 17:53:01 UTC
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>
Comment 6 Roman Dolny 2024-05-16 17:53:06 UTC
Created attachment 166838 [details] [review]
Bug 36798: Unit tests

Added unit tests.

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Comment 7 Lucas Gass (lukeg) 2024-09-23 22:21:18 UTC
Sorry Janusz, this no longer applies cleanly. Good new, seems like a simple rebase of searching.pref.
Comment 8 Janusz Kaczmarek 2024-09-23 22:57:10 UTC
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>
Comment 9 Janusz Kaczmarek 2024-09-23 22:57:12 UTC
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>
Comment 10 Janusz Kaczmarek 2024-09-23 22:57:15 UTC
Created attachment 171908 [details] [review]
Bug 36798: Unit tests

Added unit tests.

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Comment 11 Janusz Kaczmarek 2024-09-23 23:00:29 UTC
(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.
Comment 12 Lucas Gass (lukeg) 2024-09-24 13:51:34 UTC
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>
Comment 13 Lucas Gass (lukeg) 2024-09-24 13:51:37 UTC
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>
Comment 14 Lucas Gass (lukeg) 2024-09-24 13:51:40 UTC
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>
Comment 15 Katrin Fischer 2024-10-21 10:27:15 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 16 Katrin Fischer 2024-10-21 11:14:06 UTC
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?
Comment 17 Katrin Fischer 2024-11-22 10:34:10 UTC
(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*
Comment 18 Janusz Kaczmarek 2024-11-22 17:14:32 UTC
(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?
Comment 19 Katrin Fischer 2024-11-22 17:32:01 UTC
(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.
Comment 20 Janusz Kaczmarek 2024-11-22 17:37:08 UTC
(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).
Comment 21 Janusz Kaczmarek 2024-11-22 22:16:34 UTC
(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.