Issue #2673980 by dww, Berdir, AdamPS, Lendude, xjm, alexpott, plach:...

Issue #2673980 by dww, Berdir, AdamPS, Lendude, xjm, alexpott, plach: Follow-up fixes for paths in RSS views based on fields.

core/modules/views/src/Plugin/views/row/RssFields.php

@@ -3,6 +3,7 @@
 namespace Drupal\views\Plugin\views\row;
 
 use Drupal\Core\Form\FormStateInterface;
+use Drupal\Core\Url;
 
 /**
  * Renders an RSS item based on fields.
@@ -54,7 +55,7 @@ public function buildOptionsForm(&$form, FormStateInterface $form_state) {
     $form['link_field'] = [
       '#type' => 'select',
       '#title' => $this->t('Link field'),
-      '#description' => $this->t('The field that is going to be used as the RSS item link for each row. This must be a drupal relative path.'),
+      '#description' => $this->t('The field that is going to be used as the RSS item link for each row. This must either be an internal unprocessed path like "node/123" or a processed, root-relative URL as produced by fields like "Link to content".'),
       '#options' => $view_fields_labels,
       '#default_value' => $this->options['link_field'],
       '#required' => TRUE,
@@ -212,11 +213,20 @@ public function getField($index, $field_id) {
    *   A string with an absolute URL.
    */
   protected function getAbsoluteUrl($url_string) {
-    $host = \Drupal::request()->getSchemeAndHttpHost();
-
-    // @todo Views should expect and store a leading /.
-    // @see https://www.drupal.org/node/2423913
-    return $host . '/' . ltrim($url_string, '/');
+    // If the given URL already starts with a leading slash, it's been processed
+    // and we need to simply make it an absolute path by prepending the host.
+    if (strpos($url_string, '/') === 0) {
+      $host = \Drupal::request()->getSchemeAndHttpHost();
+      // @todo Views should expect and store a leading /.
+      // @see https://www.drupal.org/node/2423913
+      return $host . $url_string;
+    }
+    // Otherwise, this is an unprocessed path (e.g. node/123) and we need to run
+    // it through a Url object to allow outbound path processors to run (path
+    // aliases, language prefixes, etc).
+    else {
+      return Url::fromUserInput('/' . $url_string)->setAbsolute()->toString();
+    }
   }
 
 }

core/modules/views/tests/modules/views_test_config/test_views/views.view.test_display_feed.yml

@@ -162,6 +162,72 @@ display:
           absolute: false
           entity_type: node
           plugin_id: entity_link
+        nid:
+          id: nid
+          table: node_field_data
+          field: nid
+          relationship: none
+          group_type: group
+          admin_label: ''
+          label: ''
+          exclude: false
+          alter:
+            alter_text: true
+            text: 'node/{{ nid }}'
+            make_link: false
+            path: ''
+            absolute: false
+            external: false
+            replace_spaces: false
+            path_case: none
+            trim_whitespace: false
+            alt: ''
+            rel: ''
+            link_class: ''
+            prefix: ''
+            suffix: ''
+            target: ''
+            nl2br: false
+            max_length: 0
+            word_boundary: true
+            ellipsis: true
+            more_link: false
+            more_link_text: ''
+            more_link_path: ''
+            strip_tags: false
+            trim: false
+            preserve_tags: ''
+            html: false
+          element_type: ''
+          element_class: ''
+          element_label_type: ''
+          element_label_class: ''
+          element_label_colon: false
+          element_wrapper_type: ''
+          element_wrapper_class: ''
+          element_default_classes: true
+          empty: ''
+          hide_empty: false
+          empty_zero: false
+          hide_alter_empty: true
+          click_sort_column: value
+          type: number_integer
+          settings:
+            thousand_separator: ''
+            prefix_suffix: false
+          group_column: value
+          group_columns: {  }
+          group_rows: true
+          delta_limit: 0
+          delta_offset: 0
+          delta_reversed: false
+          delta_first_last: false
+          multi_type: separator
+          separator: ', '
+          field_api_classes: false
+          entity_type: node
+          entity_field: nid
+          plugin_id: field
       filters:
         status:
           expose:

