Merge pull request #6166 from stsewd/cleanup-views

Clean up views

Author: stsewd

readthedocs/builds/urls.py

@@ -1,20 +1,18 @@
-# -*- coding: utf-8 -*-
-
 """URL configuration for builds app."""
-from django.conf.urls import url
 
-from .views import builds_redirect_detail, builds_redirect_list
+from django.conf.urls import url
+from django.views.generic.base import RedirectView
 
 
 urlpatterns = [
     url(
-        r'^(?P<project_slug>[-\w]+)/(?P<pk>\d+)/$',
-        builds_redirect_detail,
+        r'^(?P<project_slug>[-\w]+)/(?P<build_pk>\d+)/$',
+        RedirectView.as_view(pattern_name='builds_detail', permanent=True),
         name='old_builds_detail',
     ),
     url(
         r'^(?P<project_slug>[-\w]+)/$',
-        builds_redirect_list,
+        RedirectView.as_view(pattern_name='builds_project_list', permanent=True),
         name='old_builds_project_list',
     ),
 ]

readthedocs/builds/views.py

@@ -1,35 +1,33 @@
-# -*- coding: utf-8 -*-
-
 """Views for builds app."""
 
 import logging
 import textwrap
+from urllib.parse import urlparse
 
 from django.contrib import messages
 from django.contrib.auth.decorators import login_required
 from django.http import (
     HttpResponseForbidden,
-    HttpResponsePermanentRedirect,
     HttpResponseRedirect,
 )
 from django.shortcuts import get_object_or_404
 from django.urls import reverse
 from django.utils.decorators import method_decorator
 from django.views.generic import DetailView, ListView
 from requests.utils import quote
-from urllib.parse import urlparse
 
-from readthedocs.doc_builder.exceptions import BuildEnvironmentError
 from readthedocs.builds.models import Build, Version
 from readthedocs.core.permissions import AdminPermission
 from readthedocs.core.utils import trigger_build
+from readthedocs.doc_builder.exceptions import BuildEnvironmentError
 from readthedocs.projects.models import Project
 
 
 log = logging.getLogger(__name__)
 
 
 class BuildBase:
+
     model = Build
 
     def get_queryset(self):
@@ -103,6 +101,7 @@ def get_context_data(self, **kwargs):
 
 
 class BuildDetail(BuildBase, DetailView):
+
     pk_url_kwarg = 'build_pk'
 
     def get_context_data(self, **kwargs):
@@ -152,18 +151,3 @@ def get_context_data(self, **kwargs):
         issue_url = urlparse(issue_url).geturl()
         context['issue_url'] = issue_url
         return context
-
-
-# Old build view redirects
-
-
-def builds_redirect_list(request, project_slug):  # pylint: disable=unused-argument
-    return HttpResponsePermanentRedirect(
-        reverse('builds_project_list', args=[project_slug]),
-    )
-
-
-def builds_redirect_detail(request, project_slug, pk):  # pylint: disable=unused-argument
-    return HttpResponsePermanentRedirect(
-        reverse('builds_detail', args=[project_slug, pk]),
-    )

readthedocs/core/views/hooks.py

@@ -3,7 +3,6 @@
 import logging
 
 from readthedocs.builds.constants import EXTERNAL
-from readthedocs.builds.models import Version
 from readthedocs.core.utils import trigger_build
 from readthedocs.projects.tasks import sync_repository_task
 

readthedocs/projects/urls/private.py

@@ -1,8 +1,7 @@
-# -*- coding: utf-8 -*-
-
 """Project URLs for authenticated users."""
 
 from django.conf.urls import url
+from django.views.generic.base import RedirectView
 
 from readthedocs.constants import pattern_opts
 from readthedocs.projects.backends.views import ImportDemoView, ImportWizardView
