Unify feature check for organization/project (#8920)

We are overriding classes and skipping tests con .com just because on .com we require a subscription for some features.
Now that we have subs on .org we can share more code between both sites. 
We have a setting to enable a set of features, this is for users that don't have a subscription (.org), on .com this will {} to rely entirely on the features from the subscription.

Author: stsewd

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(

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):

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):

readthedocs/organizations/views/private.py

@@ -13,7 +13,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.invitations.models import Invitation
 from readthedocs.organizations.forms import (
     OrganizationSignupForm,
@@ -28,6 +27,8 @@
     OrganizationView,
 )
 from readthedocs.projects.utils import get_csv_file
+from readthedocs.subscriptions.constants import TYPE_AUDIT_LOGS
+from readthedocs.subscriptions.models import PlanFeature
 
 
 # Organization views
@@ -183,12 +184,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 = TYPE_AUDIT_LOGS
 
     def get(self, request, *args, **kwargs):
         download_data = request.GET.get('download', False)
@@ -234,10 +236,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_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):
@@ -255,7 +257,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(
@@ -284,14 +286,15 @@ 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 _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,
+        )
 
-class OrganizationSecurityLog(SettingsOverrideObject):
-    _default_class = OrganizationSecurityLogBase
+    def _is_feature_enabled(self, organization):
+        return PlanFeature.objects.has_feature(
+            organization,
+            type=self.feature_type,
+        )

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
 
@@ -96,6 +95,7 @@ def max_concurrent_builds(self, project):
 
           - project
           - organization
+          - plan
           - default setting
 
         :param project: project to be checked
@@ -104,15 +104,21 @@ def max_concurrent_builds(self, project):
         :returns: number of max concurrent builds for the project
         :rtype: int
         """
+        from readthedocs.subscriptions.constants import TYPE_CONCURRENT_BUILDS
+        from readthedocs.subscriptions.models import PlanFeature
+
         max_concurrent_organization = None
         organization = project.organizations.first()
         if organization:
             max_concurrent_organization = organization.max_concurrent_builds
 
         return (
-            project.max_concurrent_builds or
-            max_concurrent_organization or
-            settings.RTD_MAX_CONCURRENT_BUILDS
+            project.max_concurrent_builds
+            or max_concurrent_organization
+            or PlanFeature.objects.get_feature_value(
+                project,
+                type=TYPE_CONCURRENT_BUILDS,
+            )
         )
 
     def prefetch_latest_build(self):

readthedocs/projects/tests/test_domain_views.py

@@ -5,6 +5,7 @@
 
 from readthedocs.organizations.models import Organization
 from readthedocs.projects.models import Domain, Project
+from readthedocs.subscriptions.constants import TYPE_CNAME
 from readthedocs.subscriptions.models import Plan, PlanFeature, Subscription
 
 
@@ -114,5 +115,5 @@ def setUp(self):
         self.feature = get(
             PlanFeature,
             plan=self.plan,
-            feature_type=PlanFeature.TYPE_CNAME,
+            feature_type=TYPE_CNAME,
         )

readthedocs/projects/views/private.py

@@ -39,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.invitations.models import Invitation
 from readthedocs.oauth.services import registry
@@ -80,6 +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
 
 log = structlog.get_logger(__name__)
 
@@ -752,6 +757,7 @@ class DomainMixin(ProjectAdminMixin, PrivateViewMixin):
     model = Domain
     form_class = DomainForm
     lookup_url_kwarg = 'domain_pk'
+    feature_type = TYPE_CNAME
 
     def get_success_url(self):
         return reverse('projects_domains', args=[self.get_project().slug])
@@ -763,11 +769,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)
@@ -782,12 +790,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()
@@ -806,12 +809,7 @@ def get_success_url(self):
         )
 
 
-class DomainCreate(SettingsOverrideObject):
-
-    _default_class = DomainCreateBase
-
-
-class DomainUpdateBase(DomainMixin, UpdateView):
+class DomainUpdate(DomainMixin, UpdateView):
 
     def form_valid(self, form):
         response = super().form_valid(form)
@@ -825,11 +823,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
@@ -1072,10 +1065,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 = TYPE_SEARCH_ANALYTICS
 
     def get(self, request, *args, **kwargs):
         download_data = request.GET.get('download', False)
@@ -1159,21 +1153,24 @@ 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,
+        )
 
     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 = TYPE_PAGEVIEW_ANALYTICS
 
     def get(self, request, *args, **kwargs):
         download_data = request.GET.get('download', False)
@@ -1259,12 +1256,14 @@ 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,
+        )
 
     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,
+        )

readthedocs/proxito/tests/base.py

@@ -8,7 +8,7 @@
 from django.test import TestCase
 
 from readthedocs.builds.constants import LATEST
-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
 
@@ -80,5 +80,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,
+        )