From 64f7065248cd9947ccd863dfc38a83dec2c96144 Mon Sep 17 00:00:00 2001 From: catch <6915-catch@users.noreply.drupalcode.org> Date: Wed, 10 Jul 2024 13:57:20 +0100 Subject: [PATCH] =?UTF-8?q?Issue=20#2408549=20by=20alexpott,=20narendraR,?= =?UTF-8?q?=20yash.rode,=20kunal.sachdev,=20lauriii,=20Liam=20Morland,=20W?= =?UTF-8?q?im=20Leers,=20Hardik=5FPatel=5F12,=20jofitz,=20DamienMcKenna,?= =?UTF-8?q?=20eiriksm,=20andypost,=20jenlampton,=20G=C3=A1bor=20Hojtsy,=20?= =?UTF-8?q?swentel,=20borisson=5F,=20jhedstrom,=20snehi,=20Elijah=20Lynn,?= =?UTF-8?q?=20narendra.rajwar27,=20Shubham=20Chandra,=20smustgrave,=20sime?= =?UTF-8?q?,=20AaronMcHale,=20Chi,=20karolus,=20rkoller,=20joshua.boltz,?= =?UTF-8?q?=20anavarre,=20colan,=20frob,=20Berdir,=20bircher,=20minnur,=20?= =?UTF-8?q?effulgentsia,=20quietone,=20catch,=20xjm,=20hanoii,=20benjifish?= =?UTF-8?q?er,=20worldlinemine,=20larowlan,=20longwave,=20simohell,=20shaa?= =?UTF-8?q?l,=20worldlinemine:=20Display=20status=20message=20on=20configu?= =?UTF-8?q?ration=20forms=20when=20there=20are=20overridden=20values?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (cherry picked from commit 4689516656f8b0f58569923fc786a2c780929df4) --- core/lib/Drupal/Core/Form/ConfigFormBase.php | 58 +++++++++++++++- .../config_override_message_test.info.yml | 4 ++ .../config_override_message_test.module | 17 +++++ .../src/Functional/ConfigFormOverrideTest.php | 66 +++++++++++++++---- .../system/src/Form/SiteInformationForm.php | 24 +++---- .../router_test.routing.yml | 2 +- .../router_test_directory/src/Form.php | 42 ++++++++++++ .../src/Kernel/WorkspaceIntegrationTest.php | 5 +- .../Core/Routing/ExceptionHandlingTest.php | 2 +- 9 files changed, 189 insertions(+), 31 deletions(-) create mode 100644 core/modules/config/tests/config_override_message_test/config_override_message_test.info.yml create mode 100644 core/modules/config/tests/config_override_message_test/config_override_message_test.module create mode 100644 core/modules/system/tests/modules/router_test_directory/src/Form.php diff --git a/core/lib/Drupal/Core/Form/ConfigFormBase.php b/core/lib/Drupal/Core/Form/ConfigFormBase.php index 2931e4a3e24d..6436eaebe694 100644 --- a/core/lib/Drupal/Core/Form/ConfigFormBase.php +++ b/core/lib/Drupal/Core/Form/ConfigFormBase.php @@ -2,11 +2,13 @@ namespace Drupal\Core\Form; +use Drupal\Component\Utility\NestedArray; use Drupal\Core\Config\Config; use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Config\TypedConfigManagerInterface; use Drupal\Core\Render\Element; use Drupal\Core\StringTranslation\TranslatableMarkup; +use Drupal\Core\Url; use Symfony\Component\DependencyInjection\ContainerInterface; /** @@ -86,7 +88,7 @@ public function buildForm(array $form, FormStateInterface $form_state) { // property. $form['#process'][] = '::loadDefaultValuesFromConfig'; $form['#after_build'][] = '::storeConfigKeyToFormElementMap'; - + $form['#after_build'][] = '::checkConfigOverrides'; return $form; } @@ -333,4 +335,58 @@ private static function copyFormValuesToConfig(Config $config, FormStateInterfac } } + /** + * Form #after_build callback: Adds message if overrides exist. + */ + public function checkConfigOverrides(array $form, FormStateInterface $form_state): array { + // Determine which of those editable config keys have overrides. + $override_links = []; + $map = $form_state->get(static::CONFIG_KEY_TO_FORM_ELEMENT_MAP) ?? []; + foreach ($map as $config_name => $config_keys) { + $stored_config = $this->configFactory->get($config_name); + if (!$stored_config->hasOverrides()) { + // The config has no overrides at all. Can be skipped. + continue; + } + + foreach ($config_keys as $key => $array_parents) { + if ($stored_config->hasOverrides($key)) { + $element = NestedArray::getValue($form, $array_parents); + $override_links[] = [ + 'attributes' => ['title' => $this->t("'@title' form element", ['@title' => $element['#title']])], + 'url' => Url::fromUri("internal:#{$element['#id']}"), + 'title' => $element['#title'], + ]; + } + } + } + + if (!empty($override_links)) { + $override_output = [ + '#theme' => 'links__config_overrides', + '#heading' => [ + 'text' => $this->t('These values are overridden. Changes on this form will be saved, but overrides will take precedence. See <a href="https://www.drupal.org/docs/drupal-apis/configuration-api/configuration-override-system">configuration overrides documentation</a> for more information.'), + 'level' => 'div', + ], + '#links' => $override_links, + ]; + $form['config_override_status_messages'] = [ + 'message' => [ + '#theme' => 'status_messages', + '#message_list' => ['status' => [$override_output]], + '#status_headings' => [ + 'status' => $this->t('Status message'), + ], + ], + // Ensure that the status message is at the top of the form. + '#weight' => array_reduce( + Element::children($form), + fn (int $carry, string $key) => min(($form[$key]['#weight'] ?? 0), $carry), + 0 + ) - 1, + ]; + } + return $form; + } + } diff --git a/core/modules/config/tests/config_override_message_test/config_override_message_test.info.yml b/core/modules/config/tests/config_override_message_test/config_override_message_test.info.yml new file mode 100644 index 000000000000..2b4a23bfc46f --- /dev/null +++ b/core/modules/config/tests/config_override_message_test/config_override_message_test.info.yml @@ -0,0 +1,4 @@ +name: 'Configuration override message test' +type: module +package: Testing +version: VERSION diff --git a/core/modules/config/tests/config_override_message_test/config_override_message_test.module b/core/modules/config/tests/config_override_message_test/config_override_message_test.module new file mode 100644 index 000000000000..4106499fb293 --- /dev/null +++ b/core/modules/config/tests/config_override_message_test/config_override_message_test.module @@ -0,0 +1,17 @@ +<?php + +/** + * @file + * Tests configuration override message functionality. + */ + +use Drupal\Core\Form\FormStateInterface; + +/** + * Implements hook_form_FORM_ID_alter(). + */ +function config_override_message_test_form_system_site_information_settings_alter(array &$form, FormStateInterface $form_state, string $form_id): void { + // Set a weight to a negative amount to ensure the config overrides message + // is above it. + $form['site_information']['#weight'] = -5; +} diff --git a/core/modules/config/tests/src/Functional/ConfigFormOverrideTest.php b/core/modules/config/tests/src/Functional/ConfigFormOverrideTest.php index 5459f18d8b89..5b457b99a82d 100644 --- a/core/modules/config/tests/src/Functional/ConfigFormOverrideTest.php +++ b/core/modules/config/tests/src/Functional/ConfigFormOverrideTest.php @@ -14,6 +14,18 @@ */ class ConfigFormOverrideTest extends BrowserTestBase { + /** + * Message text that appears when forms have values for overridden config. + */ + private const OVERRIDE_TEXT = 'These values are overridden. Changes on this form will be saved, but overrides will take precedence. See configuration overrides documentation for more information.'; + + /** + * Modules to enable. + * + * @var array + */ + protected static $modules = ['update', 'config_override_message_test']; + /** * {@inheritdoc} */ @@ -26,31 +38,61 @@ public function testFormsWithOverrides(): void { $this->drupalLogin($this->drupalCreateUser([ 'access administration pages', 'administer site configuration', + 'link to any page', ])); - $overridden_name = 'Site name global conf override'; + // Set up an overrides for configuration that is present in the form. + $settings['config']['system.site']['weight_select_max'] = (object) [ + 'value' => 200, + 'required' => TRUE, + ]; + $this->writeSettings($settings); - // Set up an override. + // Test that although system.site has an overridden key no override + // information is displayed because there is no corresponding form field. + $this->drupalGet('admin/config/system/site-information'); + $this->assertSession()->fieldValueEquals("site_name", 'Drupal'); + $this->assertSession()->pageTextNotContains(self::OVERRIDE_TEXT); + + // Set up an overrides for configuration that is present in the form. + $overridden_name = 'Site name global conf override'; $settings['config']['system.site']['name'] = (object) [ 'value' => $overridden_name, 'required' => TRUE, ]; + $settings['config']['update.settings']['notification']['emails'] = (object) [ + 'value' => [ + 0 => 'a@abc.com', + 1 => 'admin@example.com', + ], + 'required' => TRUE, + ]; $this->writeSettings($settings); - - // Test that everything on the form is the same, but that the override - // worked for the actual site name. $this->drupalGet('admin/config/system/site-information'); $this->assertSession()->titleEquals('Basic site settings | ' . $overridden_name); + $this->assertSession()->elementTextContains('css', 'div[data-drupal-messages]', self::OVERRIDE_TEXT); + // Ensure the configuration overrides message is at the top of the form. + $this->assertSession()->elementExists('css', 'div[data-drupal-messages] + details#edit-site-information'); + $this->assertSession()->elementContains('css', 'div[data-drupal-messages]', '<a href="#edit-site-name" title="\'Site name\' form element">Site name</a>'); $this->assertSession()->fieldValueEquals("site_name", 'Drupal'); - - // Submit the form and ensure the site name is not changed. - $edit = [ + $this->submitForm([ 'site_name' => 'Custom site name', - ]; - $this->drupalGet('admin/config/system/site-information'); - $this->submitForm($edit, 'Save configuration'); + ], 'Save configuration'); $this->assertSession()->titleEquals('Basic site settings | ' . $overridden_name); - $this->assertSession()->fieldValueEquals("site_name", $edit['site_name']); + $this->assertSession()->fieldValueEquals("site_name", 'Custom site name'); + + // Ensure it works for sequence. + $this->drupalGet('admin/reports/updates/settings'); + $this->submitForm([], 'Save configuration'); + $this->assertSession()->pageTextContainsOnce(self::OVERRIDE_TEXT); + // There are two status messages on the page due to the save. + $messages = $this->getSession()->getPage()->findAll('css', 'div[data-drupal-messages]'); + $this->assertCount(2, $messages); + $this->assertStringContainsString('The configuration options have been saved.', $messages[0]->getText()); + $this->assertTrue( + $messages[1]->hasLink('Email addresses to notify when updates are available'), + "Link to 'Email addresses to notify when updates are available' exists" + ); } } diff --git a/core/modules/system/src/Form/SiteInformationForm.php b/core/modules/system/src/Form/SiteInformationForm.php index 372e4e8d2462..dbdcdc12d502 100644 --- a/core/modules/system/src/Form/SiteInformationForm.php +++ b/core/modules/system/src/Form/SiteInformationForm.php @@ -5,6 +5,7 @@ use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Config\TypedConfigManagerInterface; use Drupal\Core\Form\ConfigFormBase; +use Drupal\Core\Form\ConfigTarget; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Path\PathValidatorInterface; use Drupal\Core\Routing\RequestContext; @@ -92,10 +93,6 @@ protected function getEditableConfigNames() { */ public function buildForm(array $form, FormStateInterface $form_state) { $site_config = $this->config('system.site'); - $site_mail = $site_config->get('mail'); - if (empty($site_mail)) { - $site_mail = ini_get('sendmail_from'); - } $form['site_information'] = [ '#type' => 'details', @@ -105,20 +102,24 @@ public function buildForm(array $form, FormStateInterface $form_state) { $form['site_information']['site_name'] = [ '#type' => 'textfield', '#title' => $this->t('Site name'), - '#default_value' => $site_config->get('name'), + '#config_target' => 'system.site:name', '#required' => TRUE, ]; $form['site_information']['site_slogan'] = [ '#type' => 'textfield', '#title' => $this->t('Slogan'), - '#default_value' => $site_config->get('slogan'), + '#config_target' => 'system.site:slogan', '#description' => $this->t("How this is used depends on your site's theme."), '#maxlength' => 255, ]; $form['site_information']['site_mail'] = [ '#type' => 'email', '#title' => $this->t('Email address'), - '#default_value' => $site_mail, + '#config_target' => new ConfigTarget( + 'system.site', + 'mail', + fromConfig: fn($value) => $value ?: ini_get('sendmail_from'), + ), '#description' => $this->t("The <em>From</em> address in automated emails sent during registration and new password requests, and other notifications. (Use an address ending in your site's domain to help prevent this email being flagged as spam.)"), '#required' => TRUE, ]; @@ -144,14 +145,14 @@ public function buildForm(array $form, FormStateInterface $form_state) { $form['error_page']['site_403'] = [ '#type' => 'textfield', '#title' => $this->t('Default 403 (access denied) page'), - '#default_value' => $site_config->get('page.403'), + '#config_target' => 'system.site:page.403', '#size' => 40, '#description' => $this->t('This page is displayed when the requested document is denied to the current user. Leave blank to display a generic "access denied" page.'), ]; $form['error_page']['site_404'] = [ '#type' => 'textfield', '#title' => $this->t('Default 404 (not found) page'), - '#default_value' => $site_config->get('page.404'), + '#config_target' => 'system.site:page.404', '#size' => 40, '#description' => $this->t('This page is displayed when no other content matches the requested document. Leave blank to display a generic "page not found" page.'), ]; @@ -203,12 +204,7 @@ public function validateForm(array &$form, FormStateInterface $form_state) { */ public function submitForm(array &$form, FormStateInterface $form_state) { $this->config('system.site') - ->set('name', $form_state->getValue('site_name')) - ->set('mail', $form_state->getValue('site_mail')) - ->set('slogan', $form_state->getValue('site_slogan')) ->set('page.front', $form_state->getValue('site_frontpage')) - ->set('page.403', $form_state->getValue('site_403')) - ->set('page.404', $form_state->getValue('site_404')) ->save(); parent::submitForm($form, $form_state); diff --git a/core/modules/system/tests/modules/router_test_directory/router_test.routing.yml b/core/modules/system/tests/modules/router_test_directory/router_test.routing.yml index 404458e422ec..3267711e7be6 100644 --- a/core/modules/system/tests/modules/router_test_directory/router_test.routing.yml +++ b/core/modules/system/tests/modules/router_test_directory/router_test.routing.yml @@ -165,7 +165,7 @@ router_test.25: router_test.26: path: '/router_test/test26' defaults: - _form: '\Drupal\system\Form\LoggingForm' + _form: '\Drupal\router_test\Form' _title: 'Cron' requirements: _access: 'TRUE' diff --git a/core/modules/system/tests/modules/router_test_directory/src/Form.php b/core/modules/system/tests/modules/router_test_directory/src/Form.php new file mode 100644 index 000000000000..33dd872be178 --- /dev/null +++ b/core/modules/system/tests/modules/router_test_directory/src/Form.php @@ -0,0 +1,42 @@ +<?php + +declare(strict_types=1); + +namespace Drupal\router_test; + +use Drupal\Core\Form\FormBase; +use Drupal\Core\Form\FormStateInterface; + +/** + * Form to test _form routing. + * + * @internal + */ +class Form extends FormBase { + + /** + * {@inheritdoc} + */ + public function getFormId() { + return 'router_test_form'; + } + + /** + * {@inheritdoc} + */ + public function buildForm(array $form, FormStateInterface $form_state): array { + $form['submit'] = [ + '#type' => 'submit', + '#value' => 'Save', + ]; + return $form; + } + + /** + * {@inheritdoc} + */ + public function submitForm(array &$form, FormStateInterface $form_state): void { + $this->messenger()->addStatus('The router_test_form form has been submitted successfully.'); + } + +} diff --git a/core/modules/workspaces/tests/src/Kernel/WorkspaceIntegrationTest.php b/core/modules/workspaces/tests/src/Kernel/WorkspaceIntegrationTest.php index 718af2449d38..71deeb74f821 100644 --- a/core/modules/workspaces/tests/src/Kernel/WorkspaceIntegrationTest.php +++ b/core/modules/workspaces/tests/src/Kernel/WorkspaceIntegrationTest.php @@ -8,9 +8,9 @@ use Drupal\Core\Form\FormState; use Drupal\Core\Session\AnonymousUserSession; use Drupal\entity_test\Entity\EntityTestMulRevPub; +use Drupal\form_test\Form\FormTestAlterForm; use Drupal\KernelTests\KernelTestBase; use Drupal\language\Entity\ConfigurableLanguage; -use Drupal\system\Form\SiteInformationForm; use Drupal\Tests\field\Traits\EntityReferenceFieldCreationTrait; use Drupal\Tests\node\Traits\ContentTypeCreationTrait; use Drupal\Tests\node\Traits\NodeCreationTrait; @@ -71,6 +71,7 @@ class WorkspaceIntegrationTest extends KernelTestBase { 'language', 'content_translation', 'path_alias', + 'form_test', ]; /** @@ -1073,7 +1074,7 @@ public function testFormCacheForRegularForms(): void { $form_builder = $this->container->get('form_builder'); $form_state = new FormState(); - $built_form = $form_builder->getForm(SiteInformationForm::class, $form_state); + $built_form = $form_builder->getForm(FormTestAlterForm::class, $form_state); $form_builder->setCache($built_form['#build_id'], $built_form, $form_state); } diff --git a/core/tests/Drupal/KernelTests/Core/Routing/ExceptionHandlingTest.php b/core/tests/Drupal/KernelTests/Core/Routing/ExceptionHandlingTest.php index f9eed1d6eaf3..1c2944a18ed7 100644 --- a/core/tests/Drupal/KernelTests/Core/Routing/ExceptionHandlingTest.php +++ b/core/tests/Drupal/KernelTests/Core/Routing/ExceptionHandlingTest.php @@ -132,7 +132,7 @@ public function testExceptionResponseGeneratedForOriginalRequest(): void { // Test with 404 path pointing to a route that uses '_form'. $response = $this->doTest404Route('/router_test/test26'); - $this->assertStringContainsString('<form class="system-logging-settings"', $response->getContent()); + $this->assertStringContainsString('<form class="router-test-form"', $response->getContent()); // Test with 404 path pointing to a route that uses '_entity_form'. $response = $this->doTest404Route('/router_test/test27'); -- GitLab