From a9a1ad006cf3a0563cec99e31ec3863515ebadfa Mon Sep 17 00:00:00 2001 From: Mateusz Zalewski Date: Tue, 2 Oct 2018 14:11:12 +0200 Subject: [PATCH 1/4] Improve scenario to check existence of offline payment methods --- features/refunding_single_order_unit.feature | 4 +++- tests/Behat/Context/Ui/RefundingContext.php | 8 ++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/features/refunding_single_order_unit.feature b/features/refunding_single_order_unit.feature index 9cd1f840..a87c5cb1 100644 --- a/features/refunding_single_order_unit.feature +++ b/features/refunding_single_order_unit.feature @@ -57,13 +57,15 @@ Feature: Refunding a single order unit Then I should not be able to see refunds button @ui - Scenario: Being able to choose refund payment method + Scenario: Being able to choose only offline payment methods Given the order "#00000022" is already paid And the store allows paying with "Another offline payment method" + And the store has a payment method "ElonPay" with a code "IN_THRUST_WE_TRUST" and Paypal Express Checkout gateway When I want to refund some units of order "#00000022" Then I should be able to choose refund payment method And there should be "Space money" payment method And there should be "Another offline payment method" payment method + And there should not be "ElonPay" payment method @application Scenario: Not being able to refund unit from an order that is unpaid diff --git a/tests/Behat/Context/Ui/RefundingContext.php b/tests/Behat/Context/Ui/RefundingContext.php index da64caab..07547ad4 100644 --- a/tests/Behat/Context/Ui/RefundingContext.php +++ b/tests/Behat/Context/Ui/RefundingContext.php @@ -303,6 +303,14 @@ public function thereShouldBePaymentMethod(string $payment): void Assert::true($this->orderRefundsPage->isPaymentMethodVisible($payment)); } + /** + * @Then there should not be :payment payment method + */ + public function thereShouldNotBePaymentMethod(string $payment): void + { + Assert::false($this->orderRefundsPage->isPaymentMethodVisible($payment)); + } + private function provideLongComment(): string { return 'Tu ne quaesieris scire nefas, quem mihi quem tibi finem di dederint, Leuconoe, nec Babylonios temptaris numeros. Ut melius quidquid erit pati. Seu plures hiemes sue tribuit Iuppiter ultimam. Qae nunc oppositis debilitat pumicibus mare Tyrrenum: sapias vina liques et spatio brevi. Spem longam resecens. Dum loquimur fugerit invida Aetas: CARPE DIEM, quam minimum credula postero.'; From 7af31dbb18e22a4a83865387541bd485a4ac3497 Mon Sep 17 00:00:00 2001 From: Mateusz Zalewski Date: Tue, 2 Oct 2018 14:28:43 +0200 Subject: [PATCH 2/4] Provide only offline payment methods to be available for refund --- ...fflineRefundPaymentMethodsProviderSpec.php | 53 +++++++++++++++++++ src/Action/Admin/OrderRefundsListAction.php | 14 +++-- .../OfflineRefundPaymentMethodsProvider.php | 34 ++++++++++++ .../RefundPaymentMethodsProviderInterface.php | 14 +++++ src/Resources/config/services/actions.xml | 2 +- src/Resources/config/services/provider.xml | 4 ++ 6 files changed, 112 insertions(+), 9 deletions(-) create mode 100644 spec/Provider/OfflineRefundPaymentMethodsProviderSpec.php create mode 100644 src/Provider/OfflineRefundPaymentMethodsProvider.php create mode 100644 src/Provider/RefundPaymentMethodsProviderInterface.php diff --git a/spec/Provider/OfflineRefundPaymentMethodsProviderSpec.php b/spec/Provider/OfflineRefundPaymentMethodsProviderSpec.php new file mode 100644 index 00000000..30ba4cfd --- /dev/null +++ b/spec/Provider/OfflineRefundPaymentMethodsProviderSpec.php @@ -0,0 +1,53 @@ +beConstructedWith($paymentMethodRepository); + } + + function it_implements_refund_payment_methods_provider_interface(): void + { + $this->shouldImplement(RefundPaymentMethodsProviderInterface::class); + } + + function it_provides_only_offline_payment_methods( + PaymentMethodRepositoryInterface $paymentMethodRepository, + ChannelInterface $channel, + PaymentMethodInterface $offlinePaymentMethod, + PaymentMethodInterface $payPalPaymentMethod, + PaymentMethodInterface $stripePaymentMethod, + GatewayConfigInterface $offlineGatewayConfig, + GatewayConfigInterface $payPalGatewayConfig, + GatewayConfigInterface $stripeGatewayConfig + ): void { + $paymentMethodRepository->findEnabledForChannel($channel)->willReturn([ + $offlinePaymentMethod, + $payPalPaymentMethod, + $stripePaymentMethod, + ]); + + $offlinePaymentMethod->getGatewayConfig()->willReturn($offlineGatewayConfig); + $offlineGatewayConfig->getFactoryName()->willReturn('offline'); + + $payPalPaymentMethod->getGatewayConfig()->willReturn($payPalGatewayConfig); + $payPalGatewayConfig->getFactoryName()->willReturn('paypal'); + + $stripePaymentMethod->getGatewayConfig()->willReturn($stripeGatewayConfig); + $stripeGatewayConfig->getFactoryName()->willReturn('stripe'); + + $this->findForChannel($channel)->shouldReturn([$offlinePaymentMethod]); + } +} diff --git a/src/Action/Admin/OrderRefundsListAction.php b/src/Action/Admin/OrderRefundsListAction.php index 1742923c..89784061 100644 --- a/src/Action/Admin/OrderRefundsListAction.php +++ b/src/Action/Admin/OrderRefundsListAction.php @@ -6,8 +6,8 @@ use Sylius\Component\Core\Model\OrderInterface; use Sylius\Component\Core\Repository\OrderRepositoryInterface; -use Sylius\Component\Core\Repository\PaymentMethodRepositoryInterface; use Sylius\RefundPlugin\Checker\OrderRefundingAvailabilityCheckerInterface; +use Sylius\RefundPlugin\Provider\RefundPaymentMethodsProviderInterface; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -23,8 +23,8 @@ final class OrderRefundsListAction /** @var OrderRefundingAvailabilityCheckerInterface */ private $orderRefundingAvailabilityChecker; - /** @var PaymentMethodRepositoryInterface */ - private $paymentMethodRepository; + /** @var RefundPaymentMethodsProviderInterface */ + private $refundPaymentMethodsProvider; /** @var Environment */ private $twig; @@ -38,14 +38,14 @@ final class OrderRefundsListAction public function __construct( OrderRepositoryInterface $orderRepository, OrderRefundingAvailabilityCheckerInterface $orderRefundingAvailabilityChecker, - PaymentMethodRepositoryInterface $paymentMethodRepository, + RefundPaymentMethodsProviderInterface $refundPaymentMethodsProvider, Environment $twig, Session $session, UrlGeneratorInterface $router ) { $this->orderRepository = $orderRepository; $this->orderRefundingAvailabilityChecker = $orderRefundingAvailabilityChecker; - $this->paymentMethodRepository = $paymentMethodRepository; + $this->refundPaymentMethodsProvider = $refundPaymentMethodsProvider; $this->twig = $twig; $this->session = $session; $this->router = $router; @@ -60,12 +60,10 @@ public function __invoke(Request $request): Response return $this->redirectToReferer($order); } - $paymentMethods = $this->paymentMethodRepository->findEnabledForChannel($order->getChannel()); - return new Response( $this->twig->render('@SyliusRefundPlugin/orderRefunds.html.twig', [ 'order' => $order, - 'payment_methods' => $paymentMethods, + 'payment_methods' => $this->refundPaymentMethodsProvider->findForChannel($order->getChannel()), ]) ); } diff --git a/src/Provider/OfflineRefundPaymentMethodsProvider.php b/src/Provider/OfflineRefundPaymentMethodsProvider.php new file mode 100644 index 00000000..93d2ad09 --- /dev/null +++ b/src/Provider/OfflineRefundPaymentMethodsProvider.php @@ -0,0 +1,34 @@ +paymentMethodRepository = $paymentMethodRepository; + } + + public function findForChannel(ChannelInterface $channel): array + { + $paymentMethods = $this->paymentMethodRepository->findEnabledForChannel($channel); + + /** @var PaymentMethodInterface $paymentMethod */ + foreach ($paymentMethods as $key => $paymentMethod) { + if ($paymentMethod->getGatewayConfig()->getFactoryName() !== 'offline') { + unset($paymentMethods[$key]); + } + } + + return $paymentMethods; + } +} diff --git a/src/Provider/RefundPaymentMethodsProviderInterface.php b/src/Provider/RefundPaymentMethodsProviderInterface.php new file mode 100644 index 00000000..f0f27411 --- /dev/null +++ b/src/Provider/RefundPaymentMethodsProviderInterface.php @@ -0,0 +1,14 @@ + - + diff --git a/src/Resources/config/services/provider.xml b/src/Resources/config/services/provider.xml index 9add5e64..7f835333 100644 --- a/src/Resources/config/services/provider.xml +++ b/src/Resources/config/services/provider.xml @@ -31,5 +31,9 @@ + + + + From 6d8bf51665d96e0b2fb30fa9f71b399bfff876e3 Mon Sep 17 00:00:00 2001 From: Mateusz Zalewski Date: Wed, 3 Oct 2018 11:34:05 +0200 Subject: [PATCH 3/4] Fix phpstan --- composer.json | 6 +++++- phpstan.neon | 3 +++ src/Action/Admin/RefundUnitsAction.php | 2 ++ src/Entity/CreditMemoUnit.php | 4 +++- src/Provider/OfflineRefundPaymentMethodsProvider.php | 2 ++ 5 files changed, 15 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index bd318502..de66fd50 100644 --- a/composer.json +++ b/composer.json @@ -39,7 +39,8 @@ "friends-of-behat/variadic-extension": "^1.1", "lakion/mink-debug-extension": "^1.2.3", "phpspec/phpspec": "^4.0", - "phpstan/phpstan-shim": "^0.9.2", + "phpstan/phpstan-shim": "^0.10", + "phpstan/phpstan-webmozart-assert": "^0.10.0", "phpunit/phpunit": "^6.5", "sylius-labs/coding-standard": "^2.0", "symfony/browser-kit": "^3.4|^4.1", @@ -62,5 +63,8 @@ "vendor/bin/phpstan.phar analyse -c phpstan.neon -l max src/", "vendor/bin/ecs check src/ spec/" ] + }, + "config": { + "sort-packages": true } } diff --git a/phpstan.neon b/phpstan.neon index 98566716..5fb202c1 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,3 +1,6 @@ +includes: + - vendor/phpstan/phpstan-webmozart-assert/extension.neon + parameters: excludes_analyse: # Makes PHPStan crash diff --git a/src/Action/Admin/RefundUnitsAction.php b/src/Action/Admin/RefundUnitsAction.php index 5aa763dd..a8af4857 100644 --- a/src/Action/Admin/RefundUnitsAction.php +++ b/src/Action/Admin/RefundUnitsAction.php @@ -12,6 +12,7 @@ use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\Session\Session; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; +use Webmozart\Assert\Assert; final class RefundUnitsAction { @@ -46,6 +47,7 @@ public function __invoke(Request $request): Response $this->session->getFlashBag()->add('success', 'sylius_refund.units_successfully_refunded'); } catch (CommandDispatchException $exception) { + Assert::notNull($exception->getPrevious()); $this->session->getFlashBag()->add('error', $exception->getPrevious()->getMessage()); } catch (\InvalidArgumentException $exception) { $this->session->getFlashBag()->add('error', $exception->getMessage()); diff --git a/src/Entity/CreditMemoUnit.php b/src/Entity/CreditMemoUnit.php index c1610876..d3742457 100644 --- a/src/Entity/CreditMemoUnit.php +++ b/src/Entity/CreditMemoUnit.php @@ -40,11 +40,13 @@ public function getTaxesTotal(): int public function serialize(): string { - return json_encode([ + $serialized = json_encode([ 'product_name' => $this->productName, 'total' => $this->total, 'taxes_total' => $this->taxesTotal, ]); + + return ($serialized !== false) ? $serialized : ''; } public static function unserialize(string $serialized): self diff --git a/src/Provider/OfflineRefundPaymentMethodsProvider.php b/src/Provider/OfflineRefundPaymentMethodsProvider.php index 93d2ad09..4ac6c1a0 100644 --- a/src/Provider/OfflineRefundPaymentMethodsProvider.php +++ b/src/Provider/OfflineRefundPaymentMethodsProvider.php @@ -7,6 +7,7 @@ use Sylius\Component\Core\Model\ChannelInterface; use Sylius\Component\Core\Model\PaymentMethodInterface; use Sylius\Component\Core\Repository\PaymentMethodRepositoryInterface; +use Webmozart\Assert\Assert; final class OfflineRefundPaymentMethodsProvider implements RefundPaymentMethodsProviderInterface { @@ -24,6 +25,7 @@ public function findForChannel(ChannelInterface $channel): array /** @var PaymentMethodInterface $paymentMethod */ foreach ($paymentMethods as $key => $paymentMethod) { + Assert::notNull($paymentMethod->getGatewayConfig()); if ($paymentMethod->getGatewayConfig()->getFactoryName() !== 'offline') { unset($paymentMethods[$key]); } From a97c5f2a2aea7510b3d228dacbea6aa76eed9c94 Mon Sep 17 00:00:00 2001 From: Mateusz Zalewski Date: Wed, 3 Oct 2018 15:15:49 +0200 Subject: [PATCH 4/4] PR review fixes --- src/Entity/CreditMemoUnit.php | 6 +++++- .../OfflineRefundPaymentMethodsProvider.php | 15 ++++++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/Entity/CreditMemoUnit.php b/src/Entity/CreditMemoUnit.php index d3742457..1d297a33 100644 --- a/src/Entity/CreditMemoUnit.php +++ b/src/Entity/CreditMemoUnit.php @@ -46,7 +46,11 @@ public function serialize(): string 'taxes_total' => $this->taxesTotal, ]); - return ($serialized !== false) ? $serialized : ''; + if ($serialized === false) { + throw new \Exception('Credit memo unit could have not been serialized'); + } + + return $serialized; } public static function unserialize(string $serialized): self diff --git a/src/Provider/OfflineRefundPaymentMethodsProvider.php b/src/Provider/OfflineRefundPaymentMethodsProvider.php index 4ac6c1a0..1dcbc158 100644 --- a/src/Provider/OfflineRefundPaymentMethodsProvider.php +++ b/src/Provider/OfflineRefundPaymentMethodsProvider.php @@ -21,16 +21,13 @@ public function __construct(PaymentMethodRepositoryInterface $paymentMethodRepos public function findForChannel(ChannelInterface $channel): array { - $paymentMethods = $this->paymentMethodRepository->findEnabledForChannel($channel); + return array_filter( + $this->paymentMethodRepository->findEnabledForChannel($channel), + function (PaymentMethodInterface $paymentMethod): bool { + Assert::notNull($paymentMethod->getGatewayConfig()); - /** @var PaymentMethodInterface $paymentMethod */ - foreach ($paymentMethods as $key => $paymentMethod) { - Assert::notNull($paymentMethod->getGatewayConfig()); - if ($paymentMethod->getGatewayConfig()->getFactoryName() !== 'offline') { - unset($paymentMethods[$key]); + return $paymentMethod->getGatewayConfig()->getFactoryName() === 'offline'; } - } - - return $paymentMethods; + ); } }