[3.x] Facet group OR within AND accross groups #325
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…y provide a new pre_query processor on facets to AND facet groups together while still allowing OR within a facet group. This is based on initial code provided by Andy Broomfield
This test creates 4 facets in two groups, and four directory entries in a test directory channel. It then tries to click on each facet, testing that the facets work (in the default configuration) as OR facets within the group, and AND facets accross the facet groups.
This will re run the query eliminating each group.
This is to address the issue in this comment #235 (comment) Instead of just using the facets from the result set, re-run the query, each time eliminating all the facets except each group, and filter out its own so there is a list of each sibling facet that could be set with an OR that also mathces an AND in the other group. This then produces a filtered list of sibling facets that should be reachable within the AND accross the groups and OR within them.
Just doing some minor cleanup while looking at the code.
The by refernece when fetching the conditions is required as otherwise the resulting filters are not correctly applied for the new search. Add the accessCheck(TRUE) to entityQueries.
Verifies that when facets are selected, facets that would offer a possible selection are visible and not hidden. This occurs due to the way facets are generated from the result set. Since each facet group is an OR, if there are entries that could be shown by selecting OR in the group whilst applying the AND filter from the other groups, they need to be selectable. Adds a test to verify this with comments to help understand what is going on, as this is a bit on the complicated side. What should happen is that in each facet group, if after applying an AND filter from the other group, verify that the correct facets are visible. Eg. All facets from group 1 which would apply when facet 3 is selected, and all facets from group 2 which would apply when facet 1 is selected, and then the results are combined. In the above case, facet 2 which is only assigned to content with facet 4 would be hidden as it would be inreachable.
Change the facet filters so that the we test the avalible facets by looping through each facet group, removing that from the query and seeing what facets from that group would be returned as an or filter. This is becuase going the other way (removing the other groups) can lead to situations where possible facets are removed.
andybroomfield
requested review from
ekes,
finnlewis,
stephen-cox,
snpower and
NDITGC
|
This should be ready for review now. |
Define $facetEntities as array and fix casing on $facetLabels
|
Amazing @andybroomfield thank you! I will find time to test. @snpower sorry it has taken so long since your original pull request. Are you able to take a look at this reworked pull request and confirm it sill works for your use case? |
ekes
reviewed
Nov 13, 2023
| // Execute the filter query and get the facets returned. | ||
| $filter_query->execute(); | ||
| $facets = $filter_query->getResults()->getExtraData('search_api_facets'); | ||
| // dpm($facets); |
There was a problem hiding this comment.
Good spot, thats done now.
ekes
reviewed
Nov 13, 2023
|
To test on local:
|
finnlewis
approved these changes
Nov 13, 2023
There was a problem hiding this comment.
Thanks @andybroomfield - I've tested locally and confirmed this works for me!
How I tested:
- Installed localgov_demo
- Added a new facet type 'Music' and new facet values to the /localgov-drupal-collaborators directory channel.
- Enabled the option:
- Confirmed that the facets are now OR'd within the facet group and AND'd between facet groups.
Nice!
|
This has now been deployed to https://www.brighton-hove.gov.uk/directories/special-educational-needs-and-disability-send-support |
|
@ekes sounded happy with this too. I think we can merge and release this baby! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a rebase of #235 and #269 onto the 3.x branch
Fix #193
Adds a new facet plugin to directories 'LocalGov Directories - AND Facet Groups' which when added to the directories facets will perform facet searches using an OR filter within a facet group and an AND filter accross the groups.
This includes the work needed to try to find all possible facets in a group, this is needed as facets only presents facets that are in the result set, but an OR facet in each group needs to check if they would be visible when the other facets apply an AND filter onto them.
Based on work by @snpower, @ekes and @andybroomfield