Problem/Motivation

This happens if you try to delete to delete the only pending revision of an entity that was created *before* enabling workflows, so the previous default revision didn't have a content_moderation entity.

Steps to reproduce:

1. Create an article.
2. Enable content_moderation, enable it for article
3. Create a draft
4. Delete that draft.
5. Boom.

The website encountered an unexpected error. Please try again later.</br></br><em class="placeholder">Drupal\Core\Entity\EntityStorageException</em>: Default revision can not be deleted in <em class="placeholder">Drupal\Core\Entity\ContentEntityStorageBase-&gt;deleteRevision()</em> (line <em class="placeholder">677</em> of <em class="placeholder">core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php</em>). <pre class="backtrace">Drupal\content_moderation\EntityOperations-&gt;entityRevisionDelete(Object) (Line: 118)
content_moderation_entity_revision_delete(Object)
call_user_func_array(&#039;content_moderation_entity_revision_delete&#039;, Array) (Line: 403)
Drupal\Core\Extension\ModuleHandler-&gt;invokeAll(&#039;entity_revision_delete&#039;, Array) (Line: 206)
Drupal\Core\Entity\EntityStorageBase-&gt;invokeHook(&#039;revision_delete&#039;, Object) (Line: 756)
Drupal\Core\Entity\ContentEntityStorageBase-&gt;invokeHook(&#039;revision_delete&#039;, Object) (Line: 681)
Drupal\Core\Entity\ContentEntityStorageBase-&gt;deleteRevision(&#039;2&#039;) (Line: 117)
Drupal\node\Form\NodeRevisionDeleteForm-&gt;submitForm(Array, Object)

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir created an issue. See original summary.

Sam152’s picture

This is a good catch and may actually have some implications beyond the bug you are reporting. In #2941736: Moderation state revisions should have their isDefaultRevision() match the host entity's, we explicitly ensure the ContentModerationState entity default status matched the host entity, so if for the steps in the IS they aren't matching, it probably has some implications for the issues blocked behind that one.

I suppose we could either be more robust in our CMS entity clean-up code, or we should ensure when matching the defaultness is impossible we create revisions of the CMS entity for the historical or previous entity revisions.

Not quite sure yet, but will be an interesting one to dig into.

asghar’s picture

Hi @Berdir,

I repeated the steps as you mentioned but I could not produce the error. I tried with fresh 8.7.x branch.

Sam152’s picture

Status: Active » Needs review
FileSize
1.35 KB

Quick test case for this bug, haven't come up with a simple solution to this yet.

Status: Needs review » Needs work

The last submitted patch, 4: 3003238-4.patch, failed testing. View results

sathish.redcrackle’s picture

Berdir’s picture

No, it's not a duplicate, just related. It used to happen more frequently, but it still can if there is no content_moderation entity yet if creating the first draft because the first revision must always be the default, so we can't keep it in sync.

Not sure if we can just not delete it, would we end up with stale entities? Maybe just delete it completely, it will be created again if a new draft is created..?

amateescu’s picture

if there is no content_moderation entity yet if creating the first draft because the first revision must always be the default, so we can't keep it in sync.

I discussed this with @Berdir a few days ago and I think I have a proposal for a nice way of dealing with this situation: when creating a new pending revision (draft) for an existing entity which is not yet tracked by a content_moderation_state entity, we create two CMS revisions. The first revision will be the default one and will point to the default revision of the moderated entity using the default published moderation state, and the second revision will be the one pointing to the newly created draft, with the new moderation state.

Sam152’s picture

This is kind of reminiscent of #2867312: Create a batch process to ensure entities that exist for newly moderated entity types get a corresponding ContentModerationState entity, so that views works., except partially restoring part of the associated CMS entities, instead of doing all of them.

One alternative I've thought of is: instead of trying to delete the specific associated CMS entity revision, delete the whole CMS entity, reverting back to the conditions things started as when moderation was enabled.

Edit: @Berdir already through of this in #7:

Not sure if we can just not delete it, would we end up with stale entities? Maybe just delete it completely, it will be created again if a new draft is created..?

I think I would be fine with either approach, but would possible lean towards one more than the other if it was significantly less complex code-wise.

Sam152’s picture

Trying both approaches. I think I prefer 3003238-delete-cms-entity-10.patch since it's less complex and additionally is probably better for performance than having to reload the default revision of an entity and do an associated CMS entity lookup.

amateescu’s picture

I don't have any preference either, I think my suggestion in #8 is a poorman's version of #2867312: Create a batch process to ensure entities that exist for newly moderated entity types get a corresponding ContentModerationState entity, so that views works..

What would solve all of these issues completely would be to store the moderation state on a field of the moderated entity :)

Sam152’s picture

What would solve all of these issues completely would be to store the moderation state on a field of the moderated entity :)

We have an issue for that :) #2835545: Provide a Workflow FieldType that references a workflow entity and tracks the current state of an entity

Given some of the complexities involved with solving relatively speaking simple issues in content moderation, I think even if someone found the motivation and time to tackle that, we'd struggle to find resources or buy-in from committers for potentially significantly disruptive architectural changes on code that has already taken quite a long time to stabilise (even though the composite CMS entity has accounted for a large chunk of the bugs over the years). Maybe I'm overestimating the scope of the work involved though.

sam.spinoy@gmail.com’s picture

Ran into this error when trying to delete a user. Used patch 3003238-delete-cms-entity-10.patch from #10, seems to work fine. User deleted, no errors.

Sam152’s picture

Okay, putting forward 3003238-delete-cms-entity-10.patch as my preferred approach to solving this.

Sam152’s picture

Just thinking about the two options here again, I think the approach put forward in #14 is actually better since it will work with all existing content and does not require any kind of upgrade path. I think it's also a bit simpler conceptually.

Is anyone able to review that?

amateescu’s picture

Status: Needs review » Reviewed & tested by the community

#14 looks good :)

Sam152’s picture

Thanks @amateescu! :)

catch’s picture

Version: 8.7.x-dev » 8.6.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 2492cdda1c to 8.7.x and de323cd29b to 8.6.x. Thanks!

  • catch committed 2492cdd on 8.7.x
    Issue #3003238 by Sam152, amateescu, Berdir: EntityStorageException:...

  • catch committed de323cd on 8.6.x
    Issue #3003238 by Sam152, amateescu, Berdir: EntityStorageException:...
Berdir’s picture

Huh, #14 is showing test fails:

8.7.x: PHP 7.2 & MySQL 5.5 24,823 pass, 2 fail

What exactly has been committed now here? :)

  • catch committed c59f10b on 8.7.x
    Revert "Issue #3003238 by Sam152, amateescu, Berdir:...

  • catch committed f49330e on 8.6.x
    Revert "Issue #3003238 by Sam152, amateescu, Berdir:...
