Dependend Facets don't reset after Conditions are not met anymore.

This adds a failing test.

I'm not sure how we should actually fix this.

This is kind of tricky to solve. We would need in url processor a method to remove a specific facet filter key, but preserve all other filters (see diff)

#2861586: Make it easier to programmatically generate facet links code from comment #46 introduces a new method on the url processor (get/setActiveFilters) which can be used for this.

I quickly discussed this with @StryKaizer and @seanB on irc today. I tried implementing our discussion but couldn't make it work - yet.

What we did discuss is this: (we discussed this in dutch on irc, so summarising instead of posting the entire conversation).

1. This could be very resource intensive, so we should take special care in implementing this.
2. The dependable's url should be rewritten (using #2861586: Make it easier to programmatically generate facet links) in the processor
3. We shouldn't make this too hard on ourselves by trying to cover all possible cases at once.

The path forward here is probably making that sure that when there's only one active item in a dependable facet, the url on the dependable facet should make sure that the depending facet's items are also unset (I know this terminology makes it hard - sorry).

What about at least showing that the active items are shown?

We have this issue as well, the fix I have implemented is to add the active values to the results list and make sure that the count limit processor checks that the result is not active before removing it. This make sure that the user is still able to disable the facet filtering after creating this situation.
It's not the best solution, but it is one that works. Because it would be a lot better that the dependable urls are rewritten.

Forgot to trim the filter value on "

I have the same issue, I tried patch #12 and it didn't seem to change anything.

I have this issue as well, patch #12 doesn't solve this but it does solve another problem.

I have a view with facets and a text search, when selecting a facet, and then searching for a certain word that doesn't have any result for the current facet selection, the facet wil stay selected but not be visible, causing the view to be filtered on the facet without it being clear. The patch does make sure that the selected filter values stay visible so they can be deselected. However it doesn't do anything about the problem with the dependent facets.

I think the fix @jefuri came up with is the correct route to go for this. Trying to figure out which dependents value needs to be removed from the depending's URL when it becomes deactivated is probably going to be difficult to implement and would require extra queries. This patch simply ensures that selected values are always visible so a user will be able to see why they are suddenly getting no results. In fact, this appears to be how the Facet API module functioned in D7 in similar situations.

I've made one small update to the patch in #12 - dependant facets that are set to only be visible when their depending has a value should still display active results in the case where they have some but the depending has no values selected.

Patch #16 works great for showing active facets that have been hidden. IMO this is super necessary when you dig yourself into a facet hole, whether they're programmatically dependent or not.

Write some codes in QueryString.php to controll the url output of facet items.
This will solve your problem.

Thanks, patch from #16 is a very good way to solve this issue, without adding a lot of overhead.

Patch from 16 worked for me and also fixed the issue that the dependent facet was not displayed on the page load if setting the "parent" facet value using URL.
The only problem - titles of the empty facet are now always displayed.

#16 worked for me as well. I'm marking as RTC as 4 people have tested now and it works.

Actually, setting this back to Needs Review as the patch solved almost all of the original issue. After applying the patch at #16, the dependent facet is at least to the user shown but it is not cleared/deleted/deselected.

I tested the patch #16, and I had the same behaviour mentioned by @xamount at #22. When I unchecked the parent/condition facet option, the dependent facet is visible now, but it isn't cleared/unchecked.

