From 4d78545b669a924ad5a6180d5564f6c6d7cbc0f7 Mon Sep 17 00:00:00 2001 From: Matt Glaman Date: Wed, 22 Mar 2017 21:33:52 -0500 Subject: [PATCH 01/10] Issue #2860264 by mikeNCM: This should cause a fail --- modules/promotion/tests/src/FunctionalJavascript/PromotionTest.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/modules/promotion/tests/src/FunctionalJavascript/PromotionTest.php b/modules/promotion/tests/src/FunctionalJavascript/PromotionTest.php index a743eb4..5411fbc 100644 --- a/modules/promotion/tests/src/FunctionalJavascript/PromotionTest.php +++ b/modules/promotion/tests/src/FunctionalJavascript/PromotionTest.php @@ -55,6 +55,7 @@ public function testCreatePromotion() { $this->getSession()->wait(2000, "jQuery('.ajax-progress').length === 0"); $edit['conditions[0][target_plugin_configuration][amount][number]'] = '50.00'; + $edit['conditions[0][target_plugin_configuration][negate]'] = 1; $this->submitForm($edit, t('Save')); $this->assertSession()->pageTextContains("Saved the $name promotion."); @@ -64,6 +65,11 @@ public function testCreatePromotion() { /** @var \Drupal\commerce\Plugin\Field\FieldType\PluginItem $offer_field */ $offer_field = Promotion::load(1)->get('offer')->first(); $this->assertEquals('0.10', $offer_field->target_plugin_configuration['amount']); + + /** @var \Drupal\commerce\Plugin\Field\FieldType\PluginItem $condition_field */ + $condition_field = Promotion::load(1)->get('conditions')->first(); + $this->assertEquals('50.00', $condition_field->target_plugin_configuration['amount']['number']); + $this->assertEquals(TRUE, $condition_field->target_plugin_configuration['negate']); } /** From 80a0b204648afde27181ee173e200f788b33a5a0 Mon Sep 17 00:00:00 2001 From: Matt Glaman Date: Wed, 22 Mar 2017 21:50:38 -0500 Subject: [PATCH 02/10] Issue #2860264 mikeNCM: Fix condition plugin submit handling --- .../src/Plugin/Commerce/PromotionCondition/OrderTotalPrice.php | 9 +++++++++ .../Commerce/PromotionCondition/PromotionConditionBase.php | 10 ++++++++++ 2 files changed, 19 insertions(+) diff --git a/modules/promotion/src/Plugin/Commerce/PromotionCondition/OrderTotalPrice.php b/modules/promotion/src/Plugin/Commerce/PromotionCondition/OrderTotalPrice.php index 0dc134b..d3df14c 100644 --- a/modules/promotion/src/Plugin/Commerce/PromotionCondition/OrderTotalPrice.php +++ b/modules/promotion/src/Plugin/Commerce/PromotionCondition/OrderTotalPrice.php @@ -52,6 +52,15 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta /** * {@inheritdoc} */ + public function submitConfigurationForm(array &$form, FormStateInterface $form_state) { + $values = $form_state->getValue($form['#parents']); + $this->configuration['amount'] = $values['amount']; + parent::submitConfigurationForm($form, $form_state); + } + + /** + * {@inheritdoc} + */ public function evaluate() { $amount = $this->configuration['amount']; if (empty($amount)) { diff --git a/modules/promotion/src/Plugin/Commerce/PromotionCondition/PromotionConditionBase.php b/modules/promotion/src/Plugin/Commerce/PromotionCondition/PromotionConditionBase.php index ccabb33..e7480bd 100644 --- a/modules/promotion/src/Plugin/Commerce/PromotionCondition/PromotionConditionBase.php +++ b/modules/promotion/src/Plugin/Commerce/PromotionCondition/PromotionConditionBase.php @@ -3,6 +3,8 @@ namespace Drupal\commerce_promotion\Plugin\Commerce\PromotionCondition; use Drupal\Core\Condition\ConditionPluginBase; +use Drupal\Core\Form\FormStateInterface; +use Drupal\Core\Form\SubformState; /** * Base class for Promotion Condition plugins. @@ -31,4 +33,12 @@ public function execute() { return $this->isNegated() ? !$result : $result; } + /** + * {@inheritdoc} + */ + public function submitConfigurationForm(array &$form, FormStateInterface $form_state) { + // @todo Update plugin_select to pass subform state automatically. + return parent::submitConfigurationForm($form, SubformState::createForSubform($form, $form_state->getCompleteForm(), $form_state)); + } + } From 50726c5c1aa49fee4a3809a9885fe90c8758f9ac Mon Sep 17 00:00:00 2001 From: Matt Glaman Date: Wed, 22 Mar 2017 21:51:14 -0500 Subject: [PATCH 03/10] Issue #2859423 mglaman: Update plugin_select element --- src/Element/PluginSelect.php | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/Element/PluginSelect.php b/src/Element/PluginSelect.php index 5146fc4..2fa1eb9 100644 --- a/src/Element/PluginSelect.php +++ b/src/Element/PluginSelect.php @@ -83,7 +83,6 @@ public static function processPluginSelect(&$element, FormStateInterface $form_s 'wrapper' => $ajax_wrapper_id, ], '#default_value' => $target_plugin_id, - '#ajax_array_parents' => $element['#array_parents'], '#required' => $element['#required'], ]; // Add a "_none" option if the element is not required. @@ -126,6 +125,9 @@ public static function processPluginSelect(&$element, FormStateInterface $form_s /** @var \Drupal\Core\Executable\ExecutableInterface $plugin */ $plugin = $plugin_manager->createInstance($target_plugin_id, $values['target_plugin_configuration']); if ($plugin instanceof PluginFormInterface) { + $element['target_plugin_configuration'] = [ + '#tree' => TRUE, + ]; $element['target_plugin_configuration'] = $plugin->buildConfigurationForm($element['target_plugin_configuration'], $form_state); } } @@ -137,14 +139,9 @@ public static function processPluginSelect(&$element, FormStateInterface $form_s * Ajax callback. */ public static function pluginFormAjax(&$form, FormStateInterface $form_state, Request $request) { - $triggering_element = $form_state->getTriggeringElement(); - while (!isset($triggering_element['#ajax_array_parents'])) { - array_pop($triggering_element['#array_parents']); - $triggering_element = NestedArray::getValue($form, $triggering_element['#array_parents']); - } - $element = NestedArray::getValue($form, $triggering_element['#ajax_array_parents']); - - return $element; + $target_plugin_id_element = $form_state->getTriggeringElement(); + $plugin_select_element = NestedArray::getValue($form, array_slice($target_plugin_id_element['#array_parents'], 0, -1)); + return $plugin_select_element; } /** @@ -180,8 +177,9 @@ public static function validatePlugin(array &$element, FormStateInterface $form_ // If a plugin was selected, create an instance and pass the configuration // values to its configuration form validation method. if ($target_plugin_id != '_none') { + /** @var \Drupal\Component\Plugin\PluginManagerInterface $plugin_manager */ $plugin_manager = \Drupal::service('plugin.manager.' . $element['#plugin_type']); - $plugin = $plugin_manager->createInstance($target_plugin_id, $values['target_plugin_configuration']); + $plugin = $plugin_manager->createInstance($target_plugin_id); if ($plugin instanceof PluginFormInterface) { $plugin->validateConfigurationForm($element['target_plugin_configuration'], $form_state); } @@ -202,8 +200,9 @@ public static function submitPlugin(array &$element, FormStateInterface $form_st // If a plugin was selected, create an instance and pass the configuration // values to its configuration form submission method. if ($target_plugin_id != '_none') { + /** @var \Drupal\Component\Plugin\PluginManagerInterface $plugin_manager */ $plugin_manager = \Drupal::service('plugin.manager.' . $element['#plugin_type']); - $plugin = $plugin_manager->createInstance($target_plugin_id, $values['target_plugin_configuration']); + $plugin = $plugin_manager->createInstance($target_plugin_id); if ($plugin instanceof PluginFormInterface) { /** @var \Drupal\Component\Plugin\ConfigurablePluginInterface $plugin */ $plugin->submitConfigurationForm($element['target_plugin_configuration'], $form_state); From 7e3d8ba6017fe2eff951c2fdea1aa58d793166c5 Mon Sep 17 00:00:00 2001 From: Matt Glaman Date: Thu, 23 Mar 2017 08:19:48 -0500 Subject: [PATCH 04/10] Use default values to construct plugin each time --- src/Element/PluginSelect.php | 10 +++++----- src/Plugin/Field/FieldType/PluginItem.php | 6 +++++- src/Plugin/Field/FieldWidget/PluginRadiosWidget.php | 2 +- src/Plugin/Field/FieldWidget/PluginSelectWidget.php | 3 ++- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/Element/PluginSelect.php b/src/Element/PluginSelect.php index 2fa1eb9..fe21714 100644 --- a/src/Element/PluginSelect.php +++ b/src/Element/PluginSelect.php @@ -65,15 +65,15 @@ public static function processPluginSelect(&$element, FormStateInterface $form_s throw new \InvalidArgumentException('The commerce_plugin_select element only supports select/radios.'); } - $values = $element['#value']; + $values = $element['#default_value']; $target_plugin_id = $values['target_plugin_id']; $ajax_wrapper_id = Html::getUniqueId('ajax-wrapper'); // Prefix and suffix used for Ajax replacement. $element['#prefix'] = '
'; $element['#suffix'] = '
'; - $element['#tree'] = TRUE; + $element['target_plugin_id'] = [ '#type' => $element['#plugin_element_type'], '#title' => $element['#title'], @@ -121,7 +121,7 @@ public static function processPluginSelect(&$element, FormStateInterface $form_s $element['target_plugin_configuration'] = [ '#type' => 'container', ]; - if ($target_plugin_id != '_none') { + if (!empty($target_plugin_id) && $target_plugin_id != '_none') { /** @var \Drupal\Core\Executable\ExecutableInterface $plugin */ $plugin = $plugin_manager->createInstance($target_plugin_id, $values['target_plugin_configuration']); if ($plugin instanceof PluginFormInterface) { @@ -179,7 +179,7 @@ public static function validatePlugin(array &$element, FormStateInterface $form_ if ($target_plugin_id != '_none') { /** @var \Drupal\Component\Plugin\PluginManagerInterface $plugin_manager */ $plugin_manager = \Drupal::service('plugin.manager.' . $element['#plugin_type']); - $plugin = $plugin_manager->createInstance($target_plugin_id); + $plugin = $plugin_manager->createInstance($target_plugin_id, $element['#default_value']['target_plugin_configuration']); if ($plugin instanceof PluginFormInterface) { $plugin->validateConfigurationForm($element['target_plugin_configuration'], $form_state); } @@ -202,7 +202,7 @@ public static function submitPlugin(array &$element, FormStateInterface $form_st if ($target_plugin_id != '_none') { /** @var \Drupal\Component\Plugin\PluginManagerInterface $plugin_manager */ $plugin_manager = \Drupal::service('plugin.manager.' . $element['#plugin_type']); - $plugin = $plugin_manager->createInstance($target_plugin_id); + $plugin = $plugin_manager->createInstance($target_plugin_id, $element['#default_value']['target_plugin_configuration']); if ($plugin instanceof PluginFormInterface) { /** @var \Drupal\Component\Plugin\ConfigurablePluginInterface $plugin */ $plugin->submitConfigurationForm($element['target_plugin_configuration'], $form_state); diff --git a/src/Plugin/Field/FieldType/PluginItem.php b/src/Plugin/Field/FieldType/PluginItem.php index a3a58b4..56d45d6 100644 --- a/src/Plugin/Field/FieldType/PluginItem.php +++ b/src/Plugin/Field/FieldType/PluginItem.php @@ -136,7 +136,11 @@ public function getTargetInstance(array $contexts = []) { * {@inheritdoc} */ public function setValue($values, $notify = TRUE) { - if (isset($values['target_plugin_configuration'])) { + if (isset($values)) { + $values += [ + 'target_plugin_configuration' => [], + ]; + // Single serialized values on shared tables for base fields are not // always unserialized. https://www.drupal.org/node/2788637 if (is_string($values['target_plugin_configuration'])) { diff --git a/src/Plugin/Field/FieldWidget/PluginRadiosWidget.php b/src/Plugin/Field/FieldWidget/PluginRadiosWidget.php index d0ffa6c..0c54158 100644 --- a/src/Plugin/Field/FieldWidget/PluginRadiosWidget.php +++ b/src/Plugin/Field/FieldWidget/PluginRadiosWidget.php @@ -31,7 +31,7 @@ public function formElement(FieldItemListInterface $items, $delta, array $elemen '#categories' => $this->fieldDefinition->getSetting('categories'), '#default_value' => [ 'target_plugin_id' => $items[$delta]->target_plugin_id, - 'target_plugin_configuration' => $items[$delta]->target_plugin_configuration, + 'target_plugin_configuration' => $items[$delta]->target_plugin_configuration ?: [], ], '#required' => $this->fieldDefinition->isRequired(), '#title' => $this->fieldDefinition->getLabel(), diff --git a/src/Plugin/Field/FieldWidget/PluginSelectWidget.php b/src/Plugin/Field/FieldWidget/PluginSelectWidget.php index b2f9a21..8739d73 100644 --- a/src/Plugin/Field/FieldWidget/PluginSelectWidget.php +++ b/src/Plugin/Field/FieldWidget/PluginSelectWidget.php @@ -24,13 +24,14 @@ class PluginSelectWidget extends WidgetBase { */ public function formElement(FieldItemListInterface $items, $delta, array $element, array &$form, FormStateInterface $form_state) { list($field_type, $derivative) = explode(':', $this->fieldDefinition->getType()); + return [ '#type' => 'commerce_plugin_select', '#plugin_type' => $derivative, '#categories' => $this->fieldDefinition->getSetting('categories'), '#default_value' => [ 'target_plugin_id' => $items[$delta]->target_plugin_id, - 'target_plugin_configuration' => $items[$delta]->target_plugin_configuration, + 'target_plugin_configuration' => $items[$delta]->target_plugin_configuration ?: [], ], '#required' => $this->fieldDefinition->isRequired(), '#title' => $this->fieldDefinition->getLabel(), From 6c554a84ee8fe55793c4c3c9969164362ef9bcf0 Mon Sep 17 00:00:00 2001 From: Matt Glaman Date: Thu, 23 Mar 2017 08:21:22 -0500 Subject: [PATCH 05/10] Remove @todo we won't do --- .../src/Plugin/Commerce/PromotionCondition/PromotionConditionBase.php | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/promotion/src/Plugin/Commerce/PromotionCondition/PromotionConditionBase.php b/modules/promotion/src/Plugin/Commerce/PromotionCondition/PromotionConditionBase.php index e7480bd..feb44d7 100644 --- a/modules/promotion/src/Plugin/Commerce/PromotionCondition/PromotionConditionBase.php +++ b/modules/promotion/src/Plugin/Commerce/PromotionCondition/PromotionConditionBase.php @@ -37,7 +37,6 @@ public function execute() { * {@inheritdoc} */ public function submitConfigurationForm(array &$form, FormStateInterface $form_state) { - // @todo Update plugin_select to pass subform state automatically. return parent::submitConfigurationForm($form, SubformState::createForSubform($form, $form_state->getCompleteForm(), $form_state)); } From 9bdde3d8f8c73c320c823c252153575e5aa5f451 Mon Sep 17 00:00:00 2001 From: Matt Glaman Date: Thu, 23 Mar 2017 09:27:05 -0500 Subject: [PATCH 06/10] Update our test for plugin select Test condition plugin didn't submit subform state (update made to offers) test negation is saved to the condition boost coverage that editing provides data persisted in ui from save --- src/Element/PluginSelect.php | 6 ++-- .../src/Plugin/Condition/UserRole.php | 3 +- tests/src/FunctionalJavascript/PluginItemTest.php | 41 ++++++++++++++++++---- 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/src/Element/PluginSelect.php b/src/Element/PluginSelect.php index fe21714..862a61f 100644 --- a/src/Element/PluginSelect.php +++ b/src/Element/PluginSelect.php @@ -65,7 +65,7 @@ public static function processPluginSelect(&$element, FormStateInterface $form_s throw new \InvalidArgumentException('The commerce_plugin_select element only supports select/radios.'); } - $values = $element['#default_value']; + $values = $element['#value']; $target_plugin_id = $values['target_plugin_id']; $ajax_wrapper_id = Html::getUniqueId('ajax-wrapper'); @@ -79,7 +79,7 @@ public static function processPluginSelect(&$element, FormStateInterface $form_s '#title' => $element['#title'], '#multiple' => FALSE, '#ajax' => [ - 'callback' => [get_called_class(), 'pluginFormAjax'], + 'callback' => [get_called_class(), 'ajaxRefresh'], 'wrapper' => $ajax_wrapper_id, ], '#default_value' => $target_plugin_id, @@ -138,7 +138,7 @@ public static function processPluginSelect(&$element, FormStateInterface $form_s /** * Ajax callback. */ - public static function pluginFormAjax(&$form, FormStateInterface $form_state, Request $request) { + public static function ajaxRefresh(&$form, FormStateInterface $form_state, Request $request) { $target_plugin_id_element = $form_state->getTriggeringElement(); $plugin_select_element = NestedArray::getValue($form, array_slice($target_plugin_id_element['#array_parents'], 0, -1)); return $plugin_select_element; diff --git a/tests/modules/commerce_test/src/Plugin/Condition/UserRole.php b/tests/modules/commerce_test/src/Plugin/Condition/UserRole.php index 8c6bf81..bfacac5 100644 --- a/tests/modules/commerce_test/src/Plugin/Condition/UserRole.php +++ b/tests/modules/commerce_test/src/Plugin/Condition/UserRole.php @@ -4,6 +4,7 @@ use Drupal\Core\Condition\ConditionPluginBase; use Drupal\Core\Form\FormStateInterface; +use Drupal\Core\Form\SubformState; /** * Provides a 'User Role' condition. @@ -49,7 +50,7 @@ public function submitConfigurationForm(array &$form, FormStateInterface $form_s // Core plugins do not respect the #parents attribute passed in form. $values = $form_state->getValue($form['#parents']); $this->configuration['roles'] = array_filter($values['roles']); - parent::submitConfigurationForm($form, $form_state); + return parent::submitConfigurationForm($form, SubformState::createForSubform($form, $form_state->getCompleteForm(), $form_state)); } /** diff --git a/tests/src/FunctionalJavascript/PluginItemTest.php b/tests/src/FunctionalJavascript/PluginItemTest.php index 4f3fb2f..df26efc 100644 --- a/tests/src/FunctionalJavascript/PluginItemTest.php +++ b/tests/src/FunctionalJavascript/PluginItemTest.php @@ -19,6 +19,13 @@ class PluginItemTest extends CommerceBrowserTestBase { use JavascriptTestTrait; /** + * The entity_test storage. + * + * @var \Drupal\Core\Entity\EntityStorageInterface + */ + protected $entityTestStorage; + + /** * Modules to enable. * * @var array @@ -62,6 +69,8 @@ protected function setUp() { $display->setComponent('test_conditions', [ 'type' => 'commerce_plugin_select', ])->save(); + + $this->entityTestStorage = $this->container->get('entity_type.manager')->getStorage('entity_test'); } /** @@ -74,30 +83,50 @@ public function testPluginSelectField() { ]); $this->drupalGet($entity->toUrl('edit-form')); $this->getSession()->getPage()->fillField('test_conditions[0][target_plugin_id]', 'commerce_test_user_role'); - $this->getSession()->wait(4000); + $this->waitForAjaxToFinish(); + $this->submitForm([ 'test_conditions[0][target_plugin_configuration][roles][test_role]' => 1, 'test_conditions[0][target_plugin_configuration][negate]' => 0, ], 'Save'); $this->assertSession()->pageTextContains('entity_test 1 has been updated.'); - \Drupal::entityTypeManager()->getStorage('entity_test')->resetCache(); - $entity = \Drupal::entityTypeManager()->getStorage('entity_test')->load($entity->id()); + $this->entityTestStorage->resetCache([$entity->id()]); + $entity = $this->entityTestStorage->load($entity->id()); $test_user1 = $this->createEntity('user', [ 'name' => 'Test user 1', 'status' => TRUE, 'roles' => ['test_role'], ]); + $user1_context = new Context(new ContextDefinition('entity:user'), $test_user1); /** @var \Drupal\commerce\Plugin\Field\FieldType\PluginItem $condition_field */ $condition_field = $entity->test_conditions->first(); $this->assertNotNull($condition_field); - // Executes and returns TRUE that user1 has role. - $user1_context = new Context(new ContextDefinition('entity:user'), $test_user1); + /** @var \Drupal\Core\Condition\ConditionInterface $plugin */ + $plugin = $condition_field->getTargetInstance(['user' => $user1_context]); + $this->assertFalse($plugin->isNegated()); + $this->assertTrue($plugin->execute()); + + // Set the condition to be negated. + $this->drupalGet($entity->toUrl('edit-form')); + $this->submitForm([ + 'test_conditions[0][target_plugin_configuration][roles][test_role]' => 1, + 'test_conditions[0][target_plugin_configuration][negate]' => 1, + ], 'Save'); + $this->assertSession()->pageTextContains('entity_test 1 has been updated.'); + + $this->entityTestStorage->resetCache([$entity->id()]); + $entity = $this->entityTestStorage->load($entity->id()); + /** @var \Drupal\commerce\Plugin\Field\FieldType\PluginItem $condition_field */ + $condition_field = $entity->test_conditions->first(); + /** @var \Drupal\Core\Condition\ConditionInterface $plugin */ + $plugin = $condition_field->getTargetInstance(['user' => $user1_context]); - $this->assertNotEmpty($condition_field->getTargetInstance(['user' => $user1_context])->execute()); + $this->assertTrue($plugin->isNegated()); + $this->assertFalse($plugin->execute()); } } From 53cecd12bb08a4064eb4150f7641cdb32b3130fe Mon Sep 17 00:00:00 2001 From: Matt Glaman Date: Thu, 23 Mar 2017 12:25:02 -0500 Subject: [PATCH 07/10] Clear plugin config values whenchanging plugin --- .../src/FunctionalJavascript/PromotionTest.php | 24 +++++++++++--------- src/Element/PluginSelect.php | 26 ++++++++++++++++++++++ 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/modules/promotion/tests/src/FunctionalJavascript/PromotionTest.php b/modules/promotion/tests/src/FunctionalJavascript/PromotionTest.php index 5411fbc..4f173be 100644 --- a/modules/promotion/tests/src/FunctionalJavascript/PromotionTest.php +++ b/modules/promotion/tests/src/FunctionalJavascript/PromotionTest.php @@ -41,23 +41,25 @@ public function testCreatePromotion() { // Check the integrity of the form. $this->assertSession()->fieldExists('name[0][value]'); + $name = $this->randomMachineName(8); + $this->getSession()->getPage()->fillField('name[0][value]', $name); $this->getSession()->getPage()->fillField('offer[0][target_plugin_id]', 'commerce_promotion_product_percentage_off'); - $this->getSession()->wait(2000, "jQuery('.ajax-progress').length === 0"); + $this->waitForAjaxToFinish(); + $this->getSession()->getPage()->fillField('offer[0][target_plugin_configuration][amount]', '10.0'); - $name = $this->randomMachineName(8); - $edit = [ - 'name[0][value]' => $name, - 'offer[0][target_plugin_configuration][amount]' => '10.0', - ]; + // Change, assert any values reset. + $this->getSession()->getPage()->fillField('offer[0][target_plugin_id]', 'commerce_promotion_product_percentage_off'); + $this->waitForAjaxToFinish(); + $this->assertSession()->fieldValueNotEquals('offer[0][target_plugin_configuration][amount]', '10.0'); + $this->getSession()->getPage()->fillField('offer[0][target_plugin_configuration][amount]', '10.0'); $this->getSession()->getPage()->fillField('conditions[0][target_plugin_id]', 'commerce_promotion_order_total_price'); - $this->getSession()->wait(2000, "jQuery('.ajax-progress').length === 0"); + $this->waitForAjaxToFinish(); + $this->getSession()->getPage()->fillField('conditions[0][target_plugin_configuration][amount][number]', '50.00'); + $this->getSession()->getPage()->checkField('conditions[0][target_plugin_configuration][negate]'); - $edit['conditions[0][target_plugin_configuration][amount][number]'] = '50.00'; - $edit['conditions[0][target_plugin_configuration][negate]'] = 1; - - $this->submitForm($edit, t('Save')); + $this->submitForm([], t('Save')); $this->assertSession()->pageTextContains("Saved the $name promotion."); $promotion_count = $this->getSession()->getPage()->find('xpath', '//table/tbody/tr/td[text()="' . $name . '"]'); $this->assertEquals(count($promotion_count), 1, 'promotions exists in the table.'); diff --git a/src/Element/PluginSelect.php b/src/Element/PluginSelect.php index 862a61f..5b5be92 100644 --- a/src/Element/PluginSelect.php +++ b/src/Element/PluginSelect.php @@ -43,6 +43,9 @@ public function getInfo() { [$class, 'processPluginSelect'], [$class, 'processAjaxForm'], ], + '#after_build' => [ + [$class, 'clearValues'], + ], '#element_validate' => [ [$class, 'validateElementSubmit'], [$class, 'validatePlugin'], @@ -212,4 +215,27 @@ public static function submitPlugin(array &$element, FormStateInterface $form_st } } + /** + * Clears the plugin-specific form values when the target plugin changes. + * + * Implemented as an #after_build callback because #after_build runs before + * validation, allowing the values to be cleared early enough to prevent the + * "Illegal choice" error. + */ + public static function clearValues(array $element, FormStateInterface $form_state) { + $triggering_element = $form_state->getTriggeringElement(); + if (!$triggering_element) { + return $element; + } + + $triggering_element_name = end($triggering_element['#parents']); + if ($triggering_element_name == 'target_plugin_id') { + $input = &$form_state->getUserInput(); + $parents = array_merge($element['#parents'], ['target_plugin_configuration']); + NestedArray::setValue($input, $parents, ''); + $element['target_plugin_configuration']['#value'] = ''; + } + + return $element; + } } From 27fc2214d08c2419052e7364584c721fb9661065 Mon Sep 17 00:00:00 2001 From: Matt Glaman Date: Thu, 23 Mar 2017 12:46:07 -0500 Subject: [PATCH 08/10] phpcs --- src/Element/PluginSelect.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Element/PluginSelect.php b/src/Element/PluginSelect.php index 5b5be92..94765c4 100644 --- a/src/Element/PluginSelect.php +++ b/src/Element/PluginSelect.php @@ -238,4 +238,5 @@ public static function clearValues(array $element, FormStateInterface $form_stat return $element; } + } From d9683a12f97f1a7d663a782dc2888f5077f393df Mon Sep 17 00:00:00 2001 From: Matt Glaman Date: Thu, 23 Mar 2017 14:03:45 -0500 Subject: [PATCH 09/10] Add test coverage for radio plugin_select --- tests/src/FunctionalJavascript/PluginItemTest.php | 69 +++++++++++++++++++++-- 1 file changed, 65 insertions(+), 4 deletions(-) diff --git a/tests/src/FunctionalJavascript/PluginItemTest.php b/tests/src/FunctionalJavascript/PluginItemTest.php index df26efc..9de2f9f 100644 --- a/tests/src/FunctionalJavascript/PluginItemTest.php +++ b/tests/src/FunctionalJavascript/PluginItemTest.php @@ -65,24 +65,85 @@ protected function setUp() { ]); $field->save(); + $this->entityTestStorage = $this->container->get('entity_type.manager')->getStorage('entity_test'); + } + + /** + * Tests the plugin_select element, as select widget. + */ + public function testPluginSelectSelectField() { $display = commerce_get_entity_display('entity_test', 'entity_test', 'form'); $display->setComponent('test_conditions', [ 'type' => 'commerce_plugin_select', ])->save(); - $this->entityTestStorage = $this->container->get('entity_type.manager')->getStorage('entity_test'); + // Add the field. + $entity = $this->createEntity('entity_test', [ + 'name' => 'Test', + ]); + $this->drupalGet($entity->toUrl('edit-form')); + $this->getSession()->getPage()->fillField('test_conditions[0][target_plugin_id]', 'commerce_test_user_role'); + $this->waitForAjaxToFinish(); + + $this->submitForm([ + 'test_conditions[0][target_plugin_configuration][roles][test_role]' => 1, + 'test_conditions[0][target_plugin_configuration][negate]' => 0, + ], 'Save'); + $this->assertSession()->pageTextContains('entity_test 1 has been updated.'); + + $this->entityTestStorage->resetCache([$entity->id()]); + $entity = $this->entityTestStorage->load($entity->id()); + + $test_user1 = $this->createEntity('user', [ + 'name' => 'Test user 1', + 'status' => TRUE, + 'roles' => ['test_role'], + ]); + $user1_context = new Context(new ContextDefinition('entity:user'), $test_user1); + + /** @var \Drupal\commerce\Plugin\Field\FieldType\PluginItem $condition_field */ + $condition_field = $entity->test_conditions->first(); + $this->assertNotNull($condition_field); + + /** @var \Drupal\Core\Condition\ConditionInterface $plugin */ + $plugin = $condition_field->getTargetInstance(['user' => $user1_context]); + $this->assertFalse($plugin->isNegated()); + $this->assertTrue($plugin->execute()); + + // Set the condition to be negated. + $this->drupalGet($entity->toUrl('edit-form')); + $this->submitForm([ + 'test_conditions[0][target_plugin_configuration][roles][test_role]' => 1, + 'test_conditions[0][target_plugin_configuration][negate]' => 1, + ], 'Save'); + $this->assertSession()->pageTextContains('entity_test 1 has been updated.'); + + $this->entityTestStorage->resetCache([$entity->id()]); + $entity = $this->entityTestStorage->load($entity->id()); + /** @var \Drupal\commerce\Plugin\Field\FieldType\PluginItem $condition_field */ + $condition_field = $entity->test_conditions->first(); + /** @var \Drupal\Core\Condition\ConditionInterface $plugin */ + $plugin = $condition_field->getTargetInstance(['user' => $user1_context]); + + $this->assertTrue($plugin->isNegated()); + $this->assertFalse($plugin->execute()); } /** - * Tests the plugin select field. + * Tests the plugin_select element, as radio widget. */ - public function testPluginSelectField() { + public function testPluginSelectRadiosField() { + $display = commerce_get_entity_display('entity_test', 'entity_test', 'form'); + $display->setComponent('test_conditions', [ + 'type' => 'commerce_plugin_radios', + ])->save(); + // Add the field. $entity = $this->createEntity('entity_test', [ 'name' => 'Test', ]); $this->drupalGet($entity->toUrl('edit-form')); - $this->getSession()->getPage()->fillField('test_conditions[0][target_plugin_id]', 'commerce_test_user_role'); + $this->getSession()->getPage()->checkField('User has role'); $this->waitForAjaxToFinish(); $this->submitForm([ From 1f82036ad134a7426caac32cd66f1be8b750c777 Mon Sep 17 00:00:00 2001 From: Matt Glaman Date: Thu, 23 Mar 2017 15:24:17 -0500 Subject: [PATCH 10/10] And fix it all! --- src/Element/PluginSelect.php | 10 +++++++--- tests/src/FunctionalJavascript/PluginItemTest.php | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Element/PluginSelect.php b/src/Element/PluginSelect.php index 94765c4..f96bf12 100644 --- a/src/Element/PluginSelect.php +++ b/src/Element/PluginSelect.php @@ -104,8 +104,8 @@ public static function processPluginSelect(&$element, FormStateInterface $form_s continue; } - // Group categorized plugins. - if (isset($definition['category'])) { + // Group categorized plugins, and if using a select element. + if (isset($definition['category']) && $element['#plugin_element_type'] == 'select') { $element['target_plugin_id']['#options'][(string) $definition['category']][$definition['id']] = $definition['label']; } else { @@ -143,7 +143,11 @@ public static function processPluginSelect(&$element, FormStateInterface $form_s */ public static function ajaxRefresh(&$form, FormStateInterface $form_state, Request $request) { $target_plugin_id_element = $form_state->getTriggeringElement(); - $plugin_select_element = NestedArray::getValue($form, array_slice($target_plugin_id_element['#array_parents'], 0, -1)); + + // Radios are an extra parent deep compared to the select. + $slice_length = ($target_plugin_id_element['#type'] == 'radio') ? -2 : -1; + + $plugin_select_element = NestedArray::getValue($form, array_slice($target_plugin_id_element['#array_parents'], 0, $slice_length)); return $plugin_select_element; } diff --git a/tests/src/FunctionalJavascript/PluginItemTest.php b/tests/src/FunctionalJavascript/PluginItemTest.php index 9de2f9f..5299574 100644 --- a/tests/src/FunctionalJavascript/PluginItemTest.php +++ b/tests/src/FunctionalJavascript/PluginItemTest.php @@ -143,7 +143,7 @@ public function testPluginSelectRadiosField() { 'name' => 'Test', ]); $this->drupalGet($entity->toUrl('edit-form')); - $this->getSession()->getPage()->checkField('User has role'); + $this->getSession()->getPage()->fillField('test_conditions[0][target_plugin_id]', 'commerce_test_user_role'); $this->waitForAjaxToFinish(); $this->submitForm([