From c48f6114c00e7aa5e62424d4ddf26544229ecb06 Mon Sep 17 00:00:00 2001 From: Safwan Rahman Date: Wed, 1 Nov 2017 01:01:13 +0600 Subject: [PATCH 1/4] [Fix #3182] Better user deletion --- readthedocs/core/signals.py | 28 +++++++++++++++++-- readthedocs/oauth/tasks.py | 6 ++++ readthedocs/projects/tasks.py | 5 ++++ .../profiles/private/delete_account.html | 2 ++ 4 files changed, 39 insertions(+), 2 deletions(-) diff --git a/readthedocs/core/signals.py b/readthedocs/core/signals.py index bbef98952e4..1556058e744 100644 --- a/readthedocs/core/signals.py +++ b/readthedocs/core/signals.py @@ -5,14 +5,19 @@ import logging from corsheaders import signals +from django.contrib.auth import get_user_model +from django.db.models.signals import pre_delete from django.dispatch import Signal -from django.db.models import Q +from django.db.models import Q, Count +from django.dispatch import receiver from future.backports.urllib.parse import urlparse +from readthedocs.oauth.tasks import bulk_delete_oauth_remote_organizations from readthedocs.projects.models import Project, Domain - +from readthedocs.projects.tasks import bulk_delete_projects log = logging.getLogger(__name__) +User = get_user_model() WHITELIST_URLS = ['/api/v2/footer_html', '/api/v2/search', '/api/v2/docsearch'] @@ -62,4 +67,23 @@ def decide_if_cors(sender, request, **kwargs): # pylint: disable=unused-argumen return False + +@receiver(pre_delete, sender=User) +def delete_projects_and_organizations(instance, *args, **kwargs): + # Here we count the owner list from the projects that the user own + # Then exclude the projects where there are more than one owner + projects_id = (instance.projects.all().annotate(num_users=Count('users')) + .exclude(num_users__gt=1) + .values_list('id', flat=True)) + + # Here we count the users list from the organization that the user belong + # Then exclude the organizations where there are more than one user + oauth_organizations_id = (instance.oauth_organizations.annotate(num_users=Count('users')) + .exclude(num_users__gt=1) + .values_list('id', flat=True)) + + bulk_delete_projects.delay(projects_id) + bulk_delete_oauth_remote_organizations.delay(oauth_organizations_id) + + signals.check_request_enabled.connect(decide_if_cors) diff --git a/readthedocs/oauth/tasks.py b/readthedocs/oauth/tasks.py index b345d443046..151dce8895b 100644 --- a/readthedocs/oauth/tasks.py +++ b/readthedocs/oauth/tasks.py @@ -1,12 +1,14 @@ """Tasks for OAuth services""" from __future__ import absolute_import +from celery import task from django.contrib.auth.models import User from djcelery import celery as celery_app from readthedocs.core.utils.tasks import PublicTask from readthedocs.core.utils.tasks import permission_check from readthedocs.core.utils.tasks import user_id_matches +from readthedocs.oauth.models import RemoteOrganization from .services import registry @@ -21,5 +23,9 @@ def run_public(self, user_id): for service in service_cls.for_user(user): service.sync() +@task() +def bulk_delete_oauth_remote_organizations(organizations_id): + RemoteOrganization.objects.filter(id__in=organizations_id).delete() + sync_remote_repositories = celery_app.tasks[SyncRemoteRepositories.name] diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 7d50ea0881c..9000d329d9c 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -984,3 +984,8 @@ def clear_html_artifacts(version): if isinstance(version, int): version = Version.objects.get(pk=version) remove_dir(version.project.rtd_build_path(version=version.slug)) + + +@task() +def bulk_delete_projects(projects_id): + Project.objects.filter(id__in=projects_id).delete() diff --git a/readthedocs/templates/profiles/private/delete_account.html b/readthedocs/templates/profiles/private/delete_account.html index 6a76f5c7576..0b7696132e5 100644 --- a/readthedocs/templates/profiles/private/delete_account.html +++ b/readthedocs/templates/profiles/private/delete_account.html @@ -12,6 +12,8 @@
{% csrf_token %} {{ form }} +
+ Be careful! This can not be undone!
{% endblock %} From 15a36c0f52109f8729eefe3036fbd3d955954aa5 Mon Sep 17 00:00:00 2001 From: Safwan Rahman Date: Wed, 1 Nov 2017 02:42:28 +0600 Subject: [PATCH 2/4] fixup according to comments --- readthedocs/core/signals.py | 21 +++++++------------ readthedocs/oauth/tasks.py | 6 ------ readthedocs/projects/tasks.py | 5 ----- .../profiles/private/delete_account.html | 5 +++-- 4 files changed, 10 insertions(+), 27 deletions(-) diff --git a/readthedocs/core/signals.py b/readthedocs/core/signals.py index 1556058e744..3f7f89192bb 100644 --- a/readthedocs/core/signals.py +++ b/readthedocs/core/signals.py @@ -5,19 +5,16 @@ import logging from corsheaders import signals -from django.contrib.auth import get_user_model +from django.conf import settings from django.db.models.signals import pre_delete from django.dispatch import Signal from django.db.models import Q, Count from django.dispatch import receiver from future.backports.urllib.parse import urlparse -from readthedocs.oauth.tasks import bulk_delete_oauth_remote_organizations from readthedocs.projects.models import Project, Domain -from readthedocs.projects.tasks import bulk_delete_projects log = logging.getLogger(__name__) -User = get_user_model() WHITELIST_URLS = ['/api/v2/footer_html', '/api/v2/search', '/api/v2/docsearch'] @@ -68,22 +65,18 @@ def decide_if_cors(sender, request, **kwargs): # pylint: disable=unused-argumen return False -@receiver(pre_delete, sender=User) -def delete_projects_and_organizations(instance, *args, **kwargs): +@receiver(pre_delete, sender=settings.AUTH_USER_MODEL) +def delete_projects_and_organizations(sender, instance, *args, **kwargs): # Here we count the owner list from the projects that the user own # Then exclude the projects where there are more than one owner - projects_id = (instance.projects.all().annotate(num_users=Count('users')) - .exclude(num_users__gt=1) - .values_list('id', flat=True)) + projects = instance.projects.all().annotate(num_users=Count('users')).exclude(num_users__gt=1) # Here we count the users list from the organization that the user belong # Then exclude the organizations where there are more than one user - oauth_organizations_id = (instance.oauth_organizations.annotate(num_users=Count('users')) - .exclude(num_users__gt=1) - .values_list('id', flat=True)) + oauth_organizations = instance.oauth_organizations.annotate(num_users=Count('users')).exclude(num_users__gt=1) - bulk_delete_projects.delay(projects_id) - bulk_delete_oauth_remote_organizations.delay(oauth_organizations_id) + projects.delete() + oauth_organizations.delete() signals.check_request_enabled.connect(decide_if_cors) diff --git a/readthedocs/oauth/tasks.py b/readthedocs/oauth/tasks.py index 151dce8895b..b345d443046 100644 --- a/readthedocs/oauth/tasks.py +++ b/readthedocs/oauth/tasks.py @@ -1,14 +1,12 @@ """Tasks for OAuth services""" from __future__ import absolute_import -from celery import task from django.contrib.auth.models import User from djcelery import celery as celery_app from readthedocs.core.utils.tasks import PublicTask from readthedocs.core.utils.tasks import permission_check from readthedocs.core.utils.tasks import user_id_matches -from readthedocs.oauth.models import RemoteOrganization from .services import registry @@ -23,9 +21,5 @@ def run_public(self, user_id): for service in service_cls.for_user(user): service.sync() -@task() -def bulk_delete_oauth_remote_organizations(organizations_id): - RemoteOrganization.objects.filter(id__in=organizations_id).delete() - sync_remote_repositories = celery_app.tasks[SyncRemoteRepositories.name] diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 9000d329d9c..7d50ea0881c 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -984,8 +984,3 @@ def clear_html_artifacts(version): if isinstance(version, int): version = Version.objects.get(pk=version) remove_dir(version.project.rtd_build_path(version=version.slug)) - - -@task() -def bulk_delete_projects(projects_id): - Project.objects.filter(id__in=projects_id).delete() diff --git a/readthedocs/templates/profiles/private/delete_account.html b/readthedocs/templates/profiles/private/delete_account.html index 0b7696132e5..567eff17cfb 100644 --- a/readthedocs/templates/profiles/private/delete_account.html +++ b/readthedocs/templates/profiles/private/delete_account.html @@ -12,8 +12,9 @@
{% csrf_token %} {{ form }} -
- Be careful! This can not be undone! +
+ {% trans "Be careful! This can not be undone!" %} +
{% endblock %} From 71c063ead3a2d85d77589c6c41f81413327c586f Mon Sep 17 00:00:00 2001 From: Safwan Rahman Date: Sat, 4 Nov 2017 00:46:54 +0600 Subject: [PATCH 3/4] Delete user after user ask to get deleted --- readthedocs/profiles/views.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/readthedocs/profiles/views.py b/readthedocs/profiles/views.py index b8cecfa7b7b..7f9e9429c50 100644 --- a/readthedocs/profiles/views.py +++ b/readthedocs/profiles/views.py @@ -196,11 +196,9 @@ def delete_account(request): if request.method == 'POST': form = UserDeleteForm(instance=request.user, data=request.POST) if form.is_valid(): - - # Do not delete the account permanently because it may create disaster - # Inactive the user instead. - request.user.is_active = False - request.user.save() + # Delete the user permanently + # It will also delete some projects where he is the only owner + request.user.delete() logout(request) messages.info(request, 'You have successfully deleted your account') From 547d61c021da6706e693dfca747f09987682d1bd Mon Sep 17 00:00:00 2001 From: Safwan Rahman Date: Fri, 1 Dec 2017 01:20:36 +0600 Subject: [PATCH 4/4] fixing lint --- readthedocs/core/signals.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/readthedocs/core/signals.py b/readthedocs/core/signals.py index 3f7f89192bb..876f0d785ff 100644 --- a/readthedocs/core/signals.py +++ b/readthedocs/core/signals.py @@ -73,7 +73,8 @@ def delete_projects_and_organizations(sender, instance, *args, **kwargs): # Here we count the users list from the organization that the user belong # Then exclude the organizations where there are more than one user - oauth_organizations = instance.oauth_organizations.annotate(num_users=Count('users')).exclude(num_users__gt=1) + oauth_organizations = (instance.oauth_organizations.annotate(num_users=Count('users')) + .exclude(num_users__gt=1)) projects.delete() oauth_organizations.delete()