Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Node::getRevisionAuthor deprecation not detected #210

Closed
timwood opened this issue Sep 30, 2021 · 6 comments
Closed

Node::getRevisionAuthor deprecation not detected #210

timwood opened this issue Sep 30, 2021 · 6 comments

Comments

@timwood
Copy link

timwood commented Sep 30, 2021

How is drupal-check installed?

drupal-check is installed as a dependency to my project with composer

Environment:

  • OS: Linux
  • PHP Version: 7.3
  • Drupal core: 8.9

Describe the bug
Node::getRevisionAuthor was deprecated in favor of RevisionLogInterface::getRevisionUser and removed in Drupal 9.0.0. This was reported in change record https://www.drupal.org/node/3069750 and is also mentioned on https://api.drupal.org/api/drupal/core%21modules%21node%21src%21Entity%21Node.php/function/Node%3A%3AgetRevisionAuthor/8.9.x.

I also opened an issue on the Drupal Upgrade Status module on Drupal.org.

@mglaman mglaman transferred this issue from mglaman/drupal-check Oct 6, 2021
@mglaman
Copy link
Owner

mglaman commented Oct 6, 2021

thanks for the report, @timwood! Can you provide a failing code sample? I have a feeling that PHPStan doesn't know that an object is actually a NodeInterface object, but if you added type hinting it would. I'd have a better idea after a sample

@timwood
Copy link
Author

timwood commented Oct 8, 2021

thanks for the report, @timwood! Can you provide a failing code sample? I have a feeling that PHPStan doesn't know that an object is actually a NodeInterface object, but if you added type hinting it would. I'd have a better idea after a sample

Hi @mglaman. Not sure how complete of a code sample you need, but this is the one line I changed within a function in the .module file to fix the issue in our custom module.

-  $params['nodeRevisionAuthor'] = $node->getRevisionAuthor()->getDisplayName();
+  $params['nodeRevisionUser'] = $node->getRevisionUser()->getDisplayName();

At the top we only had:

use Drupal\Core\Entity\EntityInterface;

As part of the fix I added:

use Drupal\Core\Entity\EntityInterface;
use Drupal\Core\Entity\RevisionLogInterface;

@mglaman
Copy link
Owner

mglaman commented Oct 8, 2021

@timwood that works. How is $node loaded or brought into scope? Is it just $node = $storage->load(1).

The quick fix whenever you get an entity from storage is to always do assert($node instanceof NodeInterface) if you know it exists. Or a if(!$node instanceof NodeInterface) {} error (which catches null and type hints)

@timwood
Copy link
Author

timwood commented Oct 8, 2021

@mglaman
In this case, I believe, $node is loaded from hook_entity_update or hook_entity_insert, passed to a callback function which checks the type (node in this case) and then calls another function that looks like this:

/**
 * Do some stuff
 *
 * @param Drupal\Core\Entity\EntityInterface $node
 *   The node being passed to the hook.
 */

function _function_name_obscured_to_protect_the_innocent(EntityInterface $node): void {

@mglaman
Copy link
Owner

mglaman commented Oct 8, 2021

I would update your function to replace EntityInterface with NodeInterface. Deprecation checks alone wouldn't warn this, but PHPStan running at level 0 or 1 probably would.

@mglaman
Copy link
Owner

mglaman commented Oct 13, 2021

Closing since the type hinting should fix, nothing much can be done in this library

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants