Skip to content

Custom domains: don't allow adding a custom domain on subprojects #8953

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 14 commits into from
May 12, 2022
9 changes: 2 additions & 7 deletions docs/user/subprojects.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
------

Expand Down
7 changes: 7 additions & 0 deletions readthedocs/projects/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,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
Expand Down
103 changes: 103 additions & 0 deletions readthedocs/projects/tests/test_domain_views.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
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


@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)
class TestDomainViewsWithOrganizations(TestDomainViews):
def setUp(self):
super().setUp()
self.org = get(
Organization, owners=[self.user], projects=[self.project, self.subproject]
)
14 changes: 8 additions & 6 deletions readthedocs/projects/views/base.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -72,7 +70,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):
Expand All @@ -92,7 +92,9 @@ 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
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)

Expand Down
9 changes: 3 additions & 6 deletions readthedocs/projects/views/private.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -786,7 +783,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)

Expand All @@ -810,7 +807,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)

Expand Down
2 changes: 1 addition & 1 deletion readthedocs/templates/projects/domain_form.html
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
{% endif %}
<form method="post" action="{{ action_url }}">{% csrf_token %}
{{ form.as_p }}
<input type="submit" value="{% trans "Save Domain" %}" {% if not enabled %}disabled{% endif %}>
<input type="submit" value="{% trans "Save Domain" %}" {% if not enabled or superproject %}disabled{% endif %}>
{% if domain.domainssl %}
<p class="help-block">{% trans 'Saving the domain will revalidate the SSL certificate' %}</p>
{% endif %}
Expand Down
45 changes: 29 additions & 16 deletions readthedocs/templates/projects/domain_list.html
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,22 @@
{% block project_edit_content_header %}{% trans "Domains" %}{% endblock %}

{% block project_edit_content %}
<p class="help_text">
{% 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 }}". <a href="{{ docs_url }}">Learn more</a>.
{% endblocktrans %}
</p>
{% if superproject %}
{% url "projects_detail" superproject.slug as superproject_url %}
<p>
{% blocktrans trimmed with superproject=superproject.name superproject_url=superproject_url domain=project.subdomain %}
This project is a subproject of <a href="{{ superproject_url }}">{{ superproject }}</a>,
its documentation will always be served from the <code>{{ domain }}</code> domain.
<a href="https://docs.readthedocs.io/page/subprojects.html#custom-domain-on-subprojects">Learn more</a>.
{% endblocktrans %}
</p>
{% else %}
<p class="help_text">
{% 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 }}". <a href="{{ docs_url }}">Learn more</a>.
{% endblocktrans %}
Comment on lines +25 to +28
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, not sure if we should show both messages when this is a subproject.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense to just show one block. We have discussed on another issue that a better UX would be to show the form as disabled, but we can come back to this as well. There are maybe a few patterns to figure out there.

</p>
{% endif %}

{% if object_list %}
<h3> {% trans "Existing Domains" %} </h3>
Expand All @@ -33,16 +44,18 @@ <h3> {% trans "Existing Domains" %} </h3>
</p>
{% endif %}

<h3> {% trans "Add new Domain" %} </h3>

{% if not enabled %}
{% include 'projects/includes/feature_disabled.html' with project=project %}
{% else %}
<form method="post" action="{% url 'projects_domains_create' project.slug %}">{% csrf_token %}
{{ form.as_p }}
<p>
<input style="display: inline;" type="submit" value="{% trans "Add" %}">
</p>
</form>
{% if not superproject %}
<h3>{% trans "Add new domain" %}</h3>

{% if not enabled %}
{% include 'projects/includes/feature_disabled.html' with project=project %}
{% else %}
<form method="post" action="{% url 'projects_domains_create' project.slug %}">{% csrf_token %}
{{ form.as_p }}
<p>
<input style="display: inline;" type="submit" value="{% trans "Add" %}">
</p>
</form>
{% endif %}
{% endif %}
{% endblock %}
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@

{% if superproject %}
<p>
{% 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 %}
</p>

<a href="{% url 'projects_subprojects' project_slug=superproject.parent.slug %}">
{% blocktrans trimmed with project=superproject.parent.name %}
<a href="{% url 'projects_subprojects' project_slug=superproject.slug %}">
{% blocktrans trimmed with project=superproject.name %}
View subprojects of {{ project }}
{% endblocktrans %}
</a>
Expand Down