Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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->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->entityRevisionDelete(Object) (Line: 118)
content_moderation_entity_revision_delete(Object)
call_user_func_array('content_moderation_entity_revision_delete', Array) (Line: 403)
Drupal\Core\Extension\ModuleHandler->invokeAll('entity_revision_delete', Array) (Line: 206)
Drupal\Core\Entity\EntityStorageBase->invokeHook('revision_delete', Object) (Line: 756)
Drupal\Core\Entity\ContentEntityStorageBase->invokeHook('revision_delete', Object) (Line: 681)
Drupal\Core\Entity\ContentEntityStorageBase->deleteRevision('2') (Line: 117)
Drupal\node\Form\NodeRevisionDeleteForm->submitForm(Array, Object)
Comments
Comment #2
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedThis 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.
Comment #3
asghar CreditAttribution: asghar commentedHi @Berdir,
I repeated the steps as you mentioned but I could not produce the error. I tried with fresh 8.7.x branch.
Comment #4
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedQuick test case for this bug, haven't come up with a simple solution to this yet.
Comment #6
sathish.redcrackle CreditAttribution: sathish.redcrackle at Red Crackle commentedFound Duplicate: https://www.drupal.org/project/drupal/issues/2933099
Comment #7
BerdirNo, 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..?
Comment #8
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedI 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.Comment #9
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedThis 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:
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.
Comment #10
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedTrying 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.Comment #11
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedI 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 :)
Comment #12
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedWe 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.
Comment #13
sam.spinoy@gmail.com CreditAttribution: sam.spinoy@gmail.com at Adapt commentedRan 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.
Comment #14
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedOkay, putting forward
3003238-delete-cms-entity-10.patch
as my preferred approach to solving this.Comment #15
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedJust 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?
Comment #16
amateescu CreditAttribution: amateescu for Pfizer, Inc. commented#14 looks good :)
Comment #17
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedThanks @amateescu! :)
Comment #18
catchCommitted and pushed 2492cdda1c to 8.7.x and de323cd29b to 8.6.x. Thanks!
Comment #21
BerdirHuh, #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? :)
Comment #24
catchhmm I'm sure it was green when I committed it. Reverted from both branches.
Comment #25
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedWhoops! This just needed to be rerolled against #2915383: The moderation_state base field is added to all revisionable entity types even if they do not have moderation enabled.
Comment #26
jibranAs code changes has been already RTBC in #16 last interdiff only include test only change so back to RTBC.
Comment #28
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedComment #29
knyshuk.vova CreditAttribution: knyshuk.vova at Internetdevels, Drupal Ukraine Community commented+1 to RTBC
Comment #31
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedRandom fails keep reverting this, but the status should be RTBC.
Comment #33
alexpottCommitted and pushed c7beade985 to 8.8.x and d78aa97227 to 8.7.x. Thanks!
Comment #36
penyaskitoMaybe related: #3043535: Exception when deleting multiple translations of a content which has moderation enabled
Comment #37
penyaskitoActually this patch fixed that one.
Comment #39
lpeabody CreditAttribution: lpeabody commentedI 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:
Comment #40
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedYou 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..