Skip to content

Commit 4366f38

Browse files
bojanzbojanz
committed
Issue #2884870 by bojanz: Plugin configuration is not validated/submitted by the plugin that built the form
1 parent df37432 commit 4366f38

File tree

14 files changed

+261
-137
lines changed

14 files changed

+261
-137
lines changed

‎modules/payment/src/Entity/PaymentGateway.php

+16
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,22 @@ public function setPluginId($plugin_id) {
137137
return $this;
138138
}
139139

140+
/**
141+
* {@inheritdoc}
142+
*/
143+
public function getPluginConfiguration() {
144+
return $this->configuration;
145+
}
146+
147+
/**
148+
* {@inheritdoc}
149+
*/
150+
public function setPluginConfiguration(array $configuration) {
151+
$this->configuration = $configuration;
152+
$this->pluginCollection = NULL;
153+
return $this;
154+
}
155+
140156
/**
141157
* {@inheritdoc}
142158
*/

‎modules/payment/src/Entity/PaymentGatewayInterface.php

+18
Original file line numberDiff line numberDiff line change
@@ -56,4 +56,22 @@ public function getPluginId();
5656
*/
5757
public function setPluginId($plugin_id);
5858

59+
/**
60+
* Gets the payment gateway plugin configuration.
61+
*
62+
* @return string
63+
* The payment gateway plugin configuration.
64+
*/
65+
public function getPluginConfiguration();
66+
67+
/**
68+
* Sets the payment gateway plugin configuration.
69+
*
70+
* @param array $configuration
71+
* The payment gateway plugin configuration.
72+
*
73+
* @return $this
74+
*/
75+
public function setPluginConfiguration(array $configuration);
76+
5977
}

‎modules/payment/src/Form/PaymentGatewayForm.php

+5-14
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,11 @@ public function form(array $form, FormStateInterface $form_state) {
102102
],
103103
];
104104
$form['configuration'] = [
105-
'#parents' => ['configuration'],
105+
'#type' => 'commerce_plugin_configuration',
106+
'#plugin_type' => 'commerce_payment_gateway',
107+
'#plugin_id' => $plugin,
108+
'#default_value' => $gateway->getPluginConfiguration(),
106109
];
107-
$form['configuration'] = $gateway->getPlugin()->buildConfigurationForm($form['configuration'], $form_state);
108110
$form['status'] = [
109111
'#type' => 'checkbox',
110112
'#title' => $this->t('Enabled'),
@@ -121,17 +123,6 @@ public static function ajaxRefresh(array $form, FormStateInterface $form_state)
121123
return $form;
122124
}
123125

124-
/**
125-
* {@inheritdoc}
126-
*/
127-
public function validateForm(array &$form, FormStateInterface $form_state) {
128-
parent::validateForm($form, $form_state);
129-
130-
/** @var \Drupal\commerce_payment\Entity\PaymentGatewayInterface $gateway */
131-
$gateway = $this->entity;
132-
$gateway->getPlugin()->validateConfigurationForm($form['configuration'], $form_state);
133-
}
134-
135126
/**
136127
* {@inheritdoc}
137128
*/
@@ -140,7 +131,7 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
140131

141132
/** @var \Drupal\commerce_payment\Entity\PaymentGatewayInterface $gateway */
142133
$gateway = $this->entity;
143-
$gateway->getPlugin()->submitConfigurationForm($form['configuration'], $form_state);
134+
$gateway->setPluginConfiguration($form_state->getValue(['configuration']));
144135
}
145136

