From 13d5833d181c9e6ed827ce32a45c7ed83c64b85f Mon Sep 17 00:00:00 2001 From: Konstantinos Arvanitakis Date: Thu, 10 Sep 2026 00:14:42 +0300 Subject: [PATCH] Fix: Fixing Stripe Payment Driver, Applying Payment Mehtod (COD) fee correctly --- config/payment.php | 8 +-- src/Checkout/Services/CheckoutService.php | 30 +++++++---- src/Payment/Drivers/StripePaymentDriver.php | 24 +++++---- .../Resources/PaymentMethodResource.php | 54 +++++++++++++++---- src/Payment/Models/PaymentMethod.php | 2 +- .../Pipelines/Cart/ApplyCashOnDeliveryFee.php | 31 ----------- .../Pipelines/Cart/ApplyPaymentMethodFee.php | 49 +++++++++++++++++ 7 files changed, 131 insertions(+), 67 deletions(-) delete mode 100644 src/Payment/Pipelines/Cart/ApplyCashOnDeliveryFee.php create mode 100644 src/Payment/Pipelines/Cart/ApplyPaymentMethodFee.php diff --git a/config/payment.php b/config/payment.php index 7fa523e..01e293c 100644 --- a/config/payment.php +++ b/config/payment.php @@ -1,6 +1,6 @@ [ - ApplyCashOnDeliveryFee::class, + ApplyPaymentMethodFee::class, ], ]; diff --git a/src/Checkout/Services/CheckoutService.php b/src/Checkout/Services/CheckoutService.php index ef80b4f..0991aba 100644 --- a/src/Checkout/Services/CheckoutService.php +++ b/src/Checkout/Services/CheckoutService.php @@ -173,19 +173,19 @@ class CheckoutService /** * Records which payment type the shopper picked (Cart::meta - * ['payment_method']) — read by e.g. Modules\Core\Payment\Pipelines\ - * Cart\ApplyCashOnDeliveryFee to add that type's own cart-total - * adjustments before recalculation. + * ['payment_method']) — read by Modules\Core\Payment\Pipelines\ + * Cart\ApplyPaymentMethodFee to add that method's own `data.fee` (if + * any) before recalculation. * * Also snapshots Cart::fingerprint() into meta, *after* saving the * chosen type — the fingerprint has to reflect the final total - * including any payment-type-specific adjustment (e.g. a COD - * surcharge), which only exists once payment_method is set and the - * cart recalculates. Captured here, server-side, rather than asked of - * the storefront: this is the last moment before initiatePayment() that - * the shopper's reviewed total is known, and initiatePayment() reads it - * back internally instead of taking a fingerprint parameter — a - * storefront should never need to know Cart::fingerprint() exists. + * including any payment-method-specific fee, which only exists once + * payment_method is set and the cart recalculates. Captured here, + * server-side, rather than asked of the storefront: this is the last + * moment before initiatePayment() that the shopper's reviewed total is + * known, and initiatePayment() reads it back internally instead of + * taking a fingerprint parameter — a storefront should never need to + * know Cart::fingerprint() exists. * * Does not itself call a payment driver — selecting a method and * initiating payment against it are deliberately separate steps, same @@ -204,7 +204,15 @@ class CheckoutService $cart->meta = [...($cart->meta?->toArray() ?? []), 'payment_method' => $type]; $cart->save(); - $cart = $cart->calculate(); + // Cart::calculate() no-ops if this cart instance was already + // calculated earlier in the request (Cart::isCalculated()) — which + // it will have been if the shopper switches payment method after + // the checkout page's first render already calculated it. Without + // recalculate() forcing a fresh run, the just-saved payment_method + // (and any fee tied to it, see ApplyPaymentMethodFee) would never + // be reflected — the summary would keep showing whichever method + // was calculated first. + $cart = $cart->recalculate(); $cart->meta = [...($cart->meta?->toArray() ?? []), 'checkout_fingerprint' => $cart->fingerprint()]; $cart->save(); diff --git a/src/Payment/Drivers/StripePaymentDriver.php b/src/Payment/Drivers/StripePaymentDriver.php index da08c6d..9a8a8f8 100644 --- a/src/Payment/Drivers/StripePaymentDriver.php +++ b/src/Payment/Drivers/StripePaymentDriver.php @@ -95,17 +95,21 @@ class StripePaymentDriver implements private function createAndConfirm(string $type, Price $amount, array $data, array $context, string $captureMethod): PaymentResult { + $params = [ + 'amount' => StripeManager::toStripeAmount($amount->value, $amount->currency), + 'currency' => $amount->currency->code, + 'capture_method' => $captureMethod, + 'confirm' => true, + ]; + + if (isset($data['payment_method'])) { + $params['payment_method'] = $data['payment_method']; + } else { + $params['automatic_payment_methods'] = ['enabled' => true]; + } + try { - $paymentIntent = Stripe::getClient()->paymentIntents->create([ - 'amount' => StripeManager::toStripeAmount($amount->value, $amount->currency), - 'currency' => $amount->currency->code, - 'capture_method' => $captureMethod, - 'confirm' => true, - 'payment_method' => $data['payment_method'] ?? null, - 'automatic_payment_methods' => isset($data['payment_method']) - ? null - : ['enabled' => true], - ]); + $paymentIntent = Stripe::getClient()->paymentIntents->create($params); } catch (ApiErrorException $e) { return $this->declined($type, $amount, $e, $context, authorizing: $captureMethod === 'manual'); } diff --git a/src/Payment/Filament/Resources/PaymentMethodResource.php b/src/Payment/Filament/Resources/PaymentMethodResource.php index 1f6842c..2fcaeaf 100644 --- a/src/Payment/Filament/Resources/PaymentMethodResource.php +++ b/src/Payment/Filament/Resources/PaymentMethodResource.php @@ -13,6 +13,7 @@ use Filament\Tables\Columns\TextColumn; use Filament\Tables\Columns\ToggleColumn; use Filament\Tables\Table; use Illuminate\Support\Facades\Event; +use Modules\Core\Payment\Contracts\Configurable; use Modules\Core\Payment\Events\PaymentMethodsReordered; use Modules\Core\Payment\Filament\Resources\PaymentMethodResource\Pages\ListPaymentMethods; use Modules\Core\Payment\Models\PaymentMethod; @@ -44,10 +45,15 @@ use Modules\Core\Payment\Services\PaymentMethodService; * already correct in the database by the time it fires. * * `driver_missing_at` (set by the `boboko:payment:sync-drivers` command - * when a row's driver no longer resolves) is surfaced as its own table + * when a row's driver no longer resolves) drives the "Driver status" * column, deliberately distinct from `enabled` — an admin needs to tell - * "I turned this off" apart from "the driver code was removed" at a - * glance, not have both look like the same disabled state. + * "I turned this off" apart from "this driver isn't usable right now" at + * a glance, not have both look like the same disabled state. That column + * also folds in Configurable::isConfigured() (e.g. Stripe with no API key + * set) — a class-resolves-but-isn't-usable state that CheckoutService:: + * getPaymentMethods() filters out identically to a missing driver, so an + * admin needs the same at-a-glance warning for it, not just a silently + * absent checkout option. * * `authorized_status` only appears in the form when `capture_mode` is * "Hold now, charge later" — it's simply unreachable for a "Charge @@ -85,13 +91,12 @@ class PaymentMethodResource extends Resource IconColumn::make('driver_missing_at') ->label('Driver status') ->boolean() - ->trueIcon('heroicon-o-exclamation-triangle') - ->falseIcon('heroicon-o-check-circle') - ->trueColor('danger') - ->falseColor('success') - ->tooltip(fn (PaymentMethod $record) => $record->driver_missing_at - ? 'Driver not found as of '.$record->driver_missing_at->diffForHumans() - : 'Driver resolves correctly'), + ->state(fn (PaymentMethod $record) => ! $record->driver_missing_at && static::driverIsConfigured($record->driver)) + ->trueIcon('heroicon-o-check-circle') + ->falseIcon('heroicon-o-exclamation-triangle') + ->trueColor('success') + ->falseColor('danger') + ->tooltip(fn (PaymentMethod $record) => static::driverStatusTooltip($record)), ToggleColumn::make('enabled') ->label('Enabled') ->updateStateUsing(fn (PaymentMethod $record, $state) => app(PaymentMethodService::class) @@ -260,4 +265,33 @@ class PaymentMethodResource extends Resource return app(PaymentDriverRegistry::class)->label($key) ?? $key; } + + /** + * False for a missing driver too, since Configurable::isConfigured() + * has nothing to ask in that case — driverStatusTooltip() below is + * what tells the two reasons apart for the admin. + */ + private static function driverIsConfigured(?string $key): bool + { + $driver = $key ? app(PaymentDriverRegistry::class)->resolve($key) : null; + + if (! $driver instanceof Configurable) { + return false; + } + + return $driver->isConfigured(); + } + + private static function driverStatusTooltip(PaymentMethod $record): string + { + if ($record->driver_missing_at) { + return 'Driver not found as of '.$record->driver_missing_at->diffForHumans(); + } + + if (! static::driverIsConfigured($record->driver)) { + return 'Driver resolves, but is missing required configuration (e.g. an API key) — it will not be offered at checkout.'; + } + + return 'Driver resolves correctly and is fully configured.'; + } } diff --git a/src/Payment/Models/PaymentMethod.php b/src/Payment/Models/PaymentMethod.php index cec62f1..25d50dc 100644 --- a/src/Payment/Models/PaymentMethod.php +++ b/src/Payment/Models/PaymentMethod.php @@ -10,7 +10,7 @@ use Illuminate\Database\Eloquent\Model; * creatable/deletable, same split Modules\Core\Shipping's own * shipping_methods table already has (see docs/payments.md): * - type: unique, machine-facing slug (Cart::meta['payment_method'], - * ApplyCashOnDeliveryFee's lookup key, every Payment event's $type). + * ApplyPaymentMethodFee's lookup key, every Payment event's $type). * - name: admin-facing label. * - driver: the Modules\Core\Payment\Services\PaymentDriverRegistry key * — NOT the same as `type`, and not unique (two rows can share one diff --git a/src/Payment/Pipelines/Cart/ApplyCashOnDeliveryFee.php b/src/Payment/Pipelines/Cart/ApplyCashOnDeliveryFee.php deleted file mode 100644 index 4ad6f67..0000000 --- a/src/Payment/Pipelines/Cart/ApplyCashOnDeliveryFee.php +++ /dev/null @@ -1,31 +0,0 @@ -meta['payment_method'] ?? null) === 'cash-on-delivery') { - $fee = (int) (PaymentMethod::where('type', 'cash-on-delivery')->value('data->fee') ?? 0); - - $cart->shippingTotal = new Price( - ($cart->shippingTotal?->value ?? 0) + $fee, - $cart->currency, - 1 - ); - } - - return $next($cart); - } -} diff --git a/src/Payment/Pipelines/Cart/ApplyPaymentMethodFee.php b/src/Payment/Pipelines/Cart/ApplyPaymentMethodFee.php new file mode 100644 index 0000000..3be6995 --- /dev/null +++ b/src/Payment/Pipelines/Cart/ApplyPaymentMethodFee.php @@ -0,0 +1,49 @@ +shippingBreakdown rather than + * bumping $cart->shippingTotal directly — the later Lunar\Pipelines\ + * Cart\CalculateTax step unconditionally recomputes shippingTotal + * (and shipping tax) from shippingBreakdown's item sum, so a value + * set only on the plain property is silently discarded before the + * cart finishes calculating. + * + * @param Closure(CartContract): mixed $next + */ + public function handle(CartContract $cart, Closure $next): mixed + { + $type = $cart->meta['payment_method'] ?? null; + + if ($type) { + $fee = (int) (PaymentMethod::where('type', $type)->first()?->data['fee'] ?? 0); + + if ($fee > 0) { + $cart->shippingBreakdown->items->put('payment-method-fee', new ShippingBreakdownItem( + name: 'Payment method fee', + identifier: 'payment-method-fee', + price: new Price($fee, $cart->currency, 1), + )); + } + } + + return $next($cart); + } +}