Problem/Motivation

The block visibility condition assumes there will be an existing revision if the node object is not empty when the condition is checked. This is not guaranteed to be true. If the node object is not empty and there's no revision, it causes a fatal error. This is happening on Panopoly and a child distribution; there may be other cases where the node object is not empty but there is not an existing revision.

Steps to reproduce

  1. Create a Panopoly 3.0.x site using its composer template: composer create-project panopoly/panopoly-composer-template:10.x-dev panopoly3 --no-interaction
  2. Install the default profile: drush -y si panopoly
  3. Add the block_condition_published module via Composer and enable it.
  4. In the Block Layout admin page, add a block to a region and configure it to display only if the node is unpublished. For example, add the Who's Online block to the Highlighted region and check the boxes for Published and Negate the condition.
  5. Save the block config and the layout.
  6. Visit a published node provided by the Panopoly install.
  7. Note the block does not appear. This is the expected behavior.
  8. Edit the node. Uncheck the Published box and save.
  9. Note the block appears. This is the expected behavior.
  10. Visit the Add Content page for a content type, for example /node/add/panopoly_content_page.
  11. Note the error: "Call to a member function get() on null in Drupal\block_condition_published\Plugin\Condition\NodePublishedState->evaluate() (line 136 of modules/contrib/block_condition_published/src/Plugin/Condition/NodePublishedState.php)."

Proposed resolution

Check to make sure the revision ID exists before trying to get the published status.

Remaining tasks

MR and test.

User interface changes

None.

API changes

None.

Data model changes

None.

Issue fork block_condition_published-3488761

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

cboyden created an issue. See original summary.

cboyden opened merge request !2

cboyden’s picture

cboyden commented
Assigned: cboyden » Unassigned
Status: Active » Needs review

The MR adds a simple check for a non-empty vid from the latest revision. If it's empty, the condition is treated the same as if the node object was empty.

grienauer’s picture

grienauer
he/him
Primary language German
Location Vienna
commented
Assigned: Unassigned » granik

Thank you for the MR! we will check it out.

granik’s picture

granik
he/him
Location Vienna
commented
Status: Needs review » Needs work

@cboyden thanks for your catch!
The problem can be, that if you skip the validation in your case, the module will not check if node is published or not. I guess there should be a check like $node->isPublished(), instead just returning TRUE.

p.s. this check has been added just because of drupal moderation states(more info here), so in case if the regular published/unpublished status is used, there is no need to check the revision state. The code just checks all possible user cases.

I'll try to work on this issue as soon as possible.

  • granik committed 1f57a810 on 1.0.x 
    Issue #3488761 by cboyden: Node object without revision causes fatal...
granik’s picture

granik
he/him
Location Vienna
commented
Version: 1.0.x-dev » 1.0.0-alpha2
Status: Needs work » Fixed

Committed to 1.0.x branch.

Status: Fixed » Closed (fixed)

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