chore(codebase): [di] modernize plugin#159
Conversation
BUT: not working for the moment as multiple service has been commented
maxperei
force-pushed
the
chore/modernize-dependency-injection
branch
from
32e2e16 to
c42fb78
Compare
| SymfonySetList::SYMFONY_63, | ||
| SymfonySetList::SYMFONY_64, | ||
| SymfonySetList::SYMFONY_71, | ||
| SymfonySetList::SYMFONY_72, |
There was a problem hiding this comment.
I think we shouldn't add set lists for sf 7.x, as the minimal version for sylius v2 is 6.4
| RelatedPaymentIdProviderInterface $relatedPaymentIdProvider, | ||
| TranslatorInterface $translator | ||
| private RequestStack $requestStack, | ||
| private ObjectRepository $refundPaymentRepository, |
There was a problem hiding this comment.
can we put at least ResourceRepositoryInterface instead of ObjectRepository ?
| * @ORM\OneToMany(targetEntity=Card::class, mappedBy="paymentMethod", orphanRemoval=true) | ||
| */ | ||
| /** @ORM\OneToMany(targetEntity=Card::class, mappedBy="paymentMethod", orphanRemoval=true) */ | ||
| #[ORM\OneToMany(targetEntity: Card::class, mappedBy: 'paymentMethod', orphanRemoval: true)] |
There was a problem hiding this comment.
To keep in mind, but Card may should be a supermappedclass and here we should reference an interface. Let's do that on another PR
| } | ||
|
|
||
| return false; | ||
| return $paymentResource->__isset('authorization') && $paymentResource->__get('authorization') instanceof PaymentAuthorization && null !== $paymentResource->__get('authorization')->__get('expires_at') && $now < $now->setTimestamp($paymentResource->__get('authorization')->__get('expires_at')); |
There was a problem hiding this comment.
for this one, i prefer the indent in the if. Maybe we can add some newline here
|
|
||
| use SyliusPluginTrait; | ||
|
|
||
| public const VERSION = '1.11.0'; |
There was a problem hiding this comment.
maybe we can remove this one ? or put 2.0.x-dev
| private RefundHistoryRepositoryInterface $payplugRefundHistoryRepository; | ||
|
|
||
| private PayPlugApiClientFactoryInterface $apiClientFactory; | ||
| public $payPlugApiClient; |
There was a problem hiding this comment.
can we add the interface here ? Why it is public ?
| use Sylius\RefundPlugin\Provider\RefundPaymentMethodsProviderInterface; | ||
| use Symfony\Component\DependencyInjection\Attribute\AsDecorator; | ||
|
|
||
| #[AsDecorator(ApplePaySupportedRefundPaymentMethodsProviderDecorator::class)] |
There was a problem hiding this comment.
weird that Amex decorate applePay
| use Sylius\RefundPlugin\Provider\RefundPaymentMethodsProviderInterface; | ||
| use Symfony\Component\DependencyInjection\Attribute\AsDecorator; | ||
|
|
||
| #[AsDecorator(BancontactSupportedRefundPaymentMethodsProviderDecorator::class)] |
| return $this->createQueryBuilder('o') | ||
| ->where('o.details LIKE :payplugPaymentId') | ||
| ->setParameter('payplugPaymentId', '%'.$payplugPaymentId.'%') | ||
| ->setParameter('payplugPaymentId', '%' . $payplugPaymentId . '%') |
There was a problem hiding this comment.
detail, but i think the % fit better in the where instead the setParameter
| @@ -11,28 +11,25 @@ | |||
|
|
|||
| final class GetCurrentRouteExtension extends AbstractExtension | |||
There was a problem hiding this comment.
detail, we should remove this and use {{ app.request.get('_route') }}
maxperei
force-pushed
the
chore/modernize-dependency-injection
branch
from
4b8fa41 to
99dd5a3
Compare
Jibbarth
left a comment
Jibbarth
left a comment
There was a problem hiding this comment.
This PR use huge enough. Let's merge and open new one if necessary
2 participants
No description provided.