Implement user account deletion with data purge #99
Conversation
Co-authored-by: me <[email protected]>
|
Cursor Agent can help with this pull request. Just |
WalkthroughThis PR introduces a complete account deletion feature. It adds a new POST endpoint that cancels active Stripe subscriptions, logs deletion events via PostHog if configured, and removes user accounts with cascade deletes. A "Danger Zone" UI section in user settings provides a confirmation dialog. Changes
Sequence DiagramsequenceDiagram
participant User
participant Frontend as User Settings UI
participant Backend as delete_account View
participant Stripe
participant PostHog
participant DB
User->>Frontend: Click Delete Account
Frontend->>User: Show Confirmation Dialog
User->>Frontend: Confirm Deletion
Frontend->>Backend: POST /delete-account/
rect rgb(200, 220, 255)
note over Backend: Validate & Process
Backend->>Stripe: Cancel Active Subscription
alt Subscription Found
Stripe-->>Backend: Cancellation OK
else No Subscription
Stripe-->>Backend: N/A
end
end
rect rgb(220, 200, 255)
note over Backend: Event Tracking
Backend->>PostHog: async_task(account_deleted)
PostHog-->>Backend: Queued
end
rect rgb(200, 230, 200)
note over Backend: Delete & Cleanup
Backend->>DB: Delete User + Cascade
DB-->>Backend: Deleted
end
Backend-->>Frontend: Redirect to Landing
Frontend->>User: Navigate Away
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
Implements user account deletion with comprehensive data purge functionality. The feature allows users to permanently delete their accounts through a new settings UI with confirmation dialog.
Key Changes:
- Added
delete_accountview that cancels Stripe subscriptions, tracks deletion events in PostHog, and leverages Django's CASCADE deletion to remove all user data (projects, blog posts, keywords, competitors, feedback) - Added URL route for the delete account endpoint
- Introduced "Danger Zone" UI section in user settings with a native
<dialog>element for confirmation - Clear user communication about what data will be deleted (projects, blog posts, subscription, settings)
Issues Found:
- Missing user logout after account deletion - user session remains active after their account is deleted
- Using broad exception catching (
StripeError) instead of specific Stripe exceptions
Confidence Score: 3/5
- Safe to merge with one critical fix needed
- The implementation correctly handles Stripe subscription cancellation and data deletion via CASCADE constraints. However, there's a critical issue where the user session is not properly terminated after account deletion, which could lead to session errors or security concerns. The broad exception handling is a style issue but doesn't affect functionality.
core/views.pyneeds attention - must add logout call after user deletion
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| core/views.py | 3/5 | Added delete_account view with Stripe subscription cancellation, PostHog tracking, and cascade deletion. Missing logout after deletion and using broad exception catching. |
| core/urls.py | 5/5 | Added URL route for delete_account view. Simple and correct addition. |
| frontend/templates/pages/user-settings.html | 5/5 | Added danger zone section with delete account UI using native dialog element. Clear warnings and proper CSRF protection. |
Sequence Diagram
sequenceDiagram
participant User
participant Browser
participant DeleteAccountView
participant StripeAPI
participant Database
participant PostHog
User->>Browser: Click "Delete Account"
Browser->>Browser: Show confirmation dialog
User->>Browser: Confirm deletion
Browser->>DeleteAccountView: POST /delete-account/
DeleteAccountView->>DeleteAccountView: Check request method
DeleteAccountView->>DeleteAccountView: Get user and profile
DeleteAccountView->>DeleteAccountView: Log deletion initiation
alt Subscription exists and is active/trialing
DeleteAccountView->>StripeAPI: stripe.Subscription.delete(subscription_id)
alt Stripe cancellation succeeds
StripeAPI-->>DeleteAccountView: Success
DeleteAccountView->>DeleteAccountView: Log success
else Stripe cancellation fails
StripeAPI-->>DeleteAccountView: StripeError
DeleteAccountView->>DeleteAccountView: Log error (continue)
end
end
alt PostHog configured
DeleteAccountView->>PostHog: async_task(track_event, "account_deleted")
Note over PostHog: Async task queued
end
DeleteAccountView->>Database: user.delete()
Note over Database: CASCADE deletes:<br/>Profile, Projects, BlogPosts,<br/>Keywords, Competitors, etc.
Database-->>DeleteAccountView: Deletion complete
DeleteAccountView->>DeleteAccountView: Log successful deletion
DeleteAccountView->>Browser: Redirect to landing page
Browser->>User: Show success message
3 files reviewed, 2 comments
| except stripe.error.StripeError as e: | ||
| logger.error( | ||
| "[DeleteAccount] Failed to cancel Stripe subscription", | ||
| user_id=user.id, | ||
| error=str(e), | ||
| exc_info=True, | ||
| ) |
There was a problem hiding this comment.
style: catching broad stripe.error.StripeError instead of specific exceptions - violates error handling guideline to avoid catching Exception or overly broad exceptions
Context Used: Context from dashboard - .cursor/rules/backend-error-handling.mdc (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: core/views.py
Line: 247:253
Comment:
**style:** catching broad `stripe.error.StripeError` instead of specific exceptions - violates error handling guideline to avoid catching `Exception` or overly broad exceptions
**Context Used:** Context from `dashboard` - .cursor/rules/backend-error-handling.mdc ([source](https://app.greptile.com/review/custom-context?memory=9343d853-8bca-46ca-b37d-fd4327d3e3d2))
How can I resolve this? If you propose a fix, please make it concise.| username = user.username | ||
| email = user.email | ||
|
|
||
| user.delete() |
There was a problem hiding this comment.
logic: user session not cleared before redirect - user remains authenticated after account deletion, which could cause session errors or security issues
| user.delete() | |
| user.delete() | |
| logout(request) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: core/views.py
Line: 287:287
Comment:
**logic:** user session not cleared before redirect - user remains authenticated after account deletion, which could cause session errors or security issues
```suggestion
user.delete()
logout(request)
```
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
frontend/templates/pages/user-settings.html (1)
389-398: Consider refactoring to use Stimulus JS instead of inline onclick.The inline
onclickhandler works but doesn't follow the project's preference for Stimulus JS controllers. Consider creating a Stimulus controller to handle the dialog interaction.As per coding guidelines.
You could create a simple controller:
// delete_account_controller.js import { Controller } from "@hotwired/stimulus" export default class extends Controller { static targets = ["dialog"] open(event) { event.preventDefault() this.dialogTarget.showModal() } close(event) { event.preventDefault() this.dialogTarget.close() } }Then update the HTML:
-<button - type="button" - onclick="document.getElementById('delete-account-dialog').showModal()" - class="inline-flex items-center rounded-md border border-red-300 bg-white px-4 py-2 text-sm font-medium text-red-700 shadow-sm hover:bg-red-50 focus:outline-none focus:ring-2 focus:ring-red-500 focus:ring-offset-2" -> +<div data-controller="delete-account"> + <button + type="button" + data-action="click->delete-account#open" + class="inline-flex items-center rounded-md border border-red-300 bg-white px-4 py-2 text-sm font-medium text-red-700 shadow-sm hover:bg-red-50 focus:outline-none focus:ring-2 focus:ring-red-500 focus:ring-offset-2" + >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
core/urls.py(1 hunks)core/views.py(1 hunks)frontend/templates/pages/user-settings.html(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.html
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
**/*.html: Prefer Stimulus JS for adding interactivity to Django templates instead of raw script elements
Leverage Stimulus data attributes to connect HTML elements with JavaScript functionality
Utilize Stimulus targets to reference specific elements within a controller
Employ Stimulus actions to handle user interactions and events
Files:
frontend/templates/pages/user-settings.html
**/*.{html,htm}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Always generate semantic HTML
Files:
frontend/templates/pages/user-settings.html
{**/*.{html,htm,css,scss},**/*_controller.@(js|ts)}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Always favor the utility-first Tailwind approach; avoid creating reusable style classes and prefer reuse via template components
Files:
frontend/templates/pages/user-settings.html
frontend/templates/**/*.html
📄 CodeRabbit inference engine (.cursor/rules/stimulus-events.mdc)
Avoid data-*-outlet links for sibling controller communication when using the event-based approach; keep controllers self-contained
Use semantic HTML elements (e.g., dialog, details/summary) in templates
Files:
frontend/templates/pages/user-settings.html
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/backend.mdc)
**/*.py: Prioritize readability and maintainability; follow PEP 8
Use descriptive variable/function names with underscoresIn Python, use try-except blocks that catch specific exception types; do not catch broad Exception.
**/*.py: Follow PEP 8 for Python code style
Use descriptive variable names with underscores (snake_case) in Python
Prefer Django's built-in features over external libraries
**/*.py: Use structlog for logging in Python code (avoid print and the standard logging module)
Include contextual key-value fields in log messages (e.g., error=str(e), exc_info=True, profile_id=profile.id)
**/*.py: Use descriptive, full-word variable names; avoid abbreviations and single-letter variables in Python
Provide context in variable names when format or type matters (e.g., include_iso_format,date)
Extract unchanging values into UPPER_CASE constants
Use intermediary variables to name parsed groups instead of using index access directly
Naming conventions: use is_/has_/can_ for booleans; include 'date' for dates; snake_case for variables/functions; PascalCase for classes
Define variables close to where they are used to keep lifespan short
Name things after what they do, not how they're used; ensure names make sense without extra context
Avoid generic names like data, info, manager; use specific, intention-revealing names
Function names should include necessary context without being verbose
If naming is hard, split functions into smaller focused parts
Maintain consistency: reuse the same verbs and nouns for the same concepts; name variables after the functions that create them
Use more descriptive names for longer-lived variables
Avoid else statements by using guard clauses for early returns
Replace simple conditionals with direct assignment when both branches call the same function with different values
Use dictionaries as dispatch tables instead of multiple equal-probability elif chains
Validate input before processing to prevent errors propagating in...
Files:
core/views.pycore/urls.py
core/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/backend.mdc)
Leverage Django's ORM; avoid raw SQL when possible
Files:
core/views.pycore/urls.py
core/views.py
📄 CodeRabbit inference engine (CLAUDE.md)
Keep Django views skinny: handle request/response only
Files:
core/views.py
🧬 Code graph analysis (2)
core/views.py (1)
core/tasks.py (1)
track_event(338-368)
core/urls.py (1)
core/views.py (1)
delete_account(221-300)
🔇 Additional comments (4)
core/urls.py (1)
40-40: LGTM!The URL pattern is correctly configured and appropriately placed in the utils section.
frontend/templates/pages/user-settings.html (1)
409-475: Excellent use of semantic HTML with the dialog element.The confirmation dialog provides clear information about what will be deleted and properly uses CSRF protection. The same Stimulus JS refactoring suggestion from the previous comment applies to the close button on line 459.
As per coding guidelines.
core/views.py (2)
223-225: Good security practice with POST-only enforcement.Correctly restricts the dangerous delete operation to POST requests only, providing CSRF protection.
271-287: Excellent documentation of cascade delete behavior.The comprehensive comment listing all related data that will be cascade-deleted is very helpful for future maintainers. The approach of saving username and email before deletion is correct.
| logger.info( | ||
| "[DeleteAccount] User initiated account deletion", | ||
| user_id=user.id, | ||
| profile_id=profile.id, | ||
| email=user.email, | ||
| ) |
There was a problem hiding this comment.
Remove PII from logs to ensure compliance.
The email address is being logged, which violates privacy best practices. Email addresses are considered PII and should not be retained in logs due to GDPR/CCPA compliance concerns.
As per coding guidelines.
Apply this diff:
logger.info(
"[DeleteAccount] User initiated account deletion",
user_id=user.id,
profile_id=profile.id,
- email=user.email,
)🤖 Prompt for AI Agents
In core/views.py around lines 230 to 235, the logger call is including the
user's email (PII); remove the email field from the log and instead log non-PII
identifiers only (e.g., user_id and profile_id) or, if needed for
troubleshooting, log a redacted/hashed email (apply a one-way hash or mask) so
raw email addresses are never written to logs; update the logger.info invocation
to omit the email key or replace it with a hashed_email/masked_email value
generated server-side before logging.
| # Cancel Stripe subscription if it exists | ||
| if profile.subscription and profile.subscription.status in ["active", "trialing"]: | ||
| try: | ||
| subscription_id = profile.subscription.id | ||
| stripe.Subscription.delete(subscription_id) | ||
| logger.info( | ||
| "[DeleteAccount] Cancelled Stripe subscription", | ||
| user_id=user.id, | ||
| subscription_id=subscription_id, | ||
| ) | ||
| except stripe.error.StripeError as e: | ||
| logger.error( | ||
| "[DeleteAccount] Failed to cancel Stripe subscription", | ||
| user_id=user.id, | ||
| error=str(e), | ||
| exc_info=True, | ||
| ) |
There was a problem hiding this comment.
Critical: Account deletion proceeds even if Stripe cancellation fails.
If the Stripe subscription cancellation fails (line 241), the error is logged but the function continues to delete the account. This could leave active subscriptions orphaned in Stripe, leading to billing issues and potential compliance problems.
Additionally, the error log includes the user's email, which is PII and should not be logged.
As per coding guidelines.
Consider one of these approaches:
Option 1: Halt deletion on Stripe failure (recommended for active subscriptions)
- except stripe.error.StripeError as e:
+ except stripe.error.StripeError as e:
logger.error(
"[DeleteAccount] Failed to cancel Stripe subscription",
user_id=user.id,
- error=str(e),
+ error=str(e),
exc_info=True,
)
+ messages.error(
+ request,
+ "Unable to cancel your subscription. Please contact support before deleting your account."
+ )
+ return redirect(reverse("settings"))Option 2: Continue but warn user
except stripe.error.StripeError as e:
logger.error(
"[DeleteAccount] Failed to cancel Stripe subscription",
user_id=user.id,
- error=str(e),
exc_info=True,
)
+ messages.warning(
+ request,
+ "Your account was deleted but we couldn't cancel your subscription. Please contact support."
+ )Also remove the email from logs:
- logger.error(
- "[DeleteAccount] Failed to cancel Stripe subscription",
- user_id=user.id,
- error=str(e),
- exc_info=True,
- )
+ logger.error(
+ "[DeleteAccount] Failed to cancel Stripe subscription",
+ user_id=user.id,
+ subscription_id=subscription_id,
+ error=str(e),
+ exc_info=True,
+ )| # Track deletion event before deleting | ||
| if settings.POSTHOG_API_KEY: | ||
| async_task( | ||
| track_event, | ||
| profile_id=profile.id, | ||
| event_name="account_deleted", | ||
| properties={ | ||
| "$set": { | ||
| "email": user.email, | ||
| "username": user.username, | ||
| }, | ||
| }, | ||
| source_function="delete_account", | ||
| group="Track Event", | ||
| ) |
There was a problem hiding this comment.
Critical race condition: PostHog tracking will likely fail.
The async_task for PostHog tracking is queued before the user deletion, but it will execute asynchronously. The track_event function (from core/tasks.py lines 337-367) attempts to fetch the Profile:
profile = Profile.objects.get(id=profile_id)However, user.delete() on line 287 will cascade-delete the Profile. When the async task executes, the Profile will likely already be deleted, causing the tracking to fail with a Profile.DoesNotExist error.
Apply this diff to track the event synchronously before deletion:
- # Track deletion event before deleting
- if settings.POSTHOG_API_KEY:
- async_task(
- track_event,
- profile_id=profile.id,
- event_name="account_deleted",
- properties={
- "$set": {
- "email": user.email,
- "username": user.username,
- },
- },
- source_function="delete_account",
- group="Track Event",
- )
+ # Track deletion event before deleting (synchronous to ensure it completes)
+ if settings.POSTHOG_API_KEY:
+ try:
+ import posthog
+ posthog.capture(
+ user.email,
+ event="account_deleted",
+ properties={
+ "profile_id": profile.id,
+ "email": user.email,
+ "current_state": profile.state,
+ "username": user.username,
+ },
+ )
+ except Exception as e:
+ logger.error(
+ "[DeleteAccount] Failed to track deletion event",
+ user_id=user.id,
+ error=str(e),
+ exc_info=True,
+ )Note: You'll need to add import posthog at the top of the file or in the function.
🤖 Prompt for AI Agents
In core/views.py around lines 255 to 269, queuing the PostHog tracking via
async_task creates a race because the background task fetches Profile by id but
the subsequent user.delete() will cascade-delete that Profile; instead, import
posthog and call PostHog synchronously before deleting the user (build the same
event properties using user.email and user.username and call
posthog.capture/identify as appropriate), then proceed with user.delete();
remove or avoid scheduling the async_task for this event to prevent
Profile.DoesNotExist errors.
| logger.info( | ||
| "[DeleteAccount] Successfully deleted user account", | ||
| username=username, | ||
| email=email, | ||
| ) |
There was a problem hiding this comment.
Remove PII from success logs.
The email address should not be logged due to privacy compliance concerns (GDPR/CCPA).
As per coding guidelines.
Apply this diff:
logger.info(
"[DeleteAccount] Successfully deleted user account",
username=username,
- email=email,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logger.info( | |
| "[DeleteAccount] Successfully deleted user account", | |
| username=username, | |
| email=email, | |
| ) | |
| logger.info( | |
| "[DeleteAccount] Successfully deleted user account", | |
| username=username, | |
| ) |
🤖 Prompt for AI Agents
In core/views.py around lines 289-293, the success log currently includes the
user's email which is PII; remove the email from the logger call and replace it
with a non-PII identifier (e.g., user_id or internal ID) or omit the field
entirely. Update the logger invocation to no longer pass email=email and, if
available, add user_id=user.id (or username only if allowed by policy) to
satisfy auditing without logging the email.
This pull request contains changes generated by a Cursor Cloud Agent
Summary by CodeRabbit