I think this is a good start for the fix, but since the expectation here (including what's written in the description of the issue) is that "it should reset", I'm setting this back to "Needs work".

With patch #16 I can confirm the problem mentioned in #22

Edit. I think the right approach could be this: https://www.drupal.org/project/facets/issues/3008061

Here's another approach, it's pretty ugly, but it works for my current use case.

Do a redirect if facet is still active

I check in the DependentFacetProcessor if the dependency conditions are removed but if there is still a facet option active.
If there is I do a redirect to a url without the facet's query parameter.

Todos

• One downside is that this approach doesn't work for ajax pages. So we need to check if the search page is ajaxified and deal with the ajax requests via the ajax controller.
• I really need to check if this works reliably when dealing with multiple active options per facet.
• I don't like the str_replace part within an urlencoded string. That could certainly use a more elegant solution.
• If one facet clears a lot of other dependent facets, you get a lot of redirects.
• There's a leftover Routematch use statement up top.

So, hey, many downsides... but the basic approach might work for you too.

We were trying to solve the same problem, and a potential resolution (certainly for our use case) was to exclude the dependant facets on the dependee, so that it would always reset when changing the dependee via the url processor.

I am not sure this solution would work nicely for the Facets REST module. I would rather show the dependee face, then resetting it.

@andy_w, patch #26 does not work for me. I am not sure what would be the expected behavior for this patch and if I need to do any other configuration to make it work. I am having the same behavior with or without a patch. See following GIF:

#2970987: Allow other modules to alter the url built for the facet item. introduced an event for 'Query string created'. This event can be leveraged to remove filter parameters for a given facet when its dependencies are no longer active.

Based on @andy_w patch on #26 I replaced $facet->id() with $this->filterKey and ":" with $this->separator inside the foreach result loop.

    foreach ($results as $result) {
      $url = $result->getUrl();
      $queryOptions = $url->getOption('query');
      if (!empty($queryOptions) && isset($queryOptions[$this->filterKey])) {
        $queryOptions[$this->filterKey] = array_filter($queryOptions[$this->filterKey], function ($option) use ($dependantFacets) {
          list($key, $value) = explode($this->separator, $option);
          return !in_array($key, $dependantFacets);
        });
        $url->setOption('query', $queryOptions);
      }
    }

This way it worked for me. Sorry for not taking the time to create a patch and the false patch I have attached. Please delete.

I've created a patch based on comment #30

Thanks, need this one!

if I disable ajax, wondering why switching from a facet item back to 'choose' breaks my search?
not a bug with the patch, but I was hoping this patch would fix that.
So I ended up using ajax although spent a lot of time trying to figure out the problem before that. Ideally it should work either way.

I tested the #31 patch and it did not work.

Facet B is dependent on a value in facet A. When a different value in facet A is selected other than the dependent trigger value, Facet B's values are still selected in the URL. This is especially problematic with AND logic facets as it can sort of trap a user into an unintended combination of values where no results generate and no way to back out.

Edit: On drupal/facets (1.8.0)

#34 Try this patch: it removes the facet B values from the other than the dependent trigger value of facet A

Note that #35 and #31 are different approaches for the same problem.

ok I was confused about the issue, ended up finding a bug that we need to fix, when a facet dissapears after unchecking a value. Took me a while to figure this out.

See related issue:

Hello,

I am testing patch from comment 35 with patch from #3047910: Add AND\OR condition for dependent facets processor and in this case it is not working.

(this is more a note for myself when my customer project will really start, if I have time to update patch.)

Hi,

Creating an MR from existing patch and updating target branch.

I have added one review comment.

Adding tag for needs tests.

What kind of tests maintainers expect for this issue?

Doing some manual tests, it does not work.

On default article content type. I have added:
- a facet on the author
- a facet on the tags field which depends that the facet on author is not empty.

If I select an author then a tag then unselect the author there is still a GET parameter for the tags facet.

I tried to write a test and I realized that @borisson_ test from comment 3 is still what needs to be done.

So I have created a dedicated branch with MR to highlight it and I have updated it to removed deprecated test methods.

Adding it into the previous MR.

Removing tag as there is a failing test.

Just ran into the same problem as described in #34, I guess, and described it here: https://www.drupal.org/project/facets/issues/3190289#comment-14461827

Should that issue be closed as duplicate?

I can confirm this is still a major issue.

Note that #35 and #31 are different approaches for the same problem.

@Grimreaper: MR!58 was based on patch #35? Should we perhaps hide the patches that were incorporated into the MR or are not relevant anymore? As there are no interdiffs and I'm new to this issue, it would be helpful to understand which approach should now be taken.

Guess this is one of the most problematic issues on Facets currently, without known workaround?

PS: Confirming MR!58 does nothing for me so far.

I now opened MR!63 based in #31 (as the branch name tells).
MR!58 looks quite similar to the MR in #3008061: Facet is not displayed when the dependent processor and renders the items using hierarchy is active and is based on #35 but doesn't fix anything for me. #35 and #31 are different approaches, as @idebr wrote in #35, perhaps @Grimreaper as maintainer can tell if one is better than the other in his eyes.

I personally more like #31 and hope I'll be able to create a working fix from that. Tests will of course be the same.

Okay, here we go with some updates:

MR!63 aka #26, #30,#31 technically works, I just tested this, but you have to switch to the Dependant query string URL processor in the facet settings:

This explains why several people reported it's not working!

As this blocks us from using Facets Pretty Paths for example, this is not an option or real fix in my eyes, but for some it may be a workaround in the meantime, if they do not need a different URL Processor. For this reason I'll close MR!63 again.

Anyway the code from this patch shows it can be fixed, so hopefully we can learn from that in MR!58 where I'll proceed now.

Opened #3272400: Dispatch QueryStringCreated event to be compatible with Facets in facets_pretty_paths which is required to be solved if MR!58 is used to make it work with facets_pretty_paths.

Lots of work to be done here! Please note that NONE of the MR will be working with fences_pretty_paths Url Processor currently due to #3272400: Dispatch QueryStringCreated event to be compatible with Facets

And #3272405: The QueryStringCreated event should pass $this (self) to retrieve information about the Url Processor would be required to cleanly implement this using the QueryStringCreated Event!

So from my perspective, there are two options for MR!58:
a) Don't use an event to implement this, but implement it directly in QueryString or even better UrlProcessorPluginBase to let other URL Processors inherit the correct functionality.
b) Postpone this on #3272405: The QueryStringCreated event should pass $this (self) to retrieve information about the Url Processor to be able to implement this more cleanly.

