Issue #1963798 by tim.plunkett, damiankloip: Fixed Page displays cannot be...

Issue #1963798 by tim.plunkett, damiankloip: Fixed Page displays cannot be deleted unless they have a valid path.

core/modules/views/lib/Drupal/views/Plugin/Core/Entity/View.php

@@ -288,13 +288,7 @@ public function newDisplay($plugin_id = 'page', $title = NULL, $id = NULL) {
   }
 
   /**
-   * Retrieves a specific display's configuration by reference.
-   *
-   * @param string $display_id
-   *   The display ID to retrieve, e.g., 'default', 'page_1', 'block_2'.
-   *
-   * @return array
-   *   A reference to the specified display configuration.
+   * {@inheritdoc}
    */
   public function &getDisplay($display_id) {
     return $this->display[$display_id];

core/modules/views/lib/Drupal/views/Plugin/views/wizard/WizardPluginBase.php

@@ -1143,10 +1143,11 @@ protected function set_validated_view(array $form, array &$form_state, ViewUI $v
   public function validateView(array $form, array &$form_state) {
     $view = $this->instantiate_view($form, $form_state);
     $errors = $view->get('executable')->validate();
-    if (!is_array($errors) || empty($errors)) {
+
+    if (empty($errors)) {
       $this->set_validated_view($form, $form_state, $view);
-      return array();
     }
+
     return $errors;
   }
 

core/modules/views/lib/Drupal/views/Tests/UI/DisplayPath.php

@@ -44,5 +44,15 @@ public function testPathUI() {
     $this->assertLink(t('view @display', array('@display' => 'Page')), 0, 'view page link found on the page.');
   }
 
-}
+  /**
+   * Tests deleting a page display that has no path.
+   */
+  public function testDeleteWithNoPath() {
+    $this->drupalGet('admin/structure/views/view/test_view');
+    $this->drupalPost(NULL, array(), t('Add Page'));
+    $this->drupalPost(NULL, array(), t('delete Page'));
+    $this->drupalPost(NULL, array(), t('Save'));
+    $this->assertRaw(format_string('The view %view has been saved.', array('%view' => 'test_view')));
+  }
 
+}

core/modules/views/lib/Drupal/views/Tests/ViewExecutableTest.php

@@ -378,19 +378,18 @@ public function testValidate() {
       $match = function($value) use ($display) {
         return strpos($value, $display->display['display_title']) !== false;
       };
-      $this->assertTrue(array_filter($validate, $match), format_string('Error message found for @id display', array('@id' => $id)));
+      $this->assertTrue(array_filter($validate[$id], $match), format_string('Error message found for @id display', array('@id' => $id)));
       $count++;
     }
 
     $this->assertEqual(count($view->displayHandlers), $count, 'Error messages from all handlers merged.');
 
     // Test that a deleted display is not included.
-    $view->displayHandlers->get('default')->deleted = TRUE;
+    $display = &$view->storage->getDisplay('default');
+    $display['deleted'] = TRUE;
     $validate_deleted = $view->validate();
 
-    $this->assertNotEqual(count($validate), count($validate_deleted));
-    // The first item was the default validation error originally.
-    $this->assertNotIdentical($validate[0], $validate_deleted[0], 'Master display has not been validated.');
+    $this->assertNotIdentical($validate, $validate_deleted, 'Master display has not been validated.');
   }
 
 }

core/modules/views/lib/Drupal/views/ViewExecutable.php

@@ -9,7 +9,7 @@
 
 use Drupal;
 use Symfony\Component\HttpFoundation\Response;
-use Drupal\views\Plugin\Core\Entity\View;
+use Drupal\views\ViewStorageInterface;
 
 /**
  * @defgroup views_objects Objects that represent a View or part of a view
@@ -426,10 +426,10 @@ class ViewExecutable {
   /**
    * Constructs a new ViewExecutable object.
    *
-   * @param Drupal\views\Plugin\Core\Entity\View $storage
+   * @param \Drupal\views\ViewStorageInterface $storage
    *   The view config entity the actual information is stored on.
    */
