diff --git a/core/lib/Drupal/Core/Form/ConfigFormBase.php b/core/lib/Drupal/Core/Form/ConfigFormBase.php index a13ac2c36b8759371703206a8681c166f5ea662d..99dba09f305d9a252c2516caba8cedd152085ae8 100644 --- a/core/lib/Drupal/Core/Form/ConfigFormBase.php +++ b/core/lib/Drupal/Core/Form/ConfigFormBase.php @@ -139,8 +139,31 @@ public function loadDefaultValuesFromConfig(array $element): array { * * @return array * The processed element. + * + * @see \Drupal\Core\Form\ConfigFormBase::buildForm() */ public function storeConfigKeyToFormElementMap(array $element, FormStateInterface $form_state): array { + // Empty the map to ensure the information is always correct after + // rebuilding the form. + $form_state->set(static::CONFIG_KEY_TO_FORM_ELEMENT_MAP, []); + + return $this->doStoreConfigMap($element, $form_state); + } + + /** + * Helper method for #after_build callback ::storeConfigKeyToFormElementMap(). + * + * @param array $element + * The element being processed. + * @param \Drupal\Core\Form\FormStateInterface $form_state + * The current form state. + * + * @return array + * The processed element. + * + * @see \Drupal\Core\Form\ConfigFormBase::storeConfigKeyToFormElementMap() + */ + protected function doStoreConfigMap(array $element, FormStateInterface $form_state): array { if (array_key_exists('#config_target', $element)) { $map = $form_state->get(static::CONFIG_KEY_TO_FORM_ELEMENT_MAP) ?? []; @@ -149,6 +172,12 @@ public function storeConfigKeyToFormElementMap(array $element, FormStateInterfac if (is_string($target)) { $target = ConfigTarget::fromString($target); } + elseif ($target->toConfig instanceof \Closure || $target->fromConfig instanceof \Closure) { + // If the form is using closures as toConfig or fromConfig callables + // then form cannot be cached. + $form_state->disableCache(); + } + foreach ($target->propertyPaths as $property_path) { if (isset($map[$target->configName][$property_path])) { throw new \LogicException(sprintf('Two #config_targets both target "%s" in the "%s" config: `%s` and `%s`.', @@ -163,7 +192,7 @@ public function storeConfigKeyToFormElementMap(array $element, FormStateInterfac $form_state->set(static::CONFIG_KEY_TO_FORM_ELEMENT_MAP, $map); } foreach (Element::children($element) as $key) { - $element[$key] = $this->storeConfigKeyToFormElementMap($element[$key], $form_state); + $element[$key] = $this->doStoreConfigMap($element[$key], $form_state); } return $element; } diff --git a/core/lib/Drupal/Core/Form/ConfigTarget.php b/core/lib/Drupal/Core/Form/ConfigTarget.php index 867dbf1a7274a88808cb9b2f96ca4b8788ced45e..55b86dc6b4b55220d3323e139cd411d31303df55 100644 --- a/core/lib/Drupal/Core/Form/ConfigTarget.php +++ b/core/lib/Drupal/Core/Form/ConfigTarget.php @@ -36,20 +36,20 @@ final class ConfigTarget { /** * Transforms a value loaded from config before it gets displayed by the form. * - * @var \Closure|null + * @var callable|null * * @see ::getValue() */ - public readonly ?\Closure $fromConfig; + public readonly mixed $fromConfig; /** * Transforms a value submitted by the form before it is set in the config. * - * @var \Closure|null + * @var callable|null * * @see ::setValue() */ - public readonly ?\Closure $toConfig; + public readonly mixed $toConfig; /** * Constructs a ConfigTarget object. @@ -86,8 +86,8 @@ public function __construct( ?callable $fromConfig = NULL, ?callable $toConfig = NULL, ) { - $this->fromConfig = $fromConfig ? $fromConfig(...) : NULL; - $this->toConfig = $toConfig ? $toConfig(...) : NULL; + $this->fromConfig = $fromConfig; + $this->toConfig = $toConfig; if (is_string($propertyPath)) { $propertyPath = [$propertyPath]; diff --git a/core/modules/system/tests/modules/form_test/form_test.routing.yml b/core/modules/system/tests/modules/form_test/form_test.routing.yml index c96086440339115ca9d5d46d45dc4b1b63f8f205..453aafb9c812a0bbce8fb69051c5e02f4436cea3 100644 --- a/core/modules/system/tests/modules/form_test/form_test.routing.yml +++ b/core/modules/system/tests/modules/form_test/form_test.routing.yml @@ -533,6 +533,8 @@ form_test.nested_config_target: path: '/form-test/nested-config-target' defaults: _form: '\Drupal\form_test\Form\NestedConfigTargetForm' + options: + _admin_route: TRUE requirements: _access: 'TRUE' @@ -540,6 +542,8 @@ form_test.tree_config_target: path: '/form-test/tree-config-target' defaults: _form: '\Drupal\form_test\Form\TreeConfigTargetForm' + options: + _admin_route: TRUE requirements: _access: 'TRUE' @@ -547,5 +551,7 @@ form_test.incorrect_config_target: path: '/form-test/incorrect-config-target' defaults: _form: '\Drupal\form_test\Form\IncorrectConfigTargetForm' + options: + _admin_route: TRUE requirements: _access: 'TRUE' diff --git a/core/modules/system/tests/modules/form_test/src/Form/TreeConfigTargetForm.php b/core/modules/system/tests/modules/form_test/src/Form/TreeConfigTargetForm.php index 5d84cd1c86757c9b31862408ac95e03f6f39723c..dbf1c427249b299372255fc6321caca025c0ab67 100644 --- a/core/modules/system/tests/modules/form_test/src/Form/TreeConfigTargetForm.php +++ b/core/modules/system/tests/modules/form_test/src/Form/TreeConfigTargetForm.php @@ -43,7 +43,42 @@ public function buildForm(array $form, FormStateInterface $form_state) { '#title' => t('Nemesis'), '#config_target' => 'form_test.object:nemesis_vegetable', ]; + + $form['test1'] = [ + '#type' => 'select', + '#title' => $this->t('Test 1'), + '#options' => [ + 'option1' => $this->t('Option 1'), + 'option2' => $this->t('Option 2'), + ], + '#ajax' => [ + 'callback' => '::updateOptions', + 'wrapper' => 'edit-test1-wrapper', + ], + '#prefix' => '<div id="edit-test1-wrapper">', + '#suffix' => '</div>', + ]; return parent::buildForm($form, $form_state); } + /** + * Updates the options of a select list. + * + * @param array $form + * An associative array containing the structure of the form. + * @param \Drupal\Core\Form\FormStateInterface $form_state + * The current state of the form. + * + * @return array + * The updated form element. + */ + public function updateOptions(array $form, FormStateInterface $form_state) { + $form['test1']['#options']['option1'] = $this->t('Option 1!!!'); + $form['test1']['#options'] += [ + 'option3' => $this->t('Option 3'), + 'option4' => $this->t('Option 4'), + ]; + return $form['test1']; + } + } diff --git a/core/modules/system/tests/src/FunctionalJavascript/Form/ConfigTargetTest.php b/core/modules/system/tests/src/FunctionalJavascript/Form/ConfigTargetTest.php new file mode 100644 index 0000000000000000000000000000000000000000..e9e092f23ee147181e5da69391dd9d9b1baecd64 --- /dev/null +++ b/core/modules/system/tests/src/FunctionalJavascript/Form/ConfigTargetTest.php @@ -0,0 +1,86 @@ +<?php + +declare(strict_types=1); + +namespace Drupal\Tests\system\FunctionalJavascript\Form; + +use Drupal\FunctionalJavascriptTests\WebDriverTestBase; + +/** + * Tests forms using #config_target and #ajax together. + * + * @group Form + */ +class ConfigTargetTest extends WebDriverTestBase { + + /** + * {@inheritdoc} + */ + protected static $modules = ['form_test']; + + /** + * {@inheritdoc} + */ + protected $defaultTheme = 'stark'; + + /** + * Tests #config_target with no callbacks. + * + * If a #config_target has no callbacks, the form can be cached. + */ + public function testTree(): void { + /** @var \Drupal\Core\KeyValueStore\KeyValueStoreExpirableInterface $key_value_expirable */ + $key_value_expirable = \Drupal::service('keyvalue.expirable')->get('form'); + + $page = $this->getSession()->getPage(); + $assert_session = $this->assertSession(); + + $this->drupalGet('/form-test/tree-config-target'); + $this->assertCount(0, $key_value_expirable->getAll()); + $page->fillField('Nemesis', 'Test'); + $assert_session->pageTextNotContains('Option 3'); + $page->selectFieldOption('test1', 'Option 2'); + $assert_session->waitForText('Option 3'); + $assert_session->pageTextContains('Option 3'); + // The ajax request should result in the form being cached. + $this->assertCount(1, $key_value_expirable->getAll()); + + $page->pressButton('Save configuration'); + $assert_session->pageTextContains('The configuration options have been saved.'); + $assert_session->fieldValueEquals('Nemesis', 'Test'); + + // The form cache will be deleted after submission. + $this->assertCount(0, $key_value_expirable->getAll()); + } + + /** + * Tests #config_target with callbacks. + * + * If a #config_target has closures as callbacks, form cache will be disabled. + */ + public function testNested(): void { + /** @var \Drupal\Core\KeyValueStore\KeyValueStoreExpirableInterface $key_value_expirable */ + $key_value_expirable = \Drupal::service('keyvalue.expirable')->get('form'); + $page = $this->getSession()->getPage(); + $assert_session = $this->assertSession(); + + $this->drupalGet('/form-test/nested-config-target'); + $this->assertCount(0, $key_value_expirable->getAll()); + $page->fillField('First choice', 'Apple'); + $page->fillField('Second choice', 'Kiwi'); + + $assert_session->pageTextNotContains('Option 3'); + $page->selectFieldOption('test1', 'Option 2'); + $assert_session->waitForText('Option 3'); + $assert_session->pageTextContains('Option 3'); + $this->assertCount(0, $key_value_expirable->getAll()); + + $page->pressButton('Save configuration'); + $assert_session->statusMessageContains('The configuration options have been saved.', 'status'); + + $assert_session->fieldValueEquals('First choice', 'Apple'); + $assert_session->fieldValueEquals('Second choice', 'Kiwi'); + $this->assertCount(0, $key_value_expirable->getAll()); + } + +} diff --git a/core/tests/Drupal/Tests/Core/Form/ConfigTargetTest.php b/core/tests/Drupal/Tests/Core/Form/ConfigTargetTest.php index 3932d14552a0791c156a34b0efc8cfee70cb96cf..5b153c0fd1dc3bffcae560b46f720fe13ff4c2ef 100644 --- a/core/tests/Drupal/Tests/Core/Form/ConfigTargetTest.php +++ b/core/tests/Drupal/Tests/Core/Form/ConfigTargetTest.php @@ -52,11 +52,60 @@ public function getFormId() { }; $form_state = new FormState(); - $test_form->storeConfigKeyToFormElementMap($form['test'], $form_state); $this->expectException(\LogicException::class); $this->expectExceptionMessage('Two #config_targets both target "admin_compact_mode" in the "system.site" config: `$form[\'test\']` and `$form[\'duplicate\']`.'); - $test_form->storeConfigKeyToFormElementMap($form['duplicate'], $form_state); + $test_form->storeConfigKeyToFormElementMap($form, $form_state); + } + + /** + * @covers \Drupal\Core\Form\ConfigFormBase::storeConfigKeyToFormElementMap + * @dataProvider providerTestFormCacheable + */ + public function testFormCacheable(bool $expected, ?callable $fromConfig, ?callable $toConfig): void { + $form = [ + 'test' => [ + '#type' => 'text', + '#default_value' => 'A test', + '#config_target' => new ConfigTarget('system.site', 'admin_compact_mode', $fromConfig, $toConfig), + '#name' => 'test', + '#array_parents' => ['test'], + ], + ]; + + $test_form = new class( + $this->prophesize(ConfigFactoryInterface::class)->reveal(), + $this->prophesize(TypedConfigManagerInterface::class)->reveal(), + ) extends ConfigFormBase { + use RedundantEditableConfigNamesTrait; + + public function getFormId() { + return 'test'; + } + + }; + $form_state = new FormState(); + // Make the form cacheable. + $form_state + ->setRequestMethod('POST') + ->setCached(); + + $test_form->storeConfigKeyToFormElementMap($form, $form_state); + + $this->assertSame($expected, $form_state->isCached()); + } + + public function providerTestFormCacheable(): array { + $closure = fn (bool $something): string => $something ? 'Yes' : 'No'; + return [ + 'No callables' => [TRUE, NULL, NULL], + 'Serializable fromConfig callable' => [TRUE, "intval", NULL], + 'Serializable toConfig callable' => [TRUE, NULL, "boolval"], + 'Serializable callables' => [TRUE, "intval", "boolval"], + 'Unserializable fromConfig callable' => [FALSE, $closure, NULL], + 'Unserializable toConfig callable' => [FALSE, NULL, $closure], + 'Unserializable callables' => [FALSE, $closure, $closure], + ]; } /**