In the meantime, we could try to make it work "dirty" but point out the incompatibility to other contrib Url Processors!

Would be nice if a maintainer could make some helpful decisions about these options and have a look at the related issues!

I think we need an implementation within the DependentFacetProcessor instead of listing to an event.
And we must double check that no other facet is using the same parameter name (which is a feature).

I took a look at the code and examined different approaches. The main issue is when different processing steps for different facets happen independently. And you that you can change orders.
Nevertheless I probably found a simple workaround and try to create a patch tomorrow.

All the approaches described here won't work well with big pipe. With big pipe each facet block is rendered on it's own in an individual request. You need to manipulate all query params of all facets if one condition for a dependent facets is not met anymore. If you have 5 individual facets and 1 dependent facet you need to execute the processor chain for the dependent facet 6 times, once for every facet.
This will be a big performance penalty.

I thought about a different approach to just prevent the conversion of the active param in a Search API condition if the dependcies are not met. Here's a patch for that. Unfortunately it will not work with conditions based on display values yet (and the tests will fail). But I'm interested in you opinion.

Great work @mkalkbrenner, I think this is a lot more going into the right direction than the MR we saw before!
The patch reads great so far, I'll now try it and eventually create a backport to 8.x-1.x if possible and useful for the project we needed this for. That's sadly still Drupal 8.9.

I improved the patch to also remove the parameter from links "on the next page".

Here's the 8.x-1.x backport of #58 if someone else needs it for manual testing. Without the test changes as the test class didn't exist there.

Patch #58 failed to apply on Drupal CI, whyever... Didn't test my backport yet.
Once it works, I'd vote for a MR.

I added the tests from the MR. Let's see what happens.

Thank you so much @mkalkbrenner, GREAT work! The tests works fine, and also my manual testing says this part is working great so far. But of course we should wait for further feedback and manual tests now.

But in my tests I also experienced a problem, which is in parts documented here: #3008061: Facet is not displayed when the dependent processor and renders the items using hierarchy is active or here #3190289: Add option to reset other facets on value change, I think. As I'm not sure yet if it's a follow-up of this problem, part of this problem, or completely separate, I'm writing here to let you decide if it belongs here or somewhere else. In the last days I thought it was the same problem, but I'm not that sure anymore...

Here's my example to reproduce:
3 Facets: A, B, C

A is a taxonomy tree (hierarchical) facet, representing a menu structure like this:
- Flowers
- Trees
-- Apple trees
-- Orange trees
- Stones
Not selecting anything shows all products.
It's the only facet shown when loading the overview page, so the user first selects a value in A. All other facets are dependent on this facet to be NOT EMPTY.

B is dependent on A (NOT EMPTY). B is a text field facet, allowing to filter our properties which are only existing on Trees results
C is dependent on A (NOT EMPTY). C is a taxonomy reference facet of colors, only existing on Flowers and Trees results

A is very dynamic, so administrators are adding further categories which makes it impossible to define dependencies on VALUES of A, instead all facets are only shown, if they return at least one value. This works great, the facets only show for categories of A, with at least one result of that facet.

The problem is, that if a filter value in facet B or C is selected (and thereby part of the URL) and you switch to a category in A which doesn't show that facet, the result is always: NO RESULTS FOUND (as no result in that combination can exist).
The user doesn't see that a filter on B or C is still active, as the facet is hidden. But even showing it, would require the user to manually fix this combination by removing that combination.

So the problem is, that the dependency on A is still met (has value) when changing "Flowers" to "Trees" - so it MIGHT be correct that the other facets values are not removed by THIS patch, but the hidden filtering should be removed to make the facets work as expected.

Still curious how to solve this. And unsure if this fix simply needs to happen or if people might want to show the facet (by option) if a filter is still set it even though it can't return results.

As a first step, I'm sure it's definitely a problem to filter AND hide the facet.

So @mkalkbrenner would be nice to decide where to proceed with that problem and sorry if you think it's off-topic, I'm absolutely unsure.

PS: Sorry, I'm not that experienced with Facets and its structure so far, as I'm using it for the first time ;)

My patch is a workaround for the major issue that facets are always processed individually. We'll never get a perfect solution unless we change that approach.
the issue queue is that big, I therefore commit this patch to have a new code base for the other issues. I consider this version to be "less broken" than the current state.

Thank you very, very much @mkalkbrenner fixing this issue and committing, agree with your decision here!

Hi,

Sorry to not have been more available.

Thanks @mkalkbrenner and contributors for the fix!

I will try to update #3047910: Add AND\OR condition for dependent facets processor.

@Grimreaper & @mkalkbrener and others who may run into a similar issue as I did, described in #65:
I updated my results from #65 (sorry for the long comment) into #3190289: Add option to reset other facets on value change and #2993101: Display facets when there are no search results which are both about the "minimum results" logic and combination of facets, but in different scenarios.

So comment #65 isn't relevant here anymore, but still for #3190289: Add option to reset other facets on value change.