Merge remote-tracking branch 'origin/remote-repository-normalization' into rel

Author: humitos

readthedocs/api/v2/serializers.py

@@ -177,7 +177,7 @@ class RemoteOrganizationSerializer(serializers.ModelSerializer):
 
     class Meta:
         model = RemoteOrganization
-        exclude = ('json', 'email', 'users')
+        exclude = ('email', 'users',)
 
 
 class RemoteRepositorySerializer(serializers.ModelSerializer):
@@ -188,16 +188,30 @@ class RemoteRepositorySerializer(serializers.ModelSerializer):
 
     # This field does create an additional query per object returned
     matches = serializers.SerializerMethodField()
+    admin = serializers.SerializerMethodField('is_admin')
 
     class Meta:
         model = RemoteRepository
-        exclude = ('json', 'users')
+        exclude = ('users',)
 
     def get_matches(self, obj):
         request = self.context['request']
         if request.user is not None and request.user.is_authenticated:
             return obj.matches(request.user)
 
+    def is_admin(self, obj):
+        request = self.context['request']
+
+        # Use annotated value from RemoteRepositoryViewSet queryset
+        if hasattr(obj, 'admin'):
+            return obj.admin
+
+        if request.user and request.user.is_authenticated:
+            return obj.remote_repository_relations.filter(
+                user=request.user, admin=True
+            ).exists()
+        return False
+
 
 class ProviderSerializer(serializers.Serializer):
 

readthedocs/api/v2/views/model_views.py

@@ -5,6 +5,7 @@
 
 from allauth.socialaccount.models import SocialAccount
 from django.conf import settings
+from django.db.models import BooleanField, Case, Value, When
 from django.shortcuts import get_object_or_404
 from django.template.loader import render_to_string
 from rest_framework import decorators, permissions, status, viewsets
@@ -20,7 +21,11 @@
 from readthedocs.projects.models import Domain, Project
 from readthedocs.storage import build_commands_storage
 
-from ..permissions import APIPermission, APIRestrictedPermission, IsOwner
+from ..permissions import (
+    APIPermission,
+    APIRestrictedPermission,
+    IsOwner,
+)
 from ..serializers import (
     BuildAdminSerializer,
     BuildCommandSerializer,
@@ -298,10 +303,10 @@ class RemoteOrganizationViewSet(viewsets.ReadOnlyModelViewSet):
     def get_queryset(self):
         return (
             self.model.objects.api(self.request.user).filter(
-                account__provider__in=[
+                remote_organization_relations__account__provider__in=[
                     service.adapter.provider_id for service in registry
-                ],
-            )
+                ]
+            ).distinct()
         )
 
 
@@ -313,7 +318,21 @@ class RemoteRepositoryViewSet(viewsets.ReadOnlyModelViewSet):
     pagination_class = RemoteProjectPagination
 
     def get_queryset(self):
-        query = self.model.objects.api(self.request.user)
+        if not self.request.user.is_authenticated:
+            return self.model.objects.none()
+
+        # TODO: Optimize this query after deployment
+        query = self.model.objects.api(self.request.user).annotate(
+            admin=Case(
+                When(
+                    remote_repository_relations__user=self.request.user,
+                    remote_repository_relations__admin=True,
+                    then=Value(True)
+                ),
+                default=Value(False),
+                output_field=BooleanField()
+            )
+        )
         full_name = self.request.query_params.get('full_name')
         if full_name is not None:
             query = query.filter(full_name__icontains=full_name)
@@ -324,18 +343,20 @@ def get_queryset(self):
         own = self.request.query_params.get('own', None)
         if own is not None:
             query = query.filter(
-                account__provider=own,
+                remote_repository_relations__account__provider=own,
                 organization=None,
             )
 
         query = query.filter(
-            account__provider__in=[
+            remote_repository_relations__account__provider__in=[
                 service.adapter.provider_id for service in registry
             ],
-        )
+        ).distinct()
 
         # optimizes for the RemoteOrganizationSerializer
-        query = query.select_related('organization')
+        query = query.select_related('organization').order_by(
+            'organization__name', 'full_name'
+        )
 
         return query
 

readthedocs/builds/models.py

@@ -66,7 +66,6 @@
 )
 from readthedocs.builds.version_slug import VersionSlugField
 from readthedocs.config import LATEST_CONFIGURATION_VERSION
-from readthedocs.core.utils import broadcast
 from readthedocs.projects.constants import (
     BITBUCKET_COMMIT_URL,
     BITBUCKET_URL,

readthedocs/core/signals.py

@@ -86,7 +86,7 @@ def decide_if_cors(sender, request, **kwargs):  # pylint: disable=unused-argumen
 
 
 @receiver(pre_delete, sender=settings.AUTH_USER_MODEL)
-def delete_projects_and_organizations(sender, instance, *args, **kwargs):
+def delete_projects(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
     # Add annotate before filter
@@ -98,16 +98,7 @@ def delete_projects_and_organizations(sender, instance, *args, **kwargs):
                                           ).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 = (
-        RemoteOrganization.objects.annotate(num_users=Count('users')
-                                            ).filter(users=instance.id
-                                                     ).exclude(num_users__gt=1)
-    )
-
     projects.delete()
-    oauth_organizations.delete()
 
 
 signals.check_request_enabled.connect(decide_if_cors)

readthedocs/core/tests/test_signals.py

@@ -1,52 +1,50 @@
 # -*- coding: utf-8 -*-
 import django_dynamic_fixture
 import pytest
+
 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:
 
-    @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
+    def test_project_get_deleted_upon_user_delete(self):
+        """If the user has Project where he is the only
+        user, upon deleting his account, the Project
         should also get deleted."""
 
-        obj = django_dynamic_fixture.get(model_class)
+        project = django_dynamic_fixture.get(Project)
         user1 = django_dynamic_fixture.get(User)
-        obj.users.add(user1)
+        project.users.add(user1)
 
-        obj.refresh_from_db()
-        assert obj.users.all().count() == 1
+        project.refresh_from_db()
+        assert project.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()
+        project = Project.objects.all().filter(id=project.id)
+        assert not project.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
+    def test_multiple_users_project_not_delete(self):
+        """Check Project which have multiple users do not
         get deleted when any of the user delete his account."""
 
-        obj = django_dynamic_fixture.get(model_class)
+        project = django_dynamic_fixture.get(Project)
         user1 = django_dynamic_fixture.get(User)
         user2 = django_dynamic_fixture.get(User)
-        obj.users.add(user1, user2)
+        project.users.add(user1, user2)
 
-        obj.refresh_from_db()
-        assert obj.users.all().count() > 1
+        project.refresh_from_db()
+        assert project.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()
+        project.refresh_from_db()
+        assert project.id
+        obj_users = project.users.all()
         assert len(obj_users) == 1
         assert user2 in obj_users

readthedocs/oauth/admin.py

@@ -4,22 +4,38 @@
 
 from django.contrib import admin
 
-from .models import RemoteOrganization, RemoteRepository
+from .models import (
+    RemoteOrganization,
+    RemoteOrganizationRelation,
+    RemoteRepository,
+    RemoteRepositoryRelation,
+)
 
 
 class RemoteRepositoryAdmin(admin.ModelAdmin):
 
     """Admin configuration for the RemoteRepository model."""
 
-    raw_id_fields = ('users',)
+    raw_id_fields = ('project', 'organization',)
 
 
-class RemoteOrganizationAdmin(admin.ModelAdmin):
+class RemoteRepositoryRelationAdmin(admin.ModelAdmin):
 
-    """Admin configuration for the RemoteOrganization model."""
+    """Admin configuration for the RemoteRepositoryRelation model."""
 
-    raw_id_fields = ('users',)
+    raw_id_fields = ('account', 'remote_repository', 'user',)
+    list_select_related = ('remote_repository', 'user',)
+
+
+class RemoteOrganizationRelationAdmin(admin.ModelAdmin):
+
+    """Admin configuration for the RemoteOrganizationRelation model."""
+
+    raw_id_fields = ('account', 'remote_organization', 'user',)
+    list_select_related = ('remote_organization', 'user',)
 
 
 admin.site.register(RemoteRepository, RemoteRepositoryAdmin)
-admin.site.register(RemoteOrganization, RemoteOrganizationAdmin)
+admin.site.register(RemoteRepositoryRelation, RemoteRepositoryRelationAdmin)
+admin.site.register(RemoteOrganization)
+admin.site.register(RemoteOrganizationRelation, RemoteOrganizationRelationAdmin)

readthedocs/oauth/constants.py

@@ -0,0 +1,9 @@
+GITHUB = 'github'
+GITLAB = 'gitlab'
+BITBUCKET = 'bitbucket'
+
+VCS_PROVIDER_CHOICES = (
+    (GITHUB, 'GitHub'),
+    (GITLAB, 'GitLab'),
+    (BITBUCKET, 'Bitbucket'),
+)

readthedocs/oauth/management/commands/load_project_remote_repo_relation.py

@@ -0,0 +1,60 @@
+import json
+
+from django.core.management.base import BaseCommand
+
+from readthedocs.oauth.models import RemoteRepository
+
+
+class Command(BaseCommand):
+    help = "Load Project and RemoteRepository Relationship from JSON file"
+
+    def add_arguments(self, parser):
+        # File path of the json file containing relationship data
+        parser.add_argument(
+            '--file',
+            required=True,
+            nargs=1,
+            type=str,
+            help='File path of the json file containing relationship data.',
+        )
+
+    def handle(self, *args, **options):
+        file = options.get('file')[0]
+
+        try:
+            # Load data from the json file
+            with open(file, 'r') as f:
+                data = json.load(f)
+        except Exception as e:
+            self.stdout.write(
+                self.style.ERROR(
+                    f'Exception occurred while trying to load the file "{file}". '
+                    f'Exception: {e}.'
+                )
+            )
+            return
+
+        for item in data:
+            try:
+                update_count = RemoteRepository.objects.filter(
+                    remote_id=item['remote_id']
+                ).update(project_id=item['project_id'])
+
+                if update_count < 1:
+                    self.stdout.write(
+                        self.style.ERROR(
+                            f"Could not update {item['slug']}'s "
+                            f"relationship with {item['html_url']}, "
+                            f"remote_id {item['remote_id']}, "
+                            f"username: {item['username']}."
+                        )
+                    )
+
+            except Exception as e:
+                self.stdout.write(
+                    self.style.ERROR(
+                        f"Exception occurred while trying to update {item['slug']}'s "
+                        f"relationship with {item['html_url']}, "
+                        f"username: {item['username']}, Exception: {e}."
+                    )
+                )

readthedocs/oauth/management/commands/reconnect_remoterepositories.py

@@ -55,7 +55,7 @@ def _connect_repositories(self, organization, no_dry_run, only_owners):
             Q(ssh_url__in=Subquery(organization.projects.values('repo'))) |
             Q(clone_url__in=Subquery(organization.projects.values('repo')))
         )
-        for remote in RemoteRepository.objects.filter(remote_query).order_by('pub_date'):
+        for remote in RemoteRepository.objects.filter(remote_query).order_by('created'):
             admin = json.loads(remote.json).get('permissions', {}).get('admin')
 
             if only_owners and remote.users.first() not in organization.owners.all():