Issue #2456599 by k4v, dawehner, larowlan, rteijeiro, cutesquirrel, yched, jhodgdon: Field node_field_revision.title needs to use an entity-aware formatter in Views

core/lib/Drupal/Core/Entity/Entity.php

@@ -163,6 +163,12 @@ abstract class Entity implements EntityInterface {
     // The links array might contain URI templates set in annotations.
     $link_templates = $this->linkTemplates();
 
+    // Links pointing to the current revision point to the actual entity. So
+    // instead of using the 'revision' link, use the 'canonical' link.
+    if ($rel === 'revision' && $this instanceof RevisionableInterface && $this->isDefaultRevision()) {
+      $rel = 'canonical';
+    }
+
     if (isset($link_templates[$rel])) {
       $route_parameters = $this->urlRouteParameters($rel);
       $route_name = "entity.{$this->entityTypeId}." . str_replace(array('-', 'drupal:'), array('_', ''), $rel);
@@ -280,6 +286,10 @@ abstract class Entity implements EntityInterface {
       // The entity ID is needed as a route parameter.
       $uri_route_parameters[$this->getEntityTypeId()] = $this->id();
     }
+    if ($rel === 'revision') {
+      $uri_route_parameters[$this->getEntityTypeId() . '_revision'] = $this->getRevisionId();
+    }
+
     return $uri_route_parameters;
   }
 

core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/StringFormatter.php

@@ -9,6 +9,7 @@ namespace Drupal\Core\Field\Plugin\Field\FieldFormatter;
 
 use Drupal\Component\Utility\SafeMarkup;
 use Drupal\Core\Entity\EntityManagerInterface;
+use Drupal\Core\Entity\RevisionableInterface;
 use Drupal\Core\Field\FieldDefinitionInterface;
 use Drupal\Core\Field\FieldItemInterface;
 use Drupal\Core\Field\FormatterBase;
@@ -93,6 +94,7 @@ class StringFormatter extends FormatterBase implements ContainerFactoryPluginInt
     $form = parent::settingsForm($form, $form_state);
 
     $entity_type = $this->entityManager->getDefinition($this->fieldDefinition->getTargetEntityTypeId());
+
     $form['link_to_entity'] = [
       '#type' => 'checkbox',
       '#title' => $this->t('Link to the @entity_label', ['@entity_label' => $entity_type->getLabel()]),
@@ -119,16 +121,14 @@ class StringFormatter extends FormatterBase implements ContainerFactoryPluginInt
    */
   public function viewElements(FieldItemListInterface $items) {
     $elements = array();
-
     $url = NULL;
-    // Add support to link to the entity itself.
-    if ($this->getSetting('link_to_entity') && ($entity = $items->getEntity()) && $entity->hasLinkTemplate('canonical')) {
-      $url = $entity->urlInfo();
+    if ($this->getSetting('link_to_entity')) {
+      // For the default revision this falls back to 'canonical'
+      $url = $items->getEntity()->urlInfo('revision');
     }
 
     foreach ($items as $delta => $item) {
       $string = $this->viewValue($item);
-
       if ($url) {
         $elements[$delta] = [
           '#type' => 'link',
@@ -140,7 +140,6 @@ class StringFormatter extends FormatterBase implements ContainerFactoryPluginInt
         $elements[$delta] = ['#markup' => $string];
       }
     }
-
     return $elements;
   }
 

core/modules/field/src/Tests/String/StringFormatterTest.php

@@ -11,7 +11,7 @@ use Drupal\Component\Utility\SafeMarkup;
 use Drupal\Component\Utility\Unicode;
 use Drupal\Core\Entity\Display\EntityViewDisplayInterface;
 use Drupal\Core\Entity\FieldableEntityInterface;
-use Drupal\entity_test\Entity\EntityTest;
+use Drupal\entity_test\Entity\EntityTestRev;
 use Drupal\field\Entity\FieldConfig;
 use Drupal\field\Entity\FieldStorageConfig;
 use Drupal\simpletest\KernelTestBase;
@@ -60,9 +60,9 @@ class StringFormatterTest extends KernelTestBase {
     $this->installConfig(array('system', 'field'));
     $this->installSchema('system', 'router');
     \Drupal::service('router.builder')->rebuild();
-    $this->installEntitySchema('entity_test');
+    $this->installEntitySchema('entity_test_rev');
 
-    $this->entityType = 'entity_test';
+    $this->entityType = 'entity_test_rev';
     $this->bundle = $this->entityType;
     $this->fieldName = Unicode::strtolower($this->randomMachineName());
 
@@ -113,7 +113,7 @@ class StringFormatterTest extends KernelTestBase {
     $value .= "\n\n<strong>" . $this->randomString() . '</strong>';
     $value .= "\n\n" . $this->randomString();
 
-    $entity = EntityTest::create(array());
+    $entity = EntityTestRev::create(array());
     $entity->{$this->fieldName}->value = $value;
 
     // Verify that all HTML is escaped and newlines are retained.
@@ -125,6 +125,10 @@ class StringFormatterTest extends KernelTestBase {
     $build = $entity->{$this->fieldName}->view();
     $this->assertTrue(!isset($build[0]['#cache']), format_string('The string formatter has no cache tags.'));
 
+    $value = $this->randomMachineName();
+    $entity->{$this->fieldName}->value = $value;
+    $entity->save();
+
     // Set the formatter to link to the entity.
     $this->display->setComponent($this->fieldName, [
       'type' => 'string',
@@ -134,13 +138,28 @@ class StringFormatterTest extends KernelTestBase {
     ]);
     $this->display->save();
 
-    $value = $this->randomMachineName();
-    $entity->{$this->fieldName}->value = $value;
-    $entity->save();
-
     $this->renderEntityFields($entity, $this->display);
     $this->assertLink($value, 0);
     $this->assertLinkByHref($entity->url());
-  }
 
+    // $entity->url('revision') falls back to the canonical URL if this is no
+    // revision.
+    $this->assertLinkByHref($entity->url('revision'));
+
+    // Make the entity a new revision.
+    $old_revision_id = $entity->getRevisionId();
+    $entity->setNewRevision(TRUE);
+    $value2 = $this->randomMachineName();
+    $entity->{$this->fieldName}->value = $value2;
+    $entity->save();
+    $entity_new_revision = \Drupal::entityManager()->getStorage('entity_test_rev')->loadRevision($old_revision_id);
+
+    $this->renderEntityFields($entity, $this->display);
+    $this->assertLink($value2, 0);
+    $this->assertLinkByHref($entity->url('revision'));
+
+    $this->renderEntityFields($entity_new_revision, $this->display);
+    $this->assertLink($value, 0);
+    $this->assertLinkByHref('/entity_test_rev/' . $entity_new_revision->id() . '/revision/' . $entity_new_revision->getRevisionId() . '/view');
+  }
 }

core/modules/node/config/schema/node.views.schema.yml

@@ -137,14 +137,6 @@ views.field.node_path:
       type: boolean
       label: 'Use absolute link (begins with "http://")'
 
-views.field.node_revision:
-  type: views.field.node
-  label: 'Node revision'
-  mapping:
-    link_to_node_revision:
-      type: boolean
-      label: 'Link this field to its content revision'
-
 views.field.node_revision_link:
   type: views_field
   label: 'Link to a node revision'

core/modules/node/node.routing.yml

@@ -50,7 +50,7 @@ entity.node.version_history:
   options:
     _node_operation_route: TRUE
 
-node.revision_show:
+entity.node.revision:
   path: '/node/{node}/revisions/{node_revision}/view'
   defaults:
     _controller: '\Drupal\node\Controller\NodeController::revisionShow'

core/modules/node/src/Controller/NodeController.php

@@ -192,7 +192,7 @@ class NodeController extends ControllerBase implements ContainerInjectionInterfa
             '#theme' => 'username',
             '#account' => $revision_author,
           );
-          $row[] = $this->t('!date by !username', array('!date' => $this->l($this->dateFormatter->format($revision->revision_timestamp->value, 'short'), new Url('node.revision_show', array('node' => $node->id(), 'node_revision' => $vid))), '!username' => drupal_render($username)))
+          $row[] = $this->t('!date by !username', array('!date' => $this->l($this->dateFormatter->format($revision->revision_timestamp->value, 'short'), new Url('entity.node.revision', array('node' => $node->id(), 'node_revision' => $vid))), '!username' => drupal_render($username)))
             . (($revision->revision_log->value != '') ? '<p class="revision-log">' . Xss::filter($revision->revision_log->value) . '</p>' : '');
 
           if ($revert_permission) {

core/modules/node/src/Entity/Node.php

@@ -63,6 +63,7 @@ use Drupal\user\UserInterface;
  *     "delete-form" = "/node/{node}/delete",
  *     "edit-form" = "/node/{node}/edit",
  *     "version-history" = "/node/{node}/revisions",
+ *     "revision" = "/node/{node}/revisions/{node_revision}/view",
  *   }
  * )
  */

core/modules/node/src/NodeViewsData.php

@@ -291,8 +291,6 @@ class NodeViewsData extends EntityViewsData {
     $data['node_field_revision']['status']['filter']['type'] = 'yes-no';
     $data['node_field_revision']['status']['filter']['use_equal'] = TRUE;
 
-    $data['node_field_revision']['title']['field']['id'] = 'node_revision';
-
     $data['node_field_revision']['langcode']['help'] = t('The language of the content or translation.');
 
     $data['node_revision']['link_to_revision'] = array(

core/modules/node/src/Plugin/views/field/Revision.php

@@ -1,80 +0,0 @@
-<?php
-
-/**
- * @file
- * Definition of Drupal\node\Plugin\views\field\Revision.
- */
-
-namespace Drupal\node\Plugin\views\field;
-
-use Drupal\Core\Form\FormStateInterface;
-use Drupal\Core\Url;
-use Drupal\views\ResultRow;
-use Drupal\views\ViewExecutable;
-use Drupal\views\Plugin\views\display\DisplayPluginBase;
-use Drupal\node\Plugin\views\field\Node;
-
-/**
- * A basic node_revision handler.
- *
- * @ingroup views_field_handlers
- *
- * @ViewsField("node_revision")
- */
-class Revision extends Node {
-
-  /**
-   * Overrides \Drupal\node\Plugin\views\field\Node::init().
-   */
-  public function init(ViewExecutable $view, DisplayPluginBase $display, array &$options = NULL) {
-    parent::init($view, $display, $options);
-
-    if (!empty($this->options['link_to_node_revision'])) {
-      $this->additional_fields['vid'] = 'vid';
-      $this->additional_fields['nid'] = 'nid';
-    }
-  }
-  protected function defineOptions() {
-    $options = parent::defineOptions();
-    $options['link_to_node_revision'] = array('default' => FALSE);
-    return $options;
-  }
-
-  /**
-   * Provide link to revision option.
-   */
-  public function buildOptionsForm(&$form, FormStateInterface $form_state) {
-    $form['link_to_node_revision'] = array(
-      '#title' => $this->t('Link this field to its content revision'),
-      '#description' => $this->t('This will override any other link you have set.'),
-      '#type' => 'checkbox',
-      '#default_value' => !empty($this->options['link_to_node_revision']),
-    );
-    parent::buildOptionsForm($form, $form_state);
-  }
-
-  /**
-   * Prepares link to the node revision.
-   *
-   * @param string $data
-   *   The XSS safe string for the link text.
-   * @param \Drupal\views\ResultRow $values
-   *   The values retrieved from a single row of a view's query result.
-   *
-   * @return string
-   *   Returns a string for the link text.
-   */
-  protected function renderLink($data, ResultRow $values) {
-    if (!empty($this->options['link_to_node_revision']) && $data !== NULL && $data !== '') {
-      $this->options['alter']['make_link'] = TRUE;
-      $nid = $this->getValue($values, 'nid');
-      $vid = $this->getValue($values, 'vid');
-      $this->options['alter']['url'] = Url::fromRoute('node.revision_show', ['node' => $nid, 'node_revision' => $vid]);
-    }
-    else {
-      return parent::renderLink($data, $values);
-    }
-    return $data;
-  }
-
-}

core/modules/node/src/Plugin/views/wizard/NodeRevision.php

@@ -103,10 +103,8 @@ class NodeRevision extends WizardPluginBase {
     $display_options['fields']['title']['alter']['html'] = 0;
     $display_options['fields']['title']['hide_empty'] = 0;
     $display_options['fields']['title']['empty_zero'] = 0;
-    $display_options['fields']['title']['link_to_node'] = 0;
-    $display_options['fields']['title']['link_to_node_revision'] = 1;
-    $display_options['fields']['title']['plugin_id'] = 'node_revision';
-
+    $display_options['fields']['title']['settings']['link_to_entity'] = 0;
+    $display_options['fields']['title']['plugin_id'] = 'field';
     return $display_options;
   }
 

core/modules/simpletest/src/Tests/SimpleTestBrowserTest.php

@@ -130,7 +130,7 @@ class SimpleTestBrowserTest extends WebTestBase {
 
     $tests = array(
       // A KernelTestBase test.
-      'Drupal\field\Tests\String\StringFormatterTest',
+      'Drupal\system\Tests\DrupalKernel\DrupalKernelTest',
       // A PHPUnit unit test.
       'Drupal\Tests\action\Unit\Menu\ActionLocalTasksTest',
       // A PHPUnit functional test.

core/modules/system/tests/modules/entity_test/entity_test.routing.yml

@@ -55,6 +55,13 @@ entity.entity_test.collection:
   requirements:
     _access: 'TRUE'
 
+entity.entity_test_rev.revision:
+  path: '/entity_test_rev/{entity_test_rev}/revision/{entity_test_rev_revision}/view'
+  defaults:
+    _entity_view: 'entity_test_rev'
+  requirements:
+    _access: 'TRUE'
+
 entity.block.test_operation:
   path: '/admin/structure/block/manage/{block}/test_operation'
   defaults:

core/modules/system/tests/modules/entity_test/src/Entity/EntityTestMulRev.php

@@ -44,6 +44,7 @@ use Drupal\entity_test\Entity\EntityTestRev;
  *     "canonical" = "/entity_test_mulrev/manage/{entity_test_mulrev}",
  *     "delete-form" = "/entity_test/delete/entity_test_mulrev/{entity_test_mulrev}",
  *     "edit-form" = "/entity_test_mulrev/manage/{entity_test_mulrev}",
+ *     "revision" = "/entity_test_mulrev/{entity_test_mulrev}/revision/{entity_test_mulrev_revision}/view",
  *   }
  * )
  */

core/modules/system/tests/modules/entity_test/src/Entity/EntityTestRev.php

@@ -40,7 +40,8 @@ use Drupal\entity_test\Entity\EntityTest;
  *   links = {
  *     "canonical" = "/entity_test_rev/manage/{entity_test_rev}",
  *     "delete-form" = "/entity_test/delete/entity_test_rev/{entity_test_rev}",
- *     "edit-form" = "/entity_test_rev/manage/{entity_test_rev}"
+ *     "edit-form" = "/entity_test_rev/manage/{entity_test_rev}",
+ *     "revision" = "/entity_test_rev/{entity_test_rev}/revision/{entity_test_rev_revision}/view",
  *   }
  * )
  */