Skip to content

feat(billing): implement upsert for payment methods and enhance webhook handling for idempotency#2534

Merged
baktun14 merged 5 commits intomainfrom
fix/stripe-errors
Jan 20, 2026
Merged

feat(billing): implement upsert for payment methods and enhance webhook handling for idempotency#2534
baktun14 merged 5 commits intomainfrom
fix/stripe-errors

Conversation

@baktun14
Copy link
Contributor

@baktun14 baktun14 commented Jan 19, 2026

Summary by CodeRabbit

  • New Features

    • Idempotent payment-method upsert: prevents duplicates, marks first method as default, and attempts remote default sync only when a new default is created.
  • Bug Fixes

    • Webhook handlers are idempotent for payment intents and refunds; refunds use cumulative refunded amounts to compute deltas and avoid double-crediting.
  • Tests

    • Expanded unit and functional tests covering webhook idempotency, refunds, and payment-method flows.
  • Chores

    • DB migration: added cumulative refunded-amount column to transactions.

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

@baktun14 baktun14 requested a review from a team as a code owner January 19, 2026 21:14
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 19, 2026

📝 Walkthrough

Walkthrough

Adds an idempotent upsert to PaymentMethodRepository, updates Stripe webhook flows to use it and to handle idempotent payment-intent/refund processing, adds an amount_refunded column and migration, and expands unit and functional tests for duplicate webhook deliveries and payment-method attachment flows.

Changes

Cohort / File(s) Summary
Repository Layer
apps/api/src/billing/repositories/payment-method/payment-method.repository.ts
Added upsert(input) that idempotently returns existing or inserts a new payment method, decides isDefault for first method, uses ON CONFLICT DO NOTHING, handles race conditions and unique-violation retry/query, and imports isUniqueViolation.
Service Layer
apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.ts
Reworked payment-intent and refund flows to be idempotent (skip if succeeded txn exists), compute refund deltas via amountRefunded, replace count-based defaulting with upsert(), and trigger remote Stripe default sync only for newly-created defaults (sync failures logged).
Unit Tests (webhook)
apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.spec.ts
Added extensive tests covering payment-method attachment idempotency, payment-intent duplicate delivery handling, refund delta and idempotency scenarios, new helpers (createPaymentMethodAttachedEvent), and updated mocks to include amountRefunded.
Functional Tests
apps/api/test/functional/stripe-webhook.spec.ts
Added end-to-end tests for idempotent handling of duplicate payment_intent.succeeded and charge.refunded webhook deliveries verifying wallet/deploymentAllowance unchanged on retries.
Schema / Migration
apps/api/drizzle/0027_unknown_whistler.sql, apps/api/src/billing/model-schemas/stripe-transaction/stripe-transaction.schema.ts, apps/api/drizzle/meta/0027_snapshot.json, apps/api/drizzle/meta/_journal.json
Added amount_refunded (amountRefunded) integer NOT NULL DEFAULT 0 to stripe_transactions; included migration SQL, schema change, snapshot, and journal entry.
Tests - Stripe service mocks
apps/api/src/billing/services/stripe/stripe.service.spec.ts
Ensured mock Stripe transactions include amountRefunded: input.amountRefunded ?? 0.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Webhook Client
    participant Service as StripeWebhookService
    participant Repo as PaymentMethodRepository
    participant DB as Database
    participant Stripe as Stripe API

    Client->>Service: payment_method.attached event
    Service->>Repo: upsert(userId, fingerprint, paymentMethodId)
    Repo->>DB: SELECT by fingerprint + paymentMethodId
    alt Record exists
        DB-->>Repo: existing record
        Repo-->>Service: {paymentMethod, isNew: false}
    else Record not found
        Repo->>DB: SELECT count(*) of user's payment methods
        DB-->>Repo: count
        Repo->>DB: INSERT new payment method (isDefault based on count) ON CONFLICT DO NOTHING
        alt Insert succeeds
            DB-->>Repo: new record
            Repo-->>Service: {paymentMethod, isNew: true}
        else Conflict / unique violation
            DB-->>Repo: conflict / error
            Repo->>DB: SELECT by fingerprint + paymentMethodId
            DB-->>Repo: return record
            Repo-->>Service: {paymentMethod, isNew: false}
        end
    end

    alt isNew = true AND paymentMethod.isDefault = true
        Service->>Stripe: Set payment method as default on customer
        alt Sync succeeds
            Stripe-->>Service: OK
        else Sync fails
            Stripe-->>Service: Error
            Service->>Service: Log STRIPE_DEFAULT_PAYMENT_METHOD_SYNC_FAILED
        end
    end

    Service-->>Client: 200 OK
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • ygrishajev
  • stalniy

