Harden Stripe renewal paid-skip for inconclusive billing reasons (#4536)

* Harden Stripe renewal paid-skip for inconclusive billing reasons

Only skip the already-paid original invoice for the initial payment or
when billing_reason is missing (ambiguous: duplicate initial webhook vs
renewal — skipping avoids double-crediting). Adds regression tests that
a duplicate initial webhook is still skipped and documents the double
DB fetch as an identity-map hit.

* Rename test identifiers to satisfy spellcheck

src/library/Payment/Adapter/Stripe.php

@@ -935,16 +935,21 @@ class Payment_Adapter_Stripe implements FOSSBilling\InjectionAwareInterface, FOS
             $this->di['em']->flush();
         }
 
-        $isInitialPayment = ($stripeInvoice->billing_reason ?? '') === 'subscription_create';
+        $billingReason = (string) ($stripeInvoice->billing_reason ?? '');
+        $isInitialPayment = $billingReason === 'subscription_create';
 
-        // Single DB fetch covers: (a) skip if already paid, (b) billing_reason fallback.
+        // The early link above already fetched the invoice, so this second find
+        // hits the identity map. It covers: (a) skip if already paid,
+        // (b) billing_reason fallback.
         if ($invoiceId) {
             $existingInvoice = $this->di['em']->getRepository(Invoice::class)->find((int) $invoiceId);
             if ($existingInvoice instanceof Invoice) {
                 // Skip if already paid — redirect flow may have processed it first. Only for the initial payment:
                 // the subscription metadata always points at the first invoice, so for a renewal it is paid by
-                // design and the payment goes to a renewal invoice below.
+                // design and the payment goes to a renewal invoice below. An inconclusive (missing)
-                if ($existingInvoice->getStatus() === Invoice::STATUS_PAID && $isInitialPayment) {
+                // billing_reason with a paid original is also skipped: minting a renewal there would
+                // double-credit a duplicate initial webhook.
+                if ($existingInvoice->getStatus() === Invoice::STATUS_PAID && ($isInitialPayment || $billingReason === '')) {
                     return false;
                 }
                 // Fallback: billing_reason inconclusive but original invoice still unpaid.

tests/Unit/Payment/Adapter/StripeTest.php

@@ -860,6 +860,120 @@ describe('handleInvoicePaymentSucceeded renewal after the first invoice was paid
         expect($processed)->toBeTrue();
         expect($tx->getInvoice()?->getId())->toBe(88);
     });
+
+    test('skips a duplicate initial payment when the original invoice is already paid', function (): void {
+        // The redirect flow pays the first invoice immediately, so when the
+        // invoice.paid webhook for the initial payment arrives the original
+        // invoice is already paid. It must not be credited a second time and
+        // must not mint a renewal invoice.
+        $tx = buildTransaction();
+        $tx->id = 61;
+
+        $stripeInvoice = new stdClass();
+        $stripeInvoice->id = 'in_init_dup';
+        $stripeInvoice->subscription = 'sub_dup';
+        $stripeInvoice->billing_reason = 'subscription_create';
+        $stripeInvoice->amount_paid = 1500;
+        $stripeInvoice->currency = 'usd';
+
+        $event = new stdClass();
+        $event->data = (object) ['object' => $stripeInvoice];
+
+        $stripeSubscription = new stdClass();
+        $stripeSubscription->id = 'sub_dup';
+        $stripeSubscription->metadata = (object) ['invoice_id' => '99', 'client_id' => '5'];
+
+        $subscriptionsMock = Mockery::mock();
+        $subscriptionsMock->shouldReceive('retrieve')->andReturn($stripeSubscription);
+        $stripeMock = Mockery::mock(StripeClient::class);
+        $stripeMock->subscriptions = $subscriptionsMock;
+        setPrivateProperty($this->adapter, 'stripe', $stripeMock);
+
+        $originalInvoice = createEntity(Invoice::class);
+        $originalInvoice->id = 99;
+        $originalInvoice->status = Invoice::STATUS_PAID;
+        $originalInvoice->currency = 'USD';
+
+        $invoiceService = Mockery::mock();
+        $invoiceService->shouldNotReceive('generateRenewalInvoiceForSubscriptionPayment', 'payInvoiceWithCredits');
+
+        $apiAdmin = Mockery::mock();
+        $apiAdmin->shouldNotReceive('client_balance_add_funds');
+
+        ['em' => $em, 'txRepo' => $txRepo, 'invoiceRepo' => $invoiceRepo] = buildEntityManagerMocks();
+        $invoiceRepo->shouldReceive('find')->with(99)->andReturn($originalInvoice);
+        $txRepo->shouldReceive('findProcessingOrProcessedByTxnId')
+            ->with('in_init_dup', null, 61)
+            ->andReturn(null);
+
+        $dbalMock = Mockery::mock(Doctrine\DBAL\Connection::class);
+        expectStripeObjectLock($dbalMock, 'in_init_dup', 1);
+        $di = container();
+        $di['dbal'] = $dbalMock;
+        $di['em'] = $em;
+        $di['mod_service'] = $di->protect(fn ($module, $service = null) => $invoiceService);
+        $this->adapter->setDi($di);
+
+        $processed = invokePrivateMethod($this->adapter, 'handleInvoicePaymentSucceeded', [$apiAdmin, $tx, $event, 1]);
+
+        expect($processed)->toBeFalse();
+    });
+
+    test('skips an inconclusive billing_reason when the original invoice is already paid', function (): void {
+        // A missing billing_reason with a paid original is ambiguous: it could
+        // be a duplicate initial webhook or a renewal. Skipping avoids
+        // double-crediting the initial payment and minting a spurious renewal.
+        $tx = buildTransaction();
+        $tx->id = 62;
+
+        $stripeInvoice = new stdClass();
+        $stripeInvoice->id = 'in_missing_reason';
+        $stripeInvoice->subscription = 'sub_missing_reason';
+        $stripeInvoice->amount_paid = 1500;
+        $stripeInvoice->currency = 'usd';
+
+        $event = new stdClass();
+        $event->data = (object) ['object' => $stripeInvoice];
+
+        $stripeSubscription = new stdClass();
+        $stripeSubscription->id = 'sub_missing_reason';
+        $stripeSubscription->metadata = (object) ['invoice_id' => '99', 'client_id' => '5'];
+
+        $subscriptionsMock = Mockery::mock();
+        $subscriptionsMock->shouldReceive('retrieve')->andReturn($stripeSubscription);
+        $stripeMock = Mockery::mock(StripeClient::class);
+        $stripeMock->subscriptions = $subscriptionsMock;
+        setPrivateProperty($this->adapter, 'stripe', $stripeMock);
+
+        $originalInvoice = createEntity(Invoice::class);
+        $originalInvoice->id = 99;
+        $originalInvoice->status = Invoice::STATUS_PAID;
+        $originalInvoice->currency = 'USD';
+
+        $invoiceService = Mockery::mock();
+        $invoiceService->shouldNotReceive('generateRenewalInvoiceForSubscriptionPayment', 'payInvoiceWithCredits');
+
+        $apiAdmin = Mockery::mock();
+        $apiAdmin->shouldNotReceive('client_balance_add_funds');
+
+        ['em' => $em, 'txRepo' => $txRepo, 'invoiceRepo' => $invoiceRepo] = buildEntityManagerMocks();
+        $invoiceRepo->shouldReceive('find')->with(99)->andReturn($originalInvoice);
+        $txRepo->shouldReceive('findProcessingOrProcessedByTxnId')
+            ->with('in_missing_reason', null, 62)
+            ->andReturn(null);
+
+        $dbalMock = Mockery::mock(Doctrine\DBAL\Connection::class);
+        expectStripeObjectLock($dbalMock, 'in_missing_reason', 1);
+        $di = container();
+        $di['dbal'] = $dbalMock;
+        $di['em'] = $em;
+        $di['mod_service'] = $di->protect(fn ($module, $service = null) => $invoiceService);
+        $this->adapter->setDi($di);
+
+        $processed = invokePrivateMethod($this->adapter, 'handleInvoicePaymentSucceeded', [$apiAdmin, $tx, $event, 1]);
+
+        expect($processed)->toBeFalse();
+    });
 });
 
 describe('resolveStripeInvoice', function (): void {