https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #72434|0 |1 is obsolete| | --- Comment #2 from David Gustafsson <glasklas@gmail.com> --- Created attachment 72435 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=72435&action=edit Bug 20334 - Option for escaping slashes in search queries Add "QueryRegexEscapeOption" system preference to provide option to escape Elasticsearch regexp delimiters (/) within queries, or alternativly to unescape escaped slashes (\/) while escaping unescaped slashes, in effect making "\/" the new regexp delimiter. How to test: 1) Run tests in ./t/Koha/SearchEngine/ElasticSearch/QueryBuilder.t 2) All tests should succeed -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff --- Comment #4 from David Gustafsson <glasklas@gmail.com> --- Ok! Yes, ready to signoff. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #72435|0 |1 is obsolete| | --- Comment #6 from David Gustafsson <glasklas@gmail.com> --- Created attachment 72471 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=72471&action=edit Bug 20334 - Option for escaping slashes in search queries Add "QueryRegexEscapeOption" system preference to provide option to escape Elasticsearch regexp delimiters (/) within queries, or alternativly to unescape escaped slashes (\/) while escaping unescaped slashes, in effect making "\/" the new regexp delimiter. How to test: 1) Run tests in ./t/Koha/SearchEngine/ElasticSearch/QueryBuilder.t 2) All tests should succeed -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 Nicolas Legrand <nicolas.legrand@bulac.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 Maksim Sen <maksim.sen@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #72830|0 |1 is obsolete| | --- Comment #8 from Maksim Sen <maksim.sen@inlibro.com> --- Created attachment 73222 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73222&action=edit Bug 20334 - Option for escaping slashes in search queries Add "QueryRegexEscapeOption" system preference to provide option to escape Elasticsearch regexp delimiters (/) within queries, or alternativly to unescape escaped slashes (\/) while escaping unescaped slashes, in effect making "\/" the new regexp delimiter. How to test: 1) Run tests in ./t/Koha/SearchEngine/ElasticSearch/QueryBuilder.t 2) All tests should succeed Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr> Signed-off-by: Maksim Sen <maksim.sen@inlibro.com> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 --- Comment #10 from David Gustafsson <glasklas@gmail.com> --- Ok, done :) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi David, 1) Can you please add an entry for the new system preference in sysprefs.sql? 2) I wonder, is this something where picking a sensible default behaviour could make sense? Honestly I am more than a little confused by the different options, but I can confirm that searching for something containing / won't work right now, which is a problem. For example, it will break when someone copies a MARC record for search. Example: Dutch : an essential grammar / William Z. Shetter and Inge Van der Cruysse-Van Antwerpen 3) The system preference doesn't mention that it applies to Elasticsearch only - I think we need to add this information. 4) My sample search: Dutch / William doesn't work with any of the settings. What am I missing? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 --- Comment #13 from David Gustafsson <glasklas@gmail.com> --- Created attachment 80978 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=80978&action=edit Bug 20334: Improve syspref description and set more sensible defaults -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 --- Comment #14 from David Gustafsson <glasklas@gmail.com> --- @Katrin Fischer: And to clarify, right now the default (Escape) is to disable regular expressions in searches (all "/", which is used as regexp delimiter in Elasticsearch, will be escaped). The "Escape unescaped" will allow regexps, but instead of "/test(ing)?/" you have to write "\/test(ing)?\/" which is harder to do by mistake. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 --- Comment #16 from David Gustafsson <glasklas@gmail.com> --- Created attachment 81517 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81517&action=edit Bug 20334: Add entry in sysprefs.sql -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 --- Comment #18 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- The file is sorted alphabetically to avoid conflicts ;) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #20 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Couldn't resist, being a librarian and all. Moving back to "Signed off" as last 2 follow-ups have been trivial and this was already signed-off before. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 --- Comment #22 from David Gustafsson <glasklas@gmail.com> --- Ops, didn't realize this was already fix. Will remove the latest patch. :) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Patch doesn't apply CC| |ere.maijala@helsinki.fi --- Comment #23 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Patch doesn't apply right now, conflict looks small. Could we get a rebase please? Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 20334 - Option for escaping slashes in search queries Using index info to reconstruct a base tree... M Koha/SearchEngine/Elasticsearch/QueryBuilder.pm M koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/searching.pref Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/searching.pref Auto-merging Koha/SearchEngine/Elasticsearch/QueryBuilder.pm CONFLICT (content): Merge conflict in Koha/SearchEngine/Elasticsearch/QueryBuilder.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 20334 - Option for escaping slashes in search queries The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-20334---Option-for-escaping-slashes-in-search--Co2Ks4.patch Also adding Ere here. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #80978|0 |1 is obsolete| | --- Comment #25 from David Gustafsson <glasklas@gmail.com> --- Created attachment 85130 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85130&action=edit Bug 20334: Improve syspref description and set more sensible defaults -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 --- Comment #27 from David Gustafsson <glasklas@gmail.com> --- Rebased against master. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 --- Comment #29 from Ere Maijala <ere.maijala@helsinki.fi> --- I don't really like the fact that a pref may affect how the search terms are interpreted. I'd rather go without the pref and try to do the right automatically: if the term looks like a regexp (i.e. start and ends with a slash), don't escape it so that regexp can be used. Otherwise escape the slashes. Could also additionally validate the regexp to make sure it works. I think this would eliminate the need to for the pref while keeping the versatility and the regular searches working properly. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 --- Comment #31 from David Gustafsson <glasklas@gmail.com> --- Hi! I see two major problems with this approach. Firstly, I don't think librarians generally expect, for example, the search terms "Directive 95/46/EC" to be interpreted as containing a regular expression. I would guess that the fraction of librarians who would ever use regular expressions in searches can be rounded down to zero :) I would probably never use it myself. The second problem is that it's probably really nasty to implement as a regular expression, perhaps not even possible, that is: escaping of unescaped slashes if the slash is odd and not in a quoted context. Would probably have to write some kind of parser. Since I think that strings within slashes are generally not meant to be regexps by those who generally use the search interface I went for the two options either to disable them completely, or be allowed, but using a syntax that one would probably not enter accidentally. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 --- Comment #33 from David Gustafsson <glasklas@gmail.com> --- Yes, it would be much easier to work on a tokenized search string with regards to quotes etc. But do we really do this? We do it in _truncate_terms, but that's only if auto-truncation is enabled. It would be nice if there was some intermediate query format that could be used instead of the raw search string though, but that seems outside the scope of this patch. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 --- Comment #35 from David Gustafsson <glasklas@gmail.com> --- Yes, that change would perhaps be pretty trivial. Still not completely obvious how to try to "autocorrect" the regexps though. The regexps in lucene are always anchored according to https://stackoverflow.com/questions/34312196/elastic-search-query-string-reg... That might be the reason why I got strange results when trying out regexps on the format "foo/bar/", which produced no results, while /foobar/ did. I also assume that the regexp is run on the tokenized values in ElasticSearch, with possible stemming, n-grams etc applied (even though I don't think we do any of these at the moment), so it's not very obvious for the user what values are actually matched against. Regexp against a ".raw" subfield behaves more like you would expect, matching against the exact value of the mapped data. I's because of theses issue I think it might be better to disable regexps altogether, possibly with a an option for enabling a less accessible syntax for expert users who know what they are doing. Slashes embedded in word characters are also interpreted as regexp by Elastic since the string "foo/bar" also results in an error. So I also believe it to be a real problem that librarians will input such strings with matching and/or not matching delimiters and get unexpected results. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 Marjorie Barry-Vila <marjorie.barry-vila@collecto.ca> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |marjorie.barry-vila@collect | |o.ca -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 --- Comment #38 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Is there something we can do to help get this moving again? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85129|0 |1 is obsolete| | --- Comment #40 from David Gustafsson <glasklas@gmail.com> --- Created attachment 93478 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93478&action=edit Bug 20334 - Option for escaping slashes in search queries Add "QueryRegexEscapeOption" system preference to provide option to escape Elasticsearch regexp delimiters (/) within queries, or alternativly to unescape escaped slashes (\/) while escaping unescaped slashes, in effect making "\/" the new regexp delimiter. How to test: 1) Run tests in ./t/Koha/SearchEngine/ElasticSearch/QueryBuilder.t 2) All tests should succeed -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 --- Comment #42 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Ere and David, are more changes needed? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 Séverine Queune <severine.queune@bulac.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |severine.queune@bulac.fr Status|Needs Signoff |Patch doesn't apply --- Comment #44 from Séverine Queune <severine.queune@bulac.fr> --- The sandbox you've requested is not ready. Some problems occurred applying patches from bug 20334: <h1>Something went wrong !</h1>Applying: Bug 20334: Improve syspref description and set more sensible defaults fatal: sha1 information is lacking or useless (installer/data/mysql/atomicupdate/bug_20334-add-syspref-QueryRegexEscapeOptions.sql). error: could not build fake ancestor Patch failed at 0001 Bug 20334: Improve syspref description and set more sensible defaults -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85129|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85129|0 |1 is obsolete| | --- Comment #46 from David Gustafsson <glasklas@gmail.com> --- Created attachment 93584 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93584&action=edit Bug 20334 - Option for escaping slashes in search queries Add "QueryRegexEscapeOption" system preference to provide option to escape Elasticsearch regexp delimiters (/) within queries, or alternativly to unescape escaped slashes (\/) while escaping unescaped slashes, in effect making "\/" the new regexp delimiter. How to test: 1) Run tests in ./t/Koha/SearchEngine/ElasticSearch/QueryBuilder.t 2) All tests should succeed Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr> Signed-off-by: Maksim Sen <maksim.sen@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85131|0 |1 is obsolete| | --- Comment #48 from David Gustafsson <glasklas@gmail.com> --- Created attachment 93586 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93586&action=edit Bug 20334: Move new system preferene into right spot in alphabet -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 sandboxes@biblibre.com <sandboxes@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #93584|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 sandboxes@biblibre.com <sandboxes@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #93586|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 --- Comment #51 from sandboxes@biblibre.com <sandboxes@biblibre.com> --- Created attachment 93665 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93665&action=edit Bug 20334: Improve syspref description and set more sensible defaults Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 --- Comment #54 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 93699 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93699&action=edit Bug 20334: Improve syspref description and set more sensible defaults Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com --- Comment #56 from Nick Clemens <nick@bywatersolutions.com> --- This works well, I don't pretend to fully understand the regex,but it does what it says. Possibly the options could be named to be a little more clear. If I understand correctly: Escape - this essentially disables regex searching since all the slashes get escaped and you cannot escape them yourself Don't Escape - does nothing, regex searching works, but slashes must be escaped for normal searching Unescape escaped - enables regex if you escape the regex slashes So maybe? Disable regex searching Enable regex searching (users must escape slashes) Enable regex searching for advanced users I think it is fine as is but needs release notes/documentation -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20334 --- Comment #57 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work! Pushed to master for 19.11.00 -- You are receiving this mail because: You are watching all bug changes.