From 5ca8576b3c683863a7b73ce2235ca5a837836234 Mon Sep 17 00:00:00 2001 From: webchick Date: Fri, 12 Apr 2013 00:18:24 -0700 Subject: [PATCH] Issue #1963798 by tim.plunkett, damiankloip: Fixed Page displays cannot be deleted unless they have a valid path. --- .../Drupal/views/Plugin/Core/Entity/View.php | 8 +----- .../Plugin/views/wizard/WizardPluginBase.php | 5 ++-- .../lib/Drupal/views/Tests/UI/DisplayPath.php | 12 ++++++++- .../Drupal/views/Tests/ViewExecutableTest.php | 9 +++---- .../views/lib/Drupal/views/ViewExecutable.php | 27 +++++++++---------- .../Drupal/views/ViewExecutableFactory.php | 6 ++--- .../lib/Drupal/views/ViewStorageInterface.php | 12 +++++++++ .../Drupal/views_ui/ViewAddFormController.php | 6 +++-- .../views_ui/ViewEditFormController.php | 6 ++--- .../views_ui/ViewFormControllerBase.php | 4 +-- .../views_ui/lib/Drupal/views_ui/ViewUI.php | 25 +++++++++-------- 11 files changed, 70 insertions(+), 50 deletions(-) diff --git a/core/modules/views/lib/Drupal/views/Plugin/Core/Entity/View.php b/core/modules/views/lib/Drupal/views/Plugin/Core/Entity/View.php index 3c7e525ff43..924cfa50c98 100644 --- a/core/modules/views/lib/Drupal/views/Plugin/Core/Entity/View.php +++ b/core/modules/views/lib/Drupal/views/Plugin/Core/Entity/View.php @@ -288,13 +288,7 @@ class View extends ConfigEntityBase implements ViewStorageInterface { } /** - * 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]; diff --git a/core/modules/views/lib/Drupal/views/Plugin/views/wizard/WizardPluginBase.php b/core/modules/views/lib/Drupal/views/Plugin/views/wizard/WizardPluginBase.php index a8099a40636..9b88bb83ce4 100644 --- a/core/modules/views/lib/Drupal/views/Plugin/views/wizard/WizardPluginBase.php +++ b/core/modules/views/lib/Drupal/views/Plugin/views/wizard/WizardPluginBase.php @@ -1143,10 +1143,11 @@ abstract class WizardPluginBase extends PluginBase implements WizardInterface { 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; } diff --git a/core/modules/views/lib/Drupal/views/Tests/UI/DisplayPath.php b/core/modules/views/lib/Drupal/views/Tests/UI/DisplayPath.php index af605777074..ca9d3d71fdc 100644 --- a/core/modules/views/lib/Drupal/views/Tests/UI/DisplayPath.php +++ b/core/modules/views/lib/Drupal/views/Tests/UI/DisplayPath.php @@ -44,5 +44,15 @@ class DisplayPath extends UITestBase { $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'))); + } +} diff --git a/core/modules/views/lib/Drupal/views/Tests/ViewExecutableTest.php b/core/modules/views/lib/Drupal/views/Tests/ViewExecutableTest.php index 9fbabe25080..41e5c8bc20d 100644 --- a/core/modules/views/lib/Drupal/views/Tests/ViewExecutableTest.php +++ b/core/modules/views/lib/Drupal/views/Tests/ViewExecutableTest.php @@ -378,19 +378,18 @@ class ViewExecutableTest extends ViewUnitTestBase { $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.'); } } diff --git a/core/modules/views/lib/Drupal/views/ViewExecutable.php b/core/modules/views/lib/Drupal/views/ViewExecutable.php index a7130bd7ed3..d9b67ad369b 100644 --- a/core/modules/views/lib/Drupal/views/ViewExecutable.php +++ b/core/modules/views/lib/Drupal/views/ViewExecutable.php @@ -9,7 +9,7 @@ namespace Drupal\views; 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 @@ class ViewExecutable { } /** - * 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; } /** diff --git a/core/modules/views/lib/Drupal/views/ViewExecutableFactory.php b/core/modules/views/lib/Drupal/views/ViewExecutableFactory.php index 59a052afb8c..5b33f57b97c 100644 --- a/core/modules/views/lib/Drupal/views/ViewExecutableFactory.php +++ b/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); } diff --git a/core/modules/views/lib/Drupal/views/ViewStorageInterface.php b/core/modules/views/lib/Drupal/views/ViewStorageInterface.php index 9bd3ba5a26f..b09acb263fa 100644 --- a/core/modules/views/lib/Drupal/views/ViewStorageInterface.php +++ b/core/modules/views/lib/Drupal/views/ViewStorageInterface.php @@ -13,4 +13,16 @@ use Drupal\Core\Config\Entity\ConfigEntityInterface; * 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); + } diff --git a/core/modules/views/views_ui/lib/Drupal/views_ui/ViewAddFormController.php b/core/modules/views/views_ui/lib/Drupal/views_ui/ViewAddFormController.php index 474c1e1427a..38a1099b139 100644 --- a/core/modules/views/views_ui/lib/Drupal/views_ui/ViewAddFormController.php +++ b/core/modules/views/views_ui/lib/Drupal/views_ui/ViewAddFormController.php @@ -141,8 +141,10 @@ class ViewAddFormController extends ViewFormControllerBase { $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); + } } } diff --git a/core/modules/views/views_ui/lib/Drupal/views_ui/ViewEditFormController.php b/core/modules/views/views_ui/lib/Drupal/views_ui/ViewEditFormController.php index f7cc5e49b05..0d06d9e718e 100644 --- a/core/modules/views/views_ui/lib/Drupal/views_ui/ViewEditFormController.php +++ b/core/modules/views/views_ui/lib/Drupal/views_ui/ViewEditFormController.php @@ -203,9 +203,9 @@ class ViewEditFormController extends ViewFormControllerBase { 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); } } diff --git a/core/modules/views/views_ui/lib/Drupal/views_ui/ViewFormControllerBase.php b/core/modules/views/views_ui/lib/Drupal/views_ui/ViewFormControllerBase.php index 3269124085b..b791226633c 100644 --- a/core/modules/views/views_ui/lib/Drupal/views_ui/ViewFormControllerBase.php +++ b/core/modules/views/views_ui/lib/Drupal/views_ui/ViewFormControllerBase.php @@ -129,9 +129,9 @@ abstract class ViewFormControllerBase extends EntityFormController { } // 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. diff --git a/core/modules/views/views_ui/lib/Drupal/views_ui/ViewUI.php b/core/modules/views/views_ui/lib/Drupal/views_ui/ViewUI.php index 2762d4f7c36..67e0fc656ac 100644 --- a/core/modules/views/views_ui/lib/Drupal/views_ui/ViewUI.php +++ b/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 @@ class ViewUI implements ViewStorageInterface { $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 @@ class ViewUI implements ViewStorageInterface { } } 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 @@ class ViewUI implements ViewStorageInterface { 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(). */