Bug 23919

Summary: Make ISSN searchable with and without hyphen
Product: Koha Reporter: Katrin Fischer <katrin.fischer>
Component: SearchingAssignee: Slava Shishkin <slavashishkin>
Status: CLOSED FIXED QA Contact: Kyle M Hall <kyle>
Severity: enhancement    
Priority: P5 - low CC: caroline.cyr-la-rose, david, dcook, jonathan.druart, kyle, lucas, nugged, slavashishkin, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Medium patch
Documentation contact: Caroline Cyr La Rose Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/659
Text to go in the release notes:
This enhancement enables searching by ISSN without using hyphens and using a space instead of the hyphen. This works in the advanced search (staff interface and OPAC) and item search (staff interface). It is enabled using a new system preference, SearchWithISSNVariations, with two options - "don't search" (the default), and "search".
Version(s) released in:
22.11.00
Bug Depends on:    
Bug Blocks: 31278, 33514, 28269    
Attachments: Bug 23919: Add new system preference SearchWithISSNVariations
Bug 23919: Zebra search by ISSN variations
Bug 23919: Add tests for biblioItems search by ISBN and ISSN with variations (sysprefs SearchWithISBNVariations/SearchWithISSNVariations)
Bug 23919: Items search by ISBN variations and ISSN variations
Bug 23919: Add tests for Items search by ISBN and ISSN with variations (sysprefs SearchWithISBNVariations/SearchWithISSNVariations)
Bug 23919: Add new system preference SearchWithISSNVariations
Bug 23919: Zebra search by ISSN variations
Bug 23919: Add tests for biblioItems search by ISBN and ISSN with
Bug 23919: Items search by ISBN variations and ISSN variations
Bug 23919: Add tests for Items search by ISBN and ISSN with variations
Bug 23919: Add new system preference SearchWithISSNVariations
Bug 23919: Zebra search by ISSN variations
Bug 23919: Add tests for biblioItems search by ISBN and ISSN with
Bug 23919: Items search by ISBN variations and ISSN variations
Bug 23919: Add tests for Items search by ISBN and ISSN with variations
Bug 23919: Add new system preference SearchWithISSNVariations
Bug 23919: Zebra search by ISSN variations
Bug 23919: Add tests for biblioItems search by ISBN and ISSN with
Bug 23919: Items search by ISBN variations and ISSN variations
Bug 23919: Add tests for Items search by ISBN and ISSN with variations
Bug 23919: Add new system preference SearchWithISSNVariations
Bug 23919: Zebra search by ISSN variations
Bug 23919: Add tests for biblioItems search by ISBN and ISSN with
Bug 23919: Items search by ISBN variations and ISSN variations
Bug 23919: Add tests for Items search by ISBN and ISSN with variations
Bug 23919: (QA follow-up) Tidy query build code

Description Katrin Fischer 2019-10-29 16:42:44 UTC
For the ISBN we have the option to activate search with and without hyphens - but this doesn't work for ISSN. We shoudl make it work there as well.
Comment 1 Slava Shishkin 2022-04-29 13:31:52 UTC Comment hidden (obsolete)
Comment 2 Slava Shishkin 2022-04-29 13:31:56 UTC Comment hidden (obsolete)
Comment 3 Slava Shishkin 2022-04-29 13:32:00 UTC Comment hidden (obsolete)
Comment 4 Slava Shishkin 2022-04-29 13:32:04 UTC Comment hidden (obsolete)
Comment 5 Slava Shishkin 2022-04-29 13:32:09 UTC Comment hidden (obsolete)
Comment 6 Slava Shishkin 2022-04-29 20:06:54 UTC Comment hidden (obsolete)
Comment 7 Slava Shishkin 2022-04-29 20:06:59 UTC Comment hidden (obsolete)
Comment 8 Slava Shishkin 2022-04-29 20:07:03 UTC Comment hidden (obsolete)
Comment 9 Slava Shishkin 2022-04-29 20:07:08 UTC Comment hidden (obsolete)
Comment 10 Slava Shishkin 2022-04-29 20:07:13 UTC Comment hidden (obsolete)
Comment 11 Slava Shishkin 2022-05-01 15:08:40 UTC
Created attachment 134440 [details] [review]
Bug 23919: Add new system preference SearchWithISSNVariations
Comment 12 Slava Shishkin 2022-05-01 15:08:45 UTC
Created attachment 134441 [details] [review]
Bug 23919: Zebra search by ISSN variations
Comment 13 Slava Shishkin 2022-05-01 15:08:49 UTC
Created attachment 134442 [details] [review]
Bug 23919: Add tests for biblioItems search by ISBN and ISSN with

