Issue #2919373 by tedbow, Wim Leers, Adita, webchick, Berdir, drpal, xjm,...

Issue #2919373 by tedbow, Wim Leers, Adita, webchick, Berdir, drpal, xjm, samuel.mortenson, bircher: Prevent Settings Tray functionality for blocks that have configuration overrides

core/modules/settings_tray/settings_tray.module

@@ -11,6 +11,8 @@
 use Drupal\settings_tray\Block\BlockEntityOffCanvasForm;
 use Drupal\settings_tray\Form\SystemBrandingOffCanvasForm;
 use Drupal\settings_tray\Form\SystemMenuOffCanvasForm;
+use Drupal\block\entity\Block;
+use Drupal\block\BlockInterface;
 
 /**
  * Implements hook_help().
@@ -54,10 +56,34 @@ function settings_tray_contextual_links_view_alter(&$element, $items) {
   }
 }
 
+/**
+ * Checks if a block has overrides.
+ *
+ * @param \Drupal\block\BlockInterface $block
+ *   The block to check for overrides.
+ *
+ * @return bool
+ *   TRUE if the block has overrides otherwise FALSE.
+ *
+ * @internal
+ */
+function _settings_tray_has_block_overrides(BlockInterface $block) {
+  // @todo Replace the following with $block->hasOverrides() in https://www.drupal.org/project/drupal/issues/2910353
+  //   and remove this function.
+  return \Drupal::config($block->getEntityType()->getConfigPrefix() . '.' . $block->id())->hasOverrides();
+}
+
 /**
  * Implements hook_block_view_alter().
  */
 function settings_tray_block_view_alter(array &$build) {
+  if (isset($build['#contextual_links']['block'])) {
+    // Ensure that contextual links vary by whether the block has config overrides
+    // or not.
+    // @see _contextual_links_to_id()
+    $build['#contextual_links']['block']['metadata']['has_overrides'] = _settings_tray_has_block_overrides($build['#block']) ? 1 : 0;
+  }
+
   // Force a new 'data-contextual-id' attribute on blocks when this module is
   // enabled so as not to reuse stale data cached client-side.
   // @todo Remove when https://www.drupal.org/node/2773591 is fixed.
@@ -80,12 +106,12 @@ function settings_tray_entity_type_build(array &$entity_types) {
  * Implements hook_preprocess_HOOK() for block templates.
  */
 function settings_tray_preprocess_block(&$variables) {
-  // Only blocks that have an settings_tray form will have a "Quick Edit" link.
-  // We could wait for the contextual links to be initialized on the client
-  // side,  and then add the class and data- attribute below there (via
-  // JavaScript). But that would mean that it would be impossible to show
-  // Settings Tray's clickable regions immediately when the page loads. When
-  // latency is high, this will cause flicker.
+  // Only blocks that have a settings_tray form and have no configuration
+  // overrides will have a "Quick Edit" link. We could wait for the contextual
+  // links to be initialized on the client side,  and then add the class and
+  // data- attribute below there (via JavaScript). But that would mean that it
+  // would be impossible to show Settings Tray's clickable regions immediately
+  // when the page loads. When latency is high, this will cause flicker.
   // @see \Drupal\settings_tray\Access\BlockPluginHasSettingsTrayFormAccessCheck
   /** @var \Drupal\settings_tray\Access\BlockPluginHasSettingsTrayFormAccessCheck $access_checker */
   $access_checker = \Drupal::service('access_check.settings_tray.block.settings_tray_form');
@@ -93,10 +119,13 @@ function settings_tray_preprocess_block(&$variables) {
   $block_plugin_manager = \Drupal::service('plugin.manager.block');
   /** @var \Drupal\Core\Block\BlockPluginInterface $block_plugin */
   $block_plugin = $block_plugin_manager->createInstance($variables['plugin_id']);
-  if ($access_checker->accessBlockPlugin($block_plugin)->isAllowed()) {
-    // Add class and attributes to all blocks to allow Javascript to target.
-    $variables['attributes']['class'][] = 'settings-tray-editable';
-    $variables['attributes']['data-drupal-settingstray'] = 'editable';
+  if (isset($variables['elements']['#contextual_links']['block']['route_parameters']['block'])) {
+    $block = Block::load($variables['elements']['#contextual_links']['block']['route_parameters']['block']);
+    if ($access_checker->accessBlockPlugin($block_plugin)->isAllowed() && !_settings_tray_has_block_overrides($block)) {
+      // Add class and attributes to all blocks to allow Javascript to target.
+      $variables['attributes']['class'][] = 'settings-tray-editable';
+      $variables['attributes']['data-drupal-settingstray'] = 'editable';
+    }
   }
 }
 

core/modules/settings_tray/settings_tray.routing.yml

@@ -6,3 +6,4 @@ entity.block.off_canvas_form:
   requirements:
     _permission: 'administer blocks'
     _access_block_plugin_has_settings_tray_form: 'TRUE'
+    _access_block_has_overrides_settings_tray_form: 'TRUE'

core/modules/settings_tray/settings_tray.services.yml

 services:
+  access_check.settings_tray.block.has_overrides:
+    class: Drupal\settings_tray\Access\BlockHasOverridesAccessCheck
+    tags:
+      - { name: access_check, applies_to: _access_block_has_overrides_settings_tray_form }
   access_check.settings_tray.block.settings_tray_form:
     class: Drupal\settings_tray\Access\BlockPluginHasSettingsTrayFormAccessCheck
     tags:

core/modules/settings_tray/src/Access/BlockHasOverridesAccessCheck.php

+<?php
+
+namespace Drupal\settings_tray\Access;
+
+use Drupal\block\BlockInterface;
+use Drupal\Core\Access\AccessResult;
+use Drupal\Core\Routing\Access\AccessInterface;
+
+/**
+ * Determines whether the requested block has a 'settings_tray' form.
+ *
+ * @internal
+ */
+class BlockHasOverridesAccessCheck implements AccessInterface {
+
+  /**
+   * Checks access for accessing a block's 'settings_tray' form.
+   *
+   * @param \Drupal\block\BlockInterface $block
+   *   The block whose 'settings_tray' form is being accessed.
+   *
+   * @return \Drupal\Core\Access\AccessResultInterface
+   *   The access result.
+   */
+  public function access(BlockInterface $block) {
+    return AccessResult::allowedIf(!_settings_tray_has_block_overrides($block));
+  }
+
+}

core/modules/settings_tray/src/Form/SystemBrandingOffCanvasForm.php

@@ -2,6 +2,7 @@
 
 namespace Drupal\settings_tray\Form;
 
+use Drupal\Core\Access\AccessResult;
 use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
 use Drupal\Core\Form\FormStateInterface;
@@ -63,10 +64,13 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta
     unset($form['block_branding']['use_site_name']['#description'], $form['block_branding']['use_site_slogan']['#description']);
 
     $site_config = $this->configFactory->getEditable('system.site');
+    // Load the immutable config to load the overrides.
+    $site_config_immutable = $this->configFactory->get('system.site');
     $form['site_information'] = [
       '#type' => 'details',
       '#title' => t('Site details'),
       '#open' => TRUE,
+      '#access' => AccessResult::allowedIf(!$site_config_immutable->hasOverrides('name') && !$site_config_immutable->hasOverrides('slogan')),
     ];
     $form['site_information']['site_name'] = [
       '#type' => 'textfield',
@@ -95,11 +99,15 @@ public function validateConfigurationForm(array &$form, FormStateInterface $form
    * {@inheritdoc}
    */
   public function submitConfigurationForm(array &$form, FormStateInterface $form_state) {
-    $site_info = $form_state->getValue('site_information');
-    $this->configFactory->getEditable('system.site')
-      ->set('name', $site_info['site_name'])
-      ->set('slogan', $site_info['site_slogan'])
-      ->save();
+    $site_config = $this->configFactory->get('system.site');
+    if (AccessResult::allowedIf(!$site_config->hasOverrides('name') && !$site_config->hasOverrides('slogan'))->isAllowed()) {
+      $site_info = $form_state->getValue('site_information');
+      $this->configFactory->getEditable('system.site')
+        ->set('name', $site_info['site_name'])
+        ->set('slogan', $site_info['site_slogan'])
+        ->save();
+    }
+
     $this->plugin->submitConfigurationForm($form, $form_state);
   }
 

core/modules/settings_tray/src/Form/SystemMenuOffCanvasForm.php

@@ -3,6 +3,8 @@
 namespace Drupal\settings_tray\Form;
 
 use Drupal\Component\Plugin\PluginInspectionInterface;
+use Drupal\Core\Access\AccessResult;
+use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
 use Drupal\Core\Entity\EntityStorageInterface;
 use Drupal\Core\Entity\EntityTypeManagerInterface;
@@ -51,17 +53,27 @@ class SystemMenuOffCanvasForm extends PluginFormBase implements ContainerInjecti
    */
   protected $entityTypeManager;
 
+  /**
+   * The config factory.
+   *
+   * @var \Drupal\Core\Config\ConfigFactoryInterface
+   */
+  protected $configFactory;
+
   /**
    * SystemMenuOffCanvasForm constructor.
    *
    * @param \Drupal\Core\Entity\EntityStorageInterface $menu_storage
    * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
    * @param \Drupal\Core\StringTranslation\TranslationInterface $string_translation
+   * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
+   *   The config factory.
    */
-  public function __construct(EntityStorageInterface $menu_storage, EntityTypeManagerInterface $entity_type_manager, TranslationInterface $string_translation) {
+  public function __construct(EntityStorageInterface $menu_storage, EntityTypeManagerInterface $entity_type_manager, TranslationInterface $string_translation, ConfigFactoryInterface $config_factory) {
     $this->menuStorage = $menu_storage;
     $this->entityTypeManager = $entity_type_manager;
     $this->stringTranslation = $string_translation;
+    $this->configFactory = $config_factory;
   }
 
   /**
@@ -71,7 +83,8 @@ public static function create(ContainerInterface $container) {
     return new static(
       $container->get('entity_type.manager')->getStorage('menu'),
       $container->get('entity_type.manager'),
-      $container->get('string_translation')
+      $container->get('string_translation'),
+      $container->get('config.factory')
     );
   }
 
@@ -87,6 +100,7 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta
       '#type' => 'details',
       '#title' => $this->t('Edit menu %label', ['%label' => $this->menu->label()]),
       '#open' => TRUE,
+      '#access' => AccessResult::allowedIf(!$this->hasMenuOverrides()),
     ];
     $form['entity_form'] += $this->getEntityForm($this->menu)->buildForm([], $form_state);
 
@@ -115,7 +129,9 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta
    */
   public function validateConfigurationForm(array &$form, FormStateInterface $form_state) {
     $this->plugin->validateConfigurationForm($form, $form_state);
-    $this->getEntityForm($this->menu)->validateForm($form, $form_state);
+    if (!$this->hasMenuOverrides()) {
+      $this->getEntityForm($this->menu)->validateForm($form, $form_state);
+    }
   }
 
   /**
@@ -123,8 +139,10 @@ public function validateConfigurationForm(array &$form, FormStateInterface $form
    */
   public function submitConfigurationForm(array &$form, FormStateInterface $form_state) {
     $this->plugin->submitConfigurationForm($form, $form_state);
-    $this->getEntityForm($this->menu)->submitForm($form, $form_state);
-    $this->menu->save();
+    if (!$this->hasMenuOverrides()) {
+      $this->getEntityForm($this->menu)->submitForm($form, $form_state);
+      $this->menu->save();
+    }
   }
 
   /**
@@ -147,7 +165,20 @@ protected function getEntityForm(MenuInterface $menu) {
    */
   public function setPlugin(PluginInspectionInterface $plugin) {
     $this->plugin = $plugin;
-    $this->menu = $this->menuStorage->load($this->plugin->getDerivativeId());
+    $this->menu = $this->menuStorage->loadOverrideFree($this->plugin->getDerivativeId());
+  }
+
+  /**
+   * Determines if the menu has configuration overrides.
+   *
+   * @return bool
+   *   TRUE if the menu has configuration overrides, otherwise FALSE.
+   */
+  protected function hasMenuOverrides() {
+    // @todo Replace the following with $this->menu->hasOverrides() in https://www.drupal.org/project/drupal/issues/2910353
+    //   and remove this function.
+    return $this->configFactory->get($this->menu->getEntityType()
+      ->getConfigPrefix() . '.' . $this->menu->id())->hasOverrides();
   }
 
 }

core/modules/settings_tray/tests/modules/settings_tray_override_test/settings_tray_override_test.info.yml

+name: 'Configuration override test for Settings Tray'
+type: module
+package: Testing
+version: VERSION
+core: 8.x
+dependencies:
+  - settings_tray

core/modules/settings_tray/tests/modules/settings_tray_override_test/settings_tray_override_test.services.yml

+services:
+  settings_tray_override_test.overrider:
+    class: Drupal\settings_tray_override_test\ConfigOverrider
+    tags:
+      - { name: config.factory.override }

core/modules/settings_tray/tests/modules/settings_tray_override_test/src/ConfigOverrider.php

+<?php
+
+namespace Drupal\settings_tray_override_test;
+
+use Drupal\Core\Cache\CacheableMetadata;
+use Drupal\Core\Config\ConfigFactoryOverrideInterface;
+use Drupal\Core\Config\StorageInterface;
+
+/**
+ * Provides an overridden block for Settings Tray testing.
+ *
+ * @see \Drupal\Tests\settings_tray\FunctionalJavascript\SettingsTrayBlockFormTest::testOverriddenDisabled()
+ */
+class ConfigOverrider implements ConfigFactoryOverrideInterface {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function loadOverrides($names) {
+    $overrides = [];
+    if (in_array('block.block.overridden_block', $names)) {
+      if (\Drupal::state()->get('settings_tray_override_test.block')) {
+        $overrides = $overrides + ['block.block.overridden_block' => ['settings' => ['label' => 'Now this will be the label.']]];
+      }
+    }
+    if (in_array('system.site', $names)) {
+      if (\Drupal::state()->get('settings_tray_override_test.site_name')) {
+        $overrides = $overrides + ['system.site' => ['name' => 'Llama Fan Club']];
+      }
+    }
+    if (in_array('system.menu.main', $names)) {
+      if (\Drupal::state()->get('settings_tray_override_test.menu')) {
+        $overrides = $overrides + ['system.menu.main' => ['label' => 'Labely label']];
+      }
+    }
+    return $overrides;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getCacheSuffix() {
+    return 'ConfigOverrider';
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function createConfigObject($name, $collection = StorageInterface::DEFAULT_COLLECTION) {
+    return NULL;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getCacheableMetadata($name) {
+    return new CacheableMetadata();
+  }
+
+}

core/modules/settings_tray/tests/src/FunctionalJavascript/SettingsTrayBlockFormTest.php

@@ -5,6 +5,7 @@
 use Drupal\block\Entity\Block;
 use Drupal\block_content\Entity\BlockContent;
 use Drupal\block_content\Entity\BlockContentType;
+use Drupal\menu_link_content\Entity\MenuLinkContent;
 use Drupal\settings_tray_test\Plugin\Block\SettingsTrayFormAnnotationIsClassBlock;
 use Drupal\settings_tray_test\Plugin\Block\SettingsTrayFormAnnotationNoneBlock;
 use Drupal\Tests\contextual\FunctionalJavascript\ContextualLinkClickTrait;
@@ -43,6 +44,9 @@ class SettingsTrayBlockFormTest extends OffCanvasTestBase {
     // cause test failures.
     'settings_tray_test_css',
     'settings_tray_test',
+    'settings_tray_override_test',
+    'menu_ui',
+    'menu_link_content',
   ];
 
   /**
@@ -75,7 +79,7 @@ public function testBlocks($theme, $block_plugin, $new_page_text, $element_selec
     $page = $this->getSession()->getPage();
     $this->enableTheme($theme);
     $block = $this->placeBlock($block_plugin);
-    $block_selector = str_replace('_', '-', $this->getBlockSelector($block));
+    $block_selector = $this->getBlockSelector($block);
     $block_id = $block->id();
     $this->drupalGet('user');
 
@@ -321,7 +325,7 @@ public function testQuickEditLinks() {
       $this->enableTheme($theme);
 
       $block = $this->placeBlock($block_plugin);
-      $block_selector = str_replace('_', '-', $this->getBlockSelector($block));
+      $block_selector = $this->getBlockSelector($block);
       // Load the same page twice.
       foreach ([1, 2] as $page_load_times) {
         $this->drupalGet('node/' . $node->id());
@@ -531,7 +535,7 @@ public function testCustomBlockLinks() {
    *   The CSS selector.
    */
   public function getBlockSelector(Block $block) {
-    return '#block-' . $block->id();
+    return '#block-' . str_replace('_', '-', $block->id());
   }
 
   /**
@@ -577,4 +581,137 @@ protected function getTestThemes() {
     });
   }
 
+  /**
+   * Tests that blocks with configuration overrides are disabled.
+   */
+  public function testOverriddenBlock() {
+    $web_assert = $this->assertSession();
+    $page = $this->getSession()->getPage();
+    $overridden_block = $this->placeBlock('system_powered_by_block', [
+      'id' => 'overridden_block',
+      'label_display' => 1,
+      'label' => 'This will be overridden.',
+    ]);
+    $this->drupalGet('user');
+    $block_selector = $this->getBlockSelector($overridden_block);
+    // Confirm the block is marked as Settings Tray editable.
+    $this->assertEquals('editable', $page->find('css', $block_selector)->getAttribute('data-drupal-settingstray'));
+    // Confirm the label is not overridden.
+    $web_assert->elementContains('css', $block_selector, 'This will be overridden.');
+    $this->enableEditMode();
+    $this->openBlockForm($block_selector);
+
+    // Confirm the block Settings Tray functionality is disabled when block is
+    // overridden.
+    $this->container->get('state')->set('settings_tray_override_test.block', TRUE);
+    $overridden_block->save();
+    $block_config = \Drupal::configFactory()->getEditable('block.block.overridden_block');
+    $block_config->set('settings', $block_config->get('settings'))->save();
+
+    $this->drupalGet('user');
+    $this->assertOverriddenBlockDisabled($overridden_block, 'Now this will be the label.');
+
+    // Test a non-overridden block does show the form in the off-canvas dialog.
+    $block = $this->placeBlock('system_powered_by_block', [
+      'label_display' => 1,
+      'label' => 'Labely label',
+    ]);
+    $this->drupalGet('user');
+    $block_selector = $this->getBlockSelector($block);
+    // Confirm the block is marked as Settings Tray editable.
+    $this->assertEquals('editable', $page->find('css', $block_selector)->getAttribute('data-drupal-settingstray'));
+    // Confirm the label is not overridden.
+    $web_assert->elementContains('css', $block_selector, 'Labely label');
+    $this->openBlockForm($block_selector);
+  }
+
+  /**
+   * Test  blocks with overridden related configuration removed when overridden.
+   */
+  public function testOverriddenConfigurationRemoved() {
+    $web_assert = $this->assertSession();
+    $page = $this->getSession()->getPage();
+
+    // Confirm the branding block does include 'site_information' section when
+    // the site name is not overridden.
+    $branding_block = $this->placeBlock('system_branding_block');
+    $this->drupalGet('user');
+    $this->enableEditMode();
+    $this->openBlockForm($this->getBlockSelector($branding_block));
+    $web_assert->fieldExists('settings[site_information][site_name]');
+    // Confirm the branding block does not include 'site_information' section
+    // when the site name is overridden.
+    $this->container->get('state')->set('settings_tray_override_test.site_name', TRUE);
+    $this->drupalGet('user');
+    $this->openBlockForm($this->getBlockSelector($branding_block));
+    $web_assert->fieldNotExists('settings[site_information][site_name]');
+    $page->pressButton('Save Site branding');
+    $this->assertElementVisibleAfterWait('css', 'div:contains(The block configuration has been saved)');
+    $web_assert->assertWaitOnAjaxRequest();
+    // Confirm we did not save changes to the configuration.
+    $this->assertEquals('Llama Fan Club', \Drupal::configFactory()->get('system.site')->get('name'));
+    $this->assertEquals('Drupal', \Drupal::configFactory()->getEditable('system.site')->get('name'));
+
+    // Add a link or the menu will not render.
+    $menu_link_content = MenuLinkContent::create([
+      'title' => 'This is on the menu',
+      'menu_name' => 'main',
+      'link' => ['uri' => 'route:<front>'],
+    ]);
+    $menu_link_content->save();
+    // Confirm the menu block does include menu section when the menu is not
+    // overridden.
+    $menu_block = $this->placeBlock('system_menu_block:main');
+    $web_assert->assertWaitOnAjaxRequest();
+    $this->drupalGet('user');
+    $web_assert->pageTextContains('This is on the menu');
+    $this->openBlockForm($this->getBlockSelector($menu_block));
+    $web_assert->elementExists('css', '#menu-overview');
+
+    // Confirm the menu block does not include menu section when the menu is
+    // overridden.
+    $this->container->get('state')->set('settings_tray_override_test.menu', TRUE);
+    $this->drupalGet('user');
+    $web_assert->pageTextContains('This is on the menu');
+    $menu_with_overrides = \Drupal::configFactory()->get('system.menu.main')->get();
+    $menu_without_overrides = \Drupal::configFactory()->getEditable('system.menu.main')->get();
+    $this->openBlockForm($this->getBlockSelector($menu_block));
+    $web_assert->elementNotExists('css', '#menu-overview');
+    $page->pressButton('Save Main navigation');
+    $this->assertElementVisibleAfterWait('css', 'div:contains(The block configuration has been saved)');
+    $web_assert->assertWaitOnAjaxRequest();
+    // Confirm we did not save changes to the configuration.
+    $this->assertEquals('Labely label', \Drupal::configFactory()->get('system.menu.main')->get('label'));
+    $this->assertEquals('Main navigation', \Drupal::configFactory()->getEditable('system.menu.main')->get('label'));
+    $this->assertEquals($menu_with_overrides, \Drupal::configFactory()->get('system.menu.main')->get());
+    $this->assertEquals($menu_without_overrides, \Drupal::configFactory()->getEditable('system.menu.main')->get());
+    $web_assert->pageTextContains('This is on the menu');
+  }
+  /**
+   * Asserts that an overridden block has Settings Tray disabled.
+   *
+   * @param \Drupal\block\Entity\Block $overridden_block
+   *   The overridden block.
+   * @param string $override_text
+   *   The override text that should appear in the block.
+   */
+  protected function assertOverriddenBlockDisabled(Block $overridden_block, $override_text) {
+    $web_assert = $this->assertSession();
+    $page = $this->getSession()->getPage();
+    $block_selector = $this->getBlockSelector($overridden_block);
+    $block_id = $overridden_block->id();
+    // Confirm the block does not have a quick edit link.
+    $contextual_links = $page->findAll('css', "$block_selector .contextual-links li a");
+    $this->assertNotEmpty($contextual_links);
+    foreach ($contextual_links as $link) {
+      $this->assertNotContains("/admin/structure/block/manage/$block_id/off-canvas", $link->getAttribute('href'));
+    }
+    // Confirm the block is not marked as Settings Tray editable.
+    $this->assertFalse($page->find('css', $block_selector)
+      ->hasAttribute('data-drupal-settingstray'));
+
+    // Confirm the text is actually overridden.
+    $web_assert->elementContains('css', $this->getBlockSelector($overridden_block), $override_text);
+  }
+
 }