[API][Payment Method] Refactor choose payment method handler

features/checkout/paying_for_order/changing_offline_payment_method_after_order_confirmation.feature

@@ -14,31 +14,31 @@ Feature: Changing the offline payment method after order confirmation
     @ui
     Scenario: Retrying the payment with different offline payment
         Given I added product "PHP T-Shirt" to the cart
-        And I complete addressing step with email "john@example.com" and "United States" based billing address
+        When I complete addressing step with email "john@example.com" and "United States" based billing address
         And I have proceeded selecting "Free" shipping method
         And I have proceeded selecting "Cash on delivery" payment method
         And I have confirmed order
-        When I change payment method to "Offline" after checkout
+        And I change payment method to "Offline" after checkout
         Then I should have chosen "Offline" payment method
 
     @ui
     Scenario: Retrying the payment with different offline payment works correctly together with inventory
         Given there is 1 unit of product "PHP T-Shirt" available in the inventory
         And this product is tracked by the inventory
-        And I added product "PHP T-Shirt" to the cart
+        When I added product "PHP T-Shirt" to the cart
         And I complete addressing step with email "john@example.com" and "United States" based billing address
         And I have proceeded selecting "Free" shipping method
         And I have proceeded selecting "Cash on delivery" payment method
         And I have confirmed order
-        When I change payment method to "Offline" after checkout
+        And I change payment method to "Offline" after checkout
         Then I should have chosen "Offline" payment method
 
     @api
     Scenario: Changing chosen offline payment method to another offline payment method after checkout
         Given I added product "PHP T-Shirt" to the cart
-        And I complete addressing step with email "john@example.com" and "United States" based billing address
-        And I have proceeded selecting "Free" shipping method
-        And I have proceeded selecting "Cash on delivery" payment method
-        And I have confirmed order
-        When I change payment method to "Offline" after checkout
+        When I complete addressing step with email "john@example.com" and "United States" based billing address
+        And I proceed selecting "Free" shipping method
+        And I proceed selecting "Cash on delivery" payment method
+        And I confirm my order
+        And I change payment method to "Offline" after checkout
         Then I should have chosen "Offline" payment method

src/Sylius/Behat/Context/Api/Shop/CheckoutContext.php

