fix(billing): defers succeeded payment intent status update to process payments reliably

[ygrishajev]

We assign credits via webhook which in turn skips this is transaction is succeeded already. So succeeded status should be assigned by a webhook handler

Summary by CodeRabbit

• Bug Fixes

  • Defer certain payment status updates until webhook confirmation to avoid premature state changes.
  • Attach and use internal transaction IDs on payment intents so webhooks reliably reconcile payments, populate receipt and card details, and include receipt URLs.
  • Improve idempotent webhook handling by preferring internal-transaction-based lookup and skipping payment-method-validation events early.

• Tests

  • Updated tests to cover deferred status flow, internal-transaction lookup, and updated webhook payloads.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Create an internal DB transaction before calling Stripe, add internal_transaction_id to PaymentIntent metadata, defer persisting certain PaymentIntent statuses (DEFERRED_STATUSES) for webhook-driven updates, and switch webhook lookup/update to use the internal transaction id (findById / updateById).

Changes

Cohort / File(s)	Change Summary
Create PaymentIntent / Stripe service apps/api/src/billing/services/stripe/stripe.service.ts	Create internal DB transaction first; add internal_transaction_id to PaymentIntent metadata; introduce #DEFERRED_STATUSES; prepare update object and only set status immediately when PaymentIntent status is not deferred; persist via repository update (deferred pattern).
Stripe webhook handling apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.ts	Early-skip for payment_method_validation; idempotency: prefer paymentIntent.metadata.internal_transaction_id -> findById; assert transaction exists; success path uses updateById(existingTransaction.id, ...) and includes stripePaymentIntentId, receiptUrl, stripeChargeId, paymentMethodType, cardBrand, cardLast4; failed/canceled paths use updateByPaymentIntentId.
Repository rename apps/api/src/billing/repositories/stripe-transaction/stripe-transaction.repository.ts	Rename public method updateStatusByPaymentIntentId(...) → updateByPaymentIntentId(...) (same signature/behavior).
Tests: webhook specs apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.spec.ts, apps/api/test/functional/stripe-webhook.spec.ts	Tests now inject internal_transaction_id in PaymentIntent metadata; use findById/updateById for idempotency/assertions; include empty metadata in some payloads.
Tests: createPaymentIntent spec apps/api/src/billing/services/stripe/stripe.service.spec.ts	Expect Stripe create call to include metadata: { internal_transaction_id: "<id>" } in mocked assertions.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant API as API / Billing Service
  participant DB as Transactions DB
  participant Stripe

  Client->>API: createPaymentIntent(request)
  API->>DB: create internal transaction (status: pending)
  DB-->>API: internalTransaction{id}
  API->>Stripe: PaymentIntent.create(metadata: {..., internal_transaction_id: id})
  Stripe-->>API: PaymentIntent(response, status)
  alt status NOT in DEFERRED_STATUSES
    API->>DB: updateById(id, {stripePaymentIntentId, status})
  else status in DEFERRED_STATUSES
    API->>DB: updateById(id, {stripePaymentIntentId})
  end

  Note over Stripe,API: Later: Stripe sends webhook with metadata.internal_transaction_id
  Stripe->>API: webhook(event)
  API->>DB: findById(internal_transaction_id)
  DB-->>API: existingTransaction
  API->>DB: updateById(existingTransaction.id, {stripePaymentIntentId, status, receiptUrl, payment details...})
  DB-->>API: updatedTransaction
  API-->>Client: emit/return final status

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• feat(billing): stripe transactions #2438 — Related changes around DB-backed stripe_transactions and repository usage patterns (internal transaction IDs).
• feat(billing): implement upsert for payment methods and enhance webhook handling for idempotency #2534 — Related webhook handling updates switching to internal transaction id and idempotent lookup/update.
• feat(billing): test charge for free trial #1898 — Related modifications to stripe.service.ts createPaymentIntent metadata and update behavior.

Suggested reviewers

