Summary: | Search filters using OR are not correctly grouped | ||
---|---|---|---|
Product: | Koha | Reporter: | Nick Clemens (kidclamp) <nick> |
Component: | Searching | Assignee: | Nick Clemens (kidclamp) <nick> |
Status: | Pushed to oldoldstable --- | QA Contact: | Martin Renvoize (ashimema) <martin.renvoize> |
Severity: | normal | ||
Priority: | P5 - low | CC: | fridolin.somers, jake.deery, januszop, lucas |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
24.11.00,24.05.06,23.11.11
|
|
Circulation function: | |||
Attachments: |
Bug 37333: Group search terms in a saved filter
Bug 37333: Group search terms in a saved filter Bug 37333: Group search terms in a saved filter Bug 37333: (follow-up) Update existing tests |
Description
Nick Clemens (kidclamp)
2024-07-11 19:40:17 UTC
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 |