"I hopped through logs and SQL rows,
Inserted defaults where the moonlight glows,
Duplicates now bounce with grace,
Refunds count each cent of trace,
A rabbit cheers for idempotent days!" 🐇✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately summarizes the main changes: implementing an upsert method for payment methods and enhancing webhook handling with idempotency logic across multiple services.

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

✨ Finishing touches
  • 📝 Generate docstrings

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In
`@apps/api/src/billing/repositories/payment-method/payment-method.repository.ts`:
- Around line 136-178: The upsert() method can hit a partial-unique constraint
on (userId, isDefault) when two requests compute isDefault=true concurrently;
update the insert conflict handling to either include that constraint or
gracefully handle the DB error: modify the
cursor.insert(...).onConflictDoNothing(...) call to also target the userId and
isDefault columns (e.g., add this.table.userId and this.table.isDefault to the
target list) so the second insert is ignored, or wrap the insert/returning block
in a try/catch that detects the specific unique/constraint violation and then
loads the existing record via findOneBy and returns isNew:false. Ensure
references to upsert, countByUserId, onConflictDoNothing, cursor.insert, and
findOneBy are used to locate the change.

In `@apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.ts`:
- Around line 90-99: The idempotency guard in stripe-webhook.service.ts only
treats existingTransaction?.status === "succeeded" as already processed, which
will reprocess refunded or other terminal-success transactions; update the check
in the Stripe webhook handler (where you call
stripeTransactionRepository.findByPaymentIntentId and inspect
existingTransaction.status) to treat "refunded" and any other terminal
success/settled statuses as already-processed (e.g., check against a set/array
like ["succeeded","refunded", ...]) and return early, ensuring you reference
existingTransaction.id and paymentIntent.id in the log message as before.

stalniy
stalniy previously approved these changes Jan 20, 2026
@codecov
Copy link

codecov bot commented Jan 20, 2026

Codecov Report

❌ Patch coverage is 45.00000% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.73%. Comparing base (93b4ee2) to head (cff8a2e).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...tories/payment-method/payment-method.repository.ts 4.76% 16 Missing and 4 partials ⚠️
.../services/stripe-webhook/stripe-webhook.service.ts 89.47% 2 Missing ⚠️

❌ Your patch status has failed because the patch coverage (45.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.
❌ Your project status has failed because the head coverage (79.35%) 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    #2534      +/-   ##
==========================================
+ Coverage   50.70%   50.73%   +0.03%     
==========================================
  Files        1063     1063              
  Lines       29490    29513      +23     
  Branches     6531     6548      +17     
==========================================
+ Hits        14952    14974      +22     
+ Misses      14230    14114     -116     
- Partials      308      425     +117     
Flag Coverage Δ
api 79.35% <45.00%> (+0.03%) ⬆️
deploy-web 31.17% <ø> (+<0.01%) ⬆️
log-collector 75.35% <ø> (ø)
notifications 87.94% <ø> (ø)
provider-console 81.48% <ø> (ø)
provider-proxy 84.35% <ø> (ø)
Files with missing lines Coverage Δ
...as/stripe-transaction/stripe-transaction.schema.ts 88.88% <ø> (ø)
.../services/stripe-webhook/stripe-webhook.service.ts 78.52% <89.47%> (+12.08%) ⬆️
...tories/payment-method/payment-method.repository.ts 16.41% <4.76%> (-6.99%) ⬇️

... and 40 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
`@apps/api/src/billing/repositories/payment-method/payment-method.repository.ts`:
- Around line 198-205: The non-null assertion on paymentMethod after calling
findOneBy can throw at runtime; update the logic around findOneBy (the
paymentMethod variable) to handle a null result defensively: check if
paymentMethod is null and either throw a clear, descriptive error (including
input.fingerprint and input.paymentMethodId) or return an explicit failure
value/object instead of using the ! operator, then return the normal {
paymentMethod, isNew: false } when present.

@baktun14 baktun14 merged commit 7d4f2dd into main Jan 20, 2026
65 of 67 checks passed
@baktun14 baktun14 deleted the fix/stripe-errors branch January 20, 2026 03:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments