Issue #3053887 by Sam152, Lal_, fenstrat, acbramley, tedbow, larowlan: Add...

Issue #3053887 by Sam152, Lal_, fenstrat, acbramley, tedbow, larowlan: Add test coverage and document why inline blocks require a new revision to be created when modified, regardless of whether a new revision of the parent has been created

core/modules/layout_builder/src/InlineBlockEntityOperations.php

@@ -163,18 +163,10 @@ public function handlePreSave(EntityInterface $entity) {
         // duplicated.
         $duplicate_blocks = TRUE;
       }
-      $new_revision = FALSE;
-      if ($entity instanceof RevisionableInterface) {
-        // If the parent entity will have a new revision create a new revision
-        // of the block.
-        // @todo Currently revisions are never created for the parent entity.
-        //   This will be fixed in https://www.drupal.org/node/2937199.
-        //   To work around this always make a revision when the parent entity
-        //   is an instance of RevisionableInterface. After the issue is fixed
-        //   only create a new revision if '$entity->isNewRevision()'.
-        $new_revision = TRUE;
-      }
-
+      // Since multiple parent entity revisions may reference common block
+      // revisions, when a block is modified, it must always result in the
+      // creation of a new block revision.
+      $new_revision = $entity instanceof RevisionableInterface;
       foreach ($this->getInlineBlockComponents($sections) as $component) {
         $this->saveInlineBlockComponent($entity, $component, $new_revision, $duplicate_blocks);
       }
@@ -252,7 +244,7 @@ protected function getBlockIdsForRevisionIds(array $revision_ids) {
    * @param \Drupal\layout_builder\SectionComponent $component
    *   The section component with an inline block.
    * @param bool $new_revision
-   *   Whether a new revision of the block should be created.
+   *   Whether a new revision of the block should be created when modified.
    * @param bool $duplicate_blocks
    *   Whether the blocks should be duplicated.
    */

core/modules/layout_builder/src/Plugin/Block/InlineBlock.php

@@ -263,7 +263,7 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta
    * Saves the block_content entity for this plugin.
    *
    * @param bool $new_revision
-   *   Whether to create new revision.
+   *   Whether to create new revision, if the block was modified.
    * @param bool $duplicate_block
    *   Whether to duplicate the "block_content" entity.
    */
@@ -283,6 +283,8 @@ public function saveBlockContent($new_revision = FALSE, $duplicate_block = FALSE
     }
 
     if ($block) {
+      // Since the custom block is only set if it was unserialized, the flag
+      // will only effect blocks which were modified or serialized originally.
       if ($new_revision) {
         $block->setNewRevision();
       }

core/modules/layout_builder/tests/src/FunctionalJavascript/InlineBlockTest.php

@@ -263,6 +263,92 @@ public function testInlineBlocksRevisioning() {
     $assert_session->pageTextNotContains('The NEW block body');
   }
 
+  /**
+   * Tests entity blocks revisioning.
+   */
+  public function testInlineBlocksRevisioningIntegrity() {
+    $this->drupalLogin($this->drupalCreateUser([
+      'access contextual links',
+      'configure any layout',
+      'administer node display',
+      'view all revisions',
+      'access content',
+      'create and edit custom blocks',
+    ]));
+    $this->drupalPostForm(
+      static::FIELD_UI_PREFIX . '/display/default',
+      ['layout[enabled]' => TRUE, 'layout[allow_custom]' => TRUE],
+      'Save'
+    );
+
+    $block_1_locator = static::INLINE_BLOCK_LOCATOR;
+    $block_2_locator = sprintf('%s + %s', static::INLINE_BLOCK_LOCATOR, static::INLINE_BLOCK_LOCATOR);
+
+    // Add two blocks to the page and assert the content in each.
+    $this->drupalGet('node/1/layout');
+    $this->addInlineBlockToLayout('Block 1', 'Block 1 original');
+    $this->addInlineBlockToLayout('Block 2', 'Block 2 original');
+    $this->assertSaveLayout();
+    $this->assertNodeRevisionContent(3, ['Block 1 original', 'Block 2 original']);
+    $this->assertBlockRevisionCountByTitle('Block 1', 1);
+    $this->assertBlockRevisionCountByTitle('Block 2', 1);
+
+    // Update the contents of one of the blocks and assert the updated content
+    // appears on the next revision.
+    $this->drupalGet('node/1/layout');
+    $this->configureInlineBlock('Block 2 original', 'Block 2 updated', $block_2_locator);
+    $this->assertSaveLayout();
+    $this->assertNodeRevisionContent(4, ['Block 1 original', 'Block 2 updated']);
+    $this->assertBlockRevisionCountByTitle('Block 1', 1);
+    $this->assertBlockRevisionCountByTitle('Block 2', 2);
+
+    // Update block 1 without creating a new revision of the parent.
+    $this->drupalGet('node/1/layout');
+    $this->configureInlineBlock('Block 1 original', 'Block 1 updated', $block_1_locator);
+    $this->getSession()->getPage()->uncheckField('revision');
+    $this->getSession()->getPage()->pressButton('Save layout');
+    $this->assertNotEmpty($this->assertSession()->waitForElement('css', '.messages--status'));
+    $this->assertNodeRevisionContent(4, ['Block 1 updated', 'Block 2 updated']);
+    $this->assertBlockRevisionCountByTitle('Block 1', 2);
+    $this->assertBlockRevisionCountByTitle('Block 2', 2);
+
+    // Reassert all of the parent revisions contain the correct block content
+    // and the integrity of the revisions was preserved.
+    $this->assertNodeRevisionContent(3, ['Block 1 original', 'Block 2 original']);
+  }
+
+  /**
+   * Assert the contents of a node revision.
+   *
+   * @param int $revision_id
+   *   The revision ID to assert.
+   * @param array $content
+   *   The content items to assert on the page.
+   */
+  protected function assertNodeRevisionContent($revision_id, array $content) {
+    $this->drupalGet("node/1/revisions/$revision_id/view");
+    foreach ($content as $content_item) {
+      $this->assertSession()->pageTextContains($content_item);
+    }
+  }
+
+  /**
+   * Assert the number of block content revisions by the block title.
+   *
+   * @param string $block_title
+   *   The block title.
+   * @param int $expected_revision_count
+   *   The revision count.
+   */
+  protected function assertBlockRevisionCountByTitle($block_title, $expected_revision_count) {
+    $actual_revision_count = $this->blockStorage->getQuery()
+      ->condition('info', $block_title)
+      ->allRevisions()
+      ->count()
+      ->execute();
+    $this->assertEquals($actual_revision_count, $expected_revision_count);
+  }
+
   /**
    * Tests that entity blocks deleted correctly.
    */