Problem/Motivation

The code has a compareNodeRevisions function which takes the node as a parameter. But it is not used at all in the code, it is pulling it from the route match instead, which is making it impossible to reuse.

  public function compareNodeRevisions(NodeInterface $node, $left_revision, $right_revision, $filter) {
    $storage = $this->entityTypeManager()->getStorage('node');
    $route_match = \Drupal::routeMatch();
    $left_revision = $storage->loadRevision($left_revision);
    $right_revision = $storage->loadRevision($right_revision);
    $build = $this->compareEntityRevisions($route_match, $left_revision, $right_revision, $filter);
    return $build;
  }

Why $node is just passed without being used. Am I missing something? Shouldn't it be passed to compareEntityRevisions instead of the route_match, and then get rid of that weird code that reload the node again using the route match and the left revision. Moreover, using the route match just makes it impossible to reuse in a route that has different parameters names or even worse a cron environment.

  public function compareEntityRevisions(RouteMatchInterface $route_match, ContentEntityInterface $left_revision, ContentEntityInterface $right_revision, $filter) {
    $entity_type_id = $left_revision->getEntityTypeId();
    /** @var \Drupal\Core\Entity\EntityInterface $entity */
    $entity = $route_match->getParameter($entity_type_id);

    $entity_type_id = $entity->getEntityTypeId();
    $storage = $this->entityTypeManager()->getStorage($entity_type_id);

I'll probably make a patch.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork diff-3385760

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:

Comments

phjou created an issue. See original summary.

phjou’s picture

Title: compareNodeRevisions ignore parameters » compareNodeRevisions ignore the node parameter
phjou’s picture

Issue summary: View changes
phjou’s picture

StatusFileSize
new2.42 KB

Just uploaded the patch, also pushed on the branch

phjou’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 4: diff-3385760-node_param_is_ignored-3.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

silvi.addweb’s picture

StatusFileSize
new2.83 KB

re-roll patch according to the latest code changes.

heddn’s picture

Version: 8.x-1.x-dev » 2.x-dev
Issue tags: +Needs reroll

prem suthar made their first commit to this issue’s fork.

acbramley’s picture

Status: Needs work » Closed (outdated)

$node is now used in compareNodeRevisions.