Bug 29077 - Warns when searching blank index
Summary: Warns when searching blank index
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-09-22 12:55 UTC by Nick Clemens
Modified: 2023-06-08 22:26 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.05.00,21.11.06


Attachments
Bug 29077: Remove blank indexes before convert (1.23 KB, patch)
2021-09-22 12:58 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29077: Remove blank indexes before convert (1.29 KB, patch)
2022-04-21 04:20 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 29077: Remove blank indexes before convert (1.35 KB, patch)
2022-04-26 13:09 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29077: (follow-up) Use another syntax (1.13 KB, patch)
2022-04-27 01:05 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 Nick Clemens 2021-09-22 12:55:14 UTC
To recreate:
1 - Be using Elasticsearch
2 - Search on OPAC with dropdown set to 'Library catalog'
3 - Warns in plack-opac-error.log
[2021/09/22 12:40:55] [WARN] Use of uninitialized value $f in pattern match (m//) at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm line 725.
[2021/09/22 12:40:55] [WARN] Use of uninitialized value $f in exists at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm line 731.
Comment 1 Nick Clemens 2021-09-22 12:58:14 UTC
Created attachment 125132 [details] [review]
Bug 29077: Remove blank indexes before convert

To test:
1 - Be using Elasticsearch
2 - Search on OPAC with dropdown set to 'Library catalog'
3 - Warns in plack-opac-error.log
4 - Apply patch
5 - Repeat
6 - No warns
7 - Search returns correct results
Comment 2 Fridolin Somers 2021-09-24 20:58:17 UTC
  @indexes = grep(/.+/, @indexes)

This is correct, but in actual code we see much more occurrences of :
  grep { $_ ne q{} } @array

In my opinion it is better.
Comment 3 Aleisha Amohia 2022-04-21 04:20:32 UTC
Created attachment 133515 [details] [review]
Bug 29077: Remove blank indexes before convert

To test:
1 - Be using Elasticsearch
2 - Search on OPAC with dropdown set to 'Library catalog'
3 - Warns in plack-opac-error.log
4 - Apply patch
5 - Repeat
6 - No warns
7 - Search returns correct results

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 4 Martin Renvoize 2022-04-26 13:09:17 UTC
Created attachment 133886 [details] [review]
Bug 29077: Remove blank indexes before convert

To test:
1 - Be using Elasticsearch
2 - Search on OPAC with dropdown set to 'Library catalog'
3 - Warns in plack-opac-error.log
4 - Apply patch
5 - Repeat
6 - No warns
7 - Search returns correct results

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize 2022-04-26 13:11:05 UTC
This works, so Passing QA.. but I agree it's a little easier to read in the other form.. happy for that to be an RM follow-up?
Comment 6 Fridolin Somers 2022-04-27 01:04:09 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 7 Fridolin Somers 2022-04-27 01:05:26 UTC
Created attachment 134006 [details] [review]
Bug 29077: (follow-up) Use another syntax

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 8 Fridolin Somers 2022-04-27 01:06:13 UTC
I use my Gauntlet of Power and snap the syntax ^^
Comment 9 Kyle M Hall 2022-05-06 17:41:40 UTC
Pushed to 21.11.x for 21.11.06
Comment 10 Victor Grousset/tuxayo 2022-06-17 03:19:20 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.