Skip to content

Clean up views #6166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions readthedocs/builds/urls.py
Original file line number Diff line number Diff line change
@@ -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',
),
]
24 changes: 4 additions & 20 deletions readthedocs/builds/views.py
Original file line number Diff line number Diff line change
@@ -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):
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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]),
)
1 change: 0 additions & 1 deletion readthedocs/core/views/hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 3 additions & 3 deletions readthedocs/projects/urls/private.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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(
Expand Down
33 changes: 13 additions & 20 deletions readthedocs/projects/views/private.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@


class PrivateViewMixin(LoginRequiredMixin):

pass


Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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


Expand Down Expand Up @@ -776,6 +768,7 @@ def get_template_names(self):


class IntegrationList(IntegrationMixin, ListView):

pass


Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand Down
11 changes: 8 additions & 3 deletions readthedocs/rtd_tests/tests/test_privacy_urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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},

Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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

Expand Down
10 changes: 1 addition & 9 deletions readthedocs/rtd_tests/tests/test_views.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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)
Expand Down