Description
Nick Clemens (kidclamp)
2020-02-03 14:47:00 UTC
AND must be capitalized, but otherwise not sure what I found here, seems to be working Ah, a search like: ti:chess AND kw:chess does fail, because it translates to: ti:chess ANDchess Created attachment 112562 [details] [review] Bug 24567: Don't strip spaces along with dangling colons This updates the regex used for removing colons to capture those with space on either side, and remove the colon while preserving the space To test: 1 - Have Koha using ES 2 - Search for: ti:chess AND chess 3 - You should get a result in sample data, otherwise replace 'chess' with a title in your catalogue 4 - Search for: ti:chess AND kw:chess 5 - No result 6 - Enable DumpTemplateVarsIntranet and DumpSearchQueryTemplate 7 - Repeate search and check page source 8 - search_query has: title:chess ANDchess 9 - Apply patch 10 - Repeat 11 - Seaerch works! 12 - query is now: title:chess AND chess Created attachment 112564 [details] [review] Bug 24567: Don't strip spaces along with dangling colons This updates the regex used for removing colons to capture those with space on either side, and remove the colon while preserving the space To test: 1 - Have Koha using ES 2 - Search for: ti:chess AND chess 3 - You should get a result in sample data, otherwise replace 'chess' with a title in your catalogue 4 - Search for: ti:chess AND kw:chess 5 - No result 6 - Enable DumpTemplateVarsIntranet and DumpSearchQueryTemplate 7 - Repeate search and check page source 8 - search_query has: title:chess ANDchess 9 - Apply patch 10 - Repeat 11 - Seaerch works! 12 - query is now: title:chess AND chess Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> FAIL Koha/SearchEngine/Elasticsearch/QueryBuilder.pm OK critic OK forbidden patterns OK git manipulation OK pod OK pod coverage OK spelling FAIL valid \3 better written as $3 \4 better written as $4 Can you please check? Created attachment 112832 [details] [review] Bug 24567: (follow-up) Use dollar sign to refer to captures Created attachment 112903 [details] [review] Bug 24567: Don't strip spaces along with dangling colons This updates the regex used for removing colons to capture those with space on either side, and remove the colon while preserving the space To test: 1 - Have Koha using ES 2 - Search for: ti:chess AND chess 3 - You should get a result in sample data, otherwise replace 'chess' with a title in your catalogue 4 - Search for: ti:chess AND kw:chess 5 - No result 6 - Enable DumpTemplateVarsIntranet and DumpSearchQueryTemplate 7 - Repeate search and check page source 8 - search_query has: title:chess ANDchess 9 - Apply patch 10 - Repeat 11 - Seaerch works! 12 - query is now: title:chess AND chess Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 112904 [details] [review] Bug 24567: (follow-up) Use dollar sign to refer to captures Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> I only now noticed the warning
> Use of uninitialized value $3 in concatenation (.) or string at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm line 943.
Moving back to FQA until sure what to do with this.
Created attachment 112906 [details] [review] Bug 24567: (QA follow-up) Remove warning in regex Without this patch we get Use of uninitialized value $3 in concatenation (.) or string at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm line 943. This converts the | OR operator to two different regexes so that the capture group variables will be defined in every case. Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Added follow-up to remove the warning. Passing QA. *** Bug 25263 has been marked as a duplicate of this bug. *** Pushed to master for 20.11, thanks to everybody involved! *** Bug 24372 has been marked as a duplicate of this bug. *** Can this be reproduced for 20.05? Pushed to 20.05.x for 20.05.10 Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed. Any chance of getting this for 19.11 Victor? It looks a reasonably simple patch to me, and we have a couple of customers enquiring about it :) Should the patch "Use dollar sign to refer to captures" be pushed to master? @Martin is that actually needed for 19.11.x? 1 - Have Koha using ES 2 - Search for: ti:chess AND chess 3 - You should get a result in sample data, otherwise replace 'chess' with a title in your catalogue 4 - Search for: ti:chess AND kw:chess 5 - No result I actually have results. 19.11.17, koha-testing-docker, marc21, ES enabled |