Skip to content
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

Redirects: limit to 100 per project #11028

Merged
merged 8 commits into from
Jan 15, 2024
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
2 changes: 0 additions & 2 deletions docs/user/api/v3.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1244,8 +1244,6 @@ Redirect create
* ``page`` and ``exact`` types require ``from_url`` and ``to_url``.
* ``clean_url_to_html`` and ``html_to_clean_url`` types do not require ``from_url`` and ``to_url``.

- Forced redirects are not enabled by default,
you can ask for them to be enabled via support.
- Position starts at 0 and is used to order redirects.

**Example response**:
Expand Down
6 changes: 3 additions & 3 deletions docs/user/user-defined-redirects.rst
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,10 @@ users will be redirected to the new URL.
Limitations and observations
~~~~~~~~~~~~~~~~~~~~~~~~~~~~

- |org_brand| users are limited to 100 redirects per project,
and |com_brand| users have a number of redirects limited by their plan.
- By default, redirects only apply on pages that don't exist.
stsewd marked this conversation as resolved.
Show resolved Hide resolved
**Forced redirects** allow you to apply redirects on existing pages,
but incur a small performance penalty, so aren't enabled by default.
You can ask for them to be enabled via support.
**Forced redirects** allow you to apply redirects on existing pages.
- Redirects aren't applied on :doc:`previews of pull requests </pull-requests>`.
You should treat these domains as ephemeral and not rely on them for user-facing content.
- You can redirect to URLs outside Read the Docs,
Expand Down
2 changes: 0 additions & 2 deletions readthedocs/api/v3/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -944,8 +944,6 @@ class Meta:
"position",
"_links",
]
# TODO: allow editing this field for projects that have this feature enabled.
read_only_fields = ["force"]

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
Expand Down
11 changes: 4 additions & 7 deletions readthedocs/projects/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -785,13 +785,10 @@ def __init__(self, *args, **kwargs):
self.fields["enabled"].widget = forms.CheckboxInput()
self.fields["enabled"].empty_value = False

if self.project.has_feature(Feature.ALLOW_FORCED_REDIRECTS):
# Remove the nullable option from the form.
# TODO: remove after migration.
self.fields["force"].widget = forms.CheckboxInput()
self.fields["force"].empty_value = False
else:
self.fields.pop("force")
# Remove the nullable option from the form.
# TODO: remove after migration.
self.fields["force"].widget = forms.CheckboxInput()
self.fields["force"].empty_value = False

def clean_project(self):
return self.project
Expand Down
5 changes: 0 additions & 5 deletions readthedocs/projects/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1951,7 +1951,6 @@ def add_features(sender, **kwargs):
CONDA_APPEND_CORE_REQUIREMENTS = "conda_append_core_requirements"
ALL_VERSIONS_IN_HTML_CONTEXT = "all_versions_in_html_context"
RECORD_404_PAGE_VIEWS = "record_404_page_views"
ALLOW_FORCED_REDIRECTS = "allow_forced_redirects"
DISABLE_PAGEVIEWS = "disable_pageviews"
RESOLVE_PROJECT_FROM_HEADER = "resolve_project_from_header"
USE_PROXIED_APIS_WITH_PREFIX = "use_proxied_apis_with_prefix"
Expand Down Expand Up @@ -2020,10 +2019,6 @@ def add_features(sender, **kwargs):
RECORD_404_PAGE_VIEWS,
_("Proxito: Record 404s page views."),
),
(
ALLOW_FORCED_REDIRECTS,
_("Proxito: Allow forced redirects."),
),
(
DISABLE_PAGEVIEWS,
_("Proxito: Disable all page views"),
Expand Down
62 changes: 62 additions & 0 deletions readthedocs/redirects/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
PAGE_REDIRECT,
)
from readthedocs.redirects.models import Redirect
from readthedocs.subscriptions.constants import TYPE_REDIRECTS_LIMIT
from readthedocs.subscriptions.products import RTDProductFeature


@override_settings(RTD_ALLOW_ORGANIZATIONS=False)
Expand Down Expand Up @@ -224,6 +226,66 @@ def test_redirects_validations(self):
"Only one redirect of type `html_to_clean_url` is allowed per project.",
)

@override_settings(
RTD_DEFAULT_FEATURES=dict(
(
RTDProductFeature(
type=TYPE_REDIRECTS_LIMIT,
value=2,
).to_item(),
)
)
)
def test_redirects_limit(self):
self.assertEqual(self.project.redirects.all().count(), 1)
url = reverse("projects_redirects_create", args=[self.project.slug])
resp = self.client.post(
url,
data={
"redirect_type": EXACT_REDIRECT,
"from_url": "a",
"to_url": "b",
"http_status": 302,
},
)
self.assertEqual(resp.status_code, 302)

