diff --git a/readthedocs/core/signals.py b/readthedocs/core/signals.py index b8510eed3fc..623a97e1900 100644 --- a/readthedocs/core/signals.py +++ b/readthedocs/core/signals.py @@ -12,6 +12,7 @@ from django.dispatch import receiver from future.backports.urllib.parse import urlparse +from readthedocs.oauth.models import RemoteOrganization from readthedocs.projects.models import Project, Domain log = logging.getLogger(__name__) @@ -79,12 +80,17 @@ def decide_if_cors(sender, request, **kwargs): # pylint: disable=unused-argumen 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 = instance.projects.all().annotate(num_users=Count('users')).exclude(num_users__gt=1) + # Add annotate before filter + # https://github.com/rtfd/readthedocs.org/pull/4577 + # https://docs.djangoproject.com/en/2.1/topics/db/aggregation/#order-of-annotate-and-filter-clauses # noqa + projects = (Project.objects.annotate(num_users=Count('users')).filter(users=instance.id) + .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 = (instance.oauth_organizations.annotate(num_users=Count('users')) - .exclude(num_users__gt=1)) + oauth_organizations = (RemoteOrganization.objects.annotate(num_users=Count('users')) + .filter(users=instance.id) + .exclude(num_users__gt=1)) projects.delete() oauth_organizations.delete() diff --git a/readthedocs/core/tests/__init__.py b/readthedocs/core/tests/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/readthedocs/core/tests/test_signals.py b/readthedocs/core/tests/test_signals.py new file mode 100644 index 00000000000..c38705a1f8c --- /dev/null +++ b/readthedocs/core/tests/test_signals.py @@ -0,0 +1,56 @@ +import pytest +import django_dynamic_fixture + +from django.contrib.auth.models import User + +from readthedocs.oauth.models import RemoteOrganization +from readthedocs.projects.models import Project + + +@pytest.mark.django_db +class TestProjectOrganizationSignal(object): + + @pytest.mark.parametrize('model_class', [Project, RemoteOrganization]) + def test_project_organization_get_deleted_upon_user_delete(self, model_class): + """ + If the user has Project or RemoteOrganization where he is the only user, + upon deleting his account, the Project or RemoteOrganization should also get + deleted. + """ + + obj = django_dynamic_fixture.get(model_class) + user1 = django_dynamic_fixture.get(User) + obj.users.add(user1) + + obj.refresh_from_db() + assert obj.users.all().count() == 1 + + # Delete the user + user1.delete() + # The object should not exist + obj = model_class.objects.all().filter(id=obj.id) + assert not obj.exists() + + @pytest.mark.parametrize('model_class', [Project, RemoteOrganization]) + def test_multiple_users_project_organization_not_delete(self, model_class): + """ + Check Project or RemoteOrganization which have multiple users do not get deleted + when any of the user delete his account. + """ + + obj = django_dynamic_fixture.get(model_class) + user1 = django_dynamic_fixture.get(User) + user2 = django_dynamic_fixture.get(User) + obj.users.add(user1, user2) + + obj.refresh_from_db() + assert obj.users.all().count() > 1 + # Delete 1 user of the project + user1.delete() + + # The project should still exist and it should have 1 user + obj.refresh_from_db() + assert obj.id + obj_users = obj.users.all() + assert len(obj_users) == 1 + assert user2 in obj_users