Skip to content

PRE-3646: fixing Multicurrency HF and removing SEIF field - #319

Merged
adumont-payplug merged 2 commits into
developfrom
feature/PRE-3646_removing_seid_field
Sep 10, 2026
Merged

PRE-3646: fixing Multicurrency HF and removing SEIF field#319
adumont-payplug merged 2 commits into
developfrom
feature/PRE-3646_removing_seid_field

Conversation

@adumont-payplug

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

Copy link
Copy Markdown
Collaborator

Description

Trois changements liés, tous issus du passage d'UHF en multidevise.

1 — Retrait du champ « Identifiant SubMerchant » du BO. Le formulaire exposait hfSubMerchantId
(PasswordType), obligatoire dès que le mode hosted_fields était sélectionné, et sa valeur partait
dans chaque payload UPC. Or ce champ relève de la configuration UDV / MID pour les paiements en
EUR
, pas d'UHF. UHF n'est ouvert qu'aux marchands autorisés par PayPlug, l'activation demandant une
configuration conséquente côté cockpit et admin : un marchand qui veut encaisser en EUR n'est pas
orienté vers UHF mais vers Integrated Payment. UHF adresse les autres devises, dont les
configurations MID ne possèdent aucun submerchant — l'envoyer provoque un 400 "Invalid parameter."
au remboursement.

2 — Transmission de la devise au remboursement. Payment::getCurrencyCode() est désormais
acheminé jusqu'au payload UPC via RefundPaymentProcessorRefundCreatorInterface
UnifiedApiRefundCreator. L'endpoint ne transportait aucune devise : le montant partait « nu » et la
plateforme devait en déduire l'unité monétaire — sans ambiguïté tant que tout est en EUR, faux dès
qu'on est multidevise.

3 — Filtre de devise de SupportedMethodsProvider volontairement désactivé. Il s'appuyait sur
account.configuration.min_amounts de l'API Retail, démontrablement faux pour un compte UHF : le
compte 1461487 annonce currencies: ['EUR'] alors qu'un paiement USD a abouti sur ce même compte
(schemeTransactionId 6a9ad8eb905d3). Laissé en place, il masque au checkout un moyen de paiement
qui fonctionne.

Motivation: permettre à UHF d'encaisser dans une devise autre que l'EUR, de bout en bout
(paiement et remboursement).

Related issue(s): PRE-3646


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

Security & Ops

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

Notes for Reviewer

Bloc commenté assumé. SupportedMethodsProvider contient un bloc if volontairement mis en
commentaire — c'est le seul du PR et il est délibéré, pas un oubli. Le commentaire qui l'accompagne
documente la décision, la preuve (compte 1461487 vs schemeTransactionId 6a9ad8eb905d3) et les
deux correctifs préalables à sa réactivation :

  1. une source de vérité fiable sur les devises d'un compte UHF — l'API Retail n'en est pas une ;
  2. la comparaison elle-même, qui lit la devise d'affichage (CurrencyContextInterface) alors que
    $paymentAmount est libellé dans la devise de base du canal. Sur un canal EUR consulté en USD,
    elle rejetait donc un paiement qui aurait été encaissé en EUR.

Sujet en cours avec les équipes API, rattaché à l'Epic multidevise
PRE-3418.

Conséquence à connaître : il n'y a plus aucun filtrage de devise au checkout pour les moyens de
paiement PayPlug. Une devise non supportée n'est plus masquée — le paiement échoue côté API au lieu
de ne pas être proposé. Le contrôle min/max est lui aussi contourné pour toute devise absente de la
map. C'est le compromis retenu pour débloquer le multidevise.

Rétrocompatibilité des configs existantes. Un moyen de paiement enregistré avant ce PR conserve
hfSubMerchantId dans sa config. Deux tests couvrent ce cas :
GatewayCredentialsResolverTest::testResolve_withALeftoverSubMerchantIdInConfig_ignoresIt et
…FormSubmissionTest::testSubmit_hostedFieldsModeWithALeftoverSubMerchantIdInStoredConfig_isValid.
La valeur résiduelle est ignorée, la config reste enregistrable. Aucune migration n'est prévue —
dites-moi si vous en voulez une pour nettoyer la colonne.

Changement de signature. GatewayCredentialsResolver::resolve() retourne désormais un string
au lieu d'un array{0: string, 1: string}, et buildCommonFields() perd son paramètre submerchant.
Les deux handlers de capture ont été adaptés.

Vérification locale. composer tests passe (484 tests, PHPStan, ECS) — mais uniquement après
avoir recopié à la main les 3 fichiers modifiés d'UPC dans vendor/payplug/unified-plugin-core/.
Ce vert local ne vaut donc pas garantie CI tant que la contrainte n'est pas relevée (cf. bloc
bloquant ci-dessus).

