Layout builder overrides on a single content item not allowed in a workspace

Workspace prevents all* forms from submitting when not in the default workspace.

* (except some entity forms, the search forms, views exposed forms, and its own forms)

This was confusing at first, due to our bug #2968110: Layout Builder's ConfigureBlockFormBase forms do not display validation errors on submit

Someone, I think @tedbow, mentioned off-hand that there would be a benefit to reimplementing Layout Builder's UI as a series of entity forms. Possibly for this reason?

In the meantime, here's a "quickfix" that is probably horribly incorrect.

Rerolling against 8.9.x

Moving this to workspaces.

#9 is the exact same patch as #7? Byte for byte...

Anyway, this is still the quickfix I posted 18 months ago. Will likely need a new approach completely.

This patch seems to work except for with custom blocks defined from /admin/structure/block/block-content/types.

When adding a custom block, I see the following in the logs after trying to save and getting a white screen.

1606513   10/Nov 19:55   php                Error      Drupal\Core\Entity\EntityStorageException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'block_content_id' cannot be null: INSERT INTO {inline_block_usage} (block_content_i
  1606512   10/Nov 19:55   node               Error      Drupal\Core\Database\IntegrityConstraintViolationException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'block_content_id' cannot

I'm not familiar with any of the layout builder codebase, but if this sparks an idea for someone, I would be happy to help work on a patch.

@adamzimmermann, attaching a patch that prevents layout builder from crashing due to empty block_content_id.

The behaviour is a bit odd with the patch though: when you first save the page with the new block in "staging" mode, the block will be marked as unpublished. Editing and setting it to published works, however, and this is better than a crash.

This patch enables Layout Builder to work together with Workspaces. It depends on #3208390: Add an API for allowing modules to mark their forms as workspace-safe and it also fixes the problem reported in #13.

Leaving at NW because it needs some test coverage too.

Here's a version of the patch from #15 for 8.9.x, for anyone who needs it.

+1. The patch in #15 also works well for another use case:

When using Domain Access and enabling access control for Custom Block entities, adding a custom block to Layout Builder and selecting a domain to restrict access causes the same "Column 'block_content_id' cannot be null" error. This patch fixes this and allows the layout to be saved.

If the block_content_id fix is something more general than the OP specific fix for Workspace, maybe it needs to be split out as a separate bug fix issue to get moved forward more easily?

Adding some tests for this. I had to make a change in EntityReferenceItem::generateSampleValue() to make the tests with workspaces work and this change could also be related to #3000950: Sample images cannot be generated while in a non-default workspace.

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Fixed failed commands on #22
Attached patch against Drupal 10.1.x
Attached interdiff

I'm not sure if I'm doing this right, but I found that layout builder would crash and throw duplicate primary key errors on entity clone. This would also create orphaned inline_block_usage rows, with empty layout_entity_id.

The cloned entity would also no longer save, due to the orphaned references.

I added a check in InlineBlockEntityOperations->saveInlineBlockComponent(), for entity id. Line 233.

The entity is always set, but in some cases the entity id would be empty, causing addUsage() to fail. This seems to work fine and nodes save layout builder properly.

This is against 9.5, however I think its relevant to this issue? Its exactly the same as the 10.1 patch, with an addition if statement- so should be fine for 10.1 as well. If the check is in the proper spot, thats an entirely different question. At my experience level... I believe so, but not positive.

I went through entity_clone, thinking the actual issue was there; but was unable to find the potential root culprit.

Fixed failed commands on #25
Attached patch against Drupal 10.1.x

Try To Fix The #26 Custom Commands Failed Patch.

Try To Fix The #27 CMF.

Updated #28 with the event from https://www.drupal.org/project/drupal/issues/3208390#comment-15200955

Converted the latest patch to a MR, changed the approach for marking LB forms as workspace-safe by using a trait and marking individual forms, so this issue doesn't depend on #3208390: Add an API for allowing modules to mark their forms as workspace-safe anymore.

Tests were added in #22, I just moved them to the Workspaces module, so removing the tag.

Still probably needs @tim.plunkett sign off, but with the schema change I imagine we would need an upgrade path right?

Replied in the MR why I don't think an upgrade path is needed.

RTBC - looks great to me and makes sense!

Would like to commit this one, but I don't understand the answer about the upgrade path entirely, so asked on the MR. Back to needs review again for that. I agree we don't need a content upgrade path, but would like to know why we don't need a config upgrade path to match the new schema.

Replied in the MR :)

OK that's a very confusing situation, but it's not a situation that was introduced here, and the explanation is spot on, so... Committed/pushed to 11.x and cherry-picked to 10.3.x, thanks! Really nice to get this one in!