Problem/Motivation

Currently when using Content and moderation

  1. create bundle that allows layout overrides and is moderated
  2. create a moderated node of bundle
  3. publish it
  4. Create a forward revision.
  5. Login as user *only* has default permissions plus "configure any layout"
  6. Save the layout override
  7. Log back in as admin.
  8. The node no longer has revision that is considered a forward revision

I have tried to create a failing test but i have never made a content moderation test and it wasn't working
For now I have recorded this screencast which shows the problem: https://youtu.be/uxuNuaI2xoo

I tried the patch #2937199-9: Track Layout override revisions on entities which support revisioning but it didn't fix it.

Proposed resolution

Saving an override is weird because it is saving the entity you update a field but you don't have to have edit permission.

If this wasn't the case the user would not be able to edit the entity.

Maybe if the entity is moderated and the user doesn't have edit access we should allow them save an override. Or maybe just if there is forward revision.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#11 2973860-revision-11-combined-2942675-43.patch23.96 KBtim.plunkett
#10 interdiff-2973860-9-10.txt1.08 KBbendeguz.csirmaz
#10 2973860-10.patch2.32 KBbendeguz.csirmaz
#9 2973860-9.patch2.23 KBbendeguz.csirmaz
#9 interdiff-2973860-8-9.txt1.18 KBbendeguz.csirmaz
#8 2973860-8.patch2.23 KBbendeguz.csirmaz

Comments

tedbow created an issue. See original summary.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented

@timmilwood asked me to have a look at this. IMO we have two problems here:

  1. We are not loading the correct revision when changing the layout. Since this operation actually changes the entity field data we'd need to start from the pending revision. This is indeed closely related to #2937199: Track Layout override revisions on entities which support revisioning and will likely need to wait for #2942907: Entity system does not provide an API for retrieving an entity variant that is safe for editing to get a proper fix.
  2. If users have no moderation permission and the entity has pending revisions, they shouldn't be allowed to change the layout, at least not until we support parallel revisioning and conflict management, because this actually means opening two "revision branches".
timmillwood’s picture

timmillwood
Primary language English
Location 🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK
commented

Thanks for looking into this @plach. I thought it might be the case where we'll need to wait for #2942907: Entity system does not provide an API for retrieving an entity variant that is safe for editing.

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

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Active » Postponed
Related issues: +#2942907: Entity system does not provide an API for retrieving an entity variant that is safe for editing

Postponing this on #2942907: Entity system does not provide an API for retrieving an entity variant that is safe for editing.

xjm’s picture

xjm
she/her
Primary language English
commented
Priority: Normal » Critical
Related issues: +#2942675: Layout builder should use the active variant of an entity to avoid orphaned revisions

Is this a duplicate of #2942675: Layout builder should use the active variant of an entity to avoid orphaned revisions? If so, we can close it as such; if not, it's also critical.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Issue tags: +Blocks-Layouts, +Layout Builder stable blocker

I believe this is still relevant on its own.

bendeguz.csirmaz’s picture

bendeguz.csirmaz commented
StatusFileSize
new 2973860-8.patch2.23 KB

Here's a rudimentary test for this.

bendeguz.csirmaz’s picture

bendeguz.csirmaz commented
StatusFileSize
new interdiff-2973860-8-9.txt1.18 KB
new 2973860-9.patch2.23 KB

Uploading the fix by @tim.plunkett

bendeguz.csirmaz’s picture

bendeguz.csirmaz commented
StatusFileSize
new 2973860-10.patch2.32 KB
new interdiff-2973860-9-10.txt1.08 KB

Fixed the test assertion - it loads the latest revision now.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Postponed » Needs review
StatusFileSize
new 2973860-revision-11-combined-2942675-43.patch23.96 KB

Combined #10 with #2942675-43: Layout builder should use the active variant of an entity to avoid orphaned revisions to see if it passes.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Postponed
johnwebdev’s picture

johnwebdev commented
Status: Postponed » Active

Blocker is in

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Issue tags: +Needs issue summary update

With #2942675: Layout builder should use the active variant of an entity to avoid orphaned revisions committed, is this still relevant? I think at very least we need an IS update.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Active » Closed (duplicate)
Issue tags: -Layout Builder stable blocker, -Needs issue summary update
Related issues: +#3032287: Add the content moderation state widget to the layout builder entity form.

Tentatively closing this in favor of #3032287: Add the content moderation state widget to the layout builder entity form..