From e802597b4338c6165fb3773669bbd5db3570a028 Mon Sep 17 00:00:00 2001 From: Adam Kasperczak Date: Mon, 19 Oct 2020 11:58:55 +0200 Subject: [PATCH] [API][Payment Method] Refactor choose payment method handler --- ...nt_method_after_order_confirmation.feature | 18 ++--- .../Context/Api/Shop/CheckoutContext.php | 4 +- .../Checkout/ChoosePaymentMethodHandler.php | 47 +++++++---- .../OrderMethodsItemExtension.php | 18 ++++- .../ChoosePaymentMethodHandlerSpec.php | 10 +++ .../OrderMethodsItemExtensionSpec.php | 80 +++++++++++++++++++ 6 files changed, 149 insertions(+), 28 deletions(-) diff --git a/features/checkout/paying_for_order/changing_offline_payment_method_after_order_confirmation.feature b/features/checkout/paying_for_order/changing_offline_payment_method_after_order_confirmation.feature index b499f08e1faa..b8c3644b9de5 100644 --- a/features/checkout/paying_for_order/changing_offline_payment_method_after_order_confirmation.feature +++ b/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 diff --git a/src/Sylius/Behat/Context/Api/Shop/CheckoutContext.php b/src/Sylius/Behat/Context/Api/Shop/CheckoutContext.php index d6745efff999..25b92ffe2916 100644 --- a/src/Sylius/Behat/Context/Api/Shop/CheckoutContext.php +++ b/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 diff --git a/src/Sylius/Bundle/ApiBundle/CommandHandler/Checkout/ChoosePaymentMethodHandler.php b/src/Sylius/Bundle/ApiBundle/CommandHandler/Checkout/ChoosePaymentMethodHandler.php index 1a9a99a6fb77..66614b11df86 100644 --- a/src/Sylius/Bundle/ApiBundle/CommandHandler/Checkout/ChoosePaymentMethodHandler.php +++ b/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,24 +68,40 @@ 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, @@ -94,6 +109,6 @@ public function __invoke(ChoosePaymentMethod $choosePaymentMethod): OrderInterfa ); $payment->setMethod($paymentMethod); - return $cart; + return $order; } } diff --git a/src/Sylius/Bundle/ApiBundle/Doctrine/QueryItemExtension/OrderMethodsItemExtension.php b/src/Sylius/Bundle/ApiBundle/Doctrine/QueryItemExtension/OrderMethodsItemExtension.php index 6fd8fd30860c..73bf38b9eee7 100644 --- a/src/Sylius/Bundle/ApiBundle/Doctrine/QueryItemExtension/OrderMethodsItemExtension.php +++ b/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; } diff --git a/src/Sylius/Bundle/ApiBundle/spec/CommandHandler/Checkout/ChoosePaymentMethodHandlerSpec.php b/src/Sylius/Bundle/ApiBundle/spec/CommandHandler/Checkout/ChoosePaymentMethodHandlerSpec.php index fb4b4d66a75d..44f46b62e190 100644 --- a/src/Sylius/Bundle/ApiBundle/spec/CommandHandler/Checkout/ChoosePaymentMethodHandlerSpec.php +++ b/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 diff --git a/src/Sylius/Bundle/ApiBundle/spec/Doctrine/QueryItemExtension/OrderMethodsItemExtensionSpec.php b/src/Sylius/Bundle/ApiBundle/spec/Doctrine/QueryItemExtension/OrderMethodsItemExtensionSpec.php index 9de00494b28c..48c304610165 100644 --- a/src/Sylius/Bundle/ApiBundle/spec/Doctrine/QueryItemExtension/OrderMethodsItemExtensionSpec.php +++ b/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,