Problem/Motivation

Saving a tagged article in Stage workspace causes WSOD by throwing Drupal\Core\Entity\EntityStorageException: This entity can only be saved in the default workspace. in Drupal\Core\Entity\Sql\SqlContentEntityStorage->save() (line 783 of /home/dm0uz/www/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).

Recreate by

  • installing standard
  • enabling workspaces
  • switching to Stage workspace
  • creating a new article with a tag
  • pressing Save

Proposed resolution

TBD

Remaining tasks

All the things

User interface changes

TBD

API changes

TBD

Data model changes

TBD

CommentFileSizeAuthor
#11 2998662-11.patch14.77 KBamateescu
#8 2998662-8.patch12.91 KBsam152
#8 interdiff.txt1.6 KBsam152
#6 2998662-6_0.patch4.43 KBdhirendra.mishra
#6 interdiff_3-6.txt7.92 KBdhirendra.mishra
#3 2998662-3.patch12.93 KBsam152
#3 Screen Shot 2018-09-11 at 4.55.33 pm.png70.3 KBsam152

Comments

Eli-T created an issue. See original summary.

sam152’s picture

sam152 commented
Related issues: +#2880149: Convert taxonomy terms to be revisionable

I imagine this is because of #2880149: Convert taxonomy terms to be revisionable. But more broadly speaking, there should probably be some validation for auto-created entities that aren't supported.

sam152’s picture

sam152 commented
Status: Active » Needs review
StatusFileSize
new Screen Shot 2018-09-11 at 4.55.33 pm.png70.3 KB
new 2998662-3.patch12.93 KB

How about something like this?

amateescu’s picture

amateescu commented
Status: Needs review » Needs work

The patch looks great! I only found a few minor points:

  1. +++ b/core/modules/workspaces/src/EntityTypeInfo.php
@@ -70,4 +70,18 @@ public function entityTypeBuild(array &$entity_types) {
+   * Alter field plugin definitions.

    Alters ... :) 

  2. +++ b/core/modules/workspaces/src/Plugin/Validation/Constraint/EntityReferenceSupportedNewEntitiesConstraintValidator.php
@@ -0,0 +1,65 @@
+  public function validate($value, Constraint $constraint) {

    Let's add an early return if $value is empty. 

  3. +++ b/core/modules/workspaces/tests/src/Kernel/EntityReferenceSupportedNewEntitiesConstraintValidatorTest.php
@@ -0,0 +1,81 @@
+    $this->assertEquals(0, $entity->validate()->count());
...
+    $this->assertEquals(1, $violations->count());

    Let's use assertCount() here :)

dhirendra.mishra’s picture

dhirendra.mishra commented
Version: 8.6.x-dev » 8.7.x-dev
Assigned: Unassigned » dhirendra.mishra
dhirendra.mishra’s picture

dhirendra.mishra commented
Status: Needs work » Needs review
StatusFileSize
new interdiff_3-6.txt7.92 KB
new 2998662-6_0.patch4.43 KB

Thanks for highlighting correction. I am uploading interdiff with #3 and patch which includes correction from #4. Please review and merge my patch.

Status: Needs review » Needs work

The last submitted patch, 6: 2998662-6_0.patch, failed testing. View results

sam152’s picture

sam152 commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt1.6 KB
new 2998662-8.patch12.91 KB

Not sure what happened with the patch in #6, looks like it messed up somehow.

I looked into the empty check and it doesn't seem necessary, our validator isn't called when the list is empty:

\Drupal\Core\TypedData\Validation\RecursiveContextualValidator::validateNode
    // If the data is a list or complex data, validate the contained list items
    // or properties. However, do not recurse if the data is empty.
    if (($data instanceof ListInterface || $data instanceof ComplexDataInterface) && !$data->isEmpty()) {

Edit: Just to expand on that, our constraint is on the field item level and the field item list doesn't recurse when empty.

amateescu’s picture

amateescu commented
Assigned: dhirendra.mishra » Unassigned
Status: Needs review » Reviewed & tested by the community

Looks great now!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work

Needs a re-roll :(

Creditting @Eli-T for creating the issue with clear steps to reproduce and @amateescu for patch review.

amateescu’s picture

amateescu commented
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new 2998662-11.patch14.77 KB

Rerolled.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed e417c7f2fd to 8.7.x and 8e94ef486c to 8.6.x. Thanks!

  • alexpott committed e417c7f on 8.7.x 
    Issue #2998662 by Sam152, dhirendra.mishra, amateescu, Eli-T: Saving a...

  • alexpott committed 8e94ef4 on 8.6.x 
    Issue #2998662 by Sam152, dhirendra.mishra, amateescu, Eli-T: Saving a...

Status: Fixed » Closed (fixed)

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