Steps to reproduce:

1. Enable moderation on articles.
2. Create an article in draft state.
3. Edit and publish it.
4. Delete it.

Expected: node is deleted and user is redirected to admin/content.
Actual: the following error happens: Fatal error: Call to a member function getRevisionId() on null in /var/www/drupal8/modules/contrib/workbench_moderation/src/ModerationInformation.php on line 167

CommentFileSizeAuthor
#12 deleting_a_moderated-2657914-12.patch2.26 KBjuampynr
#8 interdiff.txt1.83 KBjuampynr
#8 deleting_a_moderated-2657914-8.patch2.82 KBjuampynr
#3 deleting_a_moderated-2657914-2.patch2.36 KBjuampynr
#2 deleting_a_moderated-2657914-2-test-only.patch1.67 KBjuampynr

Comments

juampynr created an issue. See original summary.

juampynr’s picture

juampynr commented
Title: Deleting a noderated node causes a fatal error » Deleting a moderated node causes a fatal error
Status: Active » Needs review
StatusFileSize
new deleting_a_moderated-2657914-2-test-only.patch1.67 KB

Here is a test that demonstrates it.

juampynr’s picture

juampynr commented
StatusFileSize
new deleting_a_moderated-2657914-2.patch2.36 KB

And here is a patch that fixes it.

The last submitted patch, 2: deleting_a_moderated-2657914-2-test-only.patch, failed testing.

The last submitted patch, 2: deleting_a_moderated-2657914-2-test-only.patch, failed testing.

The last submitted patch, 2: deleting_a_moderated-2657914-2-test-only.patch, failed testing.

Crell’s picture

Crell commented

As discussed in IRC, let's move the check to bundleFormRedirect(). It seems cleaner there.

juampynr’s picture

juampynr commented
StatusFileSize
new deleting_a_moderated-2657914-8.patch2.82 KB
new interdiff.txt1.83 KB

Here it its.

Crell’s picture

Crell commented
Status: Needs review » Needs work 
  1. +++ b/src/ModerationInformation.php
@@ -178,11 +178,18 @@ class ModerationInformation implements ModerationInformationInterface {
   /**
-   * {@inheritdoc}
+   * Checks if the entity has a revision under moderation which differs from the
+   * default one. This happens when the entity is published and someone created
+   * a new draft.
    */

    This is a coding standards violation. The @inheritdoc is necessary. 

  2. +++ b/src/ModerationInformation.php
@@ -178,11 +178,18 @@ class ModerationInformation implements ModerationInformationInterface {
+    // If the entity has just been deleted, then we won't get a revision.
+    $latest_revision_id = $this->getLatestRevisionId($entity->getEntityTypeId(), $entity->id());
+    return $latest_revision_id && !($latest_revision_id == $this->getDefaultRevisionId($entity->getEntityTypeId(), $entity->id()));

    There is always a latestRevisionId(), unless core has a bug. (And in some cases core does have a bug, which has its own issue somewhere.)

    The fix we discussed belongs in the submit callback itself, not down here in the API call.

juampynr’s picture

juampynr commented

Discussed it with @Crell. We will take the chance to make hasForwardRevision() to return a boolean value.

juampynr’s picture

juampynr commented

From IRC:

<Crell> So… I had been thinking that we should only be touching the callback, but talking it through, I cans see the reason to guard further down.  That does seem to be the more defensive approach.  (If there is no revision at all, then there is no forward revision, either, naturally.)
<Crell> So I think then it's just down to coding style and docblock style issues.
juampynr’s picture

juampynr commented
Status: Needs work » Needs review
StatusFileSize
new deleting_a_moderated-2657914-12.patch2.26 KB

I looked at the interface and saw that the error happens because getDefaultRevisionId() is not doing what the interface says: to return a NULL if there is no entity. I adjusted it and now the error is gone without touching anything else.

  • Crell committed 0d12726 on 8.x-1.x authored by juampynr 
    Issue #2657914 by juampynr: Deleting a moderated node causes a fatal...
Crell’s picture

Crell commented
Status: Needs review » Fixed

Interfaces: Solving your problems since 2005. :-) Thanks, Juampy!

Status: Fixed » Closed (fixed)

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