Problem/Motivation

When using page manager to override a node view page, there's a fatal error on adding a content type condition as variant selection criteria:

Error: Call to undefined method Drupal\Core\Plugin\Context\EntityContextDefinition::setConstraint() in /var/www/core/lib/Drupal/Core/Plugin/Context/ContextDefinition.php on line 273 #0 /var/www/modules/ctools/src/Plugin/Condition/NodeType.php(37): Drupal\Core\Plugin\Context\ContextDefinition->setConstraints(Array)
#1 /var/www/modules/ctools/src/Plugin/Condition/NodeType.php(17): Drupal\ctools\Plugin\Condition\NodeType->removeConstraints(Array)
#2 /var/www/modules/ctools/src/Form/ConditionConfigure.php(117): Drupal\ctools\Plugin\Condition\NodeType->applyConstraints(Array)
#3 [internal function]: Drupal\ctools\Form\ConditionConfigure->submitForm(Array, Object(Drupal\Core\Form\FormState))
#4 /var/www/core/lib/Drupal/Core/Form/FormSubmitter.php(111): call_user_func_array(Array, Array)
#5 /var/www/core/lib/Drupal/Core/Form/FormSubmitter.php(51): Drupal\Core\Form\FormSubmitter->executeSubmitHandlers(Array, Object(Drupal\Core\Form\FormState))
#6 /var/www/core/lib/Drupal/Core/Form/FormBuilder.php(589): Drupal\Core\Form\FormSubmitter->doSubmitForm(Array, Object(Drupal\Core\Form\FormState))
#7 /var/www/core/lib/Drupal/Core/Form/FormBuilder.php(318): Drupal\Core\Form\FormBuilder->processForm('ctools_conditio...', Array, Object(Drupal\Core\Form\FormState))
#8 /var/www/core/lib/Drupal/Core/Controller/FormController.php(93): Drupal\Core\Form\FormBuilder->buildForm('ctools_conditio...', Object(Drupal\Core\Form\FormState))
#9 [internal function]: Drupal\Core\Controller\FormController->getContentResult(Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\RouteMatch))
#10 /var/www/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(123): call_user_func_array(Array, Array)

It looks like there's a simple mistake in \Drupal\Core\Plugin\Context\ContextDefinition::setConstraints() which tries to proxy the call to \Drupal\Core\Plugin\Context\EntityContextDefinition::setConstraint() instead of ::setConstraints($constraints) (added in #2986033: [regression] The BC layer for EntityContextDefinition in ContextDefinition is incomplete).

Proposed resolution

Proxy to the correct method - \Drupal\Core\Plugin\Context\EntityContextDefinition::setConstraints().

Remaining tasks

  • Needs review by someone who understands the BC layer!

Original report by Siavash

Hello,

I'm having some issues with a freshly installed Drupal site with Panels + Page Manager installed. Tried with both latest release and dev version.

I'm trying to create a variation for the node path for a specific content type. But when I go to save the content types or bundle types I get an ajax error and I'm unable to save the settings. (See screenshot)

PHP log ha an error:
Error: Call to undefined method Drupal\Core\Plugin\Context\EntityContextDefinition::setConstraint() in Drupal\Core\Plugin\Context\ContextDefinition->setConstraints() (line 273 of .../web/core/lib/Drupal/Core/Plugin/Context/ContextDefinition.php) #0

Would appreciate any support, thanks!

CommentFileSizeAuthor
#15 drupal-contextdefinition_bc_fix-2998462-15-D8.patch10.55 KBAndyF
#15 drupal-contextdefinition_bc_fix-2998462-15-D8-tests-only.patch9.86 KBAndyF
#9 drupal-contextdefinition_bc_fix-2998462-9-D8.patch707 bytesBaysaa
#6 drupal-contextdefinition_bc_fix-2998462-6-D8.patch707 bytesAndyF
Screenshot 2018-09-10 04.31.07.png341.08 KBSiavash
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Siavash created an issue. See original summary.

millionleaves’s picture

millionleaves CreditAttribution: millionleaves as a volunteer and commented

I have the same issue on a site that I recently upgraded from 8.5.6 to 8.6.0. Before upgrading, I was able to add Content Type as a condition for variant selection. Since upgrading, I can't.

I am able to add other condition types, e.g. Request Path, without any issues.

LeGront’s picture

LeGront CreditAttribution: LeGront commented

I have the same issue after update from 8.5.6 to 8.6.1

millionleaves’s picture

millionleaves CreditAttribution: millionleaves as a volunteer and commented
Priority: Normal » Critical

I'm marking this as critical since it appears that this issue is reproducible and is introduced when upgrading to 8.6.x. It represents a significant degradation of key functionality.

The only workarounds I can currently see are:

  • Create a different URL path for each content type using Pathauto, and use a Request Path condition at the variant level to select different content types.
  • Use block-level selection criteria to restrict visibility of individual blocks to specific content types. This is not possible without applying a series of patches.

Neither of these workarounds meet all use cases that can be met by setting Content Type as a variant-level condition.

Baysaa’s picture

