From f29401f276ebe06384379ed19b16aa835afa5b11 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 14 Feb 2022 18:52:13 -0500 Subject: [PATCH 01/19] Unify feature check for organization/project --- readthedocs/organizations/views/private.py | 27 +++++---- readthedocs/projects/querysets.py | 9 ++- readthedocs/projects/views/private.py | 68 ++++++++++------------ readthedocs/proxito/tests/test_full.py | 1 + readthedocs/settings/base.py | 1 + readthedocs/subscriptions/managers.py | 26 +++++++++ 6 files changed, 83 insertions(+), 49 deletions(-) diff --git a/readthedocs/organizations/views/private.py b/readthedocs/organizations/views/private.py index fa8cc2dada7..ad5ca4084f8 100644 --- a/readthedocs/organizations/views/private.py +++ b/readthedocs/organizations/views/private.py @@ -12,7 +12,6 @@ from readthedocs.audit.models import AuditLog from readthedocs.core.history import UpdateChangeReasonPostView from readthedocs.core.mixins import PrivateViewMixin -from readthedocs.core.utils.extend import SettingsOverrideObject from readthedocs.organizations.forms import ( OrganizationSignupForm, OrganizationTeamProjectForm, @@ -26,6 +25,7 @@ OrganizationView, ) from readthedocs.projects.utils import get_csv_file +from readthedocs.subscriptions.models import PlanFeature # Organization views @@ -170,12 +170,13 @@ def post(self, request, *args, **kwargs): return resp -class OrganizationSecurityLogBase(PrivateViewMixin, OrganizationMixin, ListView): +class OrganizationSecurityLog(PrivateViewMixin, OrganizationMixin, ListView): """Display security logs related to this organization.""" model = AuditLog template_name = 'organizations/security_log.html' + feature_type = PlanFeature.TYPE_AUDIT_LOGS def get(self, request, *args, **kwargs): download_data = request.GET.get('download', False) @@ -273,14 +274,16 @@ def get_queryset(self): ) return self.filter.qs - def _get_retention_days_limit(self, organization): # noqa - """From how many days we need to show data for this project?""" - return settings.RTD_AUDITLOGS_DEFAULT_RETENTION_DAYS - - def _is_enabled(self, organization): # noqa - """Should we show audit logs for this organization?""" - return True - + def _is_enabled(self, organization): + return PlanFeature.objects.has_feature( + organization, + type=self.feature_type, + ) -class OrganizationSecurityLog(SettingsOverrideObject): - _default_class = OrganizationSecurityLogBase + def _get_retention_days_limit(self, organization): + """From how many days we need to show data for this organization?""" + return PlanFeature.objects.get_feature_value( + organization, + type=self.feature_type, + default=settings.RTD_AUDITLOGS_DEFAULT_RETENTION_DAYS, + ) diff --git a/readthedocs/projects/querysets.py b/readthedocs/projects/querysets.py index 986326eafa8..6cad9b89c2e 100644 --- a/readthedocs/projects/querysets.py +++ b/readthedocs/projects/querysets.py @@ -97,6 +97,7 @@ def max_concurrent_builds(self, project): - project - organization + - plan - default setting :param project: project to be checked @@ -105,6 +106,8 @@ def max_concurrent_builds(self, project): :returns: number of max concurrent builds for the project :rtype: int """ + from readthedocs.subscriptions.models import PlanFeature + max_concurrent_organization = None organization = project.organizations.first() if organization: @@ -113,7 +116,11 @@ def max_concurrent_builds(self, project): return ( project.max_concurrent_builds or max_concurrent_organization or - settings.RTD_MAX_CONCURRENT_BUILDS + PlanFeature.objects.get_feature_value( + project, + type=PlanFeature.TYPE_CONCURRENT_BUILDS, + default=settings.RTD_MAX_CONCURRENT_BUILDS, + ) ) def prefetch_latest_build(self): diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index 8f0c4d2105f..a40b661cca8 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 @@ -40,7 +39,6 @@ ) from readthedocs.core.history import UpdateChangeReasonPostView from readthedocs.core.mixins import ListViewWithForm, PrivateViewMixin -from readthedocs.core.utils.extend import SettingsOverrideObject from readthedocs.integrations.models import HttpExchange, Integration from readthedocs.oauth.services import registry from readthedocs.oauth.tasks import attach_webhook @@ -79,6 +77,8 @@ ProjectRelationListMixin, ) from readthedocs.search.models import SearchQuery +from readthedocs.subscriptions.models import PlanFeature + log = structlog.get_logger(__name__) @@ -747,6 +747,7 @@ class DomainMixin(ProjectAdminMixin, PrivateViewMixin): model = Domain form_class = DomainForm lookup_url_kwarg = 'domain_pk' + feature_type = PlanFeature.TYPE_CNAME def get_success_url(self): return reverse('projects_domains', args=[self.get_project().slug]) @@ -758,11 +759,13 @@ def get_context_data(self, **kwargs): return context def _is_enabled(self, project): - """Should we allow custom domains for this project?""" - return True + return PlanFeature.objects.has_feature( + project, + type=self.feature_type, + ) -class DomainListBase(DomainMixin, ListViewWithForm): +class DomainList(DomainMixin, ListViewWithForm): def get_context_data(self, **kwargs): ctx = super().get_context_data(**kwargs) @@ -777,12 +780,7 @@ def get_context_data(self, **kwargs): return ctx -class DomainList(SettingsOverrideObject): - - _default_class = DomainListBase - - -class DomainCreateBase(DomainMixin, CreateView): +class DomainCreate(DomainMixin, CreateView): def post(self, request, *args, **kwargs): project = self.get_project() @@ -801,12 +799,7 @@ def get_success_url(self): ) -class DomainCreate(SettingsOverrideObject): - - _default_class = DomainCreateBase - - -class DomainUpdateBase(DomainMixin, UpdateView): +class DomainUpdate(DomainMixin, UpdateView): def post(self, request, *args, **kwargs): project = self.get_project() @@ -815,11 +808,6 @@ def post(self, request, *args, **kwargs): return HttpResponse('Action not allowed', status=401) -class DomainUpdate(SettingsOverrideObject): - - _default_class = DomainUpdateBase - - class DomainDelete(DomainMixin, DeleteView): pass @@ -1062,10 +1050,11 @@ class RegexAutomationRuleUpdate(RegexAutomationRuleMixin, UpdateView): pass -class SearchAnalyticsBase(ProjectAdminMixin, PrivateViewMixin, TemplateView): +class SearchAnalytics(ProjectAdminMixin, PrivateViewMixin, TemplateView): template_name = 'projects/projects_search_analytics.html' http_method_names = ['get'] + feature_type = PlanFeature.TYPE_SEARCH_ANALYTICS def get(self, request, *args, **kwargs): download_data = request.GET.get('download', False) @@ -1149,21 +1138,25 @@ def _get_csv_data(self): def _get_retention_days_limit(self, project): """From how many days we need to show data for this project?""" - return settings.RTD_ANALYTICS_DEFAULT_RETENTION_DAYS + return PlanFeature.objects.get_feature_value( + project, + type=self.feature_type, + default=settings.RTD_ANALYTICS_DEFAULT_RETENTION_DAYS, + ) def _is_enabled(self, project): """Should we show search analytics for this project?""" - return True - - -class SearchAnalytics(SettingsOverrideObject): - _default_class = SearchAnalyticsBase + return PlanFeature.objects.has_feature( + project, + type=self.feature_type, + ) -class TrafficAnalyticsViewBase(ProjectAdminMixin, PrivateViewMixin, TemplateView): +class TrafficAnalyticsView(ProjectAdminMixin, PrivateViewMixin, TemplateView): template_name = 'projects/project_traffic_analytics.html' http_method_names = ['get'] + feature_type = PlanFeature.TYPE_PAGEVIEW_ANALYTICS def get(self, request, *args, **kwargs): download_data = request.GET.get('download', False) @@ -1239,12 +1232,15 @@ def _get_csv_data(self): def _get_retention_days_limit(self, project): """From how many days we need to show data for this project?""" - return settings.RTD_ANALYTICS_DEFAULT_RETENTION_DAYS + return PlanFeature.objects.get_feature_value( + project, + type=self.feature_type, + default=settings.RTD_ANALYTICS_DEFAULT_RETENTION_DAYS, + ) def _is_enabled(self, project): """Should we show traffic analytics for this project?""" - return True - - -class TrafficAnalyticsView(SettingsOverrideObject): - _default_class = TrafficAnalyticsViewBase + return PlanFeature.objects.has_feature( + project, + type=self.feature_type, + ) diff --git a/readthedocs/proxito/tests/test_full.py b/readthedocs/proxito/tests/test_full.py index 49e06dbe020..e104eb3cee6 100644 --- a/readthedocs/proxito/tests/test_full.py +++ b/readthedocs/proxito/tests/test_full.py @@ -985,6 +985,7 @@ def test_sitemap_all_private_versions(self): ALLOW_PRIVATE_REPOS=True, PUBLIC_DOMAIN='dev.readthedocs.io', PUBLIC_DOMAIN_USES_HTTPS=True, + RTD_ALL_FEATURES_ENABLED=True, ) class TestCDNCache(BaseDocServing): diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index 9eacf3cab61..f6669b8658c 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -683,6 +683,7 @@ def DOCKER_LIMITS(self): DEFAULT_PRIVACY_LEVEL = 'public' DEFAULT_VERSION_PRIVACY_LEVEL = 'public' ALLOW_ADMIN = True + RTD_ALL_FEATURES_ENABLED = True # Organization settings RTD_ALLOW_ORGANIZATIONS = False diff --git a/readthedocs/subscriptions/managers.py b/readthedocs/subscriptions/managers.py index 61cc54665ae..4515a70e285 100644 --- a/readthedocs/subscriptions/managers.py +++ b/readthedocs/subscriptions/managers.py @@ -218,3 +218,29 @@ def get_feature(self, obj, type): plan__subscriptions__organization=organization, ) return feature.first() + + # pylint: disable=redefined-builtin + def get_feature_value(self, obj, type, default=None): + """ + Get the value of the given feature. + + Use this function instead of ``get_feature().value`` + when you need to respect the ``RTD_ALL_FEATURES_ENABLED`` setting. + """ + if not settings.RTD_ALL_FEATURES_ENABLED: + feature = self.get_feature(obj, type) + if feature: + return feature.value + return default + + # pylint: disable=redefined-builtin + def has_feature(self, obj, type): + """ + Get the value of the given feature. + + Use this function instead of ``bool(get_feature())`` + when you need to respect the ``RTD_ALL_FEATURES_ENABLED`` setting. + """ + if settings.RTD_ALL_FEATURES_ENABLED: + return True + return self.get_feature(obj, type) is not None From 8e339c0b52b4bad21bc4de2b32107913cf8eef00 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 16 Feb 2022 14:07:10 -0500 Subject: [PATCH 02/19] Explicit tests --- readthedocs/builds/tests/test_build_queryset.py | 13 +++++++------ readthedocs/proxito/tests/base.py | 17 +++++++++++++++-- readthedocs/proxito/tests/test_redirects.py | 1 + readthedocs/rtd_tests/tests/test_views.py | 5 ++++- 4 files changed, 27 insertions(+), 9 deletions(-) diff --git a/readthedocs/builds/tests/test_build_queryset.py b/readthedocs/builds/tests/test_build_queryset.py index 67d4e616a8b..7eb435a46de 100644 --- a/readthedocs/builds/tests/test_build_queryset.py +++ b/readthedocs/builds/tests/test_build_queryset.py @@ -1,17 +1,18 @@ -import pytest - import django_dynamic_fixture as fixture -from django.conf import settings +import pytest -from readthedocs.builds.querysets import BuildQuerySet -from readthedocs.builds.models import Build, Version +from readthedocs.builds.models import Build from readthedocs.organizations.models import Organization -from readthedocs.projects.models import Project, Feature +from readthedocs.projects.models import Project @pytest.mark.django_db class TestBuildQuerySet: + @pytest.fixture(autouse=True) + def setup_method(self, settings): + settings.RTD_ALL_FEATURES_ENABLED = True + def test_concurrent_builds(self): project = fixture.get( Project, diff --git a/readthedocs/proxito/tests/base.py b/readthedocs/proxito/tests/base.py index 4a8adb499ea..19b7af74ed6 100644 --- a/readthedocs/proxito/tests/base.py +++ b/readthedocs/proxito/tests/base.py @@ -1,5 +1,6 @@ # Copied from .org +from readthedocs.projects.constants import SSL_STATUS_VALID import django_dynamic_fixture as fixture import pytest from django.conf import settings @@ -78,5 +79,17 @@ def setUp(self): self.project.add_subproject(self.subproject_alias, alias='this-is-an-alias') # These can be set to canonical as needed in specific tests - self.domain = fixture.get(Domain, project=self.project, domain='docs1.example.com', https=True) - self.domain2 = fixture.get(Domain, project=self.project, domain='docs2.example.com', https=True) + self.domain = fixture.get( + Domain, + project=self.project, + domain='docs1.example.com', + https=True, + ssl_status=SSL_STATUS_VALID, + ) + self.domain2 = fixture.get( + Domain, + project=self.project, + domain='docs2.example.com', + https=True, + ssl_status=SSL_STATUS_VALID, + ) diff --git a/readthedocs/proxito/tests/test_redirects.py b/readthedocs/proxito/tests/test_redirects.py index d5e3cbec81c..1c1101e6b2b 100644 --- a/readthedocs/proxito/tests/test_redirects.py +++ b/readthedocs/proxito/tests/test_redirects.py @@ -9,6 +9,7 @@ @override_settings( PUBLIC_DOMAIN='dev.readthedocs.io', PUBLIC_DOMAIN_USES_HTTPS=True, + RTD_ALL_FEATURES_ENABLED=True, ) class RedirectTests(BaseDocServing): diff --git a/readthedocs/rtd_tests/tests/test_views.py b/readthedocs/rtd_tests/tests/test_views.py index 4e4b202df67..e4f3301a18e 100644 --- a/readthedocs/rtd_tests/tests/test_views.py +++ b/readthedocs/rtd_tests/tests/test_views.py @@ -4,7 +4,7 @@ from django.contrib.auth.models import User from django.core.cache import cache -from django.test import TestCase +from django.test import TestCase, override_settings from django.urls import reverse from django.utils import timezone from django_dynamic_fixture import get, new @@ -331,6 +331,9 @@ def test_rebuild_invalid_specific_commit(self, mock): self.assertEqual(r.status_code, 302) +@override_settings( + RTD_ALL_FEATURES_ENABLED=True, +) class TestSearchAnalyticsView(TestCase): """Tests for search analytics page.""" From b58e282431495316671f857ec226d5a19c940b5b Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 16 Feb 2022 14:15:43 -0500 Subject: [PATCH 03/19] swap --- readthedocs/organizations/views/private.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/readthedocs/organizations/views/private.py b/readthedocs/organizations/views/private.py index ad5ca4084f8..9f042aa0b48 100644 --- a/readthedocs/organizations/views/private.py +++ b/readthedocs/organizations/views/private.py @@ -274,12 +274,6 @@ def get_queryset(self): ) return self.filter.qs - def _is_enabled(self, organization): - return PlanFeature.objects.has_feature( - organization, - type=self.feature_type, - ) - def _get_retention_days_limit(self, organization): """From how many days we need to show data for this organization?""" return PlanFeature.objects.get_feature_value( @@ -287,3 +281,9 @@ def _get_retention_days_limit(self, organization): type=self.feature_type, default=settings.RTD_AUDITLOGS_DEFAULT_RETENTION_DAYS, ) + + def _is_enabled(self, organization): + return PlanFeature.objects.has_feature( + organization, + type=self.feature_type, + ) From 274a377d5061df1cf8a457cec3f33d4f23b468f5 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 16 Feb 2022 14:20:00 -0500 Subject: [PATCH 04/19] Another test --- readthedocs/organizations/tests/test_views.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/readthedocs/organizations/tests/test_views.py b/readthedocs/organizations/tests/test_views.py index 1ddefbd0866..092dd5cc7f6 100644 --- a/readthedocs/organizations/tests/test_views.py +++ b/readthedocs/organizations/tests/test_views.py @@ -141,7 +141,10 @@ def test_add_owner(self): self.assertTrue(user_b in self.organization.owners.all()) -@override_settings(RTD_ALLOW_ORGANIZATIONS=True) +@override_settings( + RTD_ALLOW_ORGANIZATIONS=True, + RTD_ALL_FEATURES_ENABLED=True, +) class OrganizationSecurityLogTests(TestCase): def setUp(self): From 277a5130f18617892ac8c5bd28133679a8eea30d Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 8 Mar 2022 19:48:50 -0500 Subject: [PATCH 05/19] Updates from review --- readthedocs/organizations/views/private.py | 6 +++--- readthedocs/settings/base.py | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/readthedocs/organizations/views/private.py b/readthedocs/organizations/views/private.py index 9f042aa0b48..53034e3a08d 100644 --- a/readthedocs/organizations/views/private.py +++ b/readthedocs/organizations/views/private.py @@ -221,7 +221,7 @@ def _get_csv_data(self): def get_context_data(self, **kwargs): organization = self.get_organization() context = super().get_context_data(**kwargs) - context['enabled'] = self._is_enabled(organization) + context['enabled'] = self._is_feature_enabled(organization) context['days_limit'] = self._get_retention_days_limit(organization) context['filter'] = self.filter context['AuditLog'] = AuditLog @@ -242,7 +242,7 @@ def _get_start_date(self): def _get_queryset(self): """Return the queryset without filters.""" organization = self.get_organization() - if not self._is_enabled(organization): + if not self._is_feature_enabled(organization): return AuditLog.objects.none() start_date = self._get_start_date() queryset = AuditLog.objects.filter( @@ -282,7 +282,7 @@ def _get_retention_days_limit(self, organization): default=settings.RTD_AUDITLOGS_DEFAULT_RETENTION_DAYS, ) - def _is_enabled(self, organization): + def _is_feature_enabled(self, organization): return PlanFeature.objects.has_feature( organization, type=self.feature_type, diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index f6669b8658c..9e067a143c7 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -683,6 +683,8 @@ def DOCKER_LIMITS(self): DEFAULT_PRIVACY_LEVEL = 'public' DEFAULT_VERSION_PRIVACY_LEVEL = 'public' ALLOW_ADMIN = True + # Enable all features by default, + # otherwise the organization subscription is checked. RTD_ALL_FEATURES_ENABLED = True # Organization settings From 60f001668fffede68c8533e3f15f06a8e3700247 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 8 Mar 2022 19:55:19 -0500 Subject: [PATCH 06/19] Black --- readthedocs/builds/tests/test_build_queryset.py | 1 - readthedocs/organizations/tests/test_views.py | 7 +------ readthedocs/organizations/views/private.py | 8 ++++---- readthedocs/projects/querysets.py | 6 +++--- readthedocs/projects/views/private.py | 1 - readthedocs/proxito/tests/base.py | 7 +++---- 6 files changed, 11 insertions(+), 19 deletions(-) diff --git a/readthedocs/builds/tests/test_build_queryset.py b/readthedocs/builds/tests/test_build_queryset.py index 7eb435a46de..d2daceaee27 100644 --- a/readthedocs/builds/tests/test_build_queryset.py +++ b/readthedocs/builds/tests/test_build_queryset.py @@ -8,7 +8,6 @@ @pytest.mark.django_db class TestBuildQuerySet: - @pytest.fixture(autouse=True) def setup_method(self, settings): settings.RTD_ALL_FEATURES_ENABLED = True diff --git a/readthedocs/organizations/tests/test_views.py b/readthedocs/organizations/tests/test_views.py index 092dd5cc7f6..86262f41b75 100644 --- a/readthedocs/organizations/tests/test_views.py +++ b/readthedocs/organizations/tests/test_views.py @@ -13,12 +13,7 @@ from readthedocs.audit.models import AuditLog from readthedocs.core.utils import slugify -from readthedocs.organizations.models import ( - Organization, - Team, - TeamInvite, - TeamMember, -) +from readthedocs.organizations.models import Organization, Team, TeamInvite, TeamMember from readthedocs.organizations.views import public as public_views from readthedocs.projects.models import Project from readthedocs.rtd_tests.base import RequestFactoryTestMixin diff --git a/readthedocs/organizations/views/private.py b/readthedocs/organizations/views/private.py index 53034e3a08d..5e861c8beab 100644 --- a/readthedocs/organizations/views/private.py +++ b/readthedocs/organizations/views/private.py @@ -221,10 +221,10 @@ def _get_csv_data(self): def get_context_data(self, **kwargs): organization = self.get_organization() context = super().get_context_data(**kwargs) - context['enabled'] = self._is_feature_enabled(organization) - context['days_limit'] = self._get_retention_days_limit(organization) - context['filter'] = self.filter - context['AuditLog'] = AuditLog + context["enabled"] = self._is_feature_enabled(organization) + context["days_limit"] = self._get_retention_days_limit(organization) + context["filter"] = self.filter + context["AuditLog"] = AuditLog return context def _get_start_date(self): diff --git a/readthedocs/projects/querysets.py b/readthedocs/projects/querysets.py index 6cad9b89c2e..ed2d6776eac 100644 --- a/readthedocs/projects/querysets.py +++ b/readthedocs/projects/querysets.py @@ -114,9 +114,9 @@ def max_concurrent_builds(self, project): max_concurrent_organization = organization.max_concurrent_builds return ( - project.max_concurrent_builds or - max_concurrent_organization or - PlanFeature.objects.get_feature_value( + project.max_concurrent_builds + or max_concurrent_organization + or PlanFeature.objects.get_feature_value( project, type=PlanFeature.TYPE_CONCURRENT_BUILDS, default=settings.RTD_MAX_CONCURRENT_BUILDS, diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index a40b661cca8..34d82e59bac 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -79,7 +79,6 @@ from readthedocs.search.models import SearchQuery from readthedocs.subscriptions.models import PlanFeature - log = structlog.get_logger(__name__) diff --git a/readthedocs/proxito/tests/base.py b/readthedocs/proxito/tests/base.py index 19b7af74ed6..bbf7362e311 100644 --- a/readthedocs/proxito/tests/base.py +++ b/readthedocs/proxito/tests/base.py @@ -1,6 +1,5 @@ # Copied from .org -from readthedocs.projects.constants import SSL_STATUS_VALID import django_dynamic_fixture as fixture import pytest from django.conf import settings @@ -8,7 +7,7 @@ from django.core.files.storage import get_storage_class from django.test import TestCase -from readthedocs.projects.constants import PUBLIC +from readthedocs.projects.constants import PUBLIC, SSL_STATUS_VALID from readthedocs.projects.models import Domain, Project from readthedocs.proxito.views import serve @@ -82,14 +81,14 @@ def setUp(self): self.domain = fixture.get( Domain, project=self.project, - domain='docs1.example.com', + domain="docs1.example.com", https=True, ssl_status=SSL_STATUS_VALID, ) self.domain2 = fixture.get( Domain, project=self.project, - domain='docs2.example.com', + domain="docs2.example.com", https=True, ssl_status=SSL_STATUS_VALID, ) From 8911ded1db00a1ef71472d61cad66e1eb4ac3bb3 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 22 Nov 2022 17:07:13 -0500 Subject: [PATCH 07/19] Test with alternate implementation --- .../builds/tests/test_build_queryset.py | 3 -- readthedocs/core/mixins.py | 3 +- readthedocs/organizations/tests/test_views.py | 5 +-- readthedocs/organizations/views/private.py | 4 +- readthedocs/projects/querysets.py | 4 +- .../projects/tests/test_domain_views.py | 3 +- readthedocs/projects/views/private.py | 9 ++--- readthedocs/proxito/tests/test_full.py | 4 +- readthedocs/proxito/tests/test_redirects.py | 1 - readthedocs/rtd_tests/tests/test_views.py | 3 -- readthedocs/settings/base.py | 25 ++++++++++-- readthedocs/subscriptions/constants.py | 35 +++++++++++++++++ readthedocs/subscriptions/managers.py | 18 ++++----- readthedocs/subscriptions/models.py | 38 +------------------ 14 files changed, 81 insertions(+), 74 deletions(-) diff --git a/readthedocs/builds/tests/test_build_queryset.py b/readthedocs/builds/tests/test_build_queryset.py index 09c57c4b206..72d84e5cf3d 100644 --- a/readthedocs/builds/tests/test_build_queryset.py +++ b/readthedocs/builds/tests/test_build_queryset.py @@ -8,9 +8,6 @@ @pytest.mark.django_db class TestBuildQuerySet: - @pytest.fixture(autouse=True) - def setup_method(self, settings): - settings.RTD_ALL_FEATURES_ENABLED = True def test_concurrent_builds(self): project = fixture.get( diff --git a/readthedocs/core/mixins.py b/readthedocs/core/mixins.py index d4a87db7a3a..e8a8ca6c271 100644 --- a/readthedocs/core/mixins.py +++ b/readthedocs/core/mixins.py @@ -7,6 +7,7 @@ from readthedocs.projects.models import Feature from readthedocs.subscriptions.models import PlanFeature +from readthedocs.subscriptions.constants import TYPE_CDN class ListViewWithForm(ListView): @@ -67,7 +68,7 @@ def can_be_cached(self, request): def _is_cache_enabled(self, project): """Helper function to check if CDN is enabled for a project.""" plan_has_cdn = PlanFeature.objects.get_feature( - obj=project, type=PlanFeature.TYPE_CDN + obj=project, type=TYPE_CDN ) return settings.ALLOW_PRIVATE_REPOS and ( plan_has_cdn or project.has_feature(Feature.CDN_ENABLED) diff --git a/readthedocs/organizations/tests/test_views.py b/readthedocs/organizations/tests/test_views.py index 60bbb2c59b2..1de73f450fd 100644 --- a/readthedocs/organizations/tests/test_views.py +++ b/readthedocs/organizations/tests/test_views.py @@ -147,10 +147,7 @@ def test_add_owner(self): self.assertNotIn(user_b, self.organization.owners.all()) -@override_settings( - RTD_ALLOW_ORGANIZATIONS=True, - RTD_ALL_FEATURES_ENABLED=True, -) +@override_settings(RTD_ALLOW_ORGANIZATIONS=True) class OrganizationSecurityLogTests(TestCase): def setUp(self): diff --git a/readthedocs/organizations/views/private.py b/readthedocs/organizations/views/private.py index 6459d0fc95e..9a1ebe95300 100644 --- a/readthedocs/organizations/views/private.py +++ b/readthedocs/organizations/views/private.py @@ -28,6 +28,7 @@ ) from readthedocs.projects.utils import get_csv_file from readthedocs.subscriptions.models import PlanFeature +from readthedocs.subscriptions.constants import TYPE_AUDIT_LOGS # Organization views @@ -189,7 +190,7 @@ class OrganizationSecurityLog(PrivateViewMixin, OrganizationMixin, ListView): model = AuditLog template_name = 'organizations/security_log.html' - feature_type = PlanFeature.TYPE_AUDIT_LOGS + feature_type = TYPE_AUDIT_LOGS def get(self, request, *args, **kwargs): download_data = request.GET.get('download', False) @@ -292,7 +293,6 @@ def _get_retention_days_limit(self, organization): return PlanFeature.objects.get_feature_value( organization, type=self.feature_type, - default=settings.RTD_AUDITLOGS_DEFAULT_RETENTION_DAYS, ) def _is_feature_enabled(self, organization): diff --git a/readthedocs/projects/querysets.py b/readthedocs/projects/querysets.py index 69e124a53a6..03c5751e26c 100644 --- a/readthedocs/projects/querysets.py +++ b/readthedocs/projects/querysets.py @@ -106,6 +106,7 @@ def max_concurrent_builds(self, project): :rtype: int """ from readthedocs.subscriptions.models import PlanFeature + from readthedocs.subscriptions.constants import TYPE_CONCURRENT_BUILDS max_concurrent_organization = None organization = project.organizations.first() @@ -117,8 +118,7 @@ def max_concurrent_builds(self, project): or max_concurrent_organization or PlanFeature.objects.get_feature_value( project, - type=PlanFeature.TYPE_CONCURRENT_BUILDS, - default=settings.RTD_MAX_CONCURRENT_BUILDS, + type=TYPE_CONCURRENT_BUILDS, ) ) diff --git a/readthedocs/projects/tests/test_domain_views.py b/readthedocs/projects/tests/test_domain_views.py index fd3cd4faaa3..9341aa72d49 100644 --- a/readthedocs/projects/tests/test_domain_views.py +++ b/readthedocs/projects/tests/test_domain_views.py @@ -6,6 +6,7 @@ from readthedocs.organizations.models import Organization from readthedocs.projects.models import Domain, Project from readthedocs.subscriptions.models import Plan, PlanFeature, Subscription +from readthedocs.subscriptions.constants import TYPE_CNAME @override_settings(RTD_ALLOW_ORGANIZATIONS=False) @@ -114,5 +115,5 @@ def setUp(self): self.feature = get( PlanFeature, plan=self.plan, - feature_type=PlanFeature.TYPE_CNAME, + feature_type=TYPE_CNAME, ) diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index 166c5d4ec07..be538f8d519 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -80,6 +80,7 @@ ) from readthedocs.search.models import SearchQuery from readthedocs.subscriptions.models import PlanFeature +from readthedocs.subscriptions.constants import TYPE_PAGEVIEW_ANALYTICS, TYPE_CNAME, TYPE_SEARCH_ANALYTICS log = structlog.get_logger(__name__) @@ -752,7 +753,7 @@ class DomainMixin(ProjectAdminMixin, PrivateViewMixin): model = Domain form_class = DomainForm lookup_url_kwarg = 'domain_pk' - feature_type = PlanFeature.TYPE_CNAME + feature_type = TYPE_CNAME def get_success_url(self): return reverse('projects_domains', args=[self.get_project().slug]) @@ -1064,7 +1065,7 @@ class SearchAnalytics(ProjectAdminMixin, PrivateViewMixin, TemplateView): template_name = 'projects/projects_search_analytics.html' http_method_names = ['get'] - feature_type = PlanFeature.TYPE_SEARCH_ANALYTICS + feature_type = TYPE_SEARCH_ANALYTICS def get(self, request, *args, **kwargs): download_data = request.GET.get('download', False) @@ -1151,7 +1152,6 @@ def _get_retention_days_limit(self, project): return PlanFeature.objects.get_feature_value( project, type=self.feature_type, - default=settings.RTD_ANALYTICS_DEFAULT_RETENTION_DAYS, ) def _is_enabled(self, project): @@ -1166,7 +1166,7 @@ class TrafficAnalyticsView(ProjectAdminMixin, PrivateViewMixin, TemplateView): template_name = 'projects/project_traffic_analytics.html' http_method_names = ['get'] - feature_type = PlanFeature.TYPE_PAGEVIEW_ANALYTICS + feature_type = TYPE_PAGEVIEW_ANALYTICS def get(self, request, *args, **kwargs): download_data = request.GET.get('download', False) @@ -1255,7 +1255,6 @@ def _get_retention_days_limit(self, project): return PlanFeature.objects.get_feature_value( project, type=self.feature_type, - default=settings.RTD_ANALYTICS_DEFAULT_RETENTION_DAYS, ) def _is_enabled(self, project): diff --git a/readthedocs/proxito/tests/test_full.py b/readthedocs/proxito/tests/test_full.py index 94386352d54..126b9bd5135 100644 --- a/readthedocs/proxito/tests/test_full.py +++ b/readthedocs/proxito/tests/test_full.py @@ -31,6 +31,7 @@ from readthedocs.redirects.models import Redirect from readthedocs.rtd_tests.storage import BuildMediaFileSystemStorageTest from readthedocs.subscriptions.models import Plan, PlanFeature, Subscription +from readthedocs.subscriptions.constants import TYPE_CDN from .base import BaseDocServing @@ -1211,7 +1212,6 @@ def test_serve_invalid_static_file(self, staticfiles_storage): ALLOW_PRIVATE_REPOS=True, PUBLIC_DOMAIN='dev.readthedocs.io', PUBLIC_DOMAIN_USES_HTTPS=True, - RTD_ALL_FEATURES_ENABLED=True, ) class TestCDNCache(BaseDocServing): @@ -1423,7 +1423,7 @@ def test_cache_on_plan(self): self.feature = get( PlanFeature, plan=self.plan, - feature_type=PlanFeature.TYPE_CDN, + feature_type=TYPE_CDN, ) # Delete feature plan, so we aren't using that logic diff --git a/readthedocs/proxito/tests/test_redirects.py b/readthedocs/proxito/tests/test_redirects.py index 1c1101e6b2b..d5e3cbec81c 100644 --- a/readthedocs/proxito/tests/test_redirects.py +++ b/readthedocs/proxito/tests/test_redirects.py @@ -9,7 +9,6 @@ @override_settings( PUBLIC_DOMAIN='dev.readthedocs.io', PUBLIC_DOMAIN_USES_HTTPS=True, - RTD_ALL_FEATURES_ENABLED=True, ) class RedirectTests(BaseDocServing): diff --git a/readthedocs/rtd_tests/tests/test_views.py b/readthedocs/rtd_tests/tests/test_views.py index 84c36ddbb87..7b2248b41f8 100644 --- a/readthedocs/rtd_tests/tests/test_views.py +++ b/readthedocs/rtd_tests/tests/test_views.py @@ -333,9 +333,6 @@ def test_rebuild_invalid_specific_commit(self, mock): self.assertEqual(r.status_code, 302) -@override_settings( - RTD_ALL_FEATURES_ENABLED=True, -) class TestSearchAnalyticsView(TestCase): """Tests for search analytics page.""" diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index 811a6beb622..db8a4a930a5 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -155,6 +155,28 @@ def SESSION_COOKIE_SAMESITE(self): # Number of days an invitation is valid. RTD_INVITATIONS_EXPIRATION_DAYS = 15 + @property + def RTD_DEFAULT_FEATURES(self): + # Features listed here will be available to users that don't have a + # subscription or if their subscription doesn't include the feature. + # Depending on the feature type, the numeric value represents a + # number of days or limit of the feature. + from readthedocs.subscriptions import constants + return { + constants.TYPE_CNAME: 1, + constants.TYPE_SSL: 1, + constants.TYPE_CDN: 1, + constants.TYPE_EMBED_API: 1, + # Retention days for search analytics. + constants.TYPE_SEARCH_ANALYTICS: self.RTD_ANALYTICS_DEFAULT_RETENTION_DAYS, + # Retention days for page view analytics. + constants.TYPE_PAGEVIEW_ANALYTICS: self.RTD_ANALYTICS_DEFAULT_RETENTION_DAYS, + # Retention days for audit logs. + constants.TYPE_AUDIT_LOGS: self.RTD_AUDITLOGS_DEFAULT_RETENTION_DAYS, + # Max number of concurrent builds. + constants.TYPE_CONCURRENT_BUILDS: self.RTD_MAX_CONCURRENT_BUILDS, + } + # Database and API hitting settings DONT_HIT_API = False DONT_HIT_DB = True @@ -750,9 +772,6 @@ def DOCKER_LIMITS(self): DEFAULT_PRIVACY_LEVEL = 'public' DEFAULT_VERSION_PRIVACY_LEVEL = 'public' ALLOW_ADMIN = True - # Enable all features by default, - # otherwise the organization subscription is checked. - RTD_ALL_FEATURES_ENABLED = True # Organization settings RTD_ALLOW_ORGANIZATIONS = False diff --git a/readthedocs/subscriptions/constants.py b/readthedocs/subscriptions/constants.py index ce2270e49bd..4939e3ff938 100644 --- a/readthedocs/subscriptions/constants.py +++ b/readthedocs/subscriptions/constants.py @@ -1,4 +1,39 @@ """Constants for subscriptions.""" +from django.utils.translation import gettext_lazy as _ # Days after the subscription has ended to disable the organization DISABLE_AFTER_DAYS = 30 + +# Values from `value` that represent an unlimited value. +UNLIMITED_VALUES = [None, -1] + +TYPE_CNAME = 'cname' +TYPE_CDN = 'cdn' +TYPE_SSL = 'ssl' +TYPE_SUPPORT = 'support' + +TYPE_PRIVATE_DOCS = 'private_docs' +TYPE_EMBED_API = 'embed_api' +TYPE_SEARCH_ANALYTICS = 'search_analytics' +TYPE_PAGEVIEW_ANALYTICS = 'pageviews_analytics' +TYPE_CONCURRENT_BUILDS = 'concurrent_builds' +TYPE_SSO = 'sso' +TYPE_CUSTOM_URL = 'urls' +TYPE_AUDIT_LOGS = 'audit-logs' +TYPE_AUDIT_PAGEVIEWS = 'audit-pageviews' + +FEATURE_TYPES = ( + (TYPE_CNAME, _('Custom domain')), + (TYPE_CDN, _('CDN public documentation')), + (TYPE_SSL, _('Custom SSL configuration')), + (TYPE_SUPPORT, _('Support SLA')), + (TYPE_PRIVATE_DOCS, _('Private documentation')), + (TYPE_EMBED_API, _('Embed content via API')), + (TYPE_SEARCH_ANALYTICS, _('Search analytics')), + (TYPE_PAGEVIEW_ANALYTICS, _('Pageview analytics')), + (TYPE_CONCURRENT_BUILDS, _('Concurrent builds')), + (TYPE_SSO, _('Single sign on (SSO) with Google')), + (TYPE_CUSTOM_URL, _('Custom URLs')), + (TYPE_AUDIT_LOGS, _('Audit logs')), + (TYPE_AUDIT_PAGEVIEWS, _('Record every page view')), +) diff --git a/readthedocs/subscriptions/managers.py b/readthedocs/subscriptions/managers.py index 1150c5512a0..817b0d576d9 100644 --- a/readthedocs/subscriptions/managers.py +++ b/readthedocs/subscriptions/managers.py @@ -166,7 +166,6 @@ class PlanFeatureManager(models.Manager): """Model manager for PlanFeature.""" - # pylint: disable=redefined-builtin def get_feature(self, obj, type): """ Get feature `type` for `obj`. @@ -192,21 +191,18 @@ def get_feature(self, obj, type): ) return feature.first() - # pylint: disable=redefined-builtin - def get_feature_value(self, obj, type, default=None): + def get_feature_value(self, obj, type, default=0): """ Get the value of the given feature. Use this function instead of ``get_feature().value`` when you need to respect the ``RTD_ALL_FEATURES_ENABLED`` setting. """ - if not settings.RTD_ALL_FEATURES_ENABLED: - feature = self.get_feature(obj, type) - if feature: - return feature.value - return default + feature = self.get_feature(obj, type) + if feature: + return feature.value + return settings.RTD_DEFAULT_FEATURES.get(type, default) - # pylint: disable=redefined-builtin def has_feature(self, obj, type): """ Get the value of the given feature. @@ -214,6 +210,6 @@ def has_feature(self, obj, type): Use this function instead of ``bool(get_feature())`` when you need to respect the ``RTD_ALL_FEATURES_ENABLED`` setting. """ - if settings.RTD_ALL_FEATURES_ENABLED: + if self.get_feature(obj, type) is not None: return True - return self.get_feature(obj, type) is not None + return type in settings.RTD_DEFAULT_FEATURES diff --git a/readthedocs/subscriptions/models.py b/readthedocs/subscriptions/models.py index 9a2ef4afff4..fbc7179ca11 100644 --- a/readthedocs/subscriptions/models.py +++ b/readthedocs/subscriptions/models.py @@ -9,6 +9,7 @@ from readthedocs.core.history import ExtraHistoricalRecords from readthedocs.core.utils import slugify +from readthedocs.subscriptions.constants import FEATURE_TYPES from readthedocs.organizations.models import Organization from readthedocs.subscriptions.managers import ( PlanFeatureManager, @@ -87,41 +88,6 @@ class Meta: db_table = 'organizations_planfeature' unique_together = (('plan', 'feature_type'),) - # Constants - UNLIMITED_VALUES = [None, -1] - """Values from `value` that represent an unlimited value.""" - - TYPE_CNAME = 'cname' - TYPE_CDN = 'cdn' - TYPE_SSL = 'ssl' - TYPE_SUPPORT = 'support' - - TYPE_PRIVATE_DOCS = 'private_docs' - TYPE_EMBED_API = 'embed_api' - TYPE_SEARCH_ANALYTICS = 'search_analytics' - TYPE_PAGEVIEW_ANALYTICS = 'pageviews_analytics' - TYPE_CONCURRENT_BUILDS = 'concurrent_builds' - TYPE_SSO = 'sso' - TYPE_CUSTOM_URL = 'urls' - TYPE_AUDIT_LOGS = 'audit-logs' - TYPE_AUDIT_PAGEVIEWS = 'audit-pageviews' - - TYPES = ( - (TYPE_CNAME, _('Custom domain')), - (TYPE_CDN, _('CDN public documentation')), - (TYPE_SSL, _('Custom SSL configuration')), - (TYPE_SUPPORT, _('Support SLA')), - (TYPE_PRIVATE_DOCS, _('Private documentation')), - (TYPE_EMBED_API, _('Embed content via API')), - (TYPE_SEARCH_ANALYTICS, _('Search analytics')), - (TYPE_PAGEVIEW_ANALYTICS, _('Pageview analytics')), - (TYPE_CONCURRENT_BUILDS, _('Concurrent builds')), - (TYPE_SSO, _('Single sign on (SSO) with Google')), - (TYPE_CUSTOM_URL, _('Custom URLs')), - (TYPE_AUDIT_LOGS, _('Audit logs')), - (TYPE_AUDIT_PAGEVIEWS, _('Record every page view')), - ) - # Auto fields pub_date = models.DateTimeField(_('Publication date'), auto_now_add=True) modified_date = models.DateTimeField(_('Modified date'), auto_now=True) @@ -131,7 +97,7 @@ class Meta: related_name='features', on_delete=models.CASCADE, ) - feature_type = models.CharField(_('Type'), max_length=32, choices=TYPES) + feature_type = models.CharField(_('Type'), max_length=32, choices=FEATURE_TYPES) value = models.IntegerField(_('Numeric value'), null=True, blank=True) description = models.CharField( _('Description'), From 330e1f6b82b9edd81adfdd7797e0187af05df3b2 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 22 Nov 2022 21:20:24 -0500 Subject: [PATCH 08/19] Linter --- readthedocs/core/mixins.py | 6 +-- readthedocs/organizations/views/private.py | 2 +- readthedocs/projects/querysets.py | 2 +- .../projects/tests/test_domain_views.py | 2 +- readthedocs/projects/views/private.py | 6 ++- readthedocs/proxito/tests/test_full.py | 2 +- readthedocs/subscriptions/constants.py | 52 +++++++++---------- readthedocs/subscriptions/models.py | 11 ++-- 8 files changed, 41 insertions(+), 42 deletions(-) diff --git a/readthedocs/core/mixins.py b/readthedocs/core/mixins.py index e8a8ca6c271..c064e796059 100644 --- a/readthedocs/core/mixins.py +++ b/readthedocs/core/mixins.py @@ -6,8 +6,8 @@ from vanilla import ListView from readthedocs.projects.models import Feature -from readthedocs.subscriptions.models import PlanFeature from readthedocs.subscriptions.constants import TYPE_CDN +from readthedocs.subscriptions.models import PlanFeature class ListViewWithForm(ListView): @@ -67,9 +67,7 @@ def can_be_cached(self, request): @lru_cache(maxsize=1) def _is_cache_enabled(self, project): """Helper function to check if CDN is enabled for a project.""" - plan_has_cdn = PlanFeature.objects.get_feature( - obj=project, type=TYPE_CDN - ) + plan_has_cdn = PlanFeature.objects.get_feature(obj=project, type=TYPE_CDN) return settings.ALLOW_PRIVATE_REPOS and ( plan_has_cdn or project.has_feature(Feature.CDN_ENABLED) ) diff --git a/readthedocs/organizations/views/private.py b/readthedocs/organizations/views/private.py index 9a1ebe95300..019bf40aa13 100644 --- a/readthedocs/organizations/views/private.py +++ b/readthedocs/organizations/views/private.py @@ -27,8 +27,8 @@ OrganizationView, ) from readthedocs.projects.utils import get_csv_file -from readthedocs.subscriptions.models import PlanFeature from readthedocs.subscriptions.constants import TYPE_AUDIT_LOGS +from readthedocs.subscriptions.models import PlanFeature # Organization views diff --git a/readthedocs/projects/querysets.py b/readthedocs/projects/querysets.py index 03c5751e26c..3994b793df8 100644 --- a/readthedocs/projects/querysets.py +++ b/readthedocs/projects/querysets.py @@ -105,8 +105,8 @@ def max_concurrent_builds(self, project): :returns: number of max concurrent builds for the project :rtype: int """ - from readthedocs.subscriptions.models import PlanFeature from readthedocs.subscriptions.constants import TYPE_CONCURRENT_BUILDS + from readthedocs.subscriptions.models import PlanFeature max_concurrent_organization = None organization = project.organizations.first() diff --git a/readthedocs/projects/tests/test_domain_views.py b/readthedocs/projects/tests/test_domain_views.py index 9341aa72d49..34bbcf8be48 100644 --- a/readthedocs/projects/tests/test_domain_views.py +++ b/readthedocs/projects/tests/test_domain_views.py @@ -5,8 +5,8 @@ from readthedocs.organizations.models import Organization from readthedocs.projects.models import Domain, Project -from readthedocs.subscriptions.models import Plan, PlanFeature, Subscription from readthedocs.subscriptions.constants import TYPE_CNAME +from readthedocs.subscriptions.models import Plan, PlanFeature, Subscription @override_settings(RTD_ALLOW_ORGANIZATIONS=False) diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index be538f8d519..fece00c1cc4 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -79,8 +79,12 @@ ProjectRelationListMixin, ) from readthedocs.search.models import SearchQuery +from readthedocs.subscriptions.constants import ( + TYPE_CNAME, + TYPE_PAGEVIEW_ANALYTICS, + TYPE_SEARCH_ANALYTICS, +) from readthedocs.subscriptions.models import PlanFeature -from readthedocs.subscriptions.constants import TYPE_PAGEVIEW_ANALYTICS, TYPE_CNAME, TYPE_SEARCH_ANALYTICS log = structlog.get_logger(__name__) diff --git a/readthedocs/proxito/tests/test_full.py b/readthedocs/proxito/tests/test_full.py index 126b9bd5135..5fe26d8500c 100644 --- a/readthedocs/proxito/tests/test_full.py +++ b/readthedocs/proxito/tests/test_full.py @@ -30,8 +30,8 @@ from readthedocs.proxito.views.mixins import ServeDocsMixin from readthedocs.redirects.models import Redirect from readthedocs.rtd_tests.storage import BuildMediaFileSystemStorageTest -from readthedocs.subscriptions.models import Plan, PlanFeature, Subscription from readthedocs.subscriptions.constants import TYPE_CDN +from readthedocs.subscriptions.models import Plan, PlanFeature, Subscription from .base import BaseDocServing diff --git a/readthedocs/subscriptions/constants.py b/readthedocs/subscriptions/constants.py index 4939e3ff938..d6dd62782d4 100644 --- a/readthedocs/subscriptions/constants.py +++ b/readthedocs/subscriptions/constants.py @@ -7,33 +7,33 @@ # Values from `value` that represent an unlimited value. UNLIMITED_VALUES = [None, -1] -TYPE_CNAME = 'cname' -TYPE_CDN = 'cdn' -TYPE_SSL = 'ssl' -TYPE_SUPPORT = 'support' +TYPE_CNAME = "cname" +TYPE_CDN = "cdn" +TYPE_SSL = "ssl" +TYPE_SUPPORT = "support" -TYPE_PRIVATE_DOCS = 'private_docs' -TYPE_EMBED_API = 'embed_api' -TYPE_SEARCH_ANALYTICS = 'search_analytics' -TYPE_PAGEVIEW_ANALYTICS = 'pageviews_analytics' -TYPE_CONCURRENT_BUILDS = 'concurrent_builds' -TYPE_SSO = 'sso' -TYPE_CUSTOM_URL = 'urls' -TYPE_AUDIT_LOGS = 'audit-logs' -TYPE_AUDIT_PAGEVIEWS = 'audit-pageviews' +TYPE_PRIVATE_DOCS = "private_docs" +TYPE_EMBED_API = "embed_api" +TYPE_SEARCH_ANALYTICS = "search_analytics" +TYPE_PAGEVIEW_ANALYTICS = "pageviews_analytics" +TYPE_CONCURRENT_BUILDS = "concurrent_builds" +TYPE_SSO = "sso" +TYPE_CUSTOM_URL = "urls" +TYPE_AUDIT_LOGS = "audit-logs" +TYPE_AUDIT_PAGEVIEWS = "audit-pageviews" FEATURE_TYPES = ( - (TYPE_CNAME, _('Custom domain')), - (TYPE_CDN, _('CDN public documentation')), - (TYPE_SSL, _('Custom SSL configuration')), - (TYPE_SUPPORT, _('Support SLA')), - (TYPE_PRIVATE_DOCS, _('Private documentation')), - (TYPE_EMBED_API, _('Embed content via API')), - (TYPE_SEARCH_ANALYTICS, _('Search analytics')), - (TYPE_PAGEVIEW_ANALYTICS, _('Pageview analytics')), - (TYPE_CONCURRENT_BUILDS, _('Concurrent builds')), - (TYPE_SSO, _('Single sign on (SSO) with Google')), - (TYPE_CUSTOM_URL, _('Custom URLs')), - (TYPE_AUDIT_LOGS, _('Audit logs')), - (TYPE_AUDIT_PAGEVIEWS, _('Record every page view')), + (TYPE_CNAME, _("Custom domain")), + (TYPE_CDN, _("CDN public documentation")), + (TYPE_SSL, _("Custom SSL configuration")), + (TYPE_SUPPORT, _("Support SLA")), + (TYPE_PRIVATE_DOCS, _("Private documentation")), + (TYPE_EMBED_API, _("Embed content via API")), + (TYPE_SEARCH_ANALYTICS, _("Search analytics")), + (TYPE_PAGEVIEW_ANALYTICS, _("Pageview analytics")), + (TYPE_CONCURRENT_BUILDS, _("Concurrent builds")), + (TYPE_SSO, _("Single sign on (SSO) with Google")), + (TYPE_CUSTOM_URL, _("Custom URLs")), + (TYPE_AUDIT_LOGS, _("Audit logs")), + (TYPE_AUDIT_PAGEVIEWS, _("Record every page view")), ) diff --git a/readthedocs/subscriptions/models.py b/readthedocs/subscriptions/models.py index fbc7179ca11..ba426e9b9d1 100644 --- a/readthedocs/subscriptions/models.py +++ b/readthedocs/subscriptions/models.py @@ -9,12 +9,9 @@ from readthedocs.core.history import ExtraHistoricalRecords from readthedocs.core.utils import slugify -from readthedocs.subscriptions.constants import FEATURE_TYPES from readthedocs.organizations.models import Organization -from readthedocs.subscriptions.managers import ( - PlanFeatureManager, - SubscriptionManager, -) +from readthedocs.subscriptions.constants import FEATURE_TYPES +from readthedocs.subscriptions.managers import PlanFeatureManager, SubscriptionManager class Plan(models.Model): @@ -97,8 +94,8 @@ class Meta: related_name='features', on_delete=models.CASCADE, ) - feature_type = models.CharField(_('Type'), max_length=32, choices=FEATURE_TYPES) - value = models.IntegerField(_('Numeric value'), null=True, blank=True) + feature_type = models.CharField(_("Type"), max_length=32, choices=FEATURE_TYPES) + value = models.IntegerField(_("Numeric value"), null=True, blank=True) description = models.CharField( _('Description'), max_length=255, From cfa0c66bf553161b0f39d90edfefba5bcde26774 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 22 Nov 2022 22:06:49 -0500 Subject: [PATCH 09/19] Explicit features --- readthedocs/builds/tests/test_build_queryset.py | 6 ++++++ readthedocs/organizations/tests/test_views.py | 8 +++++++- readthedocs/rtd_tests/tests/test_views.py | 6 ++++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/readthedocs/builds/tests/test_build_queryset.py b/readthedocs/builds/tests/test_build_queryset.py index 72d84e5cf3d..686701306f2 100644 --- a/readthedocs/builds/tests/test_build_queryset.py +++ b/readthedocs/builds/tests/test_build_queryset.py @@ -4,10 +4,16 @@ from readthedocs.builds.models import Build from readthedocs.organizations.models import Organization from readthedocs.projects.models import Project +from readthedocs.subscriptions.constants import TYPE_CONCURRENT_BUILDS @pytest.mark.django_db class TestBuildQuerySet: + @pytest.fixture(autouse=True) + def setup_method(self, settings): + settings.RTD_DEFAULT_FEATURES = { + TYPE_CONCURRENT_BUILDS: 4, + } def test_concurrent_builds(self): project = fixture.get( diff --git a/readthedocs/organizations/tests/test_views.py b/readthedocs/organizations/tests/test_views.py index 1de73f450fd..9b9b0c4bb6f 100644 --- a/readthedocs/organizations/tests/test_views.py +++ b/readthedocs/organizations/tests/test_views.py @@ -16,6 +16,7 @@ from readthedocs.organizations.models import Organization, Team from readthedocs.projects.models import Project from readthedocs.rtd_tests.base import RequestFactoryTestMixin +from readthedocs.subscriptions.constants import TYPE_AUDIT_LOGS @override_settings(RTD_ALLOW_ORGANIZATIONS=True) @@ -147,7 +148,12 @@ def test_add_owner(self): self.assertNotIn(user_b, self.organization.owners.all()) -@override_settings(RTD_ALLOW_ORGANIZATIONS=True) +@override_settings( + RTD_ALLOW_ORGANIZATIONS=True, + RTD_DEFAULT_FEATURES={ + TYPE_AUDIT_LOGS: 90, + }, +) class OrganizationSecurityLogTests(TestCase): def setUp(self): diff --git a/readthedocs/rtd_tests/tests/test_views.py b/readthedocs/rtd_tests/tests/test_views.py index 7b2248b41f8..097e217a4db 100644 --- a/readthedocs/rtd_tests/tests/test_views.py +++ b/readthedocs/rtd_tests/tests/test_views.py @@ -15,6 +15,7 @@ from readthedocs.projects.constants import PUBLIC from readthedocs.projects.forms import UpdateProjectForm from readthedocs.projects.models import Feature, Project +from readthedocs.subscriptions.constants import TYPE_SEARCH_ANALYTICS @mock.patch('readthedocs.projects.forms.trigger_build', mock.MagicMock()) @@ -333,6 +334,11 @@ def test_rebuild_invalid_specific_commit(self, mock): self.assertEqual(r.status_code, 302) +@override_settings( + RTD_DEFAULT_FEATURES={ + TYPE_SEARCH_ANALYTICS: 90, + } +) class TestSearchAnalyticsView(TestCase): """Tests for search analytics page.""" From bf4199b2c7d65728b1e64b28fd03e169ff96c42f Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 22 Nov 2022 22:58:23 -0500 Subject: [PATCH 10/19] One more --- readthedocs/rtd_tests/tests/test_api.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index 8ee068b7d5e..03ee8e4f0f0 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -7,6 +7,7 @@ from django.contrib.auth.models import User from django.http import QueryDict from django.test import TestCase +from django.test.utils import override_settings from django.urls import reverse from django_dynamic_fixture import get from rest_framework import status @@ -58,6 +59,7 @@ Feature, Project, ) +from readthedocs.subscriptions.constants import TYPE_CONCURRENT_BUILDS super_auth = base64.b64encode(b'super:test').decode('utf-8') eric_auth = base64.b64encode(b'eric:test').decode('utf-8') @@ -789,6 +791,11 @@ def test_init_api_project(self): {'RELEASE': 'prod'}, ) + @override_settings( + RTD_DEFAULT_FEATURES={ + TYPE_CONCURRENT_BUILDS: 4, + } + ) def test_concurrent_builds(self): expected = { 'limit_reached': False, From 33c4a30f9eea20e45e960bc486fad96e96c96a36 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 23 Nov 2022 10:47:00 -0500 Subject: [PATCH 11/19] More explicit tests --- readthedocs/core/mixins.py | 2 +- readthedocs/projects/querysets.py | 1 - readthedocs/proxito/tests/test_full.py | 6 +++++- readthedocs/proxito/tests/test_redirects.py | 6 ++++++ 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/readthedocs/core/mixins.py b/readthedocs/core/mixins.py index c064e796059..266e7084c5d 100644 --- a/readthedocs/core/mixins.py +++ b/readthedocs/core/mixins.py @@ -67,7 +67,7 @@ def can_be_cached(self, request): @lru_cache(maxsize=1) def _is_cache_enabled(self, project): """Helper function to check if CDN is enabled for a project.""" - plan_has_cdn = PlanFeature.objects.get_feature(obj=project, type=TYPE_CDN) + plan_has_cdn = PlanFeature.objects.has_feature(obj=project, type=TYPE_CDN) return settings.ALLOW_PRIVATE_REPOS and ( plan_has_cdn or project.has_feature(Feature.CDN_ENABLED) ) diff --git a/readthedocs/projects/querysets.py b/readthedocs/projects/querysets.py index 3994b793df8..c64e772a97a 100644 --- a/readthedocs/projects/querysets.py +++ b/readthedocs/projects/querysets.py @@ -1,5 +1,4 @@ """Project model QuerySet classes.""" -from django.conf import settings from django.db import models from django.db.models import Count, OuterRef, Prefetch, Q, Subquery diff --git a/readthedocs/proxito/tests/test_full.py b/readthedocs/proxito/tests/test_full.py index 5fe26d8500c..b25c5f968f8 100644 --- a/readthedocs/proxito/tests/test_full.py +++ b/readthedocs/proxito/tests/test_full.py @@ -30,7 +30,7 @@ from readthedocs.proxito.views.mixins import ServeDocsMixin from readthedocs.redirects.models import Redirect from readthedocs.rtd_tests.storage import BuildMediaFileSystemStorageTest -from readthedocs.subscriptions.constants import TYPE_CDN +from readthedocs.subscriptions.constants import TYPE_CDN, TYPE_CNAME from readthedocs.subscriptions.models import Plan, PlanFeature, Subscription from .base import BaseDocServing @@ -1212,6 +1212,10 @@ def test_serve_invalid_static_file(self, staticfiles_storage): ALLOW_PRIVATE_REPOS=True, PUBLIC_DOMAIN='dev.readthedocs.io', PUBLIC_DOMAIN_USES_HTTPS=True, + RTD_DEFAULT_FEATURES={ + TYPE_CNAME: 1, + TYPE_CDN: 1, + }, ) class TestCDNCache(BaseDocServing): diff --git a/readthedocs/proxito/tests/test_redirects.py b/readthedocs/proxito/tests/test_redirects.py index d5e3cbec81c..5c8e722ad02 100644 --- a/readthedocs/proxito/tests/test_redirects.py +++ b/readthedocs/proxito/tests/test_redirects.py @@ -3,12 +3,18 @@ import pytest from django.test import override_settings +from readthedocs.subscriptions.constants import TYPE_CNAME, TYPE_SSL + from .base import BaseDocServing @override_settings( PUBLIC_DOMAIN='dev.readthedocs.io', PUBLIC_DOMAIN_USES_HTTPS=True, + RTD_DEFAULT_FEATURES={ + TYPE_CNAME: 1, + TYPE_SSL: 1, + }, ) class RedirectTests(BaseDocServing): From d0113269565cb9ba5556d784533a35cf6f852420 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 23 Nov 2022 10:51:49 -0500 Subject: [PATCH 12/19] Linter --- readthedocs/subscriptions/managers.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/readthedocs/subscriptions/managers.py b/readthedocs/subscriptions/managers.py index 817b0d576d9..a2654bf0a5a 100644 --- a/readthedocs/subscriptions/managers.py +++ b/readthedocs/subscriptions/managers.py @@ -166,6 +166,7 @@ class PlanFeatureManager(models.Manager): """Model manager for PlanFeature.""" + # pylint: disable=redefined-builtin def get_feature(self, obj, type): """ Get feature `type` for `obj`. @@ -191,6 +192,7 @@ def get_feature(self, obj, type): ) return feature.first() + # pylint: disable=redefined-builtin def get_feature_value(self, obj, type, default=0): """ Get the value of the given feature. @@ -203,6 +205,7 @@ def get_feature_value(self, obj, type, default=0): return feature.value return settings.RTD_DEFAULT_FEATURES.get(type, default) + # pylint: disable=redefined-builtin def has_feature(self, obj, type): """ Get the value of the given feature. From 6d9f144be086458f76d39c32da6a7ffc7514eae7 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 23 Nov 2022 11:05:11 -0500 Subject: [PATCH 13/19] Update docstrings --- readthedocs/subscriptions/managers.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/readthedocs/subscriptions/managers.py b/readthedocs/subscriptions/managers.py index a2654bf0a5a..bdcfad5943e 100644 --- a/readthedocs/subscriptions/managers.py +++ b/readthedocs/subscriptions/managers.py @@ -172,7 +172,7 @@ def get_feature(self, obj, type): Get feature `type` for `obj`. :param obj: An organization or project instance. - :param type: The type of the feature (PlanFeature.TYPE_*). + :param type: The type of the feature (readthedocs.subscriptions.constants.TYPE_*). :returns: A PlanFeature object or None. """ # Avoid circular imports. @@ -198,7 +198,7 @@ def get_feature_value(self, obj, type, default=0): Get the value of the given feature. Use this function instead of ``get_feature().value`` - when you need to respect the ``RTD_ALL_FEATURES_ENABLED`` setting. + when you need to respect the ``RTD_DEFAULT_FEATURES`` setting. """ feature = self.get_feature(obj, type) if feature: @@ -211,7 +211,7 @@ def has_feature(self, obj, type): Get the value of the given feature. Use this function instead of ``bool(get_feature())`` - when you need to respect the ``RTD_ALL_FEATURES_ENABLED`` setting. + when you need to respect the ``RTD_DEFAULT_FEATURES`` setting. """ if self.get_feature(obj, type) is not None: return True From 9d23f797f86857c939bd9c0e4feb7a38d79c3ef9 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 23 Nov 2022 11:33:08 -0500 Subject: [PATCH 14/19] Update tests --- readthedocs/proxito/tests/test_headers.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/readthedocs/proxito/tests/test_headers.py b/readthedocs/proxito/tests/test_headers.py index 0e13b62935c..db74461bc2d 100644 --- a/readthedocs/proxito/tests/test_headers.py +++ b/readthedocs/proxito/tests/test_headers.py @@ -4,6 +4,7 @@ from readthedocs.builds.constants import LATEST from readthedocs.projects.models import Domain, HTTPHeader +from readthedocs.subscriptions.constants import TYPE_CDN from .base import BaseDocServing @@ -134,8 +135,19 @@ def test_cache_headers_public_projects(self): self.assertEqual(r.status_code, 200) self.assertNotIn('CDN-Cache-Control', r) - @override_settings(ALLOW_PRIVATE_REPOS=True) - def test_cache_headers_private_projects(self): + @override_settings(ALLOW_PRIVATE_REPOS=True, RTD_DEFAULT_FEATURES={}) + def test_cache_headers_private_project_no_feature(self): r = self.client.get('/en/latest/', HTTP_HOST='project.dev.readthedocs.io') self.assertEqual(r.status_code, 200) self.assertEqual(r['CDN-Cache-Control'], 'private') + + @override_settings( + ALLOW_PRIVATE_REPOS=True, + RTD_DEFAULT_FEATURES={ + TYPE_CDN: 1, + }, + ) + def test_cache_headers_private_project_with_feature(self): + r = self.client.get("/en/latest/", HTTP_HOST="project.dev.readthedocs.io") + self.assertEqual(r.status_code, 200) + self.assertEqual(r["CDN-Cache-Control"], "public") From bff4ab46fd6493ae9773cec6772c3f366f1f450e Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 4 Apr 2023 15:57:07 -0500 Subject: [PATCH 15/19] Black --- readthedocs/core/mixins.py | 1 + readthedocs/proxito/tests/base.py | 2 +- readthedocs/proxito/tests/test_full.py | 2 +- readthedocs/proxito/tests/test_headers.py | 6 +++--- readthedocs/proxito/tests/test_redirects.py | 1 - 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/readthedocs/core/mixins.py b/readthedocs/core/mixins.py index afdebc81f67..3143be2aad7 100644 --- a/readthedocs/core/mixins.py +++ b/readthedocs/core/mixins.py @@ -2,6 +2,7 @@ from django.contrib.auth.mixins import LoginRequiredMixin from vanilla import ListView + class ListViewWithForm(ListView): """List view that also exposes a create form.""" diff --git a/readthedocs/proxito/tests/base.py b/readthedocs/proxito/tests/base.py index 384a1860c2d..298fce26b55 100644 --- a/readthedocs/proxito/tests/base.py +++ b/readthedocs/proxito/tests/base.py @@ -7,8 +7,8 @@ from django.core.files.storage import get_storage_class from django.test import TestCase -from readthedocs.projects.constants import PUBLIC, SSL_STATUS_VALID from readthedocs.builds.constants import LATEST +from readthedocs.projects.constants import PUBLIC, SSL_STATUS_VALID from readthedocs.projects.models import Domain, Project from readthedocs.proxito.views import serve diff --git a/readthedocs/proxito/tests/test_full.py b/readthedocs/proxito/tests/test_full.py index 36384c8d801..7c64cba56a7 100644 --- a/readthedocs/proxito/tests/test_full.py +++ b/readthedocs/proxito/tests/test_full.py @@ -29,11 +29,11 @@ from readthedocs.projects.models import Domain, Feature, Project from readthedocs.proxito.views.mixins import ServeDocsMixin from readthedocs.redirects.models import Redirect -from readthedocs.subscriptions.constants import TYPE_CDN, TYPE_CNAME from readthedocs.rtd_tests.storage import ( BuildMediaFileSystemStorageTest, StaticFileSystemStorageTest, ) +from readthedocs.subscriptions.constants import TYPE_CDN, TYPE_CNAME from readthedocs.subscriptions.models import Plan, PlanFeature, Subscription from .base import BaseDocServing diff --git a/readthedocs/proxito/tests/test_headers.py b/readthedocs/proxito/tests/test_headers.py index 3e0d9c2967f..c5e6141425a 100644 --- a/readthedocs/proxito/tests/test_headers.py +++ b/readthedocs/proxito/tests/test_headers.py @@ -4,8 +4,8 @@ from django_dynamic_fixture import get from readthedocs.builds.constants import LATEST -from readthedocs.subscriptions.constants import TYPE_CDN from readthedocs.projects.models import Domain, Feature, HTTPHeader +from readthedocs.subscriptions.constants import TYPE_CDN from .base import BaseDocServing @@ -154,9 +154,9 @@ def test_cache_headers_public_version_with_private_projects_not_allowed(self): @override_settings(ALLOW_PRIVATE_REPOS=True, RTD_DEFAULT_FEATURES={}) def test_cache_headers_private_project_no_feature(self): - r = self.client.get('/en/latest/', HTTP_HOST='project.dev.readthedocs.io') + r = self.client.get("/en/latest/", HTTP_HOST="project.dev.readthedocs.io") self.assertEqual(r.status_code, 200) - self.assertEqual(r['CDN-Cache-Control'], 'private') + self.assertEqual(r["CDN-Cache-Control"], "private") @override_settings( ALLOW_PRIVATE_REPOS=True, diff --git a/readthedocs/proxito/tests/test_redirects.py b/readthedocs/proxito/tests/test_redirects.py index 8ef4015e899..7e9793aef36 100644 --- a/readthedocs/proxito/tests/test_redirects.py +++ b/readthedocs/proxito/tests/test_redirects.py @@ -4,7 +4,6 @@ from readthedocs.projects.models import Feature from readthedocs.proxito.constants import RedirectType - from readthedocs.subscriptions.constants import TYPE_CNAME, TYPE_SSL from .base import BaseDocServing From 652c62c90627e7b5425df3329361873b8c14406c Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 4 Apr 2023 16:17:26 -0500 Subject: [PATCH 16/19] CND is available in all plans now --- readthedocs/proxito/tests/test_full.py | 29 +---------------------- readthedocs/proxito/tests/test_headers.py | 18 -------------- 2 files changed, 1 insertion(+), 46 deletions(-) diff --git a/readthedocs/proxito/tests/test_full.py b/readthedocs/proxito/tests/test_full.py index 7c64cba56a7..f501ab1ef9c 100644 --- a/readthedocs/proxito/tests/test_full.py +++ b/readthedocs/proxito/tests/test_full.py @@ -14,7 +14,6 @@ from readthedocs.audit.models import AuditLog from readthedocs.builds.constants import EXTERNAL, INTERNAL, LATEST from readthedocs.builds.models import Version -from readthedocs.organizations.models import Organization from readthedocs.projects import constants from readthedocs.projects.constants import ( DOWNLOADABLE_MEDIA_TYPES, @@ -33,8 +32,7 @@ BuildMediaFileSystemStorageTest, StaticFileSystemStorageTest, ) -from readthedocs.subscriptions.constants import TYPE_CDN, TYPE_CNAME -from readthedocs.subscriptions.models import Plan, PlanFeature, Subscription +from readthedocs.subscriptions.constants import TYPE_CNAME from .base import BaseDocServing @@ -1517,7 +1515,6 @@ def test_404_download(self): PUBLIC_DOMAIN_USES_HTTPS=True, RTD_DEFAULT_FEATURES={ TYPE_CNAME: 1, - TYPE_CDN: 1, }, ) # We are overriding the storage class instead of using RTD_BUILD_MEDIA_STORAGE, @@ -1746,30 +1743,6 @@ def test_cache_disable_on_rtd_header_resolved_project(self): self.assertEqual(resp.headers["CDN-Cache-Control"], "private") self.assertEqual(resp.headers["Cache-Tag"], "project,project:latest") - def test_cache_on_plan(self): - self.organization = get(Organization) - self.plan = get( - Plan, - published=True, - ) - self.subscription = get( - Subscription, - plan=self.plan, - organization=self.organization, - ) - self.feature = get( - PlanFeature, - plan=self.plan, - feature_type=TYPE_CDN, - ) - - # Delete feature plan, so we aren't using that logic - Feature.objects.filter(feature_id=Feature.CDN_ENABLED).delete() - - # Add project to plan, so we're using that to enable CDN - self.organization.projects.add(self.project) - self._test_cache_control_header_project(expected_value="public") - class ProxitoV2TestCDNCache(TestCDNCache): # TODO: remove this class once the new implementation is the default. diff --git a/readthedocs/proxito/tests/test_headers.py b/readthedocs/proxito/tests/test_headers.py index c5e6141425a..aba8061beb9 100644 --- a/readthedocs/proxito/tests/test_headers.py +++ b/readthedocs/proxito/tests/test_headers.py @@ -5,7 +5,6 @@ from readthedocs.builds.constants import LATEST from readthedocs.projects.models import Domain, Feature, HTTPHeader -from readthedocs.subscriptions.constants import TYPE_CDN from .base import BaseDocServing @@ -152,23 +151,6 @@ def test_cache_headers_public_version_with_private_projects_not_allowed(self): self.assertEqual(r.status_code, 200) self.assertEqual(r["CDN-Cache-Control"], "public") - @override_settings(ALLOW_PRIVATE_REPOS=True, RTD_DEFAULT_FEATURES={}) - def test_cache_headers_private_project_no_feature(self): - r = self.client.get("/en/latest/", HTTP_HOST="project.dev.readthedocs.io") - self.assertEqual(r.status_code, 200) - self.assertEqual(r["CDN-Cache-Control"], "private") - - @override_settings( - ALLOW_PRIVATE_REPOS=True, - RTD_DEFAULT_FEATURES={ - TYPE_CDN: 1, - }, - ) - def test_cache_headers_private_project_with_feature(self): - r = self.client.get("/en/latest/", HTTP_HOST="project.dev.readthedocs.io") - self.assertEqual(r.status_code, 200) - self.assertEqual(r["CDN-Cache-Control"], "public") - @override_settings(ALLOW_PRIVATE_REPOS=True) def test_cache_headers_public_version_with_private_projects_allowed(self): r = self.client.get( From 39581af6656fae211368a13bdb3cc7790b5a14ac Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 4 Apr 2023 16:32:42 -0500 Subject: [PATCH 17/19] Don't hit the DB if subscriptions aren't allowed --- readthedocs/settings/base.py | 1 - readthedocs/subscriptions/managers.py | 14 +++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index 349178dcc99..6d1dc1e8ecf 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -167,7 +167,6 @@ def RTD_DEFAULT_FEATURES(self): return { constants.TYPE_CNAME: 1, constants.TYPE_SSL: 1, - constants.TYPE_CDN: 1, constants.TYPE_EMBED_API: 1, # Retention days for search analytics. constants.TYPE_SEARCH_ANALYTICS: self.RTD_ANALYTICS_DEFAULT_RETENTION_DAYS, diff --git a/readthedocs/subscriptions/managers.py b/readthedocs/subscriptions/managers.py index bdcfad5943e..3433fdef4e3 100644 --- a/readthedocs/subscriptions/managers.py +++ b/readthedocs/subscriptions/managers.py @@ -200,9 +200,11 @@ def get_feature_value(self, obj, type, default=0): Use this function instead of ``get_feature().value`` when you need to respect the ``RTD_DEFAULT_FEATURES`` setting. """ - feature = self.get_feature(obj, type) - if feature: - return feature.value + # Hit the DB only if subscriptions are enabled. + if settings.RTD_ALLOW_ORGANIZATIONS: + feature = self.get_feature(obj, type) + if feature: + return feature.value return settings.RTD_DEFAULT_FEATURES.get(type, default) # pylint: disable=redefined-builtin @@ -213,6 +215,8 @@ def has_feature(self, obj, type): Use this function instead of ``bool(get_feature())`` when you need to respect the ``RTD_DEFAULT_FEATURES`` setting. """ - if self.get_feature(obj, type) is not None: - return True + # Hit the DB only if subscriptions are enabled. + if settings.RTD_ALLOW_ORGANIZATIONS: + if self.get_feature(obj, type) is not None: + return True return type in settings.RTD_DEFAULT_FEATURES From 21473bcfd0b8d9c9b801799bab35751395a6312c Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 5 Apr 2023 12:04:01 -0500 Subject: [PATCH 18/19] Check for custom domains --- readthedocs/core/resolver.py | 4 +++- readthedocs/proxito/tests/test_redirects.py | 3 +-- readthedocs/settings/base.py | 1 - 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/readthedocs/core/resolver.py b/readthedocs/core/resolver.py index 24946148921..95f585017d7 100644 --- a/readthedocs/core/resolver.py +++ b/readthedocs/core/resolver.py @@ -7,6 +7,8 @@ from readthedocs.builds.constants import EXTERNAL from readthedocs.core.utils.extend import SettingsOverrideObject from readthedocs.core.utils.url import unsafe_join_url_path +from readthedocs.subscriptions.constants import TYPE_CNAME +from readthedocs.subscriptions.models import PlanFeature log = structlog.get_logger(__name__) @@ -404,7 +406,7 @@ def _use_subdomain(self): def _use_cname(self, project): """Test if to allow direct serving for project on CNAME.""" - return True + return PlanFeature.objects.has_feature(project, type=TYPE_CNAME) class Resolver(SettingsOverrideObject): diff --git a/readthedocs/proxito/tests/test_redirects.py b/readthedocs/proxito/tests/test_redirects.py index 7e9793aef36..2518228f998 100644 --- a/readthedocs/proxito/tests/test_redirects.py +++ b/readthedocs/proxito/tests/test_redirects.py @@ -4,7 +4,7 @@ from readthedocs.projects.models import Feature from readthedocs.proxito.constants import RedirectType -from readthedocs.subscriptions.constants import TYPE_CNAME, TYPE_SSL +from readthedocs.subscriptions.constants import TYPE_CNAME from .base import BaseDocServing @@ -14,7 +14,6 @@ PUBLIC_DOMAIN_USES_HTTPS=True, RTD_DEFAULT_FEATURES={ TYPE_CNAME: 1, - TYPE_SSL: 1, }, ) class RedirectTests(BaseDocServing): diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index 6d1dc1e8ecf..415e261cb5a 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -166,7 +166,6 @@ def RTD_DEFAULT_FEATURES(self): from readthedocs.subscriptions import constants return { constants.TYPE_CNAME: 1, - constants.TYPE_SSL: 1, constants.TYPE_EMBED_API: 1, # Retention days for search analytics. constants.TYPE_SEARCH_ANALYTICS: self.RTD_ANALYTICS_DEFAULT_RETENTION_DAYS, From 65b33df33fb02d921097f89a9a70c30acab26b29 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 5 Apr 2023 12:13:33 -0500 Subject: [PATCH 19/19] Be explicit in tests --- readthedocs/proxito/tests/test_middleware.py | 8 +++++++- readthedocs/rtd_tests/tests/test_footer.py | 8 +++++++- readthedocs/rtd_tests/tests/test_resolver.py | 8 +++++++- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/readthedocs/proxito/tests/test_middleware.py b/readthedocs/proxito/tests/test_middleware.py index 619598852d8..858344e7d34 100644 --- a/readthedocs/proxito/tests/test_middleware.py +++ b/readthedocs/proxito/tests/test_middleware.py @@ -16,10 +16,16 @@ from readthedocs.rtd_tests.base import RequestFactoryTestMixin from readthedocs.rtd_tests.storage import BuildMediaFileSystemStorageTest from readthedocs.rtd_tests.utils import create_user +from readthedocs.subscriptions.constants import TYPE_CNAME @pytest.mark.proxito -@override_settings(PUBLIC_DOMAIN='dev.readthedocs.io') +@override_settings( + PUBLIC_DOMAIN="dev.readthedocs.io", + RTD_DEFAULT_FEATURES={ + TYPE_CNAME: 1, + }, +) class MiddlewareTests(RequestFactoryTestMixin, TestCase): def setUp(self): diff --git a/readthedocs/rtd_tests/tests/test_footer.py b/readthedocs/rtd_tests/tests/test_footer.py index c6f823a8fdc..5ada66df22c 100644 --- a/readthedocs/rtd_tests/tests/test_footer.py +++ b/readthedocs/rtd_tests/tests/test_footer.py @@ -13,6 +13,7 @@ from readthedocs.core.middleware import ReadTheDocsSessionMiddleware from readthedocs.projects.constants import GITHUB_BRAND, GITLAB_BRAND, PUBLIC from readthedocs.projects.models import Project +from readthedocs.subscriptions.constants import TYPE_CNAME class BaseTestFooterHTML: @@ -444,7 +445,12 @@ def test_highest_version_without_tags(self): @pytest.mark.proxito -@override_settings(PUBLIC_DOMAIN='readthedocs.io') +@override_settings( + PUBLIC_DOMAIN="readthedocs.io", + RTD_DEFAULT_FEATURES={ + TYPE_CNAME: 1, + }, +) class TestFooterPerformance(TestCase): # The expected number of queries for generating the footer # This shouldn't increase unless we modify the footer API diff --git a/readthedocs/rtd_tests/tests/test_resolver.py b/readthedocs/rtd_tests/tests/test_resolver.py index 8e3e9d2f0d1..52a103971a4 100644 --- a/readthedocs/rtd_tests/tests/test_resolver.py +++ b/readthedocs/rtd_tests/tests/test_resolver.py @@ -7,9 +7,15 @@ from readthedocs.projects.constants import PRIVATE from readthedocs.projects.models import Domain, Project, ProjectRelationship from readthedocs.rtd_tests.utils import create_user +from readthedocs.subscriptions.constants import TYPE_CNAME -@override_settings(PUBLIC_DOMAIN='readthedocs.org') +@override_settings( + PUBLIC_DOMAIN="readthedocs.org", + RTD_DEFAULT_FEATURES={ + TYPE_CNAME: 1, + }, +) class ResolverBase(TestCase): def setUp(self):