diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py
index 5628efe9f36..075c3c3fa58 100644
--- a/readthedocs/projects/forms.py
+++ b/readthedocs/projects/forms.py
@@ -669,9 +669,11 @@ class RedirectForm(forms.ModelForm):
"""Form for project redirects."""
+ project = forms.CharField(widget=forms.HiddenInput(), required=False, disabled=True)
+
class Meta:
model = Redirect
- fields = ["redirect_type", "from_url", "to_url", "force"]
+ fields = ["project", "redirect_type", "from_url", "to_url", "force"]
def __init__(self, *args, **kwargs):
self.project = kwargs.pop('project', None)
@@ -685,18 +687,8 @@ def __init__(self, *args, **kwargs):
else:
self.fields.pop("force")
- def save(self, **_): # pylint: disable=arguments-differ
- # TODO this should respect the unused argument `commit`. It's not clear
- # why this needs to be a call to `create`, instead of relying on the
- # super `save()` call.
- redirect = Redirect.objects.create(
- project=self.project,
- redirect_type=self.cleaned_data["redirect_type"],
- from_url=self.cleaned_data["from_url"],
- to_url=self.cleaned_data["to_url"],
- force=self.cleaned_data.get("force", False),
- )
- return redirect
+ def clean_project(self):
+ return self.project
class DomainBaseForm(forms.ModelForm):
diff --git a/readthedocs/projects/urls/private.py b/readthedocs/projects/urls/private.py
index 1d413ba758b..06469f6812b 100644
--- a/readthedocs/projects/urls/private.py
+++ b/readthedocs/projects/urls/private.py
@@ -32,8 +32,10 @@
ProjectDelete,
ProjectNotifications,
ProjectNotificationsDelete,
- ProjectRedirects,
+ ProjectRedirectsCreate,
ProjectRedirectsDelete,
+ ProjectRedirectsList,
+ ProjectRedirectsUpdate,
ProjectTranslationsDelete,
ProjectTranslationsListAndCreate,
ProjectUpdate,
@@ -125,11 +127,21 @@
),
re_path(
r'^(?P[-\w]+)/redirects/$',
- ProjectRedirects.as_view(),
+ ProjectRedirectsList.as_view(),
name='projects_redirects',
),
re_path(
- r'^(?P[-\w]+)/redirects/delete/$',
+ r"^(?P[-\w]+)/redirects/create/$",
+ ProjectRedirectsCreate.as_view(),
+ name="projects_redirects_create",
+ ),
+ re_path(
+ r"^(?P[-\w]+)/redirects/(?P[-\w]+)/edit/$",
+ ProjectRedirectsUpdate.as_view(),
+ name="projects_redirects_edit",
+ ),
+ re_path(
+ r"^(?P[-\w]+)/redirects/(?P[-\w]+)/delete/$",
ProjectRedirectsDelete.as_view(),
name='projects_redirects_delete',
),
diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py
index 2364510956e..c39e201c1c5 100644
--- a/readthedocs/projects/views/private.py
+++ b/readthedocs/projects/views/private.py
@@ -712,44 +712,40 @@ class ProjectRedirectsMixin(ProjectAdminMixin, PrivateViewMixin):
"""Project redirects view and form view."""
+ form_class = RedirectForm
+ template_name = "redirects/redirect_form.html"
+ context_object_name = "redirect"
+ lookup_url_kwarg = "redirect_pk"
+
def get_success_url(self):
return reverse(
'projects_redirects',
args=[self.get_project().slug],
)
+ def get_queryset(self):
+ return self.get_project().redirects.all()
-class ProjectRedirects(ProjectRedirectsMixin, FormView):
- form_class = RedirectForm
- template_name = 'projects/project_redirects.html'
+class ProjectRedirectsList(ProjectRedirectsMixin, ListView):
- def form_valid(self, form):
- form.save()
- return HttpResponseRedirect(self.get_success_url())
+ template_name = "redirects/redirect_list.html"
+ context_object_name = "redirects"
- def get_context_data(self, **kwargs):
- context = super().get_context_data(**kwargs)
- project = self.get_project()
- context['redirects'] = project.redirects.all()
- return context
+class ProjectRedirectsCreate(ProjectRedirectsMixin, CreateView):
+
+ pass
-class ProjectRedirectsDelete(ProjectRedirectsMixin, GenericView):
- http_method_names = ['post']
+class ProjectRedirectsUpdate(ProjectRedirectsMixin, UpdateView):
- def post(self, request, *args, **kwargs):
- project = self.get_project()
- redirect = get_object_or_404(
- project.redirects,
- pk=request.POST.get('id_pk'),
- )
- if redirect.project == project:
- redirect.delete()
- else:
- raise Http404
- return HttpResponseRedirect(self.get_success_url())
+ pass
+
+
+class ProjectRedirectsDelete(ProjectRedirectsMixin, DeleteView):
+
+ http_method_names = ['post']
class DomainMixin(ProjectAdminMixin, PrivateViewMixin):
diff --git a/readthedocs/templates/projects/project_redirects.html b/readthedocs/redirects/templates/redirects/redirect_form.html
similarity index 51%
rename from readthedocs/templates/projects/project_redirects.html
rename to readthedocs/redirects/templates/redirects/redirect_form.html
index 449853c3723..c066fae59c6 100644
--- a/readthedocs/templates/projects/project_redirects.html
+++ b/readthedocs/redirects/templates/redirects/redirect_form.html
@@ -2,7 +2,7 @@
{% load i18n %}
-{% block title %}{% trans "Edit Redirects" %}{% endblock %}
+{% block title %}{% trans "Redirect" %}{% endblock %}
{% block project-redirects-active %}active{% endblock %}
{% block project_edit_content_header %}{% trans "Redirects" %}{% endblock %}
@@ -10,7 +10,25 @@
{% block extra_scripts %}
@@ -76,44 +74,18 @@
{% endblocktrans %}
- {% if redirects|length > 0 %}
- {% trans "Redirects" %}
-
- {% endif %}
-
+
+ {% if redirect %}
+
+ {% endif %}
{% endblock %}
diff --git a/readthedocs/redirects/templates/redirects/redirect_list.html b/readthedocs/redirects/templates/redirects/redirect_list.html
new file mode 100644
index 00000000000..998125f8931
--- /dev/null
+++ b/readthedocs/redirects/templates/redirects/redirect_list.html
@@ -0,0 +1,61 @@
+{% extends "projects/project_edit_base.html" %}
+
+{% load i18n %}
+
+{% block title %}{% trans "Redirects" %}{% endblock %}
+
+{% block project-redirects-active %}active{% endblock %}
+{% block project_edit_content_header %}{% trans "Redirects" %}{% endblock %}
+
+{% block project_edit_content %}
+
+ {% blocktrans trimmed with docs_url='https://docs.readthedocs.io/page/user-defined-redirects.html' %}
+ Add redirects for your project. This allows you to fix links to old pages that are 404ing. Learn more.
+ {% endblocktrans %}
+
+
+
+
+
+
+
+
+ {% for redirect in redirects %}
+ -
+
+ {{ redirect.get_redirect_type_display }}
+
+ {% if redirect.get_from_to_url_display %}
+
+ {{ redirect.get_from_to_url_display }}
+
+ {% endif %}
+
+
+ {% empty %}
+ -
+
+ {% trans "No redirects found." %}
+
+
+ {% endfor %}
+
+
+
+
+{% endblock %}
diff --git a/readthedocs/redirects/tests/__init__.py b/readthedocs/redirects/tests/__init__.py
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/readthedocs/redirects/tests/test_views.py b/readthedocs/redirects/tests/test_views.py
new file mode 100644
index 00000000000..48ed0b8d002
--- /dev/null
+++ b/readthedocs/redirects/tests/test_views.py
@@ -0,0 +1,82 @@
+from django.contrib.auth.models import User
+from django.test import TestCase
+from django.urls import reverse
+from django_dynamic_fixture import get
+
+from readthedocs.projects.models import Project
+from readthedocs.redirects.models import Redirect
+
+
+class TestViewsR(TestCase):
+ def setUp(self):
+ self.user = get(User)
+ self.project = get(Project, slug="test", users=[self.user])
+ self.redirect = get(
+ Redirect,
+ project=self.project,
+ redirect_type="exact",
+ from_url="/404.html",
+ to_url="/en/latest/",
+ )
+ self.client.force_login(self.user)
+
+ def test_create_redirect(self):
+ self.assertEqual(self.project.redirects.all().count(), 1)
+ resp = self.client.post(
+ reverse("projects_redirects_create", args=[self.project.slug]),
+ data={
+ "redirect_type": "page",
+ "from_url": "/config.html",
+ "to_url": "/configuration.html",
+ },
+ )
+ self.assertEqual(resp.status_code, 302)
+ self.assertEqual(self.project.redirects.all().count(), 2)
+
+ def test_update_redirect(self):
+ self.assertEqual(self.project.redirects.all().count(), 1)
+ resp = self.client.post(
+ reverse(
+ "projects_redirects_edit", args=[self.project.slug, self.redirect.pk]
+ ),
+ data={
+ "redirect_type": "page",
+ "from_url": "/config.html",
+ "to_url": "/configuration.html",
+ },
+ )
+ self.assertEqual(resp.status_code, 302)
+ redirect = self.project.redirects.get()
+ self.assertEqual(redirect.redirect_type, "page")
+ self.assertEqual(redirect.from_url, "/config.html")
+ self.assertEqual(redirect.to_url, "/configuration.html")
+ self.assertEqual(self.project.redirects.all().count(), 1)
+
+ def test_delete_redirect(self):
+ self.assertEqual(self.project.redirects.all().count(), 1)
+ resp = self.client.post(
+ reverse(
+ "projects_redirects_delete", args=[self.project.slug, self.redirect.pk]
+ ),
+ )
+ self.assertEqual(resp.status_code, 302)
+ self.assertEqual(self.project.redirects.all().count(), 0)
+
+ def test_list_redirect(self):
+ resp = self.client.get(
+ reverse("projects_redirects", args=[self.project.slug]),
+ )
+ self.assertEqual(resp.status_code, 200)
+ self.assertContains(resp, self.redirect.from_url)
+ self.assertContains(resp, self.redirect.to_url)
+
+ def test_get_redirect(self):
+ resp = self.client.get(
+ reverse(
+ "projects_redirects_edit", args=[self.project.slug, self.redirect.pk]
+ ),
+ )
+ self.assertEqual(resp.status_code, 200)
+ self.assertContains(resp, self.redirect.redirect_type)
+ self.assertContains(resp, self.redirect.from_url)
+ self.assertContains(resp, self.redirect.to_url)
diff --git a/readthedocs/rtd_tests/tests/test_privacy_urls.py b/readthedocs/rtd_tests/tests/test_privacy_urls.py
index d769590a389..646535edc2e 100644
--- a/readthedocs/rtd_tests/tests/test_privacy_urls.py
+++ b/readthedocs/rtd_tests/tests/test_privacy_urls.py
@@ -18,12 +18,8 @@
from readthedocs.core.utils.tasks import TaskNoPermission
from readthedocs.integrations.models import HttpExchange, Integration
from readthedocs.oauth.models import RemoteOrganization, RemoteRepository
-from readthedocs.projects.models import (
- Domain,
- EnvironmentVariable,
- Project,
- WebHook,
-)
+from readthedocs.projects.models import Domain, EnvironmentVariable, Project, WebHook
+from readthedocs.redirects.models import Redirect
from readthedocs.rtd_tests.utils import create_user
@@ -187,24 +183,30 @@ def setUp(self):
response_headers={'foo': 'bar'},
status_code=200,
)
+ self.redirect = get(
+ Redirect,
+ project=self.pip,
+ redirect_type="sphinx_html",
+ )
self.default_kwargs = {
- 'project_slug': self.pip.slug,
- 'subproject_slug': self.subproject.slug,
- 'version_slug': self.pip.versions.all()[0].slug,
- 'filename': 'index.html',
- 'type_': 'pdf',
- 'tag': self.tag.slug,
- 'child_slug': self.subproject.slug,
- 'build_pk': self.build.pk,
- 'domain_pk': self.domain.pk,
- 'integration_pk': self.integration.pk,
- 'exchange_pk': self.integration_exchange.pk,
- 'environmentvariable_pk': self.environment_variable.pk,
- 'automation_rule_pk': self.automation_rule.pk,
- 'steps': 1,
- 'invalid_project_slug': 'invalid_slug',
- 'webhook_pk': self.webhook.pk,
- 'webhook_exchange_pk': self.webhook_exchange.pk,
+ "project_slug": self.pip.slug,
+ "subproject_slug": self.subproject.slug,
+ "version_slug": self.pip.versions.all()[0].slug,
+ "filename": "index.html",
+ "type_": "pdf",
+ "tag": self.tag.slug,
+ "child_slug": self.subproject.slug,
+ "build_pk": self.build.pk,
+ "domain_pk": self.domain.pk,
+ "redirect_pk": self.redirect.pk,
+ "integration_pk": self.integration.pk,
+ "exchange_pk": self.integration_exchange.pk,
+ "environmentvariable_pk": self.environment_variable.pk,
+ "automation_rule_pk": self.automation_rule.pk,
+ "steps": 1,
+ "invalid_project_slug": "invalid_slug",
+ "webhook_pk": self.webhook.pk,
+ "webhook_exchange_pk": self.webhook_exchange.pk,
}
@@ -277,28 +279,31 @@ class PrivateProjectAdminAccessTest(PrivateProjectMixin, TestCase):
'/dashboard/pip/subprojects/delete/sub/': {'status_code': 302},
# 405's where we should be POST'ing
- '/dashboard/pip/users/delete/': {'status_code': 405},
- '/dashboard/pip/notifications/delete/': {'status_code': 405},
- '/dashboard/pip/redirects/delete/': {'status_code': 405},
- '/dashboard/pip/subprojects/sub/delete/': {'status_code': 405},
- '/dashboard/pip/integrations/sync/': {'status_code': 405},
- '/dashboard/pip/integrations/{integration_id}/sync/': {'status_code': 405},
- '/dashboard/pip/integrations/{integration_id}/delete/': {'status_code': 405},
- '/dashboard/pip/environmentvariables/{environmentvariable_id}/delete/': {'status_code': 405},
- '/dashboard/pip/translations/delete/sub/': {'status_code': 405},
- '/dashboard/pip/version/latest/delete_html/': {'status_code': 405},
- '/dashboard/pip/rules/{automation_rule_id}/delete/': {'status_code': 405},
- '/dashboard/pip/rules/{automation_rule_id}/move/{steps}/': {'status_code': 405},
- '/dashboard/pip/webhooks/{webhook_id}/delete/': {'status_code': 405},
+ "/dashboard/pip/users/delete/": {"status_code": 405},
+ "/dashboard/pip/notifications/delete/": {"status_code": 405},
+ "/dashboard/pip/redirects/{redirect_pk}/delete/": {"status_code": 405},
+ "/dashboard/pip/subprojects/sub/delete/": {"status_code": 405},
+ "/dashboard/pip/integrations/sync/": {"status_code": 405},
+ "/dashboard/pip/integrations/{integration_id}/sync/": {"status_code": 405},
+ "/dashboard/pip/integrations/{integration_id}/delete/": {"status_code": 405},
+ "/dashboard/pip/environmentvariables/{environmentvariable_id}/delete/": {
+ "status_code": 405
+ },
+ "/dashboard/pip/translations/delete/sub/": {"status_code": 405},
+ "/dashboard/pip/version/latest/delete_html/": {"status_code": 405},
+ "/dashboard/pip/rules/{automation_rule_id}/delete/": {"status_code": 405},
+ "/dashboard/pip/rules/{automation_rule_id}/move/{steps}/": {"status_code": 405},
+ "/dashboard/pip/webhooks/{webhook_id}/delete/": {"status_code": 405},
}
def get_url_path_ctx(self):
return {
- 'integration_id': self.integration.id,
- 'environmentvariable_id': self.environment_variable.id,
- 'automation_rule_id': self.automation_rule.id,
- 'webhook_id': self.webhook.id,
- 'steps': 1,
+ "integration_id": self.integration.id,
+ "environmentvariable_id": self.environment_variable.id,
+ "automation_rule_id": self.automation_rule.id,
+ "webhook_id": self.webhook.id,
+ "redirect_pk": self.redirect.pk,
+ "steps": 1,
}
def login(self):
@@ -322,19 +327,21 @@ class PrivateProjectUserAccessTest(PrivateProjectMixin, TestCase):
'/dashboard/pip/': {'status_code': 301},
# 405's where we should be POST'ing
- '/dashboard/pip/users/delete/': {'status_code': 405},
- '/dashboard/pip/notifications/delete/': {'status_code': 405},
- '/dashboard/pip/redirects/delete/': {'status_code': 405},
- '/dashboard/pip/subprojects/sub/delete/': {'status_code': 405},
- '/dashboard/pip/integrations/sync/': {'status_code': 405},
- '/dashboard/pip/integrations/{integration_id}/sync/': {'status_code': 405},
- '/dashboard/pip/integrations/{integration_id}/delete/': {'status_code': 405},
- '/dashboard/pip/environmentvariables/{environmentvariable_id}/delete/': {'status_code': 405},
- '/dashboard/pip/translations/delete/sub/': {'status_code': 405},
- '/dashboard/pip/version/latest/delete_html/': {'status_code': 405},
- '/dashboard/pip/rules/{automation_rule_id}/delete/': {'status_code': 405},
- '/dashboard/pip/rules/{automation_rule_id}/move/{steps}/': {'status_code': 405},
- '/dashboard/pip/webhooks/{webhook_id}/delete/': {'status_code': 405},
+ "/dashboard/pip/users/delete/": {"status_code": 405},
+ "/dashboard/pip/notifications/delete/": {"status_code": 405},
+ "/dashboard/pip/redirects/{redirect_pk}/delete/": {"status_code": 405},
+ "/dashboard/pip/subprojects/sub/delete/": {"status_code": 405},
+ "/dashboard/pip/integrations/sync/": {"status_code": 405},
+ "/dashboard/pip/integrations/{integration_id}/sync/": {"status_code": 405},
+ "/dashboard/pip/integrations/{integration_id}/delete/": {"status_code": 405},
+ "/dashboard/pip/environmentvariables/{environmentvariable_id}/delete/": {
+ "status_code": 405
+ },
+ "/dashboard/pip/translations/delete/sub/": {"status_code": 405},
+ "/dashboard/pip/version/latest/delete_html/": {"status_code": 405},
+ "/dashboard/pip/rules/{automation_rule_id}/delete/": {"status_code": 405},
+ "/dashboard/pip/rules/{automation_rule_id}/move/{steps}/": {"status_code": 405},
+ "/dashboard/pip/webhooks/{webhook_id}/delete/": {"status_code": 405},
}
# Filtered out by queryset on projects that we don't own.
@@ -342,11 +349,12 @@ class PrivateProjectUserAccessTest(PrivateProjectMixin, TestCase):
def get_url_path_ctx(self):
return {
- 'integration_id': self.integration.id,
- 'environmentvariable_id': self.environment_variable.id,
- 'automation_rule_id': self.automation_rule.id,
- 'webhook_id': self.webhook.id,
- 'steps': 1,
+ "integration_id": self.integration.id,
+ "environmentvariable_id": self.environment_variable.id,
+ "automation_rule_id": self.automation_rule.id,
+ "webhook_id": self.webhook.id,
+ "redirect_pk": self.redirect.pk,
+ "steps": 1,
}
def login(self):
diff --git a/readthedocs/rtd_tests/tests/test_views.py b/readthedocs/rtd_tests/tests/test_views.py
index 4e4b202df67..3de44848ab2 100644
--- a/readthedocs/rtd_tests/tests/test_views.py
+++ b/readthedocs/rtd_tests/tests/test_views.py
@@ -150,7 +150,9 @@ def test_project_redirects(self):
self.assertRedirectToLogin(response)
def test_project_redirects_delete(self):
- response = self.client.get('/dashboard/pip/redirects/delete/')
+ response = self.client.get(
+ reverse("projects_redirects_delete", args=["pip", 3])
+ )
self.assertRedirectToLogin(response)