Problem/Motivation

Found in #2946333: Allow synced Layout override Translations: translating labels and inline blocks

It is more obvious in that issue but it can be reproduced without that patch.

  1. Create a view that list all non-reuseable blocks and links to the block(will open edit)
  2. Create an inline block
  3. Save the layout
  4. goto the view. Update the inline block
  5. Try to edit the block in the layout builder.
  6. Get an error

This is because \Drupal\layout_builder\Plugin\Block\InlineBlock stores the revision id. and loads the block for editing.

If the block is only edited in Layout Builder it will always be the latest revision. If it is edited outside of layout builder it will not be the latest revision.

This is an edge-case now but in [# #2946333: Allow synced Layout override Translations: translating labels and inline blocks this will be a problem because translation will store translations of inline blocks so they will create new revisions of the blocks.

Proposed resolution

Use \Drupal\Core\Entity\EntityRepositoryInterface::getActive() in \Drupal\layout_builder\Plugin\Block\InlineBlock::blockForm() to get the revision that can be edited

Remaining tasks

Do it

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

CommentFileSizeAuthor
#14 3052042-8.patch5.03 KBashwinsh
#7 3052042-7.patch4.9 KBtedbow
#7 interdiff-5-7.txt697 bytestedbow
#5 3052042-5.patch4.92 KBtedbow
#5 3052042-5-TEST_ONLY.patch1.64 KBtedbow
#3 3052042-3.patch4.92 KBtedbow
#3 3052042-3-TEST-ONLY.patch1.64 KBtedbow
#2 3052042.patch4.8 KBtedbow
#2 3052042-2-TEST_ONLY.patch1.52 KBtedbow

Comments

tedbow created an issue. See original summary.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Active » Needs review
StatusFileSize
new 3052042-2-TEST_ONLY.patch1.52 KB
new 3052042.patch4.8 KB

Here is fix that loads the active revision and test.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
StatusFileSize
new 3052042-3-TEST-ONLY.patch1.64 KB
new 3052042-3.patch4.92 KB 
+++ b/core/modules/layout_builder/tests/src/FunctionalJavascript/InlineBlockTest.php
@@ -36,7 +36,21 @@ public function testInlineBlocks() {
+    $block = $this->blockStorage->load($default_block_id);
+    $page->pressButton('Save');

whoops I deleted the drupalGet() that loaded the block edit page. This only didn't fail because the current page also had a "Save" button. Fix this and added the assert for the save message text.

Status: Needs review » Needs work

The last submitted patch, 3: 3052042-3.patch, failed testing. View results

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Needs work » Needs review
StatusFileSize
new 3052042-5-TEST_ONLY.patch1.64 KB
new 3052042-5.patch4.92 KB

😔url() is deprecated.

The last submitted patch, 5: 3052042-5-TEST_ONLY.patch, failed testing. View results

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
StatusFileSize
new interdiff-5-7.txt697 bytes
new 3052042-7.patch4.9 KB 
+++ b/core/modules/layout_builder/src/Plugin/Block/InlineBlock.php
@@ -130,6 +146,10 @@ public function defaultConfiguration() {
+    if (!$this->isNew && !$block->isNew()) {
+      // Get the active block for editing purposes.
+      $block = $this->entityRepository->getActive('block_content', $block->id());
+    }

We actually only need to test here if !$block->isNew() Otherwise we can get the active block for editing.

sam152’s picture

sam152 commented
Status: Needs review » Needs work

NW for the same reasons I described in the translation issue.

acbramley’s picture

acbramley commented

My 2c - inline blocks shouldn't be editable through the UI to begin with #3075308: Inline blocks shouldn't be editable via the normal block content UI would love some feedback!

chris burge’s picture

chris burge commented

The ability to edit inline blocks outside of Layout Builder would provide UX benefits. The ability to edit via a contextual link is a good example: #3020876: Contextual links of reusable content blocks are not displayed when rendering entities built via Layout Builder

Comment left over at #3075308: Inline blocks shouldn't be editable via the normal block content UI

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

sam152’s picture

sam152 commented
Status: Needs work » Closed (won't fix)

I posted this in the translation issue, but using any revision other than the one stored in the host entity causes data integrity issues and data loss when used in combination with the "Revert revision" button that is available and has likely been pressed a whole bunch of times already. See the following issue for a deeper explanation: #3053881: Reverting entity revisions that contain custom blocks erroneously triggers EntityChangedConstraint

Since working on those issues, I haven't had any discussions to suggest that this issue should be closed in favour of #3075308: Inline blocks shouldn't be editable via the normal block content UI. Editing inline blocks outside of the LB interface needs to be explicitly denied.

@tedbow, if you feel differently feel free to reopen this or comment on any of the other data integrity issues.

chris burge’s picture

chris burge commented

This issue directly affects the feasibility of reusable blocks that can be edited in Layout Builder: #2999491: Add reusable option to inline block creation. Reusable inline blocks is basically the same problem solved in D7 by Fieldable Panels Panes with reusuable panes. The fate of this issue is tied to #2999491: Add reusable option to inline block creation.

ashwinsh’s picture

ashwinsh
He/Him
Primary language English
Location Pune
commented
StatusFileSize
new 3052042-8.patch5.03 KB

Based on https://www.drupal.org/project/layout_builder_st/issues/3067646 reference, I have updated this patch.