catch’s picture

Version: 8.6.x-dev » 8.7.x-dev
Status: Fixed » Needs work

hmm I'm sure it was green when I committed it. Reverted from both branches.

jibran’s picture

Status: Needs review » Reviewed & tested by the community

As code changes has been already RTBC in #16 last interdiff only include test only change so back to RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 25: 3003238-25.patch, failed testing. View results

Sam152’s picture

Status: Needs work » Reviewed & tested by the community
knyshuk.vova’s picture

+1 to RTBC

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 25: 3003238-25.patch, failed testing. View results

Sam152’s picture

Status: Needs work » Reviewed & tested by the community

Random fails keep reverting this, but the status should be RTBC.

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 c7beade985 to 8.8.x and d78aa97227 to 8.7.x. Thanks!

  • alexpott committed c7beade on 8.8.x
    Issue #3003238 by Sam152, Berdir, amateescu, catch, jibran:...

  • alexpott committed d78aa97 on 8.7.x
    Issue #3003238 by Sam152, Berdir, amateescu, catch, jibran:...
penyaskito’s picture

penyaskito’s picture

Actually this patch fixed that one.

Status: Fixed » Closed (fixed)

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

lpeabody’s picture

I encountered the originally reported error when we tried to cancel a user account and reassign content to the Anonymous user. After applying this patch, I was able to successfully cancel the user account while having their content reassigned to Anonymous.

However, after this operation completes, translations of the content are not editable or viewable. We get the following error when trying to edit or view a translation:

