Issue #2409417 by cafuego, jibran, larowlan: Disabling feed view display does...

Issue #2409417 by cafuego, jibran, larowlan: Disabling feed view display does not remove feed header from display it is attached to

core/modules/views/src/Tests/Plugin/DisplayAttachmentTest.php

@@ -7,6 +7,8 @@
 
 namespace Drupal\views\Tests\Plugin;
 
+use Drupal\views\Views;
+
 /**
  * Tests the attachment display plugin.
  *
@@ -20,12 +22,22 @@ class DisplayAttachmentTest extends PluginTestBase {
    *
    * @var array
    */
-  public static $testViews = array('test_display_attachment');
+  public static $testViews = array('test_display_attachment', 'test_attached_disabled');
+
+  /**
+   * Modules to enable.
+   *
+   * @var array
+   */
+  public static $modules = array('node', 'views');
 
   protected function setUp() {
     parent::setUp();
 
     $this->enableViewsTestModule();
+
+    $admin_user = $this->drupalCreateUser(array('administer site configuration'));
+    $this->drupalLogin($admin_user);
   }
 
 
@@ -45,4 +57,54 @@ public function testAttachment() {
     $this->assertEqual(count($result), 1, 'The attachment is rendered before the actual view.');
   }
 
+  /**
+   * Tests that nothing is output when the attachment displays are disabled.
+   */
+  public function testDisabledAttachments() {
+    $this->drupalCreateContentType(['type' => 'page']);
+    $this->drupalCreateNode();
+
+    // Ensure that the feed_1 display is attached to the page_1 display.
+    $view = Views::getView('test_attached_disabled');
+    $view->setDisplay('page_1');
+    $attached_displays = $view->display_handler->getAttachedDisplays();
+    $this->assertTrue(in_array('attachment_1', $attached_displays), 'The attachment_1 display is attached to the page display.');
+    $this->assertTrue(in_array('attachment_2', $attached_displays), 'The attachment_2 display is attached to the page display.');
+
+    // Check that the attachments are output on the page display.
+    $this->drupalGet('test-attached-disabled');
+
+    $result = $this->xpath('//div[contains(@class, "view-content")]');
+    $this->assertEqual(count($result), 3, 'The page view and the attachments are rendered.');
+
+    $result = $this->xpath('//div[contains(@class, "attachment-before")]');
+    $this->assertEqual(count($result), 1, 'The attachment is rendered before the page view.');
+
+    $result = $this->xpath('//div[contains(@class, "attachment-after")]');
+    $this->assertEqual(count($result), 1, 'The attachment is rendered after the page view.');
+
+    // Disable the attachment_1 display.
+    $view->displayHandlers->get('attachment_1')->setOption('enabled', FALSE);
+    $view->save();
+
+    // Test that the before attachment is not displayed.
+    $this->drupalGet('/test-attached-disabled');
+    $result = $this->xpath('//div[contains(@class, "view-content")]');
+    $this->assertEqual(count($result), 2, 'The page view and only one attachment are rendered.');
+
+    $result = $this->xpath('//div[contains(@class, "attachment-before")]');
+    $this->assertEqual(count($result), 0, 'The attachment_1 is not rendered.');
+
+    // Disable the attachment_2 display.
+    $view->displayHandlers->get('attachment_2')->setOption('enabled', FALSE);
+    $view->save();
+
+    // Test that the after attachment is not displayed.
+    $this->drupalGet('/test-attached-disabled');
+    $result = $this->xpath('//div[contains(@class, "view-content")]');
+    $this->assertEqual(count($result), 1, 'The page view is rendered without attachments.');
+
+    $result = $this->xpath('//div[contains(@class, "attachment-after")]');
+    $this->assertEqual(count($result), 0, 'The attachment_2 is not rendered.');
+  }
 }

core/modules/views/src/Tests/Plugin/DisplayFeedTest.php

@@ -7,6 +7,8 @@
 
 namespace Drupal\views\Tests\Plugin;
 
