Bug 28241 - OPACNoResultsFound {QUERY_KW} placeholder doesn't always match the search terms when commas are included in the search
Summary: OPACNoResultsFound {QUERY_KW} placeholder doesn't always match the search ter...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low trivial (vote)
Assignee: Lucas Gass
QA Contact: Testopia
URL:
Keywords:
Depends on: 7620
Blocks:
  Show dependency treegraph
 
Reported: 2021-04-27 21:22 UTC by Lucas Gass
Modified: 2021-05-25 14:38 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.06,20.05.12


Attachments
Bug 28241: Fix regex to allow for content before and after comma (1.50 KB, patch)
2021-04-27 21:27 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 28241: Fix regex to allow for content before and after comma (1.56 KB, patch)
2021-04-29 19:29 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 28241: Fix regex to allow for content before and after comma (1.62 KB, patch)
2021-05-07 18:30 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lucas Gass 2021-04-27 21:22:03 UTC
The OPACNoResultsFound sys pref allows for the  use of a the {QUERY_KW} placeholder but it doesn't always work right when commas are used in the search string. 

To recreate:

Set up the OPACNoResultsFound with the {QUERY_KW} placeholder.

Do a search with a comma that will return no results. Like "King, Martin Luther". If the returns results add some additional characters to the search until no results are returns. 

Look at the #noresultsfound HTML element. Anything before (or after) the comma is omitted. So the placeholder looks something like 'Martin Luther' instead of 'King, Martin Luther'.


The regular expression in opac-search.pl can be tweaked to allow the content before or after commas to display.
Comment 1 Lucas Gass 2021-04-27 21:27:19 UTC
Created attachment 120247 [details] [review]
Bug 28241: Fix regex to allow for content before and after comma

1. Set up the OPACNoResultsFound with the {QUERY_KW} placeholder.
2.Do a search with a comma that will return no results. Like "King, Martin Luther". If the returns results add some additional characters to the search until no results are returns.
3. Look at the #noresultsfound HTML element. Anything before (or after) the comma is omitted. So the placeholder looks something like 'Martin Luther' instead of 'King, Martin Luther'.
4. Apply patch
5. Try the search again, you should see the content before and after a comma
Comment 2 Owen Leonard 2021-04-28 11:11:03 UTC
(In reply to Lucas Gass from comment #1)
> 5. Try the search again, you should see the content before and after a comma

Instead of "searchterm1,searchterm2" I see "searchterm1+searchterm2"

Is that to be expected?
Comment 3 Andrew Fuerste-Henry 2021-04-29 19:29:24 UTC
Created attachment 120319 [details] [review]
Bug 28241: Fix regex to allow for content before and after comma

1. Set up the OPACNoResultsFound with the {QUERY_KW} placeholder.
2.Do a search with a comma that will return no results. Like "King, Martin Luther". If the returns results add some additional characters to the search until no results are returns.
3. Look at the #noresultsfound HTML element. Anything before (or after) the comma is omitted. So the placeholder looks something like 'Martin Luther' instead of 'King, Martin Luther'.
4. Apply patch
5. Try the search again, you should see the content before and after a comma

Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org>
Comment 4 Andrew Fuerste-Henry 2021-04-29 19:41:22 UTC
I've got an example subject of:
600_aKing, Martin Luther,
_cJr.,
_d1929-1968
_vJuvenile literature.

Without the patch, I get:
(Martin+AND+Juvenile+literature+AND+Jr+AND+1929-1968)

With the patch I get:
(King+Martin+Luther+AND+Juvenile+literature+AND+Jr+AND+1929-1968)

That looks correct to me.
Comment 5 Andrew Fuerste-Henry 2021-04-29 19:42:44 UTC
On second thought, "consistent" would be a better word than "correct." I wouldn't have any complaint about ditching the + signs and putting in commas and spaces, but this patch hasn't made a change there.
Comment 6 Nick Clemens 2021-05-07 18:30:57 UTC
Created attachment 120712 [details] [review]
Bug 28241: Fix regex to allow for content before and after comma

1. Set up the OPACNoResultsFound with the {QUERY_KW} placeholder.
2.Do a search with a comma that will return no results. Like "King, Martin Luther". If the returns results add some additional characters to the search until no results are returns.
3. Look at the #noresultsfound HTML element. Anything before (or after) the comma is omitted. So the placeholder looks something like 'Martin Luther' instead of 'King, Martin Luther'.
4. Apply patch
5. Try the search again, you should see the content before and after a comma

Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 7 Jonathan Druart 2021-05-10 14:19:07 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 8 Fridolin Somers 2021-05-12 14:52:11 UTC
Pushed to 20.11.x for 20.11.06
Comment 9 Andrew Fuerste-Henry 2021-05-25 13:09:32 UTC
Pushed to 20.05.x for 20.05.12
Comment 10 Victor Grousset/tuxayo 2021-05-25 14:38:57 UTC
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.