From 9fe9181b6ee64c486f178c4fc2537ffaf41c49bf Mon Sep 17 00:00:00 2001 From: Victor Vasiloi Date: Thu, 16 Apr 2020 18:27:57 +0300 Subject: [PATCH 1/4] check different shipping address field and display shipping address form when addresses are different --- ..._with_a_different_shipping_address.feature | 52 ++++++++++++++++ .../Checkout/CheckoutAddressingContext.php | 32 ++++++++++ .../Behat/Page/Shop/Checkout/AddressPage.php | 17 ++++++ .../Shop/Checkout/AddressPageInterface.php | 4 ++ .../Form/Type/Checkout/AddressType.php | 59 +++++++++++++++---- .../Resources/config/services/checkout.xml | 1 + 6 files changed, 152 insertions(+), 13 deletions(-) create mode 100644 features/checkout/addressing_order/returning_to_addressing_step_with_a_different_shipping_address.feature diff --git a/features/checkout/addressing_order/returning_to_addressing_step_with_a_different_shipping_address.feature b/features/checkout/addressing_order/returning_to_addressing_step_with_a_different_shipping_address.feature new file mode 100644 index 0000000000..d84f6e2e49 --- /dev/null +++ b/features/checkout/addressing_order/returning_to_addressing_step_with_a_different_shipping_address.feature @@ -0,0 +1,52 @@ +@checkout +Feature: Returning to addressing step with a different shipping address + In order to change the shipping address + As a Visitor + I want to return to the addressing step and change the shipping address + + Background: + Given the store operates on a single channel in "United States" + And the store has a product "Summer T-Shirt" priced at "$19.99" + And the store ships everywhere for free + + @ui + Scenario: Going back to addressing step after submitting a different shipping address + Given I have product "Summer T-Shirt" in the cart + And I am at the checkout addressing step + When I specify the email as "john.doe@example.com" + And I specify the billing address as "Brooklyn", "9036 Country Club Ave.", "11230", "United States" for "John Doe" + And I specify the shipping address as "Brooklyn", "70 Joy Ridge St", "11225", "United States" for "Jane Doe" + And I complete the addressing step + And I decide to change my address + Then different shipping address should be checked + + @ui + Scenario: Going back to addressing step after not submitting a different shipping address + Given I have product "Summer T-Shirt" in the cart + And I am at the checkout addressing step + When I specify the email as "john.doe@example.com" + And I specify the billing address as "Brooklyn", "9036 Country Club Ave.", "11230", "United States" for "John Doe" + And I complete the addressing step + And I decide to change my address + Then different shipping address should not be checked + + @ui @javascript + Scenario: Going back to addressing step after submitting a different shipping address + Given I have product "Summer T-Shirt" in the cart + And I am at the checkout addressing step + When I specify the email as "john.doe@example.com" + And I specify the billing address as "Brooklyn", "9036 Country Club Ave.", "11230", "United States" for "John Doe" + And I specify the shipping address as "Brooklyn", "70 Joy Ridge St", "11225", "United States" for "Jane Doe" + And I complete the addressing step + And I decide to change my address + And shipping address should be visible + + @ui @javascript + Scenario: Going back to addressing step after not submitting a different shipping address + Given I have product "Summer T-Shirt" in the cart + And I am at the checkout addressing step + When I specify the email as "john.doe@example.com" + And I specify the billing address as "Brooklyn", "9036 Country Club Ave.", "11230", "United States" for "John Doe" + And I complete the addressing step + And I decide to change my address + And shipping address should not be visible diff --git a/src/Sylius/Behat/Context/Ui/Shop/Checkout/CheckoutAddressingContext.php b/src/Sylius/Behat/Context/Ui/Shop/Checkout/CheckoutAddressingContext.php index d6c9e454a9..4b524b13d5 100644 --- a/src/Sylius/Behat/Context/Ui/Shop/Checkout/CheckoutAddressingContext.php +++ b/src/Sylius/Behat/Context/Ui/Shop/Checkout/CheckoutAddressingContext.php @@ -377,6 +377,38 @@ final class CheckoutAddressingContext implements Context Assert::true($this->addressComparator->equal($address, $this->addressPage->getPreFilledBillingAddress())); } + /** + * @Then different shipping address should be checked + */ + public function differentShippingAddressShouldBeChecked(): void + { + Assert::true($this->addressPage->isDifferentShippingAddressChecked()); + } + + /** + * @Then different shipping address should not be checked + */ + public function differentShippingAddressShouldNotBeChecked(): void + { + Assert::false($this->addressPage->isDifferentShippingAddressChecked()); + } + + /** + * @Then shipping address should be visible + */ + public function shippingAddressShouldBeVisible(): void + { + Assert::true($this->addressPage->isShippingAddressVisible()); + } + + /** + * @Then shipping address should not be visible + */ + public function shippingAddressShouldNotBeVisible(): void + { + Assert::false($this->addressPage->isShippingAddressVisible()); + } + /** * @Then /^I should(?:| also) be notified that the "([^"]+)" and the "([^"]+)" in (shipping|billing) details are required$/ */ diff --git a/src/Sylius/Behat/Page/Shop/Checkout/AddressPage.php b/src/Sylius/Behat/Page/Shop/Checkout/AddressPage.php index ba88316674..fcbecd8989 100644 --- a/src/Sylius/Behat/Page/Shop/Checkout/AddressPage.php +++ b/src/Sylius/Behat/Page/Shop/Checkout/AddressPage.php @@ -16,6 +16,7 @@ namespace Sylius\Behat\Page\Shop\Checkout; use Behat\Mink\Driver\Selenium2Driver; use Behat\Mink\Element\NodeElement; use Behat\Mink\Exception\ElementNotFoundException; +use Behat\Mink\Exception\UnsupportedDriverActionException; use Behat\Mink\Session; use DMore\ChromeDriver\ChromeDriver; use FriendsOfBehat\PageObjectExtension\Page\SymfonyPage; @@ -68,6 +69,21 @@ class AddressPage extends SymfonyPage implements AddressPageInterface $billingAddressSwitch->check(); } + public function isDifferentShippingAddressChecked(): bool + { + return $this->getElement('different_shipping_address')->isChecked(); + } + + public function isShippingAddressVisible(): bool + { + try { + return $this->getElement('shipping_address')->isVisible(); + } catch (UnsupportedDriverActionException $e) { + // it's visible by default and is being hidden with JS + return true; + } + } + public function checkInvalidCredentialsValidation(): bool { /** @var NodeElement $validationElement */ @@ -285,6 +301,7 @@ class AddressPage extends SymfonyPage implements AddressPageInterface 'login_password' => '[data-test-password-input]', 'login_validation_error' => '[data-test-login-validation-error]', 'next_step' => '[data-test-next-step]', + 'shipping_address' => '#sylius-shipping-address', 'shipping_address_book' => '[data-test-shipping-address] [data-test-address-book]', 'shipping_city' => '[data-test-shipping-city]', 'shipping_country' => '[data-test-shipping-country]', diff --git a/src/Sylius/Behat/Page/Shop/Checkout/AddressPageInterface.php b/src/Sylius/Behat/Page/Shop/Checkout/AddressPageInterface.php index 34e4a8c790..130319e62d 100644 --- a/src/Sylius/Behat/Page/Shop/Checkout/AddressPageInterface.php +++ b/src/Sylius/Behat/Page/Shop/Checkout/AddressPageInterface.php @@ -71,4 +71,8 @@ interface AddressPageInterface extends SymfonyPageInterface /** @return string[] */ public function getAvailableBillingCountries(): array; + + public function isDifferentShippingAddressChecked(): bool; + + public function isShippingAddressVisible(): bool; } diff --git a/src/Sylius/Bundle/CoreBundle/Form/Type/Checkout/AddressType.php b/src/Sylius/Bundle/CoreBundle/Form/Type/Checkout/AddressType.php index a6db4e84d9..782cc1e3b8 100644 --- a/src/Sylius/Bundle/CoreBundle/Form/Type/Checkout/AddressType.php +++ b/src/Sylius/Bundle/CoreBundle/Form/Type/Checkout/AddressType.php @@ -16,7 +16,8 @@ namespace Sylius\Bundle\CoreBundle\Form\Type\Checkout; use Sylius\Bundle\AddressingBundle\Form\Type\AddressType as SyliusAddressType; use Sylius\Bundle\CoreBundle\Form\Type\Customer\CustomerCheckoutGuestType; use Sylius\Bundle\ResourceBundle\Form\Type\AbstractResourceType; -use Sylius\Component\Core\Model\CustomerInterface; +use Sylius\Component\Addressing\Comparator\AddressComparatorInterface; + use Sylius\Component\Core\Model\CustomerInterface; use Sylius\Component\Core\Model\OrderInterface; use Sylius\Component\Customer\Model\CustomerAwareInterface; use Symfony\Component\Form\Extension\Core\Type\CheckboxType; @@ -29,29 +30,61 @@ use Webmozart\Assert\Assert; final class AddressType extends AbstractResourceType { + /** @var AddressComparatorInterface|null */ + private $addressComparator; + + public function __construct(string $dataClass, array $validationGroups = [], ?AddressComparatorInterface $addressComparator = null) + { + parent::__construct($dataClass, $validationGroups); + + if (null === $addressComparator) { + @trigger_error( + sprintf( + 'Not passing an $addressComparator to %s constructor is deprecated since Sylius 1.7 and will be removed in Sylius 2.0.', + __CLASS__, + ), + \E_USER_DEPRECATED + ); + } + + $this->addressComparator = $addressComparator; + } + + /** + * {@inheritdoc} + */ public function buildForm(FormBuilderInterface $builder, array $options): void { $builder - ->add('differentBillingAddress', CheckboxType::class, [ - 'mapped' => false, - 'required' => false, - 'label' => 'sylius.form.checkout.addressing.different_billing_address', - ]) - ->add('differentShippingAddress', CheckboxType::class, [ - 'mapped' => false, - 'required' => false, - 'label' => 'sylius.form.checkout.addressing.different_shipping_address', - ]) - ->addEventListener(FormEvents::PRE_SET_DATA, static function (FormEvent $event): void { + ->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event): void { $form = $event->getForm(); $order = $event->getData(); - /** @var OrderInterface $order */ Assert::isInstanceOf($order, OrderInterface::class); $channel = $order->getChannel(); + $differentBillingOrShippingAddressChecked = null !== $this->addressComparator + && null !== $order->getBillingAddress() && null !== $order->getShippingAddress() + && !$this->addressComparator->equal($order->getBillingAddress(), $order->getShippingAddress()); + $form + ->add('differentBillingAddress', CheckboxType::class, [ + 'mapped' => false, + 'required' => false, + 'label' => 'sylius.form.checkout.addressing.different_billing_address', + 'attr' => [ + 'checked' => $differentBillingOrShippingAddressChecked, + ], + ]) + ->add('differentShippingAddress', CheckboxType::class, [ + 'mapped' => false, + 'required' => false, + 'label' => 'sylius.form.checkout.addressing.different_shipping_address', + 'attr' => [ + 'checked' => $differentBillingOrShippingAddressChecked, + ], + ]) ->add('shippingAddress', SyliusAddressType::class, [ 'shippable' => true, 'constraints' => [new Valid()], diff --git a/src/Sylius/Bundle/CoreBundle/Resources/config/services/checkout.xml b/src/Sylius/Bundle/CoreBundle/Resources/config/services/checkout.xml index c89ae5768e..ba8066e8a0 100644 --- a/src/Sylius/Bundle/CoreBundle/Resources/config/services/checkout.xml +++ b/src/Sylius/Bundle/CoreBundle/Resources/config/services/checkout.xml @@ -39,6 +39,7 @@ %sylius.model.order.class% %sylius.form.type.checkout_address.validation_groups% + From 949d6f07cd8153546dea0a59b9f6c2c4fb156bad Mon Sep 17 00:00:00 2001 From: Victor Vasiloi Date: Sun, 17 May 2020 13:19:19 +0300 Subject: [PATCH 2/4] review updates --- .../Behat/Page/Shop/Checkout/AddressPage.php | 2 +- .../Form/Type/Checkout/AddressType.php | 62 ++++++++++++------- .../Resources/config/services/checkout.xml | 2 +- 3 files changed, 41 insertions(+), 25 deletions(-) diff --git a/src/Sylius/Behat/Page/Shop/Checkout/AddressPage.php b/src/Sylius/Behat/Page/Shop/Checkout/AddressPage.php index fcbecd8989..2278fda38a 100644 --- a/src/Sylius/Behat/Page/Shop/Checkout/AddressPage.php +++ b/src/Sylius/Behat/Page/Shop/Checkout/AddressPage.php @@ -301,7 +301,7 @@ class AddressPage extends SymfonyPage implements AddressPageInterface 'login_password' => '[data-test-password-input]', 'login_validation_error' => '[data-test-login-validation-error]', 'next_step' => '[data-test-next-step]', - 'shipping_address' => '#sylius-shipping-address', + 'shipping_address' => '[data-test-shipping-address]', 'shipping_address_book' => '[data-test-shipping-address] [data-test-address-book]', 'shipping_city' => '[data-test-shipping-city]', 'shipping_country' => '[data-test-shipping-country]', diff --git a/src/Sylius/Bundle/CoreBundle/Form/Type/Checkout/AddressType.php b/src/Sylius/Bundle/CoreBundle/Form/Type/Checkout/AddressType.php index 782cc1e3b8..44160703ec 100644 --- a/src/Sylius/Bundle/CoreBundle/Form/Type/Checkout/AddressType.php +++ b/src/Sylius/Bundle/CoreBundle/Form/Type/Checkout/AddressType.php @@ -40,7 +40,7 @@ final class AddressType extends AbstractResourceType if (null === $addressComparator) { @trigger_error( sprintf( - 'Not passing an $addressComparator to %s constructor is deprecated since Sylius 1.7 and will be removed in Sylius 2.0.', + 'Not passing an $addressComparator to "%s" constructor is deprecated since Sylius 1.8 and will be impossible in Sylius 2.0.', __CLASS__, ), \E_USER_DEPRECATED @@ -56,35 +56,26 @@ final class AddressType extends AbstractResourceType public function buildForm(FormBuilderInterface $builder, array $options): void { $builder - ->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event): void { + ->add('differentBillingAddress', CheckboxType::class, [ + 'mapped' => false, + 'required' => false, + 'label' => 'sylius.form.checkout.addressing.different_billing_address', + ]) + ->add('differentShippingAddress', CheckboxType::class, [ + 'mapped' => false, + 'required' => false, + 'label' => 'sylius.form.checkout.addressing.different_shipping_address', + ]) + ->addEventListener(FormEvents::PRE_SET_DATA, static function (FormEvent $event): void { $form = $event->getForm(); - $order = $event->getData(); /** @var OrderInterface $order */ + $order = $event->getData(); + Assert::isInstanceOf($order, OrderInterface::class); $channel = $order->getChannel(); - $differentBillingOrShippingAddressChecked = null !== $this->addressComparator - && null !== $order->getBillingAddress() && null !== $order->getShippingAddress() - && !$this->addressComparator->equal($order->getBillingAddress(), $order->getShippingAddress()); - $form - ->add('differentBillingAddress', CheckboxType::class, [ - 'mapped' => false, - 'required' => false, - 'label' => 'sylius.form.checkout.addressing.different_billing_address', - 'attr' => [ - 'checked' => $differentBillingOrShippingAddressChecked, - ], - ]) - ->add('differentShippingAddress', CheckboxType::class, [ - 'mapped' => false, - 'required' => false, - 'label' => 'sylius.form.checkout.addressing.different_shipping_address', - 'attr' => [ - 'checked' => $differentBillingOrShippingAddressChecked, - ], - ]) ->add('shippingAddress', SyliusAddressType::class, [ 'shippable' => true, 'constraints' => [new Valid()], @@ -96,6 +87,18 @@ final class AddressType extends AbstractResourceType ]) ; }) + ->addEventListener(FormEvents::POST_SET_DATA, function (FormEvent $event): void { + $form = $event->getForm(); + /** @var OrderInterface $order */ + $order = $event->getData(); + + Assert::isInstanceOf($order, OrderInterface::class); + + $areAddressesDifferent = $this->areAddressesDifferent($order); + + $form->get('differentBillingAddress')->setData($areAddressesDifferent); + $form->get('differentShippingAddress')->setData($areAddressesDifferent); + }) ->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event) use ($options): void { $form = $event->getForm(); $resource = $event->getData(); @@ -147,4 +150,17 @@ final class AddressType extends AbstractResourceType { return 'sylius_checkout_address'; } + + private function areAddressesDifferent(OrderInterface $order): bool + { + if (null === $this->addressComparator) { + return false; + } + + if (null === $order->getBillingAddress() || null === $order->getShippingAddress()) { + return false; + } + + return !$this->addressComparator->equal($order->getBillingAddress(), $order->getShippingAddress()); + } } diff --git a/src/Sylius/Bundle/CoreBundle/Resources/config/services/checkout.xml b/src/Sylius/Bundle/CoreBundle/Resources/config/services/checkout.xml index ba8066e8a0..650eb100d6 100644 --- a/src/Sylius/Bundle/CoreBundle/Resources/config/services/checkout.xml +++ b/src/Sylius/Bundle/CoreBundle/Resources/config/services/checkout.xml @@ -39,7 +39,7 @@ %sylius.model.order.class% %sylius.form.type.checkout_address.validation_groups% - + From bf7ff7f76be0063ac755717e32a9d745ee38da5c Mon Sep 17 00:00:00 2001 From: Victor Vasiloi Date: Sun, 17 May 2020 13:27:18 +0300 Subject: [PATCH 3/4] simplify address comparison --- .../CoreBundle/Form/Type/Checkout/AddressType.php | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/Sylius/Bundle/CoreBundle/Form/Type/Checkout/AddressType.php b/src/Sylius/Bundle/CoreBundle/Form/Type/Checkout/AddressType.php index 44160703ec..d6b9487b86 100644 --- a/src/Sylius/Bundle/CoreBundle/Form/Type/Checkout/AddressType.php +++ b/src/Sylius/Bundle/CoreBundle/Form/Type/Checkout/AddressType.php @@ -17,7 +17,8 @@ use Sylius\Bundle\AddressingBundle\Form\Type\AddressType as SyliusAddressType; use Sylius\Bundle\CoreBundle\Form\Type\Customer\CustomerCheckoutGuestType; use Sylius\Bundle\ResourceBundle\Form\Type\AbstractResourceType; use Sylius\Component\Addressing\Comparator\AddressComparatorInterface; - use Sylius\Component\Core\Model\CustomerInterface; +use Sylius\Component\Core\Model\AddressInterface; +use Sylius\Component\Core\Model\CustomerInterface; use Sylius\Component\Core\Model\OrderInterface; use Sylius\Component\Customer\Model\CustomerAwareInterface; use Symfony\Component\Form\Extension\Core\Type\CheckboxType; @@ -94,7 +95,7 @@ final class AddressType extends AbstractResourceType Assert::isInstanceOf($order, OrderInterface::class); - $areAddressesDifferent = $this->areAddressesDifferent($order); + $areAddressesDifferent = $this->areAddressesDifferent($order->getBillingAddress(), $order->getShippingAddress()); $form->get('differentBillingAddress')->setData($areAddressesDifferent); $form->get('differentShippingAddress')->setData($areAddressesDifferent); @@ -151,16 +152,12 @@ final class AddressType extends AbstractResourceType return 'sylius_checkout_address'; } - private function areAddressesDifferent(OrderInterface $order): bool + private function areAddressesDifferent(?AddressInterface $firstAddress, ?AddressInterface $secondAddress): bool { - if (null === $this->addressComparator) { + if (null === $this->addressComparator || null === $firstAddress || null === $secondAddress) { return false; } - if (null === $order->getBillingAddress() || null === $order->getShippingAddress()) { - return false; - } - - return !$this->addressComparator->equal($order->getBillingAddress(), $order->getShippingAddress()); + return !$this->addressComparator->equal($firstAddress, $secondAddress); } } From 7cd003db1765272b69d12d30d9ca5d3b55008fe0 Mon Sep 17 00:00:00 2001 From: Victor Vasiloi Date: Sun, 28 Jun 2020 14:17:52 +0300 Subject: [PATCH 4/4] fix psalm errors --- .../CoreBundle/Form/Type/Checkout/AddressType.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Sylius/Bundle/CoreBundle/Form/Type/Checkout/AddressType.php b/src/Sylius/Bundle/CoreBundle/Form/Type/Checkout/AddressType.php index d6b9487b86..49e8a2a6c5 100644 --- a/src/Sylius/Bundle/CoreBundle/Form/Type/Checkout/AddressType.php +++ b/src/Sylius/Bundle/CoreBundle/Form/Type/Checkout/AddressType.php @@ -69,11 +69,11 @@ final class AddressType extends AbstractResourceType ]) ->addEventListener(FormEvents::PRE_SET_DATA, static function (FormEvent $event): void { $form = $event->getForm(); + + Assert::isInstanceOf($event->getData(), OrderInterface::class); + /** @var OrderInterface $order */ $order = $event->getData(); - - Assert::isInstanceOf($order, OrderInterface::class); - $channel = $order->getChannel(); $form @@ -90,11 +90,11 @@ final class AddressType extends AbstractResourceType }) ->addEventListener(FormEvents::POST_SET_DATA, function (FormEvent $event): void { $form = $event->getForm(); + + Assert::isInstanceOf($event->getData(), OrderInterface::class); + /** @var OrderInterface $order */ $order = $event->getData(); - - Assert::isInstanceOf($order, OrderInterface::class); - $areAddressesDifferent = $this->areAddressesDifferent($order->getBillingAddress(), $order->getShippingAddress()); $form->get('differentBillingAddress')->setData($areAddressesDifferent);