variations (sysprefs SearchWithISBNVariations/SearchWithISSNVariations)
Comment 14 Slava Shishkin 2022-05-01 15:08:54 UTC
Created attachment 134443 [details] [review]
Bug 23919: Items search by ISBN variations and ISSN variations
Comment 15 Slava Shishkin 2022-05-01 15:08:58 UTC
Created attachment 134444 [details] [review]
Bug 23919: Add tests for Items search by ISBN and ISSN with variations

(sysprefs SearchWithISBNVariations/SearchWithISSNVariations)
Comment 16 Lucas Gass 2022-05-18 21:42:42 UTC
Hello Slava,

Do you have a test plan we can use when trying this patchset?


Thanks!
Comment 17 Slava Shishkin 2022-05-23 10:03:14 UTC
Hello Lucas, sure.

1. SearchWithISSNVariations setting is missing in the system preference
2. Apply patch
3. Check that SearchWithISSNVariations is present in the system preferences
4. Check that SearchWithISSNVariations has "search" and "don't search" parameters
Comment 18 David Nind 2022-05-23 22:36:02 UTC
Here is an updated test plan I came up with - I'll add a sign-off shortly. I hope this covers all the changes and things to test for!

This enhancement enables searching by ISSN without using hyphens and using a space instead of the hyphen. This works in the advanced search (staff interface and OPAC) and item search (staff interface).

It is enabled using a new system preference, SearchWithISSNVariations, with two options - "don't search" (the default), and "search".

This is similar to the SearchWithISBNVariations system preference.

