From 4d14403b0069e4eb3e5e0ded494f046d70034650 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 21 Feb 2022 14:13:16 -0500 Subject: [PATCH 01/10] Custom domains: warn users about adding a custom domain to a subproject Needs https://github.com/readthedocs/readthedocs.org/pull/8945/. --- readthedocs/projects/models.py | 7 +++++++ readthedocs/projects/views/base.py | 4 +++- readthedocs/projects/views/private.py | 5 +---- .../templates/projects/domain_list.html | 21 ++++++++++++++----- .../projects/projectrelationship_list.html | 6 +++--- 5 files changed, 30 insertions(+), 13 deletions(-) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 214d9d00d6c..8a8376ea534 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -734,6 +734,13 @@ def is_subproject(self): """Return whether or not this project is a subproject.""" return self.superprojects.exists() + @property + def superproject(self): + relationship = self.get_parent_relationship() + if relationship: + return relationship.parent + return None + @property def alias(self): """Return the alias (as subproject) if it's a subproject.""" # noqa diff --git a/readthedocs/projects/views/base.py b/readthedocs/projects/views/base.py index ba76f7c137f..bbb911f63a1 100644 --- a/readthedocs/projects/views/base.py +++ b/readthedocs/projects/views/base.py @@ -72,7 +72,9 @@ def get_project(self): def get_context_data(self, **kwargs): """Add project to context data.""" context = super().get_context_data(**kwargs) - context['project'] = self.get_project() + project = self.get_project() + context['project'] = project + context['superproject'] = project and project.superproject return context def get_form(self, data=None, files=None, **kwargs): diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index 8f0c4d2105f..7ea361b8fac 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -435,10 +435,7 @@ def get_success_url(self): class ProjectRelationshipList(ProjectRelationListMixin, ProjectRelationshipMixin, ListView): - def get_context_data(self, **kwargs): - ctx = super().get_context_data(**kwargs) - ctx['superproject'] = self.project.superprojects.first() - return ctx + pass class ProjectRelationshipCreate(ProjectRelationshipMixin, CreateView): diff --git a/readthedocs/templates/projects/domain_list.html b/readthedocs/templates/projects/domain_list.html index 500570bd507..1d3a56ebeb7 100644 --- a/readthedocs/templates/projects/domain_list.html +++ b/readthedocs/templates/projects/domain_list.html @@ -12,11 +12,22 @@ {% block project_edit_content_header %}{% trans "Domains" %}{% endblock %} {% block project_edit_content %} -

- {% blocktrans trimmed with docs_url='https://docs.readthedocs.io/page/custom_domains.html' %} - Configuring a custom domain allows you to serve your documentation from a domain other than "{{ default_domain }}". Learn more. - {% endblocktrans %} -

+ {% if superproject %} + {% url "projects_detail" superproject.slug as superproject_url %} +

+ {% blocktrans trimmed with superproject=superproject.name superproject_url=superproject_url domain=project.subdomain %} + This project is a subproject of {{ superproject }}, + its documentation will always be served from the {{ domain }} domain. + Learn more. + {% endblocktrans %} +

+ {% else %} +

+ {% blocktrans trimmed with docs_url='https://docs.readthedocs.io/page/custom_domains.html' %} + Configuring a custom domain allows you to serve your documentation from a domain other than "{{ default_domain }}". Learn more. + {% endblocktrans %} +

+ {% endif %} {% if object_list %}

{% trans "Existing Domains" %}

diff --git a/readthedocs/templates/projects/projectrelationship_list.html b/readthedocs/templates/projects/projectrelationship_list.html index d5a5894eef8..de4e0be73ba 100644 --- a/readthedocs/templates/projects/projectrelationship_list.html +++ b/readthedocs/templates/projects/projectrelationship_list.html @@ -21,14 +21,14 @@ {% if superproject %}

- {% blocktrans trimmed with project=superproject.parent.name %} + {% blocktrans trimmed with project=superproject.name %} This project is already configured as a subproject of {{ project }}. Nested subprojects are not currently supported. {% endblocktrans %}

