From 36c37d269fe88bf65d1c1e9317e8241ab00f28f7 Mon Sep 17 00:00:00 2001 From: mglaman Date: Thu, 16 Mar 2017 12:35:41 -0500 Subject: [PATCH 1/4] Issue #2861002 by mglaman: Coupon redemption form needs severe improvements and bug fixes --- .../CheckoutFlow/CheckoutFlowBase.php | 11 +- .../src/Element/CouponRedemptionForm.php | 135 +++++++++++++----- .../commerce_promotion_test.module | 4 +- .../src/Functional/CouponRedemptionTest.php | 10 +- 4 files changed, 121 insertions(+), 39 deletions(-) diff --git a/modules/checkout/src/Plugin/Commerce/CheckoutFlow/CheckoutFlowBase.php b/modules/checkout/src/Plugin/Commerce/CheckoutFlow/CheckoutFlowBase.php index 98cfca23c7..59c6e4d948 100644 --- a/modules/checkout/src/Plugin/Commerce/CheckoutFlow/CheckoutFlowBase.php +++ b/modules/checkout/src/Plugin/Commerce/CheckoutFlow/CheckoutFlowBase.php @@ -246,11 +246,18 @@ public function getFormId() { * {@inheritdoc} */ public function buildForm(array $form, FormStateInterface $form_state, $step_id = NULL) { - // The $step_id argument is optional only because PHP disallows adding required - // arguments to an existing interface's method. + // The $step_id argument is optional only because PHP disallows adding + // required arguments to an existing interface's method. if (empty($step_id)) { throw new \InvalidArgumentException('The $step_id cannot be empty.'); } + if ($form_state->isRebuilding()) { + // Ensure a fresh order whenever form is rebuilt, such as during an + // operation which uses AJAX and may have altered the order. + $order_storage = $this->entityTypeManager->getStorage('commerce_order'); + $this->order = $order_storage->load($this->order->id()); + } + $steps = $this->getVisibleSteps(); $form['#tree'] = TRUE; $form['#step_id'] = $step_id; diff --git a/modules/promotion/src/Element/CouponRedemptionForm.php b/modules/promotion/src/Element/CouponRedemptionForm.php index 42b7683818..b5c2e6d6a0 100644 --- a/modules/promotion/src/Element/CouponRedemptionForm.php +++ b/modules/promotion/src/Element/CouponRedemptionForm.php @@ -6,6 +6,7 @@ use Drupal\commerce_order\Entity\OrderInterface; use Drupal\commerce_promotion\Entity\CouponInterface; use Drupal\commerce_promotion\Entity\PromotionInterface; +use Drupal\Component\Utility\NestedArray; use Drupal\Core\Form\FormStateInterface; /** @@ -31,10 +32,14 @@ public function getInfo() { $class = get_class($this); return [ // The order to which the coupon will be applied to. - '#order' => NULL, + '#order_id' => NULL, '#title' => t('Coupon code'), '#description' => t('Enter your coupon code here.'), '#submit_title' => t('Apply coupon'), + '#submit_message' => t('Coupon applied'), + '#remove_title' => t('Remove coupon'), + // The coupon ID. + '#default_value' => NULL, '#process' => [ [$class, 'attachElementSubmit'], [$class, 'processForm'], @@ -67,11 +72,14 @@ public function getInfo() { * The processed form element. */ public static function processForm(array $element, FormStateInterface $form_state, array &$complete_form) { - if (empty($element['#order'])) { - throw new \InvalidArgumentException('The commerce_coupon_redemption_form element requires the #order property.'); + if (empty($element['#order_id'])) { + throw new \InvalidArgumentException('The commerce_coupon_redemption_form element requires the #order_id property.'); } - if (!$element['#order'] instanceof OrderInterface) { - throw new \InvalidArgumentException('The commerce_coupon_redemption_form #order property must be an order entity.'); + + $order_storage = \Drupal::entityTypeManager()->getStorage('commerce_order'); + $order = $order_storage->load($element['#order_id']); + if (!$order instanceof OrderInterface) { + throw new \InvalidArgumentException('The commerce_coupon_redemption #order_id must be a valid order ID.'); } $id_prefix = implode('-', $element['#parents']); @@ -80,7 +88,8 @@ public static function processForm(array $element, FormStateInterface $form_stat // $wrapper_id = Html::getUniqueId($id_prefix . '-ajax-wrapper'); $wrapper_id = $id_prefix . '-ajax-wrapper'; - $form_state->set('order', $element['#order']); + $has_coupons = !$order->get('coupons')->isEmpty(); + $element = [ '#tree' => TRUE, '#prefix' => '
', @@ -92,21 +101,99 @@ public static function processForm(array $element, FormStateInterface $form_stat '#type' => 'textfield', '#title' => $element['#title'], '#description' => $element['#description'], + '#access' => !$has_coupons, ]; $element['apply'] = [ '#type' => 'submit', '#value' => $element['#submit_title'], '#name' => 'apply_coupon', '#limit_validation_errors' => [ - array_merge($element['#parents'], ['code']), + $element['#parents'], + ], + '#submit' => [ + [get_called_class(), 'addCoupon'], + ], + '#ajax' => [ + 'callback' => [get_called_class(), 'ajaxRefresh'], + 'wrapper' => $element['#wrapper_id'], + ], + '#access' => !$has_coupons, + ]; + + $element['remove'] = [ + '#type' => 'submit', + '#value' => $element['#remove_title'], + '#name' => 'remove_coupon', + '#ajax' => [ + 'callback' => [get_called_class(), 'ajaxRefresh'], + 'wrapper' => $element['#wrapper_id'], + ], + '#weight' => 50, + '#limit_validation_errors' => [ + $element['#parents'], + ], + '#submit' => [ + [get_called_class(), 'removeCoupon'], ], + '#access' => $has_coupons, ]; return $element; } /** - * Validates the coupon redemption form. + * Ajax callback. + */ + public static function ajaxRefresh(array $form, FormStateInterface $form_state) { + $parents = $form_state->getTriggeringElement()['#parents']; + array_pop($parents); + return NestedArray::getValue($form, $parents); + } + + /** + * Apply coupon submit callback. + */ + public static function addCoupon(array $form, FormStateInterface $form_state) { + $triggering_element = $form_state->getTriggeringElement(); + $parents = $triggering_element['#parents']; + array_pop($parents); + $element = NestedArray::getValue($form, $parents); + + $entity_type_manager = \Drupal::entityTypeManager(); + $order_storage = $entity_type_manager->getStorage('commerce_order'); + /** @var \Drupal\commerce_order\Entity\OrderInterface $order */ + $order = $order_storage->load($element['#order_id']); + + $coupon = $form_state->getValue($parents); + $order->get('coupons')->appendItem($coupon); + drupal_set_message($element['#submit_message']); + + $order->save(); + $form_state->setRebuild(); + } + + /** + * Remove coupon submit callback. + */ + public static function removeCoupon(array $form, FormStateInterface $form_state) { + $triggering_element = $form_state->getTriggeringElement(); + $parents = $triggering_element['#parents']; + array_pop($parents); + $element = NestedArray::getValue($form, $parents); + + $entity_type_manager = \Drupal::entityTypeManager(); + $order_storage = $entity_type_manager->getStorage('commerce_order'); + /** @var \Drupal\commerce_order\Entity\OrderInterface $order */ + $order = $order_storage->load($element['#order_id']); + + $order->get('coupons')->setValue([]); + + $order->save(); + $form_state->setRebuild(); + } + + /** + * Validates the coupon redemption element. * * @param array $element * The form element. @@ -119,10 +206,11 @@ public static function validateForm(array &$element, FormStateInterface $form_st if (empty($coupon_code)) { return; } + $entity_type_manager = \Drupal::entityTypeManager(); $code_path = implode('][', $coupon_parents); + $order_storage = $entity_type_manager->getStorage('commerce_order'); /** @var \Drupal\commerce_order\Entity\OrderInterface $order */ - $order = $form_state->get('order'); - $entity_type_manager = \Drupal::entityTypeManager(); + $order = $order_storage->load($element['#order_id']); /** @var \Drupal\commerce_promotion\CouponStorageInterface $coupon_storage */ $coupon_storage = $entity_type_manager->getStorage('commerce_promotion_coupon'); @@ -155,10 +243,13 @@ public static function validateForm(array &$element, FormStateInterface $form_st $form_state->setErrorByName($code_path, t('Coupon is invalid')); return; } + else { + $form_state->setValueForElement($element, $coupon); + } } /** - * Submits the coupon redemption form. + * Submits the coupon redemption element. * * @param array $element * The form element. @@ -166,27 +257,7 @@ public static function validateForm(array &$element, FormStateInterface $form_st * The current state of the form. */ public static function submitForm(array &$element, FormStateInterface $form_state) { - $coupon_parents = array_merge($element['#parents'], ['code']); - $coupon_code = $form_state->getValue($coupon_parents); - /** @var \Drupal\commerce_order\Entity\OrderInterface $order */ - $order = $form_state->get('order'); - $entity_type_manager = \Drupal::entityTypeManager(); - $order_type_storage = $entity_type_manager->getStorage('commerce_order_type'); - /** @var \Drupal\commerce_promotion\PromotionStorageInterface $promotion_storage */ - $promotion_storage = $entity_type_manager->getStorage('commerce_promotion'); - /** @var \Drupal\commerce_promotion\CouponStorageInterface $coupon_storage */ - $coupon_storage = $entity_type_manager->getStorage('commerce_promotion_coupon'); - - $coupon = $coupon_storage->loadByCode($coupon_code); - /** @var \Drupal\commerce_order\Entity\OrderTypeInterface $order_type */ - $order_type = $order_type_storage->load($order->bundle()); - $promotion = $promotion_storage->loadByCoupon($order_type, $order->getStore(), $coupon); - - if (self::couponApplies($order, $promotion, $coupon)) { - $order->get('coupons')->appendItem($coupon); - $order->save(); - drupal_set_message(t('Coupon applied')); - } + // The coupon was set as element value when validated. } /** diff --git a/modules/promotion/tests/modules/commerce_promotion_test/commerce_promotion_test.module b/modules/promotion/tests/modules/commerce_promotion_test/commerce_promotion_test.module index 531a063800..ea6554a826 100644 --- a/modules/promotion/tests/modules/commerce_promotion_test/commerce_promotion_test.module +++ b/modules/promotion/tests/modules/commerce_promotion_test/commerce_promotion_test.module @@ -13,13 +13,13 @@ use Drupal\Core\Form\FormStateInterface; function commerce_promotion_test_form_views_form_commerce_cart_form_default_alter(&$form, FormStateInterface $form_state, $form_id) { // We know that view forms are build on the base ID plus arguments. $order_id = substr($form_id, strlen('views_form_commerce_cart_form_default_')); - $order = \Drupal::entityTypeManager()->getStorage('commerce_order')->load($order_id); $form['coupons'] = [ '#type' => 'commerce_coupon_redemption_form', - '#order' => $order, + '#order_id' => $order_id, '#title' => t('Promotion code'), '#description' => t('Enter your promotion code to redeem a discount.'), '#submit_title' => t('Apply'), + '#remove_title' => t('Remove promotion'), ]; } diff --git a/modules/promotion/tests/src/Functional/CouponRedemptionTest.php b/modules/promotion/tests/src/Functional/CouponRedemptionTest.php index 6e95e3cf96..010e75ee3e 100644 --- a/modules/promotion/tests/src/Functional/CouponRedemptionTest.php +++ b/modules/promotion/tests/src/Functional/CouponRedemptionTest.php @@ -125,11 +125,15 @@ public function testCouponRedemption() { $this->getSession()->getPage()->fillField('coupons[code]', $existing_coupon->getCode()); $this->getSession()->getPage()->pressButton('Apply'); + $this->assertSession()->pageTextContains('Coupon applied'); - $this->getSession()->getPage()->fillField('coupons[code]', $existing_coupon->getCode()); - $this->getSession()->getPage()->pressButton('Apply'); - $this->assertSession()->pageTextContains('Coupon has already been redeemed'); + $this->assertSession()->fieldNotExists('coupons[code]'); + $this->assertSession()->buttonNotExists('Apply'); + $this->getSession()->getPage()->pressButton('Remove promotion'); + + $this->assertSession()->fieldExists('coupons[code]'); + $this->assertSession()->buttonExists('Apply'); } } From 2c06dd3750a4660f08584c2a2872554804853da7 Mon Sep 17 00:00:00 2001 From: Bojan Zivanovic Date: Fri, 17 Mar 2017 01:11:04 +0100 Subject: [PATCH 2/4] Update CheckoutFlowBase.php --- .../src/Plugin/Commerce/CheckoutFlow/CheckoutFlowBase.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/modules/checkout/src/Plugin/Commerce/CheckoutFlow/CheckoutFlowBase.php b/modules/checkout/src/Plugin/Commerce/CheckoutFlow/CheckoutFlowBase.php index 59c6e4d948..774d9c366b 100644 --- a/modules/checkout/src/Plugin/Commerce/CheckoutFlow/CheckoutFlowBase.php +++ b/modules/checkout/src/Plugin/Commerce/CheckoutFlow/CheckoutFlowBase.php @@ -252,8 +252,7 @@ public function buildForm(array $form, FormStateInterface $form_state, $step_id throw new \InvalidArgumentException('The $step_id cannot be empty.'); } if ($form_state->isRebuilding()) { - // Ensure a fresh order whenever form is rebuilt, such as during an - // operation which uses AJAX and may have altered the order. + // Ensure a fresh order, in case an ajax submit has modified it. $order_storage = $this->entityTypeManager->getStorage('commerce_order'); $this->order = $order_storage->load($this->order->id()); } From 34951a444174c13dca10ddd3a79b2f97c1296150 Mon Sep 17 00:00:00 2001 From: Bojan Zivanovic Date: Fri, 17 Mar 2017 01:26:17 +0100 Subject: [PATCH 3/4] Update CouponRedemptionForm.php --- .../src/Element/CouponRedemptionForm.php | 60 ++++++------------- 1 file changed, 19 insertions(+), 41 deletions(-) diff --git a/modules/promotion/src/Element/CouponRedemptionForm.php b/modules/promotion/src/Element/CouponRedemptionForm.php index b5c2e6d6a0..9013df1a29 100644 --- a/modules/promotion/src/Element/CouponRedemptionForm.php +++ b/modules/promotion/src/Element/CouponRedemptionForm.php @@ -10,18 +10,23 @@ use Drupal\Core\Form\FormStateInterface; /** - * Provides a form element for embedding the coupon redemption form. + * Provides a form element for redeeming a coupon. * * Usage example: * @code - * $form['coupons'] = [ + * $form['coupon'] = [ * '#type' => 'commerce_coupon_redemption_form', - * // The order to which the coupon will be applied to. - * '#order' => $order, + * '#title' => t('Coupon code'), + * '#default_value' => $coupon_id, + * '#order_id' => $order_id, * ]; * @endcode + * The element value ($form_state->getValue('coupon')) will be the + * coupon ID. Note that the order is not saved if the element was + * submitted as a result of the main form being submitted. It is the + * responsibility of the caller to update the order in that case. * - * @RenderElement("commerce_coupon_redemption_form") + * @FormElement("commerce_coupon_redemption_form") */ class CouponRedemptionForm extends CommerceElementBase { @@ -31,8 +36,6 @@ class CouponRedemptionForm extends CommerceElementBase { public function getInfo() { $class = get_class($this); return [ - // The order to which the coupon will be applied to. - '#order_id' => NULL, '#title' => t('Coupon code'), '#description' => t('Enter your coupon code here.'), '#submit_title' => t('Apply coupon'), @@ -40,17 +43,13 @@ public function getInfo() { '#remove_title' => t('Remove coupon'), // The coupon ID. '#default_value' => NULL, + '#order_id' => NULL, '#process' => [ - [$class, 'attachElementSubmit'], [$class, 'processForm'], ], '#element_validate' => [ - [$class, 'validateElementSubmit'], [$class, 'validateForm'], ], - '#commerce_element_submit' => [ - [$class, 'submitForm'], - ], '#theme_wrappers' => ['container'], ]; } @@ -66,7 +65,7 @@ public function getInfo() { * The complete form structure. * * @throws \InvalidArgumentException - * Thrown when the #order property is empty or invalid entity. + * Thrown when the #order_id property is empty or invalid. * * @return array * The processed form element. @@ -75,21 +74,19 @@ public static function processForm(array $element, FormStateInterface $form_stat if (empty($element['#order_id'])) { throw new \InvalidArgumentException('The commerce_coupon_redemption_form element requires the #order_id property.'); } - $order_storage = \Drupal::entityTypeManager()->getStorage('commerce_order'); $order = $order_storage->load($element['#order_id']); if (!$order instanceof OrderInterface) { throw new \InvalidArgumentException('The commerce_coupon_redemption #order_id must be a valid order ID.'); } + $has_coupons = !$order->get('coupons')->isEmpty(); $id_prefix = implode('-', $element['#parents']); // @todo We cannot use unique IDs, or multiple elements on a page currently. // @see https://www.drupal.org/node/2675688 // $wrapper_id = Html::getUniqueId($id_prefix . '-ajax-wrapper'); $wrapper_id = $id_prefix . '-ajax-wrapper'; - $has_coupons = !$order->get('coupons')->isEmpty(); - $element = [ '#tree' => TRUE, '#prefix' => '
', @@ -111,7 +108,7 @@ public static function processForm(array $element, FormStateInterface $form_stat $element['#parents'], ], '#submit' => [ - [get_called_class(), 'addCoupon'], + [get_called_class(), 'applyCoupon'], ], '#ajax' => [ 'callback' => [get_called_class(), 'ajaxRefresh'], @@ -119,7 +116,6 @@ public static function processForm(array $element, FormStateInterface $form_stat ], '#access' => !$has_coupons, ]; - $element['remove'] = [ '#type' => 'submit', '#value' => $element['#remove_title'], @@ -153,7 +149,7 @@ public static function ajaxRefresh(array $form, FormStateInterface $form_state) /** * Apply coupon submit callback. */ - public static function addCoupon(array $form, FormStateInterface $form_state) { + public static function applyCoupon(array $form, FormStateInterface $form_state) { $triggering_element = $form_state->getTriggeringElement(); $parents = $triggering_element['#parents']; array_pop($parents); @@ -166,10 +162,9 @@ public static function addCoupon(array $form, FormStateInterface $form_state) { $coupon = $form_state->getValue($parents); $order->get('coupons')->appendItem($coupon); - drupal_set_message($element['#submit_message']); - $order->save(); $form_state->setRebuild(); + drupal_set_message($element['#submit_message']); } /** @@ -187,7 +182,6 @@ public static function removeCoupon(array $form, FormStateInterface $form_state) $order = $order_storage->load($element['#order_id']); $order->get('coupons')->setValue([]); - $order->save(); $form_state->setRebuild(); } @@ -208,10 +202,10 @@ public static function validateForm(array &$element, FormStateInterface $form_st } $entity_type_manager = \Drupal::entityTypeManager(); $code_path = implode('][', $coupon_parents); + $order_storage = $entity_type_manager->getStorage('commerce_order'); /** @var \Drupal\commerce_order\Entity\OrderInterface $order */ $order = $order_storage->load($element['#order_id']); - /** @var \Drupal\commerce_promotion\CouponStorageInterface $coupon_storage */ $coupon_storage = $entity_type_manager->getStorage('commerce_promotion_coupon'); $coupon = $coupon_storage->loadByCode($coupon_code); @@ -219,7 +213,6 @@ public static function validateForm(array &$element, FormStateInterface $form_st $form_state->setErrorByName($code_path, t('Coupon is invalid')); return; } - foreach ($order->get('coupons') as $item) { if ($item->target_id == $coupon->id()) { $form_state->setErrorByName($code_path, t('Coupon has already been redeemed')); @@ -230,7 +223,6 @@ public static function validateForm(array &$element, FormStateInterface $form_st $order_type_storage = $entity_type_manager->getStorage('commerce_order_type'); /** @var \Drupal\commerce_promotion\PromotionStorageInterface $promotion_storage */ $promotion_storage = $entity_type_manager->getStorage('commerce_promotion'); - /** @var \Drupal\commerce_order\Entity\OrderTypeInterface $order_type */ $order_type = $order_type_storage->load($order->bundle()); $promotion = $promotion_storage->loadByCoupon($order_type, $order->getStore(), $coupon); @@ -238,30 +230,16 @@ public static function validateForm(array &$element, FormStateInterface $form_st $form_state->setErrorByName($code_path, t('Coupon is invalid')); return; } - if (!self::couponApplies($order, $promotion, $coupon)) { $form_state->setErrorByName($code_path, t('Coupon is invalid')); return; } - else { - $form_state->setValueForElement($element, $coupon); - } - } - /** - * Submits the coupon redemption element. - * - * @param array $element - * The form element. - * @param \Drupal\Core\Form\FormStateInterface $form_state - * The current state of the form. - */ - public static function submitForm(array &$element, FormStateInterface $form_state) { - // The coupon was set as element value when validated. + $form_state->setValueForElement($element, $coupon); } /** - * Checks if a coupon applies. + * Checks whether a coupon applies. * * @param \Drupal\commerce_order\Entity\OrderInterface $order * The order. From ff923448a51b89d8291d00ccdd1eb556ae8b77a2 Mon Sep 17 00:00:00 2001 From: Bojan Zivanovic Date: Fri, 17 Mar 2017 01:30:01 +0100 Subject: [PATCH 4/4] Update CouponRedemptionForm.php --- modules/promotion/src/Element/CouponRedemptionForm.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/promotion/src/Element/CouponRedemptionForm.php b/modules/promotion/src/Element/CouponRedemptionForm.php index 9013df1a29..2d4798c879 100644 --- a/modules/promotion/src/Element/CouponRedemptionForm.php +++ b/modules/promotion/src/Element/CouponRedemptionForm.php @@ -22,7 +22,7 @@ * ]; * @endcode * The element value ($form_state->getValue('coupon')) will be the - * coupon ID. Note that the order is not saved if the element was + * coupon ID. Note that the order is not saved if the element was * submitted as a result of the main form being submitted. It is the * responsibility of the caller to update the order in that case. *