Problem/Motivation

The facet dependencies system should get ported to d8 as well.

Currently, there are 3 facet depencies that are available with a basic install of facetapi + facetapi_bonus. Of those 3 (roles, bundles, another facet) only 1 is actually worth porting to d8 imho. I think roles and bundles shouldn't be configured on the facet-level but should be configured on the display level (block / panel / ...). Another facet as a dependency is a good use-case though and needs to be ported.

Proposed resolution

Port the "another facet" facet dependency.

Remaining tasks

  1. Unit test
  2. Integration test

Comments

borisson_ created an issue. See original summary.

borisson_’s picture

In last night's weekly facet-hangout, we discussed this.

DependencyProcessor → a special kind of processor that gets all the facets and can operate on that. pre_query, after the url processor has run.

borisson_’s picture

Status: Active » Postponed
Related issues: +#2588731: Create an interface for processors
borisson_’s picture

Status: Postponed » Active

Setting back to active, work on this issue can start.

borisson_’s picture

Status: Active » Needs review
FileSize
4.42 KB

Attached is a start of what could be a solution for this. Needs more discussion before moving forward.

borisson_’s picture

Status: Needs review » Needs work

The patch attached in #5 will no longer work. Setting back to needs work.

borisson_’s picture

Status: Needs work » Active

I was thinking about this issue and I think the approach in #5 is a bad idea. Instead of trying to do this in a processor we should be handling this as a visibility condition.

Those conditions implement the ConditionInterface, in the ::evaluate method we can do magic that returns this.

Setting this issue back to active. We should discuss this at this week's hangout.

borisson_’s picture

Issue tags: +Needs committer feedback
borisson_’s picture

Issue tags: -Needs committer feedback

Removing tag, was discussed in the facetapi hangout of 09/11.

We should figure out if it's possible to do in a conditionInterface, we didn't port the other facet dependencies to d8 because we figured the conditions of a block were good enough. It would make sense for them to be grouped.

borisson_’s picture

Status: Active » Needs work
FileSize
4.65 KB

Attached is what we should do if we're going down the ConditionInterface route. The most important method in the class is not yet filled out (::evaluate) but I've described how I think it should work.

borisson_’s picture

Issue summary: View changes
FileSize
102.91 KB

This is how the condition would look, I think this makes sense if we get it to work.

borisson_’s picture

Version: » 8.x-1.x-dev
Assigned: Unassigned » borisson_
Issue summary: View changes
Status: Needs work » Needs review
FileSize
6.43 KB
5.36 KB

So, attached patch is enough to do this for facets, facet values don't work yet. Up next is a unit test to verify this behavior before I break anything trying to implement the facet value.

borisson_’s picture

Added a unit test to make sure this keeps working.

Up next is implementing facet value.

borisson_’s picture

Status: Needs review » Needs work

The last submitted patch, 14: port_facet_dependencies-2596447-14.patch, failed testing.

borisson_’s picture

Fixes typo that made the test fail.

borisson_’s picture

borisson_’s picture

I figured the patch in #17 was ready but needed some extra integration tests because of negation. Looks like I was very much right. Can't get the test to pass because there are still some things going on I don't really understand.

borisson_’s picture

We don't have to do negation ourselves, tests now pass + unit tests are simpler. Let's hope the testbot agrees.

The last submitted patch, 18: port_facet_dependencies-2596447-18.patch, failed testing.

The last submitted patch, 14: port_facet_dependencies-2596447-14.patch, failed testing.

The last submitted patch, 18: port_facet_dependencies-2596447-18.patch, failed testing.

borisson_’s picture

Reuploading so that all tests can run as expected. Also unassigned this issue.

Status: Needs review » Needs work

The last submitted patch, 23: port_facet_dependencies-2596447-23.patch, failed testing.

The last submitted patch, 23: port_facet_dependencies-2596447-23.patch, failed testing.

borisson_’s picture

borisson_’s picture

I reviewed this patch again and added in some docs, otherwise this looks good I think.

borisson_’s picture

Status: Needs review » Fixed

  • borisson_ committed b3e1b54 on 8.x-1.x
    Issue #2596447 by borisson_: Port facet dependencies
    

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.