From 5570a73c9bade31b563a2bd32af72eec67381597 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 11 Apr 2023 22:59:26 -0500 Subject: [PATCH 01/35] Subscriptions: use djstripe for products/features --- readthedocs/api/v3/permissions.py | 4 +- readthedocs/core/resolver.py | 4 +- readthedocs/organizations/views/private.py | 30 +++------ readthedocs/projects/querysets.py | 6 +- readthedocs/projects/views/private.py | 54 ++++------------ readthedocs/proxito/views/mixins.py | 4 +- readthedocs/settings/base.py | 19 +++--- readthedocs/subscriptions/managers.py | 60 ----------------- readthedocs/subscriptions/models.py | 3 +- readthedocs/subscriptions/products.py | 75 ++++++++++++++++++++++ 10 files changed, 120 insertions(+), 139 deletions(-) create mode 100644 readthedocs/subscriptions/products.py diff --git a/readthedocs/api/v3/permissions.py b/readthedocs/api/v3/permissions.py index fb0e5098ea6..da8419c798b 100644 --- a/readthedocs/api/v3/permissions.py +++ b/readthedocs/api/v3/permissions.py @@ -2,7 +2,7 @@ from readthedocs.core.utils.extend import SettingsOverrideObject from readthedocs.subscriptions.constants import TYPE_EMBED_API -from readthedocs.subscriptions.models import PlanFeature +from readthedocs.subscriptions.products import get_feature class HasEmbedAPIAccess(BasePermission): @@ -24,7 +24,7 @@ class HasEmbedAPIAccess(BasePermission): def has_permission(self, request, view): project = view._get_project() # The project is None when the is requesting a section from an external site. - if project and not PlanFeature.objects.has_feature(project, TYPE_EMBED_API): + if project and not get_feature(project, type=TYPE_EMBED_API): return False return True diff --git a/readthedocs/core/resolver.py b/readthedocs/core/resolver.py index 95f585017d7..0f4c5f9c911 100644 --- a/readthedocs/core/resolver.py +++ b/readthedocs/core/resolver.py @@ -8,7 +8,7 @@ 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 +from readthedocs.subscriptions.products import get_feature log = structlog.get_logger(__name__) @@ -406,7 +406,7 @@ def _use_subdomain(self): def _use_cname(self, project): """Test if to allow direct serving for project on CNAME.""" - return PlanFeature.objects.has_feature(project, type=TYPE_CNAME) + return bool(get_feature(project, type=TYPE_CNAME)) class Resolver(SettingsOverrideObject): diff --git a/readthedocs/organizations/views/private.py b/readthedocs/organizations/views/private.py index 29295bd70ef..bddb8414333 100644 --- a/readthedocs/organizations/views/private.py +++ b/readthedocs/organizations/views/private.py @@ -28,7 +28,7 @@ ) from readthedocs.projects.utils import get_csv_file from readthedocs.subscriptions.constants import TYPE_AUDIT_LOGS -from readthedocs.subscriptions.models import PlanFeature +from readthedocs.subscriptions.products import get_feature # Organization views @@ -236,8 +236,9 @@ 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) + feature = self._get_feature(organization) + context["enabled"] = bool(feature) + context["days_limit"] = feature.value context["filter"] = self.filter context["AuditLog"] = AuditLog return context @@ -246,18 +247,17 @@ def _get_start_date(self): """Get the date to show logs from.""" organization = self.get_organization() creation_date = organization.pub_date.date() - retention_limit = self._get_retention_days_limit(organization) - if retention_limit in [None, -1]: - # Unlimited. + feature = self._get_feature(organization) + if feature.unlimited: return creation_date - start_date = timezone.now().date() - timezone.timedelta(days=retention_limit) + start_date = timezone.now().date() - timezone.timedelta(days=feature.value) # The max we can go back is to the creation of the organization. return max(start_date, creation_date) def _get_queryset(self): """Return the queryset without filters.""" organization = self.get_organization() - if not self._is_feature_enabled(organization): + if not self._get_feature(organization): return AuditLog.objects.none() start_date = self._get_start_date() queryset = AuditLog.objects.filter( @@ -286,15 +286,5 @@ def get_queryset(self): ) return self.filter.qs - 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, - ) - - def _is_feature_enabled(self, organization): - return PlanFeature.objects.has_feature( - organization, - type=self.feature_type, - ) + def _get_feature(self, organization): + return get_feature(organization, self.feature_type) diff --git a/readthedocs/projects/querysets.py b/readthedocs/projects/querysets.py index c64e772a97a..a5fcc53e855 100644 --- a/readthedocs/projects/querysets.py +++ b/readthedocs/projects/querysets.py @@ -5,6 +5,7 @@ from readthedocs.core.permissions import AdminPermission from readthedocs.core.utils.extend import SettingsOverrideObject from readthedocs.projects import constants +from readthedocs.subscriptions.products import get_feature class ProjectQuerySetBase(models.QuerySet): @@ -105,7 +106,6 @@ def max_concurrent_builds(self, 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() @@ -115,10 +115,10 @@ def max_concurrent_builds(self, project): return ( project.max_concurrent_builds or max_concurrent_organization - or PlanFeature.objects.get_feature_value( + or get_feature( project, type=TYPE_CONCURRENT_BUILDS, - ) + ).value ) def prefetch_latest_build(self): diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index fece00c1cc4..d91386d00e7 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -84,7 +84,7 @@ TYPE_PAGEVIEW_ANALYTICS, TYPE_SEARCH_ANALYTICS, ) -from readthedocs.subscriptions.models import PlanFeature +from readthedocs.subscriptions.products import get_feature log = structlog.get_logger(__name__) @@ -769,10 +769,7 @@ def get_context_data(self, **kwargs): return context def _is_enabled(self, project): - return PlanFeature.objects.has_feature( - project, - type=self.feature_type, - ) + return bool(get_feature(project, type=self.feature_type)) class DomainList(DomainMixin, ListViewWithForm): @@ -1080,7 +1077,7 @@ def get(self, request, *args, **kwargs): def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) project = self.get_project() - enabled = self._is_enabled(project) + enabled = bool(self._get_feature(project)) context.update({'enabled': enabled}) if not enabled: return context @@ -1115,12 +1112,11 @@ def _get_csv_data(self): """Generate raw csv data of search queries.""" project = self.get_project() now = timezone.now().date() - retention_limit = self._get_retention_days_limit(project) - if retention_limit in [None, -1]: - # Unlimited. + feature = self._get_feature(project) + if feature.unlimited: days_ago = project.pub_date.date() else: - days_ago = now - timezone.timedelta(days=retention_limit) + days_ago = now - timezone.timedelta(days=feature.value) values = [ ('Created Date', 'created'), @@ -1151,19 +1147,8 @@ def _get_csv_data(self): csv_data.insert(0, [header for header, _ in values]) return get_csv_file(filename=filename, csv_data=csv_data) - def _get_retention_days_limit(self, project): - """From how many days we need to show data for this project?""" - 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 PlanFeature.objects.has_feature( - project, - type=self.feature_type, - ) + def _get_feature(self, project): + return get_feature(project, type=self.feature_type) class TrafficAnalyticsView(ProjectAdminMixin, PrivateViewMixin, TemplateView): @@ -1181,7 +1166,7 @@ def get(self, request, *args, **kwargs): def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) project = self.get_project() - enabled = self._is_enabled(project) + enabled = bool(self._get_feature(project)) context.update({'enabled': enabled}) if not enabled: return context @@ -1217,12 +1202,12 @@ def get_context_data(self, **kwargs): def _get_csv_data(self): project = self.get_project() now = timezone.now().date() - retention_limit = self._get_retention_days_limit(project) - if retention_limit in [None, -1]: + feature = self._get_feature(project) + if feature.unlimited: # Unlimited. days_ago = project.pub_date.date() else: - days_ago = now - timezone.timedelta(days=retention_limit) + days_ago = now - timezone.timedelta(days=feature.value) values = [ ('Date', 'date'), @@ -1254,16 +1239,5 @@ def _get_csv_data(self): csv_data.insert(0, [header for header, _ in values]) return get_csv_file(filename=filename, csv_data=csv_data) - def _get_retention_days_limit(self, project): - """From how many days we need to show data for this project?""" - 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 PlanFeature.objects.has_feature( - project, - type=self.feature_type, - ) + def _get_feature(self, project): + return get_feature(project, type=self.feature_type) diff --git a/readthedocs/proxito/views/mixins.py b/readthedocs/proxito/views/mixins.py index 7dc65c4cd0b..93dc83b5072 100644 --- a/readthedocs/proxito/views/mixins.py +++ b/readthedocs/proxito/views/mixins.py @@ -26,7 +26,7 @@ from readthedocs.redirects.exceptions import InfiniteRedirectException from readthedocs.storage import build_media_storage, staticfiles_storage from readthedocs.subscriptions.constants import TYPE_AUDIT_PAGEVIEWS -from readthedocs.subscriptions.models import PlanFeature +from readthedocs.subscriptions.products import get_feature log = structlog.get_logger(__name__) @@ -215,7 +215,7 @@ def _is_audit_enabled(self, project): This feature is different from page views analytics, as it records every page view individually with more metadata like the user, IP, etc. """ - return PlanFeature.objects.has_feature(project, TYPE_AUDIT_PAGEVIEWS) + return bool(get_feature(project, type=TYPE_AUDIT_PAGEVIEWS)) def _serve_static_file(self, request, filename): return self._serve_file( diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index 415e261cb5a..3f3769199d1 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -164,18 +164,21 @@ def RTD_DEFAULT_FEATURES(self): # 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_EMBED_API: 1, + from readthedocs.subscriptions.products import RTDProductFeature + return dict(( + RTDProductFeature(type=constants.TYPE_CNAME).to_item(), + RTDProductFeature(type=constants.TYPE_EMBED_API).to_item(), # Retention days for search analytics. - constants.TYPE_SEARCH_ANALYTICS: self.RTD_ANALYTICS_DEFAULT_RETENTION_DAYS, + RTDProductFeature(type=constants.TYPE_SEARCH_ANALYTICS, value=self.RTD_ANALYTICS_DEFAULT_RETENTION_DAYS).to_item(), # Retention days for page view analytics. - constants.TYPE_PAGEVIEW_ANALYTICS: self.RTD_ANALYTICS_DEFAULT_RETENTION_DAYS, + RTDProductFeature(type=constants.TYPE_PAGEVIEW_ANALYTICS, value=self.RTD_ANALYTICS_DEFAULT_RETENTION_DAYS).to_item(), # Retention days for audit logs. - constants.TYPE_AUDIT_LOGS: self.RTD_AUDITLOGS_DEFAULT_RETENTION_DAYS, + RTDProductFeature(type=constants.TYPE_AUDIT_LOGS, value=self.RTD_AUDITLOGS_DEFAULT_RETENTION_DAYS).to_item(), # Max number of concurrent builds. - constants.TYPE_CONCURRENT_BUILDS: self.RTD_MAX_CONCURRENT_BUILDS, - } + RTDProductFeature(type=constants.TYPE_CONCURRENT_BUILDS, value=self.RTD_MAX_CONCURRENT_BUILDS).to_item(), + )) + + RTD_PRODUCTS = {} # Database and API hitting settings DONT_HIT_API = False diff --git a/readthedocs/subscriptions/managers.py b/readthedocs/subscriptions/managers.py index 3433fdef4e3..bba40eab98d 100644 --- a/readthedocs/subscriptions/managers.py +++ b/readthedocs/subscriptions/managers.py @@ -160,63 +160,3 @@ def _get_plan(self, stripe_price): stripe_price=stripe_price.id, ) return None - - -class PlanFeatureManager(models.Manager): - - """Model manager for PlanFeature.""" - - # pylint: disable=redefined-builtin - 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 (readthedocs.subscriptions.constants.TYPE_*). - :returns: A PlanFeature object or None. - """ - # Avoid circular imports. - from readthedocs.organizations.models import Organization - from readthedocs.projects.models import Project - - if isinstance(obj, Project): - organization = obj.organizations.first() - elif isinstance(obj, Organization): - organization = obj - else: - raise TypeError - - feature = self.filter( - feature_type=type, - plan__subscriptions__organization=organization, - ) - return feature.first() - - # pylint: disable=redefined-builtin - 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_DEFAULT_FEATURES`` setting. - """ - # 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 - 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_DEFAULT_FEATURES`` setting. - """ - # 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 diff --git a/readthedocs/subscriptions/models.py b/readthedocs/subscriptions/models.py index ba426e9b9d1..d6c0489d3ff 100644 --- a/readthedocs/subscriptions/models.py +++ b/readthedocs/subscriptions/models.py @@ -11,7 +11,7 @@ from readthedocs.core.utils import slugify from readthedocs.organizations.models import Organization from readthedocs.subscriptions.constants import FEATURE_TYPES -from readthedocs.subscriptions.managers import PlanFeatureManager, SubscriptionManager +from readthedocs.subscriptions.managers import SubscriptionManager class Plan(models.Model): @@ -102,7 +102,6 @@ class Meta: null=True, blank=True, ) - objects = PlanFeatureManager() def __str__(self): return '{plan} feature: {feature}'.format( diff --git a/readthedocs/subscriptions/products.py b/readthedocs/subscriptions/products.py new file mode 100644 index 00000000000..8bcbed9c791 --- /dev/null +++ b/readthedocs/subscriptions/products.py @@ -0,0 +1,75 @@ +from dataclasses import dataclass + +from django.conf import settings + +from readthedocs.subscriptions.constants import FEATURE_TYPES + + +@dataclass(slots=True) +class RTDProductFeature: + type: str + value: int = 0 + unlimited: bool = False + + @property + def description(self): + return dict(FEATURE_TYPES).get(self.type) + + def to_item(self): + return (self.type, self) + + +@dataclass(slots=True) +class RTDProduct: + stripe_id: str + listed: bool + features: dict[str, RTDProductFeature] + + def to_item(self): + return (self.stripe_id, self) + + +def get_listed_products(): + return [product for product in settings.RTD_PRODUCTS.values() if product.listed] + + +def get_products_with_feature(feature): + return [ + product + for product in settings.RTD_PRODUCTS.values() + if feature in product.features + ] + + +def get_feature(obj, type) -> RTDProductFeature: + # Hit the DB only if subscriptions are enabled. + if settings.RTD_PRODUCTS: + from djstripe import models as djstripe + + from readthedocs.organizations.models import Organization + from readthedocs.projects.models import Project + + if isinstance(obj, Project): + organization = obj.organizations.first() + elif isinstance(obj, Organization): + organization = obj + else: + raise TypeError + + # A subscription can have multiple products, but we only want + # the product from the organization that has the feature we are looking for. + stripe_products_id = [ + product.stripe_id for product in get_products_with_feature(type) + ] + stripe_product_id = ( + djstripe.Product.objects.filter( + id__in=stripe_products_id, + prices__subscription_items__subscription__rtd_organization=organization, + ).values_list("id", flat=True) + # TODO: maybe we should merge features from multiple products? + .first() + ) + if stripe_product_id is not None: + return settings.RTD_PRODUCTS[stripe_product_id].features[type] + + return settings.RTD_DEFAULT_FEATURES.get(type) From 703cd655350e12d11a12844720742ddecab07fe0 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 12 Apr 2023 15:10:55 -0500 Subject: [PATCH 02/35] Fix tests --- .../builds/tests/test_build_queryset.py | 7 ++-- readthedocs/organizations/tests/test_views.py | 7 ++-- readthedocs/projects/views/private.py | 4 +- readthedocs/proxito/tests/test_full.py | 37 +++++++++++++------ readthedocs/proxito/tests/test_middleware.py | 5 +-- readthedocs/proxito/tests/test_redirects.py | 5 +-- readthedocs/rtd_tests/tests/test_api.py | 7 ++-- readthedocs/rtd_tests/tests/test_footer.py | 5 +-- .../rtd_tests/tests/test_middleware.py | 5 +-- readthedocs/rtd_tests/tests/test_resolver.py | 5 +-- readthedocs/rtd_tests/tests/test_views.py | 7 ++-- 11 files changed, 54 insertions(+), 40 deletions(-) diff --git a/readthedocs/builds/tests/test_build_queryset.py b/readthedocs/builds/tests/test_build_queryset.py index 686701306f2..9b446e56104 100644 --- a/readthedocs/builds/tests/test_build_queryset.py +++ b/readthedocs/builds/tests/test_build_queryset.py @@ -5,15 +5,16 @@ from readthedocs.organizations.models import Organization from readthedocs.projects.models import Project from readthedocs.subscriptions.constants import TYPE_CONCURRENT_BUILDS +from readthedocs.subscriptions.products import RTDProductFeature @pytest.mark.django_db class TestBuildQuerySet: @pytest.fixture(autouse=True) def setup_method(self, settings): - settings.RTD_DEFAULT_FEATURES = { - TYPE_CONCURRENT_BUILDS: 4, - } + settings.RTD_DEFAULT_FEATURES = dict( + [RTDProductFeature(type=TYPE_CONCURRENT_BUILDS, value=4).to_item()] + ) 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 6a10aa8fd96..a23dab2a23f 100644 --- a/readthedocs/organizations/tests/test_views.py +++ b/readthedocs/organizations/tests/test_views.py @@ -17,6 +17,7 @@ from readthedocs.projects.models import Project from readthedocs.rtd_tests.base import RequestFactoryTestMixin from readthedocs.subscriptions.constants import TYPE_AUDIT_LOGS +from readthedocs.subscriptions.products import RTDProductFeature @override_settings(RTD_ALLOW_ORGANIZATIONS=True) @@ -150,9 +151,9 @@ def test_add_owner(self): @override_settings( RTD_ALLOW_ORGANIZATIONS=True, - RTD_DEFAULT_FEATURES={ - TYPE_AUDIT_LOGS: 90, - }, + RTD_DEFAULT_FEATURES=dict( + [RTDProductFeature(type=TYPE_AUDIT_LOGS, value=90).to_item()] + ), ) class OrganizationSecurityLogTests(TestCase): diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index d91386d00e7..1650eec754e 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -1124,7 +1124,7 @@ def _get_csv_data(self): ('Total Results', 'total_results'), ] data = [] - if self._is_enabled(project): + if feature: data = ( SearchQuery.objects.filter( project=project, @@ -1216,7 +1216,7 @@ def _get_csv_data(self): ('Views', 'view_count'), ] data = [] - if self._is_enabled(project): + if feature: data = ( PageView.objects.filter( project=project, diff --git a/readthedocs/proxito/tests/test_full.py b/readthedocs/proxito/tests/test_full.py index 06169da2de2..758de118add 100644 --- a/readthedocs/proxito/tests/test_full.py +++ b/readthedocs/proxito/tests/test_full.py @@ -32,6 +32,7 @@ StaticFileSystemStorageTest, ) from readthedocs.subscriptions.constants import TYPE_AUDIT_PAGEVIEWS, TYPE_CNAME +from readthedocs.subscriptions.products import RTDProductFeature from .base import BaseDocServing @@ -616,9 +617,13 @@ def test_track_html_files_only(self): self.assertIn('x-accel-redirect', resp) self.assertEqual(AuditLog.objects.all().count(), 0) - url = '/en/latest/awesome.html' - host = 'project.dev.readthedocs.io' - with override_settings(RTD_DEFAULT_FEATURES={TYPE_AUDIT_PAGEVIEWS: 1}): + url = "/en/latest/awesome.html" + host = "project.dev.readthedocs.io" + with override_settings( + RTD_DEFAULT_FEATURES=dict( + [RTDProductFeature(type=TYPE_AUDIT_PAGEVIEWS).to_item()] + ) + ): resp = self.client.get(url, HTTP_HOST=host) self.assertIn('x-accel-redirect', resp) self.assertEqual(AuditLog.objects.all().count(), 1) @@ -629,11 +634,19 @@ def test_track_html_files_only(self): self.assertEqual(log.resource, url) self.assertEqual(log.action, AuditLog.PAGEVIEW) - with override_settings(RTD_DEFAULT_FEATURES={TYPE_AUDIT_PAGEVIEWS: 1}): + with override_settings( + RTD_DEFAULT_FEATURES=dict( + [RTDProductFeature(type=TYPE_AUDIT_PAGEVIEWS).to_item()] + ) + ): resp = self.client.get("/en/latest/awesome.js", HTTP_HOST=host) self.assertIn("x-accel-redirect", resp) - with override_settings(RTD_DEFAULT_FEATURES={TYPE_AUDIT_PAGEVIEWS: 1}): + with override_settings( + RTD_DEFAULT_FEATURES=dict( + [RTDProductFeature(type=TYPE_AUDIT_PAGEVIEWS).to_item()] + ) + ): resp = self.client.get("/en/latest/awesome.css", HTTP_HOST=host) self.assertIn("x-accel-redirect", resp) self.assertEqual(AuditLog.objects.all().count(), 1) @@ -646,9 +659,13 @@ def test_track_downloads(self): ) self.assertEqual(AuditLog.objects.all().count(), 0) - url = '/_/downloads/en/latest/pdf/' - host = 'project.dev.readthedocs.io' - with override_settings(RTD_DEFAULT_FEATURES={TYPE_AUDIT_PAGEVIEWS: 1}): + url = "/_/downloads/en/latest/pdf/" + host = "project.dev.readthedocs.io" + with override_settings( + RTD_DEFAULT_FEATURES=dict( + [RTDProductFeature(type=TYPE_AUDIT_PAGEVIEWS).to_item()] + ) + ): resp = self.client.get(url, HTTP_HOST=host) self.assertIn('x-accel-redirect', resp) self.assertEqual(AuditLog.objects.all().count(), 1) @@ -1511,9 +1528,7 @@ def test_404_download(self): ALLOW_PRIVATE_REPOS=True, PUBLIC_DOMAIN='dev.readthedocs.io', PUBLIC_DOMAIN_USES_HTTPS=True, - RTD_DEFAULT_FEATURES={ - TYPE_CNAME: 1, - }, + RTD_DEFAULT_FEATURES=dict([RTDProductFeature(type=TYPE_CNAME).to_item()]), ) # We are overriding the storage class instead of using RTD_BUILD_MEDIA_STORAGE, # since the setting is evaluated just once (first test to use the storage diff --git a/readthedocs/proxito/tests/test_middleware.py b/readthedocs/proxito/tests/test_middleware.py index 858344e7d34..53b0d035291 100644 --- a/readthedocs/proxito/tests/test_middleware.py +++ b/readthedocs/proxito/tests/test_middleware.py @@ -17,14 +17,13 @@ from readthedocs.rtd_tests.storage import BuildMediaFileSystemStorageTest from readthedocs.rtd_tests.utils import create_user from readthedocs.subscriptions.constants import TYPE_CNAME +from readthedocs.subscriptions.products import RTDProductFeature @pytest.mark.proxito @override_settings( PUBLIC_DOMAIN="dev.readthedocs.io", - RTD_DEFAULT_FEATURES={ - TYPE_CNAME: 1, - }, + RTD_DEFAULT_FEATURES=dict([RTDProductFeature(type=TYPE_CNAME).to_item()]), ) class MiddlewareTests(RequestFactoryTestMixin, TestCase): diff --git a/readthedocs/proxito/tests/test_redirects.py b/readthedocs/proxito/tests/test_redirects.py index 2518228f998..fc8102f1fea 100644 --- a/readthedocs/proxito/tests/test_redirects.py +++ b/readthedocs/proxito/tests/test_redirects.py @@ -5,6 +5,7 @@ from readthedocs.projects.models import Feature from readthedocs.proxito.constants import RedirectType from readthedocs.subscriptions.constants import TYPE_CNAME +from readthedocs.subscriptions.products import RTDProductFeature from .base import BaseDocServing @@ -12,9 +13,7 @@ @override_settings( PUBLIC_DOMAIN='dev.readthedocs.io', PUBLIC_DOMAIN_USES_HTTPS=True, - RTD_DEFAULT_FEATURES={ - TYPE_CNAME: 1, - }, + RTD_DEFAULT_FEATURES=dict([RTDProductFeature(type=TYPE_CNAME).to_item()]), ) class RedirectTests(BaseDocServing): diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index 294a70f496f..2816d6ef7f8 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -61,6 +61,7 @@ Project, ) from readthedocs.subscriptions.constants import TYPE_CONCURRENT_BUILDS +from readthedocs.subscriptions.products import RTDProductFeature super_auth = base64.b64encode(b'super:test').decode('utf-8') eric_auth = base64.b64encode(b'eric:test').decode('utf-8') @@ -847,9 +848,9 @@ def test_init_api_project(self): ) @override_settings( - RTD_DEFAULT_FEATURES={ - TYPE_CONCURRENT_BUILDS: 4, - } + RTD_DEFAULT_FEATURES=dict( + [RTDProductFeature(type=TYPE_CONCURRENT_BUILDS, value=4).to_item()] + ), ) def test_concurrent_builds(self): expected = { diff --git a/readthedocs/rtd_tests/tests/test_footer.py b/readthedocs/rtd_tests/tests/test_footer.py index 5ada66df22c..39371a9f3e2 100644 --- a/readthedocs/rtd_tests/tests/test_footer.py +++ b/readthedocs/rtd_tests/tests/test_footer.py @@ -14,6 +14,7 @@ from readthedocs.projects.constants import GITHUB_BRAND, GITLAB_BRAND, PUBLIC from readthedocs.projects.models import Project from readthedocs.subscriptions.constants import TYPE_CNAME +from readthedocs.subscriptions.products import RTDProductFeature class BaseTestFooterHTML: @@ -447,9 +448,7 @@ def test_highest_version_without_tags(self): @pytest.mark.proxito @override_settings( PUBLIC_DOMAIN="readthedocs.io", - RTD_DEFAULT_FEATURES={ - TYPE_CNAME: 1, - }, + RTD_DEFAULT_FEATURES=dict([RTDProductFeature(type=TYPE_CNAME).to_item()]), ) class TestFooterPerformance(TestCase): # The expected number of queries for generating the footer diff --git a/readthedocs/rtd_tests/tests/test_middleware.py b/readthedocs/rtd_tests/tests/test_middleware.py index 7fcf4bef1b1..bab178e746d 100644 --- a/readthedocs/rtd_tests/tests/test_middleware.py +++ b/readthedocs/rtd_tests/tests/test_middleware.py @@ -17,13 +17,12 @@ from readthedocs.projects.models import Domain, Project, ProjectRelationship from readthedocs.rtd_tests.utils import create_user from readthedocs.subscriptions.constants import TYPE_EMBED_API +from readthedocs.subscriptions.products import RTDProductFeature @override_settings( PUBLIC_DOMAIN='readthedocs.io', - RTD_DEFAULT_FEATURES={ - TYPE_EMBED_API: 1, - }, + RTD_DEFAULT_FEATURES=dict([RTDProductFeature(type=TYPE_EMBED_API).to_item()]), ) class TestCORSMiddleware(TestCase): diff --git a/readthedocs/rtd_tests/tests/test_resolver.py b/readthedocs/rtd_tests/tests/test_resolver.py index 52a103971a4..e369b080543 100644 --- a/readthedocs/rtd_tests/tests/test_resolver.py +++ b/readthedocs/rtd_tests/tests/test_resolver.py @@ -8,13 +8,12 @@ from readthedocs.projects.models import Domain, Project, ProjectRelationship from readthedocs.rtd_tests.utils import create_user from readthedocs.subscriptions.constants import TYPE_CNAME +from readthedocs.subscriptions.products import RTDProductFeature @override_settings( PUBLIC_DOMAIN="readthedocs.org", - RTD_DEFAULT_FEATURES={ - TYPE_CNAME: 1, - }, + RTD_DEFAULT_FEATURES=dict([RTDProductFeature(type=TYPE_CNAME).to_item()]), ) class ResolverBase(TestCase): diff --git a/readthedocs/rtd_tests/tests/test_views.py b/readthedocs/rtd_tests/tests/test_views.py index 097e217a4db..e47ad8c7556 100644 --- a/readthedocs/rtd_tests/tests/test_views.py +++ b/readthedocs/rtd_tests/tests/test_views.py @@ -16,6 +16,7 @@ from readthedocs.projects.forms import UpdateProjectForm from readthedocs.projects.models import Feature, Project from readthedocs.subscriptions.constants import TYPE_SEARCH_ANALYTICS +from readthedocs.subscriptions.products import RTDProductFeature @mock.patch('readthedocs.projects.forms.trigger_build', mock.MagicMock()) @@ -335,9 +336,9 @@ def test_rebuild_invalid_specific_commit(self, mock): @override_settings( - RTD_DEFAULT_FEATURES={ - TYPE_SEARCH_ANALYTICS: 90, - } + RTD_DEFAULT_FEATURES=dict( + [RTDProductFeature(type=TYPE_SEARCH_ANALYTICS, value=90).to_item()] + ), ) class TestSearchAnalyticsView(TestCase): From efde180529a1c19e74fe3376811ec3ac898b104f Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 12 Apr 2023 16:42:26 -0500 Subject: [PATCH 03/35] Updates --- readthedocs/organizations/views/private.py | 2 +- readthedocs/projects/views/private.py | 39 +++++++++---------- readthedocs/settings/base.py | 3 ++ readthedocs/subscriptions/products.py | 36 ++++++++++++++++- .../projects/project_traffic_analytics.html | 2 +- .../projects/projects_search_analytics.html | 2 +- 6 files changed, 59 insertions(+), 25 deletions(-) diff --git a/readthedocs/organizations/views/private.py b/readthedocs/organizations/views/private.py index bddb8414333..736bea8ed06 100644 --- a/readthedocs/organizations/views/private.py +++ b/readthedocs/organizations/views/private.py @@ -238,7 +238,7 @@ def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) feature = self._get_feature(organization) context["enabled"] = bool(feature) - context["days_limit"] = feature.value + context["days_limit"] = feature.value if feature else 0 context["filter"] = self.filter context["AuditLog"] = AuditLog return context diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index 1650eec754e..c300e23d8bc 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -1113,6 +1113,8 @@ def _get_csv_data(self): project = self.get_project() now = timezone.now().date() feature = self._get_feature(project) + if not feature: + raise Http404 if feature.unlimited: days_ago = project.pub_date.date() else: @@ -1123,16 +1125,14 @@ def _get_csv_data(self): ('Query', 'query'), ('Total Results', 'total_results'), ] - data = [] - if feature: - data = ( - SearchQuery.objects.filter( - project=project, - created__date__gte=days_ago, - ) - .order_by('-created') - .values_list(*[value for _, value in values]) + data = ( + SearchQuery.objects.filter( + project=project, + created__date__gte=days_ago, ) + .order_by("-created") + .values_list(*[value for _, value in values]) + ) filename = 'readthedocs_search_analytics_{project_slug}_{start}_{end}.csv'.format( project_slug=project.slug, @@ -1203,8 +1203,9 @@ def _get_csv_data(self): project = self.get_project() now = timezone.now().date() feature = self._get_feature(project) + if not feature: + raise Http404 if feature.unlimited: - # Unlimited. days_ago = project.pub_date.date() else: days_ago = now - timezone.timedelta(days=feature.value) @@ -1215,17 +1216,15 @@ def _get_csv_data(self): ('Path', 'path'), ('Views', 'view_count'), ] - data = [] - if feature: - data = ( - PageView.objects.filter( - project=project, - date__gte=days_ago, - status=200, - ) - .order_by('-date') - .values_list(*[value for _, value in values]) + data = ( + PageView.objects.filter( + project=project, + date__gte=days_ago, + status=200, ) + .order_by("-date") + .values_list(*[value for _, value in values]) + ) filename = 'readthedocs_traffic_analytics_{project_slug}_{start}_{end}.csv'.format( project_slug=project.slug, diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index 3f3769199d1..c530abaa3ea 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -178,6 +178,9 @@ def RTD_DEFAULT_FEATURES(self): RTDProductFeature(type=constants.TYPE_CONCURRENT_BUILDS, value=self.RTD_MAX_CONCURRENT_BUILDS).to_item(), )) + # A dictionary of Stripe products mapped to a RTDProduct object. + # In .org we don't have subscriptions/products, default features are + # defined in RTD_DEFAULT_FEATURES. RTD_PRODUCTS = {} # Database and API hitting settings diff --git a/readthedocs/subscriptions/products.py b/readthedocs/subscriptions/products.py index 8bcbed9c791..68a73c27245 100644 --- a/readthedocs/subscriptions/products.py +++ b/readthedocs/subscriptions/products.py @@ -1,3 +1,9 @@ +""" +Read the Docs products. + +This module contains the dataclasses that represent the products available. +The available products are defined in the ``RTD_PRODUCTS`` setting. +""" from dataclasses import dataclass from django.conf import settings @@ -7,8 +13,11 @@ @dataclass(slots=True) class RTDProductFeature: + # readthedocs.subscriptions.constants.TYPE_* type: str + # Depending on the feature type, this number can represent a numeric limit, number of days, etc. value: int = 0 + # If this feature is unlimited for this product. unlimited: bool = False @property @@ -16,24 +25,39 @@ def description(self): return dict(FEATURE_TYPES).get(self.type) def to_item(self): - return (self.type, self) + """ + Return a tuple with the feature type and the feature itself. + + Useful to use it as a dictionary item. + """ + return self.type, self @dataclass(slots=True) class RTDProduct: + """A local representation of a Stripe product.""" + stripe_id: str + # If this product should be available to users to purchase. listed: bool features: dict[str, RTDProductFeature] def to_item(self): - return (self.stripe_id, self) + """ + Return a tuple with the stripe_id and the product itself. + + Useful to use it as a dictionary item. + """ + return self.stripe_id, self def get_listed_products(): + """Return a list of products that are available to users to purchase.""" return [product for product in settings.RTD_PRODUCTS.values() if product.listed] def get_products_with_feature(feature): + """Return a list of products that have the given feature.""" return [ product for product in settings.RTD_PRODUCTS.values() @@ -42,6 +66,14 @@ def get_products_with_feature(feature): def get_feature(obj, type) -> RTDProductFeature: + """ + Get the feature object for the given type of the object. + + If the object doesn't have the feature, return the default feature or None. + + :param obj: An organization or project instance. + :param type: The type of the feature (readthedocs.subscriptions.constants.TYPE_*). + """ # Hit the DB only if subscriptions are enabled. if settings.RTD_PRODUCTS: from djstripe import models as djstripe diff --git a/readthedocs/templates/projects/project_traffic_analytics.html b/readthedocs/templates/projects/project_traffic_analytics.html index 245b4f4d56b..078719a0ca9 100644 --- a/readthedocs/templates/projects/project_traffic_analytics.html +++ b/readthedocs/templates/projects/project_traffic_analytics.html @@ -45,7 +45,7 @@

