Problem/Motivation

In #3004536: Move the Layout Builder UI into an entity form for better integration with other content authoring modules and core features, we turned the layout builder page into an entity form. Something that came out of this change was the ability for other modules to add integrations to the lifecycle of an update of a layout. In order to provide content moderation integration, users must be able to choose which state content should transition to when an entity is saved.

Proposed resolution

Add the moderation state widget to the layout entity form:

Doing this unblocks draftable layouts.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Sam152 created an issue. See original summary.

Sam152’s picture

Issue summary: View changes
Sam152’s picture

Here is a patch taken from #3004536.50 to kick it off, which will apply but fail based on the other blockers. The hook it is implementing currently doesn't exist.

Sam152’s picture

tim.plunkett’s picture

From what I can see, the full patch from #2942907: Entity system does not provide an API for retrieving an entity variant that is safe for editing isn't needed to make this test pass.
Once the form issue lands, the only necessary change is this.
That said, it might not mean we don't need that issue, just that the test doesn't cover the need.

Idk if this counts as a "workaround"? If that other issue landed and the right revision was loaded, would $entity->toUrl('canonical') begin pointing to the correct place?

Status: Needs review » Needs work

The last submitted patch, 6: 3032287-revision-6-combined-3004536.patch, failed testing. View results

tim.plunkett’s picture

Current page is "/node/1/latest", but "/subdirectory/node/1/latest" expected

I think the assertion needs a setAbsolute() or something

Sam152’s picture

We should review the test when it's combined with the form blocker. I was only able to get a fully working prototype by repacing LatestVersionSectionStorageOverride::deriveContextsFromRoute with a content_moderation-aware implementation.

Sam152’s picture

Sam152’s picture

If that other issue landed and the right revision was loaded, would $entity->toUrl('canonical') begin pointing to the correct place?

Hm, I'm afraid not. latest-version is a content_moderation provided route.

tim.plunkett’s picture

Status: Needs work » Postponed

Ahh, so it is! I saw it in core/core.link_relation_types.yml and misunderstood that significance.

Sam152’s picture

Title: [PP-2] Add the content moderation state widget to the layout builder entity form. » [PP-1] Add the content moderation state widget to the layout builder entity form.
FileSize
1.26 KB
6.65 KB

Adding to the test to prove why #2942907: Entity system does not provide an API for retrieving an entity variant that is safe for editing is a blocker.

In theory, content moderation could take the approach in the entity form sandbox and do something like the following, loading the latest revision and fiddling with the redirect URL:

/**
 * Implements hook_layout_builder_section_storage_alter().
 */
function content_moderation_layout_builder_section_storage_alter(&$definitions) {
  // @todo https://www.drupal.org/project/drupal/issues/3032287.
  $definitions['overrides']->setClass(LatestVersionOverridesSectionStorage::class);
}

Regardless of the blocker, not sure how to solve the redirect component.

Sam152’s picture

Status: Postponed » Needs review
Sam152’s picture

Status: Needs review » Postponed
Sam152’s picture

tim.plunkett’s picture

Issue tags: +Blocks-Layouts

Tagging

tim.plunkett’s picture

Title: [PP-1] Add the content moderation state widget to the layout builder entity form. » Add the content moderation state widget to the layout builder entity form.
Status: Postponed » Needs work

Blocker is in!

Sam152’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
139.77 KB
3.77 KB
7.85 KB

I was able to make this patch green with either:

diff --git a/core/modules/layout_builder/src/Form/OverridesEntityForm.php b/core/modules/layout_builder/src/Form/OverridesEntityForm.php
index 6373e1062b..0493797061 100644
--- a/core/modules/layout_builder/src/Form/OverridesEntityForm.php
+++ b/core/modules/layout_builder/src/Form/OverridesEntityForm.php
@@ -8,6 +8,7 @@
 use Drupal\Core\Entity\EntityRepositoryInterface;
 use Drupal\Core\Entity\EntityTypeBundleInfoInterface;
 use Drupal\Core\Form\FormStateInterface;
+use Drupal\Core\Plugin\Context\EntityContext;
 use Drupal\layout_builder\LayoutTempstoreRepositoryInterface;
 use Drupal\layout_builder\Plugin\SectionStorage\OverridesSectionStorage;
 use Drupal\layout_builder\SectionStorageInterface;
@@ -118,6 +119,7 @@ public function save(array $form, FormStateInterface $form_state) {
 
     $this->layoutTempstoreRepository->delete($this->sectionStorage);
     $this->messenger()->addStatus($this->t('The layout override has been saved.'));
+    $this->sectionStorage->setContext('entity', EntityContext::fromEntity($this->entity));
     $form_state->setRedirectUrl($this->sectionStorage->getRedirectUrl());
     return $return;
   }

or

