Implement user account deletion with data purge #99
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -217,6 +217,89 @@ def get_context_data(self, **kwargs): | |||||||||||||||||||
| return context | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| @login_required | ||||||||||||||||||||
| def delete_account(request): | ||||||||||||||||||||
| """Delete user account and all associated data.""" | ||||||||||||||||||||
| if request.method != "POST": | ||||||||||||||||||||
| messages.error(request, "Invalid request method.") | ||||||||||||||||||||
| return redirect(reverse("settings")) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| user = request.user | ||||||||||||||||||||
| profile = user.profile | ||||||||||||||||||||
|
|
||||||||||||||||||||
| logger.info( | ||||||||||||||||||||
| "[DeleteAccount] User initiated account deletion", | ||||||||||||||||||||
| user_id=user.id, | ||||||||||||||||||||
| profile_id=profile.id, | ||||||||||||||||||||
| email=user.email, | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| # 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, | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
Comment on lines
+247
to
+253
Contributor
There was a problem hiding this comment. style: catching broad Context Used: Context from Prompt To Fix With AIThis 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.
Comment on lines
+237
to
+253
Contributor
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", | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
Comment on lines
+255
to
+269
Contributor
There was a problem hiding this comment. Critical race condition: PostHog tracking will likely fail. The profile = Profile.objects.get(id=profile_id)However, 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 🤖 Prompt for AI Agents |
||||||||||||||||||||
|
|
||||||||||||||||||||
| # Django will cascade delete all related data: | ||||||||||||||||||||
| # - Profile (CASCADE) | ||||||||||||||||||||
| # - Projects (CASCADE via Profile) | ||||||||||||||||||||
| # - BlogPostTitleSuggestion (CASCADE via Project) | ||||||||||||||||||||
| # - GeneratedBlogPost (CASCADE via Project) | ||||||||||||||||||||
| # - ProjectPage (CASCADE via Project) | ||||||||||||||||||||
| # - Competitor (CASCADE via Project) | ||||||||||||||||||||
| # - ProjectKeyword (CASCADE via Project) | ||||||||||||||||||||
| # - AutoSubmissionSetting (CASCADE via Project) | ||||||||||||||||||||
| # - Feedback (CASCADE via Profile) | ||||||||||||||||||||
| # - ProfileStateTransition (SET_NULL via Profile) | ||||||||||||||||||||
| # - EmailAddress objects (CASCADE via allauth) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| username = user.username | ||||||||||||||||||||
| email = user.email | ||||||||||||||||||||
|
|
||||||||||||||||||||
| user.delete() | ||||||||||||||||||||
|
Contributor
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
Suggested change
Prompt To Fix With AIThis 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. |
||||||||||||||||||||
|
|
||||||||||||||||||||
| logger.info( | ||||||||||||||||||||
| "[DeleteAccount] Successfully deleted user account", | ||||||||||||||||||||
| username=username, | ||||||||||||||||||||
| email=email, | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
Comment on lines
+289
to
+293
Contributor
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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||
|
|
||||||||||||||||||||
| messages.success( | ||||||||||||||||||||
| request, | ||||||||||||||||||||
| "Your account and all associated data have been permanently deleted. We're sorry to see you go!", | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| return redirect(reverse("landing")) | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| @login_required | ||||||||||||||||||||
| def resend_confirmation_email(request): | ||||||||||||||||||||
| user = request.user | ||||||||||||||||||||
|
|
||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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