-  public function __construct(View $storage) {
+  public function __construct(ViewStorageInterface $storage) {
     // Reference the storage and the executable to each other.
     $this->storage = $storage;
     $this->storage->set('executable', $this);
@@ -1732,35 +1732,34 @@ public function destroy() {
   }
 
   /**
-   * Make sure the view is completely valid.
+   * Makes sure the view is completely valid.
    *
-   * @return
-   *   TRUE if the view is valid; an array of error strings if it is not.
+   * @return array
+   *   An array of error strings. This will be empty if there are no validation
+   *   errors.
    */
   public function validate() {
-    $this->initDisplay();
-
     $errors = array();
-    $this->display_errors = NULL;
 
+    $this->initDisplay();
     $current_display = $this->current_display;
+
     foreach ($this->displayHandlers as $id => $display) {
       if (!empty($display)) {
-        if (!empty($display->deleted)) {
+        if (!empty($display->display['deleted'])) {
           continue;
         }
 
         $result = $this->displayHandlers->get($id)->validate();
         if (!empty($result) && is_array($result)) {
-          $errors = array_merge($errors, $result);
-          // Mark this display as having validation errors.
-          $this->display_errors[$id] = TRUE;
+          $errors[$id] =  $result;
         }
       }
     }
 
     $this->setDisplay($current_display);
-    return $errors ? $errors : TRUE;
+
+    return $errors;
   }
 
   /**

core/modules/views/lib/Drupal/views/ViewExecutableFactory.php

@@ -7,7 +7,7 @@
 
 namespace Drupal\views;
 
-use Drupal\views\Plugin\Core\Entity\View;
+use Drupal\views\ViewStorageInterface;
 
 /**
  * Defines the cache backend factory.
@@ -17,13 +17,13 @@ class ViewExecutableFactory {
   /**
    * Instantiates a ViewExecutable class.
    *
-   * @param \Drupal\views\Plugin\Core\Entity\View $view
+   * @param \Drupal\views\ViewStorageInterface $view
    *   A view entity instance.
    *
    * @return \Drupal\views\ViewExecutable
    *   A ViewExecutable instance.
    */
-  public static function get(View $view) {
+  public static function get(ViewStorageInterface $view) {
     return new ViewExecutable($view);
   }
 

core/modules/views/lib/Drupal/views/ViewStorageInterface.php

@@ -13,4 +13,16 @@
  * Defines an interface for View storage classes.
  */
 interface ViewStorageInterface extends \IteratorAggregate, ConfigEntityInterface {
+
+  /**
+   * Retrieves a specific display's configuration by reference.
+   *
+   * @param string $display_id
+   *   The display ID to retrieve, e.g., 'default', 'page_1', 'block_2'.
+   *
+   * @return array
+   *   A reference to the specified display configuration.
+   */
+  public function &getDisplay($display_id);
+
 }

core/modules/views/views_ui/lib/Drupal/views_ui/ViewAddFormController.php

@@ -141,8 +141,10 @@ public function validate(array $form, array &$form_state) {
     $form_state['wizard_instance'] = $wizard_instance;
     $errors = $form_state['wizard_instance']->validateView($form, $form_state);
 
-    foreach ($errors as $name => $message) {
-      form_set_error($name, $message);
+    foreach ($errors as $display_errors) {
+      foreach ($display_errors as $name => $message) {
+        form_set_error($name, $message);
+      }
     }
   }
 

core/modules/views/views_ui/lib/Drupal/views_ui/ViewEditFormController.php

@@ -203,9 +203,9 @@ public function validate(array $form, array &$form_state) {
     parent::validate($form, $form_state);
 
     $view = $this->getEntity($form_state);
-    $errors = $view->get('executable')->validate();
-    if ($errors !== TRUE) {
-      foreach ($errors as $error) {
+
+    foreach ($view->get('executable')->validate() as $display_errors) {
+      foreach ($display_errors as $error) {
         form_set_error('', $error);
       }
     }

core/modules/views/views_ui/lib/Drupal/views_ui/ViewFormControllerBase.php

@@ -129,9 +129,9 @@ public function getDisplayTabs(ViewUI $view) {
     }
 
     // Mark the display tab as red to show validation errors.
-    $view->get('executable')->validate();
+    $errors = $view->get('executable')->validate();
     foreach ($view->get('display') as $id => $display) {
-      if (!empty($view->display_errors[$id])) {
+      if (!empty($errors[$id])) {
         // Always show the tab.
         $tabs[$id]['#access'] = TRUE;
         // Add a class to mark the error and a title to make a hover tip.

core/modules/views/views_ui/lib/Drupal/views_ui/ViewUI.php

@@ -7,6 +7,7 @@
 
 namespace Drupal\views_ui;
 
+use Drupal\views\Views;
 use Drupal\views\ViewExecutable;
 use Drupal\Core\Database\Database;
 use Drupal\Core\TypedData\ContextAwareInterface;
@@ -27,13 +28,6 @@ class ViewUI implements ViewStorageInterface {
    */
   public $editing = FALSE;
 
-  /**
-   * Stores an array of errors for any displays.
-   *
-   * @var array
-   */
-  public $display_errors;
-
   /**
    * Stores an array of displays that have been changed.
    *
@@ -149,7 +143,7 @@ class ViewUI implements ViewStorageInterface {
   public function __construct(ViewStorageInterface $storage) {
     $this->entityType = 'view';
     $this->storage = $storage;
-    $this->executable = $storage->get('executable');
+    $this->executable = Views::executableFactory()->get($this);
   }
 
   /**
@@ -530,7 +524,7 @@ public function renderPreview($display_id, $args = array()) {
     $output = '';
 
     $errors = $this->executable->validate();
-    if ($errors === TRUE) {
+    if (empty($errors)) {
       $this->ajax = TRUE;
       $this->executable->live_preview = TRUE;
       $this->views_ui_context = TRUE;
@@ -668,8 +662,10 @@ public function renderPreview($display_id, $args = array()) {
       }
     }
     else {
-      foreach ($errors as $error) {
-        drupal_set_message($error, 'error');
+      foreach ($errors as $display_errors) {
+        foreach ($display_errors as $error) {
+          drupal_set_message($error, 'error');
+        }
       }
       $preview = t('Unable to preview due to validation errors.');
     }
@@ -742,6 +738,13 @@ public function __call($method, $args) {
     return call_user_func_array(array($this->storage, $method), $args);
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function &getDisplay($display_id) {
+    return $this->storage->getDisplay($display_id);
+  }
+
   /**
    * Implements \IteratorAggregate::getIterator().
    */