@@ -45,7 +44,8 @@
         name='projects_import_demo',
     ),
     url(
-        r'^(?P<project_slug>[-\w]+)/$', private.project_manage,
+        r'^(?P<project_slug>[-\w]+)/$',
+        RedirectView.as_view(pattern_name='projects_detail', permanent=True),
         name='projects_manage',
     ),
     url(

readthedocs/projects/views/private.py

@@ -73,6 +73,7 @@
 
 
 class PrivateViewMixin(LoginRequiredMixin):
+
     pass
 
 
@@ -110,19 +111,6 @@ def get_context_data(self, **kwargs):
         return context
 
 
-@login_required
-def project_manage(__, project_slug):
-    """
-    Project management view.
-
-    Where you will have links to edit the projects' configuration, edit the
-    files associated with that project, etc.
-
-    Now redirects to the normal /projects/<slug> view.
-    """
-    return HttpResponseRedirect(reverse('projects_detail', args=[project_slug]))
-
-
 class ProjectUpdate(ProjectSpamMixin, PrivateViewMixin, UpdateView):
 
     form_class = UpdateProjectForm
@@ -453,17 +441,18 @@ def get_context_data(self, **kwargs):
 
 
 class ProjectRelationshipCreate(ProjectRelationshipMixin, CreateView):
+
     pass
 
 
 class ProjectRelationshipUpdate(ProjectRelationshipMixin, UpdateView):
+
     pass
 
 
 class ProjectRelationshipDelete(ProjectRelationshipMixin, DeleteView):
 
-    def get(self, request, *args, **kwargs):
-        return self.http_method_not_allowed(request, *args, **kwargs)
+    http_method_names = ['post']
 
 
 @login_required
@@ -727,14 +716,17 @@ def get_context_data(self, **kwargs):
 
 
 class DomainCreate(DomainMixin, CreateView):
+
     pass
 
 
 class DomainUpdate(DomainMixin, UpdateView):
+
     pass
 
 
 class DomainDelete(DomainMixin, DeleteView):
+
     pass
 
 
@@ -776,6 +768,7 @@ def get_template_names(self):
 
 
 class IntegrationList(IntegrationMixin, ListView):
+
     pass
 
 
@@ -824,8 +817,7 @@ def get_template_names(self):
 
 class IntegrationDelete(IntegrationMixin, DeleteView):
 
-    def get(self, request, *args, **kwargs):
-        return self.http_method_not_allowed(request, *args, **kwargs)
+    http_method_names = ['post']
 
 
 class IntegrationExchangeDetail(IntegrationMixin, DetailView):
@@ -900,22 +892,23 @@ def get_success_url(self):
 
 
 class EnvironmentVariableList(EnvironmentVariableMixin, ListView):
+
     pass
 
 
 class EnvironmentVariableCreate(EnvironmentVariableMixin, CreateView):
+
     pass
 
 
 class EnvironmentVariableDetail(EnvironmentVariableMixin, DetailView):
+
     pass
 
 
 class EnvironmentVariableDelete(EnvironmentVariableMixin, DeleteView):
 
-    # This removes the delete confirmation
-    def get(self, request, *args, **kwargs):
-        return self.http_method_not_allowed(request, *args, **kwargs)
+    http_method_names = ['post']
 
 
 @login_required

readthedocs/rtd_tests/tests/test_privacy_urls.py

@@ -234,9 +234,9 @@ def is_admin(self):
 class PrivateProjectAdminAccessTest(PrivateProjectMixin, TestCase):
 
     response_data = {
-        # Places where we 302 on success -- These delete pages should probably be 405'ing
+        # Places where we 302 on success, and 301 for old pages -- These delete pages should probably be 405'ing
         '/dashboard/import/manual/demo/': {'status_code': 302},
-        '/dashboard/pip/': {'status_code': 302},
+        '/dashboard/pip/': {'status_code': 301},
         '/dashboard/pip/subprojects/delete/sub/': {'status_code': 302},
         '/dashboard/pip/translations/delete/sub/': {'status_code': 302},
 
@@ -277,7 +277,7 @@ class PrivateProjectUserAccessTest(PrivateProjectMixin, TestCase):
         '/dashboard/import/manual/demo/': {'status_code': 302},
 
         # Unauth access redirect for non-owners
-        '/dashboard/pip/': {'status_code': 302},
+        '/dashboard/pip/': {'status_code': 301},
 
         # 405's where we should be POST'ing
         '/dashboard/pip/users/delete/': {'status_code': 405},
@@ -311,6 +311,11 @@ class PrivateProjectUnauthAccessTest(PrivateProjectMixin, TestCase):
     # Auth protected
     default_status_code = 302
 
+    response_data = {
+        # Old url, it redirects to a view that doesn't requires login.
+        '/dashboard/pip/': {'status_code': 301},
+    }
+
     def login(self):
         pass
 

readthedocs/rtd_tests/tests/test_views.py

@@ -1,9 +1,7 @@
 import csv
-import io
 from urllib.parse import urlsplit
 
 import mock
-import pytest
 from django.contrib.auth.models import User
 from django.test import TestCase
 from django.urls import reverse
@@ -12,12 +10,10 @@
 
 from readthedocs.builds.constants import EXTERNAL, LATEST
 from readthedocs.builds.models import Build, Version
-from readthedocs.core.models import UserProfile
 from readthedocs.core.permissions import AdminPermission
 from readthedocs.projects.constants import PUBLIC
 from readthedocs.projects.forms import UpdateProjectForm
-from readthedocs.projects.models import Feature, HTMLFile, Project
-from readthedocs.search.models import SearchQuery
+from readthedocs.projects.models import Feature, Project
 
 
 class Testmaker(TestCase):
@@ -89,10 +85,6 @@ def test_import_wizard_demo(self):
         response = self.client.get('/dashboard/import/manual/demo/')
         self.assertRedirectToLogin(response)
 
-    def test_projects_manage(self):
-        response = self.client.get('/dashboard/pip/')
-        self.assertRedirectToLogin(response)
-
     def test_edit(self):
         response = self.client.get('/dashboard/pip/edit/')
         self.assertRedirectToLogin(response)