Issue #2884870 by bojanz: Plugin configuration is not validated/submitted by the plugin that built the form

Author: bojanz

modules/payment/src/Entity/PaymentGateway.php

@@ -137,6 +137,22 @@ public function setPluginId($plugin_id) {
     return $this;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function getPluginConfiguration() {
+    return $this->configuration;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function setPluginConfiguration(array $configuration) {
+    $this->configuration = $configuration;
+    $this->pluginCollection = NULL;
+    return $this;
+  }
+
   /**
    * {@inheritdoc}
    */

modules/payment/src/Entity/PaymentGatewayInterface.php

@@ -56,4 +56,22 @@ public function getPluginId();
    */
   public function setPluginId($plugin_id);
 
+  /**
+   * Gets the payment gateway plugin configuration.
+   *
+   * @return string
+   *   The payment gateway plugin configuration.
+   */
+  public function getPluginConfiguration();
+
+  /**
+   * Sets the payment gateway plugin configuration.
+   *
+   * @param array $configuration
+   *   The payment gateway plugin configuration.
+   *
+   * @return $this
+   */
+  public function setPluginConfiguration(array $configuration);
+
 }

modules/payment/src/Form/PaymentGatewayForm.php

@@ -102,9 +102,11 @@ public function form(array $form, FormStateInterface $form_state) {
       ],
     ];
     $form['configuration'] = [
-      '#parents' => ['configuration'],
+      '#type' => 'commerce_plugin_configuration',
+      '#plugin_type' => 'commerce_payment_gateway',
+      '#plugin_id' => $plugin,
+      '#default_value' => $gateway->getPluginConfiguration(),
     ];
-    $form['configuration'] = $gateway->getPlugin()->buildConfigurationForm($form['configuration'], $form_state);
     $form['status'] = [
       '#type' => 'checkbox',
       '#title' => $this->t('Enabled'),
@@ -121,17 +123,6 @@ public static function ajaxRefresh(array $form, FormStateInterface $form_state)
     return $form;
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function validateForm(array &$form, FormStateInterface $form_state) {
-    parent::validateForm($form, $form_state);
-
-    /** @var \Drupal\commerce_payment\Entity\PaymentGatewayInterface $gateway */
-    $gateway = $this->entity;
-    $gateway->getPlugin()->validateConfigurationForm($form['configuration'], $form_state);
-  }
-
   /**
    * {@inheritdoc}
    */
@@ -140,7 +131,7 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
 
     /** @var \Drupal\commerce_payment\Entity\PaymentGatewayInterface $gateway */
     $gateway = $this->entity;
-    $gateway->getPlugin()->submitConfigurationForm($form['configuration'], $form_state);
+    $gateway->setPluginConfiguration($form_state->getValue(['configuration']));
   }
 
   /**

modules/payment/src/Plugin/Commerce/PaymentGateway/PaymentGatewayBase.php

@@ -35,6 +35,8 @@ abstract class PaymentGatewayBase extends PluginBase implements PaymentGatewayIn
   /**
    * The ID of the parent config entity.
    *
+   * Not available while the plugin is being configured.
+   *
    * @var string
    */
   protected $entityId;
