Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
"ext-json": "*",
"giggsey/libphonenumber-for-php": "^8.12",
"payplug/payplug-php": "^4.0",
"payplug/unified-plugin-core": "dev-master",
"php-http/message-factory": "^1.1",
"sylius/refund-plugin": "^2.0",
"sylius/sylius": "^2.0",
Expand Down Expand Up @@ -57,6 +58,12 @@
"webmozart/assert": "^1.8"
},
"prefer-stable": true,
"repositories": [
{
"type": "vcs",
"url": "https://github.com/payplug/unified-plugin-core.git"
}
Comment thread
jhoaraupp marked this conversation as resolved.
],
"autoload": {
"psr-4": {
"PayPlug\\SyliusPayPlugPlugin\\": "src/"
Expand Down
6 changes: 6 additions & 0 deletions config/services.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ services:
bind:
Psr\Log\LoggerInterface: '@monolog.logger.payplug'

# PRE-3469: explicit alias, matching this file's convention for other interfaces below —
# relying on Symfony to auto-resolve a singly-implemented interface does not work for plain
# autowiring (confirmed: omitting this alias fails container compilation outright, since
# NotifyPaymentRequestHandler's #[AsMessageHandler] service is always instantiated).
PayplugUnifiedCore\Contracts\IOrderStateMutator: '@PayPlug\SyliusPayPlugPlugin\PaymentProcessing\PayplugOrderStateMutator'

PayPlug\SyliusPayPlugPlugin\Repository\PaymentRepositoryInterface:
class: PayPlug\SyliusPayPlugPlugin\Repository\PaymentRepository
parent: sylius.repository.payment
Expand Down
1,555 changes: 1,555 additions & 0 deletions docs/superpowers/plans/2026-07-21-pre-3469-upc-contracts-rework.md

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# PRE-3469 rework — real UPC contract wiring

## Context

[PRE-3469](https://payplug-prod.atlassian.net/browse/PRE-3469) originally asked for isolated proof-of-concept skeletons (under `src/Spike/`) validating that 4 Unified Plugin Core (UPC) contracts — `IOrderStateMutator`, `ITokenCache`, `IConfigurationRepository`, `IPaymentRepository` — are satisfiable against real Sylius APIs, before freezing the interfaces. That work landed in commit `690d9f5` and is fully written up in `src/Spike/VALIDATION.md`.

The ticket's "Résultats attendus" were updated twice on 2026-07-20. The current version requires:

- Real, production-wired implementations (not isolated `src/Spike/` skeletons) for `PayplugOrderStateMutator`, `PayplugTokenCache`, `PayplugConfigurationRepository`.
- `IPaymentRepository` explicitly **out of scope** (tied to a Value Object too coupled to the not-yet-built Unified API) — the existing `SyliusPaymentRepository` sketch is sufficient as-is.
- No regression on existing plugin functionality (PHPUnit + Behat stay green).
- Any blocking friction documented, or the interfaces adjusted.

This spec covers reworking the branch to meet the updated scope.

## Existing production code map

Research (see conversation) found the production analogs each contract needs to be validated against:

| Contract | Production analog | Notes |
|---|---|---|
| `IOrderStateMutator` | `PaymentTransitionApplier` (called from `NotifyPaymentRequestHandler`/`StatusPaymentRequestHandler`, the webhook/status-poll path) and `PaymentStateResolver` (called from the CLI reconciliation command `payplug:update-payment-state`) | Two separate, non-unified implementations of the same guarded `can()`/`apply()` idiom already exist in production; this ticket does not unify them. |
| `ITokenCache` | `PayPlugApiClientFactory::getTokenForGatewayConfig()`'s inline `Symfony\Contracts\Cache\CacheInterface` usage, caching the OAuth JWT access token per gateway factory/live-test scope | Confirmed by the ticket's second update: targets the OAuth JWT cache, not card/token storage (saved cards are a permanent Doctrine entity, `Card.php`, uninvolved with any cache). |
| `IConfigurationRepository` | Read side: `PayPlugApiClientFactory::getTokenForGatewayConfig()` (`client_id`/`client_secret` scoped by `config['live']`). Write side: `UnifiedAuthenticationController::oauthCallback()` (persists `live_client`/`test_client`, busts the token cache). | No production equivalent exists for `getPublicKeyId()`/`getPublicKeyValue()` — Hosted Fields public-key material isn't implemented anywhere in the plugin today. |
| `IPaymentRepository` | None — out of scope | `SyliusPaymentRepository` + `PayplugOperation` entity + `Version20260720100000` migration stay as an unwired sketch. |

## Design

### 1. `PayplugOrderStateMutator` — real production call site

Keep the existing mapping logic from `src/Spike/SyliusOrderStateMutator.php`: `PaymentOutcome::PAID/AUTHORIZED/CAPTURE_REQUIRED/REFUNDED/FAILED` → `PaymentTransitions::TRANSITION_COMPLETE/AUTHORIZE/PROCESS/REFUND/FAIL`, `THREE_DS_PENDING` as a confirmed no-op, `StateMachineInterface::can()` guard before `apply()`, explicit `EntityManagerInterface::flush()` after.

Wire it as an **additive** call inside `NotifyPaymentRequestHandler::__invoke()`, immediately after the existing `PaymentTransitionApplier::apply($payment)` call. The handler already has everything needed to derive a `PaymentOutcome` from the same status signal `PaymentTransitionApplier` consumes (`$payment->getDetails()['status']`, populated upstream by `PaymentNotificationHandler::treat()`); translate that into a `PaymentOutcome` value and call `$orderStateMutator->apply($order->getId(), $outcome)`.

Because `PaymentTransitionApplier::apply()` already applied the transition earlier in the same handler invocation, `PayplugOrderStateMutator`'s own `can()` guard will find the transition no longer applicable and no-op. This proves the mutator works against a real, live webhook event without replacing or risking the existing transition-application path. No change to `PaymentStateResolver` or the CLI reconciliation command.

### 2. `PayplugTokenCache` — real integration test, no production call site

No change to `PayPlugApiClientFactory::getTokenForGatewayConfig()`. Promote the spike's existing unit-mocked test (`tests/PHPUnit/Spike/SyliusTokenCacheTest.php`) with a new real-infrastructure test that exercises `get`/`set`/`delete` against an actual `cache.app`-equivalent PSR-6 pool (filesystem or array adapter, same spirit as `SpikeIntegrationTest`'s real MariaDB), rather than a mocked `CacheItemPoolInterface`. This validates `getItem`/`save`/`deleteItem` "dans le flux réel" per the ticket, without introducing any new code on a live request path that gates authentication for every PayPlug gateway.

### 3. `PayplugConfigurationRepository` — real integration test, no production call site

Same treatment as `PayplugTokenCache`: add a real integration test that constructs `SyliusConfigurationRepository` against an actual `GatewayConfigInterface` on a real `PaymentMethod` fixture persisted via Doctrine (rather than a mock), validating `get`/`set`/`getClientId`/`getClientSecret` against real Sylius config storage. No change to `PayPlugApiClientFactory` or `UnifiedAuthenticationController`.

`getPublicKeyId()`/`getPublicKeyValue()` read from two new config array keys, `public_key_id` and `public_key_value`, added alongside the existing `live_client`/`test_client` keys, defaulting to an empty string when absent. Nothing in production writes these keys yet — this is implementable end-to-end and ready for whenever Hosted Fields lands, documented as a friction note rather than worked around or blocked on.

### 4. `IPaymentRepository` — unchanged

`SyliusPaymentRepository`, `Entity/PayplugOperation.php`, and the `Version20260720100000` migration stay exactly as they are: an unwired sketch, correctly out of scope per the ticket.

### 5. Documentation

Update `src/Spike/VALIDATION.md` to reflect the new verdict per contract: which ones are now genuinely wired into production vs. validated only through real-infrastructure tests, and the two friction notes (`public_key_id`/`public_key_value` have no writer yet; `IOrderStateMutator`/`IConfigurationRepository` production analogs exist as more than one non-unified implementation, which this ticket does not consolidate).

## Testing / regression safety

- Existing PHPUnit + Behat suites must stay green after the changes.
- The one production code change (the additive `PayplugOrderStateMutator` call in `NotifyPaymentRequestHandler`) is guarded by the existing `can()` check, so it cannot alter current transition behavior — only observe/replay it against an already-applied transition.
- New real-infrastructure tests for `PayplugTokenCache` and `PayplugConfigurationRepository` are added, not replacing the existing unit-mocked spike tests.

## Out of scope

- Unifying `PaymentTransitionApplier` and `PaymentStateResolver` into a single `IOrderStateMutator`-backed implementation.
- Any production wiring of `IPaymentRepository`.
- Building Hosted Fields / any actual consumer of `public_key_id`/`public_key_value`.
62 changes: 62 additions & 0 deletions migrations/Version20260720100000.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
<?php

declare(strict_types=1);

namespace PayPlug\SyliusPayPlugPlugin\Migrations;

use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;

/**
* PRE-3469 spike only: schema for the throwaway PayplugOperation entity (src/Spike/Entity).
* A real migration — not a manual step or a SchemaTool call from the test — because the
* mapping is registered whenever kernel.environment=test (see
* PayPlugSyliusPayPlugExtension::prependSpikeDoctrineMapping()), which includes routine
* `sylius:fixtures:load` runs on a fresh checkout, not just the spike's own integration test.
* Without this migration, `sylius:fixtures:load` fails for anyone setting up the test
* application from scratch — found by testing this on a clean database, not by reasoning about
* it. Guarded to APP_ENV=test in both directions so a real merchant deployment never gets this
* table created (up) or, if it somehow did, never gets it dropped outside test either (down) —
* a normal plugin migration otherwise runs unconditionally, including in production. Drop this
* migration together with src/Spike/ once the spike is closed.
*/
final class Version20260720100000 extends AbstractMigration
{
public function getDescription(): string
{
return 'PRE-3469 spike: add payplug_operation table for the throwaway PayplugOperation entity.';
}

public function up(Schema $schema): void
{
$this->skipIf(
'test' !== ($_SERVER['APP_ENV'] ?? $_ENV['APP_ENV'] ?? null),
'PRE-3469 spike-only schema, not applied outside the test environment.',
);

$this->addSql('CREATE TABLE payplug_operation (
id INT AUTO_INCREMENT NOT NULL,
operation_id VARCHAR(255) NOT NULL,
exec_code VARCHAR(255) NOT NULL,
outcome VARCHAR(255) NOT NULL,
amount INT NOT NULL,
order_id VARCHAR(255) NOT NULL,
treated TINYINT(1) NOT NULL,
treated_at DATETIME DEFAULT NULL COMMENT \'(DC2Type:datetime_immutable)\',
created_at DATETIME NOT NULL COMMENT \'(DC2Type:datetime_immutable)\',
UNIQUE INDEX payplug_operation_id_unique (operation_id),
INDEX payplug_operation_order_id_idx (order_id),
PRIMARY KEY(id)
) DEFAULT CHARACTER SET UTF8 COLLATE `UTF8_unicode_ci` ENGINE = InnoDB');
}

public function down(Schema $schema): void
{
$this->skipIf(
'test' !== ($_SERVER['APP_ENV'] ?? $_ENV['APP_ENV'] ?? null),
'PRE-3469 spike-only schema, not applied outside the test environment.',
);

$this->addSql('DROP TABLE payplug_operation');
}
}
6 changes: 5 additions & 1 deletion phpunit.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,16 @@
<testsuites>
<testsuite name="Test Suite">
<directory>tests/PHPUnit</directory>
<!-- PRE-3469 spike: needs a provisioned DB + fixtures + the payplug_operation table,
none of which a fresh checkout or CI has. Not part of the default run — see its
own docblock for how to run it explicitly. -->
<exclude>tests/PHPUnit/Spike/SpikeIntegrationTest.php</exclude>
</testsuite>
</testsuites>

<php>
<ini name="error_reporting" value="-1" />
<server name="KERNEL_CLASS_PATH" value="Sylius\TestApplication\Kernel" />
<server name="KERNEL_CLASS" value="Sylius\TestApplication\Kernel" />
<server name="IS_DOCTRINE_ORM_SUPPORTED" value="true" />

<server name="APP_ENV" value="test" force="true" />
Expand Down
62 changes: 62 additions & 0 deletions src/Command/Handler/NotifyPaymentRequestHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,14 @@

use Payplug\Resource\Payment;
use PayPlug\SyliusPayPlugPlugin\ApiClient\PayPlugApiClientFactoryInterface;
use PayPlug\SyliusPayPlugPlugin\ApiClient\PayPlugApiClientInterface;
use PayPlug\SyliusPayPlugPlugin\Command\NotifyPaymentRequest;
use PayPlug\SyliusPayPlugPlugin\Handler\PaymentNotificationHandler;
use PayPlug\SyliusPayPlugPlugin\Handler\RefundNotificationHandler;
use PayPlug\SyliusPayPlugPlugin\PaymentProcessing\PaymentTransitionApplier;
use PayplugUnifiedCore\Contracts\IOrderStateMutator;
use PayplugUnifiedCore\Models\PaymentOutcome;
use Psr\Log\LoggerInterface;
use Sylius\Abstraction\StateMachine\StateMachineInterface;
use Sylius\Bundle\PaymentBundle\Provider\PaymentRequestProviderInterface;
use Sylius\Component\Core\Model\PaymentInterface;
Expand All @@ -19,13 +23,30 @@
#[AsMessageHandler]
class NotifyPaymentRequestHandler
{
/**
* PRE-3469: additive translation from the PayPlug status vocabulary (the same one
* PaymentTransitionApplier already maps from) to UPC's PaymentOutcome vocabulary, for the
* real IOrderStateMutator call below. Statuses with no PaymentOutcome equivalent
* (e.g. STATUS_ABORTED/STATUS_CANCELED*, which PaymentTransitionApplier maps to
* TRANSITION_CANCEL) are intentionally absent here — they're skipped, never force-mapped.
*
* @var array<string, string>
*/
private const STATUS_TO_OUTCOME = [
PayPlugApiClientInterface::STATUS_CAPTURED => PaymentOutcome::PAID,
PayPlugApiClientInterface::STATUS_AUTHORIZED => PaymentOutcome::AUTHORIZED,
PayPlugApiClientInterface::FAILED => PaymentOutcome::FAILED,
];

public function __construct(
private PaymentRequestProviderInterface $paymentRequestProvider,
private StateMachineInterface $stateMachine,
private PayPlugApiClientFactoryInterface $apiClientFactory,
private PaymentNotificationHandler $paymentNotificationHandler,
private RefundNotificationHandler $refundNotificationHandler,
private PaymentTransitionApplier $paymentTransitionApplier,
private IOrderStateMutator $orderStateMutator,
private LoggerInterface $logger,
) {}

public function __invoke(NotifyPaymentRequest $notifyPaymentRequest): void
Expand Down Expand Up @@ -67,6 +88,7 @@ public function __invoke(NotifyPaymentRequest $notifyPaymentRequest): void
$payment->setDetails($details->getArrayCopy());
if ($resource instanceof Payment) {
$this->paymentTransitionApplier->apply($payment);
$this->applyOrderStateMutator($payment);
}

$this->stateMachine->apply(
Expand All @@ -85,4 +107,44 @@ public function __invoke(NotifyPaymentRequest $notifyPaymentRequest): void
);
}
}

/**
* PRE-3469: additive real call site for PayplugOrderStateMutator. PaymentTransitionApplier
* has already applied the real transition above by the time this runs, so the mutator's own
* can()-guard makes this a no-op in the normal case — this exists to prove the contract works
* against a live webhook event, not to change behavior. Any failure here is caught and
* logged, never allowed to affect the primary notification flow above.
*/
private function applyOrderStateMutator(PaymentInterface $payment): void
{
$details = $payment->getDetails(); // @phpstan-ignore-line - getDetails() return mixed
$status = $details['status'] ?? '';
$outcome = self::STATUS_TO_OUTCOME[$status] ?? null;
if (null === $outcome) {
return;
}

$order = $payment->getOrder();
if (null === $order) {
return;
}

// PayplugOrderStateMutator resolves the order's *last* payment internally (its contract
// is keyed by order ID, not payment ID). On a multi-payment order — e.g. a failed attempt
// followed by a retry — that could be a different payment than the one this webhook is
// actually about. Skip rather than risk transitioning the wrong payment.
if ($order->getLastPayment()?->getId() !== $payment->getId()) {
return;
}

try {
$this->orderStateMutator->apply((string) $order->getId(), $outcome); // @phpstan-ignore-line - ResourceInterface::getId() return mixed
} catch (\Throwable $e) {
$this->logger->warning('[PayPlug] PayplugOrderStateMutator additive call failed.', [
'sylius_payment_id' => $payment->getId(),
'outcome' => $outcome,
'exception' => $e->getMessage(),
]);
}
}
}
Loading
Loading