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.
Port the "another facet" facet dependency.
| Comment | File | Size | Author |
|---|---|---|---|
| #27 | port_facet_dependencies-2596447-27.patch | 19.75 KB | borisson_ |
| #27 | interdiff.txt | 5.57 KB | borisson_ |
Comments
Comment #2
borisson_In last night's weekly facet-hangout, we discussed this.
Comment #3
borisson_Postponing on #2588731: Create an interface for processors
Comment #4
borisson_Setting back to active, work on this issue can start.
Comment #5
borisson_Attached is a start of what could be a solution for this. Needs more discussion before moving forward.
Comment #6
borisson_The patch attached in #5 will no longer work. Setting back to needs work.
Comment #7
borisson_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::evaluatemethod we can do magic that returns this.Setting this issue back to active. We should discuss this at this week's hangout.
Comment #8
borisson_Comment #9
borisson_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.
Comment #10
borisson_Attached is what we should do if we're going down the
ConditionInterfaceroute. The most important method in the class is not yet filled out (::evaluate) but I've described how I think it should work.Comment #11
borisson_This is how the condition would look, I think this makes sense if we get it to work.
Comment #12
borisson_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.
Comment #13
borisson_Added a unit test to make sure this keeps working.
Up next is implementing facet value.
Comment #14
borisson_Needs integration tests as well, those are up next.
Comment #16
borisson_Fixes typo that made the test fail.
Comment #17
borisson_Comment #18
borisson_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.
Comment #19
borisson_We don't have to do negation ourselves, tests now pass + unit tests are simpler. Let's hope the testbot agrees.
Comment #23
borisson_Reuploading so that all tests can run as expected. Also unassigned this issue.
Comment #26
borisson_Previous patch was empty. This one is the correct one.
Comment #27
borisson_I reviewed this patch again and added in some docs, otherwise this looks good I think.
Comment #28
borisson_