Improve the way entity forms are "disabled" early when an entity is being edited in a workspace

This looks like the error is because one of these values is NULL:

     ->condition('vid', $term->bundle())
      ->condition('name', $term->getName())
      ->condition('langcode', $term->language()->getId())

Can you add debug where the query is built, to check whether this is the case for any of them? If this is the case, and you can confirm that it doesn't happen when workspaces isn't enabled, that might be a way to track this down.

The problem here is Drupal\workspaces\EntityOperations::entityFormAlter(), which runs entity validation before the term object is fully populated with data. I posted a patch in the contrib issue, but let's use this one to explore other approaches for "disabling" entity forms when a certain entity is being edited inside a workspace.

@amateescu, great find!

Maybe someone with more insights could have a look!

I'm also having this problem, but with Drupal 9.5.10 on PHP 8.1 and Link fields.

I'm using a Link field with unlimited values, but this also happens when you have limited values >1 but not all of them are being used. This is due to this pre-validation on the form load that will validate the empty placeholder fields (which have a null value).

I've tried it on a fresh install (versions are above) with only Workspaces turned on, created a content type with an unlimited Link field. Created a node with at least one value in the Link field. Went to the edit form for the node to see the deprecation notices. This would likely be fatal in PHP 8.2.

Deprecated: preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated in /var/www/html/web/core/lib/Drupal/Core/Url.php on line 281

Deprecated: parse_url(): Passing null to parameter #1 ($url) of type string is deprecated in /var/www/html/web/core/lib/Drupal/Core/Url.php on line 284

Deprecated: strpos(): Passing null to parameter #1 ($haystack) of type string is deprecated in /var/www/html/web/core/lib/Drupal/Core/Url.php on line 289

I can confirm the notices posted in #6, same scenario, a link field with unlimited values. Here the full steps to replicate on Vanilla Drupal 10.1.x:

• Install the workspaces module
• Add a link field to the page content type, cardinality unlimited, standards for other settings
• Create a content of type page, adding a link with values and clicking the "Add another item" button not populating the 2nd link item
• Save the node and re-open the edit form

The mentioned notices will be displayed. Here one with a full backtrace:

Deprecated function: preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated in Drupal\Core\Url::fromUri() (line 281 of core/lib/Drupal/Core/Url.php).
Drupal\Core\Url::fromUri(NULL, Array) (Line: 175)
Drupal\link\Plugin\Field\FieldType\LinkItem->getUrl() (Line: 27)
Drupal\link\Plugin\Validation\Constraint\LinkTypeConstraintValidator->validate(Object, Object) (Line: 202)
Drupal\Core\TypedData\Validation\RecursiveContextualValidator->validateConstraints(Object, '000000000000058c0000000000000000', Array) (Line: 154)
Drupal\Core\TypedData\Validation\RecursiveContextualValidator->validateNode(Object) (Line: 164)
Drupal\Core\TypedData\Validation\RecursiveContextualValidator->validateNode(Object) (Line: 164)
Drupal\Core\TypedData\Validation\RecursiveContextualValidator->validateNode(Object, Array, 1) (Line: 106)
Drupal\Core\TypedData\Validation\RecursiveContextualValidator->validate(Object, NULL, NULL) (Line: 93)
Drupal\Core\TypedData\Validation\RecursiveValidator->validate(Object) (Line: 132)
Drupal\Core\TypedData\TypedData->validate() (Line: 485)
Drupal\Core\Entity\ContentEntityBase->validate() (Line: 279)
Drupal\workspaces\EntityOperations->entityFormAlter(Array, Object, 'node_page_edit_form') (Line: 64)
workspaces_form_alter(Array, Object, 'node_page_edit_form') (Line: 545)
Drupal\Core\Extension\ModuleHandler->alter('form', Array, Object, 'node_page_edit_form') (Line: 840)
Drupal\Core\Form\FormBuilder->prepareForm('node_page_edit_form', Array, Object) (Line: 284)
Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 73)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 592)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 166)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 74)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 704)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

The problem is basically that the following code in \Drupal\workspaces\EntityOperations::entityFormAlter validates all entity constraints every time it runs on a form of a workspace supported entity type:

    foreach ($entity->validate()->getEntityViolations() as $violation) {
      if ($violation->getConstraint() instanceof EntityWorkspaceConflictConstraint) {
        $form['#markup'] = $violation->getMessage();
        $form['#access'] = FALSE;
        continue;
      }
    }

Instead we could just only validate the acutally workspace related constraints, adding a patch which will do exactly that.

Even though this is a task seems like the change that should have test coverage?

Ran the 11.x tests on #7 and all green.

Brought up in #workspace slack channel and @amateescu brought up this already had test coverage at \Drupal\Tests\workspaces\Functional\WorkspaceConcurrentEditingTest::testConcurrentEditing

LGTM!

I'm triaging RTBC issues → . I read the IS and the comments. I didn't find any unanswered questions.

I read the patch, which should be testing against 11.x, and see that it is adding a constructor parameter. Because of that we need to follow the deprecation policy for Constructor parameter additions → .

There is also no test here. However, there is a brief explanation in #9.

I am setting this back to Needs Work for the deprecation notice.

Started a MR with a new approach that builds upon the entity form controller override that was added in 🐛 [PP-1] Prevent content from being deleted when there is an active workspace Postponed . That service is new in 10.3.x, so it doesn't need any deprecation handling :)

Updated the issue summary.

Believe probably good to get this in before 10.3 so don't need a CR for the new service parameter.

Leaning on tests being green, applied the MR locally and tested basic functionality of workspace
Create a space
Create content in the stage space
Publish content

Believe this refactor is good.

This is actually a bug because in HEAD, if you have Layout Builder enabled and edit an entity in a workspace, when you visit `/node/X/layout` in Live, the concurrent editing message is not displayed. Added a test for this problem and keeping the issue at RTBC because it's a simple test addition with no other functional change to the MR.

Had two minor points on the MR, one of which was wrong because this is new code in 10.3 anyway, and the other was a small change that's done.

Committed/pushed to 11.x and cherry-picked to 10.4.x and 10.3.x, thanks!

catch → closed merge request !7740