It would be nice to support multiple facet values for other facets block condition.

CommentFileSizeAuthor
#6 2784945-5.patch5.47 KBmpp
#2 2784945.patch2.08 KBmpp

Comments

mpp created an issue. See original summary.

mpp’s picture

mpp commented
Assigned: mpp » Unassigned
Status: Active » Needs review
StatusFileSize
new 2784945.patch2.08 KB
borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented

That's great, this will need extra tests to ensure that this will work though, at least in IntegrationTest::testFacetDependencies. Tests currently still fail, so we're blocked on #2784849: Tests fail w/ out of memory error., #2777483: Unmet dependencies.

Status: Needs review » Needs work

The last submitted patch, 2: 2784945.patch, failed testing.

The last submitted patch, 2: 2784945.patch, failed testing.

mpp’s picture

mpp commented
StatusFileSize
new 2784945-5.patch5.47 KB

Hi! I added some tests to OtherFacetTest to check its behavior with multiple (active & inactive) values.
Drupal\Tests\facets\Unit\Plugin\Condition\OtherFacetTest now gives 7 passes, 0 fails, 0 exceptions

About the dependencies issue and class not found, isn't that a core issue?

I had a similar error with a migrate event subscriber (https://www.drupal.org/node/2776235). This problem didn't occur before 8.1.6 after that I had to put extra dependencies to my install profile.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented

I think Christian also provided a patch for this in #2792847: OtherFacet condition - non-empty and multiple values, can you confirm that this is a duplicate? We can probably close this issue and keep working on that one? At the very least, an additional review on that patch would be very much appreciated.

mpp’s picture

mpp commented
Status: Needs work » Closed (duplicate)

This is indeed a duplicate of #2792847, closing.

mpp’s picture

mpp commented
Related issues: +#2792847: OtherFacet condition - non-empty and multiple values