diff --git a/core/modules/layout_builder/src/Plugin/SectionStorage/OverridesSectionStorage.php b/core/modules/layout_builder/src/Plugin/SectionStorage/OverridesSectionStorage.php
index 4678c375d4..019e96ef4b 100644
--- a/core/modules/layout_builder/src/Plugin/SectionStorage/OverridesSectionStorage.php
+++ b/core/modules/layout_builder/src/Plugin/SectionStorage/OverridesSectionStorage.php
@@ -315,7 +315,7 @@ public function getDefaultSectionStorage() {
    * {@inheritdoc}
    */
   public function getRedirectUrl() {
-    $entity = $this->getEntity();
+    $entity = $this->entityRepository->getActive($this->getEntity()->getEntityTypeId(), $this->getEntity()->id());
     $rel = $entity instanceof RevisionableInterface && !$entity->isDefaultRevision() ? 'latest-version' : 'canonical';
     return $entity->toUrl($rel);
   }

However, the issue remains that the latest-revision route is provided by content_moderation.

How about a new hook instead? Adding a -1001 weight creates a form like:

tim.plunkett’s picture

Priority: Normal » Major
Issue tags: +Layout Builder stable blocker
Sam152’s picture

If we feel like the hook is the right approach to integrate the latest-version route, we don't need either of those. What do you think of using that approach?

Sam152’s picture

FileSize
5.15 KB
6.84 KB
149.09 KB

Latest approach based on slack discussion.

tim.plunkett’s picture

+1 to that approach, looks much cleaner!

Leaving this for someone more familiar with Content Moderation to RTBC, but signing off on it from the Layout Builder side of things.

tim.plunkett’s picture

Also, needs IS summary

Sam152’s picture

Issue summary: View changes
Issue tags: -Needs issue summary update
amateescu’s picture

  1. +++ b/core/modules/content_moderation/content_moderation.module
    @@ -191,6 +192,17 @@ function content_moderation_entity_view(array &$build, EntityInterface $entity,
    +function content_moderation_layout_builder_overrides_entity_form_display_alter(EntityFormDisplayInterface $display) {
    +  $display->setComponent('moderation_state', [
    +    'type' => 'moderation_state_default',
    +    'weight' => -900,
    +    'settings' => [],
    +  ]);
    +}
    

    Shouldn't this be done the other way around?

    Layout Builder should implement hook_entity_form_display_alter(), check if $context['form_mode'] === 'layout_builder', then check if CM is enabled and if $context['entity_type'], $context['bundle'] is moderated.

  2. +++ b/core/modules/content_moderation/src/EntityTypeInfo.php
    @@ -364,7 +364,7 @@ public function formAlter(array &$form, FormStateInterface $form_state, $form_id
    -      in_array($form_object->getOperation(), ['edit', 'default'], TRUE) &&
    +      in_array($form_object->getOperation(), ['edit', 'default', 'layout_builder'], TRUE) &&
    

    This highlights a deficiency in the way CM allows an entity form to provide moderation.

    We probably need to introduce and check for a "#moderatable" flag on the form instead of hard-coding a few form operations.

    But that's not a problem with this patch and should be discussed in a followup.

  3. +++ b/core/modules/content_moderation/tests/src/Functional/LayoutBuilderContentModerationIntegrationTest.php
    @@ -0,0 +1,128 @@
    + * @group content_moderation
    

    We should also add @group layout_builder.

  4. +++ b/core/modules/content_moderation/tests/src/Functional/LayoutBuilderContentModerationIntegrationTest.php
    @@ -0,0 +1,128 @@
    +    $this->assertSession()->checkboxChecked('revision');
    +    $this->assertSession()->fieldDisabled('revision');
    

    We can use the $assert_session variable defined at the top.

Sam152’s picture

Thanks for the review.

1. This is a new hook that layout builder is firing, built with this use case in mind. I think it makes more sense for this hook to live in CM, then for another module to implement it and do a check for CM being installed. I'm thinking of this as content moderation is integrating with LB, not the other way around.
2. Good point, I think #2935939: Content Moderation hardcodes the 'edit' and 'default' form modes — other form modes are not supported is a good launching point for that.
3. Never seen this, but makes perfect sense to me.
4. Good catch.

amateescu’s picture

Right, the new hook is what tripped me up at first. Isn't it just a "shorthand/duplicate" of doing this?

function example_entity_form_display_alter(\Drupal\Core\Entity\Display\EntityFormDisplayInterface $form_display, array $context) {
  if ($context['form_mode'] === 'layout_builder') {
    ...
  }
}
amateescu’s picture

Status: Needs review » Reviewed & tested by the community

Discussed #26.1 at length with @Sam152, and he will open a followup with the summary and our conclusions.

I don't think that issue needs to be a blocker for this, it's more of a "clean up the way LB provides its layout_builder form operation" task, so I think the latest patch is good to go as-is.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 27: 3032287-27.patch, failed testing. View results

Sam152’s picture

Status: Needs work » Reviewed & tested by the community

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

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed 1e33bb23d3 to 8.8.x and cc7e5f4c68 to 8.7.x. Thanks!

It's great that the followup exists already. It was the one question I had when reading and testing the patch.

  • alexpott committed d4f1cb5 on 8.8.x
    Issue #3032287 by Sam152, tim.plunkett, amateescu: Add the content...

  • alexpott committed c351484 on 8.7.x
    Issue #3032287 by Sam152, tim.plunkett, amateescu: Add the content...

Status: Fixed » Closed (fixed)

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

xjm’s picture

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