• baktun14

Poem

"I hop where tiny ids are sown,
A secret seed in metadata grown,
I wait for webhooks, patient and still,
Then nudge the record with gentle will,
🐰🌿"

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: deferring payment intent status updates to webhooks for reliable payment processing, which aligns with the PR's core objective.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@apps/api/src/billing/services/stripe/stripe.service.ts`:
- Around line 227-233: The transaction can be missing stripePaymentIntentId when
a webhook races in; to fix, persist stripePaymentIntentId on the transaction
before calling Stripe (set the field in the same code path that builds the
PaymentIntent metadata using stripeTransactionRepository.updateById with
stripePaymentIntentId populated), add a fallback lookup in the webhook handler
so findByPaymentIntentId() falls back to finding by userId + customerId when
missing, and make updateStatusByPaymentIntentId() return/throw on failure so the
caller (the code that calls topUpWallet) checks the return value and
aborts/retries and logs an error instead of proceeding to topUpWallet when the
status update did not succeed.

[codecov]

Codecov Report

❌ Patch coverage is 83.33333% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.48%. Comparing base (730f81a) to head (9b146f6).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
.../services/stripe-webhook/stripe-webhook.service.ts	81.81%	2 Missing ⚠️
.../api/src/billing/services/stripe/stripe.service.ts	83.33%	1 Missing ⚠️

❌ Your project status has failed because the head coverage (79.30%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2552      +/-   ##
==========================================
- Coverage   50.77%   50.48%   -0.29%     
==========================================
  Files        1070     1060      -10     
  Lines       29718    29380     -338     
  Branches     6587     6551      -36     
==========================================
- Hits        15089    14833     -256     
+ Misses      14217    14135      -82     
  Partials      412      412              

Flag	Coverage Δ		*Carryforward flag
api	79.30% <83.33%> (-0.02%)	⬇️
deploy-web	31.46% <ø> (ø)		Carriedforward from 730f81a
log-collector	?
notifications	87.94% <ø> (ø)		Carriedforward from 730f81a
provider-console	81.48% <ø> (ø)		Carriedforward from 730f81a
provider-proxy	84.35% <ø> (ø)		Carriedforward from 730f81a

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines	Coverage Δ
...tripe-transaction/stripe-transaction.repository.ts	42.55% <100.00%> (ø)
.../api/src/billing/services/stripe/stripe.service.ts	74.07% <83.33%> (-1.24%)	⬇️
.../services/stripe-webhook/stripe-webhook.service.ts	78.84% <81.81%> (+0.32%)	⬆️

... and 11 files with indirect coverage changes

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.ts`:
- Around line 92-97: The code currently uses assert(existingTransaction, 500,
...) which will throw when a matching transaction isn't found; change this to a
graceful early exit: after resolving existingTransaction (from
paymentIntent.metadata.internal_transaction_id ?
this.stripeTransactionRepository.findById(...) :
this.stripeTransactionRepository.findByPaymentIntentId(...)), check if
existingTransaction is falsy, log a warning (e.g., this.logger.warn or similar)
describing the missing internal transaction for the given
paymentIntent.id/metadata.internal_transaction_id and return from the webhook
handler (do not throw) so Stripe receives a 2xx and the event is skipped for
intents that never created internal transactions.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.spec.ts`:
- Around line 120-144: The test for tryToTopUpWalletFromPaymentIntent is missing
an assertion that stripeTransactionRepository.updateById is called to mark the
internal transaction as "succeeded" with charge details; update the test to
include latest_charge in the created PaymentIntent event and mock
stripeService.charges.retrieve to return a charge object (id,
payment_method_details.card.brand, payment_method_details.card.last4,
receipt_url), then assert stripeTransactionRepository.updateById was called with
the internalTransaction.id and an update payload containing status: "succeeded"
plus stripeChargeId, cardBrand, cardLast4, and receiptUrl (and keep existing
asserts for refillService.topUpWallet and findById).