From 72db586b7bc63d1ff5ce1ff403d43e45e0dd79ee Mon Sep 17 00:00:00 2001 From: adumont-payplug Date: Fri, 11 Sep 2026 15:12:16 +0200 Subject: [PATCH 1/5] PRE-3628: add per-channel gateway conflict checker --- src/Checker/GatewayChannelConflictChecker.php | 78 ++++++++ src/Repository/PaymentMethodRepository.php | 17 ++ .../PaymentMethodRepositoryInterface.php | 5 + .../GatewayChannelConflictCheckerTest.php | 177 ++++++++++++++++++ 4 files changed, 277 insertions(+) create mode 100644 src/Checker/GatewayChannelConflictChecker.php create mode 100644 tests/PHPUnit/Checker/GatewayChannelConflictCheckerTest.php diff --git a/src/Checker/GatewayChannelConflictChecker.php b/src/Checker/GatewayChannelConflictChecker.php new file mode 100644 index 00000000..a72197dd --- /dev/null +++ b/src/Checker/GatewayChannelConflictChecker.php @@ -0,0 +1,78 @@ + + */ + public function findConflicts(PaymentMethodInterface $paymentMethod, string $factoryName): array + { + $channelCodes = $this->channelCodes($paymentMethod); + + if (!$paymentMethod->isEnabled() || [] === $channelCodes) { + return []; + } + + $conflicts = []; + foreach ($this->paymentMethodRepository->findEnabledByGatewayName($factoryName) as $rival) { + if ($this->isSamePaymentMethod($paymentMethod, $rival) || !$rival->isEnabled()) { + continue; + } + + foreach ($rival->getChannels() as $rivalChannel) { + if ( + $rivalChannel instanceof ChannelInterface && + \in_array($rivalChannel->getCode(), $channelCodes, true) + ) { + $conflicts[] = ['channel' => $rivalChannel, 'paymentMethod' => $rival]; + } + } + } + + return $conflicts; + } + + /** + * @return list + */ + private function channelCodes(PaymentMethodInterface $paymentMethod): array + { + $channelCodes = []; + foreach ($paymentMethod->getChannels() as $channel) { + $channelCode = $channel->getCode(); + if (null !== $channelCode) { + $channelCodes[] = $channelCode; + } + } + + return $channelCodes; + } + + /** + * On creation the subject has no id yet, so it can never match a persisted rival. + */ + private function isSamePaymentMethod(PaymentMethodInterface $subject, PaymentMethodInterface $rival): bool + { + return null !== $subject->getId() && $subject->getId() === $rival->getId(); + } +} diff --git a/src/Repository/PaymentMethodRepository.php b/src/Repository/PaymentMethodRepository.php index d2e12daa..87b133e3 100644 --- a/src/Repository/PaymentMethodRepository.php +++ b/src/Repository/PaymentMethodRepository.php @@ -20,4 +20,21 @@ public function findOneByGatewayName(string $gatewayFactoryName): ?PaymentMethod ->getSingleResult() ; } + + public function findEnabledByGatewayName(string $gatewayFactoryName): array + { + /** @var array $paymentMethods */ + $paymentMethods = $this->createQueryBuilder('o') + ->innerJoin('o.gatewayConfig', 'gatewayConfig') + ->leftJoin('o.channels', 'channel') + ->addSelect('channel') + ->andWhere('gatewayConfig.factoryName = :gatewayFactoryName') + ->andWhere('o.enabled = true') + ->setParameter('gatewayFactoryName', $gatewayFactoryName) + ->getQuery() + ->getResult() + ; + + return $paymentMethods; + } } diff --git a/src/Repository/PaymentMethodRepositoryInterface.php b/src/Repository/PaymentMethodRepositoryInterface.php index 54f8c2e9..21b79171 100644 --- a/src/Repository/PaymentMethodRepositoryInterface.php +++ b/src/Repository/PaymentMethodRepositoryInterface.php @@ -10,4 +10,9 @@ interface PaymentMethodRepositoryInterface extends BasePaymentMethodRepositoryInterface { public function findOneByGatewayName(string $gatewayFactoryName): ?PaymentMethodInterface; + + /** + * @return array + */ + public function findEnabledByGatewayName(string $gatewayFactoryName): array; } diff --git a/tests/PHPUnit/Checker/GatewayChannelConflictCheckerTest.php b/tests/PHPUnit/Checker/GatewayChannelConflictCheckerTest.php new file mode 100644 index 00000000..178b78fd --- /dev/null +++ b/tests/PHPUnit/Checker/GatewayChannelConflictCheckerTest.php @@ -0,0 +1,177 @@ +paymentMethodRepository = $this->createMock(PaymentMethodRepositoryInterface::class); + $this->checker = new GatewayChannelConflictChecker($this->paymentMethodRepository); + } + + public function testFindConflicts_enabledRivalSharesChannel_isReported(): void + { + $subject = $this->paymentMethod(null, true, ['WEB_FR']); + $rival = $this->paymentMethod(7, true, ['WEB_FR'], 'CB 1'); + + $this->paymentMethodRepository + ->expects(self::once()) + ->method('findEnabledByGatewayName') + ->with(PayPlugGatewayFactory::FACTORY_NAME) + ->willReturn([$rival]) + ; + + $conflicts = $this->checker->findConflicts($subject, PayPlugGatewayFactory::FACTORY_NAME); + + self::assertCount(1, $conflicts); + self::assertSame('WEB_FR', $conflicts[0]['channel']->getCode()); + self::assertSame('CB 1', $conflicts[0]['paymentMethod']->getName()); + } + + public function testFindConflicts_channelSetsAreDisjoint_isAllowed(): void + { + $subject = $this->paymentMethod(null, true, ['WEB_FR']); + $rival = $this->paymentMethod(7, true, ['WEB_IT'], 'CB 1'); + + $this->paymentMethodRepository->method('findEnabledByGatewayName')->willReturn([$rival]); + + self::assertSame([], $this->checker->findConflicts($subject, PayPlugGatewayFactory::FACTORY_NAME)); + } + + public function testFindConflicts_twoSharedChannels_bothAreReported(): void + { + $subject = $this->paymentMethod(null, true, ['WEB_FR', 'WEB_IT', 'WEB_BE']); + $rival = $this->paymentMethod(7, true, ['WEB_IT', 'WEB_BE'], 'CB 1'); + + $this->paymentMethodRepository->method('findEnabledByGatewayName')->willReturn([$rival]); + + $conflicts = $this->checker->findConflicts($subject, PayPlugGatewayFactory::FACTORY_NAME); + + self::assertCount(2, $conflicts); + self::assertSame( + ['WEB_IT', 'WEB_BE'], + array_map(static fn (array $conflict): ?string => $conflict['channel']->getCode(), $conflicts), + ); + } + + public function testFindConflicts_subjectIsDisabled_isAllowedWithoutQuerying(): void + { + $subject = $this->paymentMethod(null, false, ['WEB_FR']); + + $this->paymentMethodRepository->expects(self::never())->method('findEnabledByGatewayName'); + + self::assertSame([], $this->checker->findConflicts($subject, PayPlugGatewayFactory::FACTORY_NAME)); + } + + public function testFindConflicts_subjectHasNoChannel_isAllowedWithoutQuerying(): void + { + $subject = $this->paymentMethod(null, true, []); + + $this->paymentMethodRepository->expects(self::never())->method('findEnabledByGatewayName'); + + self::assertSame([], $this->checker->findConflicts($subject, PayPlugGatewayFactory::FACTORY_NAME)); + } + + /** + * Editing an existing gateway must not make it conflict with itself. + */ + public function testFindConflicts_rivalIsTheSubjectItself_isAllowed(): void + { + $subject = $this->paymentMethod(7, true, ['WEB_FR']); + $itself = $this->paymentMethod(7, true, ['WEB_FR'], 'CB 1'); + + $this->paymentMethodRepository->method('findEnabledByGatewayName')->willReturn([$itself]); + + self::assertSame([], $this->checker->findConflicts($subject, PayPlugGatewayFactory::FACTORY_NAME)); + } + + /** + * The repository already filters on `enabled`, but the rule is re-asserted here so the whole + * rule is expressed — and testable — in one place. + */ + public function testFindConflicts_rivalIsDisabled_isAllowed(): void + { + $subject = $this->paymentMethod(null, true, ['WEB_FR']); + $rival = $this->paymentMethod(7, false, ['WEB_FR'], 'CB 1'); + + $this->paymentMethodRepository->method('findEnabledByGatewayName')->willReturn([$rival]); + + self::assertSame([], $this->checker->findConflicts($subject, PayPlugGatewayFactory::FACTORY_NAME)); + } + + public function testFindConflicts_rivalHasNoChannel_isAllowed(): void + { + $subject = $this->paymentMethod(null, true, ['WEB_FR']); + $rival = $this->paymentMethod(7, true, [], 'CB 1'); + + $this->paymentMethodRepository->method('findEnabledByGatewayName')->willReturn([$rival]); + + self::assertSame([], $this->checker->findConflicts($subject, PayPlugGatewayFactory::FACTORY_NAME)); + } + + /** + * Different factory types never conflict: the lookup is scoped to the factory being saved. + */ + public function testFindConflicts_queriesOnlyTheGivenFactory(): void + { + $subject = $this->paymentMethod(null, true, ['WEB_FR']); + + $this->paymentMethodRepository + ->expects(self::once()) + ->method('findEnabledByGatewayName') + ->with(OneyGatewayFactory::FACTORY_NAME) + ->willReturn([]) + ; + + self::assertSame([], $this->checker->findConflicts($subject, OneyGatewayFactory::FACTORY_NAME)); + } + + /** + * @param list $channelCodes + * + * @return PaymentMethodInterface&MockObject + */ + private function paymentMethod( + ?int $id, + bool $enabled, + array $channelCodes, + string $name = 'CB', + ): PaymentMethodInterface { + $channels = []; + foreach ($channelCodes as $channelCode) { + $channel = $this->createMock(ChannelInterface::class); + $channel->method('getCode')->willReturn($channelCode); + $channels[] = $channel; + } + + $paymentMethod = $this->createMock(PaymentMethodInterface::class); + $paymentMethod->method('getId')->willReturn($id); + $paymentMethod->method('isEnabled')->willReturn($enabled); + $paymentMethod->method('getName')->willReturn($name); + $paymentMethod->method('getChannels')->willReturn(new ArrayCollection($channels)); + + return $paymentMethod; + } +} From 686274a76bb9376a70b2d03791b11f30045936a3 Mon Sep 17 00:00:00 2001 From: adumont-payplug Date: Fri, 11 Sep 2026 15:26:14 +0200 Subject: [PATCH 2/5] PRE-3628: scope gateway uniqueness validation per channel --- ruleset/phpstan-baseline.neon | 6 - .../Extension/PaymentMethodTypeExtension.php | 138 ++++++++++++++++++ .../Type/AbstractGatewayConfigurationType.php | 38 ----- ...urationTypeExtensionFormSubmissionTest.php | 8 +- .../AbstractGatewayConfigurationTypeTest.php | 51 +------ .../PayPlugGatewayConfigurationTypeTest.php | 2 - translations/messages.en.yml | 4 +- translations/messages.fr.yml | 4 +- translations/messages.it.yml | 4 +- 9 files changed, 148 insertions(+), 107 deletions(-) create mode 100644 src/Gateway/Form/Extension/PaymentMethodTypeExtension.php diff --git a/ruleset/phpstan-baseline.neon b/ruleset/phpstan-baseline.neon index 7736db9b..90db5724 100644 --- a/ruleset/phpstan-baseline.neon +++ b/ruleset/phpstan-baseline.neon @@ -670,12 +670,6 @@ parameters: count: 1 path: ../src/Gateway/Form/Type/AbstractGatewayConfigurationType.php - - - message: '#^Cannot call method getId\(\) on mixed\.$#' - identifier: method.nonObject - count: 1 - path: ../src/Gateway/Form/Type/AbstractGatewayConfigurationType.php - - message: '#^Cannot access offset ''secretKey'' on mixed\.$#' identifier: offsetAccess.nonOffsetAccessible diff --git a/src/Gateway/Form/Extension/PaymentMethodTypeExtension.php b/src/Gateway/Form/Extension/PaymentMethodTypeExtension.php new file mode 100644 index 00000000..86b8016b --- /dev/null +++ b/src/Gateway/Form/Extension/PaymentMethodTypeExtension.php @@ -0,0 +1,138 @@ +addEventListener(FormEvents::POST_SUBMIT, function (FormEvent $event): void { + $form = $event->getForm(); + $paymentMethod = $this->resolvePayPlugPaymentMethod($form, $event->getData()); + + if (null === $paymentMethod) { + return; + } + + /** @var GatewayConfigInterface $gatewayConfig */ + $gatewayConfig = $paymentMethod->getGatewayConfig(); + + $this->addChannelConflictErrors($form, $paymentMethod, (string) $gatewayConfig->getFactoryName()); + }); + } + + public static function getExtendedTypes(): iterable + { + return [PaymentMethodType::class]; + } + + /** + * Returns the submitted payment method only when it is one of ours. + * + * "One of ours" is decided by the gateway configuration form's own type rather than by a + * hardcoded factory-name list: every PayPlug gateway configuration type extends + * `AbstractGatewayConfigurationType` and nothing else does, so the test stays exact when an + * eighth gateway is added. + */ + private function resolvePayPlugPaymentMethod(FormInterface $form, mixed $data): ?PaymentMethodInterface + { + if (!$data instanceof PaymentMethodInterface) { + return null; + } + + $gatewayConfig = $data->getGatewayConfig(); + + $isPayPlugPaymentMethod = $gatewayConfig instanceof GatewayConfigInterface && + null !== $gatewayConfig->getFactoryName() && + $this->hasPayPlugConfigurationType($form); + + return $isPayPlugPaymentMethod ? $data : null; + } + + /** + * `GatewayConfigType` only adds the `config` child when the factory has a registered + * configuration type, hence the `has()` guards. + */ + private function hasPayPlugConfigurationType(FormInterface $form): bool + { + if (!$form->has('gatewayConfig') || !$form->get('gatewayConfig')->has('config')) { + return false; + } + + $configurationType = $form->get('gatewayConfig')->get('config')->getConfig()->getType()->getInnerType(); + + return $configurationType instanceof AbstractGatewayConfigurationType; + } + + private function addChannelConflictErrors( + FormInterface $form, + PaymentMethodInterface $paymentMethod, + string $factoryName, + ): void { + if (!$form->has('channels')) { + return; + } + + $flashedMessages = []; + foreach ($this->conflictChecker->findConflicts($paymentMethod, $factoryName) as $conflict) { + $message = $this->translator->trans( + 'payplug_sylius_payplug_plugin.form.gateway_channel_conflict', + [ + '%channel%' => (string) $conflict['channel']->getCode(), + '%payment_method%' => (string) $conflict['paymentMethod']->getName(), + ], + ); + + $form->get('channels')->addError(new FormError($message)); + + if (!\in_array($message, $flashedMessages, true)) { + $flashedMessages[] = $message; + $this->flash($message); + } + } + } + + private function flash(string $message): void + { + $session = $this->requestStack->getSession(); + if ($session instanceof FlashBagAwareSessionInterface) { + $session->getFlashBag()->add('error', $message); + } + } +} diff --git a/src/Gateway/Form/Type/AbstractGatewayConfigurationType.php b/src/Gateway/Form/Type/AbstractGatewayConfigurationType.php index 168ebfbb..951b0325 100644 --- a/src/Gateway/Form/Type/AbstractGatewayConfigurationType.php +++ b/src/Gateway/Form/Type/AbstractGatewayConfigurationType.php @@ -6,16 +6,13 @@ use Doctrine\Common\Collections\Collection; use PayPlug\SyliusPayPlugPlugin\Gateway\PayPlugGatewayFactory; -use Sylius\Bundle\PayumBundle\Model\GatewayConfigInterface; use Sylius\Component\Core\Model\ChannelInterface; -use Sylius\Component\Resource\Repository\RepositoryInterface; use Symfony\Component\Form\AbstractType; use Symfony\Component\Form\Extension\Core\Type\CheckboxType; use Symfony\Component\Form\FormBuilderInterface; use Symfony\Component\Form\FormError; use Symfony\Component\Form\FormEvent; use Symfony\Component\Form\FormEvents; -use Symfony\Component\Form\FormInterface; use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Contracts\Translation\TranslatorInterface; @@ -31,7 +28,6 @@ class AbstractGatewayConfigurationType extends AbstractType public function __construct( protected TranslatorInterface $translator, - private RepositoryInterface $gatewayConfigRepository, protected RequestStack $requestStack, ) { } @@ -57,12 +53,6 @@ public function buildForm(FormBuilderInterface $builder, array $options): void 'required' => false, ]) ->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event): void { - $this->checkCreationRequirements( - $this->gatewayFactoryTitle, - $this->gatewayFactoryName, - $event->getForm(), - ); - /** @phpstan-ignore-next-line */ $formChannels = $event->getForm()->getParent()->getParent()->get('channels'); $dataFormChannels = $formChannels->getData(); @@ -96,34 +86,6 @@ public function buildForm(FormBuilderInterface $builder, array $options): void ; } - private function canBeCreated(string $factoryName): bool - { - $alreadyExists = $this->gatewayConfigRepository->findOneBy(['factoryName' => $factoryName]); - - return !$alreadyExists instanceof GatewayConfigInterface; - } - - private function checkCreationRequirements( - string $factoryTitle, - string $factoryName, - FormInterface $form, - ): void { - /** @phpstan-ignore-next-line */ - $paymentMethod = $form->getParent()->getParent()->getData(); - - if (null !== $paymentMethod->getId()) { - return; - } - - if ($this->canBeCreated($factoryName)) { - return; - } - - $message = $this->translator->trans('payplug_sylius_payplug_plugin.form.only_one_gateway_allowed', ['%gateway_title%' => $factoryTitle]); - /* @phpstan-ignore-next-line */ - $form->getParent()->getParent()->get('enabled')->addError(new FormError($message)); - } - /** * Hook for subtypes to scope the base-currency-per-channel restriction below. * Default: always enforced, preserving today's behavior for every gateway that doesn't diff --git a/tests/PHPUnit/Gateway/Form/Extension/PayPlugGatewayConfigurationTypeExtensionFormSubmissionTest.php b/tests/PHPUnit/Gateway/Form/Extension/PayPlugGatewayConfigurationTypeExtensionFormSubmissionTest.php index 861aad54..cc06267e 100644 --- a/tests/PHPUnit/Gateway/Form/Extension/PayPlugGatewayConfigurationTypeExtensionFormSubmissionTest.php +++ b/tests/PHPUnit/Gateway/Form/Extension/PayPlugGatewayConfigurationTypeExtensionFormSubmissionTest.php @@ -11,7 +11,6 @@ use PHPUnit\Framework\MockObject\MockObject; use Sylius\Component\Core\Model\ChannelInterface; use Sylius\Component\Currency\Model\CurrencyInterface; -use Sylius\Component\Resource\Repository\RepositoryInterface; use Symfony\Component\Form\Extension\Core\Type\FormType; use Symfony\Component\Form\Extension\Core\Type\TextType; use Symfony\Component\Form\Test\Traits\ValidatorExtensionTrait; @@ -46,16 +45,13 @@ protected function getTypes(): array $translator = $this->createMock(TranslatorInterface::class); $translator->method('trans')->willReturnCallback(static fn (string $id) => $id); - $gatewayConfigRepository = $this->createMock(RepositoryInterface::class); - $gatewayConfigRepository->method('findOneBy')->willReturn(null); - $request = new Request(); $request->setSession(new Session(new MockArraySessionStorage())); $requestStack = new RequestStack(); $requestStack->push($request); return [ - new PayPlugGatewayConfigurationType($translator, $gatewayConfigRepository, $requestStack), + new PayPlugGatewayConfigurationType($translator, $requestStack), ]; } @@ -330,8 +326,6 @@ private function createRootForm(?ArrayCollection $channels = null): \Symfony\Com $paymentMethod = new class() { public function getId(): ?int { - // Non-null so AbstractGatewayConfigurationType::checkCreationRequirements() - // short-circuits without needing a configured gatewayConfigRepository. return 1; } }; diff --git a/tests/PHPUnit/Gateway/Form/Type/AbstractGatewayConfigurationTypeTest.php b/tests/PHPUnit/Gateway/Form/Type/AbstractGatewayConfigurationTypeTest.php index 619aa70a..7fa32700 100644 --- a/tests/PHPUnit/Gateway/Form/Type/AbstractGatewayConfigurationTypeTest.php +++ b/tests/PHPUnit/Gateway/Form/Type/AbstractGatewayConfigurationTypeTest.php @@ -5,80 +5,35 @@ namespace Tests\PayPlug\SyliusPayPlugPlugin\PHPUnit\Gateway\Form\Type; use PayPlug\SyliusPayPlugPlugin\Gateway\Form\Type\AbstractGatewayConfigurationType; -use PayPlug\SyliusPayPlugPlugin\Gateway\OneyGatewayFactory; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Sylius\Bundle\PayumBundle\Model\GatewayConfigInterface; use Sylius\Component\Core\Model\ChannelInterface; -use Sylius\Component\Resource\Repository\RepositoryInterface; use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Contracts\Translation\TranslatorInterface; /** - * Covers the one-payment-method-per-gateway-factory rule enforced by canBeCreated(). + * Covers the hooks `AbstractGatewayConfigurationType` exposes to its per-gateway subtypes. * - * canBeCreated() is private and only reachable through the form PRE_SUBMIT listener, which would - * require mocking a full three-level form tree; it is therefore invoked here via reflection. + * The per-channel uniqueness rule they used to sit next to now lives in + * `GatewayChannelConflictChecker` and `PaymentMethodTypeExtension`. */ final class AbstractGatewayConfigurationTypeTest extends TestCase { - private RepositoryInterface&MockObject $gatewayConfigRepository; - private TranslatorInterface&MockObject $translator; private AbstractGatewayConfigurationType $type; protected function setUp(): void { - $this->gatewayConfigRepository = $this->createMock(RepositoryInterface::class); $this->translator = $this->createMock(TranslatorInterface::class); $this->translator->method('trans')->willReturnCallback(static fn (string $id) => $id); $this->type = new AbstractGatewayConfigurationType( $this->translator, - $this->gatewayConfigRepository, $this->createMock(RequestStack::class), ); } - /** - * Every PayPlug-family factory, including `payplug` itself, is limited to one PaymentMethod. - */ - public function testCanBeCreated_otherFactoryAlreadyConfigured_isRefused(): void - { - $this->gatewayConfigRepository - ->expects(self::once()) - ->method('findOneBy') - ->with(['factoryName' => OneyGatewayFactory::FACTORY_NAME]) - ->willReturn($this->createMock(GatewayConfigInterface::class)) - ; - - self::assertFalse($this->canBeCreated(OneyGatewayFactory::FACTORY_NAME)); - } - - public function testCanBeCreated_otherFactoryNotYetConfigured_isAllowed(): void - { - $this->gatewayConfigRepository - ->expects(self::once()) - ->method('findOneBy') - ->with(['factoryName' => OneyGatewayFactory::FACTORY_NAME]) - ->willReturn(null) - ; - - self::assertTrue($this->canBeCreated(OneyGatewayFactory::FACTORY_NAME)); - } - - private function canBeCreated(string $factoryName): bool - { - $method = new \ReflectionMethod(AbstractGatewayConfigurationType::class, 'canBeCreated'); - $method->setAccessible(true); - - /** @var bool $result */ - $result = $method->invoke($this->type, $factoryName); - - return $result; - } - /** * Default hook implementation: every gateway subtype that doesn't override it keeps * today's behavior of always enforcing the base currency. diff --git a/tests/PHPUnit/Gateway/Form/Type/PayPlugGatewayConfigurationTypeTest.php b/tests/PHPUnit/Gateway/Form/Type/PayPlugGatewayConfigurationTypeTest.php index 882ac793..ad7553f3 100644 --- a/tests/PHPUnit/Gateway/Form/Type/PayPlugGatewayConfigurationTypeTest.php +++ b/tests/PHPUnit/Gateway/Form/Type/PayPlugGatewayConfigurationTypeTest.php @@ -9,7 +9,6 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Sylius\Component\Core\Model\ChannelInterface; -use Sylius\Component\Resource\Repository\RepositoryInterface; use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Contracts\Translation\TranslatorInterface; @@ -26,7 +25,6 @@ protected function setUp(): void $this->type = new PayPlugGatewayConfigurationType( $this->translator, - $this->createMock(RepositoryInterface::class), $this->createMock(RequestStack::class), ); } diff --git a/translations/messages.en.yml b/translations/messages.en.yml index a486cfb3..ad257614 100644 --- a/translations/messages.en.yml +++ b/translations/messages.en.yml @@ -141,8 +141,8 @@ payplug_sylius_payplug_plugin: submit: Confirm and continue base_currency_not_euro: | Channel #channel_code#: #payment_method# is only available on channels with EURO as a currency - only_one_gateway_allowed: | - Please note that the %gateway_title% payment method has already been set. To change it, go to your payment methods. + gateway_channel_conflict: | + Channel %channel% is already linked to the enabled payment method "%payment_method%". Disable it, or remove that channel from one of the two. one_click_enable: Enable One click one_click_help: | Allow your customers to save their credit card details for later diff --git a/translations/messages.fr.yml b/translations/messages.fr.yml index 4c17f7ad..20792f3b 100644 --- a/translations/messages.fr.yml +++ b/translations/messages.fr.yml @@ -161,8 +161,8 @@ payplug_sylius_payplug_plugin: submit: Valider et continuer base_currency_not_euro: | Canal #channel_code# : #payment_method# n’est disponible que sur des canaux dont la devise est l’EURO - only_one_gateway_allowed: | - Attention, le moyen de paiement %gateway_title% existe déjà. Pour le modifier, rendez-vous sur vos moyens de paiement. + gateway_channel_conflict: | + Le canal %channel% est déjà rattaché au moyen de paiement actif « %payment_method% ». Désactivez-le, ou retirez ce canal de l'un des deux. one_click_enable: Activer le One click one_click_help: | Permettez à vos clients d'enregistrer leurs coordonnées de carte de paiement pour effectuer ultérieurement diff --git a/translations/messages.it.yml b/translations/messages.it.yml index 16e60374..1dc84f2f 100644 --- a/translations/messages.it.yml +++ b/translations/messages.it.yml @@ -141,8 +141,8 @@ payplug_sylius_payplug_plugin: submit: Convalida e continua base_currency_not_euro: | Il canale #channel_code# : #payment_method# è disponibile solo per i canali la cui valuta è in EURO - only_one_gateway_allowed: | - Attenzione: il metodo di pagamento %gateway_title% è già definito. Per modificarlo, vai ai tuoi metodi di pagamento. + gateway_channel_conflict: | + Il canale %channel% è già collegato al metodo di pagamento attivo "%payment_method%". Disattivalo oppure rimuovi quel canale da uno dei due. one_click_enable: Attiva un clic one_click_help: | Consenti ai tuoi clienti di salvare i dettagli della loro carta di credito per dopo From b8f4e1941afbd3584fbeb6b0f883b658b4ba8931 Mon Sep 17 00:00:00 2001 From: adumont-payplug Date: Fri, 11 Sep 2026 15:51:58 +0200 Subject: [PATCH 3/5] PRE-3628: validate base currency against submitted channels --- ruleset/phpstan-baseline.neon | 6 - .../Extension/PaymentMethodTypeExtension.php | 52 +++++- .../Type/AbstractGatewayConfigurationType.php | 51 ++---- .../Type/PayPlugGatewayConfigurationType.php | 8 +- ...urationTypeExtensionFormSubmissionTest.php | 167 ++---------------- .../AbstractGatewayConfigurationTypeTest.php | 46 ++--- .../PayPlugGatewayConfigurationTypeTest.php | 2 - 7 files changed, 98 insertions(+), 234 deletions(-) diff --git a/ruleset/phpstan-baseline.neon b/ruleset/phpstan-baseline.neon index 90db5724..2e4ae26f 100644 --- a/ruleset/phpstan-baseline.neon +++ b/ruleset/phpstan-baseline.neon @@ -664,12 +664,6 @@ parameters: count: 1 path: ../src/Gateway/AbstractGatewayFactory.php - - - message: '#^Cannot call method add\(\) on mixed\.$#' - identifier: method.nonObject - count: 1 - path: ../src/Gateway/Form/Type/AbstractGatewayConfigurationType.php - - message: '#^Cannot access offset ''secretKey'' on mixed\.$#' identifier: offsetAccess.nonOffsetAccessible diff --git a/src/Gateway/Form/Extension/PaymentMethodTypeExtension.php b/src/Gateway/Form/Extension/PaymentMethodTypeExtension.php index 86b8016b..608eadc0 100644 --- a/src/Gateway/Form/Extension/PaymentMethodTypeExtension.php +++ b/src/Gateway/Form/Extension/PaymentMethodTypeExtension.php @@ -7,6 +7,7 @@ use PayPlug\SyliusPayPlugPlugin\Checker\GatewayChannelConflictChecker; use PayPlug\SyliusPayPlugPlugin\Gateway\Form\Type\AbstractGatewayConfigurationType; use Sylius\Bundle\PaymentBundle\Form\Type\PaymentMethodType; +use Sylius\Component\Core\Model\ChannelInterface; use Sylius\Component\Core\Model\PaymentMethodInterface; use Sylius\Component\Payment\Model\GatewayConfigInterface; use Symfony\Component\Form\AbstractTypeExtension; @@ -54,6 +55,7 @@ public function buildForm(FormBuilderInterface $builder, array $options): void $gatewayConfig = $paymentMethod->getGatewayConfig(); $this->addChannelConflictErrors($form, $paymentMethod, (string) $gatewayConfig->getFactoryName()); + $this->addBaseCurrencyErrors($form, $paymentMethod, $gatewayConfig); }); } @@ -80,7 +82,7 @@ private function resolvePayPlugPaymentMethod(FormInterface $form, mixed $data): $isPayPlugPaymentMethod = $gatewayConfig instanceof GatewayConfigInterface && null !== $gatewayConfig->getFactoryName() && - $this->hasPayPlugConfigurationType($form); + null !== $this->resolveConfigurationType($form); return $isPayPlugPaymentMethod ? $data : null; } @@ -88,16 +90,21 @@ private function resolvePayPlugPaymentMethod(FormInterface $form, mixed $data): /** * `GatewayConfigType` only adds the `config` child when the factory has a registered * configuration type, hence the `has()` guards. + * + * The per-gateway currency policy is read back off the configuration type instance rather than + * duplicated into a registry here: it already lives one-class-per-gateway, and only the CB type + * narrows it (to Integrated Payment). Form types are stateless services, so calling their + * public hooks is safe. */ - private function hasPayPlugConfigurationType(FormInterface $form): bool + private function resolveConfigurationType(FormInterface $form): ?AbstractGatewayConfigurationType { if (!$form->has('gatewayConfig') || !$form->get('gatewayConfig')->has('config')) { - return false; + return null; } $configurationType = $form->get('gatewayConfig')->get('config')->getConfig()->getType()->getInnerType(); - return $configurationType instanceof AbstractGatewayConfigurationType; + return $configurationType instanceof AbstractGatewayConfigurationType ? $configurationType : null; } private function addChannelConflictErrors( @@ -128,6 +135,43 @@ private function addChannelConflictErrors( } } + private function addBaseCurrencyErrors( + FormInterface $form, + PaymentMethodInterface $paymentMethod, + GatewayConfigInterface $gatewayConfig, + ): void { + $configurationType = $this->resolveConfigurationType($form); + + if ( + !$form->has('channels') || + null === $configurationType || + !$configurationType->shouldValidateBaseCurrency($gatewayConfig->getConfig()) + ) { + return; + } + + $flashedMessages = []; + foreach ($paymentMethod->getChannels() as $channel) { + if (!$channel instanceof ChannelInterface) { + continue; + } + + $baseCurrency = $channel->getBaseCurrency(); + + if (null === $baseCurrency || $configurationType->getBaseCurrencyCode() === $baseCurrency->getCode()) { + continue; + } + + $message = $configurationType->baseCurrencyViolationMessage($channel); + $form->get('channels')->addError(new FormError($message)); + + if (!\in_array($message, $flashedMessages, true)) { + $flashedMessages[] = $message; + $this->flash($message); + } + } + } + private function flash(string $message): void { $session = $this->requestStack->getSession(); diff --git a/src/Gateway/Form/Type/AbstractGatewayConfigurationType.php b/src/Gateway/Form/Type/AbstractGatewayConfigurationType.php index 951b0325..c659359f 100644 --- a/src/Gateway/Form/Type/AbstractGatewayConfigurationType.php +++ b/src/Gateway/Form/Type/AbstractGatewayConfigurationType.php @@ -4,16 +4,11 @@ namespace PayPlug\SyliusPayPlugPlugin\Gateway\Form\Type; -use Doctrine\Common\Collections\Collection; use PayPlug\SyliusPayPlugPlugin\Gateway\PayPlugGatewayFactory; use Sylius\Component\Core\Model\ChannelInterface; use Symfony\Component\Form\AbstractType; use Symfony\Component\Form\Extension\Core\Type\CheckboxType; use Symfony\Component\Form\FormBuilderInterface; -use Symfony\Component\Form\FormError; -use Symfony\Component\Form\FormEvent; -use Symfony\Component\Form\FormEvents; -use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Contracts\Translation\TranslatorInterface; class AbstractGatewayConfigurationType extends AbstractType @@ -28,7 +23,6 @@ class AbstractGatewayConfigurationType extends AbstractType public function __construct( protected TranslatorInterface $translator, - protected RequestStack $requestStack, ) { } @@ -52,50 +46,20 @@ public function buildForm(FormBuilderInterface $builder, array $options): void 'mapped' => false, 'required' => false, ]) - ->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event): void { - /** @phpstan-ignore-next-line */ - $formChannels = $event->getForm()->getParent()->getParent()->get('channels'); - $dataFormChannels = $formChannels->getData(); - if (!$dataFormChannels instanceof Collection) { - return; - } - - $rawData = $event->getData(); - if (!\is_array($rawData) || !$this->shouldValidateBaseCurrency($rawData)) { - return; - } - - $flashedMessages = []; - /** @var ChannelInterface $dataFormChannel */ - foreach ($dataFormChannels as $key => $dataFormChannel) { - $baseCurrency = $dataFormChannel->getBaseCurrency(); - if (null === $baseCurrency) { - continue; - } - $baseCurrencyCode = $baseCurrency->getCode(); - if ($this->gatewayBaseCurrencyCode !== $baseCurrencyCode) { - $message = $this->baseCurrencyViolationMessage($dataFormChannel); - $formChannels->get((string) $key)->addError(new FormError($message)); - if (!\in_array($message, $flashedMessages, true)) { - $flashedMessages[] = $message; - $this->requestStack->getSession()->getFlashBag()->add('error', $message); - } - } - } - }) ; } /** - * Hook for subtypes to scope the base-currency-per-channel restriction below. + * Hook for subtypes to scope the base-currency-per-channel restriction enforced by + * PaymentMethodTypeExtension. * Default: always enforced, preserving today's behavior for every gateway that doesn't * override this (Bancontact, American Express, Scalapay, Wero, Oney...). * * @see baseCurrencyViolationMessage() Companion hook customizing the message this guards. * - * @param array $rawFormData Raw PRE_SUBMIT data of the gateway config form. + * @param array $gatewayConfig Mapped gateway configuration, as stored on GatewayConfig. */ - protected function shouldValidateBaseCurrency(array $rawFormData): bool + public function shouldValidateBaseCurrency(array $gatewayConfig): bool { return true; } @@ -107,7 +71,7 @@ protected function shouldValidateBaseCurrency(array $rawFormData): bool * * @see shouldValidateBaseCurrency() Companion hook scoping when this message is used. */ - protected function baseCurrencyViolationMessage(ChannelInterface $channel): string + public function baseCurrencyViolationMessage(ChannelInterface $channel): string { return $this->translator->trans( 'payplug_sylius_payplug_plugin.form.base_currency_not_euro', @@ -117,4 +81,9 @@ protected function baseCurrencyViolationMessage(ChannelInterface $channel): stri ], ); } + + public function getBaseCurrencyCode(): string + { + return $this->gatewayBaseCurrencyCode; + } } diff --git a/src/Gateway/Form/Type/PayPlugGatewayConfigurationType.php b/src/Gateway/Form/Type/PayPlugGatewayConfigurationType.php index 1344b3d1..6ce1bfea 100644 --- a/src/Gateway/Form/Type/PayPlugGatewayConfigurationType.php +++ b/src/Gateway/Form/Type/PayPlugGatewayConfigurationType.php @@ -28,11 +28,11 @@ final class PayPlugGatewayConfigurationType extends AbstractGatewayConfiguration * Only `integrated_payment` requires every associated channel to be EUR; the redirected * and `hosted_fields` display modes both work in any currency. * - * @param array $rawFormData + * @param array $gatewayConfig */ - protected function shouldValidateBaseCurrency(array $rawFormData): bool + public function shouldValidateBaseCurrency(array $gatewayConfig): bool { - return PayPlugGatewayFactory::DISPLAY_MODE_INTEGRATED_PAYMENT === ($rawFormData[PayPlugGatewayFactory::DISPLAY_MODE_FIELD] ?? null); + return PayPlugGatewayFactory::DISPLAY_MODE_INTEGRATED_PAYMENT === ($gatewayConfig[PayPlugGatewayFactory::DISPLAY_MODE_FIELD] ?? null); } /** @@ -40,7 +40,7 @@ protected function shouldValidateBaseCurrency(array $rawFormData): bool * (redirected/hosted_fields both return false there), so this message can be specific to * that mode rather than the generic per-gateway wording. */ - protected function baseCurrencyViolationMessage(ChannelInterface $channel): string + public function baseCurrencyViolationMessage(ChannelInterface $channel): string { return $this->translator->trans('payplug_sylius_payplug_plugin.form.integrated_payment_currency_incompatible'); } diff --git a/tests/PHPUnit/Gateway/Form/Extension/PayPlugGatewayConfigurationTypeExtensionFormSubmissionTest.php b/tests/PHPUnit/Gateway/Form/Extension/PayPlugGatewayConfigurationTypeExtensionFormSubmissionTest.php index cc06267e..ebc4732a 100644 --- a/tests/PHPUnit/Gateway/Form/Extension/PayPlugGatewayConfigurationTypeExtensionFormSubmissionTest.php +++ b/tests/PHPUnit/Gateway/Form/Extension/PayPlugGatewayConfigurationTypeExtensionFormSubmissionTest.php @@ -4,21 +4,13 @@ namespace Tests\PayPlug\SyliusPayPlugPlugin\PHPUnit\Gateway\Form\Extension; -use Doctrine\Common\Collections\ArrayCollection; use PayPlug\SyliusPayPlugPlugin\Gateway\Form\Extension\PayPlugGatewayConfigurationTypeExtension; use PayPlug\SyliusPayPlugPlugin\Gateway\Form\Type\PayPlugGatewayConfigurationType; use PayPlug\SyliusPayPlugPlugin\Gateway\PayPlugGatewayFactory; -use PHPUnit\Framework\MockObject\MockObject; -use Sylius\Component\Core\Model\ChannelInterface; -use Sylius\Component\Currency\Model\CurrencyInterface; use Symfony\Component\Form\Extension\Core\Type\FormType; use Symfony\Component\Form\Extension\Core\Type\TextType; use Symfony\Component\Form\Test\Traits\ValidatorExtensionTrait; use Symfony\Component\Form\Test\TypeTestCase; -use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpFoundation\RequestStack; -use Symfony\Component\HttpFoundation\Session\Session; -use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage; use Symfony\Contracts\Translation\TranslatorInterface; /** @@ -30,9 +22,13 @@ * * This test exercises the real Symfony form lifecycle (via TypeTestCase, no mocked * FormBuilderInterface) end-to-end, including a realistic 3-level parent chain - * (root -> gatewayConfig -> config), because the extended type's own inherited - * AbstractGatewayConfigurationType::buildForm() PRE_SUBMIT listener walks - * getParent()->getParent() to reach the payment method entity and its "channels" field. + * (root -> gatewayConfig -> config). + * + * The base-currency-per-channel check that used to also be exercised here (via + * AbstractGatewayConfigurationType's own PRE_SUBMIT listener) has moved to + * PaymentMethodTypeExtension's POST_SUBMIT listener on the root PaymentMethodType form; it is + * covered by AbstractGatewayConfigurationTypeTest and PayPlugGatewayConfigurationTypeTest's hook + * tests instead, since this file's minimal 3-level tree doesn't wire up PaymentMethodTypeExtension. */ final class PayPlugGatewayConfigurationTypeExtensionFormSubmissionTest extends TypeTestCase { @@ -45,13 +41,8 @@ protected function getTypes(): array $translator = $this->createMock(TranslatorInterface::class); $translator->method('trans')->willReturnCallback(static fn (string $id) => $id); - $request = new Request(); - $request->setSession(new Session(new MockArraySessionStorage())); - $requestStack = new RequestStack(); - $requestStack->push($request); - return [ - new PayPlugGatewayConfigurationType($translator, $requestStack), + new PayPlugGatewayConfigurationType($translator), ]; } @@ -224,104 +215,12 @@ public function testSubmit_hostedFieldsModeWithALeftoverSubMerchantIdInStoredCon self::assertTrue($form->isValid()); } - /** - * PRE-3553: selecting a non-EUR channel while `integrated_payment` is selected must be - * rejected with a message specific to this feature ("...not compatible with Integrated - * Payment"), not the generic per-gateway `base_currency_not_euro` wording every other - * PayPlug-family gateway subtype still uses (Bancontact, American Express, Scalapay...). - */ - public function testSubmit_integratedPaymentModeWithNonEurChannel_isInvalidWithCurrencyIncompatibleMessage(): void - { - $form = $this->createRootForm($this->buildChannels(['USD'])); - - // clearMissing=false: "channels" isn't part of this submitted payload (only - // gatewayConfig.config is), and the default clearMissing=true would otherwise call - // submit(null) on it regardless - wiping the Collection set via createRootForm() before - // the currency-check listener ever runs, even though it's not disabled. - $form->submit([ - 'gatewayConfig' => [ - 'config' => [ - PayPlugGatewayFactory::ONE_CLICK => false, - PayPlugGatewayFactory::DEFERRED_CAPTURE => false, - PayPlugGatewayFactory::DISPLAY_MODE_FIELD => PayPlugGatewayFactory::DISPLAY_MODE_INTEGRATED_PAYMENT, - PayPlugGatewayFactory::HF_IDENTIFIER => '', - ], - ], - ], false); - - self::assertFalse($form->isValid(), 'Form must be invalid when integrated_payment is selected but an associated channel is not EUR.'); - - // The error is added to the specific channel's own child sub-form (mirroring the real - // `channels` field being `multiple => true, expanded => true`, one child per channel), - // not directly to the "channels" form itself. - $channelErrors = $form->get('channels')->get('0')->getErrors(); - self::assertCount(1, $channelErrors); - self::assertSame( - 'payplug_sylius_payplug_plugin.form.integrated_payment_currency_incompatible', - $channelErrors[0]->getMessage(), - ); - } - - /** - * The same non-EUR channel must NOT be rejected for hosted_fields or redirected mode — only - * integrated_payment requires every associated channel to be EUR. - */ - public function testSubmit_hostedFieldsModeWithNonEurChannel_isValid(): void - { - $form = $this->createRootForm($this->buildChannels(['USD'])); - - $form->submit([ - 'gatewayConfig' => [ - 'config' => [ - PayPlugGatewayFactory::ONE_CLICK => false, - PayPlugGatewayFactory::DEFERRED_CAPTURE => false, - PayPlugGatewayFactory::DISPLAY_MODE_FIELD => PayPlugGatewayFactory::DISPLAY_MODE_HOSTED_FIELDS, - PayPlugGatewayFactory::HF_IDENTIFIER => 'acct_123', - ], - ], - ], false); - - self::assertTrue($form->isValid()); - } - - /** - * @param list $currencyCodes - * - * @return ArrayCollection - */ - private function buildChannels(array $currencyCodes): ArrayCollection - { - $channels = []; - foreach ($currencyCodes as $index => $currencyCode) { - $currency = $this->createMock(CurrencyInterface::class); - $currency->method('getCode')->willReturn($currencyCode); - - /** @var ChannelInterface&MockObject $channel */ - $channel = $this->createMock(ChannelInterface::class); - $channel->method('getCode')->willReturn('channel_' . $index); - $channel->method('getBaseCurrency')->willReturn($currency); - - $channels[] = $channel; - } - - return new ArrayCollection($channels); - } - /** * Builds a minimal but realistic 3-level tree: root (the PaymentMethod form, exposing * "channels") -> gatewayConfig -> config (PayPlugGatewayConfigurationType, the type under - * test). This mirrors production nesting closely enough to exercise - * AbstractGatewayConfigurationType's inherited PRE_SUBMIT listener (which the extended type - * still carries) without it fatal-erroring on missing parents. - * - * @param ArrayCollection|null $channels Real channel data for the - * "channels" field, needed by - * tests exercising the currency - * check. Left null (an unset, - * non-Collection field) for tests - * that don't care about it. + * test). */ - private function createRootForm(?ArrayCollection $channels = null): \Symfony\Component\Form\FormInterface + private function createRootForm(): \Symfony\Component\Form\FormInterface { $paymentMethod = new class() { public function getId(): ?int @@ -331,55 +230,13 @@ public function getId(): ?int }; $root = $this->factory->createBuilder(FormType::class, $paymentMethod, ['data_class' => null]); - if (null !== $channels) { - // A bare FormType (no data_class) round-trips setData()/getData() untouched - unlike - // TextType, it has no model-to-view transformer that would choke on a Collection. It - // needs one child per channel, named by its collection key, because the production - // currency-check listener does `$formChannels->get((string) $key)->addError(...)` - - // mirroring the real `channels` field being a `multiple => true, expanded => true` - // ChoiceType, which creates one child sub-form per choice - and, critically, sets - // `error_bubbling => false` on those children (ChoiceType.php), unlike a bare - // FormType's default of bubbling errors up to its parent when compound. Without this, - // addError() on a channel's sub-form bubbles all the way to the root instead of - // staying on that sub-form - purely a test-double mismatch, not a production concern. - // NOTE: this field is NOT `disabled => true` - Form::isValid() unconditionally returns - // true for a disabled form regardless of its errors, and Form::getErrors(true) skips - // any child that isSubmitted() && isValid() when aggregating - together those two - // rules mean a disabled "channels" would make the whole root form always report valid - // no matter what error is added deep inside it. Its pre-set data survives submission - // instead via `$form->submit($data, false)` (clearMissing=false) at the call site, - // which is not disabled but also isn't reset by an absent key. - $channelsBuilder = $root->create('channels', FormType::class, [ - 'mapped' => false, - 'data_class' => null, - ]); - foreach ($channels as $key => $channel) { - $channelsBuilder->add((string) $key, FormType::class, [ - 'mapped' => false, - 'data_class' => null, - 'error_bubbling' => false, - ]); - } - $root->add($channelsBuilder); - } else { - $root->add('channels', TextType::class, ['mapped' => false]); - } + $root->add('channels', TextType::class, ['mapped' => false]); $gatewayConfig = $root->create('gatewayConfig', FormType::class, ['mapped' => false]); $gatewayConfig->add('config', PayPlugGatewayConfigurationType::class); $root->add($gatewayConfig); - $form = $root->getForm(); - if (null !== $channels) { - // Force the root's own lazy defaultDataSet initialization (and its mapDataToForms - // cascade, which would otherwise reset the unmapped "channels" field to null the - // first time anything touches this form) to run now, BEFORE setting "channels"'s - // real data below - so our setData() call is the last word, not overwritten by it. - $form->getData(); - $form->get('channels')->setData($channels); - } - - return $form; + return $root->getForm(); } } diff --git a/tests/PHPUnit/Gateway/Form/Type/AbstractGatewayConfigurationTypeTest.php b/tests/PHPUnit/Gateway/Form/Type/AbstractGatewayConfigurationTypeTest.php index 7fa32700..bfb7878e 100644 --- a/tests/PHPUnit/Gateway/Form/Type/AbstractGatewayConfigurationTypeTest.php +++ b/tests/PHPUnit/Gateway/Form/Type/AbstractGatewayConfigurationTypeTest.php @@ -5,10 +5,11 @@ namespace Tests\PayPlug\SyliusPayPlugPlugin\PHPUnit\Gateway\Form\Type; use PayPlug\SyliusPayPlugPlugin\Gateway\Form\Type\AbstractGatewayConfigurationType; +use PayPlug\SyliusPayPlugPlugin\Gateway\Form\Type\PayPlugGatewayConfigurationType; +use PayPlug\SyliusPayPlugPlugin\Gateway\PayPlugGatewayFactory; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Sylius\Component\Core\Model\ChannelInterface; -use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Contracts\Translation\TranslatorInterface; /** @@ -30,7 +31,6 @@ protected function setUp(): void $this->type = new AbstractGatewayConfigurationType( $this->translator, - $this->createMock(RequestStack::class), ); } @@ -40,19 +40,8 @@ protected function setUp(): void */ public function testShouldValidateBaseCurrency_defaultImplementation_alwaysReturnsTrue(): void { - self::assertTrue($this->shouldValidateBaseCurrency([])); - self::assertTrue($this->shouldValidateBaseCurrency(['anything' => 'irrelevant'])); - } - - private function shouldValidateBaseCurrency(array $data): bool - { - $method = new \ReflectionMethod(AbstractGatewayConfigurationType::class, 'shouldValidateBaseCurrency'); - $method->setAccessible(true); - - /** @var bool $result */ - $result = $method->invoke($this->type, $data); - - return $result; + self::assertTrue($this->type->shouldValidateBaseCurrency([])); + self::assertTrue($this->type->shouldValidateBaseCurrency(['anything' => 'irrelevant'])); } /** @@ -67,18 +56,31 @@ public function testBaseCurrencyViolationMessage_defaultImplementation_returnsGe self::assertSame( 'payplug_sylius_payplug_plugin.form.base_currency_not_euro', - $this->baseCurrencyViolationMessage($channel), + $this->type->baseCurrencyViolationMessage($channel), ); } - private function baseCurrencyViolationMessage(ChannelInterface $channel): string + public function testGetBaseCurrencyCode_defaultImplementation_isEuro(): void { - $method = new \ReflectionMethod(AbstractGatewayConfigurationType::class, 'baseCurrencyViolationMessage'); - $method->setAccessible(true); + self::assertSame('EUR', $this->type->getBaseCurrencyCode()); + } - /** @var string $result */ - $result = $method->invoke($this->type, $channel); + /** + * The only subtype that narrows the hook: Integrated Payment is the only display mode that + * requires every associated channel to be EUR. + */ + public function testShouldValidateBaseCurrency_payPlugType_onlyAppliesToIntegratedPayment(): void + { + $type = new PayPlugGatewayConfigurationType( + $this->translator, + ); - return $result; + self::assertTrue($type->shouldValidateBaseCurrency([ + PayPlugGatewayFactory::DISPLAY_MODE_FIELD => PayPlugGatewayFactory::DISPLAY_MODE_INTEGRATED_PAYMENT, + ])); + self::assertFalse($type->shouldValidateBaseCurrency([ + PayPlugGatewayFactory::DISPLAY_MODE_FIELD => PayPlugGatewayFactory::DISPLAY_MODE_HOSTED_FIELDS, + ])); + self::assertFalse($type->shouldValidateBaseCurrency([])); } } diff --git a/tests/PHPUnit/Gateway/Form/Type/PayPlugGatewayConfigurationTypeTest.php b/tests/PHPUnit/Gateway/Form/Type/PayPlugGatewayConfigurationTypeTest.php index ad7553f3..aab4bdb2 100644 --- a/tests/PHPUnit/Gateway/Form/Type/PayPlugGatewayConfigurationTypeTest.php +++ b/tests/PHPUnit/Gateway/Form/Type/PayPlugGatewayConfigurationTypeTest.php @@ -9,7 +9,6 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Sylius\Component\Core\Model\ChannelInterface; -use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Contracts\Translation\TranslatorInterface; final class PayPlugGatewayConfigurationTypeTest extends TestCase @@ -25,7 +24,6 @@ protected function setUp(): void $this->type = new PayPlugGatewayConfigurationType( $this->translator, - $this->createMock(RequestStack::class), ); } From d85a3d492fa80d93e78a069fe55b4f8a2b9566ca Mon Sep 17 00:00:00 2001 From: adumont-payplug Date: Fri, 11 Sep 2026 16:03:36 +0200 Subject: [PATCH 4/5] PRE-3628: fix CB base-currency gate to read mapped config --- .../Extension/PaymentMethodTypeExtension.php | 30 +-- .../Type/PayPlugGatewayConfigurationType.php | 10 +- .../PaymentMethodTypeExtensionTest.php | 181 ++++++++++++++++++ .../AbstractGatewayConfigurationTypeTest.php | 8 +- .../PayPlugGatewayConfigurationTypeTest.php | 10 +- 5 files changed, 218 insertions(+), 21 deletions(-) create mode 100644 tests/PHPUnit/Gateway/Form/Extension/PaymentMethodTypeExtensionTest.php diff --git a/src/Gateway/Form/Extension/PaymentMethodTypeExtension.php b/src/Gateway/Form/Extension/PaymentMethodTypeExtension.php index 608eadc0..c32061a7 100644 --- a/src/Gateway/Form/Extension/PaymentMethodTypeExtension.php +++ b/src/Gateway/Form/Extension/PaymentMethodTypeExtension.php @@ -45,9 +45,10 @@ public function buildForm(FormBuilderInterface $builder, array $options): void { $builder->addEventListener(FormEvents::POST_SUBMIT, function (FormEvent $event): void { $form = $event->getForm(); - $paymentMethod = $this->resolvePayPlugPaymentMethod($form, $event->getData()); + $configurationType = $this->resolveConfigurationType($form); + $paymentMethod = $this->resolvePayPlugPaymentMethod($event->getData()); - if (null === $paymentMethod) { + if (null === $configurationType || null === $paymentMethod) { return; } @@ -55,7 +56,7 @@ public function buildForm(FormBuilderInterface $builder, array $options): void $gatewayConfig = $paymentMethod->getGatewayConfig(); $this->addChannelConflictErrors($form, $paymentMethod, (string) $gatewayConfig->getFactoryName()); - $this->addBaseCurrencyErrors($form, $paymentMethod, $gatewayConfig); + $this->addBaseCurrencyErrors($form, $paymentMethod, $gatewayConfig, $configurationType); }); } @@ -65,14 +66,11 @@ public static function getExtendedTypes(): iterable } /** - * Returns the submitted payment method only when it is one of ours. - * - * "One of ours" is decided by the gateway configuration form's own type rather than by a - * hardcoded factory-name list: every PayPlug gateway configuration type extends - * `AbstractGatewayConfigurationType` and nothing else does, so the test stays exact when an - * eighth gateway is added. + * Returns the submitted payment method only when it carries a gateway config with a factory + * name; the listener pairs this with `resolveConfigurationType()` to decide whether the + * payment method is one of ours. */ - private function resolvePayPlugPaymentMethod(FormInterface $form, mixed $data): ?PaymentMethodInterface + private function resolvePayPlugPaymentMethod(mixed $data): ?PaymentMethodInterface { if (!$data instanceof PaymentMethodInterface) { return null; @@ -81,13 +79,17 @@ private function resolvePayPlugPaymentMethod(FormInterface $form, mixed $data): $gatewayConfig = $data->getGatewayConfig(); $isPayPlugPaymentMethod = $gatewayConfig instanceof GatewayConfigInterface && - null !== $gatewayConfig->getFactoryName() && - null !== $this->resolveConfigurationType($form); + null !== $gatewayConfig->getFactoryName(); return $isPayPlugPaymentMethod ? $data : null; } /** + * Resolving to non-null is what makes a submitted payment method "one of ours": the decision + * rests on the gateway configuration form's own type rather than on a hardcoded factory-name + * list, since every PayPlug gateway configuration type extends `AbstractGatewayConfigurationType` + * and nothing else does, so the test stays exact when an eighth gateway is added. + * * `GatewayConfigType` only adds the `config` child when the factory has a registered * configuration type, hence the `has()` guards. * @@ -139,12 +141,10 @@ private function addBaseCurrencyErrors( FormInterface $form, PaymentMethodInterface $paymentMethod, GatewayConfigInterface $gatewayConfig, + AbstractGatewayConfigurationType $configurationType, ): void { - $configurationType = $this->resolveConfigurationType($form); - if ( !$form->has('channels') || - null === $configurationType || !$configurationType->shouldValidateBaseCurrency($gatewayConfig->getConfig()) ) { return; diff --git a/src/Gateway/Form/Type/PayPlugGatewayConfigurationType.php b/src/Gateway/Form/Type/PayPlugGatewayConfigurationType.php index 6ce1bfea..9a5c69b9 100644 --- a/src/Gateway/Form/Type/PayPlugGatewayConfigurationType.php +++ b/src/Gateway/Form/Type/PayPlugGatewayConfigurationType.php @@ -28,11 +28,17 @@ final class PayPlugGatewayConfigurationType extends AbstractGatewayConfiguration * Only `integrated_payment` requires every associated channel to be EUR; the redirected * and `hosted_fields` display modes both work in any currency. * - * @param array $gatewayConfig + * The mode is read back through `resolveDisplayMode()` rather than off a display-mode key: + * `DISPLAY_MODE_FIELD` is an unmapped admin form field and never reaches the persisted config, + * which instead carries the two `INTEGRATED_PAYMENT`/`HOSTED_FIELDS` booleans written by + * `resolveDisplayModeFlags()`. Going through the canonical reader also inherits its + * hosted-fields-wins tie-break when both flags are somehow true. + * + * @param array $gatewayConfig Mapped gateway configuration, as stored on GatewayConfig. */ public function shouldValidateBaseCurrency(array $gatewayConfig): bool { - return PayPlugGatewayFactory::DISPLAY_MODE_INTEGRATED_PAYMENT === ($gatewayConfig[PayPlugGatewayFactory::DISPLAY_MODE_FIELD] ?? null); + return PayPlugGatewayFactory::DISPLAY_MODE_INTEGRATED_PAYMENT === PayPlugGatewayFactory::resolveDisplayMode($gatewayConfig); } /** diff --git a/tests/PHPUnit/Gateway/Form/Extension/PaymentMethodTypeExtensionTest.php b/tests/PHPUnit/Gateway/Form/Extension/PaymentMethodTypeExtensionTest.php new file mode 100644 index 00000000..ff4c0f4d --- /dev/null +++ b/tests/PHPUnit/Gateway/Form/Extension/PaymentMethodTypeExtensionTest.php @@ -0,0 +1,181 @@ +translator = $this->createMock(TranslatorInterface::class); + $this->translator->method('trans')->willReturnCallback(static fn (string $id) => $id); + } + + /** + * PRE-3553: Integrated Payment is EUR-only, and the error must land on the `channels` field. + */ + public function testPostSubmit_integratedPaymentOnNonEurChannel_addsCurrencyErrorOnChannels(): void + { + self::assertSame( + ['payplug_sylius_payplug_plugin.form.integrated_payment_currency_incompatible'], + $this->submitPayPlugPaymentMethod([PayPlugGatewayFactory::INTEGRATED_PAYMENT => true], 'USD'), + ); + } + + public function testPostSubmit_integratedPaymentOnEuroChannel_addsNoError(): void + { + self::assertSame( + [], + $this->submitPayPlugPaymentMethod([PayPlugGatewayFactory::INTEGRATED_PAYMENT => true], 'EUR'), + ); + } + + /** + * The CB gate narrows the check to Integrated Payment: Hosted Fields works in any currency. + */ + public function testPostSubmit_hostedFieldsOnNonEurChannel_addsNoError(): void + { + self::assertSame( + [], + $this->submitPayPlugPaymentMethod([PayPlugGatewayFactory::HOSTED_FIELDS => true], 'USD'), + ); + } + + /** + * Runs the extension's POST_SUBMIT listener over a CB payment method carrying a single channel. + * + * @param array $mappedGatewayConfig as returned by GatewayConfigInterface::getConfig() + * + * @return list messages of the FormErrors added to the `channels` child + */ + private function submitPayPlugPaymentMethod(array $mappedGatewayConfig, string $baseCurrencyCode): array + { + $currency = $this->createMock(CurrencyInterface::class); + $currency->method('getCode')->willReturn($baseCurrencyCode); + + $channel = $this->createMock(ChannelInterface::class); + $channel->method('getCode')->willReturn('channel_code'); + $channel->method('getBaseCurrency')->willReturn($currency); + + $gatewayConfig = $this->createMock(GatewayConfigInterface::class); + $gatewayConfig->method('getFactoryName')->willReturn(PayPlugGatewayFactory::FACTORY_NAME); + $gatewayConfig->method('getConfig')->willReturn($mappedGatewayConfig); + + $paymentMethod = $this->createMock(PaymentMethodInterface::class); + $paymentMethod->method('getGatewayConfig')->willReturn($gatewayConfig); + $paymentMethod->method('getChannels')->willReturn(new ArrayCollection([$channel])); + + $errors = []; + $channelsForm = $this->createMock(FormInterface::class); + $channelsForm->method('addError')->willReturnCallback( + function (FormError $error) use (&$errors, $channelsForm): FormInterface { + $errors[] = $error->getMessage(); + + return $channelsForm; + }, + ); + + ($this->capturePostSubmitListener())(new FormEvent($this->buildRootForm($channelsForm), $paymentMethod)); + + return $errors; + } + + /** + * Minimal stand-in for the `paymentMethod` → `gatewayConfig` → `config` form tree the extension + * walks to resolve the gateway's configuration type. + */ + private function buildRootForm(FormInterface $channelsForm): FormInterface + { + $resolvedType = $this->createMock(ResolvedFormTypeInterface::class); + $resolvedType->method('getInnerType')->willReturn(new PayPlugGatewayConfigurationType($this->translator)); + + $formConfig = $this->createMock(FormConfigInterface::class); + $formConfig->method('getType')->willReturn($resolvedType); + + $configForm = $this->createMock(FormInterface::class); + $configForm->method('getConfig')->willReturn($formConfig); + + $gatewayConfigForm = $this->createMock(FormInterface::class); + $gatewayConfigForm->method('has')->willReturnCallback(static fn (string $name): bool => 'config' === $name); + $gatewayConfigForm->method('get')->willReturn($configForm); + + $form = $this->createMock(FormInterface::class); + $form->method('has')->willReturnCallback( + static fn (string $name): bool => \in_array($name, ['gatewayConfig', 'channels'], true), + ); + $form->method('get')->willReturnCallback( + static fn (string $name): FormInterface => 'channels' === $name ? $channelsForm : $gatewayConfigForm, + ); + + return $form; + } + + private function capturePostSubmitListener(): callable + { + $repository = $this->createMock(PaymentMethodRepositoryInterface::class); + $repository->method('findEnabledByGatewayName')->willReturn([]); + + $extension = new PaymentMethodTypeExtension( + new GatewayChannelConflictChecker($repository), + $this->translator, + $this->createMock(RequestStack::class), + ); + + $listener = null; + $builder = $this->createMock(FormBuilderInterface::class); + $builder->method('addEventListener')->willReturnCallback( + function (string $eventName, callable $callback) use (&$listener, $builder): FormBuilderInterface { + if (FormEvents::POST_SUBMIT === $eventName) { + $listener = $callback; + } + + return $builder; + }, + ); + + $extension->buildForm($builder, []); + + self::assertIsCallable($listener); + + return $listener; + } +} diff --git a/tests/PHPUnit/Gateway/Form/Type/AbstractGatewayConfigurationTypeTest.php b/tests/PHPUnit/Gateway/Form/Type/AbstractGatewayConfigurationTypeTest.php index bfb7878e..1f8eb048 100644 --- a/tests/PHPUnit/Gateway/Form/Type/AbstractGatewayConfigurationTypeTest.php +++ b/tests/PHPUnit/Gateway/Form/Type/AbstractGatewayConfigurationTypeTest.php @@ -68,6 +68,10 @@ public function testGetBaseCurrencyCode_defaultImplementation_isEuro(): void /** * The only subtype that narrows the hook: Integrated Payment is the only display mode that * requires every associated channel to be EUR. + * + * Keyed by the persisted `integratedPayment`/`hostedFields` booleans, which is what the caller + * passes (`GatewayConfigInterface::getConfig()`); the `hostedFieldsMode` form field is unmapped + * and never appears in that array. */ public function testShouldValidateBaseCurrency_payPlugType_onlyAppliesToIntegratedPayment(): void { @@ -76,10 +80,10 @@ public function testShouldValidateBaseCurrency_payPlugType_onlyAppliesToIntegrat ); self::assertTrue($type->shouldValidateBaseCurrency([ - PayPlugGatewayFactory::DISPLAY_MODE_FIELD => PayPlugGatewayFactory::DISPLAY_MODE_INTEGRATED_PAYMENT, + PayPlugGatewayFactory::INTEGRATED_PAYMENT => true, ])); self::assertFalse($type->shouldValidateBaseCurrency([ - PayPlugGatewayFactory::DISPLAY_MODE_FIELD => PayPlugGatewayFactory::DISPLAY_MODE_HOSTED_FIELDS, + PayPlugGatewayFactory::HOSTED_FIELDS => true, ])); self::assertFalse($type->shouldValidateBaseCurrency([])); } diff --git a/tests/PHPUnit/Gateway/Form/Type/PayPlugGatewayConfigurationTypeTest.php b/tests/PHPUnit/Gateway/Form/Type/PayPlugGatewayConfigurationTypeTest.php index aab4bdb2..8edacc1e 100644 --- a/tests/PHPUnit/Gateway/Form/Type/PayPlugGatewayConfigurationTypeTest.php +++ b/tests/PHPUnit/Gateway/Form/Type/PayPlugGatewayConfigurationTypeTest.php @@ -27,17 +27,23 @@ protected function setUp(): void ); } + /** + * The arrays below are keyed by the persisted `integratedPayment`/`hostedFields` booleans + * written by `PayPlugGatewayFactory::resolveDisplayModeFlags()`, i.e. the shape the production + * caller hands the hook (`GatewayConfigInterface::getConfig()`). The `hostedFieldsMode` form + * field is unmapped and never reaches the persisted config. + */ public function testShouldValidateBaseCurrency_integratedPaymentSelected_returnsTrue(): void { self::assertTrue($this->shouldValidateBaseCurrency([ - PayPlugGatewayFactory::DISPLAY_MODE_FIELD => PayPlugGatewayFactory::DISPLAY_MODE_INTEGRATED_PAYMENT, + PayPlugGatewayFactory::INTEGRATED_PAYMENT => true, ])); } public function testShouldValidateBaseCurrency_hostedFieldsSelected_returnsFalse(): void { self::assertFalse($this->shouldValidateBaseCurrency([ - PayPlugGatewayFactory::DISPLAY_MODE_FIELD => PayPlugGatewayFactory::DISPLAY_MODE_HOSTED_FIELDS, + PayPlugGatewayFactory::HOSTED_FIELDS => true, ])); } From 293e7737bdac3b001a329b931e5e1d384eaf495c Mon Sep 17 00:00:00 2001 From: adumont-payplug Date: Fri, 11 Sep 2026 16:55:36 +0200 Subject: [PATCH 5/5] PRE-3628: address final review polish items - de-dup CB base-currency form errors, not just flashes - flash() no longer throws with no request/session - drop 8 dead gatewayFactoryName property declarations - suppress PHPMD unused-param on shouldValidateBaseCurrency() - assert PaymentMethodTypeExtension::getExtendedTypes() - tighten PaymentMethodRepository docblocks to list<> --- ruleset/phpstan-baseline.neon | 6 ++++ .../Extension/PaymentMethodTypeExtension.php | 33 ++++++++++++++----- .../Type/AbstractGatewayConfigurationType.php | 4 +-- ...mericanExpressGatewayConfigurationType.php | 2 -- .../Type/ApplePayGatewayConfigurationType.php | 2 -- .../BancontactGatewayConfigurationType.php | 2 -- .../Type/OneyGatewayConfigurationType.php | 2 -- .../Type/PayPlugGatewayConfigurationType.php | 2 -- .../Type/ScalapayGatewayConfigurationType.php | 2 -- .../Type/WeroGatewayConfigurationType.php | 2 -- src/Repository/PaymentMethodRepository.php | 5 ++- .../PaymentMethodRepositoryInterface.php | 2 +- .../PaymentMethodTypeExtensionTest.php | 10 ++++++ 13 files changed, 48 insertions(+), 26 deletions(-) diff --git a/ruleset/phpstan-baseline.neon b/ruleset/phpstan-baseline.neon index 2e4ae26f..76179e7d 100644 --- a/ruleset/phpstan-baseline.neon +++ b/ruleset/phpstan-baseline.neon @@ -664,6 +664,12 @@ parameters: count: 1 path: ../src/Gateway/AbstractGatewayFactory.php + - + message: '#^PHPDoc tag @SuppressWarnings has invalid value \(\(PHPMD\.UnusedFormalParameter\)\)\: Unexpected token "\.UnusedFormalParameter\)", expected ''\)'' at offset 554 on line 11$#' + identifier: phpDoc.parseError + count: 1 + path: ../src/Gateway/Form/Type/AbstractGatewayConfigurationType.php + - message: '#^Cannot access offset ''secretKey'' on mixed\.$#' identifier: offsetAccess.nonOffsetAccessible diff --git a/src/Gateway/Form/Extension/PaymentMethodTypeExtension.php b/src/Gateway/Form/Extension/PaymentMethodTypeExtension.php index c32061a7..8e679713 100644 --- a/src/Gateway/Form/Extension/PaymentMethodTypeExtension.php +++ b/src/Gateway/Form/Extension/PaymentMethodTypeExtension.php @@ -18,6 +18,7 @@ use Symfony\Component\Form\FormInterface; use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\HttpFoundation\Session\FlashBagAwareSessionInterface; +use Symfony\Component\HttpFoundation\Session\SessionInterface; use Symfony\Contracts\Translation\TranslatorInterface; /** @@ -150,7 +151,7 @@ private function addBaseCurrencyErrors( return; } - $flashedMessages = []; + $seenMessages = []; foreach ($paymentMethod->getChannels() as $channel) { if (!$channel instanceof ChannelInterface) { continue; @@ -163,20 +164,36 @@ private function addBaseCurrencyErrors( } $message = $configurationType->baseCurrencyViolationMessage($channel); - $form->get('channels')->addError(new FormError($message)); - if (!\in_array($message, $flashedMessages, true)) { - $flashedMessages[] = $message; - $this->flash($message); + if (\in_array($message, $seenMessages, true)) { + continue; } + + $seenMessages[] = $message; + $form->get('channels')->addError(new FormError($message)); + $this->flash($message); } } private function flash(string $message): void { - $session = $this->requestStack->getSession(); - if ($session instanceof FlashBagAwareSessionInterface) { - $session->getFlashBag()->add('error', $message); + $session = $this->resolveSession(); + + if (!$session instanceof FlashBagAwareSessionInterface) { + return; + } + + $session->getFlashBag()->add('error', $message); + } + + private function resolveSession(): ?SessionInterface + { + $request = $this->requestStack->getCurrentRequest(); + + if (null === $request || !$request->hasSession()) { + return null; } + + return $request->getSession(); } } diff --git a/src/Gateway/Form/Type/AbstractGatewayConfigurationType.php b/src/Gateway/Form/Type/AbstractGatewayConfigurationType.php index c659359f..867dbcab 100644 --- a/src/Gateway/Form/Type/AbstractGatewayConfigurationType.php +++ b/src/Gateway/Form/Type/AbstractGatewayConfigurationType.php @@ -17,8 +17,6 @@ class AbstractGatewayConfigurationType extends AbstractType protected string $gatewayFactoryTitle = ''; - protected string $gatewayFactoryName = ''; - protected string $gatewayBaseCurrencyCode = PayPlugGatewayFactory::BASE_CURRENCY_CODE; public function __construct( @@ -58,6 +56,8 @@ public function buildForm(FormBuilderInterface $builder, array $options): void * @see baseCurrencyViolationMessage() Companion hook customizing the message this guards. * * @param array $gatewayConfig Mapped gateway configuration, as stored on GatewayConfig. + * + * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ public function shouldValidateBaseCurrency(array $gatewayConfig): bool { diff --git a/src/Gateway/Form/Type/AmericanExpressGatewayConfigurationType.php b/src/Gateway/Form/Type/AmericanExpressGatewayConfigurationType.php index 4387942b..7231686e 100644 --- a/src/Gateway/Form/Type/AmericanExpressGatewayConfigurationType.php +++ b/src/Gateway/Form/Type/AmericanExpressGatewayConfigurationType.php @@ -19,7 +19,5 @@ final class AmericanExpressGatewayConfigurationType extends AbstractGatewayConfi { protected string $gatewayFactoryTitle = AmericanExpressGatewayFactory::FACTORY_TITLE; - protected string $gatewayFactoryName = AmericanExpressGatewayFactory::FACTORY_NAME; - protected string $gatewayBaseCurrencyCode = AmericanExpressGatewayFactory::BASE_CURRENCY_CODE; } diff --git a/src/Gateway/Form/Type/ApplePayGatewayConfigurationType.php b/src/Gateway/Form/Type/ApplePayGatewayConfigurationType.php index 1f3da3ec..2c831a0d 100644 --- a/src/Gateway/Form/Type/ApplePayGatewayConfigurationType.php +++ b/src/Gateway/Form/Type/ApplePayGatewayConfigurationType.php @@ -19,7 +19,5 @@ final class ApplePayGatewayConfigurationType extends AbstractGatewayConfiguratio { protected string $gatewayFactoryTitle = ApplePayGatewayFactory::FACTORY_TITLE; - protected string $gatewayFactoryName = ApplePayGatewayFactory::FACTORY_NAME; - protected string $gatewayBaseCurrencyCode = ApplePayGatewayFactory::BASE_CURRENCY_CODE; } diff --git a/src/Gateway/Form/Type/BancontactGatewayConfigurationType.php b/src/Gateway/Form/Type/BancontactGatewayConfigurationType.php index 25eac35b..c7bf6803 100644 --- a/src/Gateway/Form/Type/BancontactGatewayConfigurationType.php +++ b/src/Gateway/Form/Type/BancontactGatewayConfigurationType.php @@ -19,7 +19,5 @@ final class BancontactGatewayConfigurationType extends AbstractGatewayConfigurat { protected string $gatewayFactoryTitle = BancontactGatewayFactory::FACTORY_TITLE; - protected string $gatewayFactoryName = BancontactGatewayFactory::FACTORY_NAME; - protected string $gatewayBaseCurrencyCode = BancontactGatewayFactory::BASE_CURRENCY_CODE; } diff --git a/src/Gateway/Form/Type/OneyGatewayConfigurationType.php b/src/Gateway/Form/Type/OneyGatewayConfigurationType.php index fd548251..b446b709 100644 --- a/src/Gateway/Form/Type/OneyGatewayConfigurationType.php +++ b/src/Gateway/Form/Type/OneyGatewayConfigurationType.php @@ -19,7 +19,5 @@ final class OneyGatewayConfigurationType extends AbstractGatewayConfigurationTyp { protected string $gatewayFactoryTitle = OneyGatewayFactory::FACTORY_TITLE; - protected string $gatewayFactoryName = OneyGatewayFactory::FACTORY_NAME; - protected string $gatewayBaseCurrencyCode = OneyGatewayFactory::BASE_CURRENCY_CODE; } diff --git a/src/Gateway/Form/Type/PayPlugGatewayConfigurationType.php b/src/Gateway/Form/Type/PayPlugGatewayConfigurationType.php index 9a5c69b9..309e5478 100644 --- a/src/Gateway/Form/Type/PayPlugGatewayConfigurationType.php +++ b/src/Gateway/Form/Type/PayPlugGatewayConfigurationType.php @@ -20,8 +20,6 @@ final class PayPlugGatewayConfigurationType extends AbstractGatewayConfiguration { protected string $gatewayFactoryTitle = PayPlugGatewayFactory::FACTORY_TITLE; - protected string $gatewayFactoryName = PayPlugGatewayFactory::FACTORY_NAME; - protected string $gatewayBaseCurrencyCode = PayPlugGatewayFactory::BASE_CURRENCY_CODE; /** diff --git a/src/Gateway/Form/Type/ScalapayGatewayConfigurationType.php b/src/Gateway/Form/Type/ScalapayGatewayConfigurationType.php index ec89fb96..d240aa04 100644 --- a/src/Gateway/Form/Type/ScalapayGatewayConfigurationType.php +++ b/src/Gateway/Form/Type/ScalapayGatewayConfigurationType.php @@ -19,7 +19,5 @@ final class ScalapayGatewayConfigurationType extends AbstractGatewayConfiguratio { protected string $gatewayFactoryTitle = ScalapayGatewayFactory::FACTORY_TITLE; - protected string $gatewayFactoryName = ScalapayGatewayFactory::FACTORY_NAME; - protected string $gatewayBaseCurrencyCode = ScalapayGatewayFactory::BASE_CURRENCY_CODE; } diff --git a/src/Gateway/Form/Type/WeroGatewayConfigurationType.php b/src/Gateway/Form/Type/WeroGatewayConfigurationType.php index 39d1a035..79055931 100644 --- a/src/Gateway/Form/Type/WeroGatewayConfigurationType.php +++ b/src/Gateway/Form/Type/WeroGatewayConfigurationType.php @@ -19,7 +19,5 @@ final class WeroGatewayConfigurationType extends AbstractGatewayConfigurationTyp { protected string $gatewayFactoryTitle = WeroGatewayFactory::FACTORY_TITLE; - protected string $gatewayFactoryName = WeroGatewayFactory::FACTORY_NAME; - protected string $gatewayBaseCurrencyCode = WeroGatewayFactory::BASE_CURRENCY_CODE; } diff --git a/src/Repository/PaymentMethodRepository.php b/src/Repository/PaymentMethodRepository.php index 87b133e3..a7564201 100644 --- a/src/Repository/PaymentMethodRepository.php +++ b/src/Repository/PaymentMethodRepository.php @@ -6,6 +6,7 @@ use Sylius\Bundle\CoreBundle\Doctrine\ORM\PaymentMethodRepository as BasePaymentMethodRepository; use Sylius\Component\Core\Model\PaymentMethodInterface; +use Webmozart\Assert\Assert; final class PaymentMethodRepository extends BasePaymentMethodRepository implements PaymentMethodRepositoryInterface { @@ -23,7 +24,6 @@ public function findOneByGatewayName(string $gatewayFactoryName): ?PaymentMethod public function findEnabledByGatewayName(string $gatewayFactoryName): array { - /** @var array $paymentMethods */ $paymentMethods = $this->createQueryBuilder('o') ->innerJoin('o.gatewayConfig', 'gatewayConfig') ->leftJoin('o.channels', 'channel') @@ -35,6 +35,9 @@ public function findEnabledByGatewayName(string $gatewayFactoryName): array ->getResult() ; + Assert::isList($paymentMethods); + Assert::allIsInstanceOf($paymentMethods, PaymentMethodInterface::class); + return $paymentMethods; } } diff --git a/src/Repository/PaymentMethodRepositoryInterface.php b/src/Repository/PaymentMethodRepositoryInterface.php index 21b79171..1baad5f7 100644 --- a/src/Repository/PaymentMethodRepositoryInterface.php +++ b/src/Repository/PaymentMethodRepositoryInterface.php @@ -12,7 +12,7 @@ interface PaymentMethodRepositoryInterface extends BasePaymentMethodRepositoryIn public function findOneByGatewayName(string $gatewayFactoryName): ?PaymentMethodInterface; /** - * @return array + * @return list */ public function findEnabledByGatewayName(string $gatewayFactoryName): array; } diff --git a/tests/PHPUnit/Gateway/Form/Extension/PaymentMethodTypeExtensionTest.php b/tests/PHPUnit/Gateway/Form/Extension/PaymentMethodTypeExtensionTest.php index ff4c0f4d..6cd07ea0 100644 --- a/tests/PHPUnit/Gateway/Form/Extension/PaymentMethodTypeExtensionTest.php +++ b/tests/PHPUnit/Gateway/Form/Extension/PaymentMethodTypeExtensionTest.php @@ -12,6 +12,7 @@ use PayPlug\SyliusPayPlugPlugin\Repository\PaymentMethodRepositoryInterface; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Sylius\Bundle\PaymentBundle\Form\Type\PaymentMethodType; use Sylius\Component\Core\Model\ChannelInterface; use Sylius\Component\Core\Model\PaymentMethodInterface; use Sylius\Component\Currency\Model\CurrencyInterface; @@ -80,6 +81,15 @@ public function testPostSubmit_hostedFieldsOnNonEurChannel_addsNoError(): void ); } + /** + * This extension must target the base `PaymentMethodType`, not the AdminBundle subtype, so that + * the AdminBundle form (which extends the base type) inherits the listener too. + */ + public function testGetExtendedTypes_returnsPaymentMethodType(): void + { + self::assertSame([PaymentMethodType::class], PaymentMethodTypeExtension::getExtendedTypes()); + } + /** * Runs the extension's POST_SUBMIT listener over a CB payment method carrying a single channel. *