When a search filter contains terms joined by OR: subject:programming OR subject:internet The search filter is added to a search like: title:perl AND subject:programming OR subject:internet This negates the full effect of the filter as it is not considered as a single part
Created attachment 168864 [details] [review] Bug 37333: Group search terms in a saved filter This patch surrounds the expanded query of a search filter in parnetheses to correctly group the terms when searching To test: 1 - Enable system preference SavedSearchFilters 2 - In staff client, perform an advanced search for: subject:internet OR subject:programming 3 - Note results 4 - Save search as filter, enabled in staff client and opac 5 - Search for 'a' 6 - Apply filter from top of facets on left hand side 7 - Note results didn't change 8 - Apply patch, restart all 9 - Reload search, it now limits correctly 10 - Test on OPAC 11 - Test on both Zebra and Elastic 12 - Sign off :-)
Created attachment 168894 [details] [review] Bug 37333: Group search terms in a saved filter This patch surrounds the expanded query of a search filter in parnetheses to correctly group the terms when searching To test: 1 - Enable system preference SavedSearchFilters 2 - In staff client, perform an advanced search for: subject:internet OR subject:programming 3 - Note results 4 - Save search as filter, enabled in staff client and opac 5 - Search for 'a' 6 - Apply filter from top of facets on left hand side 7 - Note results didn't change 8 - Apply patch, restart all 9 - Reload search, it now limits correctly 10 - Test on OPAC 11 - Test on both Zebra and Elastic 12 - Sign off :-) Signed-off-by: Jake Deery <jake.deery@ptfs-europe.com>
Hi Nick, All looks good to me. I can see before vs. after on the results page: - 11 result(s) found for 'kw,wrdl: a' with limit(s): 'and su=(internet) OR su=(programming)'. + 11 result(s) found for 'kw,wrdl: a' with limit(s): 'and (su=(internet) OR su=(programming))'. … and the results do change, now closer to what I think is correct, and definitely in a more concice order. Probably need a search nerd to QA this, but for now, here is my sign-off, with thanks! Jake
Created attachment 169099 [details] [review] Bug 37333: Group search terms in a saved filter This patch surrounds the expanded query of a search filter in parnetheses to correctly group the terms when searching To test: 1 - Enable system preference SavedSearchFilters 2 - In staff client, perform an advanced search for: subject:internet OR subject:programming 3 - Note results 4 - Save search as filter, enabled in staff client and opac 5 - Search for 'a' 6 - Apply filter from top of facets on left hand side 7 - Note results didn't change 8 - Apply patch, restart all 9 - Reload search, it now limits correctly 10 - Test on OPAC 11 - Test on both Zebra and Elastic 12 - Sign off :-) Signed-off-by: Jake Deery <jake.deery@ptfs-europe.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Thanks Nick, patch I see is trivial, but it's great to see the mammoth set to unit tests to go with it. Passing QA
Unit tests! \o/
Pushed for 24.11! Well done everyone, thank you!
It seems that after applying this patch the test: t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t fails :( The patch adds an additional level of parentheses which in turn makes "Handle search filters" section fail in 4 subtests. Could you confirm this?
Correct; I've already asked Nick to take a look. My hunch is that it's OK and we just need to update the tests, but i wanted him to confirm that before I went ahead and did that.
(In reply to Martin Renvoize from comment #9) > My hunch is that it's OK and we just need to update the tests, but i wanted > him to confirm that before I went ahead and did that. Yes, I agree, this is probably just about updating the test patterns. Now this stops my playing around with QueryBuilder.t ;)
Created attachment 169308 [details] [review] Bug 37333: (follow-up) Update existing tests We add a level of bracket nesting. I checked with Nick that this shouldn't cause any issues in the queries and we agreed it made sense so I've gone ahead and updated the tests to reflect the change. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Backported to 24.05.x for upcoming 24.05.06
Pushed to 23.11.x for 23.11.11