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

Author: safwanrahman

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,16 @@ 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://bit.ly/2Nne6ZJ
+    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/__init__.py

@@ -0,0 +1,35 @@
+import pytest
+
+from django.contrib.auth.models import User
+from django_dynamic_fixture import G
+
+from readthedocs.oauth.models import RemoteOrganization
+from readthedocs.projects.models import Project
+
+
+@pytest.mark.django_db
+class TestProjectOrganizationSignal(object):
+
+    @pytest.mark.parametrize('model', [Project, RemoteOrganization])
+    def test_multiple_users_project_organization_not_delete(self, model):
+        """
+        Check Project or RemoteOrganization which have multiple users do not get deleted
+        when any of the user delete his account.
+        """
+
+        obj = G(model)
+        user1 = G(User)
+        user2 = G(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