[Fix #4576] Do not delete projects which have multiple users

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()

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