@@ -300,7 +300,6 @@ public function iProvideAdditionalNotesLike(string $notes): void
 
     /**
      * @Given I confirmed my order
-     * @Given I have confirmed order
      * @When I confirm my order
      * @When I try to confirm my order
      * @When /^the (?:visitor|customer) confirm his order$/
@@ -340,7 +339,7 @@ public function iConfirmMyOrder(): void
      * @Given /^the (?:visitor|customer) has proceeded ("[^"]+" shipping method)$/
      * @When /^the visitor try to proceed with ("[^"]+" shipping method) in the customer cart$/
      * @When I try to change shipping method to :shippingMethod
-     * @Given I have proceeded selecting :shippingMethod shipping method
+     * @Given I proceed selecting :shippingMethod shipping method
      */
     public function iProceededWithShippingMethod(ShippingMethodInterface $shippingMethod): void
     {
@@ -386,6 +385,7 @@ public function iCompleteTheShippingStepWithFirstShippingMethod(): void
      * @When I choose :paymentMethod payment method
      * @When I select :paymentMethod payment method
      * @When I have proceeded selecting :paymentMethod payment method
+     * @When I proceed selecting :paymentMethod payment method
      * @When /^the (?:customer|visitor) proceed with ("[^"]+" payment)$/
      * @Given /^the (?:customer|visitor) has proceeded ("[^"]+" payment)$/
      * @When I try to change payment method to :paymentMethod payment

src/Sylius/Bundle/ApiBundle/CommandHandler/Checkout/ChoosePaymentMethodHandler.php

@@ -17,7 +17,6 @@
 use Sylius\Bundle\ApiBundle\Command\Checkout\ChoosePaymentMethod;
 use Sylius\Component\Core\Model\OrderInterface;
 use Sylius\Component\Core\Model\PaymentMethodInterface;
-use Sylius\Component\Core\OrderCheckoutStates;
 use Sylius\Component\Core\OrderCheckoutTransitions;
 use Sylius\Component\Core\Repository\OrderRepositoryInterface;
 use Sylius\Component\Core\Repository\PaymentMethodRepositoryInterface;
@@ -69,31 +68,47 @@ public function __invoke(ChoosePaymentMethod $choosePaymentMethod): OrderInterfa
         $payment = $this->paymentRepository->findOneByOrderId($choosePaymentMethod->paymentId, $cart->getId());
         Assert::notNull($payment, 'Can not find payment with given identifier.');
 
-        if (in_array(
-            $cart->getCheckoutState(),
-            [OrderCheckoutStates::STATE_SHIPPING_SELECTED, OrderCheckoutStates::STATE_SHIPPING_SKIPPED, OrderCheckoutStates::STATE_PAYMENT_SELECTED],
-            true)
-        ) {
-            $stateMachine = $this->stateMachineFactory->get($cart, OrderCheckoutTransitions::GRAPH);
+        if ($cart->getState() === OrderInterface::STATE_CART) {
+            return $this->choosePaymentMethodOnCheckout($cart, $payment, $paymentMethod);
+        }
+
+        if ($cart->getState() === OrderInterface::STATE_NEW) {
+            return $this->changePaymentMethod($cart, $payment, $paymentMethod);
+        }
 
-            Assert::true($stateMachine->can(
-                OrderCheckoutTransitions::TRANSITION_SELECT_PAYMENT),
-                'Order cannot have payment method assigned.'
-            );
+        throw new \InvalidArgumentException('Payment method can not be set');
+    }
 
-            $payment->setMethod($paymentMethod);
-            $stateMachine->apply(OrderCheckoutTransitions::TRANSITION_SELECT_PAYMENT);
+    private function choosePaymentMethodOnCheckout(
+        OrderInterface $cart,
+        PaymentInterface $payment,
+        PaymentMethodInterface $paymentMethod
+    ): OrderInterface {
+        $stateMachine = $this->stateMachineFactory->get($cart, OrderCheckoutTransitions::GRAPH);
 
-            return $cart;
-        }
+        Assert::true($stateMachine->can(
+            OrderCheckoutTransitions::TRANSITION_SELECT_PAYMENT),
+            'Order cannot have payment method assigned.'
+        );
 
+        $payment->setMethod($paymentMethod);
+        $stateMachine->apply(OrderCheckoutTransitions::TRANSITION_SELECT_PAYMENT);
+
+        return $cart;
+    }
+
+    private function changePaymentMethod(
+        OrderInterface $order,
+        PaymentInterface $payment,
+        PaymentMethodInterface $paymentMethod
+    ): OrderInterface {
         Assert::same(
             $payment->getState(),
             PaymentInterface::STATE_NEW,
             'Can not change payment method for this payment'
         );
         $payment->setMethod($paymentMethod);
 
-        return $cart;
+        return $order;
     }
 }

src/Sylius/Bundle/ApiBundle/Doctrine/QueryItemExtension/OrderMethodsItemExtension.php