+use Drupal\views\Views;
+
 /**
  * Tests the feed display plugin.
  *
@@ -20,7 +22,7 @@ class DisplayFeedTest extends PluginTestBase {
    *
    * @var array
    */
-  public static $testViews = array('test_display_feed');
+  public static $testViews = array('test_display_feed', 'test_attached_disabled');
 
   /**
    * Modules to enable.
@@ -74,4 +76,36 @@ public function testFeedOutput() {
     $this->assertTrue(strpos($feed_icon[0]['href'], 'test-feed-display.xml'), 'The feed icon was found.');
   }
 
+  /**
+   * Tests that nothing is output when the feed display is disabled.
+   */
+  public function testDisabledFeed() {
+    $this->drupalCreateContentType(['type' => 'page']);
+    $this->drupalCreateNode();
+
+    // Ensure that the feed_1 display is attached to the page_1 display.
+    $view = Views::getView('test_attached_disabled');
+    $view->setDisplay('page_1');
+    $attached_displays = $view->display_handler->getAttachedDisplays();
+    $this->assertTrue(in_array('feed_1', $attached_displays), 'The feed display is attached to the page display.');
+
+    // Check that the rss header is output on the page display.
+    $this->drupalGet('/test-attached-disabled');
+    $feed_header = $this->xpath('//link[@rel="alternate"]');
+    $this->assertEqual($feed_header[0]['type'], 'application/rss+xml', 'The feed link has the type application/rss+xml.');
+    $this->assertTrue(strpos($feed_header[0]['href'], 'test-attached-disabled.xml'), 'Page display contains the correct feed URL.');
+
+    // Disable the feed display.
+    $view->displayHandlers->get('feed_1')->setOption('enabled', FALSE);
+    $view->save();
+
+    // Ensure there is no link rel present on the page.
+    $this->drupalGet('/test-attached-disabled');
+    $result = $this->xpath('//link[@rel="alternate"]');
+    $this->assertTrue(empty($result), 'Page display does not contain a feed header.');
+
+    // Ensure the feed attachment returns 'Not found'.
+    $this->drupalGet('/test-attached-disabled.xml');
+    $this->assertResponse(404);
+  }
 }

core/modules/views/src/ViewExecutable.php

@@ -1578,8 +1578,12 @@ public function attachDisplays() {
     $this->is_attachment = TRUE;
     // Find out which other displays attach to the current one.
     foreach ($this->display_handler->getAttachedDisplays() as $id) {
-      $cloned_view = Views::executableFactory()->get($this->storage);
-      $this->displayHandlers->get($id)->attachTo($cloned_view, $this->current_display, $this->element);
+      $display_handler = $this->displayHandlers->get($id);
+      // Only attach enabled attachments.
+      if ($display_handler->isEnabled()) {
+        $cloned_view = Views::executableFactory()->get($this->storage);
+        $display_handler->attachTo($cloned_view, $this->current_display, $this->element);
+      }
     }
     $this->is_attachment = FALSE;
   }

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

