Issue #2815829 by godotislate, tim.plunkett, smustgrave, alexpott, xjm,...

Issue #2815829 by godotislate, tim.plunkett, smustgrave, alexpott, xjm, bircher, paulocs: Adding or editing a block through the UI saves the entity twice

(cherry picked from commit 24b8523c)

core/lib/Drupal/Core/Condition/ConditionPluginCollection.php

@@ -44,7 +44,12 @@ public function getConfiguration() {
       unset($instance_config['context_mapping']);
       ksort($default_config);
       ksort($instance_config);
-      if ($default_config === $instance_config) {
+      // With PHP 8 type juggling, there should not be an issue using equal
+      // operator instead of identical operator. Allowing looser comparison here
+      // will prevent configuration from being erroneously exported when values
+      // are updated via form elements that return values of the wrong type, for
+      // example, '0'/'1' vs FALSE/TRUE.
+      if ($default_config == $instance_config) {
         unset($configuration[$instance_id]);
       }
     }

core/modules/block/src/BlockForm.php

@@ -309,13 +309,6 @@ public function validateForm(array &$form, FormStateInterface $form_state) {
   protected function validateVisibility(array $form, FormStateInterface $form_state) {
     // Validate visibility condition settings.
     foreach ($form_state->getValue('visibility') as $condition_id => $values) {
-      // All condition plugins use 'negate' as a Boolean in their schema.
-      // However, certain form elements may return it as 0/1. Cast here to
-      // ensure the data is in the expected type.
-      if (array_key_exists('negate', $values)) {
-        $form_state->setValue(['visibility', $condition_id, 'negate'], (bool) $values['negate']);
-      }
-
       // Allow the condition to validate the form.
       $condition = $form_state->get(['conditions', $condition_id]);
       $condition->validateConfigurationForm($form['visibility'][$condition_id], SubformState::createForSubform($form['visibility'][$condition_id], $form, $form_state));
@@ -342,9 +335,13 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
     }
 
     $this->submitVisibility($form, $form_state);
+  }
 
-    // Save the settings of the plugin.
-    $entity->save();
+  /**
+   * {@inheritdoc}
+   */
+  public function save(array $form, FormStateInterface $form_state) {
+    $value = parent::save($form, $form_state);
 
     $this->messenger()->addStatus($this->t('The block configuration has been saved.'));
     $form_state->setRedirect(
@@ -354,6 +351,8 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
       ],
       ['query' => ['block-placement' => Html::getClass($this->entity->id())]]
     );
+
+    return $value;
   }
 
   /**

core/tests/Drupal/KernelTests/Core/Condition/ConditionPluginCollectionTest.php

+<?php
+
+namespace Drupal\KernelTests\Core\Condition;
+
+use Drupal\Core\Condition\ConditionPluginCollection;
+use Drupal\KernelTests\KernelTestBase;
+
+/**
+ * @coversDefaultClass \Drupal\Core\Condition\ConditionPluginCollection
+ *
+ * @group Condition
+ */
+class ConditionPluginCollectionTest extends KernelTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $modules = [
+    'system',
+    'user',
+    'path_alias',
+  ];
+
+  /**
+   * @covers ::getConfiguration
+   */
+  public function testGetConfiguration(): void {
+    // Include a condition that has custom configuration and a type mismatch on
+    // 'negate' by using 0 instead of FALSE.
+    $configuration['request_path'] = [
+      'id' => 'request_path',
+      'negate' => 0,
+      'context_mapping' => [],
+      'pages' => '/user/*',
+    ];
+    // Include a condition that matches default values but with a type mismatch
+    // on 'negate' by using 0 instead of FALSE. This condition will be removed,
+    // because condition configurations that match default values with "=="
+    // comparison are not saved or exported.
+    $configuration['user_role'] = [
+      'id' => 'user_role',
+      'negate' => '0',
+      'context_mapping' => [],
+      'roles' => [],
+    ];
+    $collection = new ConditionPluginCollection(\Drupal::service('plugin.manager.condition'), $configuration);
+
+    $expected['request_path'] = [
+      'id' => 'request_path',
+      'negate' => 0,
+      'context_mapping' => [],
+      'pages' => '/user/*',
+    ];
+    // NB: The 'user_role' property should not exist in expected set.
+    $this->assertSame($expected, $collection->getConfiguration());
+  }
+
+}