@@ -67,7 +67,23 @@ private function applyUserRulesToItem(
         string $httpRequestMethodType,
         string $operationName
     ): void {
-        if ($operationName === 'shop_select_payment_method') {
+        if ($user instanceof ShopUserInterface && $operationName === 'shop_select_payment_method') {
+            $queryBuilder
+                ->andWhere(sprintf('%s.customer = :customer', $rootAlias))
+                ->setParameter('customer', $user->getCustomer()->getId())
+            ;
+
+            return;
+        }
+
+        if ($user === null && $operationName === 'shop_select_payment_method') {
+            $queryBuilder
+                ->leftJoin(sprintf('%s.customer', $rootAlias), 'customer')
+                ->leftJoin('customer.user', 'user')
+                ->andWhere('user IS NULL')
+                ->orWhere(sprintf('%s.customer IS NULL', $rootAlias))
+            ;
+
             return;
         }
 

src/Sylius/Bundle/ApiBundle/spec/CommandHandler/Checkout/ChoosePaymentMethodHandlerSpec.php

@@ -18,6 +18,7 @@
 use SM\Factory\FactoryInterface;
 use SM\StateMachine\StateMachineInterface;
 use Sylius\Bundle\ApiBundle\Command\Checkout\ChoosePaymentMethod;
+use Sylius\Component\Core\Model\Order;
 use Sylius\Component\Core\Model\OrderInterface;
 use Sylius\Component\Core\Model\PaymentInterface;
 use Sylius\Component\Core\Model\PaymentMethodInterface;
@@ -63,6 +64,9 @@ function it_assigns_chosen_payment_method_to_specified_payment_while_checkout(
         $cart->getId()->willReturn('111');
 
         $paymentRepository->findOneByOrderId('123', '111')->willReturn($payment);
+
+        $cart->getState()->willReturn(OrderInterface::STATE_CART);
+
         $payment->setMethod($paymentMethod)->shouldBeCalled();
 
         $stateMachine->apply('select_payment')->shouldBeCalled();
@@ -88,6 +92,9 @@ function it_assigns_chosen_payment_method_to_specified_payment_after_checkout(
         $cart->getId()->willReturn('111');
 
         $paymentRepository->findOneByOrderId('123', '111')->willReturn($payment);
+
+        $cart->getState()->willReturn(OrderInterface::STATE_NEW);
+
         $payment->getState()->willReturn(PaymentInterface::STATE_NEW);
         $payment->setMethod($paymentMethod)->shouldBeCalled();
 
@@ -215,6 +222,9 @@ function it_throws_an_exception_if_payment_is_in_different_state_than_new(
         $cart->getId()->willReturn('111');
 
         $paymentRepository->findOneByOrderId('123', '111')->willReturn($payment);
+
+        $cart->getState()->willReturn(OrderInterface::STATE_FULFILLED);
+
         $payment->getState()->willReturn(PaymentInterface::STATE_CANCELLED);
 
         $this

src/Sylius/Bundle/ApiBundle/spec/Doctrine/QueryItemExtension/OrderMethodsItemExtensionSpec.php

@@ -245,6 +245,86 @@ function it_applies_conditions_to_put_order_with_state_cart_and_with_null_user_a
         );
     }
 
+    function it_applies_conditions_to_shop_select_payment_method_operation_with_given_user(
+        UserContextInterface $userContext,
+        QueryBuilder $queryBuilder,
+        ShopUserInterface $shopUser,
+        CustomerInterface $customer,
+        QueryNameGeneratorInterface $queryNameGenerator
+    ): void {
+        $queryBuilder->getRootAliases()->willReturn(['o']);
+
+        $userContext->getUser()->willReturn($shopUser);
+
+        $shopUser->getCustomer()->willReturn($customer);
+        $customer->getId()->willReturn(1);
+        $shopUser->getRoles()->willReturn(['ROLE_USER']);
+
+        $queryBuilder
+            ->andWhere(sprintf('%s.customer = :customer', 'o'))
+            ->shouldBeCalled()
+            ->willReturn($queryBuilder)
+        ;
+
+        $queryBuilder
+            ->setParameter('customer', 1)
+            ->shouldBeCalled()
+            ->willReturn($queryBuilder)
+        ;
+
+        $this->applyToItem(
+            $queryBuilder,
+            $queryNameGenerator,
+            OrderInterface::class,
+            ['tokenValue' => 'xaza-tt_fee'],
+            'shop_select_payment_method',
+            [ContextKeys::HTTP_REQUEST_METHOD_TYPE => Request::METHOD_PATCH],
+        );
+    }
+
+    function it_applies_conditions_to_shop_select_payment_method_operation_with_user_equal_null(
+        UserContextInterface $userContext,
+        QueryBuilder $queryBuilder,
+        QueryNameGeneratorInterface $queryNameGenerator
+    ): void {
+        $queryBuilder->getRootAliases()->willReturn(['o']);
+
+        $userContext->getUser()->willReturn(null);
+
+        $queryBuilder
+            ->leftJoin(sprintf('%s.customer', 'o'), 'customer')
+            ->shouldBeCalled()
+            ->willReturn($queryBuilder)
+        ;
+
+        $queryBuilder
+            ->leftJoin('customer.user', 'user')
+            ->shouldBeCalled()
+            ->willReturn($queryBuilder)
+        ;
+
+        $queryBuilder
+            ->andWhere('user IS NULL')
+            ->shouldBeCalled()
+            ->willReturn($queryBuilder)
+        ;
+
+        $queryBuilder
+            ->orWhere(sprintf('%s.customer IS NULL', 'o'))
+            ->shouldBeCalled()
+            ->willReturn($queryBuilder)
+        ;
+
+        $this->applyToItem(
+            $queryBuilder,
+            $queryNameGenerator,
+            OrderInterface::class,
+            ['tokenValue' => 'xaza-tt_fee'],
+            'shop_select_payment_method',
+            [ContextKeys::HTTP_REQUEST_METHOD_TYPE => Request::METHOD_PATCH],
+        );
+    }
+
     function it_applies_conditions_to_patch_order_with_state_cart_and_with_null_user_and_not_null_customer_if_present_user_is_null_and_present_customer_is_not_null(
         UserContextInterface $userContext,
         QueryBuilder $queryBuilder,