Test plan:
1. Search for an ISSN number with a hyphen, without a hyphen (no space), and with a space instead of the hyphen (example record: 1064-3923 (record #359 in test data for koha-testing-docker)):
   (a) standard search (staff inerface and OPAC) => result found with hyphen, not found if no hyphen or space used [note: no change after patch applied]
   (b) ISSN keyword field in the advanced search (staff interface and OPAC) => result found, not found if no hyphen or space used
   (c) item search in the staff interface (acces from Search drop down list in the navigation at the top of the page > Item search > field search - change to ISSN) => result found, not found if no hyphen or space used
2. Search for SearchWithISSNVariations in the system preferences search => no system preference found with that name.
3. Apply the patch, update the database (updatedatabase), flush_memcached, restart_all (I also reindexed - not sure whether this is necessary: koha-rebuild-zebra -f -v kohadev) (koha-testing-docker).
4. Search for the system preference SearchWith - system preference SearchWithISSNVariations found, with default setting set as "don't search".
5. Check that the text for the system preference reads well (it uses similar wording to the SearchWithISBNVariations system preference).
6. Repeat step 1 with SearchWithISSNVariations set as "don't search" => current behavior, that is, no results found if no hyphen or space used.
7. Repeat step 1 with SearchWithISSNVariations set as "search" => should now find the record with and without the hyphen and with a space instead of the hyphen for steps 1(b) and 1(c).
8. Run the tests - these should pass:
   . prove t/Search/buildQuery.t
   . prove t/db_dependent/Items.t
8. Sign off.

Notes:

- I think the QA Team will probably want the commit message(s?) to follow the guidelines for the descriptions (see https://wiki.koha-community.org/wiki/Commit_messages#Bug.2Ffeature_description)

- Maybe an additional enhancement (as a separate bug) would be to make this work when searching on the ISSN number in the serials search.
Comment 19 David Nind 2022-05-23 22:37:50 UTC
Created attachment 135288 [details] [review]
Bug 23919: Add new system preference SearchWithISSNVariations

Signed-off-by: David Nind <david@davidnind.com>
Comment 20 David Nind 2022-05-23 22:37:55 UTC
Created attachment 135289 [details] [review]
Bug 23919: Zebra search by ISSN variations

Signed-off-by: David Nind <david@davidnind.com>
Comment 21 David Nind 2022-05-23 22:38:00 UTC
Created attachment 135290 [details] [review]
Bug 23919: Add tests for biblioItems search by ISBN and ISSN with

variations (sysprefs SearchWithISBNVariations/SearchWithISSNVariations)

Signed-off-by: David Nind <david@davidnind.com>
Comment 22 David Nind 2022-05-23 22:38:05 UTC
Created attachment 135291 [details] [review]
Bug 23919: Items search by ISBN variations and ISSN variations

Signed-off-by: David Nind <david@davidnind.com>
Comment 23 David Nind 2022-05-23 22:38:10 UTC
Created attachment 135292 [details] [review]
Bug 23919: Add tests for Items search by ISBN and ISSN with variations

(sysprefs SearchWithISBNVariations/SearchWithISSNVariations)

Signed-off-by: David Nind <david@davidnind.com>
Comment 24 David Cook 2022-05-26 01:04:07 UTC
(In reply to Katrin Fischer from comment #0)
> For the ISBN we have the option to activate search with and without hyphens
> - but this doesn't work for ISSN. We shoudl make it work there as well.

Curious about this one...

Since Zebra normalizes the data by stripping out hyphens, it shouldn't be relevant for Zebra?
Comment 25 Katrin Fischer 2022-05-26 16:15:33 UTC
(In reply to David Cook from comment #24)
> (In reply to Katrin Fischer from comment #0)
> > For the ISBN we have the option to activate search with and without hyphens
> > - but this doesn't work for ISSN. We shoudl make it work there as well.
> 
> Curious about this one...
> 
> Since Zebra normalizes the data by stripping out hyphens, it shouldn't be
> relevant for Zebra?

IIRC if you search without the hyphen, you will not find the case with hyphen as it's treated as a space (or it might be in our installations, but it looks like we are not the only ones)
Comment 26 David Cook 2022-05-27 01:49:20 UTC
(In reply to Katrin Fischer from comment #25)
> IIRC if you search without the hyphen, you will not find the case with
> hyphen as it's treated as a space (or it might be in our installations, but
> it looks like we are not the only ones)

Ahhh right. My mistake. That makes sense.
Comment 27 Kyle M Hall 2022-07-29 13:23:29 UTC
Created attachment 138311 [details] [review]
Bug 23919: Add new system preference SearchWithISSNVariations

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 28 Kyle M Hall 2022-07-29 13:23:40 UTC
Created attachment 138312 [details] [review]
Bug 23919: Zebra search by ISSN variations

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 29 Kyle M Hall 2022-07-29 13:23:44 UTC
Created attachment 138313 [details] [review]
Bug 23919: Add tests for biblioItems search by ISBN and ISSN with

variations (sysprefs SearchWithISBNVariations/SearchWithISSNVariations)

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 30 Kyle M Hall 2022-07-29 13:23:47 UTC
Created attachment 138314 [details] [review]
Bug 23919: Items search by ISBN variations and ISSN variations

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 31 Kyle M Hall 2022-07-29 13:23:51 UTC
Created attachment 138315 [details] [review]
Bug 23919: Add tests for Items search by ISBN and ISSN with variations

(sysprefs SearchWithISBNVariations/SearchWithISSNVariations)

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 32 Tomás Cohen Arazi 2022-08-01 14:53:00 UTC
No mention of Elasticsearch in the test plan? Does this already work on Elasticsearch?
Comment 33 Katrin Fischer 2022-08-01 14:57:17 UTC
It's a search expansion, not an indexing change - searching for different variations. The search engine should not matter :)
Comment 34 Tomás Cohen Arazi 2022-08-01 16:56:54 UTC
Created attachment 138480 [details] [review]
Bug 23919: (QA follow-up) Tidy query build code

This is really opinionated, but I found this to be much cleaner to read
and thought it was worth pusing as well.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 35 Tomás Cohen Arazi 2022-08-01 17:11:11 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 36 Lucas Gass 2022-09-29 15:54:34 UTC
Enhancement will not be backported to 22.05.x
Comment 37 Katrin Fischer 2023-04-13 07:58:03 UTC
I am sad to say, but adding this to item search was not a great idea :(

Item search is SQL based, not a search engine, which changes things quite a bit.

We are exploding the search now with up to 4 forms of ISBN into something like:

isbn like 'isbn1' or isbn like 'isbn2' ...

But: We are not truncating. 

As soon as there is more than one ISBN or ISSN (these are repeatable fields) or the library actually follows MARC21 standard adding ISBD punctuation, this fails. We store in biblioitems.isbn like this:

isbn1 ; | isbn2 ; | isbn3.
isbn1.

This also seems to create quite a performance issue on big databases and it can't be turned off separately from the normal search.

I've filed bug 33514.
Comment 38 Jonathan Druart 2023-05-11 10:24:07 UTC
(In reply to Katrin Fischer from comment #37)
> I am sad to say, but adding this to item search was not a great idea :(
> 
> Item search is SQL based, not a search engine, which changes things quite a
> bit.
> 
> We are exploding the search now with up to 4 forms of ISBN into something
> like:
> 
> isbn like 'isbn1' or isbn like 'isbn2' ...
> 
> But: We are not truncating. 
> 
> As soon as there is more than one ISBN or ISSN (these are repeatable fields)
> or the library actually follows MARC21 standard adding ISBD punctuation,
> this fails. We store in biblioitems.isbn like this:
> 
> isbn1 ; | isbn2 ; | isbn3.
> isbn1.
> 
> This also seems to create quite a performance issue on big databases and it
> can't be turned off separately from the normal search.
> 
> I've filed bug 33514.

Can you please address bug 33514? In the meanwhile should we remove this?
Comment 39 Katrin Fischer 2023-05-14 15:35:13 UTC
(In reply to Jonathan Druart from comment #38)
> (In reply to Katrin Fischer from comment #37)
> > I am sad to say, but adding this to item search was not a great idea :(
> > 
> > Item search is SQL based, not a search engine, which changes things quite a
> > bit.
> > 
> > We are exploding the search now with up to 4 forms of ISBN into something
> > like:
> > 
> > isbn like 'isbn1' or isbn like 'isbn2' ...
> > 
> > But: We are not truncating. 
> > 
> > As soon as there is more than one ISBN or ISSN (these are repeatable fields)
> > or the library actually follows MARC21 standard adding ISBD punctuation,
> > this fails. We store in biblioitems.isbn like this:
> > 
> > isbn1 ; | isbn2 ; | isbn3.
> > isbn1.
> > 
> > This also seems to create quite a performance issue on big databases and it
> > can't be turned off separately from the normal search.
> > 
> > I've filed bug 33514.
> 
> Can you please address bug 33514? In the meanwhile should we remove this?

What do you mean with "address"? It's all explained there as well.
Comment 40 Jonathan Druart 2023-05-15 08:30:42 UTC
I was actually asking the author of 23919 (here) to fix the bug they introduced.
Comment 41 Jonathan Druart 2023-05-24 07:45:33 UTC
Can you please help on the follow-up bug report? See bug 33514.