+langcode: und
+status: true
+dependencies:
+  config:
+    - core.entity_view_mode.node.teaser
+  module:
+    - node
+    - user
+id: test_attached_disabled
+label: test_attached_disabled
+module: views
+description: ''
+tag: ''
+base_table: node_field_data
+base_field: nid
+core: 8.x
+display:
+  default:
+    display_plugin: default
+    id: default
+    display_title: Master
+    position: 0
+    display_options:
+      access:
+        type: perm
+        options:
+          perm: 'access content'
+      cache:
+        type: none
+        options: {  }
+      query:
+        type: views_query
+        options:
+          disable_sql_rewrite: false
+          distinct: false
+          replica: false
+          query_comment: ''
+          query_tags: {  }
+      exposed_form:
+        type: basic
+        options:
+          submit_button: Apply
+          reset_button: false
+          reset_button_label: Reset
+          exposed_sorts_label: 'Sort by'
+          expose_sort_order: true
+          sort_asc_label: Asc
+          sort_desc_label: Desc
+      pager:
+        type: full
+        options:
+          items_per_page: 10
+          offset: 0
+          id: 0
+          total_pages: null
+          expose:
+            items_per_page: false
+            items_per_page_label: 'Items per page'
+            items_per_page_options: '5, 10, 25, 50'
+            items_per_page_options_all: false
+            items_per_page_options_all_label: '- All -'
+            offset: false
+            offset_label: Offset
+          tags:
+            previous: '‹ previous'
+            next: 'next ›'
+            first: '« first'
+            last: 'last »'
+          quantity: 9
+      style:
+        type: default
+      row:
+        type: 'entity:node'
+        options:
+          view_mode: teaser
+      fields:
+        title:
+          id: title
+          table: node_field_data
+          field: title
+          entity_type: node
+          entity_field: title
+          label: ''
+          alter:
+            alter_text: false
+            make_link: false
+            absolute: false
+            trim: false
+            word_boundary: false
+            ellipsis: false
+            strip_tags: false
+            html: false
+          hide_empty: false
+          empty_zero: false
+          link_to_node: true
+          plugin_id: node
+          relationship: none
+          group_type: group
+          admin_label: ''
+          exclude: false
+          element_type: ''
+          element_class: ''
+          element_label_type: ''
+          element_label_class: ''
+          element_label_colon: true
+          element_wrapper_type: ''
+          element_wrapper_class: ''
+          element_default_classes: true
+          empty: ''
+          hide_alter_empty: true
+      filters:
+        status:
+          value: true
+          table: node_field_data
+          field: status
+          plugin_id: boolean
+          entity_type: node
+          entity_field: status
+          id: status
+          expose:
+            operator: ''
+          group: 1
+      sorts:
+        created:
+          id: created
+          table: node_field_data
+          field: created
+          order: DESC
+          entity_type: node
+          entity_field: created
+          plugin_id: date
+          relationship: none
+          group_type: group
+          admin_label: ''
+          exposed: false
+          expose:
+            label: ''
+          granularity: second
+      title: Page
+      header: {  }
+      footer: {  }
+      empty: {  }
+      relationships: {  }
+      arguments: {  }
+      display_extenders: {  }
+  attachment_1:
+    display_plugin: attachment
+    id: attachment_1
+    display_title: 'Attachment Before'
+    position: 2
+    display_options:
+      display_extenders: {  }
+      pager:
+        type: some
+      style:
+        type: rss
+      row:
+        type: node_rss
+      displays:
+        page_1: page_1
+        default: '0'
+      attachment_position: before
+      enabled: true
+      title: Attachment Before
+      defaults:
+        title: false
+      display_description: ''
+  attachment_2:
+    display_plugin: attachment
+    id: attachment_2
+    display_title: 'Attachment After'
+    position: 2
+    display_options:
+      display_extenders: {  }
+      pager:
+        type: some
+      style:
+        type: rss
+      row:
+        type: node_rss
+      displays:
+        page_1: page_1
+        default: '0'
+      attachment_position: after
+      enabled: true
+      title: Attachment After
+      defaults:
+        title: false
+      display_description: ''
+  feed_1:
+    display_plugin: feed
+    id: feed_1
+    display_title: Feed
+    position: 5
+    display_options:
+      display_extenders: {  }
+      pager:
+        type: some
+      style:
+        type: rss
+      row:
+        type: node_rss
+      path: test-attached-disabled.xml
+      displays:
+        page_1: page_1
+        default: '0'
+      enabled: true
+      title: Feed
+      defaults:
+        title: false
+      sitename_title: false
+  page_1:
+    display_plugin: page
+    id: page_1
+    display_title: Page
+    position: 1
+    display_options:
+      display_extenders: {  }
+      path: test-attached-disabled

core/modules/views/tests/src/Unit/ViewExecutableTest.php

@@ -406,6 +406,10 @@ public function testAttachDisplays() {
       ->disableOriginalConstructor()
       ->getMock();
 
+    $page_display->expects($this->atLeastOnce())
+      ->method('isEnabled')
+      ->willReturn(TRUE);
+
     $display_collection = $this->getMockBuilder('Drupal\views\DisplayPluginCollection')
       ->disableOriginalConstructor()
       ->getMock();