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

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

juampynr created an issue. See original summary.

juampynr’s picture

Title: Deleting a noderated node causes a fatal error » Deleting a moderated node causes a fatal error
Status: Active » Needs review
FileSize
1.67 KB

Here is a test that demonstrates it.

juampynr’s picture

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

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

juampynr’s picture

Crell’s picture

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

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

juampynr’s picture

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

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

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.