- - {% blocktrans trimmed with project=superproject.parent.name %} + + {% blocktrans trimmed with project=superproject.name %} View subprojects of {{ project }} {% endblocktrans %} From 73d1f3deb2af0214d9c5cd86d5afcd9f3eae605c Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 2 Mar 2022 17:25:51 -0500 Subject: [PATCH 02/10] Lint --- readthedocs/projects/models.py | 16 ++++------------ readthedocs/projects/views/base.py | 15 ++++++++------- readthedocs/projects/views/private.py | 1 - 3 files changed, 12 insertions(+), 20 deletions(-) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 8a8376ea534..c036598240b 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -2,12 +2,12 @@ import fnmatch import hashlib import hmac -import structlog import os import re from shlex import quote from urllib.parse import urlparse +import structlog from allauth.socialaccount.providers import registry as allauth_registry from django.conf import settings from django.conf.urls import include @@ -21,10 +21,7 @@ from django.utils.functional import cached_property from django.utils.translation import gettext_lazy as _ from django.views import defaults -from django_extensions.db.fields import ( - CreationDateTimeField, - ModificationDateTimeField, -) +from django_extensions.db.fields import CreationDateTimeField, ModificationDateTimeField from django_extensions.db.models import TimeStampedModel from taggit.managers import TaggableManager @@ -54,12 +51,7 @@ from readthedocs.storage import build_media_storage from readthedocs.vcs_support.backends import backend_cls -from .constants import ( - MEDIA_TYPE_EPUB, - MEDIA_TYPE_HTMLZIP, - MEDIA_TYPE_PDF, - MEDIA_TYPES, -) +from .constants import MEDIA_TYPE_EPUB, MEDIA_TYPE_HTMLZIP, MEDIA_TYPE_PDF, MEDIA_TYPES log = structlog.get_logger(__name__) @@ -676,9 +668,9 @@ def proxito_urlconf(self): It is used for doc serving on projects that have their own ``urlconf``. """ from readthedocs.projects.views.public import ProjectDownloadMedia + from readthedocs.proxito.urls import core_urls from readthedocs.proxito.views.serve import ServeDocs from readthedocs.proxito.views.utils import proxito_404_page_handler - from readthedocs.proxito.urls import core_urls class ProxitoURLConf: diff --git a/readthedocs/projects/views/base.py b/readthedocs/projects/views/base.py index bbb911f63a1..290afe6c8f5 100644 --- a/readthedocs/projects/views/base.py +++ b/readthedocs/projects/views/base.py @@ -1,11 +1,9 @@ """Mix-in classes for project views.""" -import structlog from functools import lru_cache +import structlog from django.conf import settings -from django.http import HttpResponseRedirect -from django.shortcuts import render, get_object_or_404 -from django.urls import reverse +from django.shortcuts import get_object_or_404, render from readthedocs.projects.models import Project @@ -73,8 +71,8 @@ def get_context_data(self, **kwargs): """Add project to context data.""" context = super().get_context_data(**kwargs) project = self.get_project() - context['project'] = project - context['superproject'] = project and project.superproject + context["project"] = project + context["superproject"] = project and project.superproject return context def get_form(self, data=None, files=None, **kwargs): @@ -94,7 +92,10 @@ class ProjectSpamMixin: def get(self, request, *args, **kwargs): if 'readthedocsext.spamfighting' in settings.INSTALLED_APPS: - from readthedocsext.spamfighting.utils import is_show_dashboard_denied # noqa + # noqa + from readthedocsext.spamfighting.utils import ( + is_show_dashboard_denied, + ) if is_show_dashboard_denied(self.get_project()): return render(request, template_name='spam.html', status=410) diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index 7ea361b8fac..e3cb2c38c22 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -1,7 +1,6 @@ """Project views for authenticated users.""" import structlog - from allauth.socialaccount.models import SocialAccount from django.conf import settings from django.contrib import messages From 74c69cdbca4251e50b4790ca6de3ccbc2fd5efb1 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 2 Mar 2022 17:26:17 -0500 Subject: [PATCH 03/10] Lint --- readthedocs/projects/models.py | 1 - readthedocs/projects/views/base.py | 4 +--- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index c036598240b..a2c94e7a97d 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -31,7 +31,6 @@ from readthedocs.core.history import ExtraHistoricalRecords from readthedocs.core.resolver import resolve, resolve_domain from readthedocs.core.utils import slugify -from readthedocs.doc_builder.constants import DOCKER_LIMITS from readthedocs.projects import constants from readthedocs.projects.exceptions import ProjectConfigurationError from readthedocs.projects.managers import HTMLFileManager diff --git a/readthedocs/projects/views/base.py b/readthedocs/projects/views/base.py index 290afe6c8f5..08b0da8f28c 100644 --- a/readthedocs/projects/views/base.py +++ b/readthedocs/projects/views/base.py @@ -93,9 +93,7 @@ class ProjectSpamMixin: def get(self, request, *args, **kwargs): if 'readthedocsext.spamfighting' in settings.INSTALLED_APPS: # noqa - from readthedocsext.spamfighting.utils import ( - is_show_dashboard_denied, - ) + from readthedocsext.spamfighting.utils import is_show_dashboard_denied if is_show_dashboard_denied(self.get_project()): return render(request, template_name='spam.html', status=410) From 85ea9a866c854a0258156159d9f9e9d34f0c7be7 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 2 Mar 2022 17:39:05 -0500 Subject: [PATCH 04/10] Lint --- readthedocs/projects/views/base.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/readthedocs/projects/views/base.py b/readthedocs/projects/views/base.py index 08b0da8f28c..2ba29212e06 100644 --- a/readthedocs/projects/views/base.py +++ b/readthedocs/projects/views/base.py @@ -92,8 +92,9 @@ class ProjectSpamMixin: def get(self, request, *args, **kwargs): if 'readthedocsext.spamfighting' in settings.INSTALLED_APPS: - # noqa - from readthedocsext.spamfighting.utils import is_show_dashboard_denied + from readthedocsext.spamfighting.utils import ( # noqa + is_show_dashboard_denied, + ) if is_show_dashboard_denied(self.get_project()): return render(request, template_name='spam.html', status=410) From 60ca4cd70b067ce131e83eda3019d079ece8b511 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 31 Mar 2022 10:20:20 -0500 Subject: [PATCH 05/10] Don't allow adding/editing domains on subprojects --- readthedocs/projects/views/private.py | 4 +-- .../templates/projects/domain_form.html | 2 +- .../templates/projects/domain_list.html | 26 ++++++++++--------- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index e3cb2c38c22..60400f1bf17 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -782,7 +782,7 @@ class DomainCreateBase(DomainMixin, CreateView): def post(self, request, *args, **kwargs): project = self.get_project() - if self._is_enabled(project): + if self._is_enabled(project) and not project.superproject: return super().post(request, *args, **kwargs) return HttpResponse('Action not allowed', status=401) @@ -806,7 +806,7 @@ class DomainUpdateBase(DomainMixin, UpdateView): def post(self, request, *args, **kwargs): project = self.get_project() - if self._is_enabled(project): + if self._is_enabled(project) and not project.superproject: return super().post(request, *args, **kwargs) return HttpResponse('Action not allowed', status=401) diff --git a/readthedocs/templates/projects/domain_form.html b/readthedocs/templates/projects/domain_form.html index debe65ef8bc..4c292474a23 100644 --- a/readthedocs/templates/projects/domain_form.html +++ b/readthedocs/templates/projects/domain_form.html @@ -49,7 +49,7 @@ {% endif %}
{% csrf_token %} {{ form.as_p }} - + {% if domain.domainssl %}

{% trans 'Saving the domain will revalidate the SSL certificate' %}

{% endif %} diff --git a/readthedocs/templates/projects/domain_list.html b/readthedocs/templates/projects/domain_list.html index 1d3a56ebeb7..3c304ebccaf 100644 --- a/readthedocs/templates/projects/domain_list.html +++ b/readthedocs/templates/projects/domain_list.html @@ -14,7 +14,7 @@ {% block project_edit_content %} {% if superproject %} {% url "projects_detail" superproject.slug as superproject_url %} -

+

{% blocktrans trimmed with superproject=superproject.name superproject_url=superproject_url domain=project.subdomain %} This project is a subproject of {{ superproject }}, its documentation will always be served from the {{ domain }} domain. @@ -44,16 +44,18 @@

{% trans "Existing Domains" %}

{% endif %} -

{% trans "Add new Domain" %}

- - {% if not enabled %} - {% include 'projects/includes/feature_disabled.html' with project=project %} - {% else %} - {% csrf_token %} - {{ form.as_p }} -

- -

-
+ {% if not superproject %} +

{% trans "Add new Domain" %}

+ + {% if not enabled %} + {% include 'projects/includes/feature_disabled.html' with project=project %} + {% else %} +
{% csrf_token %} + {{ form.as_p }} +

+ +

+
+ {% endif %} {% endif %} {% endblock %} From 403d1e699caeea912449a7da4f70271ea40bbdd5 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 31 Mar 2022 10:24:21 -0500 Subject: [PATCH 06/10] Update docs --- docs/user/subprojects.rst | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/docs/user/subprojects.rst b/docs/user/subprojects.rst index d734246d001..41753175137 100644 --- a/docs/user/subprojects.rst +++ b/docs/user/subprojects.rst @@ -42,15 +42,10 @@ https://docs.example.com/projects/bar/en/latest/ Custom domain on subprojects ---------------------------- -Adding a custom domain to a subproject is allowed, -but your documentation will always be served from +Adding a custom domain to a subproject is not allowed, +since your documentation will always be served from the domain of the parent project. -For example, if the domain of a parent project is ``https://docs.example.com``, -and you add the ``https://subproject.example.com/`` domain to one of its subprojects, -it will always redirect to the domain of the parent project -``https://docs.example.com/projects/subproject/``. - Search ------ From 5a8b9261a424649a3d9a614c602d15e4b69ef5b3 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 31 Mar 2022 13:13:40 -0500 Subject: [PATCH 07/10] Tests --- .../projects/tests/test_domain_views.py | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 readthedocs/projects/tests/test_domain_views.py diff --git a/readthedocs/projects/tests/test_domain_views.py b/readthedocs/projects/tests/test_domain_views.py new file mode 100644 index 00000000000..1a75a5420f5 --- /dev/null +++ b/readthedocs/projects/tests/test_domain_views.py @@ -0,0 +1,107 @@ +from unittest import mock + +from django.contrib.auth.models import User +from django.test import TestCase, override_settings +from django.urls import reverse +from django_dynamic_fixture import get + +from readthedocs.organizations.models import Organization +from readthedocs.projects.models import Domain, Project +from readthedocs.projects.views.private import DomainMixin + + +@override_settings(RTD_ALLOW_ORGANIZATIONS=False) +class TestDomainViews(TestCase): + def setUp(self): + self.user = get(User, username="user") + self.project = get(Project, users=[self.user], slug="project") + self.subproject = get(Project, users=[self.user], slug="subproject") + self.project.add_subproject(self.subproject) + self.client.force_login(self.user) + + def test_domain_creation(self): + self.assertEqual(self.project.domains.count(), 0) + + resp = self.client.post( + reverse("projects_domains_create", args=[self.project.slug]), + data={"domain": "test.example.com"}, + ) + self.assertEqual(resp.status_code, 302) + self.assertEqual(self.project.domains.count(), 1) + + domain = self.project.domains.first() + self.assertEqual(domain.domain, "test.example.com") + + def test_domain_deletion(self): + domain = get(Domain, project=self.project, domain="test.example.com") + self.assertEqual(self.project.domains.count(), 1) + + resp = self.client.post( + reverse("projects_domains_delete", args=[self.project.slug, domain.pk]), + ) + self.assertEqual(resp.status_code, 302) + self.assertEqual(self.project.domains.count(), 0) + + def test_domain_edit(self): + domain = get( + Domain, project=self.project, domain="test.example.com", canonical=False + ) + + self.assertEqual(domain.canonical, False) + resp = self.client.post( + reverse("projects_domains_edit", args=[self.project.slug, domain.pk]), + data={"canonical": True}, + ) + self.assertEqual(resp.status_code, 302) + self.assertEqual(self.project.domains.count(), 1) + + domain = self.project.domains.first() + self.assertEqual(domain.domain, "test.example.com") + self.assertEqual(domain.canonical, True) + + def test_adding_domain_on_subproject(self): + self.assertEqual(self.subproject.domains.count(), 0) + + resp = self.client.post( + reverse("projects_domains_create", args=[self.subproject.slug]), + data={"domain": "test.example.com"}, + ) + self.assertEqual(resp.status_code, 401) + self.assertEqual(self.subproject.domains.count(), 0) + + def test_delete_domain_on_subproject(self): + domain = get(Domain, project=self.subproject, domain="test.example.com") + self.assertEqual(self.subproject.domains.count(), 1) + + resp = self.client.post( + reverse("projects_domains_delete", args=[self.subproject.slug, domain.pk]), + ) + self.assertEqual(resp.status_code, 302) + self.assertEqual(self.subproject.domains.count(), 0) + + def test_edit_domain_on_subproject(self): + domain = get( + Domain, project=self.subproject, domain="test.example.com", canonical=False + ) + + self.assertEqual(domain.canonical, False) + resp = self.client.post( + reverse("projects_domains_edit", args=[self.subproject.slug, domain.pk]), + data={"canonical": True}, + ) + self.assertEqual(resp.status_code, 401) + self.assertEqual(self.subproject.domains.count(), 1) + + domain = self.subproject.domains.first() + self.assertEqual(domain.domain, "test.example.com") + self.assertEqual(domain.canonical, False) + + +@override_settings(RTD_ALLOW_ORGANIZATIONS=True) +@mock.patch.object(DomainMixin, "_is_audit_enabled") +class TestDomainViewsWithOrganizations(TestDomainViews): + def setUp(self): + super().setUp() + self.org = get( + Organization, owners=[self.user], projects=[self.project, self.subproject] + ) From 7fb3fb1c04a3afb8a2e8bc75a04c9fe9bb11efd9 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 31 Mar 2022 13:49:14 -0500 Subject: [PATCH 08/10] Fix tests --- readthedocs/projects/tests/test_domain_views.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/readthedocs/projects/tests/test_domain_views.py b/readthedocs/projects/tests/test_domain_views.py index 1a75a5420f5..8cc9cf9dffa 100644 --- a/readthedocs/projects/tests/test_domain_views.py +++ b/readthedocs/projects/tests/test_domain_views.py @@ -98,10 +98,14 @@ def test_edit_domain_on_subproject(self): @override_settings(RTD_ALLOW_ORGANIZATIONS=True) -@mock.patch.object(DomainMixin, "_is_audit_enabled") class TestDomainViewsWithOrganizations(TestDomainViews): def setUp(self): super().setUp() self.org = get( Organization, owners=[self.user], projects=[self.project, self.subproject] ) + self.patcher = mock.patch.object(DomainMixin, "_is_enabled", return_value=True) + self.patcher.start() + + def tearDown(self): + self.patcher.stop() From 3a70a80d7a1c36e8e65c2383b35ec55f6c544fd9 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 31 Mar 2022 15:19:38 -0500 Subject: [PATCH 09/10] Fix tests Wait for a proper solution to make tests pass on .com --- readthedocs/projects/tests/test_domain_views.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/readthedocs/projects/tests/test_domain_views.py b/readthedocs/projects/tests/test_domain_views.py index 8cc9cf9dffa..f5e0a7bbe23 100644 --- a/readthedocs/projects/tests/test_domain_views.py +++ b/readthedocs/projects/tests/test_domain_views.py @@ -1,5 +1,3 @@ -from unittest import mock - from django.contrib.auth.models import User from django.test import TestCase, override_settings from django.urls import reverse @@ -7,7 +5,6 @@ from readthedocs.organizations.models import Organization from readthedocs.projects.models import Domain, Project -from readthedocs.projects.views.private import DomainMixin @override_settings(RTD_ALLOW_ORGANIZATIONS=False) @@ -104,8 +101,3 @@ def setUp(self): self.org = get( Organization, owners=[self.user], projects=[self.project, self.subproject] ) - self.patcher = mock.patch.object(DomainMixin, "_is_enabled", return_value=True) - self.patcher.start() - - def tearDown(self): - self.patcher.stop() From c0dba16086b752e8b3b9512e5a2956932abedd60 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 11 May 2022 18:57:04 -0500 Subject: [PATCH 10/10] Update readthedocs/templates/projects/domain_list.html Co-authored-by: Anthony --- readthedocs/templates/projects/domain_list.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/templates/projects/domain_list.html b/readthedocs/templates/projects/domain_list.html index 3c304ebccaf..787c7997149 100644 --- a/readthedocs/templates/projects/domain_list.html +++ b/readthedocs/templates/projects/domain_list.html @@ -45,7 +45,7 @@

{% trans "Existing Domains" %}

{% endif %} {% if not superproject %} -

{% trans "Add new Domain" %}

+

{% trans "Add new domain" %}

{% if not enabled %} {% include 'projects/includes/feature_disabled.html' with project=project %}