Skip to content

PRE-3683: resolve the /account payload per gateway config - #324

Merged
adumont-payplug merged 1 commit into
feature/PRE-3440_multi_shop_configurationfrom
fix/PRE-3683_per_config_account_memo
Sep 15, 2026
Merged

adumont-payplug merged 1 commit into
feature/PRE-3440_multi_shop_configurationfrom
fix/PRE-3683_per_config_account_memo

Conversation

@adumont-payplug

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

Copy link
Copy Markdown
Collaborator

Description

SupportedMethodsProvider::provide() resolved the PayPlug /account payload once per call and reused it for every payment method in the loop:

$authorizedCurrencies ??= $this->resolveAuthorizedCurrencies($factoryName);
$allowedCountries    ??= $this->resolveAllowedCountries($factoryName);

Two things were wrong with it:

  1. The first matching payment method's account governed every later one — its authorized currencies drove the currency gate, its allowed_countries drove the billing-country gate, for all of them.
  2. The lookup went through PayPlugApiClientFactory::create($factoryName), i.e. findOneBy(['factoryName' => …]), so the account was that of an arbitrary config for the factory — the lowest id — regardless of which payment method was being filtered. With two CB gateways on disjoint channels (now possible since PRE-3628/PRE-3629), the second channel's checkout was filtered against the first channel's PayPlug account.

The fix memoizes the /account payload per gateway config and resolves the client with createForPaymentMethod(), the idiom already used at the other 16 call sites. The currency/country helpers now take the account array and are pure, so one config costs one /account read instead of two, and the one-call-per-account optimisation is kept rather than traded away for correctness.

accountMemoKey() keys on the persisted id, falling back to spl_object_id() for a config that has not been flushed — a null id would otherwise collide with every other unsaved one. The $paymentMethodKey derived by splitting the factory name on _ is untouched: it keys into the /account payload, not into credentials.

Motivation: on a multiboutique shop, a merchant's second CB gateway was hidden or shown at checkout according to a different account's currencies, countries and amount limits.

Related issue(s): PRE-3683 — follow-up 2/4 of the PRE-3630 spike (§3b), under epic PRE-3440.

⚠️ Stacked PR — based on feature/PRE-3629_disable_claimed_channels, to merge after it.

Out of scope: IpnAction and IntegratedPaymentController are the last two create($factoryName) callers and share the same "arbitrary config for this factory" property. That reads as credential-scoping work (follow-up 3/4 or 4/4) and is deliberately left alone here.


Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Checklist

Code Quality

  • Code is linted and formatted — ecs clean, phpstan level max clean
  • No unnecessary commented-out code or debug logs
  • No hardcoded values (use env variables or config)

Testing

  • Unit tests added / updated

Three tests added to SupportedMethodsProviderTest, each watched failing on the old code first:

  • …filtersEachAgainstItsOwnCurrencies — two methods, two accounts (EUR-only vs USD-only); before the fix both survived an EUR checkout.
  • …filtersEachAgainstItsOwnAllowedCountries — same split for the billing-country gate (FR vs DE).
  • …readsThatAccountOnce — two methods sharing one gateway config hit /account once between them, guarding the memo against a per-method regression.

Full suite: 545 tests / 1017 assertions green.

composer tests still aborts at phpmd — 43 violations, the exact same count before and after this change (verified by stashing), all pre-existing on develop: long class names, $e, and PayplugCardPersister::persist() complexity. provide() sits at cyclomatic 10 on both sides.

Security & Ops

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

Base automatically changed from feature/PRE-3629_disable_claimed_channels to feature/PRE-3440_multi_shop_configuration September 14, 2026 14:34
@adumont-payplug
adumont-payplug added this pull request to stack #326 September 14, 2026 15:20
@adumont-payplug
adumont-payplug force-pushed the fix/PRE-3683_per_config_account_memo branch from c1e9874 to 9d83d25 Compare September 14, 2026 15:23

@hdelaforce-payplug hdelaforce-payplug left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Revue automatisée (haute exhaustivité). Le fix corrige bien le bug décrit : le passage à createForPaymentMethod() + la mémoïsation par config résolvent correctement le cas multi-shop. Deux points relevés en commentaires inline, le premier me semble à considérer avant merge, le second est mineur.

À noter, écarté après vérification : pas de bug d'ordre d'arguments dans les 6 decorators (named arguments déjà en place sur cette branche) ; IpnAction/IntegratedPaymentController partagent le même défaut "config arbitraire" mais c'est explicitement mentionné comme hors-scope (follow-up) dans la description de la PR.

Comment thread src/Provider/SupportedMethodsProvider.php
Comment thread src/Provider/SupportedMethodsProvider.php
@adumont-payplug
adumont-payplug removed this pull request from stack #326 September 15, 2026 08:15
@adumont-payplug
adumont-payplug added this pull request to stack #328 September 15, 2026 08:17
@adumont-payplug
adumont-payplug merged commit 5d63bd5 into feature/PRE-3440_multi_shop_configuration Sep 15, 2026
13 checks passed
@adumont-payplug
adumont-payplug deleted the fix/PRE-3683_per_config_account_memo branch September 15, 2026 08:17
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