From 6602ccd841ba3a85511129c2851ea8773e0f84cb Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Mon, 28 Sep 2026 05:40:42 +0200 Subject: [PATCH 01/15] Get history entry id from its sequence on PostgreSQL Without it, entries state was never updated and a notification sent again was stored twice. --- lib/GaletteStripe/StripeHistory.php | 5 ++++- .../Controllers/tests/units/StripeController.php | 12 ++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/lib/GaletteStripe/StripeHistory.php b/lib/GaletteStripe/StripeHistory.php index 5b901e9..a29a01e 100644 --- a/lib/GaletteStripe/StripeHistory.php +++ b/lib/GaletteStripe/StripeHistory.php @@ -95,7 +95,10 @@ public function add(array|string $action, string $argument = '', string $query = $insert = $this->zdb->insert($this->getTableName()); $insert->values($values); $this->zdb->execute($insert); - $this->id = (int)$this->zdb->driver->getLastGeneratedValue(); + //without the sequence name, pgsql gives no value + $this->id = (int)$this->zdb->connection->getLastGeneratedValue( + $this->zdb->isPostgres() ? $this->zdb->getSequenceName($this->getTableName(), 'id', prefixed: true) : null + ); Analog::log( 'An entry has been added in stripe history', diff --git a/tests/GaletteStripe/Controllers/tests/units/StripeController.php b/tests/GaletteStripe/Controllers/tests/units/StripeController.php index 6f266fb..fa71c83 100644 --- a/tests/GaletteStripe/Controllers/tests/units/StripeController.php +++ b/tests/GaletteStripe/Controllers/tests/units/StripeController.php @@ -446,8 +446,20 @@ public function testWebhookSignedWithSecret(): void $this->assertSame(1, $this->countContributions($member->id)); $history = $this->zdb->execute($this->zdb->select(STRIPE_PREFIX . StripeHistory::TABLE))->current(); + $this->assertSame(StripeHistory::STATE_PROCESSED, (int)$history->state); $this->assertSame('Jane Doe', $history->payer_name); + //Stripe sends notifications again until it gets an answer: store only once + $test_response = $this->postWebhook($this->getSucceededEvent($member->id, 5, 1250), 'whsec_test'); + $this->assertSame(200, $test_response->getStatusCode()); + $this->expectLogEntry(Analog::WARNING, 'has already been processed'); + $this->expectNoLogEntry(); + $this->assertSame(2, $this->countHistory()); + $this->assertSame(1, $this->countContributions($member->id)); + $select = $this->zdb->select(STRIPE_PREFIX . StripeHistory::TABLE); + $select->order(StripeHistory::PK . ' DESC'); + $this->assertSame(StripeHistory::STATE_ALREADYDONE, (int)$this->zdb->execute($select)->current()->state); + //a notification signed with another secret is refused $test_response = $this->postWebhook($this->getSucceededEvent($member->id, 5, 1250), 'whsec_other'); $this->assertSame(400, $test_response->getStatusCode()); From ddf99803d53c55d8b6a2b6e25442554829f5ad82 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Mon, 28 Sep 2026 05:42:18 +0200 Subject: [PATCH 02/15] Store payments without their details, never without history --- .../Controllers/StripeController.php | 7 +- lib/GaletteStripe/StripeHistory.php | 34 +++++-- .../tests/units/StripeController.php | 91 ++++++++++++++++++- 3 files changed, 117 insertions(+), 15 deletions(-) diff --git a/lib/GaletteStripe/Controllers/StripeController.php b/lib/GaletteStripe/Controllers/StripeController.php index c9c0853..825af19 100644 --- a/lib/GaletteStripe/Controllers/StripeController.php +++ b/lib/GaletteStripe/Controllers/StripeController.php @@ -462,10 +462,13 @@ public function webhook(Request $request, Response $response): Response if ( isset($post['type']) && $post['type'] == 'payment_intent.succeeded' - && $post['data']['object']['metadata']['item_id'] + && !empty($post['data']['object']['metadata']['item_id']) ) { $sh = new StripeHistory($this->zdb, $this->login, $this->preferences); - $sh->add($post); + if (!$sh->add($post)) { + //Stripe will send it again + return $response->withStatus(500, 'Internal error'); + } // are we working on a real contribution? $real_contrib = false; diff --git a/lib/GaletteStripe/StripeHistory.php b/lib/GaletteStripe/StripeHistory.php index a29a01e..1b73f15 100644 --- a/lib/GaletteStripe/StripeHistory.php +++ b/lib/GaletteStripe/StripeHistory.php @@ -75,20 +75,36 @@ public function add(array|string $action, string $argument = '', string $query = { $stripe = new Stripe($this->zdb, $this->preferences); $request = $action; - $payment_method = $this->getStripePaymentMethod($request['data']['object']['payment_method']); - $charge = $this->getStripeCharge($request['data']['object']['latest_charge']); + $intent = $request['data']['object']; + + //payer, method and receipt are details: the payment is stored without them + $payment_method = []; + $charge = []; + try { + if (!empty($intent['payment_method'])) { + $payment_method = $this->getStripePaymentMethod($intent['payment_method']); + } + if (!empty($intent['latest_charge'])) { + $charge = $this->getStripeCharge($intent['latest_charge']); + } + } catch (\Throwable $e) { + Analog::log( + 'Unable to get details of Stripe payment ' . $intent['id'] . ' | ' . $e->getMessage(), + Analog::WARNING + ); + } try { $values = [ 'history_date' => date('Y-m-d H:i:s'), - 'intent_id' => $request['data']['object']['id'], - 'payer_name' => $payment_method['billing_details']['name'], - 'member_id' => $request['data']['object']['metadata']['member_id'] ?? 0, - 'comments' => $request['data']['object']['metadata']['item_name'], - 'amount' => $stripe->isZeroDecimal($stripe->getCurrency()) ? $request['data']['object']['amount'] : $request['data']['object']['amount'] / 100, - 'method' => $payment_method['type'], + 'intent_id' => $intent['id'], + 'payer_name' => $payment_method['billing_details']['name'] ?? null, + 'member_id' => $intent['metadata']['member_id'] ?? 0, + 'comments' => $intent['metadata']['item_name'] ?? null, + 'amount' => $stripe->isZeroDecimal($stripe->getCurrency()) ? $intent['amount'] : $intent['amount'] / 100, + 'method' => $payment_method['type'] ?? $intent['payment_method_types'][0] ?? '', 'state' => self::STATE_NONE, - 'receipt_url' => $charge['receipt_url'], + 'receipt_url' => $charge['receipt_url'] ?? null, 'request' => Galette::jsonEncode($request) ]; diff --git a/tests/GaletteStripe/Controllers/tests/units/StripeController.php b/tests/GaletteStripe/Controllers/tests/units/StripeController.php index fa71c83..f37e355 100644 --- a/tests/GaletteStripe/Controllers/tests/units/StripeController.php +++ b/tests/GaletteStripe/Controllers/tests/units/StripeController.php @@ -38,6 +38,13 @@ class StripeController extends GaletteRoutingTestCase */ private array $api_calls = []; + /** + * Paths of the (fake) Stripe API that fail + * + * @var array + */ + private array $api_down = []; + /** * Set up tests */ @@ -45,14 +52,21 @@ public function setUp(): void { parent::setUp(); $this->api_calls = []; + $this->api_down = []; //never reach Stripe: answer as the API would ApiRequestor::setHttpClient( - new class (fn(array $call) => $this->api_calls[] = $call) implements ClientInterface { + new class ( + fn(array $call) => $this->api_calls[] = $call, + fn(string $path): bool => array_any($this->api_down, fn(string $down) => str_starts_with($path, $down)) + ) implements ClientInterface { /** - * @param \Closure(array): mixed $record Records calls + * @param \Closure(array): mixed $record Records calls + * @param \Closure(string): bool $is_down Is API path failing */ - public function __construct(private readonly \Closure $record) - { + public function __construct( + private readonly \Closure $record, + private readonly \Closure $is_down + ) { } /** @@ -77,6 +91,9 @@ public function request( ): array { ($this->record)(['method' => $method, 'url' => $absUrl, 'params' => $params]); $path = (string)parse_url($absUrl, PHP_URL_PATH); + if (($this->is_down)($path)) { + return [json_encode(['error' => ['message' => 'Service unavailable']]), 503, []]; + } $body = match (true) { str_starts_with($path, '/v1/payment_methods/') => [ 'id' => 'pm_test', @@ -406,6 +423,72 @@ public function testPreferencesSecrets(): void $this->expectNoLogEntry(); } + /** + * Payment is stored even without its details + */ + public function testWebhookWithoutPaymentDetails(): void + { + $member = $this->getMemberOne(); + $this->setStripePref('stripe_webhook_secret', 'whsec_test'); + $this->setStripePref('stripe_privkey', 'sk_test_fake'); + + $event = $this->getSucceededEvent($member->id, 5, 1250); + $event['data']['object']['payment_method'] = null; + $event['data']['object']['latest_charge'] = null; + $event['data']['object']['payment_method_types'] = ['sepa_debit']; + $test_response = $this->postWebhook($event, 'whsec_test'); + + $this->assertSame(200, $test_response->getStatusCode()); + $this->expectNoLogEntry(); + $this->assertSame([], $this->api_calls); + $this->assertSame(1, $this->countContributions($member->id)); + $history = $this->zdb->execute($this->zdb->select(STRIPE_PREFIX . StripeHistory::TABLE))->current(); + $this->assertNull($history->payer_name); + $this->assertNull($history->receipt_url); + $this->assertSame('sepa_debit', $history->method); + + //details cannot be retrieved + $this->api_down = ['/v1/payment_methods/']; + $event = $this->getSucceededEvent($member->id, 5, 1250); + $event['data']['object']['id'] = 'pi_other'; + $test_response = $this->postWebhook($event, 'whsec_test'); + + $this->assertSame(200, $test_response->getStatusCode()); + $this->expectLogEntry(Analog::WARNING, 'Unable to get details of Stripe payment pi_other'); + $this->expectNoLogEntry(); + $this->assertSame(2, $this->countContributions($member->id)); + } + + /** + * No contribution is stored when the payment cannot be added to history + */ + public function testWebhookHistoryFailure(): void + { + $member = $this->getMemberOne(); + $this->setStripePref('stripe_webhook_secret', 'whsec_test'); + $this->setStripePref('stripe_privkey', 'sk_test_fake'); + + //too long for its column + $event = $this->getSucceededEvent($member->id, 5, 1250); + $event['data']['object']['id'] = str_repeat('pi_', 100); + //on PostgreSQL, an error aborts the whole test transaction + $savepoint = $this->zdb->isPostgres(); + if ($savepoint) { + $this->zdb->db->query('SAVEPOINT history_failure', \Laminas\Db\Adapter\Adapter::QUERY_MODE_EXECUTE); + } + $test_response = $this->postWebhook($event, 'whsec_test'); + if ($savepoint) { + $this->zdb->db->query('ROLLBACK TO SAVEPOINT history_failure', \Laminas\Db\Adapter\Adapter::QUERY_MODE_EXECUTE); + } + + $this->assertSame(500, $test_response->getStatusCode()); + $this->expectLogEntry(Analog::ERROR, 'Query error'); + $this->expectLogEntry(Analog::ERROR, 'An error occurred trying to add log entry.'); + $this->expectNoLogEntry(); + $this->assertSame(0, $this->countHistory()); + $this->assertSame(0, $this->countContributions($member->id)); + } + /** * Webhook refuses notifications while no secret is configured */ From 7135643337df5235503272b6e53d2e8f1cbdbd7b Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Mon, 28 Sep 2026 05:43:07 +0200 Subject: [PATCH 03/15] Round checkout amount to the cent --- lib/GaletteStripe/Stripe.php | 3 ++- .../Controllers/tests/units/StripeController.php | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/GaletteStripe/Stripe.php b/lib/GaletteStripe/Stripe.php index bae919b..c658858 100644 --- a/lib/GaletteStripe/Stripe.php +++ b/lib/GaletteStripe/Stripe.php @@ -264,7 +264,8 @@ public function checkout(array $metadata, string $amount, string $currency): arr { try { $stripe = new StripeClient($this->getPrivKey()); - $checkout_amount = $this->isZeroDecimal($currency) ? round((float)$amount) : (float)$amount * 100; + //19.99 * 100 is 1998.9999999999998 + $checkout_amount = $this->isZeroDecimal($currency) ? round((float)$amount) : round((float)$amount * 100); $session = $stripe->checkout->sessions->create([ 'success_url' => $this->preferences->getURL() . '/plugins/stripe/success?session_id={CHECKOUT_SESSION_ID}', 'cancel_url' => $this->preferences->getURL() . '/plugins/stripe/cancel', diff --git a/tests/GaletteStripe/Controllers/tests/units/StripeController.php b/tests/GaletteStripe/Controllers/tests/units/StripeController.php index f37e355..d120de5 100644 --- a/tests/GaletteStripe/Controllers/tests/units/StripeController.php +++ b/tests/GaletteStripe/Controllers/tests/units/StripeController.php @@ -355,6 +355,12 @@ public function testCheckoutChecksAmount(): void ['member_id' => $member->id, 'item_id' => 5, 'item_name' => 'donation in money'], $params['payment_intent_data']['metadata'] ); + + //amount is rounded to the cent, not truncated + $this->api_calls = []; + $test_response = $this->postCheckout(['item_id' => '5', 'amount' => '19.99']); + $this->assertSame(301, $test_response->getStatusCode()); + $this->assertSame(1999, $this->api_calls[0]['params']['line_items'][0]['price_data']['unit_amount']); } /** From 397504f73a092a6031ed8e0d5e22fccc9d727728 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Mon, 28 Sep 2026 05:44:23 +0200 Subject: [PATCH 04/15] Fall back to the default currency when none is stored --- lib/GaletteStripe/Stripe.php | 9 +++++---- .../tests/units/StripeController.php | 20 +++++++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/lib/GaletteStripe/Stripe.php b/lib/GaletteStripe/Stripe.php index c658858..932eff2 100644 --- a/lib/GaletteStripe/Stripe.php +++ b/lib/GaletteStripe/Stripe.php @@ -62,8 +62,9 @@ public function __construct(Db $zdb, Preferences $preferences) $this->pubkey = null; $this->privkey = null; $this->webhook_secret = null; + //installation defaults $this->country = 'FR'; - $this->currency = null; + $this->currency = 'eur'; $this->load(); } @@ -607,9 +608,9 @@ public function getAllCurrencies(string $country): array * Is currency a zero-decimal? * https://docs.stripe.com/currencies#zero-decimal * - * @param string $currency Currency + * @param ?string $currency Currency, null when not configured */ - public function isZeroDecimal(string $currency): bool + public function isZeroDecimal(?string $currency): bool { $zeroDecimalCurrencies = [ "bif", @@ -629,7 +630,7 @@ public function isZeroDecimal(string $currency): bool "xpf" ]; - return in_array($currency, $zeroDecimalCurrencies); + return in_array(strtolower((string)$currency), $zeroDecimalCurrencies, true); } /** diff --git a/tests/GaletteStripe/Controllers/tests/units/StripeController.php b/tests/GaletteStripe/Controllers/tests/units/StripeController.php index d120de5..7ed1bea 100644 --- a/tests/GaletteStripe/Controllers/tests/units/StripeController.php +++ b/tests/GaletteStripe/Controllers/tests/units/StripeController.php @@ -495,6 +495,26 @@ public function testWebhookHistoryFailure(): void $this->assertSame(0, $this->countContributions($member->id)); } + /** + * Payment form is displayed even when no currency has been configured + */ + public function testFormWithoutCurrency(): void + { + $this->setTypeAmount(5, 10); + $this->setStripePref('stripe_pubkey', 'pk_test_public'); + $this->setStripePref('stripe_privkey', 'sk_test_fake'); + $delete = $this->zdb->delete(STRIPE_PREFIX . Stripe::TABLE); + $delete->where(['nom_pref' => 'stripe_currency']); + $this->zdb->execute($delete); + + $test_response = $this->app->handle($this->createRequest('stripe_form')); + $this->assertSame(200, $test_response->getStatusCode()); + $this->expectNoLogEntry(); + $body = (string)$test_response->getBody(); + $this->assertStringContainsString('name="item_id" id="in5"', $body); + $this->assertStringContainsString('€', $body); + } + /** * Webhook refuses notifications while no secret is configured */ From 51503f1999407d1431f9d6040ce17ebe8ff2001c Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Mon, 28 Sep 2026 05:44:32 +0200 Subject: [PATCH 05/15] Do not break settings scripts when the country is not proposed --- templates/default/stripe_preferences.html.twig | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/templates/default/stripe_preferences.html.twig b/templates/default/stripe_preferences.html.twig index 09bee19..8945e25 100644 --- a/templates/default/stripe_preferences.html.twig +++ b/templates/default/stripe_preferences.html.twig @@ -133,8 +133,9 @@ {% block javascripts %}