146137
/**

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

+6-3
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ abstract class PaymentGatewayBase extends PluginBase implements PaymentGatewayIn
3535
/**
3636
* The ID of the parent config entity.
3737
*
38+
* Not available while the plugin is being configured.
39+
*
3840
* @var string
3941
*/
4042
protected $entityId;
@@ -73,9 +75,10 @@ public function __construct(array $configuration, $plugin_id, $plugin_definition
7375
parent::__construct($configuration, $plugin_id, $plugin_definition);
7476

7577
$this->entityTypeManager = $entity_type_manager;
76-
// The plugin most know the ID of its parent config entity.
77-
$this->entityId = $configuration['_entity_id'];
78-
unset($configuration['_entity_id']);
78+
if (isset($configuration['_entity_id'])) {
79+
$this->entityId = $configuration['_entity_id'];
80+
unset($configuration['_entity_id']);
81+
}
7982
// Instantiate the types right away to ensure that their IDs are valid.
8083
$this->paymentType = $payment_type_manager->createInstance($this->pluginDefinition['payment_type']);
8184
foreach ($this->pluginDefinition['payment_method_types'] as $plugin_id) {

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

+4-4
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ public function testPaymentGatewayCreation() {
3939
$values = [
4040
'label' => 'Example',
4141
'plugin' => 'example_offsite_redirect',
42-
'configuration[redirect_method]' => 'post',
43-
'configuration[mode]' => 'test',
42+
'configuration[form][redirect_method]' => 'post',
43+
'configuration[form][mode]' => 'test',
4444
'status' => '1',
4545
// Setting the 'id' can fail if focus switches to another field.
4646
// This is a bug in the machine name JS that can be reproduced manually.
@@ -76,8 +76,8 @@ public function testPaymentGatewayEditing() {
7676

7777
$this->drupalGet('admin/commerce/config/payment-gateways/manage/' . $payment_gateway->id());
7878
$values += [
79-
'configuration[redirect_method]' => 'get',
80-
'configuration[mode]' => 'live',
79+
'configuration[form][redirect_method]' => 'get',
80+
'configuration[form][mode]' => 'live',
8181
];
8282
$this->submitForm($values, 'Save');
8383

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

+8-8
Original file line numberDiff line numberDiff line change
@@ -51,18 +51,18 @@ public function testCreatePromotion() {
5151

5252
$this->getSession()->getPage()->selectFieldOption('offer[0][plugin_select][target_plugin_id]', 'commerce_promotion_product_percentage_off');
5353
$this->waitForAjaxToFinish();
54-
$this->getSession()->getPage()->fillField('offer[0][plugin_select][target_plugin_configuration][amount]', '10.0');
54+
$this->getSession()->getPage()->fillField('offer[0][plugin_select][target_plugin_configuration][form][amount]', '10.0');
5555

5656
// Change, assert any values reset.
5757
$this->getSession()->getPage()->selectFieldOption('offer[0][plugin_select][target_plugin_id]', 'commerce_promotion_order_percentage_off');
5858
$this->waitForAjaxToFinish();
59-
$this->assertSession()->fieldValueNotEquals('offer[0][plugin_select][target_plugin_configuration][amount]', '10.0');
60-
$this->getSession()->getPage()->fillField('offer[0][plugin_select][target_plugin_configuration][amount]', '10.0');
59+
$this->assertSession()->fieldValueNotEquals('offer[0][plugin_select][target_plugin_configuration][form][amount]', '10.0');
60+
$this->getSession()->getPage()->fillField('offer[0][plugin_select][target_plugin_configuration][form][amount]', '10.0');
6161

6262
$this->getSession()->getPage()->selectFieldOption('conditions[0][plugin_select][target_plugin_id]', 'commerce_promotion_order_total_price');
6363
$this->waitForAjaxToFinish();
64-
$this->getSession()->getPage()->fillField('conditions[0][plugin_select][target_plugin_configuration][amount][number]', '50.00');
65-
$this->getSession()->getPage()->checkField('conditions[0][plugin_select][target_plugin_configuration][negate]');
64+
$this->getSession()->getPage()->fillField('conditions[0][plugin_select][target_plugin_configuration][form][amount][number]', '50.00');
65+
$this->getSession()->getPage()->checkField('conditions[0][plugin_select][target_plugin_configuration][form][negate]');
6666

6767
// Confirm that the usage limit widget works properly.
6868
$this->getSession()->getPage()->hasCheckedField(' Unlimited');
@@ -110,13 +110,13 @@ public function testCreatePromotionWithEndDate() {
110110
$name = $this->randomMachineName(8);
111111
$edit = [
112112
'name[0][value]' => $name,
113-
'offer[0][plugin_select][target_plugin_configuration][amount]' => '10.0',
113+
'offer[0][plugin_select][target_plugin_configuration][form][amount]' => '10.0',
114114
];
115115

116116
$this->getSession()->getPage()->fillField('conditions[0][plugin_select][target_plugin_id]', 'commerce_promotion_order_total_price');
117117
$this->waitForAjaxToFinish();
118118

119-
$edit['conditions[0][plugin_select][target_plugin_configuration][amount][number]'] = '50.00';
119+
$edit['conditions[0][plugin_select][target_plugin_configuration][form][amount][number]'] = '50.00';
120120

121121
// Set an end date.
122122
$this->getSession()->getPage()->checkField('end_date[0][has_value]');
@@ -155,7 +155,7 @@ public function testEditPromotion() {
155155
$new_promotion_name = $this->randomMachineName(8);
156156
$edit = [
157157
'name[0][value]' => $new_promotion_name,
158-
'offer[0][plugin_select][target_plugin_configuration][amount]' => '20',
158+
'offer[0][plugin_select][target_plugin_configuration][form][amount]' => '20',
159159
];
160160
$this->submitForm($edit, 'Save');
161161

‎modules/tax/src/Entity/TaxType.php

+16
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,22 @@ public function setPluginId($plugin_id) {
110110
return $this;
111111
}
112112

113+
/**
114+
* {@inheritdoc}
115+
*/
116+
public function getPluginConfiguration() {
117+
return $this->configuration;
118+
}
119+
120+
/**
121+
* {@inheritdoc}
122+
*/
123+
public function setPluginConfiguration(array $configuration) {
124+
$this->configuration = $configuration;
125+
$this->pluginCollection = NULL;
126+
return $this;
127+
}
128+
113129
/**
114130
* {@inheritdoc}
115131
*/

‎modules/tax/src/Entity/TaxTypeInterface.php

+18
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,22 @@ public function getPluginId();
3838
*/
3939
public function setPluginId($plugin_id);
4040

41+
/**
42+
* Gets the tax type plugin configuration.
43+
*
44+
* @return string
45+
* The tax type plugin configuration.
46+
*/
47+
public function getPluginConfiguration();
48+
49+
/**
50+
* Sets the tax type plugin configuration.
51+
*
52+
* @param array $configuration
53+
* The tax type plugin configuration.
54+
*
55+
* @return $this
56+
*/
57+
public function setPluginConfiguration(array $configuration);
58+
4159
}

‎modules/tax/src/Form/TaxTypeForm.php

+5-14
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,11 @@ public function form(array $form, FormStateInterface $form_state) {
9292
],
9393
];
9494
$form['configuration'] = [
95-
'#parents' => ['configuration'],
95+
'#type' => 'commerce_plugin_configuration',
96+
'#plugin_type' => 'commerce_tax_type',
97+
'#plugin_id' => $plugin,
98+
'#default_value' => $type->getPluginConfiguration(),
9699
];
97-
$form['configuration'] = $type->getPlugin()->buildConfigurationForm($form['configuration'], $form_state);
98100
$form['status'] = [
99101
'#type' => 'checkbox',
100102
'#title' => $this->t('Enabled'),
@@ -111,17 +113,6 @@ public static function ajaxRefresh(array $form, FormStateInterface $form_state)
111113
return $form;
112114
}
113115

114-
/**
115-
* {@inheritdoc}
116-
*/
117-
public function validateForm(array &$form, FormStateInterface $form_state) {
118-
parent::validateForm($form, $form_state);
119-
120-
/** @var \Drupal\commerce_tax\Entity\TaxTypeInterface $type */
121-
$type = $this->entity;
122-
$type->getPlugin()->validateConfigurationForm($form['configuration'], $form_state);
123-
}
124-
125116
/**
126117
* {@inheritdoc}
127118
*/
@@ -130,7 +121,7 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
130121

131122
/** @var \Drupal\commerce_tax\Entity\TaxTypeInterface $type */
132123
$type = $this->entity;
133-
$type->getPlugin()->submitConfigurationForm($form['configuration'], $form_state);
124+
$type->setPluginConfiguration($form_state->getValue(['configuration']));
134125
}
135126

136127
/**

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

+3-6
Original file line numberDiff line numberDiff line change
@@ -273,19 +273,16 @@ public function removeTerritorySubmit(array $form, FormStateInterface $form_stat
273273
* {@inheritdoc}
274274
*/
275275
public function validateConfigurationForm(array &$form, FormStateInterface $form_state) {
276-
if (!isset($form['territories'])) {
277-
// The form was built by a different plugin, and is now in the process
278-
// of being rebuilt. Temporary workaround for #2884870.
279-
return;
280-
}
281-
282276
$values = $form_state->getValue($form['#parents']);
277+
// Filter out the button rows.
283278
$values['rates'] = array_filter($values['rates'], function ($rate) {
284279
return !empty($rate) && !isset($rate['add_rate']);
285280
});
286281
$values['territories'] = array_filter($values['territories'], function ($territory) {
287282
return !empty($territory) && !isset($territory['add_territory']);
288283
});
284+
$form_state->setValue($form['#parents'], $values);
285+
289286
if (empty($values['rates'])) {
290287
$form_state->setError($form['rates'], $this->t('Please add at least one rate.'));
291288
}

‎modules/tax/src/Plugin/Commerce/TaxType/TaxTypeBase.php

+6-3
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ abstract class TaxTypeBase extends PluginBase implements TaxTypeInterface, Conta
3939
/**
4040
* The ID of the parent config entity.
4141
*
42+
* Not available while the plugin is being configured.
43+
*
4244
* @var string
4345
*/
4446
protected $entityId;
@@ -69,9 +71,10 @@ public function __construct(array $configuration, $plugin_id, $plugin_definition
6971

7072
$this->entityTypeManager = $entity_type_manager;
7173
$this->eventDispatcher = $event_dispatcher;
72-
// The plugin most know the ID of its parent config entity.
73-
$this->entityId = $configuration['_entity_id'];
74-
unset($configuration['_entity_id']);
74+
if (isset($configuration['_entity_id'])) {
75+
$this->entityId = $configuration['_entity_id'];
76+
unset($configuration['_entity_id']);
77+
}
7578
$this->setConfiguration($configuration);
7679
}
7780

0 commit comments

Comments
 (0)