Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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
}
}
4 changes: 3 additions & 1 deletion features/refunding_single_order_unit.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
includes:
- vendor/phpstan/phpstan-webmozart-assert/extension.neon

parameters:
excludes_analyse:
# Makes PHPStan crash
Expand Down
53 changes: 53 additions & 0 deletions spec/Provider/OfflineRefundPaymentMethodsProviderSpec.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php

declare(strict_types=1);

namespace spec\Sylius\RefundPlugin\Provider;

use Payum\Core\Model\GatewayConfigInterface;
use PhpSpec\ObjectBehavior;
use Sylius\Component\Core\Model\ChannelInterface;
use Sylius\Component\Core\Model\PaymentMethodInterface;
use Sylius\Component\Core\Repository\PaymentMethodRepositoryInterface;
use Sylius\RefundPlugin\Provider\RefundPaymentMethodsProviderInterface;

final class OfflineRefundPaymentMethodsProviderSpec extends ObjectBehavior
{
function let(PaymentMethodRepositoryInterface $paymentMethodRepository): void
{
$this->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]);
}
}
14 changes: 6 additions & 8 deletions src/Action/Admin/OrderRefundsListAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -23,8 +23,8 @@ final class OrderRefundsListAction
/** @var OrderRefundingAvailabilityCheckerInterface */
private $orderRefundingAvailabilityChecker;

/** @var PaymentMethodRepositoryInterface */
private $paymentMethodRepository;
/** @var RefundPaymentMethodsProviderInterface */
private $refundPaymentMethodsProvider;

/** @var Environment */
private $twig;
Expand All @@ -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;
Expand All @@ -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()),
])
);
}
Expand Down
2 changes: 2 additions & 0 deletions src/Action/Admin/RefundUnitsAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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());
Copy link
Contributor

@bartoszpietrzak1994 bartoszpietrzak1994 Oct 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using Assert::notNull() may result in throwing unhandled InvalidArgumentException - what about simple and more user friendly solution:

if (null === $exception->getPrevious()) {
    $this->session->getFlashBag()->add('error', 'sylius.ui.unexpected_error_occurred');
}

wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing is, there will always be a previous exception, as it's done automatically and silencing it could make more harm than good 😄 I'd leave it as it is (without it PHPStan is not passing)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok then 👍

$this->session->getFlashBag()->add('error', $exception->getPrevious()->getMessage());
} catch (\InvalidArgumentException $exception) {
$this->session->getFlashBag()->add('error', $exception->getMessage());
Expand Down
8 changes: 7 additions & 1 deletion src/Entity/CreditMemoUnit.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,17 @@ 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,
]);

if ($serialized === false) {
throw new \Exception('Credit memo unit could have not been serialized');
}

return $serialized;
}

public static function unserialize(string $serialized): self
Expand Down
33 changes: 33 additions & 0 deletions src/Provider/OfflineRefundPaymentMethodsProvider.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

declare(strict_types=1);

namespace Sylius\RefundPlugin\Provider;

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
{
/** @var PaymentMethodRepositoryInterface */
private $paymentMethodRepository;

public function __construct(PaymentMethodRepositoryInterface $paymentMethodRepository)
{
$this->paymentMethodRepository = $paymentMethodRepository;
}

public function findForChannel(ChannelInterface $channel): array
{
return array_filter(
$this->paymentMethodRepository->findEnabledForChannel($channel),
function (PaymentMethodInterface $paymentMethod): bool {
Assert::notNull($paymentMethod->getGatewayConfig());

return $paymentMethod->getGatewayConfig()->getFactoryName() === 'offline';
}
);
}
}
14 changes: 14 additions & 0 deletions src/Provider/RefundPaymentMethodsProviderInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

declare(strict_types=1);

namespace Sylius\RefundPlugin\Provider;

use Sylius\Component\Core\Model\ChannelInterface;
use Sylius\Component\Core\Model\PaymentMethodInterface;

interface RefundPaymentMethodsProviderInterface
{
/** @return array|PaymentMethodInterface[] */
public function findForChannel(ChannelInterface $channel): array;
}
2 changes: 1 addition & 1 deletion src/Resources/config/services/actions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
<service id="Sylius\RefundPlugin\Action\Admin\OrderRefundsListAction">
<argument type="service" id="sylius.repository.order" />
<argument type="service" id="Sylius\RefundPlugin\Checker\OrderRefundingAvailabilityChecker" />
<argument type="service" id="sylius.repository.payment_method" />
<argument type="service" id="Sylius\RefundPlugin\Provider\RefundPaymentMethodsProviderInterface" />
<argument type="service" id="twig" />
<argument type="service" id="session" />
<argument type="service" id="router" />
Expand Down
4 changes: 4 additions & 0 deletions src/Resources/config/services/provider.xml
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,9 @@
<service id="Sylius\RefundPlugin\Provider\RelatedPaymentIdProviderInterface" class="Sylius\RefundPlugin\Provider\DefaultRelatedPaymentIdProvider">
<argument type="service" id="sylius.repository.order" />
</service>

<service id="Sylius\RefundPlugin\Provider\RefundPaymentMethodsProviderInterface" class="Sylius\RefundPlugin\Provider\OfflineRefundPaymentMethodsProvider">
<argument type="service" id="sylius.repository.payment_method" />
</service>
</services>
</container>
8 changes: 8 additions & 0 deletions tests/Behat/Context/Ui/RefundingContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.';
Expand Down