Skip to content

Feature/pre 3628 gateway uniq per channel - #321

Merged
adumont-payplug merged 5 commits into
feature/PRE-3440_multi_shop_configurationfrom
feature/PRE-3628_gateway_uniq_per_channel
Sep 14, 2026
Merged

adumont-payplug merged 5 commits into
feature/PRE-3440_multi_shop_configurationfrom
feature/PRE-3628_gateway_uniq_per_channel

Conversation

@adumont-payplug

@adumont-payplug adumont-payplug commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Description

Scopes the gateway uniqueness rule per channel instead of per instance, so a merchant running several shops on one Sylius instance can connect a distinct PayPlug account per shop.

Before: AbstractGatewayConfigurationType::canBeCreated() did a global findOneBy(['factoryName' => …]) and refused any second gateway config of the same factory anywhere on the instance. It was also creation-only — checkCreationRequirements() returned early whenever the PaymentMethod already had an id, so edits were never validated.

After: a channel may be linked to at most one enabled gateway config per factory type. Two gateways of the same factory ("CB 1" and "CB 2") may coexist and both be enabled as long as their channel sets are disjoint. Different factory types never conflict. The rule runs on creation and edit, and the error names the conflicting channel and the payment method already holding it instead of "only one gateway allowed".

Motivation: first story of the Multiboutique epic — a multi-enseigne retail merchant currently cannot give each shop its own PayPlug account.

Related issue(s): PRE-3628 — epic PRE-3440


Why the validation moved forms

Both validations lived in PRE_SUBMIT on the nested paymentMethod.gatewayConfig.config form. Sylius adds channels to the payment-method form from CoreBundle's own type extension — i.e. after gatewayConfig — and form children are submitted in insertion order. So that listener runs before enabled and channels have been submitted and can only ever see persisted data.

The per-channel rule needs the submitted channel set and the submitted enabled flag, so it cannot live there. POST_SUBMIT on the root PaymentMethod form is the first point where all three exist.

Two pre-existing defects fixed in passing

The EUR-only base-currency check sat in that same listener and was broken by the same root cause:

  1. It validated the wrong channels. It read $formChannels->getData() — the persisted channels. On creation the entity has none, so it validated nothing at all; on edit it validated the stale set rather than the one being submitted.
  2. It 500'd when it did fire. It iterated the data Collection (offsets 0, 1, …) and then called $formChannels->get((string) $key). ChannelChoiceType sets choice_value => 'code', so the expanded choice's children are named by channel code. Form::get() throws OutOfBoundsException on an unknown child — so a genuine EUR violation produced a 500, not a form error.

Both are fixed by the relocation; errors now attach to the channels field itself.

How it's built

  • src/Checker/GatewayChannelConflictChecker.php — the rule, form-free and DB-free, so it unit-tests without a form tree. Channels are matched by code, not object identity.
  • src/Gateway/Form/Extension/PaymentMethodTypeExtension.phpPOST_SUBMIT on the root form, running the conflict check and the base-currency check.
  • PaymentMethodRepository::findEnabledByGatewayName() — enabled methods of one factory, with channels fetch-joined to avoid N+1.
  • canBeCreated() / checkCreationRequirements() and the whole PRE_SUBMIT listener are gone; the base type's constructor loses two now-unused dependencies, and two PHPStan baseline entries are deleted rather than carried.

"Is this a PayPlug gateway?" is decided by instanceof AbstractGatewayConfigurationType on the config child's inner form type, not by a hardcoded factory-name list — exact by construction across all seven gateways, and one less list to update when an eighth is added.

Behaviour changes worth a reviewer's attention

  • Two enabled gateways of the same factory are now possible. That is the point of the ticket, but it is a real change in what the admin accepts.
  • Edits are now validated. A payment method that saved fine before can now be rejected on edit.
  • The base-currency check now actually fires on creation, where it previously validated nothing. Some configurations that used to save will now be refused — correctly.

Testing

  • phpunit tests/PHPUnit — 531 passing, 976 assertions
  • phpstan (level max) — clean; ecs — clean; phpmd — 43, one below the pre-existing 44
  • Unit tests: 9 cases on the conflict rule (overlap, disjoint sets, multiple shared channels, disabled subject, disabled rival, self-on-edit, no channels, factory scoping) plus a composition test pinning the CB integrated-payment currency gate against the real persisted config shape
  • Manual pass across gateway types in a local multi-channel shop

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)

Checklist

Code Quality

  • Code is linted and formatted
  • No unnecessary commented-out code or debug logs
  • No hardcoded values (use env variables or config)

Testing

  • Unit tests added / updated
  • New/changed code is covered by tests — SonarCloud Quality Gate (coverage on new code) passes on the sonarcloud CI job

Security & Ops

  • No sensitive data or secrets introduced
  • Logging and error handling are appropriate

Out of scope — follow-ups

This branch makes multi-gateway configuration possible, but several sites still assume one gateway config per factory type and will resolve an arbitrary one. None are touched here; they are why the epic sequences PRE-3630 (credential-resolution spike) after this story:

Site Consequence
src/Controller/IpnAction.php:103 Has $paymentMethod in hand, then calls create($factoryName) — webhooks verified against the wrong account's secret key. Highest consequence.
src/Controller/IntegratedPaymentController.php:87 Same shape — wrong-account payment creation.
src/Provider/Payment/ApplePayPaymentProvider.php:52,209 findOneByGatewayName() with no channel filter — a shopper on one channel can be assigned another channel's payment method. Not credential resolution, so arguably outside PRE-3630's current scope.
src/Provider/OneySupportedPaymentChoiceProvider.php:42 Reads fees_for off an arbitrary Oney gateway.
src/Upc/SyliusUpcConfigurationRepository.php:84 Global findOneBy.
src/Twig/OneyExtension.php:35 Global findOneBy.

This targets the epic branch feature/PRE-3440_multi_shop_configuration, not develop, so none of the above reaches merchants as a result of this merge.

Also deliberately deferred: the channel-selector UX showing already-taken channels (PRE-3629); any database-level uniqueness constraint (the rule is an admin-form guard — fixtures, the API and direct SQL bypass it); Behat coverage.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.

Tip: disable this comment in your organization's Code Review settings.

@adumont-payplug
adumont-payplug changed the base branch from develop to feature/PRE-3440_multi_shop_configuration September 14, 2026 07:40
- 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<>
@adumont-payplug
adumont-payplug force-pushed the feature/PRE-3628_gateway_uniq_per_channel branch from b6e5334 to 293e773 Compare September 14, 2026 08:09
@adumont-payplug
adumont-payplug merged commit 9c7458f into feature/PRE-3440_multi_shop_configuration Sep 14, 2026
6 checks passed
@adumont-payplug
adumont-payplug deleted the feature/PRE-3628_gateway_uniq_per_channel branch September 14, 2026 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants