From fd6b96cd237ede5f448384e17ec08dc111f49079 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 25 Jun 2019 18:52:34 +0200 Subject: [PATCH 01/18] Make Project.respository.type a ChoiceField --- readthedocs/api/v3/serializers.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/readthedocs/api/v3/serializers.py b/readthedocs/api/v3/serializers.py index f820449091d..918d4bf7b6b 100644 --- a/readthedocs/api/v3/serializers.py +++ b/readthedocs/api/v3/serializers.py @@ -9,7 +9,7 @@ from rest_framework import serializers from readthedocs.builds.models import Build, Version -from readthedocs.projects.constants import LANGUAGES, PROGRAMMING_LANGUAGES +from readthedocs.projects.constants import LANGUAGES, PROGRAMMING_LANGUAGES, REPO_CHOICES from readthedocs.projects.models import Project from readthedocs.redirects.models import Redirect, TYPE_CHOICES as REDIRECT_TYPE_CHOICES @@ -313,7 +313,10 @@ def get_project_homepage(self, obj): class RepositorySerializer(serializers.Serializer): url = serializers.CharField(source='repo') - type = serializers.CharField(source='repo_type') + type = serializers.ChoiceField( + source='repo_type', + choices=REPO_CHOICES, + ) class ProjectLinksSerializer(BaseLinksSerializer): From f6073e705ddb251021335e89dba4a14d3d32efef Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 25 Jun 2019 18:53:09 +0200 Subject: [PATCH 02/18] Grant permissions on 'create' action This is needed to render the Form for POST when using BrowsableAPIRenderer and hitting /projects/ --- readthedocs/api/v3/permissions.py | 1 + 1 file changed, 1 insertion(+) diff --git a/readthedocs/api/v3/permissions.py b/readthedocs/api/v3/permissions.py index c067c1f9422..d269d6072ea 100644 --- a/readthedocs/api/v3/permissions.py +++ b/readthedocs/api/v3/permissions.py @@ -16,6 +16,7 @@ def has_permission(self, request, view): if is_authenticated: if view.basename == 'projects' and any([ view.action == 'list', + view.action == 'create', # used to create Form in BrowsableAPIRenderer view.action is None, # needed for BrowsableAPIRenderer ]): # hitting ``/projects/``, allowing From f344d829f787186dafdfe5b3b2967439aa00f2e7 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 25 Jun 2019 18:54:31 +0200 Subject: [PATCH 03/18] Use standard DRF status codes --- readthedocs/api/v3/views.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/readthedocs/api/v3/views.py b/readthedocs/api/v3/views.py index 5958c81c737..a3ec295967f 100644 --- a/readthedocs/api/v3/views.py +++ b/readthedocs/api/v3/views.py @@ -1,6 +1,7 @@ import django_filters.rest_framework as filters from django.utils.safestring import mark_safe from rest_flex_fields.views import FlexFieldsMixin +from rest_framework import status from rest_framework.authentication import TokenAuthentication from rest_framework.decorators import action from rest_framework.metadata import SimpleMetadata @@ -174,7 +175,7 @@ def superproject(self, request, project_slug): data = self.get_serializer(superproject).data return Response(data) except Exception: - return Response(status=404) + return Response(status=status.HTTP_404_NOT_FOUND) class SubprojectRelationshipViewSet(APIv3Settings, NestedViewSetMixin, @@ -263,7 +264,7 @@ def update(self, request, *args, **kwargs): # ``httpOnly`` on our cookies and the ``PUT/PATCH`` method are triggered # via Javascript super().update(request, *args, **kwargs) - return Response(status=204) + return Response(status=status.HTTP_204_NO_CONTENT) class BuildsViewSet(APIv3Settings, NestedViewSetMixin, ProjectQuerySetMixin, @@ -303,11 +304,11 @@ def create(self, request, **kwargs): # pylint: disable=arguments-differ if build: data.update({'triggered': True}) - status = 202 + code = status.HTTP_202_ACCEPTED else: data.update({'triggered': False}) - status = 400 - return Response(data=data, status=status) + code = status.HTTP_400_BAD_REQUEST + return Response(data=data, status=code) class RedirectsViewSet(APIv3Settings, NestedViewSetMixin, ProjectQuerySetMixin, From 9081474e45b5077b51bfc653c440841d7e6b0f16 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 25 Jun 2019 18:55:09 +0200 Subject: [PATCH 04/18] Refactor 'trigger_initial_build' to allow calling from other places --- readthedocs/core/utils/__init__.py | 23 ++++++++++++++++++++++- readthedocs/projects/views/private.py | 17 +++-------------- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/readthedocs/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index dbb8245c6fb..d7a4ef30fc2 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -5,7 +5,7 @@ import os import re -from celery import chord, group +from celery import chord, group, chain from django.conf import settings from django.utils.functional import keep_lazy from django.utils.safestring import SafeText, mark_safe @@ -165,6 +165,27 @@ def trigger_build(project, version=None, record=True, force=False): return (update_docs_task.apply_async(), build) +def trigger_initial_build(project, user): + """ + Trigger initial build after project is imported. + + :param project: project's documentation to be built + :returns: Celery AsyncResult promise + """ + + update_docs, build = prepare_build(project) + if (update_docs, build) == (None, None): + return None + + from readthedocs.oauth.tasks import attach_webhook + task_promise = chain( + attach_webhook.si(project.pk, user.pk), + update_docs, + ) + async_result = task_promise.apply_async() + return async_result + + def send_email( recipient, subject, template, template_html, context=None, request=None, from_email=None, **kwargs diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index 7a06b2afa49..aeaf509bb56 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -3,7 +3,6 @@ import logging from allauth.socialaccount.models import SocialAccount -from celery import chain from django.conf import settings from django.contrib import messages from django.contrib.auth.decorators import login_required @@ -24,9 +23,9 @@ from vanilla import CreateView, DeleteView, DetailView, GenericView, UpdateView from readthedocs.builds.forms import VersionForm -from readthedocs.builds.models import Build, Version +from readthedocs.builds.models import Version from readthedocs.core.mixins import ListViewWithForm, LoginRequiredMixin -from readthedocs.core.utils import broadcast, prepare_build, trigger_build +from readthedocs.core.utils import broadcast, trigger_build, trigger_initial_build from readthedocs.integrations.models import HttpExchange, Integration from readthedocs.oauth.services import registry from readthedocs.oauth.tasks import attach_webhook @@ -272,17 +271,7 @@ def done(self, form_list, **kwargs): ) def trigger_initial_build(self, project): - """Trigger initial build.""" - update_docs, build = prepare_build(project) - if (update_docs, build) == (None, None): - return None - - task_promise = chain( - attach_webhook.si(project.pk, self.request.user.pk), - update_docs, - ) - async_result = task_promise.apply_async() - return async_result + return trigger_initial_build(project, self.request.user) def is_advanced(self): """Determine if the user selected the `show advanced` field.""" From 6aa7eda6c028be27e926e63d079b1dfebf19c9e2 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 25 Jun 2019 18:58:09 +0200 Subject: [PATCH 05/18] View/Serializer to import a Project from APIv3 --- readthedocs/api/v3/serializers.py | 16 +++++++++++ readthedocs/api/v3/views.py | 47 +++++++++++++++++++++++++++++-- 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/readthedocs/api/v3/serializers.py b/readthedocs/api/v3/serializers.py index 918d4bf7b6b..87c7bfc5828 100644 --- a/readthedocs/api/v3/serializers.py +++ b/readthedocs/api/v3/serializers.py @@ -389,6 +389,22 @@ def get_translations(self, obj): return self._absolute_url(path) +class ProjectCreateSerializer(FlexFieldsModelSerializer): + + """Serializer used to Import a Project.""" + + repository = RepositorySerializer(source='*') + + class Meta: + model = Project + fields = ( + 'name', + 'language', + 'repository', + 'project_url', # project_homepage + ) + + class ProjectSerializer(FlexFieldsModelSerializer): language = LanguageSerializer() diff --git a/readthedocs/api/v3/views.py b/readthedocs/api/v3/views.py index a3ec295967f..4b417c3ef09 100644 --- a/readthedocs/api/v3/views.py +++ b/readthedocs/api/v3/views.py @@ -31,6 +31,7 @@ BuildCreateSerializer, BuildSerializer, ProjectSerializer, + ProjectCreateSerializer, RedirectCreateSerializer, RedirectDetailSerializer, VersionSerializer, @@ -67,7 +68,7 @@ class APIv3Settings: class ProjectsViewSet(APIv3Settings, NestedViewSetMixin, ProjectQuerySetMixin, - FlexFieldsMixin, ReadOnlyModelViewSet): + FlexFieldsMixin, CreateModelMixin, ReadOnlyModelViewSet): # Markdown docstring is automatically rendered by BrowsableAPIRenderer. @@ -115,14 +116,13 @@ class ProjectsViewSet(APIv3Settings, NestedViewSetMixin, ProjectQuerySetMixin, * Subprojects of a project: ``/api/v3/projects/{project_slug}/subprojects/`` * Superproject of a project: ``/api/v3/projects/{project_slug}/superproject/`` - Go to [https://docs.readthedocs.io/en/stable/api/v3.html](https://docs.readthedocs.io/en/stable/api/v3.html) + Go to [https://docs.readthedocs.io/page/api/v3.html](https://docs.readthedocs.io/page/api/v3.html) for a complete documentation of the APIv3. """ # noqa model = Project lookup_field = 'slug' lookup_url_kwarg = 'project_slug' - serializer_class = ProjectSerializer filterset_class = ProjectFilter queryset = Project.objects.all() permit_list_expands = [ @@ -131,6 +131,20 @@ class ProjectsViewSet(APIv3Settings, NestedViewSetMixin, ProjectQuerySetMixin, 'active_versions.last_build.config', ] + def get_serializer_class(self): + """ + Return correct serializer depending on the action. + + For GET it returns a serializer with many fields and on PUT/PATCH/POST, + it return a serializer to validate just a few fields. + """ + if self.action in ('list', 'retrieve'): + return ProjectSerializer + elif self.action in ('create',): + return ProjectCreateSerializer + # elif self.action in ('update', 'partial_update'): + # return ProjectUpdateSerializer + def get_queryset(self): # Allow hitting ``/api/v3/projects/`` to list their own projects if self.basename == 'projects' and self.action == 'list': @@ -166,6 +180,33 @@ def get_view_description(self, *args, **kwargs): # pylint: disable=arguments-di return mark_safe(description.format(project_slug=project.slug)) return description + def create(self, request, *args, **kwargs): + """ + Override method to importing a Project. + + * Save the Project object + * Assign the user from the request as owner + * Sent project_import signal + * Trigger an initial Build + """ + serializer = self.get_serializer(data=request.data) + serializer.is_valid(raise_exception=True) + project = serializer.save() + headers = self.get_success_headers(serializer.data) + + # TODO: these lines need to be adapted for Corporate + project.users.add(request.user) + project_import.send(sender=project, request=request) + trigger_initial_build(project, request.user) + + # Full render Project + serializer = ProjectSerializer(instance=project) + return Response( + serializer.data, + status=status.HTTP_201_CREATED, + headers=headers, + ) + @action(detail=True, methods=['get']) def superproject(self, request, project_slug): """Return the superproject of a ``Project``.""" From 44d0f2f373b2e69dcbb0746ed2bf1169cbe0b10d Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Thu, 27 Jun 2019 13:32:40 +0200 Subject: [PATCH 06/18] Endpoint to import a project --- .../tests/responses/projects-list_POST.json | 47 ++++++++++++++++ readthedocs/api/v3/tests/test_projects.py | 56 +++++++++++++++++++ readthedocs/api/v3/views.py | 2 +- readthedocs/settings/base.py | 1 + 4 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 readthedocs/api/v3/tests/responses/projects-list_POST.json diff --git a/readthedocs/api/v3/tests/responses/projects-list_POST.json b/readthedocs/api/v3/tests/responses/projects-list_POST.json new file mode 100644 index 00000000000..cbfd52297f6 --- /dev/null +++ b/readthedocs/api/v3/tests/responses/projects-list_POST.json @@ -0,0 +1,47 @@ +{ + "_links": { + "_self": "https://readthedocs.org/api/v3/projects/test-project/", + "builds": "https://readthedocs.org/api/v3/projects/test-project/builds/", + "subprojects": "https://readthedocs.org/api/v3/projects/test-project/subprojects/", + "superproject": "https://readthedocs.org/api/v3/projects/test-project/superproject/", + "translations": "https://readthedocs.org/api/v3/projects/test-project/translations/", + "versions": "https://readthedocs.org/api/v3/projects/test-project/versions/" + }, + "created": "2019-04-29T14:00:00Z", + "default_branch": "master", + "default_version": "latest", + "description": null, + "id": 4, + "language": { + "code": "en", + "name": "English" + }, + "modified": "2019-04-29T14:00:00Z", + "name": "Test Project", + "privacy_level": { + "code": "public", + "name": "Public" + }, + "programming_language": { + "code": "words", + "name": "Only Words" + }, + "repository": { + "type": "git", + "url": "https://github.com/rtfd/template" + }, + "slug": "test-project", + "subproject_of": null, + "tags": [], + "translation_of": null, + "urls": { + "documentation": "http://readthedocs.org/docs/test-project/en/latest/", + "project_homepage": null + }, + "users": [ + { + "created": "2019-04-29T10:00:00Z", + "username": "testuser" + } + ] +} diff --git a/readthedocs/api/v3/tests/test_projects.py b/readthedocs/api/v3/tests/test_projects.py index 8bf4b6cd23c..7444577102f 100644 --- a/readthedocs/api/v3/tests/test_projects.py +++ b/readthedocs/api/v3/tests/test_projects.py @@ -1,4 +1,5 @@ import datetime +import mock import json from pathlib import Path @@ -384,6 +385,7 @@ def test_projects_versions_detail_unique(self): ) self.assertEqual(response.status_code, 200) + def test_unauthed_projects_redirects_list(self): response = self.client.get( reverse( @@ -529,3 +531,57 @@ def test_projects_redirects_detail_delete(self): ) self.assertEqual(response.status_code, 204) self.assertEqual(self.project.redirects.count(), 0) + + + @mock.patch('readthedocs.api.v3.views.trigger_initial_build') + @mock.patch('readthedocs.api.v3.views.project_import') + def test_import_project(self, project_import, trigger_initial_build): + data = { + 'name': 'Test Project', + 'repository': { + 'url': 'https://github.com/rtfd/template', + 'type': 'git', + }, + } + + self.client.credentials(HTTP_AUTHORIZATION=f'Token {self.token.key}') + response = self.client.post(reverse('projects-list'), data) + self.assertEqual(response.status_code, 201) + + query = Project.objects.filter(slug='test-project') + self.assertTrue(query.exists()) + + project = query.first() + self.assertEqual(project.name, 'Test Project') + self.assertEqual(project.slug, 'test-project') + self.assertEqual(project.repo, 'https://github.com/rtfd/template') + self.assertEqual(project.language, 'en') + self.assertIn(self.me, project.users.all()) + + # Signal sent + project_import.send.assert_has_calls( + [ + mock.call( + sender=project, + request=mock.ANY, + ), + ], + ) + + # Build triggered + trigger_initial_build.assert_has_calls( + [ + mock.call( + project, + self.me, + ), + ], + ) + + response_json = response.json() + response_json['created'] = '2019-04-29T14:00:00Z' + response_json['modified'] = '2019-04-29T14:00:00Z' + self.assertDictEqual( + response_json, + self._get_response_dict('projects-list_POST'), + ) diff --git a/readthedocs/api/v3/views.py b/readthedocs/api/v3/views.py index 4b417c3ef09..ed10d08181e 100644 --- a/readthedocs/api/v3/views.py +++ b/readthedocs/api/v3/views.py @@ -138,7 +138,7 @@ def get_serializer_class(self): For GET it returns a serializer with many fields and on PUT/PATCH/POST, it return a serializer to validate just a few fields. """ - if self.action in ('list', 'retrieve'): + if self.action in ('list', 'retrieve', 'superproject'): return ProjectSerializer elif self.action in ('create',): return ProjectCreateSerializer diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index adb17effefd..456d0cf4573 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -507,6 +507,7 @@ def USE_PROMOS(self): # noqa 'user': '60/minute', }, 'PAGE_SIZE': 10, + 'TEST_REQUEST_DEFAULT_FORMAT': 'json', } SILENCED_SYSTEM_CHECKS = ['fields.W342'] From ff5b088be8262a5a510e1b0436f37d75121cfc3e Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 2 Jul 2019 14:07:43 +0200 Subject: [PATCH 07/18] Refactor Import Project to share the flow --- readthedocs/api/v3/views.py | 31 ++++++------------ readthedocs/core/utils/__init__.py | 23 +------------- readthedocs/projects/views/mixins.py | 46 +++++++++++++++++++++++++++ readthedocs/projects/views/private.py | 22 ++++++------- 4 files changed, 65 insertions(+), 57 deletions(-) diff --git a/readthedocs/api/v3/views.py b/readthedocs/api/v3/views.py index ed10d08181e..7a1c8646e3a 100644 --- a/readthedocs/api/v3/views.py +++ b/readthedocs/api/v3/views.py @@ -21,7 +21,8 @@ from readthedocs.builds.models import Build, Version from readthedocs.core.utils import trigger_build from readthedocs.projects.models import Project -from readthedocs.redirects.models import Redirect +from readthedocs.projects.views.mixins import ProjectImportMixin + from .filters import BuildFilter, ProjectFilter, VersionFilter from .mixins import ProjectQuerySetMixin @@ -68,7 +69,8 @@ class APIv3Settings: class ProjectsViewSet(APIv3Settings, NestedViewSetMixin, ProjectQuerySetMixin, - FlexFieldsMixin, CreateModelMixin, ReadOnlyModelViewSet): + FlexFieldsMixin, ProjectImportMixin, CreateModelMixin, + ReadOnlyModelViewSet): # Markdown docstring is automatically rendered by BrowsableAPIRenderer. @@ -180,32 +182,17 @@ def get_view_description(self, *args, **kwargs): # pylint: disable=arguments-di return mark_safe(description.format(project_slug=project.slug)) return description - def create(self, request, *args, **kwargs): + def perform_create(self, serializer): """ - Override method to importing a Project. + Import Project. - * Save the Project object - * Assign the user from the request as owner - * Sent project_import signal - * Trigger an initial Build + Trigger our internal mechanism to import a project after it's saved in + the database. """ serializer = self.get_serializer(data=request.data) serializer.is_valid(raise_exception=True) project = serializer.save() - headers = self.get_success_headers(serializer.data) - - # TODO: these lines need to be adapted for Corporate - project.users.add(request.user) - project_import.send(sender=project, request=request) - trigger_initial_build(project, request.user) - - # Full render Project - serializer = ProjectSerializer(instance=project) - return Response( - serializer.data, - status=status.HTTP_201_CREATED, - headers=headers, - ) + self.import_project(project, [], self.request) @action(detail=True, methods=['get']) def superproject(self, request, project_slug): diff --git a/readthedocs/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index d7a4ef30fc2..dbb8245c6fb 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -5,7 +5,7 @@ import os import re -from celery import chord, group, chain +from celery import chord, group from django.conf import settings from django.utils.functional import keep_lazy from django.utils.safestring import SafeText, mark_safe @@ -165,27 +165,6 @@ def trigger_build(project, version=None, record=True, force=False): return (update_docs_task.apply_async(), build) -def trigger_initial_build(project, user): - """ - Trigger initial build after project is imported. - - :param project: project's documentation to be built - :returns: Celery AsyncResult promise - """ - - update_docs, build = prepare_build(project) - if (update_docs, build) == (None, None): - return None - - from readthedocs.oauth.tasks import attach_webhook - task_promise = chain( - attach_webhook.si(project.pk, user.pk), - update_docs, - ) - async_result = task_promise.apply_async() - return async_result - - def send_email( recipient, subject, template, template_html, context=None, request=None, from_email=None, **kwargs diff --git a/readthedocs/projects/views/mixins.py b/readthedocs/projects/views/mixins.py index 670caa21f83..1a72dc8c846 100644 --- a/readthedocs/projects/views/mixins.py +++ b/readthedocs/projects/views/mixins.py @@ -2,9 +2,12 @@ """Mixin classes for project views.""" +from celery import chain from django.shortcuts import get_object_or_404 +from readthedocs.core.utils import prepare_build from readthedocs.projects.models import Project +from readthedocs.projects.signals import project_import class ProjectRelationMixin: @@ -44,3 +47,46 @@ def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) context[self.project_context_object_name] = self.get_project() return context + + +class ProjectImportMixin: + + """Helpers to import a Project.""" + + def import_project(self, project, tags, request): + """ + Import a Project into Read the Docs. + + - Add the user from request as maintainer + - Set all the tags to the project + - Send Django Signal + - Trigger initial build + """ + project.users.add(request.user) + for tag in tags: + project.tags.add(tag) + + # TODO: this signal could be removed, or used for sync task + project_import.send(sender=project, request=request) + + self.trigger_initial_build(project, request.user) + + def trigger_initial_build(self, project, user): + """ + Trigger initial build after project is imported. + + :param project: project's documentation to be built + :returns: Celery AsyncResult promise + """ + + update_docs, build = prepare_build(project) + if (update_docs, build) == (None, None): + return None + + from readthedocs.oauth.tasks import attach_webhook + task_promise = chain( + attach_webhook.si(project.pk, user.pk), + update_docs, + ) + async_result = task_promise.apply_async() + return async_result diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index aeaf509bb56..4e4443ec6c7 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -25,7 +25,7 @@ from readthedocs.builds.forms import VersionForm from readthedocs.builds.models import Version from readthedocs.core.mixins import ListViewWithForm, LoginRequiredMixin -from readthedocs.core.utils import broadcast, trigger_build, trigger_initial_build +from readthedocs.core.utils import broadcast, trigger_build from readthedocs.integrations.models import HttpExchange, Integration from readthedocs.oauth.services import registry from readthedocs.oauth.tasks import attach_webhook @@ -56,8 +56,8 @@ WebHook, ) from readthedocs.projects.notifications import EmailConfirmNotification -from readthedocs.projects.signals import project_import from readthedocs.projects.views.base import ProjectAdminMixin, ProjectSpamMixin +from readthedocs.projects.views.mixins import ProjectImportMixin from ..tasks import retry_domain_verification @@ -216,7 +216,10 @@ def project_delete(request, project_slug): return render(request, 'projects/project_delete.html', context) -class ImportWizardView(ProjectSpamMixin, PrivateViewMixin, SessionWizardView): +class ImportWizardView( + ProjectImportMixin, ProjectSpamMixin, PrivateViewMixin, + SessionWizardView, +): """Project import wizard.""" @@ -254,32 +257,25 @@ def done(self, form_list, **kwargs): # Save the basics form to create the project instance, then alter # attributes directly from other forms project = basics_form.save() - tags = form_data.pop('tags', []) - for tag in tags: - project.tags.add(tag) for field, value in list(form_data.items()): if field in extra_fields: setattr(project, field, value) project.save() - # TODO: this signal could be removed, or used for sync task - project_import.send(sender=project, request=self.request) + tags = form_data.pop('tags', []) + self.import_project(project, tags, self.request) - self.trigger_initial_build(project) return HttpResponseRedirect( reverse('projects_detail', args=[project.slug]), ) - def trigger_initial_build(self, project): - return trigger_initial_build(project, self.request.user) - def is_advanced(self): """Determine if the user selected the `show advanced` field.""" data = self.get_cleaned_data_for_step('basics') or {} return data.get('advanced', True) -class ImportDemoView(PrivateViewMixin, View): +class ImportDemoView(PrivateViewMixin, ProjectImportMixin, View): """View to pass request on to import form to import demo project.""" From a5b0015ce49597010ad724d76a562a246dceab9b Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 3 Jul 2019 14:58:29 +0200 Subject: [PATCH 08/18] Re-write test to match the new implementation (Mixin) --- .../tests/responses/projects-list_POST.json | 4 +-- readthedocs/api/v3/tests/test_projects.py | 30 ++++--------------- 2 files changed, 7 insertions(+), 27 deletions(-) diff --git a/readthedocs/api/v3/tests/responses/projects-list_POST.json b/readthedocs/api/v3/tests/responses/projects-list_POST.json index cbfd52297f6..4a803f6f931 100644 --- a/readthedocs/api/v3/tests/responses/projects-list_POST.json +++ b/readthedocs/api/v3/tests/responses/projects-list_POST.json @@ -7,7 +7,7 @@ "translations": "https://readthedocs.org/api/v3/projects/test-project/translations/", "versions": "https://readthedocs.org/api/v3/projects/test-project/versions/" }, - "created": "2019-04-29T14:00:00Z", + "created": "2019-04-29T10:00:00Z", "default_branch": "master", "default_version": "latest", "description": null, @@ -16,7 +16,7 @@ "code": "en", "name": "English" }, - "modified": "2019-04-29T14:00:00Z", + "modified": "2019-04-29T12:00:00Z", "name": "Test Project", "privacy_level": { "code": "public", diff --git a/readthedocs/api/v3/tests/test_projects.py b/readthedocs/api/v3/tests/test_projects.py index 7444577102f..1a88040724f 100644 --- a/readthedocs/api/v3/tests/test_projects.py +++ b/readthedocs/api/v3/tests/test_projects.py @@ -533,9 +533,7 @@ def test_projects_redirects_detail_delete(self): self.assertEqual(self.project.redirects.count(), 0) - @mock.patch('readthedocs.api.v3.views.trigger_initial_build') - @mock.patch('readthedocs.api.v3.views.project_import') - def test_import_project(self, project_import, trigger_initial_build): + def test_import_project(self): data = { 'name': 'Test Project', 'repository': { @@ -557,30 +555,12 @@ def test_import_project(self, project_import, trigger_initial_build): self.assertEqual(project.repo, 'https://github.com/rtfd/template') self.assertEqual(project.language, 'en') self.assertIn(self.me, project.users.all()) - - # Signal sent - project_import.send.assert_has_calls( - [ - mock.call( - sender=project, - request=mock.ANY, - ), - ], - ) - - # Build triggered - trigger_initial_build.assert_has_calls( - [ - mock.call( - project, - self.me, - ), - ], - ) + self.assertEqual(project.builds.count(), 1) response_json = response.json() - response_json['created'] = '2019-04-29T14:00:00Z' - response_json['modified'] = '2019-04-29T14:00:00Z' + response_json['created'] = '2019-04-29T10:00:00Z' + response_json['modified'] = '2019-04-29T12:00:00Z' + self.assertDictEqual( response_json, self._get_response_dict('projects-list_POST'), From 2c8852471b05e28a5b7bf40c69af8bf67cff2fe0 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 3 Jul 2019 14:58:51 +0200 Subject: [PATCH 09/18] Use one serializer to validate input and a different one in the response --- readthedocs/api/v3/views.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/readthedocs/api/v3/views.py b/readthedocs/api/v3/views.py index 7a1c8646e3a..62cb5d86a44 100644 --- a/readthedocs/api/v3/views.py +++ b/readthedocs/api/v3/views.py @@ -182,6 +182,22 @@ def get_view_description(self, *args, **kwargs): # pylint: disable=arguments-di return mark_safe(description.format(project_slug=project.slug)) return description + def create(self, request, *args, **kwargs): + """ + Import Project. + + Override to use a different serializer in the response. + """ + serializer = self.get_serializer(data=request.data) + serializer.is_valid(raise_exception=True) + self.perform_create(serializer) + headers = self.get_success_headers(serializer.data) + + # Use serializer that fully render a Project + serializer = ProjectSerializer(instance=serializer.instance) + + return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers) + def perform_create(self, serializer): """ Import Project. From 58bc94eed8cc37ae0946fc85d327dcee3bf51536 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 3 Jul 2019 16:56:45 +0200 Subject: [PATCH 10/18] Remove tags attribute before being set tags is a special attribute and needs to be set via `.add` method. --- readthedocs/projects/views/private.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index 4e4443ec6c7..d1aee18def7 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -257,12 +257,15 @@ def done(self, form_list, **kwargs): # Save the basics form to create the project instance, then alter # attributes directly from other forms project = basics_form.save() + + # Remove tags to avoid setting them in raw instead of using ``.add`` + tags = form_data.pop('tags', []) + for field, value in list(form_data.items()): if field in extra_fields: setattr(project, field, value) project.save() - tags = form_data.pop('tags', []) self.import_project(project, tags, self.request) return HttpResponseRedirect( From 92c0e8aa736fd735aefef427d39d92f7cd7d96b1 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 3 Jul 2019 17:01:21 +0200 Subject: [PATCH 11/18] Avoid elif when using return --- readthedocs/api/v3/views.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/readthedocs/api/v3/views.py b/readthedocs/api/v3/views.py index 62cb5d86a44..11388888e33 100644 --- a/readthedocs/api/v3/views.py +++ b/readthedocs/api/v3/views.py @@ -142,10 +142,9 @@ def get_serializer_class(self): """ if self.action in ('list', 'retrieve', 'superproject'): return ProjectSerializer - elif self.action in ('create',): + + if self.action in ('create',): return ProjectCreateSerializer - # elif self.action in ('update', 'partial_update'): - # return ProjectUpdateSerializer def get_queryset(self): # Allow hitting ``/api/v3/projects/`` to list their own projects From 3d7092850d94a2ae0dff8c81aa1453135677e09e Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 3 Jul 2019 17:01:32 +0200 Subject: [PATCH 12/18] Use the same attributes for trigger_initial_build --- readthedocs/projects/views/private.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index d1aee18def7..3d398e45c79 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -308,7 +308,7 @@ def get(self, request, *args, **kwargs): if form.is_valid(): project = form.save() project.save() - self.trigger_initial_build(project) + self.trigger_initial_build(project, request.user) messages.success( request, _('Your demo project is currently being imported'), @@ -335,13 +335,13 @@ def get_form_kwargs(self): """Form kwargs passed in during instantiation.""" return {'user': self.request.user} - def trigger_initial_build(self, project): + def trigger_initial_build(self, project, user): """ Trigger initial build. Allow to override the behavior from outside. """ - return trigger_build(project) + return trigger_build(project, user) class ImportView(PrivateViewMixin, TemplateView): From 1896e337234a2989759c700e7da7552559d14b70 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 8 Jul 2019 09:36:18 +0200 Subject: [PATCH 13/18] Call trigger_build properly --- readthedocs/projects/views/private.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index 3d398e45c79..cf9a90dd52b 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -341,7 +341,7 @@ def trigger_initial_build(self, project, user): Allow to override the behavior from outside. """ - return trigger_build(project, user) + return trigger_build(project) class ImportView(PrivateViewMixin, TemplateView): From 9a19a0eef318a8da61ebad3bb5a0c5e1822ebd0d Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 8 Jul 2019 09:36:30 +0200 Subject: [PATCH 14/18] More attributes when importing a Project --- readthedocs/api/v3/serializers.py | 4 ++- .../tests/responses/projects-list_POST.json | 6 ++-- readthedocs/api/v3/tests/test_projects.py | 29 +++++++++++++++++++ 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/readthedocs/api/v3/serializers.py b/readthedocs/api/v3/serializers.py index 87c7bfc5828..46221999494 100644 --- a/readthedocs/api/v3/serializers.py +++ b/readthedocs/api/v3/serializers.py @@ -394,14 +394,16 @@ class ProjectCreateSerializer(FlexFieldsModelSerializer): """Serializer used to Import a Project.""" repository = RepositorySerializer(source='*') + homepage = serializers.URLField(source='project_url', required=False) class Meta: model = Project fields = ( 'name', 'language', + 'programming_language', 'repository', - 'project_url', # project_homepage + 'homepage', ) diff --git a/readthedocs/api/v3/tests/responses/projects-list_POST.json b/readthedocs/api/v3/tests/responses/projects-list_POST.json index 4a803f6f931..99a584be6e9 100644 --- a/readthedocs/api/v3/tests/responses/projects-list_POST.json +++ b/readthedocs/api/v3/tests/responses/projects-list_POST.json @@ -23,8 +23,8 @@ "name": "Public" }, "programming_language": { - "code": "words", - "name": "Only Words" + "code": "py", + "name": "Python" }, "repository": { "type": "git", @@ -36,7 +36,7 @@ "translation_of": null, "urls": { "documentation": "http://readthedocs.org/docs/test-project/en/latest/", - "project_homepage": null + "project_homepage": "http://template.readthedocs.io/" }, "users": [ { diff --git a/readthedocs/api/v3/tests/test_projects.py b/readthedocs/api/v3/tests/test_projects.py index 1a88040724f..91619cf8781 100644 --- a/readthedocs/api/v3/tests/test_projects.py +++ b/readthedocs/api/v3/tests/test_projects.py @@ -540,6 +540,8 @@ def test_import_project(self): 'url': 'https://github.com/rtfd/template', 'type': 'git', }, + 'homepage': 'http://template.readthedocs.io/', + 'programming_language': 'py', } self.client.credentials(HTTP_AUTHORIZATION=f'Token {self.token.key}') @@ -554,6 +556,9 @@ def test_import_project(self): self.assertEqual(project.slug, 'test-project') self.assertEqual(project.repo, 'https://github.com/rtfd/template') self.assertEqual(project.language, 'en') + self.assertEqual(project.programming_language, 'py') + self.assertEqual(project.privacy_level, 'public') + self.assertEqual(project.project_url, 'http://template.readthedocs.io/') self.assertIn(self.me, project.users.all()) self.assertEqual(project.builds.count(), 1) @@ -565,3 +570,27 @@ def test_import_project(self): response_json, self._get_response_dict('projects-list_POST'), ) + + def test_import_project_with_extra_fields(self): + data = { + 'name': 'Test Project', + 'repository': { + 'url': 'https://github.com/rtfd/template', + 'type': 'git', + }, + 'default_version': 'v1.0', # ignored: field not allowed + } + + self.client.credentials(HTTP_AUTHORIZATION=f'Token {self.token.key}') + response = self.client.post(reverse('projects-list'), data) + self.assertEqual(response.status_code, 201) + + query = Project.objects.filter(slug='test-project') + self.assertTrue(query.exists()) + + project = query.first() + self.assertEqual(project.name, 'Test Project') + self.assertEqual(project.slug, 'test-project') + self.assertEqual(project.repo, 'https://github.com/rtfd/template') + self.assertNotEqual(project.default_version, 'v1.0') + self.assertIn(self.me, project.users.all()) From d550922475139acedc7c163b535616e8885b3508 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 9 Jul 2019 10:10:23 +0200 Subject: [PATCH 15/18] Add note about superproject "action" --- readthedocs/api/v3/views.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/readthedocs/api/v3/views.py b/readthedocs/api/v3/views.py index 11388888e33..6d0aee08b61 100644 --- a/readthedocs/api/v3/views.py +++ b/readthedocs/api/v3/views.py @@ -141,6 +141,8 @@ def get_serializer_class(self): it return a serializer to validate just a few fields. """ if self.action in ('list', 'retrieve', 'superproject'): + # NOTE: ``superproject`` is the @action defined in the + # ProjectViewSet that returns the superproject of a project. return ProjectSerializer if self.action in ('create',): From f1538649fc45fdce8a2a14564154b8872893e973 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 9 Jul 2019 10:10:48 +0200 Subject: [PATCH 16/18] Make explicit comparison --- readthedocs/api/v3/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/api/v3/views.py b/readthedocs/api/v3/views.py index 6d0aee08b61..8335dc4a494 100644 --- a/readthedocs/api/v3/views.py +++ b/readthedocs/api/v3/views.py @@ -145,7 +145,7 @@ def get_serializer_class(self): # ProjectViewSet that returns the superproject of a project. return ProjectSerializer - if self.action in ('create',): + if self.action == 'create': return ProjectCreateSerializer def get_queryset(self): From 28b7606d81a7a99a2663bccb8bb8cffa0daa95da Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 9 Jul 2019 10:16:48 +0200 Subject: [PATCH 17/18] Rename method and make argument a kwarg --- readthedocs/api/v3/views.py | 2 +- readthedocs/projects/views/mixins.py | 5 ++++- readthedocs/projects/views/private.py | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/readthedocs/api/v3/views.py b/readthedocs/api/v3/views.py index 8335dc4a494..316da40a6fc 100644 --- a/readthedocs/api/v3/views.py +++ b/readthedocs/api/v3/views.py @@ -209,7 +209,7 @@ def perform_create(self, serializer): serializer = self.get_serializer(data=request.data) serializer.is_valid(raise_exception=True) project = serializer.save() - self.import_project(project, [], self.request) + self.finish_import_project(self.request, project) @action(detail=True, methods=['get']) def superproject(self, request, project_slug): diff --git a/readthedocs/projects/views/mixins.py b/readthedocs/projects/views/mixins.py index 1a72dc8c846..f78473558f0 100644 --- a/readthedocs/projects/views/mixins.py +++ b/readthedocs/projects/views/mixins.py @@ -53,7 +53,7 @@ class ProjectImportMixin: """Helpers to import a Project.""" - def import_project(self, project, tags, request): + def finish_import_project(self, request, project, tags=None): """ Import a Project into Read the Docs. @@ -62,6 +62,9 @@ def import_project(self, project, tags, request): - Send Django Signal - Trigger initial build """ + if not tags: + tags = [] + project.users.add(request.user) for tag in tags: project.tags.add(tag) diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index cf9a90dd52b..af8aa1f08c9 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -266,7 +266,7 @@ def done(self, form_list, **kwargs): setattr(project, field, value) project.save() - self.import_project(project, tags, self.request) + self.finish_import_project(self.request, project, tags) return HttpResponseRedirect( reverse('projects_detail', args=[project.slug]), From b29b3b5745d6523354a78a5ffb7d650777afb554 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 9 Jul 2019 10:50:50 +0200 Subject: [PATCH 18/18] Solve issues after merge --- .../api/v3/tests/responses/projects-list_POST.json | 1 + readthedocs/api/v3/tests/test_projects.py | 1 - readthedocs/api/v3/views.py | 3 +-- readthedocs/projects/views/mixins.py | 8 +++++++- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/readthedocs/api/v3/tests/responses/projects-list_POST.json b/readthedocs/api/v3/tests/responses/projects-list_POST.json index 99a584be6e9..979aa133858 100644 --- a/readthedocs/api/v3/tests/responses/projects-list_POST.json +++ b/readthedocs/api/v3/tests/responses/projects-list_POST.json @@ -2,6 +2,7 @@ "_links": { "_self": "https://readthedocs.org/api/v3/projects/test-project/", "builds": "https://readthedocs.org/api/v3/projects/test-project/builds/", + "redirects": "https://readthedocs.org/api/v3/projects/test-project/redirects/", "subprojects": "https://readthedocs.org/api/v3/projects/test-project/subprojects/", "superproject": "https://readthedocs.org/api/v3/projects/test-project/superproject/", "translations": "https://readthedocs.org/api/v3/projects/test-project/translations/", diff --git a/readthedocs/api/v3/tests/test_projects.py b/readthedocs/api/v3/tests/test_projects.py index 91619cf8781..39c68b3212b 100644 --- a/readthedocs/api/v3/tests/test_projects.py +++ b/readthedocs/api/v3/tests/test_projects.py @@ -1,5 +1,4 @@ import datetime -import mock import json from pathlib import Path diff --git a/readthedocs/api/v3/views.py b/readthedocs/api/v3/views.py index 316da40a6fc..ce1b85df191 100644 --- a/readthedocs/api/v3/views.py +++ b/readthedocs/api/v3/views.py @@ -22,6 +22,7 @@ from readthedocs.core.utils import trigger_build from readthedocs.projects.models import Project from readthedocs.projects.views.mixins import ProjectImportMixin +from readthedocs.redirects.models import Redirect from .filters import BuildFilter, ProjectFilter, VersionFilter @@ -206,8 +207,6 @@ def perform_create(self, serializer): Trigger our internal mechanism to import a project after it's saved in the database. """ - serializer = self.get_serializer(data=request.data) - serializer.is_valid(raise_exception=True) project = serializer.save() self.finish_import_project(self.request, project) diff --git a/readthedocs/projects/views/mixins.py b/readthedocs/projects/views/mixins.py index f78473558f0..c65ec3f01bc 100644 --- a/readthedocs/projects/views/mixins.py +++ b/readthedocs/projects/views/mixins.py @@ -55,12 +55,18 @@ class ProjectImportMixin: def finish_import_project(self, request, project, tags=None): """ - Import a Project into Read the Docs. + Perform last steps to import a project into Read the Docs. - Add the user from request as maintainer - Set all the tags to the project - Send Django Signal - Trigger initial build + + It requires the Project was already saved into the DB. + + :param request: Django Request object + :param project: Project instance just imported (already saved) + :param tags: tags to add to the project """ if not tags: tags = []