From fdcb280419d8edc77a27287baedbfccde4865a28 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 5 Aug 2024 16:44:36 +0200 Subject: [PATCH 1/6] Addons: prepare Proxito and dashboard to enable them by default Prepare Proxito's code and dashboard to enable addons by default starting on October 7th, as planned: https://about.readthedocs.com/blog/2024/07/addons-by-default/ Note this code is temporary and can be deployed _before_ reaching that particular date. The idea is to remove this code once this behavior becomes the default. Related https://github.com/readthedocs/readthedocs.org/issues/11474 --- readthedocs/projects/forms.py | 14 +++++++++++++- readthedocs/proxito/middleware.py | 12 ++++++++++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index 76b7e27d716..10f42d1a23f 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -1,16 +1,19 @@ """Project forms.""" +import datetime import json from random import choice from re import fullmatch from urllib.parse import urlparse +import pytz from allauth.socialaccount.models import SocialAccount from django import forms from django.conf import settings from django.contrib.auth.models import User from django.db.models import Q from django.urls import reverse +from django.utils import timezone from django.utils.translation import gettext_lazy as _ from readthedocs.builds.constants import INTERNAL @@ -662,14 +665,23 @@ class Meta: def __init__(self, *args, **kwargs): self.project = kwargs.pop("project", None) + + tzinfo = pytz.timezone("America/Los_Angeles") + addons_enabled_by_default = timezone.now() > datetime.datetime( + 2024, 10, 7, 0, 0, 0, tzinfo=tzinfo + ) + addons, created = AddonsConfig.objects.get_or_create(project=self.project) if created: - addons.enabled = False + addons.enabled = addons_enabled_by_default addons.save() kwargs["instance"] = addons super().__init__(*args, **kwargs) + if addons_enabled_by_default: + self.fields.pop("enabled") + def clean(self): if ( self.cleaned_data["flyout_sorting"] == ADDONS_FLYOUT_SORTING_CUSTOM_PATTERN diff --git a/readthedocs/proxito/middleware.py b/readthedocs/proxito/middleware.py index edcf80acbae..ef4efa74a81 100644 --- a/readthedocs/proxito/middleware.py +++ b/readthedocs/proxito/middleware.py @@ -31,7 +31,7 @@ unresolver, ) from readthedocs.core.utils import get_cache_tag -from readthedocs.projects.models import Project +from readthedocs.projects.models import Feature, Project from readthedocs.proxito.cache import add_cache_tags, cache_response, private_response from readthedocs.proxito.redirects import redirect_to_https @@ -289,11 +289,19 @@ def add_hosting_integrations_headers(self, request, response): version_slug = getattr(request, "path_version_slug", "") if project_slug: + # TODO: update this code once DISABLE_SPHINX_MANIPULATION and addons becomes the default + # https://about.readthedocs.com/blog/2024/07/addons-by-default/ + disable_sphinx_manipulation_enabled = Feature.objects.filter( + feature_id=Feature.DISABLE_SPHINX_MANIPULATION, + projects__slug=Project.objects.filter(slug=project_slug).first(), + ).exists() + force_addons = Project.objects.filter( slug=project_slug, addons__enabled=True, ).exists() - if force_addons: + + if force_addons or disable_sphinx_manipulation_enabled: response["X-RTD-Force-Addons"] = "true" return From 235514af2c5ea19657bc5bd1ea8b1d0044182047 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 7 Aug 2024 11:35:30 +0200 Subject: [PATCH 2/6] Update tests --- .../projects/tests/test_build_tasks.py | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/readthedocs/projects/tests/test_build_tasks.py b/readthedocs/projects/tests/test_build_tasks.py index 71dc7097418..916afb42cdd 100644 --- a/readthedocs/projects/tests/test_build_tasks.py +++ b/readthedocs/projects/tests/test_build_tasks.py @@ -535,8 +535,11 @@ def test_successful_build( "builder": mock.ANY, } + # NOTE: `request_history[5]` is a temporal notification that will be removed after October 7th + # https://github.com/readthedocs/readthedocs.org/pull/11514 + # Update build state: installing - assert self.requests_mock.request_history[5].json() == { + assert self.requests_mock.request_history[6].json() == { "id": 1, "state": "installing", "commit": "a1b2c3", @@ -599,7 +602,7 @@ def test_successful_build( }, } # Update build state: building - assert self.requests_mock.request_history[6].json() == { + assert self.requests_mock.request_history[7].json() == { "id": 1, "state": "building", "commit": "a1b2c3", @@ -609,7 +612,7 @@ def test_successful_build( "error": "", } # Update build state: uploading - assert self.requests_mock.request_history[7].json() == { + assert self.requests_mock.request_history[8].json() == { "id": 1, "state": "uploading", "commit": "a1b2c3", @@ -619,9 +622,9 @@ def test_successful_build( "error": "", } # Update version state - assert self.requests_mock.request_history[8]._request.method == "PATCH" - assert self.requests_mock.request_history[8].path == "/api/v2/version/1/" - assert self.requests_mock.request_history[8].json() == { + assert self.requests_mock.request_history[9]._request.method == "PATCH" + assert self.requests_mock.request_history[9].path == "/api/v2/version/1/" + assert self.requests_mock.request_history[9].json() == { "addons": False, "build_data": None, "built": True, @@ -631,11 +634,13 @@ def test_successful_build( "has_htmlzip": True, } # Set project has valid clone - assert self.requests_mock.request_history[9]._request.method == "PATCH" - assert self.requests_mock.request_history[9].path == "/api/v2/project/1/" - assert self.requests_mock.request_history[9].json() == {"has_valid_clone": True} - # Update build state: finished, success and builder + assert self.requests_mock.request_history[10]._request.method == "PATCH" + assert self.requests_mock.request_history[10].path == "/api/v2/project/1/" assert self.requests_mock.request_history[10].json() == { + "has_valid_clone": True + } + # Update build state: finished, success and builder + assert self.requests_mock.request_history[11].json() == { "id": 1, "state": "finished", "commit": "a1b2c3", @@ -647,8 +652,8 @@ def test_successful_build( "error": "", } - assert self.requests_mock.request_history[11]._request.method == "POST" - assert self.requests_mock.request_history[11].path == "/api/v2/revoke/" + assert self.requests_mock.request_history[12]._request.method == "POST" + assert self.requests_mock.request_history[12].path == "/api/v2/revoke/" assert BuildData.objects.all().exists() From cfb6f1f1943764e1fbe7164fef0f7170fdeccdb8 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 7 Aug 2024 12:07:23 +0200 Subject: [PATCH 3/6] Revert "Update tests" This reverts commit 235514af2c5ea19657bc5bd1ea8b1d0044182047. --- .../projects/tests/test_build_tasks.py | 29 ++++++++----------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/readthedocs/projects/tests/test_build_tasks.py b/readthedocs/projects/tests/test_build_tasks.py index 916afb42cdd..71dc7097418 100644 --- a/readthedocs/projects/tests/test_build_tasks.py +++ b/readthedocs/projects/tests/test_build_tasks.py @@ -535,11 +535,8 @@ def test_successful_build( "builder": mock.ANY, } - # NOTE: `request_history[5]` is a temporal notification that will be removed after October 7th - # https://github.com/readthedocs/readthedocs.org/pull/11514 - # Update build state: installing - assert self.requests_mock.request_history[6].json() == { + assert self.requests_mock.request_history[5].json() == { "id": 1, "state": "installing", "commit": "a1b2c3", @@ -602,7 +599,7 @@ def test_successful_build( }, } # Update build state: building - assert self.requests_mock.request_history[7].json() == { + assert self.requests_mock.request_history[6].json() == { "id": 1, "state": "building", "commit": "a1b2c3", @@ -612,7 +609,7 @@ def test_successful_build( "error": "", } # Update build state: uploading - assert self.requests_mock.request_history[8].json() == { + assert self.requests_mock.request_history[7].json() == { "id": 1, "state": "uploading", "commit": "a1b2c3", @@ -622,9 +619,9 @@ def test_successful_build( "error": "", } # Update version state - assert self.requests_mock.request_history[9]._request.method == "PATCH" - assert self.requests_mock.request_history[9].path == "/api/v2/version/1/" - assert self.requests_mock.request_history[9].json() == { + assert self.requests_mock.request_history[8]._request.method == "PATCH" + assert self.requests_mock.request_history[8].path == "/api/v2/version/1/" + assert self.requests_mock.request_history[8].json() == { "addons": False, "build_data": None, "built": True, @@ -634,13 +631,11 @@ def test_successful_build( "has_htmlzip": True, } # Set project has valid clone - assert self.requests_mock.request_history[10]._request.method == "PATCH" - assert self.requests_mock.request_history[10].path == "/api/v2/project/1/" - assert self.requests_mock.request_history[10].json() == { - "has_valid_clone": True - } + assert self.requests_mock.request_history[9]._request.method == "PATCH" + assert self.requests_mock.request_history[9].path == "/api/v2/project/1/" + assert self.requests_mock.request_history[9].json() == {"has_valid_clone": True} # Update build state: finished, success and builder - assert self.requests_mock.request_history[11].json() == { + assert self.requests_mock.request_history[10].json() == { "id": 1, "state": "finished", "commit": "a1b2c3", @@ -652,8 +647,8 @@ def test_successful_build( "error": "", } - assert self.requests_mock.request_history[12]._request.method == "POST" - assert self.requests_mock.request_history[12].path == "/api/v2/revoke/" + assert self.requests_mock.request_history[11]._request.method == "POST" + assert self.requests_mock.request_history[11].path == "/api/v2/revoke/" assert BuildData.objects.all().exists() From feb025754c6a124f52fe1563aa171bef081145fa Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 12 Aug 2024 14:40:06 +0200 Subject: [PATCH 4/6] Allow "for Business" to disable addons completely Keep the ability to let commercial users to disable Addons completely. This means that our Cloudflare working won't even inject them. --- readthedocs/projects/forms.py | 3 +- readthedocs/proxito/middleware.py | 70 ++++++++++++++++++++----------- 2 files changed, 48 insertions(+), 25 deletions(-) diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index 10f42d1a23f..a09e72f35b1 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -679,7 +679,8 @@ def __init__(self, *args, **kwargs): kwargs["instance"] = addons super().__init__(*args, **kwargs) - if addons_enabled_by_default: + # Keep the ability to disable addons completely on Read the Docs for Business + if not settings.RTD_ALLOW_ORGANIZATIONS and addons_enabled_by_default: self.fields.pop("enabled") def clean(self): diff --git a/readthedocs/proxito/middleware.py b/readthedocs/proxito/middleware.py index ef4efa74a81..35e20ee19fa 100644 --- a/readthedocs/proxito/middleware.py +++ b/readthedocs/proxito/middleware.py @@ -5,9 +5,11 @@ Additional processing is done to get the project from the URL in the ``views.py`` as well. """ +import datetime import re from urllib.parse import urlparse +import pytz import structlog from corsheaders.middleware import ( ACCESS_CONTROL_ALLOW_METHODS, @@ -16,7 +18,7 @@ from django.conf import settings from django.core.exceptions import SuspiciousOperation from django.shortcuts import redirect -from django.urls import reverse +from django.urls import reverse, timezone from django.utils.deprecation import MiddlewareMixin from django.utils.encoding import iri_to_uri from django.utils.html import escape @@ -289,31 +291,51 @@ def add_hosting_integrations_headers(self, request, response): version_slug = getattr(request, "path_version_slug", "") if project_slug: - # TODO: update this code once DISABLE_SPHINX_MANIPULATION and addons becomes the default - # https://about.readthedocs.com/blog/2024/07/addons-by-default/ - disable_sphinx_manipulation_enabled = Feature.objects.filter( - feature_id=Feature.DISABLE_SPHINX_MANIPULATION, - projects__slug=Project.objects.filter(slug=project_slug).first(), - ).exists() - - force_addons = Project.objects.filter( - slug=project_slug, - addons__enabled=True, - ).exists() - - if force_addons or disable_sphinx_manipulation_enabled: - response["X-RTD-Force-Addons"] = "true" - return - - if version_slug: - addons = Version.objects.filter( - project__slug=project_slug, - slug=version_slug, - addons=True, + tzinfo = pytz.timezone("America/Los_Angeles") + addons_enabled_by_default = timezone.now() > datetime.datetime( + 2024, + 10, + 7, + 0, + 0, + 0, + tzinfo=tzinfo, + ) + if addons_enabled_by_default: + addons = Project.objects.filter( + slug=project_slug, addons__enabled=True + ).exists() + + if addons: + response["X-RTD-Force-Addons"] = "true" + return + + else: + # TODO: remove "else" code once DISABLE_SPHINX_MANIPULATION and addons becomes the default + # https://about.readthedocs.com/blog/2024/07/addons-by-default/ + disable_sphinx_manipulation_enabled = Feature.objects.filter( + feature_id=Feature.DISABLE_SPHINX_MANIPULATION, + projects__slug=Project.objects.filter(slug=project_slug).first(), ).exists() - if addons: - response["X-RTD-Hosting-Integrations"] = "true" + force_addons = Project.objects.filter( + slug=project_slug, + addons__enabled=True, + ).exists() + + if force_addons or disable_sphinx_manipulation_enabled: + response["X-RTD-Force-Addons"] = "true" + return + + if version_slug: + addons = Version.objects.filter( + project__slug=project_slug, + slug=version_slug, + addons=True, + ).exists() + + if addons: + response["X-RTD-Hosting-Integrations"] = "true" def add_cors_headers(self, request, response): """ From 801cee98d29328b8c0a833118b2248cc31f4601c Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 12 Aug 2024 15:34:38 +0200 Subject: [PATCH 5/6] Create `AddonsConfig` on project import/add --- readthedocs/projects/views/private.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index d72719c92b0..7882866e853 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -73,6 +73,7 @@ WebHookForm, ) from readthedocs.projects.models import ( + AddonsConfig, Domain, EmailHook, EnvironmentVariable, @@ -361,6 +362,9 @@ def done(self, form_list, **kwargs): # attributes directly from other forms project = basics_form.save() + # Create an AddonsConfig object for this project. + AddonsConfig.objects.get_or_create(project=project) + self.finish_import_project(self.request, project) return HttpResponseRedirect( From 79e7ebdc408d470ea5017e0df4ff5fe80f2986a7 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 12 Aug 2024 15:36:29 +0200 Subject: [PATCH 6/6] Typo --- readthedocs/proxito/middleware.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/readthedocs/proxito/middleware.py b/readthedocs/proxito/middleware.py index 35e20ee19fa..03928ffc20a 100644 --- a/readthedocs/proxito/middleware.py +++ b/readthedocs/proxito/middleware.py @@ -18,7 +18,8 @@ from django.conf import settings from django.core.exceptions import SuspiciousOperation from django.shortcuts import redirect -from django.urls import reverse, timezone +from django.urls import reverse +from django.utils import timezone from django.utils.deprecation import MiddlewareMixin from django.utils.encoding import iri_to_uri from django.utils.html import escape