core/modules/views/tests/src/Functional/Plugin/DisplayFeedTest.php

@@ -3,6 +3,7 @@
 namespace Drupal\Tests\views\Functional\Plugin;
 
 use Drupal\Core\Url;
+use Drupal\Tests\Traits\Core\PathAliasTestTrait;
 use Drupal\Tests\views\Functional\ViewTestBase;
 use Drupal\views\Views;
 
@@ -14,6 +15,8 @@
  */
 class DisplayFeedTest extends ViewTestBase {
 
+  use PathAliasTestTrait;
+
   /**
    * Views used by this test.
    *
@@ -122,13 +125,29 @@ public function testFeedFieldOutput() {
         ],
       ],
     ]);
+
+    // Create an alias to verify that outbound processing runs on the link and
+    // ensure that the node actually contains that.
+    $this->createPathAlias('/node/' . $node->id(), '/the-article-alias');
+
     $node_link = $node->toUrl()->setAbsolute()->toString();
+    $this->assertContains('/the-article-alias', $node_link);
 
     $this->drupalGet('test-feed-display-fields.xml');
     $this->assertEquals($node_title, $this->getSession()->getDriver()->getText('//item/title'));
     $this->assertEquals($node_link, $this->getSession()->getDriver()->getText('//item/link'));
     // Verify HTML is properly escaped in the description field.
     $this->assertRaw('<p>A paragraph</p>');
+
+    // Change the display to use the nid field, which is rewriting output as
+    // 'node/{{ nid }}' and make sure things are still working.
+    $view = Views::getView('test_display_feed');
+    $display = &$view->storage->getDisplay('feed_2');
+    $display['display_options']['row']['options']['link_field'] = 'nid';
+    $view->save();
+    $this->drupalGet('test-feed-display-fields.xml');
+    $this->assertEquals($node_title, $this->getSession()->getDriver()->getText('//item/title'));
+    $this->assertEquals($node_link, $this->getSession()->getDriver()->getText('//item/link'));
   }
 
   /**

core/modules/views/tests/src/Functional/Plugin/DisplayFeedTranslationTest.php

@@ -2,8 +2,10 @@
 
 namespace Drupal\Tests\views\Functional\Plugin;
 
+use Drupal\Tests\Traits\Core\PathAliasTestTrait;
 use Drupal\Tests\views\Functional\ViewTestBase;
 use Drupal\language\Entity\ConfigurableLanguage;
+use Drupal\node\Entity\Node;
 
 /**
  * Tests the feed display plugin with translated content.
@@ -13,6 +15,8 @@
  */
 class DisplayFeedTranslationTest extends ViewTestBase {
 
+  use PathAliasTestTrait;
+
   /**
    * Views used by this test.
    *
@@ -89,6 +93,7 @@ public function testFeedFieldOutput() {
       ],
       'langcode' => 'en',
     ]);
+
     $es_translation = $node->addTranslation('es');
     $es_translation->set('title', 'es');
     $es_translation->set('body', [['value' => 'Algo en Español']]);
@@ -99,6 +104,30 @@ public function testFeedFieldOutput() {
     $pt_br_translation->set('body', [['value' => 'Algo em Português']]);
     $pt_br_translation->save();
 
+    // First, check everything with raw node paths (e.g. node/1).
+    $this->checkFeedResults('raw-node-path', $node);
+
+    // Now, create path aliases for each translation.
+    $node_path = '/node/' . $node->id();
+    $this->createPathAlias($node_path, "$node_path/en-alias");
+    $this->createPathAlias($node_path, "$node_path/es-alias", 'es');
+    $this->createPathAlias($node_path, "$node_path/pt-br-alias", 'pt-br');
+    // Save the node again, to clear the cache on the feed.
+    $node->save();
+    // Assert that all the results are correct using path aliases.
+    $this->checkFeedResults('path-alias', $node);
+  }
+
+  /**
+   * Checks the feed results for the given style of node links.
+   *
+   * @param string $link_style
+   *   What style of links do we expect? Either 'raw-node-path' or 'path-alias'.
+   *   Only used for human-readable assert failure messages.
+   * @param \Drupal\node\Entity\Node $node
+   *   The node entity that's been created.
+   */
+  protected function checkFeedResults($link_style, Node $node) {
     /** @var \Drupal\Core\Language\LanguageManagerInterface $languageManager */
     $language_manager = \Drupal::languageManager()->reset();
 
@@ -131,7 +160,7 @@ public function testFeedFieldOutput() {
 
     $items = $this->getSession()->getDriver()->find('//channel/item');
     // There should only be 3 items in the feed.
-    $this->assertCount(3, $items);
+    $this->assertCount(3, $items, "$link_style: 3 items in feed");
 
     // Don't rely on the sort order of the items in the feed. Instead, each
     // item's title is the langcode for that item. Iterate over all the items,
@@ -140,13 +169,13 @@ public function testFeedFieldOutput() {
     // what we expect for the given langcode.
     foreach ($items as $item) {
       $title_element = $item->findAll('xpath', 'title');
-      $this->assertCount(1, $title_element);
+      $this->assertCount(1, $title_element, "$link_style: Missing title element");
       $langcode = $title_element[0]->getText();
-      $this->assertArrayHasKey($langcode, $expected);
+      $this->assertArrayHasKey($langcode, $expected, "$link_style: Missing expected output for $langcode");
       foreach ($expected[$langcode] as $key => $expected_value) {
         $elements = $item->findAll('xpath', $key);
-        $this->assertCount(1, $elements);
-        $this->assertEquals($expected_value, $elements[0]->getText());
+        $this->assertCount(1, $elements, "$link_style: Xpath $key missing");
+        $this->assertEquals($expected_value, $elements[0]->getText(), "$link_style: Unexpected value for $key");
       }
     }
   }

core/modules/views/tests/src/Kernel/Plugin/RssFieldsTest.php

+<?php
+
+namespace Drupal\Tests\views\Kernel\Plugin;
+
+use Drupal\Tests\node\Traits\ContentTypeCreationTrait;
+use Drupal\Tests\node\Traits\NodeCreationTrait;
+use Drupal\Tests\user\Traits\UserCreationTrait;
+use Drupal\Tests\views\Kernel\ViewsKernelTestBase;
+use Drupal\views\Views;
+
+/**
+ * Tests \Drupal\views\Plugin\views\row\RssFields.
+ *
+ * @group views
+ */
+class RssFieldsTest extends ViewsKernelTestBase {
+  use NodeCreationTrait;
+  use ContentTypeCreationTrait;
+  use UserCreationTrait;
+
+  /**
+   * {@inheritdoc}
+   */
+  public static $modules = ['node', 'field', 'text', 'filter'];
+
+  /**
+   * {@inheritdoc}
+   */
+  public static $testViews = ['test_display_feed'];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp($import_test_views = TRUE) {
+    parent::setUp($import_test_views);
+
+    $this->installConfig(['node', 'filter']);
+    $this->installEntitySchema('user');
+    $this->installEntitySchema('node');
+    $this->installEntitySchema('path_alias');
+    $this->createContentType(['type' => 'article']);
+  }
+
+  /**
+   * Tests correct processing of link fields.
+   *
+   * This overlaps with \Drupal\Tests\views\Functional\Plugin\DisplayFeedTest to
+   * ensure that root-relative links also work in a scenario without
+   * subdirectory.
+   */
+  public function testLink() {
+    // Set up the current user as uid 1 so the test doesn't need to deal with
+    // permission.
+    $this->setUpCurrentUser(['uid' => 1]);
+
+    $node = $this->createNode([
+      'type' => 'article',
+      'title' => 'Article title',
+      'body' => [
+        0 => [
+          'value' => 'A paragraph',
+          'format' => filter_default_format(),
+        ],
+      ],
+    ]);
+
+    $node_url = $node->toUrl()->setAbsolute()->toString();
+
+    $renderer = $this->container->get('renderer');
+
+    $view = Views::getView('test_display_feed');
+    $output = $view->preview('feed_2');
+    $output = (string) $renderer->renderRoot($output);
+    $this->assertContains('<link>' . $node_url . '</link>', $output);
+  }
+
+}