@@ -73,9 +75,10 @@ public function __construct(array $configuration, $plugin_id, $plugin_definition
     parent::__construct($configuration, $plugin_id, $plugin_definition);
 
     $this->entityTypeManager = $entity_type_manager;
-    // The plugin most know the ID of its parent config entity.
-    $this->entityId = $configuration['_entity_id'];
-    unset($configuration['_entity_id']);
+    if (isset($configuration['_entity_id'])) {
+      $this->entityId = $configuration['_entity_id'];
+      unset($configuration['_entity_id']);
+    }
     // Instantiate the types right away to ensure that their IDs are valid.
     $this->paymentType = $payment_type_manager->createInstance($this->pluginDefinition['payment_type']);
     foreach ($this->pluginDefinition['payment_method_types'] as $plugin_id) {

modules/payment/tests/src/Functional/PaymentGatewayTest.php

@@ -39,8 +39,8 @@ public function testPaymentGatewayCreation() {
     $values = [
       'label' => 'Example',
       'plugin' => 'example_offsite_redirect',
-      'configuration[redirect_method]' => 'post',
-      'configuration[mode]' => 'test',
+      'configuration[form][redirect_method]' => 'post',
+      'configuration[form][mode]' => 'test',
       'status' => '1',
       // Setting the 'id' can fail if focus switches to another field.
       // This is a bug in the machine name JS that can be reproduced manually.
@@ -76,8 +76,8 @@ public function testPaymentGatewayEditing() {
 
     $this->drupalGet('admin/commerce/config/payment-gateways/manage/' . $payment_gateway->id());
     $values += [
-      'configuration[redirect_method]' => 'get',
-      'configuration[mode]' => 'live',
+      'configuration[form][redirect_method]' => 'get',
+      'configuration[form][mode]' => 'live',
     ];
     $this->submitForm($values, 'Save');
 

modules/promotion/tests/src/FunctionalJavascript/PromotionTest.php

@@ -51,18 +51,18 @@ public function testCreatePromotion() {
 
     $this->getSession()->getPage()->selectFieldOption('offer[0][plugin_select][target_plugin_id]', 'commerce_promotion_product_percentage_off');
     $this->waitForAjaxToFinish();
-    $this->getSession()->getPage()->fillField('offer[0][plugin_select][target_plugin_configuration][amount]', '10.0');
+    $this->getSession()->getPage()->fillField('offer[0][plugin_select][target_plugin_configuration][form][amount]', '10.0');
 
     // Change, assert any values reset.
     $this->getSession()->getPage()->selectFieldOption('offer[0][plugin_select][target_plugin_id]', 'commerce_promotion_order_percentage_off');
     $this->waitForAjaxToFinish();
-    $this->assertSession()->fieldValueNotEquals('offer[0][plugin_select][target_plugin_configuration][amount]', '10.0');
-    $this->getSession()->getPage()->fillField('offer[0][plugin_select][target_plugin_configuration][amount]', '10.0');
+    $this->assertSession()->fieldValueNotEquals('offer[0][plugin_select][target_plugin_configuration][form][amount]', '10.0');
+    $this->getSession()->getPage()->fillField('offer[0][plugin_select][target_plugin_configuration][form][amount]', '10.0');
 
     $this->getSession()->getPage()->selectFieldOption('conditions[0][plugin_select][target_plugin_id]', 'commerce_promotion_order_total_price');
     $this->waitForAjaxToFinish();
-    $this->getSession()->getPage()->fillField('conditions[0][plugin_select][target_plugin_configuration][amount][number]', '50.00');
-    $this->getSession()->getPage()->checkField('conditions[0][plugin_select][target_plugin_configuration][negate]');
+    $this->getSession()->getPage()->fillField('conditions[0][plugin_select][target_plugin_configuration][form][amount][number]', '50.00');
+    $this->getSession()->getPage()->checkField('conditions[0][plugin_select][target_plugin_configuration][form][negate]');
 
     // Confirm that the usage limit widget works properly.
     $this->getSession()->getPage()->hasCheckedField(' Unlimited');
@@ -110,13 +110,13 @@ public function testCreatePromotionWithEndDate() {
     $name = $this->randomMachineName(8);
     $edit = [
       'name[0][value]' => $name,
-      'offer[0][plugin_select][target_plugin_configuration][amount]' => '10.0',
+      'offer[0][plugin_select][target_plugin_configuration][form][amount]' => '10.0',
     ];
 
     $this->getSession()->getPage()->fillField('conditions[0][plugin_select][target_plugin_id]', 'commerce_promotion_order_total_price');
     $this->waitForAjaxToFinish();
 
-    $edit['conditions[0][plugin_select][target_plugin_configuration][amount][number]'] = '50.00';
+    $edit['conditions[0][plugin_select][target_plugin_configuration][form][amount][number]'] = '50.00';
 
     // Set an end date.
     $this->getSession()->getPage()->checkField('end_date[0][has_value]');
@@ -155,7 +155,7 @@ public function testEditPromotion() {
     $new_promotion_name = $this->randomMachineName(8);
     $edit = [
       'name[0][value]' => $new_promotion_name,
-      'offer[0][plugin_select][target_plugin_configuration][amount]' => '20',
+      'offer[0][plugin_select][target_plugin_configuration][form][amount]' => '20',
     ];
     $this->submitForm($edit, 'Save');
 

modules/tax/src/Entity/TaxType.php

@@ -110,6 +110,22 @@ public function setPluginId($plugin_id) {
     return $this;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function getPluginConfiguration() {
+    return $this->configuration;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function setPluginConfiguration(array $configuration) {
+    $this->configuration = $configuration;
+    $this->pluginCollection = NULL;
+    return $this;
+  }
+
   /**
    * {@inheritdoc}
    */

modules/tax/src/Entity/TaxTypeInterface.php

@@ -38,4 +38,22 @@ public function getPluginId();
    */
   public function setPluginId($plugin_id);
 
+  /**
+   * Gets the tax type plugin configuration.
+   *
+   * @return string
+   *   The tax type plugin configuration.
+   */
+  public function getPluginConfiguration();
+
+  /**
+   * Sets the tax type plugin configuration.
+   *
+   * @param array $configuration
+   *   The tax type plugin configuration.
+   *
+   * @return $this
+   */
+  public function setPluginConfiguration(array $configuration);
+
 }

modules/tax/src/Form/TaxTypeForm.php

@@ -92,9 +92,11 @@ public function form(array $form, FormStateInterface $form_state) {
       ],
     ];
     $form['configuration'] = [
-      '#parents' => ['configuration'],
+      '#type' => 'commerce_plugin_configuration',
+      '#plugin_type' => 'commerce_tax_type',
+      '#plugin_id' => $plugin,
+      '#default_value' => $type->getPluginConfiguration(),
     ];
-    $form['configuration'] = $type->getPlugin()->buildConfigurationForm($form['configuration'], $form_state);
     $form['status'] = [
       '#type' => 'checkbox',
       '#title' => $this->t('Enabled'),
@@ -111,17 +113,6 @@ public static function ajaxRefresh(array $form, FormStateInterface $form_state)
     return $form;
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function validateForm(array &$form, FormStateInterface $form_state) {
-    parent::validateForm($form, $form_state);
-
-    /** @var \Drupal\commerce_tax\Entity\TaxTypeInterface $type */
-    $type = $this->entity;
-    $type->getPlugin()->validateConfigurationForm($form['configuration'], $form_state);
-  }
-
   /**
    * {@inheritdoc}
    */
@@ -130,7 +121,7 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
 
     /** @var \Drupal\commerce_tax\Entity\TaxTypeInterface $type */
     $type = $this->entity;
-    $type->getPlugin()->submitConfigurationForm($form['configuration'], $form_state);
+    $type->setPluginConfiguration($form_state->getValue(['configuration']));
   }
 
   /**

modules/tax/src/Plugin/Commerce/TaxType/Custom.php

@@ -273,19 +273,16 @@ public function removeTerritorySubmit(array $form, FormStateInterface $form_stat
    * {@inheritdoc}
    */
   public function validateConfigurationForm(array &$form, FormStateInterface $form_state) {
-    if (!isset($form['territories'])) {
-      // The form was built by a different plugin, and is now in the process
-      // of being rebuilt. Temporary workaround for #2884870.
-      return;
-    }
-
     $values = $form_state->getValue($form['#parents']);
+    // Filter out the button rows.
     $values['rates'] = array_filter($values['rates'], function ($rate) {
       return !empty($rate) && !isset($rate['add_rate']);
     });
     $values['territories'] = array_filter($values['territories'], function ($territory) {
       return !empty($territory) && !isset($territory['add_territory']);
     });
+    $form_state->setValue($form['#parents'], $values);
+
     if (empty($values['rates'])) {
       $form_state->setError($form['rates'], $this->t('Please add at least one rate.'));
     }