Baysaa CreditAttribution: Baysaa at Fabb commented
Related issues: +#2999031: ContextDefinition: Call to undefined method Drupal\Core\Plugin\Context\EntityContextDefinition::setConstraint

Linking to core issue with patch.

Sounds like this was originally caused by #2986033: [regression] The BC layer for EntityContextDefinition in ContextDefinition is incomplete

AndyF’s picture

AndyF CreditAttribution: AndyF at Fabb commented
Project: Page Manager » Drupal core
Version: 8.x-4.x-dev » 8.7.x-dev
Component: Code » base system
Priority: Critical » Major
Issue summary: View changes
Status: Active » Needs review
FileSize
drupal-contextdefinition_bc_fix-2998462-6-D8.patch707 bytes

Thanks @Baysaa, I've marked that issue as duplicate (I don't think I should've created a new one).

I'm tentatively moving this to core, and bumping down the priority, at most it's major imho. Tracing through, the issue seems to come from a call to \Drupal\Core\Plugin\Context\EntityContextDefinition::setConstraint() from \Drupal\Core\Plugin\Context\ContextDefinition::setConstraints(). It involves a BC layer that was added in #2986033: [regression] The BC layer for EntityContextDefinition in ContextDefinition is incomplete, and it looks like it's a simple mistake in the name of the method to call on the BC layer. Certainly changing the call to setConstraints($constraints) seems to fix the page manager issue for me (patch attached). If someone who knows can confirm it's ok, I'm happy to look at adding a regression test.

Thanks!

Baysaa’s picture

Baysaa CreditAttribution: Baysaa at Fabb commented

Patch in #6 works for me. Tested by adding all conditions that come out of the box on a clean install.

Baysaa’s picture

Baysaa CreditAttribution: Baysaa as a volunteer commented
Issue summary: View changes
Baysaa’s picture

Baysaa CreditAttribution: Baysaa at Fabb commented
FileSize
drupal-contextdefinition_bc_fix-2998462-9-D8.patch707 bytes

Re-uploading patch to trigger testbot to run against drupal and not page_manager.

millionleaves’s picture

millionleaves CreditAttribution: millionleaves as a volunteer and commented

Thanks. Applying the patch in #9 has resolved the issue for me, in that I can now add Content Type as a selection criteria for a variant.

Baysaa’s picture

Baysaa CreditAttribution: Baysaa at Fabb commented

Just removing an old patch from display.

Pasqualle’s picture

Pasqualle
Location 🇭🇺 Budapest
CreditAttribution: Pasqualle as a volunteer commented
Status: Needs review » Reviewed & tested by the community

simple logical fix..

markdc’s picture

markdc
his lordship
Primary language English
CreditAttribution: markdc commented

Thanks for the patch. #9 works.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented
Component: base system » plugin system
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

This was introduced by #2986033: [regression] The BC layer for EntityContextDefinition in ContextDefinition is incomplete, itself a regression, but included a test.
This issue should expand that test or include a new one.

AndyF’s picture

AndyF CreditAttribution: AndyF at Fabb commented
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
drupal-contextdefinition_bc_fix-2998462-15-D8-tests-only.patch9.86 KB
drupal-contextdefinition_bc_fix-2998462-15-D8.patch10.55 KB

Here's a first pass at some tests, which I did in collaboration with @Baysaa: feedback welcome. It doesn't touch the existing fix.

Thanks!

The last submitted patch, 15: drupal-contextdefinition_bc_fix-2998462-15-D8-tests-only.patch, failed testing. View results

crafter’s picture

crafter CreditAttribution: crafter as a volunteer commented

This problem is still there.

Patrick Ryan’s picture

Patrick Ryan CreditAttribution: Patrick Ryan commented

+1 #15 works as expected

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented
Status: Needs review » Needs work

Looks ready for RTBC except the deprecations say 8.6 and should be 8.7.

AndyF’s picture

AndyF CreditAttribution: AndyF at Fabb commented
Status: Needs work » Needs review

Thanks for your time!

the deprecations say 8.6 and should be 8.7

I think it should be 8.6 here - the deprecation has already been added (see 099f414) and we're just testing it. Tentatively setting back to needs review, please let me know if I've missed something.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented
Status: Needs review » Reviewed & tested by the community

Oh, right you are! Thanks :)

markdc’s picture

markdc
his lordship
Primary language English
CreditAttribution: markdc commented

#15 is working for me. Thanks for the relatively quick fix.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Fixed

Creditting @tim.plunkett, @fatmarker and @millionleaves for patch review and manual testing.

Committed 14606cc and bbcfd53938 pushed to 8.7.x and 8.6.x. Thanks!

  • alexpott committed 14606cc on 8.7.x 
    Issue #2998462 by AndyF, Baysaa, Siavash, tim.plunkett, millionleaves,...

  • alexpott committed bbcfd53 on 8.6.x 
    Issue #2998462 by AndyF, Baysaa, Siavash, tim.plunkett, millionleaves,...
superlolo95’s picture

superlolo95 CreditAttribution: superlolo95 commented

#15 is working for me.
Thanks

Status: Fixed » Closed (fixed)

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