The website encountered an unexpected error. Please try again later.
Error: Call to a member function wasDefaultRevision() on null in Drupal\content_moderation\ModerationInformation->hasPendingRevision() (line 155 of core/modules/content_moderation/src/ModerationInformation.php).
Drupal\content_moderation\ModerationInformation->hasPendingRevision(Object) (Line: 58)
Drupal\content_moderation\Access\LatestRevisionCheck->access(Object, Object, Object)
call_user_func_array(Array, Array) (Line: 159)
Drupal\Core\Access\AccessManager->performCheck('access_check.latest_revision', Object) (Line: 135)
Drupal\Core\Access\AccessManager->check(Object, Object, NULL, 1) (Line: 92)
Drupal\Core\Access\AccessManager->checkNamedRoute('entity.node.latest_version', Array, Object, 1) (Line: 347)
Drupal\Core\Menu\LocalTaskManager->getTasksBuild('entity.node.canonical', Object) (Line: 378)
Drupal\Core\Menu\LocalTaskManager->getLocalTasks('entity.node.canonical', 0) (Line: 95)
Drupal\Core\Menu\Plugin\Block\LocalTasksBlock->build() (Line: 203)
Drupal\block\BlockViewBuilder::preRender(Array)
call_user_func('Drupal\block\BlockViewBuilder::preRender', Array) (Line: 378)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 195)
Drupal\Core\Render\Renderer->render(Array) (Line: 490)
Drupal\Core\Template\TwigExtension->escapeFilter(Object, Array, 'html', NULL, 1) (Line: 505)
__TwigTemplate_2ceb26e7fc995aae37335c48a584ee2063789642a929e3e2494fa4489b4bc923___519642887->block_post_footer(Array, Array) (Line: 216)
Twig\Template->displayBlock('post_footer', Array, Array) (Line: 86)
__TwigTemplate_9233483584eb43fca7bf22811c4bf4d13d0ebdfe3dcf2718f8ec5c2fa73f25d9->doDisplay(Array, Array) (Line: 443)
Twig\Template->displayWithErrorHandling(Array, Array) (Line: 414)
Twig\Template->display(Array, Array) (Line: 437)
__TwigTemplate_2ceb26e7fc995aae37335c48a584ee2063789642a929e3e2494fa4489b4bc923___519642887->doDisplay(Array, Array) (Line: 443)
Twig\Template->displayWithErrorHandling(Array, Array) (Line: 414)
Twig\Template->display(Array) (Line: 99)
__TwigTemplate_2ceb26e7fc995aae37335c48a584ee2063789642a929e3e2494fa4489b4bc923->doDisplay(Array, Array) (Line: 443)
Twig\Template->displayWithErrorHandling(Array, Array) (Line: 414)
Twig\Template->display(Array, Array) (Line: 60)
__TwigTemplate_2ba928d93a4f1bb3197cf6b40105ca07c9385def426dff6af53291a9ce79d9de->doDisplay(Array, Array) (Line: 443)
Twig\Template->displayWithErrorHandling(Array, Array) (Line: 414)
Twig\Template->display(Array, Array) (Line: 59)
__TwigTemplate_e0ace811dbc6269671389756ad6a3fc58bf0d5dfa737a8a214800f80e5a7469c->doDisplay(Array, Array) (Line: 443)
Twig\Template->displayWithErrorHandling(Array, Array) (Line: 414)
Twig\Template->display(Array) (Line: 422)
Twig\Template->render(Array) (Line: 64)
twig_render_template('profiles/custom/bax_commercial/themes/bax_commercial_theme/templates/page/page--node--therapy-area-patient.html.twig', Array) (Line: 384)
Drupal\Core\Theme\ThemeManager->render('page', Array) (Line: 437)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 195)
Drupal\Core\Render\Renderer->render(Array) (Line: 490)
Drupal\Core\Template\TwigExtension->escapeFilter(Object, Array, 'html', NULL, 1) (Line: 112)
__TwigTemplate_4387f8264d610a7d659383928ff31737e535130c0c7865bf53bde01d3f11be78->block_page_wrapper(Array, Array) (Line: 216)
Twig\Template->displayBlock('page_wrapper', Array, Array) (Line: 110)
__TwigTemplate_9e134684a7a811da5710940b30a14ff1d0cedde54e6ef390495485518d185910->doDisplay(Array, Array) (Line: 443)
Twig\Template->displayWithErrorHandling(Array, Array) (Line: 414)
Twig\Template->display(Array, Array) (Line: 70)
__TwigTemplate_4387f8264d610a7d659383928ff31737e535130c0c7865bf53bde01d3f11be78->doDisplay(Array, Array) (Line: 443)
Twig\Template->displayWithErrorHandling(Array, Array) (Line: 414)
Twig\Template->display(Array) (Line: 422)
Twig\Template->render(Array) (Line: 64)
twig_render_template('profiles/custom/bax_commercial/themes/bax_commercial_theme/templates/html/html.html.twig', Array) (Line: 384)
Drupal\Core\Theme\ThemeManager->render('html', Array) (Line: 437)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 195)
Drupal\Core\Render\Renderer->render(Array) (Line: 147)
Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}() (Line: 582)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 148)
Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object, Object) (Line: 90)
Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object, 'kernel.view', Object)
call_user_func(Array, Object, 'kernel.view', Object) (Line: 111)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.view', Object) (Line: 156)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 693)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
Sam152’s picture

You are reporting an issue that was also reported here. I'm going to open a new issue to capture this problem: #3062900: The combination of content moderation, translations and cancelling user accounts is broken..