fix(stripe): store customer ID in profile to prevent duplicate customers#842
Merged
Conversation
Contributor
WalkthroughReplace per-user Stripe customer storage on User with storage on UserMembership; update donation subscription flow to retrieve/create Stripe Customer via UserMembership, persist customer ID there, and add a test ensuring existing Stripe customers are reused. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server as Django View
participant DB as UserMembership
participant Stripe
Client->>Server: POST /create_donation_subscription (auth)
Server->>DB: lookup UserMembership for user
alt membership has stripe_customer_id
Server->>Stripe: Customer.retrieve(stripe_customer_id)
Stripe-->>Server: existing Customer
else no membership or retrieval error
Server->>Stripe: Customer.create(email...)
Stripe-->>Server: new Customer
Server->>DB: create/update UserMembership with customer id
end
Server->>Stripe: PaymentIntent.create(off_session, customer=...)
Stripe-->>Server: PaymentIntent
Server-->>Client: 200 OK (client_secret, ... )
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Fixes Stripe customer duplication during donation subscriptions by persisting and reusing a Stripe customer ID tied to a user’s Profile.
Changes:
- Add a new
StripeCustomermodel (OneToOne withProfile) to persiststripe_customer_id. - Update
create_donation_subscriptionto retrieve an existing Stripe customer viaStripeCustomer, otherwise create and persist one. - Register
StripeCustomerin Django admin for visibility/management.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
web/views.py |
Reuses an existing Stripe customer for authenticated users via StripeCustomer; persists newly created Stripe customer IDs. |
web/models.py |
Introduces StripeCustomer model linked to Profile with unique/indexed stripe_customer_id. |
web/admin.py |
Adds StripeCustomerAdmin with search/filter and select_related optimization. |
web/migrations/0064_stripecustomer.py |
Creates the StripeCustomer table and constraints. |
github-actions
Bot
added
files-changed: 5
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/views.py (1)
5539-5563:⚠️ Potential issue | 🟡 MinorFix unused
profile_createdvariables (lint).Both
get_or_createcalls unpack a value that isn't used. Prefix with_to satisfy linting.🧹 Proposed fix
- profile, profile_created = Profile.objects.get_or_create( + profile, _profile_created = Profile.objects.get_or_create( user=request.user, defaults={"is_teacher": False} ) @@ - profile, profile_created = Profile.objects.get_or_create( + profile, _profile_created = Profile.objects.get_or_create( user=request.user, defaults={"is_teacher": False} )
🤖 Fix all issues with AI agents
In `@web/migrations/0064_stripecustomer.py`:
- Around line 18-23: Remove the redundant db_index=True from the
"stripe_customer_id" CharField definition (models.CharField) in the model that
defines that field so only unique=True remains; then run makemigrations to
regenerate the migration (the redundant index will be dropped in the new
migration, replacing the change currently shown in 0064_stripecustomer.py).
In `@web/models.py`:
- Around line 211-213: Remove the redundant db_index=True on the
stripe_customer_id field in the model (leave max_length=100, unique=True and
help_text intact); locate the stripe_customer_id models.CharField declaration in
the model class in web/models.py and delete the db_index argument, then
regenerate and apply migrations (makemigrations/migrate) so the schema matches
the updated field definition.
In `@web/tests/test_donation_views.py`:
- Around line 15-28: The current test_reuses_existing_stripe_customer asserts
stripe.Customer.retrieve was called but doesn't verify no extra StripeCustomer
row was created; update this test to capture the StripeCustomer.count() before
the POST and assert it remains unchanged after the request (reference
StripeCustomer and test_reuses_existing_stripe_customer), and also assert the
mocked stripe.PaymentIntent.create was used and response contains expected
fields if applicable. Additionally add separate tests covering (1) new-customer
flow where no StripeCustomer exists (assert a new StripeCustomer is created and
stripe.Customer.retrieve is not called but stripe.Customer.create /
PaymentIntent.create is), (2) unauthenticated POST to
create_donation_subscription returning the proper auth error, and (3) Stripe API
failure handling by having stripe.Customer.retrieve or
stripe.PaymentIntent.create raise an exception and asserting the view returns
the correct error response; use mocks on stripe.Customer.retrieve,
stripe.Customer.create, and stripe.PaymentIntent.create to simulate these
scenarios.
- Around line 9-13: Add missing docstrings: add a module-level docstring at the
top of web/tests/test_donation_views.py describing the test module purpose, then
add a class docstring to CreateDonationSubscriptionViewTests summarizing what
the test class verifies (e.g., creation of donation subscriptions and related
behaviors), and add brief docstrings to any test methods (including setUp if
desired) explaining their role; update CreateDonationSubscriptionViewTests and
its setUp method to include these docstrings so the file complies with the
project rule requiring docstrings on modules, classes, and functions.
In `@web/views.py`:
- Around line 5537-5547: The except block handling access to
profile.stripe_customer should also catch StripeCustomer.DoesNotExist to avoid
crashing when an authenticated user has no related StripeCustomer; update the
try/except around Profile.objects.get_or_create / profile.stripe_customer /
stripe.Customer.retrieve to include StripeCustomer.DoesNotExist alongside
AttributeError and stripe.error.InvalidRequestError (reference Profile,
profile.stripe_customer, StripeCustomer.DoesNotExist, and
stripe.Customer.retrieve).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@web/tests/test_donation_views.py`:
- Around line 11-18: The test's setUp in CreateDonationSubscriptionViewTests
depends on a Profile being auto-created (likely via a post_save signal) and uses
Profile.objects.get_or_create to ensure one exists; add a brief inline comment
or a class-level docstring near setUp or above the
CreateDonationSubscriptionViewTests class explaining that the project
auto-creates Profile instances via a post_save signal and that get_or_create is
used here to tolerate that behavior and avoid test flakes, referencing
Profile.objects.get_or_create so future maintainers know why this pattern is
used.
- Around line 20-33: The test test_reuses_existing_stripe_customer currently
patches stripe.PaymentIntent.create as mock_payment_intent but doesn't assert it
was invoked; update the test to assert that mock_payment_intent was called and
that its call includes the reused customer ID (stripe.Customer.retrieve returns
id "cus_existing") — reference the patched symbols stripe.Customer.retrieve and
stripe.PaymentIntent.create and add an assertion using
mock_payment_intent.assert_called_once_with (or assert_any_call) that verifies
the customer argument equals "cus_existing" (and include other expected args
like amount/currency if required by the view).
In `@web/views.py`:
- Around line 5555-5561: The code is creating a UserMembership with a hard-coded
plan_id=1 which is brittle; change the logic in the UserMembership
creation/update block so you do not create a new membership with a fixed
plan_id: instead try to fetch an existing UserMembership for request.user and
only update its stripe_customer_id if found (use the existing membership
variable returned by UserMembership.objects.get or filter), or if creation is
required resolve a proper donor/default plan via the Plan lookup (e.g., query
Plan by a stable key/name like "donor" or a configured DEFAULT_DONOR_PLAN) and
handle Plan.DoesNotExist by failing gracefully or skipping plan assignment;
update references to membership.stripe_customer_id and membership.save()
accordingly so you never rely on plan_id=1.
Contributor
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@web/views.py`:
- Around line 5639-5645: Don't create a UserMembership with a hard-coded
plan_id; change the get_or_create/membership logic in this block so you only
create membership with no brittle defaults or you resolve the intended plan via
a stable key: call UserMembership.objects.get_or_create(user=request.user) (no
plan_id default), then if a plan must be assigned look up the Plan by a stable
identifier (e.g., Plan.objects.get(key='donor') or a configured
DEFAULT_PLAN_KEY) and handle Plan.DoesNotExist by skipping assignment or using a
safe fallback; also keep the existing stripe_customer_id assignment/update on
the resulting membership (update membership.stripe_customer_id and
membership.save()) so you don't silently create memberships with plan_id=1.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issues
Fixes #838
Checklist
Problem
The create_donation_subscription function was trying to save stripe_customer_id on the User model, but this field doesn't exist. This caused duplicate Stripe customers to be created for each donation.
Solution
Used the existing UserMembership.stripe_customer_id field instead of creating a new model.
Summary by CodeRabbit
Tests
Bug Fixes