Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Default shipping and payment method resolver #11482

Open
wants to merge 13 commits into
base: 1.12
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/Sylius/Bundle/CoreBundle/Resources/config/services.xml
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,15 @@

<service id="sylius.payment_method_resolver.default" class="Sylius\Component\Core\Resolver\DefaultPaymentMethodResolver">
<argument type="service" id="sylius.repository.payment_method" />
<argument type="service" id="sylius.payment_methods_resolver" />
</service>
<service id="Sylius\Component\Payment\Resolver\DefaultPaymentMethodResolverInterface" alias="sylius.payment_method_resolver.default" />

<service id="sylius.shipping_method_resolver.default" class="Sylius\Component\Core\Resolver\EligibleDefaultShippingMethodResolver">
<argument type="service" id="sylius.repository.shipping_method" />
<argument type="service" id="sylius.shipping_method_eligibility_checker" />
<argument type="service" id="sylius.zone_matcher" />
<argument type="service" id="sylius.shipping_methods_resolver" />
</service>
<service id="Sylius\Component\Shipping\Resolver\DefaultShippingMethodResolverInterface" alias="sylius.shipping_method_resolver.default" />

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
<service id="sylius.shipping_method_resolver.default"
class="Sylius\Component\Shipping\Resolver\DefaultShippingMethodResolver">
<argument type="service" id="sylius.repository.shipping_method"/>
<argument type="service" id="sylius.shipping_methods_resolver" />
</service>
<service id="Sylius\Component\Shipping\Resolver\DefaultShippingMethodResolverInterface"
alias="sylius.shipping_method_resolver.default"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,37 +13,59 @@

namespace Sylius\Component\Core\Resolver;

use Sylius\Component\Core\Model\ChannelInterface;
use Sylius\Component\Core\Model\PaymentInterface;
use Sylius\Component\Core\Repository\PaymentMethodRepositoryInterface;
use Sylius\Component\Payment\Exception\UnresolvedDefaultPaymentMethodException;
use Sylius\Component\Payment\Model\PaymentInterface as BasePaymentInterface;
use Sylius\Component\Payment\Model\PaymentMethodInterface;
use Sylius\Component\Payment\Resolver\DefaultPaymentMethodResolverInterface;
use Sylius\Component\Payment\Resolver\PaymentMethodsResolverInterface;
use Webmozart\Assert\Assert;

