[Checkout] Preventing payment step completion without a selected method #6217

tuka217 · 2016-09-26T13:06:03Z

Q	A
Bug fix?	yes
New feature?	no
BC breaks?	no
Related tickets	-
License	MIT

michalmarcinkowski · 2016-09-27T09:23:58Z

src/Sylius/Behat/Context/Ui/CheckoutContext.php

+    public function iShouldNotBeAbleToCompleteThePaymentStep()
+    {
+        Assert::true(
+            $this->selectPaymentPage->hasNoAvailableNextStepButton(),


How about isNextStepButtonUnavailable?

michalmarcinkowski · 2016-09-27T09:29:45Z

.../Bundle/ShopBundle/Resources/views/Checkout/SelectPayment/_no_payment_methods_available.twig

+        <div class="header">{{ 'sylius.ui.warning'|trans }}</div>
+        <p>{{ 'sylius.ui.no_payment_methods_available'|trans }}.</p>
+    </div>
+</div>


Missing blank line

michalmarcinkowski · 2016-09-27T09:32:33Z

src/Sylius/Component/Core/Resolver/DefaultPaymentMethodResolver.php

+        /** @var CorePaymentInterface $shipment */
+        Assert::isInstanceOf($shipment, CorePaymentInterface::class);
+
+        $paymentMethods = $this->paymentMethodRepository->findBy(['enabled' => true]);


We should use $this->paymentMethodRepository->findEnabledForChannel($channel) method.

michalmarcinkowski · 2016-09-27T09:33:40Z

src/Sylius/Component/Core/Resolver/DefaultPaymentMethodResolver.php

+            if ($channel->hasPaymentMethod($paymentMethod)) {
+                return $paymentMethod;
+            }
+        }


This logic will not be necessary after the above comment, but should be removed from the DefaultShippingMethodResolver also (in a separate PR).

GSadee · 2016-09-28T08:03:47Z

...eckout/paying_for_order/preventing_payment_step_completion_without_a_selected_method.feature

@@ -0,0 +1,53 @@
+@checkout
+Feature: Preventing payment step completion without a selected method
+    In order to be prevent from finish payment step without selected method


I guess it should be prevented

the above + from finishing the payment step without selecting a method / from finishing the payment step with no method selected

GSadee · 2016-09-28T08:03:58Z

...eckout/paying_for_order/preventing_payment_step_completion_without_a_selected_method.feature

+Feature: Preventing payment step completion without a selected method
+    In order to be prevent from finish payment step without selected method
+    As a Customer
+    I want to be prevent from finish payment step without selected method


GSadee · 2016-09-28T08:11:21Z

src/Sylius/Behat/Context/Ui/CheckoutContext.php

@@ -1212,6 +1212,36 @@ public function thereShouldBeInformationAboutNoShippingMethodsAvailableFormMyShi
    }

    /**
+     * @Given I do not select any payment method


…ut step [PaymentBundle] Fix not blank constraint for payment method from payment

…t methods

[Payment] Add UnresolvedDefaultPaymentMethodException, DefaultPaymentMethodResolver

…ocessor

NoResponseMate · 2016-09-29T12:58:37Z

src/Sylius/Behat/Context/Ui/CheckoutContext.php

+    {
+        Assert::true(
+            $this->selectPaymentPage->isNextStepButtonUnavailable(),
+           'The "next step" button should be disabled, but it does not.'


but it is not. although I'd be in favour of dropping this part in every method as it doesn't bring any value. If the message is in an exception and it says The "next step" button should be disabled. it's quite obvious what happened without an overly explicit explanation.

NoResponseMate · 2016-09-29T13:00:47Z

src/Sylius/Bundle/PaymentBundle/Resources/translations/validators.en.yml

@@ -19,6 +19,8 @@ sylius:
    payment:
        currency_code:
            not_valid: The currency code you entered is invalid.
+        method:
+            not_blank: 'Please select payment method.'


a payment method.

NoResponseMate · 2016-09-29T13:09:29Z

src/Sylius/Bundle/ShopBundle/Resources/views/Checkout/SelectPayment/_form.html.twig

                {% endfor %}
            </div>
        </div>
+    {% else %}


Is it even physically possible to get to this checkout step without a payment?

If I should suppose that channel cannot be created without payment method then it is impossible. Otherwise you can create a shop without payment method at all and then it could happen.

NoResponseMate · 2016-09-29T13:13:58Z

src/Sylius/Component/Payment/spec/Exception/UnresolvedDefaultPaymentMethodExceptionSpec.php

+ */
+final class UnresolvedDefaultPaymentMethodExceptionSpec extends ObjectBehavior
+{
+


NoResponseMate · 2016-09-29T13:14:42Z

src/Sylius/Component/Payment/spec/Resolver/DefaultPaymentMethodResolverSpec.php

+        $this->getDefaultPaymentMethod($payment)->shouldReturn($firstPaymentMethod);
+    }
+
+    function it_throws_exception_if_there_is_no_enabled_payment_methods(


there_are_no

NoResponseMate · 2016-09-29T13:15:14Z

src/Sylius/Component/Payment/spec/Resolver/DefaultPaymentMethodResolverSpec.php

+namespace spec\Sylius\Component\Payment\Resolver;
+
+use PhpSpec\ObjectBehavior;
+use Prophecy\Argument;


NoResponseMate · 2016-09-29T14:51:36Z

src/Sylius/Bundle/PaymentBundle/Resources/translations/validators.en.yml

@@ -36,4 +38,4 @@ sylius:
                not_blank: Please enter the fee percent.
        code:
            not_blank: 'Please enter payment method code.'
-            unique: 'The payment method with given code already exists.'
+            unique: 'The payment method with given code already exists.'


Missing blank line.

tuka217 · 2016-09-29T15:16:18Z

tests/Controller/CheckoutPaymentApiTest.php

@@ -54,7 +54,7 @@ public function it_does_not_allow_to_select_payment_for_order_that_is_not_addres
        $this->addressOrder($orderId);

        $url = sprintf('/api/checkouts/select-payment/%d', $orderId);
-        $this->client->request('PUT', $url, [], [], static::$authorizedHeaderWithContentType);
+        $this->client->request('PATCH', $url, [], [], static::$authorizedHeaderWithContentType);


The other way is remove now this test until api's tests will be rebuild.

michalmarcinkowski · 2016-09-29T22:19:26Z

Thank you Ania! 👍

[Checkout] Preventing payment step completion without a selected method

tuka217 force-pushed the checkout-payment-validation branch 2 times, most recently from c31369a to 9c05d47 Compare September 27, 2016 08:45

michalmarcinkowski added the Bug Fix label Sep 27, 2016

michalmarcinkowski reviewed Sep 27, 2016

View reviewed changes

tuka217 force-pushed the checkout-payment-validation branch 5 times, most recently from a2eb195 to 69c3289 Compare September 27, 2016 14:16

GSadee requested changes Sep 28, 2016

View reviewed changes

tuka217 force-pushed the checkout-payment-validation branch from 154da14 to 9caf1c7 Compare September 29, 2016 11:12

tuka217 changed the title ~~[WIP] [Checkout] Preventing payment step completion without a selected method~~ [Checkout] Preventing payment step completion without a selected method Sep 29, 2016

tuka217 added 6 commits September 29, 2016 13:22

[feature][Behat] Add scenatios with implementation

cb110cc

[CoreBundle] Enable validation for selecting payment method on checko…

6b956d2

…ut step [PaymentBundle] Fix not blank constraint for payment method from payment

[ShopBundle] Disable next step button if there is no available paymen…

8fec701

…t methods

[Core] Introduce DefaultPaymentMethodResolver

6d1509b

[Payment] Add UnresolvedDefaultPaymentMethodException, DefaultPaymentMethodResolver

[CoreBundle] Remove OrderPaymentListener

69f82d1

[CoreBundle][Core] Add DefaultPaymentMethodResolver to OrderPyamentPr…

83f898c

…ocessor

GSadee approved these changes Sep 29, 2016

View reviewed changes

NoResponseMate reviewed Sep 29, 2016

View reviewed changes

[ShopBundle] Fix payment method updating or order thank you page

eef98fa

tuka217 force-pushed the checkout-payment-validation branch from 9caf1c7 to fa1d4f7 Compare September 29, 2016 13:46

tuka217 force-pushed the checkout-payment-validation branch from fa1d4f7 to b435df6 Compare September 29, 2016 13:49

NoResponseMate reviewed Sep 29, 2016

View reviewed changes

tuka217 commented Sep 29, 2016

View reviewed changes

tuka217 force-pushed the checkout-payment-validation branch 2 times, most recently from 9d969fa to 2114400 Compare September 29, 2016 15:22

Fix after RV

15b65c0

tuka217 force-pushed the checkout-payment-validation branch from 2114400 to 15b65c0 Compare September 29, 2016 15:24

michalmarcinkowski merged commit 8da8c30 into Sylius:master Sep 29, 2016

pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019

Merge pull request Sylius#6217 from tuka217/checkout-payment-validation

967734e

[Checkout] Preventing payment step completion without a selected method

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Checkout] Preventing payment step completion without a selected method #6217