{% trans "Daily pageview totals" %}

- +
{% if track_404 %} diff --git a/readthedocs/templates/projects/projects_search_analytics.html b/readthedocs/templates/projects/projects_search_analytics.html index 5c43f66f4bd..de801d87103 100644 --- a/readthedocs/templates/projects/projects_search_analytics.html +++ b/readthedocs/templates/projects/projects_search_analytics.html @@ -49,7 +49,7 @@

{% trans "Daily search totals" %}


- +
{% endblock %} From d20756588e4aecc08e10e9cf1a51ee5178fcb968 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 12 Apr 2023 17:02:11 -0500 Subject: [PATCH 04/35] Linter --- readthedocs/subscriptions/products.py | 1 + 1 file changed, 1 insertion(+) diff --git a/readthedocs/subscriptions/products.py b/readthedocs/subscriptions/products.py index 68a73c27245..5a5ce998df0 100644 --- a/readthedocs/subscriptions/products.py +++ b/readthedocs/subscriptions/products.py @@ -35,6 +35,7 @@ def to_item(self): @dataclass(slots=True) class RTDProduct: + """A local representation of a Stripe product.""" stripe_id: str From b7f9c85a10736b8da6c34686bceb39b15539ef47 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 12 Apr 2023 18:49:23 -0500 Subject: [PATCH 05/35] More updates --- readthedocs/api/v3/permissions.py | 2 +- readthedocs/core/resolver.py | 2 +- readthedocs/projects/querysets.py | 2 +- readthedocs/projects/views/private.py | 6 +-- readthedocs/proxito/views/mixins.py | 2 +- readthedocs/subscriptions/constants.py | 3 -- readthedocs/subscriptions/forms.py | 23 ++++++---- readthedocs/subscriptions/products.py | 23 ++++++---- .../subscriptions/subscription_detail.html | 4 +- readthedocs/subscriptions/tests/test_views.py | 43 +++++++++++++++---- readthedocs/subscriptions/views.py | 15 +++---- 11 files changed, 77 insertions(+), 48 deletions(-) diff --git a/readthedocs/api/v3/permissions.py b/readthedocs/api/v3/permissions.py index da8419c798b..e3da34db30e 100644 --- a/readthedocs/api/v3/permissions.py +++ b/readthedocs/api/v3/permissions.py @@ -24,7 +24,7 @@ class HasEmbedAPIAccess(BasePermission): def has_permission(self, request, view): project = view._get_project() # The project is None when the is requesting a section from an external site. - if project and not get_feature(project, type=TYPE_EMBED_API): + if project and not get_feature(project, feature_type=TYPE_EMBED_API): return False return True diff --git a/readthedocs/core/resolver.py b/readthedocs/core/resolver.py index 0f4c5f9c911..234aef2d23f 100644 --- a/readthedocs/core/resolver.py +++ b/readthedocs/core/resolver.py @@ -406,7 +406,7 @@ def _use_subdomain(self): def _use_cname(self, project): """Test if to allow direct serving for project on CNAME.""" - return bool(get_feature(project, type=TYPE_CNAME)) + return bool(get_feature(project, feature_type=TYPE_CNAME)) class Resolver(SettingsOverrideObject): diff --git a/readthedocs/projects/querysets.py b/readthedocs/projects/querysets.py index a5fcc53e855..837032a0d20 100644 --- a/readthedocs/projects/querysets.py +++ b/readthedocs/projects/querysets.py @@ -117,7 +117,7 @@ def max_concurrent_builds(self, project): or max_concurrent_organization or get_feature( project, - type=TYPE_CONCURRENT_BUILDS, + feature_type=TYPE_CONCURRENT_BUILDS, ).value ) diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index c300e23d8bc..30d2aab653a 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -769,7 +769,7 @@ def get_context_data(self, **kwargs): return context def _is_enabled(self, project): - return bool(get_feature(project, type=self.feature_type)) + return bool(get_feature(project, feature_type=self.feature_type)) class DomainList(DomainMixin, ListViewWithForm): @@ -1148,7 +1148,7 @@ def _get_csv_data(self): return get_csv_file(filename=filename, csv_data=csv_data) def _get_feature(self, project): - return get_feature(project, type=self.feature_type) + return get_feature(project, feature_type=self.feature_type) class TrafficAnalyticsView(ProjectAdminMixin, PrivateViewMixin, TemplateView): @@ -1239,4 +1239,4 @@ def _get_csv_data(self): return get_csv_file(filename=filename, csv_data=csv_data) def _get_feature(self, project): - return get_feature(project, type=self.feature_type) + return get_feature(project, feature_type=self.feature_type) diff --git a/readthedocs/proxito/views/mixins.py b/readthedocs/proxito/views/mixins.py index 93dc83b5072..c2e48bf1676 100644 --- a/readthedocs/proxito/views/mixins.py +++ b/readthedocs/proxito/views/mixins.py @@ -215,7 +215,7 @@ def _is_audit_enabled(self, project): This feature is different from page views analytics, as it records every page view individually with more metadata like the user, IP, etc. """ - return bool(get_feature(project, type=TYPE_AUDIT_PAGEVIEWS)) + return bool(get_feature(project, feature_type=TYPE_AUDIT_PAGEVIEWS)) def _serve_static_file(self, request, filename): return self._serve_file( diff --git a/readthedocs/subscriptions/constants.py b/readthedocs/subscriptions/constants.py index d6dd62782d4..900a054372c 100644 --- a/readthedocs/subscriptions/constants.py +++ b/readthedocs/subscriptions/constants.py @@ -4,9 +4,6 @@ # 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" diff --git a/readthedocs/subscriptions/forms.py b/readthedocs/subscriptions/forms.py index 64f3492ef19..1814d14afd9 100644 --- a/readthedocs/subscriptions/forms.py +++ b/readthedocs/subscriptions/forms.py @@ -1,26 +1,31 @@ """Subscriptions forms.""" from django import forms -from django.urls import reverse from django.utils.translation import gettext_lazy as _ +from djstripe import models as djstripe -from readthedocs.subscriptions.models import Plan +from readthedocs.subscriptions.products import get_listed_products class PlanForm(forms.Form): """Form to create a subscription after the previous one has ended.""" - plan = forms.ChoiceField() + price = forms.ChoiceField() def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.fields['plan'].choices = [ - (plan.pk, f'{plan.name} (${plan.price})') - for plan in Plan.objects.filter(published=True).order_by('price') + products_id = [product.stripe_id for product in get_listed_products()] + stripe_prices = ( + djstripe.Price.objects.filter(product__id__in=products_id) + .select_related("product") + .order_by("unit_amount") + ) + self.fields["price"].choices = [ + (price.pk, f"{price.product.name} ({price.human_readable_price})") + for price in stripe_prices ] - pricing_page = reverse('pricing') - self.fields['plan'].help_text = _( - f'Check our pricing page ' + self.fields["price"].help_text = _( + f'Check our pricing page ' 'for more information about each plan.' ) diff --git a/readthedocs/subscriptions/products.py b/readthedocs/subscriptions/products.py index 5a5ce998df0..96b3357d4d0 100644 --- a/readthedocs/subscriptions/products.py +++ b/readthedocs/subscriptions/products.py @@ -19,10 +19,10 @@ class RTDProductFeature: value: int = 0 # If this feature is unlimited for this product. unlimited: bool = False + description: str = "" - @property - def description(self): - return dict(FEATURE_TYPES).get(self.type) + def get_description(self): + return self.description or dict(FEATURE_TYPES).get(self.type) def to_item(self): """ @@ -52,21 +52,26 @@ def to_item(self): return self.stripe_id, self +def get_product(stripe_id): + """Return the product with the given stripe_id.""" + return settings.RTD_PRODUCTS.get(stripe_id) + + def get_listed_products(): """Return a list of products that are available to users to purchase.""" return [product for product in settings.RTD_PRODUCTS.values() if product.listed] -def get_products_with_feature(feature): +def get_products_with_feature(feature_type): """Return a list of products that have the given feature.""" return [ product for product in settings.RTD_PRODUCTS.values() - if feature in product.features + if feature_type in product.features ] -def get_feature(obj, type) -> RTDProductFeature: +def get_feature(obj, feature_type) -> RTDProductFeature: """ Get the feature object for the given type of the object. @@ -92,7 +97,7 @@ def get_feature(obj, type) -> RTDProductFeature: # A subscription can have multiple products, but we only want # the product from the organization that has the feature we are looking for. stripe_products_id = [ - product.stripe_id for product in get_products_with_feature(type) + product.stripe_id for product in get_products_with_feature(feature_type) ] stripe_product_id = ( djstripe.Product.objects.filter( @@ -103,6 +108,6 @@ def get_feature(obj, type) -> RTDProductFeature: .first() ) if stripe_product_id is not None: - return settings.RTD_PRODUCTS[stripe_product_id].features[type] + return settings.RTD_PRODUCTS[stripe_product_id].features[feature_type] - return settings.RTD_DEFAULT_FEATURES.get(type) + return settings.RTD_DEFAULT_FEATURES.get(feature_type) diff --git a/readthedocs/subscriptions/templates/subscriptions/subscription_detail.html b/readthedocs/subscriptions/templates/subscriptions/subscription_detail.html index 2bd51d35250..8171296d387 100644 --- a/readthedocs/subscriptions/templates/subscriptions/subscription_detail.html +++ b/readthedocs/subscriptions/templates/subscriptions/subscription_detail.html @@ -42,8 +42,8 @@
{% trans "Plan features:" %}
{% for feature in features %} - - {{ feature.description_display }} + + {{ feature.get_description }} {% endfor %}
diff --git a/readthedocs/subscriptions/tests/test_views.py b/readthedocs/subscriptions/tests/test_views.py index 61d280cab7f..db4246bcc0b 100644 --- a/readthedocs/subscriptions/tests/test_views.py +++ b/readthedocs/subscriptions/tests/test_views.py @@ -11,10 +11,29 @@ from djstripe.enums import SubscriptionStatus from readthedocs.organizations.models import Organization +from readthedocs.subscriptions.constants import TYPE_PRIVATE_DOCS from readthedocs.subscriptions.models import Plan, Subscription - - -@override_settings(RTD_ALLOW_ORGANIZATIONS=True) +from readthedocs.subscriptions.products import RTDProduct, RTDProductFeature + + +@override_settings( + RTD_ALLOW_ORGANIZATIONS=True, + RTD_PRODUCTS=dict( + ( + RTDProduct( + stripe_id="prod_a1b2c3", + listed=True, + features=dict( + ( + RTDProductFeature( + type=TYPE_PRIVATE_DOCS, + ).to_item(), + ) + ), + ).to_item(), + ) + ), +) class SubscriptionViewTests(TestCase): """Subscription view tests.""" @@ -27,6 +46,16 @@ def setUp(self): published=True, stripe_id=settings.RTD_ORG_DEFAULT_STRIPE_SUBSCRIPTION_PRICE, ) + self.stripe_product = get( + djstripe.Product, + id="prod_a1b2c3", + ) + self.stripe_price = get( + djstripe.Price, + id=settings.RTD_ORG_DEFAULT_STRIPE_SUBSCRIPTION_PRICE, + unit_amount=50000, + product=self.stripe_product, + ) self.stripe_subscription = self._create_stripe_subscription( customer_id=self.organization.stripe_id, subscription_id="sub_a1b2c3d4", @@ -60,13 +89,9 @@ def _create_stripe_subscription( status=SubscriptionStatus.active, customer=stripe_customer, ) - stripe_price = get( - djstripe.Price, - unit_amount=50000, - ) - stripe_item = get( + get( djstripe.SubscriptionItem, - price=stripe_price, + price=self.stripe_price, subscription=stripe_subscription, ) return stripe_subscription diff --git a/readthedocs/subscriptions/views.py b/readthedocs/subscriptions/views.py index a49d74aae2e..05172d4e79c 100644 --- a/readthedocs/subscriptions/views.py +++ b/readthedocs/subscriptions/views.py @@ -15,7 +15,7 @@ from readthedocs.organizations.views.base import OrganizationMixin from readthedocs.subscriptions.forms import PlanForm -from readthedocs.subscriptions.models import Plan +from readthedocs.subscriptions.products import get_product from readthedocs.subscriptions.utils import get_or_create_stripe_customer log = structlog.get_logger(__name__) @@ -66,7 +66,7 @@ def redirect_to_checkout(self, form): ): raise Http404() - plan = get_object_or_404(Plan, id=form.cleaned_data['plan']) + stripe_price = get_object_or_404(djstripe.Price, id=form.cleaned_data["price"]) url = self.request.build_absolute_uri(self.get_success_url()) organization = self.get_organization() @@ -78,8 +78,8 @@ def redirect_to_checkout(self, form): payment_method_types=['card'], line_items=[ { - 'price': plan.stripe_id, - 'quantity': 1, + "price": stripe_price.id, + "quantity": 1, } ], mode='subscription', @@ -91,7 +91,7 @@ def redirect_to_checkout(self, form): log.exception( 'Error while creating a Stripe checkout session.', organization_slug=organization.slug, - plan_slug=plan.slug, + price=stripe_price.id, ) messages.error( self.request, @@ -115,12 +115,9 @@ def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) stripe_subscription = self.get_object() if stripe_subscription: - context[ - "features" - ] = self.get_organization().subscription.plan.features.all() - stripe_price = stripe_subscription.items.first().price context["stripe_price"] = stripe_price + context["features"] = get_product(stripe_price.product.id).features # When Stripe marks the subscription as ``past_due``, # it means the usage of RTD service for the current period/month was not paid at all. From 4e11f6f0eca00d3c214eb49c773fb33d005fd62b Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 12 Apr 2023 19:20:20 -0500 Subject: [PATCH 06/35] Linter --- readthedocs/subscriptions/forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/subscriptions/forms.py b/readthedocs/subscriptions/forms.py index 1814d14afd9..fc38b43f738 100644 --- a/readthedocs/subscriptions/forms.py +++ b/readthedocs/subscriptions/forms.py @@ -26,6 +26,6 @@ def __init__(self, *args, **kwargs): for price in stripe_prices ] self.fields["price"].help_text = _( - f'Check our pricing page ' + 'Check our pricing page ' 'for more information about each plan.' ) From 27f9003081d3d2f15b80f424aeb02fd55e3fa6ff Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 12 Apr 2023 20:26:46 -0500 Subject: [PATCH 07/35] Default --- readthedocs/projects/querysets.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/readthedocs/projects/querysets.py b/readthedocs/projects/querysets.py index 837032a0d20..4d55f9b2729 100644 --- a/readthedocs/projects/querysets.py +++ b/readthedocs/projects/querysets.py @@ -112,13 +112,12 @@ def max_concurrent_builds(self, project): if organization: max_concurrent_organization = organization.max_concurrent_builds + feature = get_feature(project, feature_type=TYPE_CONCURRENT_BUILDS) + feature_value = feature.value if feature else 1 return ( project.max_concurrent_builds or max_concurrent_organization - or get_feature( - project, - feature_type=TYPE_CONCURRENT_BUILDS, - ).value + or feature_value ) def prefetch_latest_build(self): From 2aa04f8c97085f095cd3cd13952a135611e61989 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 13 Apr 2023 11:32:37 -0500 Subject: [PATCH 08/35] Default listed to False --- readthedocs/subscriptions/products.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/readthedocs/subscriptions/products.py b/readthedocs/subscriptions/products.py index 96b3357d4d0..47337930fed 100644 --- a/readthedocs/subscriptions/products.py +++ b/readthedocs/subscriptions/products.py @@ -39,9 +39,9 @@ class RTDProduct: """A local representation of a Stripe product.""" stripe_id: str - # If this product should be available to users to purchase. - listed: bool features: dict[str, RTDProductFeature] + # If this product should be available to users to purchase. + listed: bool = False def to_item(self): """ From 356f18d6c7b34a1b207eca9e97e1c5df8a64ee98 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 13 Apr 2023 12:18:03 -0500 Subject: [PATCH 09/35] Fix tests --- .../projects/tests/test_domain_views.py | 25 ++++++------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/readthedocs/projects/tests/test_domain_views.py b/readthedocs/projects/tests/test_domain_views.py index 34bbcf8be48..bff1b942e34 100644 --- a/readthedocs/projects/tests/test_domain_views.py +++ b/readthedocs/projects/tests/test_domain_views.py @@ -6,10 +6,13 @@ 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 +from readthedocs.subscriptions.products import RTDProductFeature -@override_settings(RTD_ALLOW_ORGANIZATIONS=False) +@override_settings( + RTD_ALLOW_ORGANIZATIONS=False, + RTD_DEFAULT_FEATURES=dict([RTDProductFeature(type=TYPE_CNAME).to_item()]), +) class TestDomainViews(TestCase): def setUp(self): self.user = get(User, username="user") @@ -96,24 +99,12 @@ def test_edit_domain_on_subproject(self): self.assertEqual(domain.canonical, False) -@override_settings(RTD_ALLOW_ORGANIZATIONS=True) +@override_settings( + RTD_ALLOW_ORGANIZATIONS=True, +) class TestDomainViewsWithOrganizations(TestDomainViews): def setUp(self): super().setUp() self.org = get( Organization, owners=[self.user], projects=[self.project, self.subproject] ) - self.plan = get( - Plan, - published=True, - ) - self.subscription = get( - Subscription, - plan=self.plan, - organization=self.org, - ) - self.feature = get( - PlanFeature, - plan=self.plan, - feature_type=TYPE_CNAME, - ) From 3c65b4b0cbcdda92b3b368e131bb9924611fe549 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 13 Apr 2023 13:47:32 -0500 Subject: [PATCH 10/35] Black --- readthedocs/projects/tests/test_domain_views.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/readthedocs/projects/tests/test_domain_views.py b/readthedocs/projects/tests/test_domain_views.py index bff1b942e34..d4f1ef8ac54 100644 --- a/readthedocs/projects/tests/test_domain_views.py +++ b/readthedocs/projects/tests/test_domain_views.py @@ -99,9 +99,7 @@ def test_edit_domain_on_subproject(self): self.assertEqual(domain.canonical, False) -@override_settings( - RTD_ALLOW_ORGANIZATIONS=True, -) +@override_settings(RTD_ALLOW_ORGANIZATIONS=True) class TestDomainViewsWithOrganizations(TestDomainViews): def setUp(self): super().setUp() From e4320635a311f2fbd11a4d1aff5c510350624914 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 18 Apr 2023 13:29:02 -0500 Subject: [PATCH 11/35] Fixes --- readthedocs/subscriptions/products.py | 2 +- readthedocs/subscriptions/views.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/readthedocs/subscriptions/products.py b/readthedocs/subscriptions/products.py index 47337930fed..0e16cbc2cc6 100644 --- a/readthedocs/subscriptions/products.py +++ b/readthedocs/subscriptions/products.py @@ -52,7 +52,7 @@ def to_item(self): return self.stripe_id, self -def get_product(stripe_id): +def get_product(stripe_id) -> RTDProduct: """Return the product with the given stripe_id.""" return settings.RTD_PRODUCTS.get(stripe_id) diff --git a/readthedocs/subscriptions/views.py b/readthedocs/subscriptions/views.py index 05172d4e79c..e81e4385047 100644 --- a/readthedocs/subscriptions/views.py +++ b/readthedocs/subscriptions/views.py @@ -117,7 +117,7 @@ def get_context_data(self, **kwargs): if stripe_subscription: stripe_price = stripe_subscription.items.first().price context["stripe_price"] = stripe_price - context["features"] = get_product(stripe_price.product.id).features + context["features"] = get_product(stripe_price.product.id).features.values() # When Stripe marks the subscription as ``past_due``, # it means the usage of RTD service for the current period/month was not paid at all. From 646289296d9777d09949534b200d4175414df54d Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 18 Apr 2023 13:48:36 -0500 Subject: [PATCH 12/35] Fix --- readthedocs/subscriptions/forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/subscriptions/forms.py b/readthedocs/subscriptions/forms.py index fc38b43f738..53f122fd581 100644 --- a/readthedocs/subscriptions/forms.py +++ b/readthedocs/subscriptions/forms.py @@ -22,7 +22,7 @@ def __init__(self, *args, **kwargs): .order_by("unit_amount") ) self.fields["price"].choices = [ - (price.pk, f"{price.product.name} ({price.human_readable_price})") + (price.id, f"{price.product.name} ({price.human_readable_price})") for price in stripe_prices ] self.fields["price"].help_text = _( From aeadfb69f853aa96a88b9cabb2469da4d7083964 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 25 Jul 2023 18:27:12 -0500 Subject: [PATCH 13/35] Migrate new tests --- readthedocs/api/v3/tests/test_builds.py | 7 ++++--- readthedocs/embed/v3/tests/test_access.py | 5 ++--- readthedocs/organizations/tests/test_views.py | 12 +++--------- readthedocs/rtd_tests/tests/test_core_utils.py | 7 ++++--- readthedocs/rtd_tests/tests/test_resolver.py | 4 +--- 5 files changed, 14 insertions(+), 21 deletions(-) diff --git a/readthedocs/api/v3/tests/test_builds.py b/readthedocs/api/v3/tests/test_builds.py index 7d23358c587..a7d8a75c19f 100644 --- a/readthedocs/api/v3/tests/test_builds.py +++ b/readthedocs/api/v3/tests/test_builds.py @@ -4,6 +4,7 @@ from django.urls import reverse from readthedocs.subscriptions.constants import TYPE_CONCURRENT_BUILDS +from readthedocs.subscriptions.products import RTDProductFeature from .mixins import APIEndpointMixin @@ -11,9 +12,9 @@ @override_settings( RTD_ALLOW_ORGANIZATIONS=False, ALLOW_PRIVATE_REPOS=False, - RTD_DEFAULT_FEATURES={ - TYPE_CONCURRENT_BUILDS: 4, - }, + RTD_DEFAULT_FEATURES=dict( + [RTDProductFeature(TYPE_CONCURRENT_BUILDS, value=4).to_item()] + ), ) @mock.patch('readthedocs.projects.tasks.builds.update_docs_task', mock.MagicMock()) class BuildsEndpointTests(APIEndpointMixin): diff --git a/readthedocs/embed/v3/tests/test_access.py b/readthedocs/embed/v3/tests/test_access.py index 4697125f6c6..955916c36e9 100644 --- a/readthedocs/embed/v3/tests/test_access.py +++ b/readthedocs/embed/v3/tests/test_access.py @@ -12,15 +12,14 @@ from readthedocs.projects.constants import PRIVATE, PUBLIC from readthedocs.projects.models import Project from readthedocs.subscriptions.constants import TYPE_EMBED_API +from readthedocs.subscriptions.products import RTDProductFeature @override_settings( USE_SUBDOMAIN=True, PUBLIC_DOMAIN="readthedocs.io", RTD_ALLOW_ORGAZATIONS=False, - RTD_DEFAULT_FEATURES={ - TYPE_EMBED_API: 1, - }, + RTD_DEFAULT_FEATURES=dict([RTDProductFeature(TYPE_EMBED_API).to_item()]), ) @mock.patch("readthedocs.embed.v3.views.build_media_storage") class TestEmbedAPIV3Access(TestCase): diff --git a/readthedocs/organizations/tests/test_views.py b/readthedocs/organizations/tests/test_views.py index faf07aa59c9..644f437e511 100644 --- a/readthedocs/organizations/tests/test_views.py +++ b/readthedocs/organizations/tests/test_views.py @@ -381,9 +381,7 @@ def test_organization_signup(self): @override_settings( RTD_ALLOW_ORGANIZATIONS=True, - RTD_DEFAULT_FEATURES={ - TYPE_AUDIT_LOGS: 90, - }, + RTD_DEFAULT_FEATURES=dict([RTDProductFeature(TYPE_AUDIT_LOGS, value=90).to_item()]), ) class OrganizationUnspecifiedChooser(TestCase): def setUp(self): @@ -429,9 +427,7 @@ def test_choose_organization_edit(self): @override_settings( RTD_ALLOW_ORGANIZATIONS=True, - RTD_DEFAULT_FEATURES={ - TYPE_AUDIT_LOGS: 90, - }, + RTD_DEFAULT_FEATURES=dict([RTDProductFeature(TYPE_AUDIT_LOGS, value=90).to_item()]), ) class OrganizationUnspecifiedSingleOrganizationRedirect(TestCase): def setUp(self): @@ -457,9 +453,7 @@ def test_unspecified_slug_redirects_to_organization_edit(self): @override_settings( RTD_ALLOW_ORGANIZATIONS=True, - RTD_DEFAULT_FEATURES={ - TYPE_AUDIT_LOGS: 90, - }, + RTD_DEFAULT_FEATURES=dict([RTDProductFeature(TYPE_AUDIT_LOGS, value=90).to_item()]), ) class OrganizationUnspecifiedNoOrganizationRedirect(TestCase): def setUp(self): diff --git a/readthedocs/rtd_tests/tests/test_core_utils.py b/readthedocs/rtd_tests/tests/test_core_utils.py index a6e1e9b7821..d2dbf0ee6e8 100644 --- a/readthedocs/rtd_tests/tests/test_core_utils.py +++ b/readthedocs/rtd_tests/tests/test_core_utils.py @@ -12,12 +12,13 @@ from readthedocs.doc_builder.exceptions import BuildMaxConcurrencyError from readthedocs.projects.models import Project from readthedocs.subscriptions.constants import TYPE_CONCURRENT_BUILDS +from readthedocs.subscriptions.products import RTDProductFeature @override_settings( - RTD_DEFAULT_FEATURES={ - TYPE_CONCURRENT_BUILDS: 4, - } + RTD_DEFAULT_FEATURES=dict( + [RTDProductFeature(TYPE_CONCURRENT_BUILDS, value=4).to_item()] + ), ) class CoreUtilTests(TestCase): diff --git a/readthedocs/rtd_tests/tests/test_resolver.py b/readthedocs/rtd_tests/tests/test_resolver.py index 10ba68389e1..e76a5ae94fb 100644 --- a/readthedocs/rtd_tests/tests/test_resolver.py +++ b/readthedocs/rtd_tests/tests/test_resolver.py @@ -912,9 +912,7 @@ def test_subproject_with_translation_without_custom_domain(self): ) @override_settings( - RTD_DEFAULT_FEATURES={ - TYPE_CNAME: 1, - }, + RTD_DEFAULT_FEATURES=dict([RTDProductFeature(TYPE_CNAME).to_item()]), ) def test_subproject_with_translation_with_custom_domain(self): fixture.get( From 90f692802e8024bb98ce5eb848c913c7807b94af Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 25 Jul 2023 19:05:55 -0500 Subject: [PATCH 14/35] Fix linter --- readthedocs/organizations/views/private.py | 1 - readthedocs/projects/views/private.py | 6 ++---- readthedocs/subscriptions/managers.py | 1 - readthedocs/subscriptions/models.py | 2 -- readthedocs/subscriptions/views.py | 2 -- 5 files changed, 2 insertions(+), 10 deletions(-) diff --git a/readthedocs/organizations/views/private.py b/readthedocs/organizations/views/private.py index 799648b3359..df83524c533 100644 --- a/readthedocs/organizations/views/private.py +++ b/readthedocs/organizations/views/private.py @@ -308,7 +308,6 @@ def get_queryset(self): queryset = self._get_queryset() # Set filter on self, so we can use it in the context. # Without executing it twice. - # pylint: disable=attribute-defined-outside-init self.filter = OrganizationSecurityLogFilter( self.request.GET, queryset=queryset, diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index 441402584d9..39a46daa33c 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -97,7 +97,6 @@ class ProjectDashboard(PrivateViewMixin, ListView): model = Project template_name = 'projects/project_dashboard.html' - # pylint: disable=arguments-differ def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) # Set the default search to search files instead of projects @@ -134,7 +133,7 @@ def get_queryset(self): def get(self, request, *args, **kwargs): self.validate_primary_email(request.user) - return super(ProjectDashboard, self).get(self, request, *args, **kwargs) + return super().get(self, request, *args, **kwargs) class ProjectMixin(PrivateViewMixin): @@ -283,7 +282,7 @@ def _set_initial_dict(self): else: self.initial_dict = self.storage.data.get(self.initial_dict_key, {}) - def post(self, *args, **kwargs): # pylint: disable=arguments-differ + def post(self, *args, **kwargs): self._set_initial_dict() log.bind(user_username=self.request.user.username) @@ -928,7 +927,6 @@ class IntegrationWebhookSync(IntegrationMixin, GenericView): """ def post(self, request, *args, **kwargs): - # pylint: disable=unused-argument if 'integration_pk' in kwargs: integration = self.get_integration() update_webhook(self.get_project(), integration, request=request) diff --git a/readthedocs/subscriptions/managers.py b/readthedocs/subscriptions/managers.py index bba40eab98d..f616f61a4e0 100644 --- a/readthedocs/subscriptions/managers.py +++ b/readthedocs/subscriptions/managers.py @@ -139,7 +139,6 @@ def update_from_stripe(self, *, rtd_subscription, stripe_subscription): rtd_subscription.save() return rtd_subscription - # pylint: disable=no-self-use def _get_plan(self, stripe_price): from readthedocs.subscriptions.models import Plan diff --git a/readthedocs/subscriptions/models.py b/readthedocs/subscriptions/models.py index d6c0489d3ff..af65a608e61 100644 --- a/readthedocs/subscriptions/models.py +++ b/readthedocs/subscriptions/models.py @@ -65,7 +65,6 @@ def get_absolute_url(self): def __str__(self): return f"{self.name} ({self.stripe_id})" - # pylint: disable=signature-differs def save(self, *args, **kwargs): if not self.slug: self.slug = slugify(self.name) @@ -198,7 +197,6 @@ def default_trial_end_date(self): self.organization.pub_date + timedelta(days=self.plan.trial) ) - # pylint: disable=signature-differs def save(self, *args, **kwargs): if self.trial_end_date is None: self.trial_end_date = self.default_trial_end_date() diff --git a/readthedocs/subscriptions/views.py b/readthedocs/subscriptions/views.py index e81e4385047..8cb29c53dad 100644 --- a/readthedocs/subscriptions/views.py +++ b/readthedocs/subscriptions/views.py @@ -44,7 +44,6 @@ def get(self, request, *args, **kwargs): context = self.get_context_data(form=form) return self.render_to_response(context) - # pylint: disable=unused-argument def post(self, request, *args, **kwargs): form = self.get_form(data=request.POST, files=request.FILES) if not form.is_valid(): @@ -150,7 +149,6 @@ def get_success_url(self): args=[self.get_organization().slug], ) - # pylint: disable=unused-argument def post(self, request, *args, **kwargs): """Redirect the user to the Stripe billing portal.""" organization = self.get_organization() From 8409136e22da1fe368ca669621bdf50eea8f525b Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 26 Jul 2023 12:46:23 -0500 Subject: [PATCH 15/35] Migrate more features --- readthedocs/embed/tests/test_api.py | 9 +++++---- readthedocs/embed/v3/tests/test_internal_pages.py | 7 ++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/readthedocs/embed/tests/test_api.py b/readthedocs/embed/tests/test_api.py index 75c8fa3eaa3..31ff8cc61c2 100644 --- a/readthedocs/embed/tests/test_api.py +++ b/readthedocs/embed/tests/test_api.py @@ -13,6 +13,7 @@ from readthedocs.projects.constants import MKDOCS, PUBLIC from readthedocs.projects.models import Project from readthedocs.subscriptions.constants import TYPE_EMBED_API +from readthedocs.subscriptions.products import RTDProductFeature data_path = Path(__file__).parent.resolve() / 'data' @@ -34,10 +35,10 @@ def setup_method(self, settings): self.version.save() settings.USE_SUBDOMAIN = True - settings.PUBLIC_DOMAIN = 'readthedocs.io' - settings.RTD_DEFAULT_FEATURES = { - TYPE_EMBED_API: 1, - } + settings.PUBLIC_DOMAIN = "readthedocs.io" + settings.RTD_DEFAULT_FEATURES = dict( + [RTDProductFeature(TYPE_EMBED_API).to_item()] + ) def get(self, client, *args, **kwargs): """Wrapper around ``client.get`` to be overridden in the proxied api tests.""" diff --git a/readthedocs/embed/v3/tests/test_internal_pages.py b/readthedocs/embed/v3/tests/test_internal_pages.py index 321e7c1520d..1606218d355 100644 --- a/readthedocs/embed/v3/tests/test_internal_pages.py +++ b/readthedocs/embed/v3/tests/test_internal_pages.py @@ -11,6 +11,7 @@ from readthedocs.projects.models import Project from readthedocs.subscriptions.constants import TYPE_EMBED_API +from readthedocs.subscriptions.products import RTDProductFeature from .utils import srcdir @@ -24,9 +25,9 @@ def setup_method(self, settings): settings.USE_SUBDOMAIN = True settings.PUBLIC_DOMAIN = 'readthedocs.io' settings.RTD_EMBED_API_EXTERNAL_DOMAINS = [] - settings.RTD_DEFAULT_FEATURES = { - TYPE_EMBED_API: 1, - } + settings.RTD_DEFAULT_FEATURES = dict( + [RTDProductFeature(TYPE_EMBED_API).to_item()] + ) self.api_url = reverse('embed_api_v3') From 91284b940081c2da70df170add9dc78c0ca80bb1 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 26 Jul 2023 14:18:29 -0500 Subject: [PATCH 16/35] Small fixes - Use plan instead of price for the form - Better default description - Check if the subscription exists --- readthedocs/subscriptions/forms.py | 6 +++--- readthedocs/subscriptions/products.py | 9 ++++++++- .../templates/subscriptions/subscription_detail.html | 2 +- readthedocs/subscriptions/views.py | 2 +- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/readthedocs/subscriptions/forms.py b/readthedocs/subscriptions/forms.py index 53f122fd581..104217a7ac7 100644 --- a/readthedocs/subscriptions/forms.py +++ b/readthedocs/subscriptions/forms.py @@ -11,7 +11,7 @@ class PlanForm(forms.Form): """Form to create a subscription after the previous one has ended.""" - price = forms.ChoiceField() + plan = forms.ChoiceField() def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -21,11 +21,11 @@ def __init__(self, *args, **kwargs): .select_related("product") .order_by("unit_amount") ) - self.fields["price"].choices = [ + self.fields["plan"].choices = [ (price.id, f"{price.product.name} ({price.human_readable_price})") for price in stripe_prices ] - self.fields["price"].help_text = _( + self.fields["plan"].help_text = _( 'Check our pricing page ' 'for more information about each plan.' ) diff --git a/readthedocs/subscriptions/products.py b/readthedocs/subscriptions/products.py index 0e16cbc2cc6..d9cc087d8b6 100644 --- a/readthedocs/subscriptions/products.py +++ b/readthedocs/subscriptions/products.py @@ -22,7 +22,14 @@ class RTDProductFeature: description: str = "" def get_description(self): - return self.description or dict(FEATURE_TYPES).get(self.type) + if self.description: + return self.description + default_description = dict(FEATURE_TYPES).get(self.type) + if self.unlimited: + return f"{default_description} (unlimited)" + if self.value: + return f"{default_description} ({self.value})" + return default_description def to_item(self): """ diff --git a/readthedocs/subscriptions/templates/subscriptions/subscription_detail.html b/readthedocs/subscriptions/templates/subscriptions/subscription_detail.html index 8171296d387..68de01a29f3 100644 --- a/readthedocs/subscriptions/templates/subscriptions/subscription_detail.html +++ b/readthedocs/subscriptions/templates/subscriptions/subscription_detail.html @@ -26,7 +26,7 @@ {% endif %} - {% if stripe_subscription.status != 'canceled' %} + {% if stripe_subscription and stripe_subscription.status != 'canceled' %}
{% trans "Plan" %}:
diff --git a/readthedocs/subscriptions/views.py b/readthedocs/subscriptions/views.py index 8cb29c53dad..6aa872788b3 100644 --- a/readthedocs/subscriptions/views.py +++ b/readthedocs/subscriptions/views.py @@ -65,7 +65,7 @@ def redirect_to_checkout(self, form): ): raise Http404() - stripe_price = get_object_or_404(djstripe.Price, id=form.cleaned_data["price"]) + stripe_price = get_object_or_404(djstripe.Price, id=form.cleaned_data["plan"]) url = self.request.build_absolute_uri(self.get_success_url()) organization = self.get_organization() From 52a36c5942c0fab4cb3ac19c075061da0a0d1cbc Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 26 Jul 2023 14:21:38 -0500 Subject: [PATCH 17/35] Document local testing --- docs/dev/subscriptions.rst | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 docs/dev/subscriptions.rst diff --git a/docs/dev/subscriptions.rst b/docs/dev/subscriptions.rst new file mode 100644 index 00000000000..56b12741ec5 --- /dev/null +++ b/docs/dev/subscriptions.rst @@ -0,0 +1,32 @@ +Subscriptions +============= + +Subscriptions are available on |com_brand|, +we make use of Stripe to handle the payments and subscriptions. + +Local testing +------------- + +To test subscriptions locally, you need to have access to the Stripe account, +and define the following settings with the keys from Stripe test mode: + +- ``STRIPE_SECRET``: https://dashboard.stripe.com/test/apikeys +- ``STRIPE_TEST_SECRET_KEY``: https://dashboard.stripe.com/test/apikeys +- ``DJSTRIPE_WEBHOOK_SECRET``: https://dashboard.stripe.com/test/webhooks + +To test the webhook locally, you need to run your local instance with ngrok, for example: + +.. code-block:: bash + + ngrok http 80 + inv docker.up --http-domain xxx.ngrok.io + +If this is your first time setting up subscriptions, you will to re-sync djstripe with Stripe: + +.. code-block:: bash + + inv docker.manage djstripe_sync_models + +The subscription settings (``RTD_PRODUCTS``) already mapped to match the Stripe prices from the test mode. +To subscribe to any plan, you can use any `test card from Stripe `__, +for example: ``4242 4242 4242 4242`` (use any future date and any value for the other fields). From 3d9ea1982d2f36badce60432471deea203b16f10 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 26 Jul 2023 14:23:33 -0500 Subject: [PATCH 18/35] Include in index --- docs/dev/index.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/dev/index.rst b/docs/dev/index.rst index bed5cdd90a0..99ebeaca194 100644 --- a/docs/dev/index.rst +++ b/docs/dev/index.rst @@ -27,5 +27,6 @@ or taking the open source Read the Docs codebase for your own custom installatio i18n server-side-search search-integration + subscriptions settings tests From 29faab514f89d0581ee315c9cb7a9365793a3a23 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 26 Jul 2023 16:56:00 -0500 Subject: [PATCH 19/35] Filter by active prices --- readthedocs/subscriptions/forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/subscriptions/forms.py b/readthedocs/subscriptions/forms.py index 104217a7ac7..96007ae4418 100644 --- a/readthedocs/subscriptions/forms.py +++ b/readthedocs/subscriptions/forms.py @@ -17,7 +17,7 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) products_id = [product.stripe_id for product in get_listed_products()] stripe_prices = ( - djstripe.Price.objects.filter(product__id__in=products_id) + djstripe.Price.objects.filter(product__id__in=products_id, active=True) .select_related("product") .order_by("unit_amount") ) From f9b060cdd7acba43fd034bc278848325e7d98232 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 27 Jul 2023 16:33:29 -0500 Subject: [PATCH 20/35] Better description for page views logs --- readthedocs/subscriptions/constants.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/subscriptions/constants.py b/readthedocs/subscriptions/constants.py index 900a054372c..d46514a1fe4 100644 --- a/readthedocs/subscriptions/constants.py +++ b/readthedocs/subscriptions/constants.py @@ -32,5 +32,5 @@ (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_AUDIT_PAGEVIEWS, _("Audit logs for every page view")), ) From 2399515aa0ba91440e5bfeb315511d486bb5384a Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 31 Jul 2023 15:33:23 -0500 Subject: [PATCH 21/35] Support for multiple products --- readthedocs/subscriptions/products.py | 57 ++++++++++++++----- .../subscriptions/subscription_detail.html | 25 +++++++- readthedocs/subscriptions/views.py | 30 ++++++++-- 3 files changed, 90 insertions(+), 22 deletions(-) diff --git a/readthedocs/subscriptions/products.py b/readthedocs/subscriptions/products.py index d9cc087d8b6..58e5e4841ed 100644 --- a/readthedocs/subscriptions/products.py +++ b/readthedocs/subscriptions/products.py @@ -4,6 +4,7 @@ This module contains the dataclasses that represent the products available. The available products are defined in the ``RTD_PRODUCTS`` setting. """ +import copy from dataclasses import dataclass from django.conf import settings @@ -39,6 +40,25 @@ def to_item(self): """ return self.type, self + def __add__(self, other: "RTDProductFeature"): + """Add the values of two features.""" + if self.type != other.type: + raise ValueError("Cannot add features of different types") + new_feature = copy.copy(self) + if self.unlimited or other.unlimited: + new_feature.unlimited = True + else: + new_feature.value = self.value + other.value + return new_feature + + def __mul__(self, value: int): + """Multiply the value of a feature by a number.""" + new_feature = copy.copy(self) + if self.unlimited: + return new_feature + new_feature.value = self.value * value + return new_feature + @dataclass(slots=True) class RTDProduct: @@ -49,6 +69,9 @@ class RTDProduct: features: dict[str, RTDProductFeature] # If this product should be available to users to purchase. listed: bool = False + # If this product is an extra that can be added to a main plan. + # For example, an extra builder. + extra: bool = False def to_item(self): """ @@ -69,7 +92,7 @@ def get_listed_products(): return [product for product in settings.RTD_PRODUCTS.values() if product.listed] -def get_products_with_feature(feature_type): +def get_products_with_feature(feature_type) -> list[RTDProduct]: """Return a list of products that have the given feature.""" return [ product @@ -89,8 +112,6 @@ def get_feature(obj, feature_type) -> RTDProductFeature: """ # Hit the DB only if subscriptions are enabled. if settings.RTD_PRODUCTS: - from djstripe import models as djstripe - from readthedocs.organizations.models import Organization from readthedocs.projects.models import Project @@ -101,20 +122,26 @@ def get_feature(obj, feature_type) -> RTDProductFeature: else: raise TypeError + stripe_subscription = organization.get_or_create_stripe_subscription() + # A subscription can have multiple products, but we only want - # the product from the organization that has the feature we are looking for. - stripe_products_id = [ + # the products from the organization that has the feature we are looking for. + available_stripe_products_id = [ product.stripe_id for product in get_products_with_feature(feature_type) ] - stripe_product_id = ( - djstripe.Product.objects.filter( - id__in=stripe_products_id, - prices__subscription_items__subscription__rtd_organization=organization, - ).values_list("id", flat=True) - # TODO: maybe we should merge features from multiple products? - .first() - ) - if stripe_product_id is not None: - return settings.RTD_PRODUCTS[stripe_product_id].features[feature_type] + subscription_items = stripe_subscription.items.filter( + price__product__id__in=available_stripe_products_id + ).select_related("price__product") + final_rtd_feature = None + for subscription_item in subscription_items: + rtd_feature = settings.RTD_PRODUCTS[ + subscription_item.price.product.id + ].features[feature_type] + if final_rtd_feature is None: + final_rtd_feature = rtd_feature * subscription_item.quantity + else: + final_rtd_feature += rtd_feature * subscription_item.quantity + if final_rtd_feature: + return final_rtd_feature return settings.RTD_DEFAULT_FEATURES.get(feature_type) diff --git a/readthedocs/subscriptions/templates/subscriptions/subscription_detail.html b/readthedocs/subscriptions/templates/subscriptions/subscription_detail.html index 68de01a29f3..bfbe99a97e5 100644 --- a/readthedocs/subscriptions/templates/subscriptions/subscription_detail.html +++ b/readthedocs/subscriptions/templates/subscriptions/subscription_detail.html @@ -31,15 +31,34 @@
{% trans "Plan" %}:
- {{ stripe_price.product.name }} + {{ main_product.stripe_price.product.name }} - ({{ stripe_price.human_readable_price }}) + ({{ main_product.stripe_price.human_readable_price }})
+ {% if extra_products %} +
{% trans "Extra products:" %}
+ {% for extra_product in extra_products %} +
+ + {{ extra_product.stripe_price.product.name }} + + + ({{ extra_product.stripe_price.human_readable_price }}) + + {% if extra_product.quantity > 1 %} + + (x{{ extra_product.quantity }}) + + {% endif %} +
+ {% endfor %} + {% endif %} + {% if features %} -
{% trans "Plan features:" %}
+
{% trans "Features:" %}
{% for feature in features %} diff --git a/readthedocs/subscriptions/views.py b/readthedocs/subscriptions/views.py index 6aa872788b3..4d274e534ca 100644 --- a/readthedocs/subscriptions/views.py +++ b/readthedocs/subscriptions/views.py @@ -114,10 +114,32 @@ def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) stripe_subscription = self.get_object() if stripe_subscription: - stripe_price = stripe_subscription.items.first().price - context["stripe_price"] = stripe_price - context["features"] = get_product(stripe_price.product.id).features.values() - + main_product = None + extra_products = [] + features = {} + for item in stripe_subscription.items.all().select_related( + "price__product" + ): + rtd_product = get_product(item.price.product.id) + product = { + "stripe_price": item.price, + "quantity": item.quantity, + } + + if rtd_product.extra: + extra_products.append(product) + else: + main_product = product + + for feature_type, feature in rtd_product.features.items(): + if feature_type not in features: + features[feature_type] = feature + else: + features[feature_type] += feature * item.quantity + + context["main_product"] = main_product + context["extra_products"] = extra_products + context["features"] = features.values() # When Stripe marks the subscription as ``past_due``, # it means the usage of RTD service for the current period/month was not paid at all. # Show the end date as the last period the customer paid. From a8950d32becd04cc81c9651aa16d5dd4f236fc7b Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 31 Jul 2023 15:52:59 -0500 Subject: [PATCH 22/35] Migration --- .../0002_alter_planfeature_feature_type.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 readthedocs/subscriptions/migrations/0002_alter_planfeature_feature_type.py diff --git a/readthedocs/subscriptions/migrations/0002_alter_planfeature_feature_type.py b/readthedocs/subscriptions/migrations/0002_alter_planfeature_feature_type.py new file mode 100644 index 00000000000..60344ef9dde --- /dev/null +++ b/readthedocs/subscriptions/migrations/0002_alter_planfeature_feature_type.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.20 on 2023-07-31 20:51 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('subscriptions', '0001_squashed'), + ] + + operations = [ + migrations.AlterField( + model_name='planfeature', + name='feature_type', + field=models.CharField(choices=[('cname', 'Custom domain'), ('cdn', 'CDN public documentation'), ('ssl', 'Custom SSL configuration'), ('support', 'Support SLA'), ('private_docs', 'Private documentation'), ('embed_api', 'Embed content via API'), ('search_analytics', 'Search analytics'), ('pageviews_analytics', 'Pageview analytics'), ('concurrent_builds', 'Concurrent builds'), ('sso', 'Single sign on (SSO) with Google'), ('urls', 'Custom URLs'), ('audit-logs', 'Audit logs'), ('audit-pageviews', 'Audit logs for every page view')], max_length=32, verbose_name='Type'), + ), + ] From 308403503f391844fad1676cbd46bcec66ee579c Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 31 Jul 2023 15:57:49 -0500 Subject: [PATCH 23/35] format --- .../0002_alter_planfeature_feature_type.py | 27 +++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/readthedocs/subscriptions/migrations/0002_alter_planfeature_feature_type.py b/readthedocs/subscriptions/migrations/0002_alter_planfeature_feature_type.py index 60344ef9dde..b598b6b506f 100644 --- a/readthedocs/subscriptions/migrations/0002_alter_planfeature_feature_type.py +++ b/readthedocs/subscriptions/migrations/0002_alter_planfeature_feature_type.py @@ -4,15 +4,32 @@ class Migration(migrations.Migration): - dependencies = [ - ('subscriptions', '0001_squashed'), + ("subscriptions", "0001_squashed"), ] operations = [ migrations.AlterField( - model_name='planfeature', - name='feature_type', - field=models.CharField(choices=[('cname', 'Custom domain'), ('cdn', 'CDN public documentation'), ('ssl', 'Custom SSL configuration'), ('support', 'Support SLA'), ('private_docs', 'Private documentation'), ('embed_api', 'Embed content via API'), ('search_analytics', 'Search analytics'), ('pageviews_analytics', 'Pageview analytics'), ('concurrent_builds', 'Concurrent builds'), ('sso', 'Single sign on (SSO) with Google'), ('urls', 'Custom URLs'), ('audit-logs', 'Audit logs'), ('audit-pageviews', 'Audit logs for every page view')], max_length=32, verbose_name='Type'), + model_name="planfeature", + name="feature_type", + field=models.CharField( + choices=[ + ("cname", "Custom domain"), + ("cdn", "CDN public documentation"), + ("ssl", "Custom SSL configuration"), + ("support", "Support SLA"), + ("private_docs", "Private documentation"), + ("embed_api", "Embed content via API"), + ("search_analytics", "Search analytics"), + ("pageviews_analytics", "Pageview analytics"), + ("concurrent_builds", "Concurrent builds"), + ("sso", "Single sign on (SSO) with Google"), + ("urls", "Custom URLs"), + ("audit-logs", "Audit logs"), + ("audit-pageviews", "Audit logs for every page view"), + ], + max_length=32, + verbose_name="Type", + ), ), ] From 45a1574e30ddd83c7531139fbc645bb79ff26512 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 31 Jul 2023 16:58:55 -0500 Subject: [PATCH 24/35] More docs --- docs/dev/subscriptions.rst | 59 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/docs/dev/subscriptions.rst b/docs/dev/subscriptions.rst index 56b12741ec5..b5359649b31 100644 --- a/docs/dev/subscriptions.rst +++ b/docs/dev/subscriptions.rst @@ -3,6 +3,7 @@ Subscriptions Subscriptions are available on |com_brand|, we make use of Stripe to handle the payments and subscriptions. +We use dj-stripe to handle the integration with Stripe. Local testing ------------- @@ -30,3 +31,61 @@ If this is your first time setting up subscriptions, you will to re-sync djstrip The subscription settings (``RTD_PRODUCTS``) already mapped to match the Stripe prices from the test mode. To subscribe to any plan, you can use any `test card from Stripe `__, for example: ``4242 4242 4242 4242`` (use any future date and any value for the other fields). + +Modeling +-------- + +Subscriptions are attached to an organization (customer), +and can have multiple prices of products attached to it. +A product can have multiple prices, usually monthly and yearly. + +When a user subscribes to a plan (product), they are subscribing to a price of a product, +for example, the monthly price of the "Basic plan" product. + +A subscription has a "main" product (``RTDProduct(extra=False)``), +and can have several "extra" products (``RTDProduct(extra=True)``). +For example, an organization can have a subscription with a "Basic Plan" product, and an "Extra builder" product. + +Each product is mapped to a set of features (``RTD_PRODUCTS``) that the user will have access to +(different prices of the same product have the same features). +If a subscription has multiple products, the features are multiplied by the quantity and added together. +For example, if a subscription has a "Basic Plan" product with a two concurrent builders, +and an "Extra builder" product with quantity three, the total number of concurrent builders the +organization has will be five. + +Life cycle of a subscription +---------------------------- + +When a new organization is created, a stripe customer is created for that organization, +and this customer is subscribed to the trial product (``RTD_ORG_DEFAULT_STRIPE_SUBSCRIPTION_PRICE``). + +After the trial period is over, the subscription is canceled, +and their organization is disabled. + +During or after the trial a user can upgrade their subscription to a paid plan +(``RTDProduct(listed=True)``). + +Custom products +--------------- + +We provide 3 paid plans that users can subscribe to: Basic, Advanced and Pro. +Additionally, we provide an Enterprise plan, this plan is customized for each customer, +and it's manually created by the RTD core team. + +To create a custom plan, you need to create a new product in Stripe, +and add the product id to the ``RTD_PRODUCTS`` setting mapped to the features that the plan will provide. +After that, you can create a subscription for the organization with the custom product, +our appliction will automatically relate this new product to the organization. + +Extra products +-------------- + +We have one extra product: Extra builder. + +To create a new extra product, you need to create a new product in Stripe, +and add the product id to the ``RTD_PRODUCTS`` setting mapped to the features that the +extra product will provide, this product should have the ``extra`` attribute set to ``True``. + +To subscribe an organization to an extra product, +you just need to add the product to its subscription with the desired quantity, +our appliction will automatically relate this new product to the organization. From 9f6e3bf700ee76e282021e658a0fc92fb4841f6d Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 31 Jul 2023 18:48:29 -0500 Subject: [PATCH 25/35] Use new logic to get subscription --- readthedocs/organizations/models.py | 4 +++- readthedocs/subscriptions/utils.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/readthedocs/organizations/models.py b/readthedocs/organizations/models.py index 07f12a4985b..9a56144b52e 100644 --- a/readthedocs/organizations/models.py +++ b/readthedocs/organizations/models.py @@ -136,7 +136,9 @@ def get_or_create_stripe_subscription(self): # This only happens during development. log.warning("No default subscription created.") return None + return self.get_stripe_subscription() + def get_stripe_subscription(self): # Active subscriptions take precedence over non-active subscriptions, # otherwise we return the must recently created subscription. active_subscription = self.stripe_customer.subscriptions.filter( @@ -144,7 +146,7 @@ def get_or_create_stripe_subscription(self): ).first() if active_subscription: return active_subscription - return self.stripe_customer.subscriptions.latest() + return self.stripe_customer.subscriptions.order_by("created").last() def get_absolute_url(self): return reverse('organization_detail', args=(self.slug,)) diff --git a/readthedocs/subscriptions/utils.py b/readthedocs/subscriptions/utils.py index 805c29531cf..a2cb16dde0f 100644 --- a/readthedocs/subscriptions/utils.py +++ b/readthedocs/subscriptions/utils.py @@ -78,7 +78,7 @@ def get_or_create_stripe_subscription(organization): The subscription will be created with the default price and a trial period. """ stripe_customer = get_or_create_stripe_customer(organization) - stripe_subscription = stripe_customer.subscriptions.order_by("created").last() + stripe_subscription = organization.get_stripe_subscription() if not stripe_subscription: # TODO: djstripe 2.6.x doesn't return the subscription object # on subscribe(), but 2.7.x (unreleased) does! From 03d5139d18097ba6106ecf8b368d02277bf0c053 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 31 Jul 2023 19:04:33 -0500 Subject: [PATCH 26/35] Fix --- readthedocs/subscriptions/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/subscriptions/views.py b/readthedocs/subscriptions/views.py index 4d274e534ca..6103e208836 100644 --- a/readthedocs/subscriptions/views.py +++ b/readthedocs/subscriptions/views.py @@ -133,7 +133,7 @@ def get_context_data(self, **kwargs): for feature_type, feature in rtd_product.features.items(): if feature_type not in features: - features[feature_type] = feature + features[feature_type] = feature * item.quantity else: features[feature_type] += feature * item.quantity From df0a7d5846ba2627d94b49e4c83b8de048dd977a Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 31 Jul 2023 19:05:57 -0500 Subject: [PATCH 27/35] Invert --- readthedocs/subscriptions/views.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/readthedocs/subscriptions/views.py b/readthedocs/subscriptions/views.py index 6103e208836..c70e5a62625 100644 --- a/readthedocs/subscriptions/views.py +++ b/readthedocs/subscriptions/views.py @@ -132,10 +132,10 @@ def get_context_data(self, **kwargs): main_product = product for feature_type, feature in rtd_product.features.items(): - if feature_type not in features: - features[feature_type] = feature * item.quantity - else: + if feature_type in features: features[feature_type] += feature * item.quantity + else: + features[feature_type] = feature * item.quantity context["main_product"] = main_product context["extra_products"] = extra_products From dd42fd99fac0098f29dcbdcbef71d29c0bbc458c Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 31 Jul 2023 19:11:44 -0500 Subject: [PATCH 28/35] Linter --- readthedocs/organizations/models.py | 19 +++++++++---------- readthedocs/subscriptions/utils.py | 1 - 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/readthedocs/organizations/models.py b/readthedocs/organizations/models.py index 9a56144b52e..f201e96704b 100644 --- a/readthedocs/organizations/models.py +++ b/readthedocs/organizations/models.py @@ -159,7 +159,7 @@ def users(self): def members(self): return AdminPermission.members(self) - def save(self, *args, **kwargs): # pylint: disable=signature-differs + def save(self, *args, **kwargs): if not self.slug: self.slug = slugify(self.name) @@ -175,7 +175,6 @@ def get_stripe_metadata(self): "org:slug": self.slug, } - # pylint: disable=no-self-use def add_member(self, user, team): """ Add member to organization team. @@ -280,7 +279,7 @@ def get_absolute_url(self): def __str__(self): return self.name - def save(self, *args, **kwargs): # pylint: disable=signature-differs + def save(self, *args, **kwargs): if not self.slug: self.slug = slugify(self.name) super().save(*args, **kwargs) @@ -321,7 +320,7 @@ def __str__(self): team=self.team, ) - def save(self, *args, **kwargs): # pylint: disable=signature-differs + def save(self, *args, **kwargs): hash_ = salted_hmac( # HMAC key per applications '.'.join([self.__module__, self.__class__.__name__]), @@ -347,12 +346,12 @@ def migrate(self): content_type = ContentType.objects.get_for_model(self.team) invitation, created = Invitation.objects.get_or_create( token=self.hash, - defaults=dict( - from_user=owner, - to_email=self.email, - content_type=content_type, - object_id=self.team.pk, - ), + defaults={ + "from_user": owner, + "to_email": self.email, + "content_type": content_type, + "object_id": self.team.pk, + }, ) self.teammember_set.all().delete() return invitation, created diff --git a/readthedocs/subscriptions/utils.py b/readthedocs/subscriptions/utils.py index a2cb16dde0f..a9929be7203 100644 --- a/readthedocs/subscriptions/utils.py +++ b/readthedocs/subscriptions/utils.py @@ -48,7 +48,6 @@ def get_or_create_stripe_customer(organization): try: # TODO: Don't fully trust on djstripe yet, # the customer may not be in our DB yet. - # pylint: disable=protected-access stripe_customer = djstripe.Customer._get_or_retrieve( organization.stripe_id ) From b89f7aad93d18bad83691c1eeec57640269d1ed4 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 31 Jul 2023 19:16:10 -0500 Subject: [PATCH 29/35] Fix test on .com --- readthedocs/subscriptions/products.py | 32 +++++++++++++-------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/readthedocs/subscriptions/products.py b/readthedocs/subscriptions/products.py index 58e5e4841ed..ade09140f3c 100644 --- a/readthedocs/subscriptions/products.py +++ b/readthedocs/subscriptions/products.py @@ -122,26 +122,26 @@ def get_feature(obj, feature_type) -> RTDProductFeature: else: raise TypeError - stripe_subscription = organization.get_or_create_stripe_subscription() - # A subscription can have multiple products, but we only want # the products from the organization that has the feature we are looking for. available_stripe_products_id = [ product.stripe_id for product in get_products_with_feature(feature_type) ] - subscription_items = stripe_subscription.items.filter( - price__product__id__in=available_stripe_products_id - ).select_related("price__product") - final_rtd_feature = None - for subscription_item in subscription_items: - rtd_feature = settings.RTD_PRODUCTS[ - subscription_item.price.product.id - ].features[feature_type] - if final_rtd_feature is None: - final_rtd_feature = rtd_feature * subscription_item.quantity - else: - final_rtd_feature += rtd_feature * subscription_item.quantity - if final_rtd_feature: - return final_rtd_feature + stripe_subscription = organization.get_or_create_stripe_subscription() + if stripe_subscription: + subscription_items = stripe_subscription.items.filter( + price__product__id__in=available_stripe_products_id + ).select_related("price__product") + final_rtd_feature = None + for subscription_item in subscription_items: + rtd_feature = settings.RTD_PRODUCTS[ + subscription_item.price.product.id + ].features[feature_type] + if final_rtd_feature is None: + final_rtd_feature = rtd_feature * subscription_item.quantity + else: + final_rtd_feature += rtd_feature * subscription_item.quantity + if final_rtd_feature: + return final_rtd_feature return settings.RTD_DEFAULT_FEATURES.get(feature_type) From f1486f633c98f28e9196a8751eca4667b4febee1 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 31 Jul 2023 19:34:40 -0500 Subject: [PATCH 30/35] Quantity --- readthedocs/subscriptions/tests/test_event_handlers.py | 3 +++ readthedocs/subscriptions/tests/test_models.py | 2 ++ readthedocs/subscriptions/tests/test_views.py | 1 + 3 files changed, 6 insertions(+) diff --git a/readthedocs/subscriptions/tests/test_event_handlers.py b/readthedocs/subscriptions/tests/test_event_handlers.py index 51b6335fb29..b32956dc19b 100644 --- a/readthedocs/subscriptions/tests/test_event_handlers.py +++ b/readthedocs/subscriptions/tests/test_event_handlers.py @@ -196,6 +196,7 @@ def test_cancel_trial_subscription_after_trial_has_ended( djstripe.SubscriptionItem, id="si_KOcEsHCktPUedU", price=price, + quantity=1, subscription=stripe_subscription, ) event = get( @@ -240,6 +241,7 @@ def test_dont_cancel_normal_subscription_after_trial_has_ended( djstripe.SubscriptionItem, id="si_KOcEsHCktPUedU", price=price, + quantity=1, subscription=stripe_subscription, ) event = get( @@ -312,6 +314,7 @@ def test_subscription_canceled_trial_subscription(self, notification_send): djstripe.SubscriptionItem, id="si_KOcEsHCktPUedU", price=price, + quantity=1, subscription=stripe_subscription, ) diff --git a/readthedocs/subscriptions/tests/test_models.py b/readthedocs/subscriptions/tests/test_models.py index 7916930eadd..fc27ebd2666 100644 --- a/readthedocs/subscriptions/tests/test_models.py +++ b/readthedocs/subscriptions/tests/test_models.py @@ -65,6 +65,7 @@ def test_update(self): djstripe.SubscriptionItem, id="si_KOcEsHCktPUedU", price=price, + quantity=1, subscription=stripe_subscription, ) @@ -119,6 +120,7 @@ def test_replace_subscription(self): djstripe.SubscriptionItem, id="si_KOcEsHCktPUedU", price=price, + quantity=1, subscription=stripe_subscription, ) diff --git a/readthedocs/subscriptions/tests/test_views.py b/readthedocs/subscriptions/tests/test_views.py index db4246bcc0b..9af45be012a 100644 --- a/readthedocs/subscriptions/tests/test_views.py +++ b/readthedocs/subscriptions/tests/test_views.py @@ -92,6 +92,7 @@ def _create_stripe_subscription( get( djstripe.SubscriptionItem, price=self.stripe_price, + quantity=1, subscription=stripe_subscription, ) return stripe_subscription From 6c385148b52f919af27e8f53b6d547e9658c65b1 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 31 Jul 2023 20:05:17 -0500 Subject: [PATCH 31/35] Tests --- .../subscriptions/tests/test_products.py | 34 +++++++++++++ readthedocs/subscriptions/tests/test_views.py | 49 +++++++++++++++++-- 2 files changed, 78 insertions(+), 5 deletions(-) create mode 100644 readthedocs/subscriptions/tests/test_products.py diff --git a/readthedocs/subscriptions/tests/test_products.py b/readthedocs/subscriptions/tests/test_products.py new file mode 100644 index 00000000000..6a60d648fed --- /dev/null +++ b/readthedocs/subscriptions/tests/test_products.py @@ -0,0 +1,34 @@ +from django.test import TestCase + +from readthedocs.subscriptions.constants import TYPE_AUDIT_LOGS, TYPE_CONCURRENT_BUILDS +from readthedocs.subscriptions.products import RTDProductFeature + + +class TestRTDProductFeature(TestCase): + def test_add_feature(self): + feature_a = RTDProductFeature(TYPE_CONCURRENT_BUILDS, value=1) + feature_b = RTDProductFeature(TYPE_CONCURRENT_BUILDS, value=2) + feature_c = feature_a + feature_b + + self.assertEqual(feature_c.value, 3) + self.assertEqual(feature_c.type, TYPE_CONCURRENT_BUILDS) + + def test_add_different_features(self): + feature_a = RTDProductFeature(TYPE_CONCURRENT_BUILDS, value=1) + feature_b = RTDProductFeature(TYPE_AUDIT_LOGS, value=2) + + with self.assertRaises(ValueError): + feature_a + feature_b + + def test_multiply_feature(self): + feature_a = RTDProductFeature(TYPE_CONCURRENT_BUILDS, value=3) + feature_b = feature_a * 2 + + self.assertEqual(feature_b.value, 6) + self.assertEqual(feature_b.type, TYPE_CONCURRENT_BUILDS) + + feature_a = RTDProductFeature(TYPE_CONCURRENT_BUILDS, value=3) + feature_b = feature_a * 1 + + self.assertEqual(feature_b.value, 3) + self.assertEqual(feature_b.type, TYPE_CONCURRENT_BUILDS) diff --git a/readthedocs/subscriptions/tests/test_views.py b/readthedocs/subscriptions/tests/test_views.py index 9af45be012a..afee5b6339b 100644 --- a/readthedocs/subscriptions/tests/test_views.py +++ b/readthedocs/subscriptions/tests/test_views.py @@ -11,7 +11,10 @@ from djstripe.enums import SubscriptionStatus from readthedocs.organizations.models import Organization -from readthedocs.subscriptions.constants import TYPE_PRIVATE_DOCS +from readthedocs.subscriptions.constants import ( + TYPE_CONCURRENT_BUILDS, + TYPE_PRIVATE_DOCS, +) from readthedocs.subscriptions.models import Plan, Subscription from readthedocs.subscriptions.products import RTDProduct, RTDProductFeature @@ -19,19 +22,26 @@ @override_settings( RTD_ALLOW_ORGANIZATIONS=True, RTD_PRODUCTS=dict( - ( + [ RTDProduct( stripe_id="prod_a1b2c3", listed=True, features=dict( - ( + [ RTDProductFeature( type=TYPE_PRIVATE_DOCS, ).to_item(), - ) + ] ), ).to_item(), - ) + RTDProduct( + stripe_id="prod_extra_builder", + extra=True, + features=dict( + [RTDProductFeature(TYPE_CONCURRENT_BUILDS, value=1).to_item()] + ), + ).to_item(), + ] ), ) class SubscriptionViewTests(TestCase): @@ -56,6 +66,16 @@ def setUp(self): unit_amount=50000, product=self.stripe_product, ) + self.extra_product = get( + djstripe.Product, + id="prod_extra_builder", + ) + self.extra_price = get( + djstripe.Price, + id="price_extra_builder", + unit_amount=50000, + product=self.extra_product, + ) self.stripe_subscription = self._create_stripe_subscription( customer_id=self.organization.stripe_id, subscription_id="sub_a1b2c3d4", @@ -102,6 +122,25 @@ def test_active_subscription(self): self.assertEqual(resp.status_code, 200) self.assertEqual(resp.context["stripe_subscription"], self.stripe_subscription) self.assertContains(resp, "active") + self.assertNotContains(resp, "Extra products:") + # The subscribe form isn't shown, but the manage susbcription button is. + self.assertContains(resp, "Manage Subscription") + self.assertNotContains(resp, "Create Subscription") + + def test_active_subscription_with_extra_product(self): + get( + djstripe.SubscriptionItem, + price=self.extra_price, + quantity=2, + subscription=self.stripe_subscription, + ) + resp = self.client.get( + reverse("subscription_detail", args=[self.organization.slug]) + ) + self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.context["stripe_subscription"], self.stripe_subscription) + self.assertContains(resp, "active") + self.assertContains(resp, "Extra products:") # The subscribe form isn't shown, but the manage susbcription button is. self.assertContains(resp, 'Manage Subscription') self.assertNotContains(resp, 'Create Subscription') From 95eb6f1d9b5c089a094ecf127d4e07e4b760a393 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 31 Jul 2023 20:11:25 -0500 Subject: [PATCH 32/35] More tests --- readthedocs/subscriptions/products.py | 4 ++-- .../subscriptions/tests/test_products.py | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/readthedocs/subscriptions/products.py b/readthedocs/subscriptions/products.py index ade09140f3c..a016ebfdba4 100644 --- a/readthedocs/subscriptions/products.py +++ b/readthedocs/subscriptions/products.py @@ -110,8 +110,8 @@ def get_feature(obj, feature_type) -> RTDProductFeature: :param obj: An organization or project instance. :param type: The type of the feature (readthedocs.subscriptions.constants.TYPE_*). """ - # Hit the DB only if subscriptions are enabled. - if settings.RTD_PRODUCTS: + # Hit the DB only if subscriptions and organizations are enabled. + if settings.RTD_PRODUCTS and settings.RTD_ALLOW_ORGANIZATIONS: from readthedocs.organizations.models import Organization from readthedocs.projects.models import Project diff --git a/readthedocs/subscriptions/tests/test_products.py b/readthedocs/subscriptions/tests/test_products.py index 6a60d648fed..d114aa53be2 100644 --- a/readthedocs/subscriptions/tests/test_products.py +++ b/readthedocs/subscriptions/tests/test_products.py @@ -10,9 +10,18 @@ def test_add_feature(self): feature_b = RTDProductFeature(TYPE_CONCURRENT_BUILDS, value=2) feature_c = feature_a + feature_b + self.assertEqual(feature_c.unlimited, False) self.assertEqual(feature_c.value, 3) self.assertEqual(feature_c.type, TYPE_CONCURRENT_BUILDS) + def test_add_feature_unlimited(self): + feature_a = RTDProductFeature(TYPE_CONCURRENT_BUILDS, value=1) + feature_b = RTDProductFeature(TYPE_CONCURRENT_BUILDS, unlimited=True) + feature_c = feature_a + feature_b + + self.assertEqual(feature_c.unlimited, True) + self.assertEqual(feature_c.type, TYPE_CONCURRENT_BUILDS) + def test_add_different_features(self): feature_a = RTDProductFeature(TYPE_CONCURRENT_BUILDS, value=1) feature_b = RTDProductFeature(TYPE_AUDIT_LOGS, value=2) @@ -25,10 +34,19 @@ def test_multiply_feature(self): feature_b = feature_a * 2 self.assertEqual(feature_b.value, 6) + self.assertEqual(feature_b.unlimited, False) self.assertEqual(feature_b.type, TYPE_CONCURRENT_BUILDS) feature_a = RTDProductFeature(TYPE_CONCURRENT_BUILDS, value=3) feature_b = feature_a * 1 self.assertEqual(feature_b.value, 3) + self.assertEqual(feature_b.unlimited, False) + self.assertEqual(feature_b.type, TYPE_CONCURRENT_BUILDS) + + def test_multiply_feature_unlimited(self): + feature_a = RTDProductFeature(TYPE_CONCURRENT_BUILDS, unlimited=True) + feature_b = feature_a * 2 + + self.assertEqual(feature_b.unlimited, True) self.assertEqual(feature_b.type, TYPE_CONCURRENT_BUILDS) From 19dd947ac83431d2579e6d1de8d40b55b013bfa3 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 31 Jul 2023 20:36:48 -0500 Subject: [PATCH 33/35] Fix test on .com --- readthedocs/subscriptions/products.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/readthedocs/subscriptions/products.py b/readthedocs/subscriptions/products.py index a016ebfdba4..b0513942185 100644 --- a/readthedocs/subscriptions/products.py +++ b/readthedocs/subscriptions/products.py @@ -122,6 +122,11 @@ def get_feature(obj, feature_type) -> RTDProductFeature: else: raise TypeError + # This happens when running tests on .com only. + # In production projects are always associated with an organization. + if not organization: + return settings.RTD_DEFAULT_FEATURES.get(feature_type) + # A subscription can have multiple products, but we only want # the products from the organization that has the feature we are looking for. available_stripe_products_id = [ From 62f29d396797f154c01231b168e561afd505d027 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 3 Aug 2023 13:19:11 -0500 Subject: [PATCH 34/35] Avoid nesting --- readthedocs/subscriptions/products.py | 80 ++++++++++++++------------- 1 file changed, 42 insertions(+), 38 deletions(-) diff --git a/readthedocs/subscriptions/products.py b/readthedocs/subscriptions/products.py index b0513942185..af27da70cd0 100644 --- a/readthedocs/subscriptions/products.py +++ b/readthedocs/subscriptions/products.py @@ -111,42 +111,46 @@ def get_feature(obj, feature_type) -> RTDProductFeature: :param type: The type of the feature (readthedocs.subscriptions.constants.TYPE_*). """ # Hit the DB only if subscriptions and organizations are enabled. - if settings.RTD_PRODUCTS and settings.RTD_ALLOW_ORGANIZATIONS: - from readthedocs.organizations.models import Organization - from readthedocs.projects.models import Project - - if isinstance(obj, Project): - organization = obj.organizations.first() - elif isinstance(obj, Organization): - organization = obj - else: - raise TypeError - - # This happens when running tests on .com only. - # In production projects are always associated with an organization. - if not organization: - return settings.RTD_DEFAULT_FEATURES.get(feature_type) - - # A subscription can have multiple products, but we only want - # the products from the organization that has the feature we are looking for. - available_stripe_products_id = [ - product.stripe_id for product in get_products_with_feature(feature_type) - ] - stripe_subscription = organization.get_or_create_stripe_subscription() - if stripe_subscription: - subscription_items = stripe_subscription.items.filter( - price__product__id__in=available_stripe_products_id - ).select_related("price__product") - final_rtd_feature = None - for subscription_item in subscription_items: - rtd_feature = settings.RTD_PRODUCTS[ - subscription_item.price.product.id - ].features[feature_type] - if final_rtd_feature is None: - final_rtd_feature = rtd_feature * subscription_item.quantity - else: - final_rtd_feature += rtd_feature * subscription_item.quantity - if final_rtd_feature: - return final_rtd_feature - + if not settings.RTD_PRODUCTS or not settings.RTD_ALLOW_ORGANIZATIONS: + return settings.RTD_DEFAULT_FEATURES.get(feature_type) + + from readthedocs.organizations.models import Organization + from readthedocs.projects.models import Project + + if isinstance(obj, Project): + organization = obj.organizations.first() + elif isinstance(obj, Organization): + organization = obj + else: + raise TypeError + + # This happens when running tests on .com only. + # In production projects are always associated with an organization. + if not organization: + return settings.RTD_DEFAULT_FEATURES.get(feature_type) + + # A subscription can have multiple products, but we only want + # the products from the organization that has the feature we are looking for. + available_stripe_products_id = [ + product.stripe_id for product in get_products_with_feature(feature_type) + ] + stripe_subscription = organization.get_or_create_stripe_subscription() + if stripe_subscription: + subscription_items = stripe_subscription.items.filter( + price__product__id__in=available_stripe_products_id + ).select_related("price__product") + final_rtd_feature = None + for subscription_item in subscription_items: + rtd_feature = settings.RTD_PRODUCTS[ + subscription_item.price.product.id + ].features[feature_type] + if final_rtd_feature is None: + final_rtd_feature = rtd_feature * subscription_item.quantity + else: + final_rtd_feature += rtd_feature * subscription_item.quantity + if final_rtd_feature: + return final_rtd_feature + + # Fallback to the default feature if the organization + # doesn't have a subscription with the feature. return settings.RTD_DEFAULT_FEATURES.get(feature_type) From 83e26d13056fe0887f77c64ed8815a01a8770c20 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 10 Aug 2023 12:32:28 -0500 Subject: [PATCH 35/35] Update from review --- docs/dev/subscriptions.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/dev/subscriptions.rst b/docs/dev/subscriptions.rst index b5359649b31..df994c057fd 100644 --- a/docs/dev/subscriptions.rst +++ b/docs/dev/subscriptions.rst @@ -36,7 +36,7 @@ Modeling -------- Subscriptions are attached to an organization (customer), -and can have multiple prices of products attached to it. +and can have multiple products attached to it. A product can have multiple prices, usually monthly and yearly. When a user subscribes to a plan (product), they are subscribing to a price of a product,