class DefaultPaymentMethodResolver implements DefaultPaymentMethodResolverInterface
{
/** @var PaymentMethodRepositoryInterface */
protected $paymentMethodRepository;

public function __construct(PaymentMethodRepositoryInterface $paymentMethodRepository)
{
/** @var PaymentMethodsResolverInterface|null */
private $paymentMethodsResolver;
vvasiloi marked this conversation as resolved.
Show resolved Hide resolved

public function __construct(
PaymentMethodRepositoryInterface $paymentMethodRepository,
?PaymentMethodsResolverInterface $paymentMethodsResolver = null
) {
$this->paymentMethodRepository = $paymentMethodRepository;

if (null === $paymentMethodsResolver) {
@trigger_error(
sprintf(
'Not passing an $paymentMethodsResolver to "%s" constructor is deprecated since Sylius 1.8 and will be impossible in Sylius 2.0.',
self::class
),
\E_USER_DEPRECATED
);
}

$this->paymentMethodsResolver = $paymentMethodsResolver;
}

/**
* @param BasePaymentInterface|PaymentInterface $payment
*
* @throws UnresolvedDefaultPaymentMethodException
*/
public function getDefaultPaymentMethod(BasePaymentInterface $subject): PaymentMethodInterface
public function getDefaultPaymentMethod(BasePaymentInterface $payment): PaymentMethodInterface
{
/** @var PaymentInterface $subject */
Assert::isInstanceOf($subject, PaymentInterface::class);
Assert::isInstanceOf($payment, PaymentInterface::class);

$channel = $payment->getOrder()->getChannel();

/** @var ChannelInterface $channel */
$channel = $subject->getOrder()->getChannel();
if (null !== $this->paymentMethodsResolver) {
$paymentMethods = $this->paymentMethodsResolver->getSupportedMethods($payment);
} else {
$paymentMethods = $this->paymentMethodRepository->findEnabledForChannel($channel);
}

$paymentMethods = $this->paymentMethodRepository->findEnabledForChannel($channel);
if (empty($paymentMethods)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (empty($paymentMethods)) {
if (!$paymentMethods) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe count($paymentMethods) === 0?
Or use reset($paymentMethods) and check if it's false instead of relying on the numeric index ($paymentMethods[0]).

throw new UnresolvedDefaultPaymentMethodException();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,48 +17,104 @@
use Sylius\Component\Addressing\Model\ZoneInterface;
use Sylius\Component\Core\Model\AddressInterface;
use Sylius\Component\Core\Model\ChannelInterface;
use Sylius\Component\Core\Model\OrderInterface;
use Sylius\Component\Core\Model\ShipmentInterface as CoreShipmentInterface;
use Sylius\Component\Core\Repository\ShippingMethodRepositoryInterface;
use Sylius\Component\Shipping\Checker\Eligibility\ShippingMethodEligibilityCheckerInterface;
use Sylius\Component\Shipping\Exception\UnresolvedDefaultShippingMethodException;
use Sylius\Component\Shipping\Model\ShipmentInterface;
use Sylius\Component\Shipping\Model\ShippingMethodInterface;
use Sylius\Component\Shipping\Resolver\DefaultShippingMethodResolverInterface;
use Sylius\Component\Shipping\Resolver\ShippingMethodsResolverInterface;
use Webmozart\Assert\Assert;

final class EligibleDefaultShippingMethodResolver implements DefaultShippingMethodResolverInterface
{
/** @var ShippingMethodRepositoryInterface */
private $shippingMethodRepository;
private ?ShippingMethodRepositoryInterface $shippingMethodRepository;

/** @var ShippingMethodEligibilityCheckerInterface */
private $shippingMethodEligibilityChecker;
private ShippingMethodEligibilityCheckerInterface $shippingMethodEligibilityChecker;

/** @var ZoneMatcherInterface */
private $zoneMatcher;
private ZoneMatcherInterface $zoneMatcher;

private ?ShippingMethodsResolverInterface $shippingMethodsResolver;

public function __construct(
ShippingMethodRepositoryInterface $shippingMethodRepository,
?ShippingMethodRepositoryInterface $shippingMethodRepository = null,
ShippingMethodEligibilityCheckerInterface $shippingMethodEligibilityChecker,
ZoneMatcherInterface $zoneMatcher
ZoneMatcherInterface $zoneMatcher,
?ShippingMethodsResolverInterface $shippingMethodsResolver = null
) {
$this->shippingMethodRepository = $shippingMethodRepository;
$this->shippingMethodEligibilityChecker = $shippingMethodEligibilityChecker;
$this->zoneMatcher = $zoneMatcher;

Assert::false(
null === $shippingMethodRepository && null === $shippingMethodsResolver,
sprintf(
'You must pass to "%s" constructor either a $shippingMethodRepository, or a $shippingMethodsResolver, or both.',
__CLASS__,
)
);

if (null !== $shippingMethodRepository) {
@trigger_error(
sprintf(
'Passing an $shippingMethodRepository to "%s" constructor is deprecated since Sylius 1.9 and the argument will be removed in Sylius 2.0.',
self::class
),
\E_USER_DEPRECATED
);
}

if (null === $shippingMethodsResolver) {
@trigger_error(
sprintf(
'Not passing a $shippingMethodsResolver to "%s" constructor is deprecated since Sylius 1.9 and the argument will be required starting with Sylius 2.0.',
__CLASS__
),
\E_USER_DEPRECATED
);
}

$this->shippingMethodsResolver = $shippingMethodsResolver;
}

/**
* @param ShipmentInterface|CoreShipmentInterface $shipment
*
* @throws UnresolvedDefaultShippingMethodException
*/
public function getDefaultShippingMethod(ShipmentInterface $shipment): ShippingMethodInterface
{
/** @var CoreShipmentInterface $shipment */
Assert::isInstanceOf($shipment, CoreShipmentInterface::class);

/** @var OrderInterface $order */
$order = $shipment->getOrder();
/** @var ChannelInterface $channel */
$channel = $order->getChannel();
if (null !== $this->shippingMethodsResolver) {
return $this->getFromResolver($shipment);
}

return $this->getFromRepository($shipment);
}

$shippingMethods = $this->getShippingMethods($channel, $order->getShippingAddress());
/**
* @throws UnresolvedDefaultShippingMethodException
*/
private function getFromResolver(CoreShipmentInterface $shipment): ShippingMethodInterface
{
$shippingMethods = $this->shippingMethodsResolver->getSupportedMethods($shipment);

if (empty($shippingMethods)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (empty($shippingMethods)) {
if (!$shippingMethods) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

throw new UnresolvedDefaultShippingMethodException();
}

return $shippingMethods[0];
}

/**
* @throws UnresolvedDefaultShippingMethodException
*/
private function getFromRepository(CoreShipmentInterface $shipment): ShippingMethodInterface
{
$order = $shipment->getOrder();
$shippingMethods = $this->getShippingMethods($order->getChannel(), $order->getShippingAddress());

foreach ($shippingMethods as $shippingMethod) {
if ($this->shippingMethodEligibilityChecker->isEligible($shipment, $shippingMethod)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
use Sylius\Component\Shipping\Exception\UnresolvedDefaultShippingMethodException;
use Sylius\Component\Shipping\Model\ShipmentInterface as BaseShipmentInterface;
use Sylius\Component\Shipping\Resolver\DefaultShippingMethodResolverInterface;
use Sylius\Component\Shipping\Resolver\ShippingMethodsResolverInterface;

final class EligibleDefaultShippingMethodResolverSpec extends ObjectBehavior
{
Expand Down Expand Up @@ -135,4 +136,57 @@ function it_throws_an_exception_if_passed_shipment_is_not_core_shipment_object(B
{
$this->shouldThrow(InvalidArgumentException::class)->during('getDefaultShippingMethod', [$shipment]);
}

function it_returns_the_first_method_from_the_shipping_methods_resolver_when_passed(
ChannelInterface $channel,
OrderInterface $order,
ShipmentInterface $shipment,
ShippingMethodInterface $firstShippingMethod,
ShippingMethodInterface $secondShippingMethod,
ShippingMethodInterface $thirdShippingMethod,
ShippingMethodRepositoryInterface $shippingMethodRepository,
ShippingMethodEligibilityCheckerInterface $shippingMethodEligibilityChecker,
ZoneMatcherInterface $zoneMatcher,
ShippingMethodsResolverInterface $shippingMethodsResolver
): void {
$this->beConstructedWith(
$shippingMethodRepository,
$shippingMethodEligibilityChecker,
$zoneMatcher,
$shippingMethodsResolver
);

$shipment->getOrder()->willReturn($order);
$order->getChannel()->willReturn($channel);
$order->getShippingAddress()->willReturn(null);

$shippingMethodRepository
->findEnabledForChannel($channel)
->willReturn([$firstShippingMethod, $secondShippingMethod])
;

$shippingMethodEligibilityChecker->isEligible($shipment, $firstShippingMethod)->willReturn(false);
$shippingMethodEligibilityChecker->isEligible($shipment, $secondShippingMethod)->willReturn(true);
$shippingMethodEligibilityChecker->isEligible($shipment, $thirdShippingMethod)->willReturn(true);

$shippingMethodsResolver
->getSupportedMethods($shipment)
->willReturn([$thirdShippingMethod])
;

$this->getDefaultShippingMethod($shipment)->shouldReturn($thirdShippingMethod);
}

function it_throws_an_exception_if_neither_a_repository_nor_a_resolver_is_passed_to_the_constructor(
ShippingMethodEligibilityCheckerInterface $shippingMethodEligibilityChecker,
ZoneMatcherInterface $zoneMatcher
): void {
$this->beConstructedWith(
null,
$shippingMethodEligibilityChecker,
$zoneMatcher
);

$this->shouldThrow(InvalidArgumentException::class)->duringInstantiation();
}
}