@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 self-assigned this Sep 7, 2026
@adumont-payplug
adumont-payplug force-pushed the feature/PRE-3646_removing_seid_field branch from 53f80d9 to 0a362bd Compare September 7, 2026 14:14
@adumont-payplug
adumont-payplug force-pushed the feature/PRE-3646_removing_seid_field branch from 0a362bd to 8511b26 Compare September 7, 2026 14:41
@jhoaraupp

Copy link
Copy Markdown
Contributor

Review

Résumé : 0 critique, 1 majeur (documentation/process, pas de code), 2 mineurs, 1 nit — le code en lui-même est solide et bien testé ; le principal point d'action est de vérifier si le bandeau "bloquant" de la description est toujours d'actualité.

Findings

[HIGH] Le bandeau "⚠️ Bloquant — ne pas merger en l'état" semble obsolète

La description indique que cette PR ne peut pas être mergée tant que unified-plugin-core#31 (rendant submerchantExternalId optionnel) n'est pas publié, car composer.json requiert encore ^1.1.0 qui ne contient pas ce correctif. Or le composer.lock de cette PR résout déjà payplug/unified-plugin-core en 1.1.1, publiée le 2026-09-07 :

🚀 UPC 1.1.1 — Corrective release making submerchantExternalId optional across the payment-creation and refund paths, and adding an optional currency to refunds — non-EUR configurations can now be paid and refunded, which 1.1.0 made impossible.

Comme la contrainte ^1.1.0 de composer.json autorise déjà 1.1.1, et que le lock file l'a déjà résolue, le blocage décrit dans la description semble déjà levé. Les signatures de CommonFieldsDto / UnifiedApiPaymentService::createRefund() en 1.1.1 correspondent bien à la façon dont cette PR les appelle (4 args positionnels pour CommonFieldsDto, null pour le submerchant + $currency en fin de createRefund).

→ À confirmer : la CI est-elle bien verte maintenant ? Si oui, mettre à jour/retirer le bandeau bloquant plutôt que de laisser un texte obsolète qui pourrait faire bloquer la PR inutilement.

[LOW] Changement grumphp.yml du security-checker, sans rapport avec la PR

-        securitychecker_symfony: ~
+        securitychecker_composeraudit:
+            abandoned: report

Aucun rapport avec la fonctionnalité devise/remboursement, et non mentionné dans la description. Pas un bug, mais soit à scinder dans un commit/PR séparé, soit à expliquer (ex : securitychecker_symfony a-t-il été déprécié/retiré suite à une montée de version de grumphp ?).

[LOW] down() de la migration volontairement no-op

public function down(Schema $schema): void
{
    // Deliberately not reversible: ...
}

Raisonnement solide (valeur type credential, plus lue nulle part), mais la migration n'est de fait pas réversible. Acceptable si c'est une décision d'équipe assumée.

[NIT] Gestion incohérente de la casse de la devise entre les deux constructeurs de payload
PaymentCaptureContextBuilder::buildCommonFields() met en majuscule explicitement :

$common = new CommonFieldsDto($accountId, $amount, \strtoupper($currencyCode), $orderId);

alors que le nouveau chemin de remboursement transmet $payment->getCurrencyCode() tel quel (RefundPaymentProcessorUnifiedApiRefundCreator). Les codes devise Sylius sont normalement déjà en majuscules, donc probablement sans impact réel, mais à harmoniser par cohérence.

Points positifs

  • Tous les appelants internes de SupportedMethodsProvider::provide() sont passés en arguments nommés (paymentCurrencyCode:, billingCountryCode:) précisément parce que le nouveau paramètre a été inséré au milieu de la signature — ce qui évite une régression réelle d'argument positionnel qui aurait sinon silencieusement inversé devise/pays.
  • Suppression complète et cohérente de hfSubMerchantId — aucune référence résiduelle trouvée dans src, templates, config, translations, Behat.
  • GatewayCredentialsResolver::resolve() tolère correctement une valeur hfSubMerchantId résiduelle dans une config déjà persistée, avec test dédié.
  • La migration protège l'UPDATE avec JSON_CONTAINS_PATH(...) et factory_name = 'payplug' — no-op sur les lignes non concernées.
  • Bonne couverture de tests sur la partie délicate (SupportedMethodsProvider) : divergence devise d'affichage vs devise du paiement, exemption UHF du filtre de devise, et vérification que UHF respecte toujours les bornes de montant quand la devise est bien annoncée.

@adumont-payplug
adumont-payplug force-pushed the feature/PRE-3646_removing_seid_field branch from ab33740 to 8b0eabf Compare September 9, 2026 15:29

@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 merged commit 4bdc487 into develop Sep 10, 2026
13 checks passed
@adumont-payplug
adumont-payplug deleted the feature/PRE-3646_removing_seid_field branch September 10, 2026 07:37
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