resp = self.client.post(
url,
data={
"redirect_type": EXACT_REDIRECT,
"from_url": "c",
"to_url": "d",
"http_status": 302,
},
)
self.assertEqual(resp.status_code, 200)
self.assertContains(
resp,
"This project has reached the limit of 2 redirects.",
)
self.assertEqual(self.project.redirects.all().count(), 2)

# Update works
resp = self.client.post(
reverse(
"projects_redirects_edit", args=[self.project.slug, self.redirect.pk]
),
data={
"redirect_type": PAGE_REDIRECT,
"http_status": 302,
},
)
self.assertEqual(resp.status_code, 302)

# Delete works
resp = self.client.post(
reverse(
"projects_redirects_delete", args=[self.project.slug, self.redirect.pk]
),
)
self.assertEqual(resp.status_code, 302)


@override_settings(RTD_ALLOW_ORGANIZATIONS=True)
class TestViewsWithOrganizations(TestViews):
Expand Down
28 changes: 28 additions & 0 deletions readthedocs/redirects/validators.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
from django.conf import settings
from django.core.exceptions import ValidationError
from django.utils.translation import gettext_lazy as _

from readthedocs.redirects.constants import (
CLEAN_URL_TO_HTML_REDIRECT,
EXACT_REDIRECT,
HTML_TO_CLEAN_URL_REDIRECT,
PAGE_REDIRECT,
)
from readthedocs.subscriptions.constants import TYPE_REDIRECTS_LIMIT
from readthedocs.subscriptions.products import get_feature


def validate_redirect(
Expand All @@ -18,6 +22,10 @@ def validate_redirect(
(used in forms), and in the Django Rest Framework serializer (used in the API).
Since DRF doesn't call the clean method of the model.
"""
# Check for the limit if we are creating a new redirect.
if not pk:
_check_redirects_limit(project, error_class)

if redirect_type in [EXACT_REDIRECT, PAGE_REDIRECT]:
if from_url.endswith("$rest"):
raise error_class("The $rest wildcard has been removed in favor of *.")
Expand All @@ -38,3 +46,23 @@ def validate_redirect(
raise error_class(
f"Only one redirect of type `{redirect_type}` is allowed per project."
)


def _check_redirects_limit(project, error_class):
"""Check if the project has reached the limit on the number of redirects."""
feature = get_feature(project, TYPE_REDIRECTS_LIMIT)
if feature.unlimited:
return

if project.redirects.count() >= feature.value:
msg = _(
f"This project has reached the limit of {feature.value} redirects."
" Consider replacing some of your redirects with a wildcard redirect."
)
if settings.ALLOW_PRIVATE_REPOS:
msg = _(
f"This project has reached the limit of {feature.value} redirects."
" Consider replacing some of your redirects with a wildcard redirect,"
" or upgrade your plan."
)
raise error_class(msg)
2 changes: 2 additions & 0 deletions readthedocs/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ def RTD_DEFAULT_FEATURES(self):
RTDProductFeature(type=constants.TYPE_AUDIT_LOGS, value=self.RTD_AUDITLOGS_DEFAULT_RETENTION_DAYS).to_item(),
# Max number of concurrent builds.
RTDProductFeature(type=constants.TYPE_CONCURRENT_BUILDS, value=self.RTD_MAX_CONCURRENT_BUILDS).to_item(),
# Max number of redirects allowed per project.
RTDProductFeature(type=constants.TYPE_REDIRECTS_LIMIT, value=100).to_item(),
))

# A dictionary of Stripe products mapped to a RTDProduct object.
Expand Down
2 changes: 2 additions & 0 deletions readthedocs/subscriptions/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
TYPE_CUSTOM_URL = "urls"
TYPE_AUDIT_LOGS = "audit-logs"
TYPE_AUDIT_PAGEVIEWS = "audit-pageviews"
TYPE_REDIRECTS_LIMIT = "redirects-limit"

FEATURE_TYPES = (
(TYPE_CNAME, _("Custom domain")),
Expand All @@ -33,4 +34,5 @@
(TYPE_CUSTOM_URL, _("Custom URLs")),
(TYPE_AUDIT_LOGS, _("Audit logs")),
(TYPE_AUDIT_PAGEVIEWS, _("Audit logs for every page view")),
(TYPE_REDIRECTS_LIMIT, _("Redirects limit")),
)