[Checkout] Preventing payment step completion without a selected method #6217

tuka217 commented Sep 26, 2016

michalmarcinkowski Sep 27, 2016

michalmarcinkowski Sep 27, 2016

michalmarcinkowski Sep 27, 2016

michalmarcinkowski Sep 27, 2016

GSadee Sep 28, 2016

NoResponseMate Sep 29, 2016

GSadee Sep 28, 2016

GSadee Sep 28, 2016

NoResponseMate Sep 29, 2016

NoResponseMate Sep 29, 2016

NoResponseMate Sep 29, 2016

tuka217 Sep 29, 2016 •

edited

Loading

NoResponseMate Sep 29, 2016

NoResponseMate Sep 29, 2016

NoResponseMate Sep 29, 2016

NoResponseMate Sep 29, 2016

tuka217 Sep 29, 2016 •

edited

Loading

michalmarcinkowski commented Sep 29, 2016

[Checkout] Preventing payment step completion without a selected method #6217

[Checkout] Preventing payment step completion without a selected method #6217

Conversation

tuka217 commented Sep 26, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tuka217 Sep 29, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tuka217 Sep 29, 2016 • edited Loading

Choose a reason for hiding this comment

michalmarcinkowski commented Sep 29, 2016

tuka217 Sep 29, 2016 •

edited

Loading

tuka217 Sep 29, 2016 •

edited

Loading