From fbbb39624cc966d2c3971aee67d343d8f376aaa0 Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Wed, 15 Jan 2025 16:00:00 -0800 Subject: [PATCH 01/18] shared stuff should be migrated with this, first try with available_plans --- shared/django_apps/codecov_auth/models.py | 29 ++++---- .../services/org_level_token_service.py | 8 ++- shared/plan/constants.py | 9 ++- shared/plan/service.py | 71 ++++++++++--------- 4 files changed, 62 insertions(+), 55 deletions(-) diff --git a/shared/django_apps/codecov_auth/models.py b/shared/django_apps/codecov_auth/models.py index fb800bc57..3880c5c2b 100644 --- a/shared/django_apps/codecov_auth/models.py +++ b/shared/django_apps/codecov_auth/models.py @@ -1,12 +1,11 @@ +import binascii import logging import os import uuid -from dataclasses import asdict from datetime import datetime from hashlib import md5 -from typing import Self, Optional +from typing import Optional, Self -import binascii from django.contrib.postgres.fields import ArrayField, CITextField from django.contrib.sessions.models import Session as DjangoSession from django.db import models @@ -30,7 +29,7 @@ from shared.django_apps.codecov_auth.managers import OwnerManager from shared.django_apps.core.managers import RepositoryManager from shared.django_apps.core.models import DateTimeWithoutTZField, Repository -from shared.plan.constants import USER_PLAN_REPRESENTATIONS, PlanName +from shared.plan.constants import PlanName # Added to avoid 'doesn't declare an explicit app_label and isn't in an application in INSTALLED_APPS' error\ # Needs to be called the same as the API app @@ -221,10 +220,9 @@ def pretty_plan(self) -> dict | None: This is how we represent the details of a plan to a user, see plan.constants.py We inject quantity to make plan management easier on api, see PlanSerializer """ - if self.plan in USER_PLAN_REPRESENTATIONS: - plan_details = asdict(USER_PLAN_REPRESENTATIONS[self.plan]) - plan_details.update({"quantity": self.plan_seat_count}) - return plan_details + plan_details = Plan.objects.get(name=self.plan) + plan_details.update({"quantity": self.plan_seat_count}) + return plan_details def can_activate_user(self, user: User | None = None) -> bool: """ @@ -593,16 +591,15 @@ def avatar_url(self, size=DEFAULT_AVATAR_SIZE): def pretty_plan(self): if self.account: return self.account.pretty_plan - if self.plan in USER_PLAN_REPRESENTATIONS: - plan_details = asdict(USER_PLAN_REPRESENTATIONS[self.plan]) - # update with quantity they've purchased - # allows api users to update the quantity - # by modifying the "plan", sidestepping - # some iffy data modeling + plan_details = Plan.objects.get(name=self.plan) + # update with quantity they've purchased + # allows api users to update the quantity + # by modifying the "plan", sidestepping + # some iffy data modeling - plan_details.update({"quantity": self.plan_user_count}) - return plan_details + plan_details.update({"quantity": self.plan_user_count}) + return plan_details def can_activate_user(self, owner_user: Self) -> bool: owner_org = self diff --git a/shared/django_apps/codecov_auth/services/org_level_token_service.py b/shared/django_apps/codecov_auth/services/org_level_token_service.py index 508b3de81..b5d5dcb10 100644 --- a/shared/django_apps/codecov_auth/services/org_level_token_service.py +++ b/shared/django_apps/codecov_auth/services/org_level_token_service.py @@ -5,8 +5,7 @@ from django.dispatch import receiver from django.forms import ValidationError -from shared.django_apps.codecov_auth.models import OrganizationLevelToken, Owner -from shared.plan.constants import USER_PLAN_REPRESENTATIONS +from shared.django_apps.codecov_auth.models import OrganizationLevelToken, Owner, Plan log = logging.getLogger(__name__) @@ -18,9 +17,12 @@ class OrgLevelTokenService(object): -- only 1 token per Owner """ + # MIGHT BE ABLE TO REMOVE THIS AND SUBSEQUENT DOWNSTREAM STUFF @classmethod def org_can_have_upload_token(cls, org: Owner): - return org.plan in USER_PLAN_REPRESENTATIONS + return org.plan in Plan.objects.filter(is_active=True).values_list( + "name", flat=True + ) @classmethod def get_or_create_org_token(cls, org: Owner): diff --git a/shared/plan/constants.py b/shared/plan/constants.py index 6823ef850..d1d85ba94 100644 --- a/shared/plan/constants.py +++ b/shared/plan/constants.py @@ -73,6 +73,8 @@ class TierName(enum.Enum): TEAM = "team" PRO = "pro" ENTERPRISE = "enterprise" + SENTRY = "sentry" + TRIAL = "trial" @dataclass(repr=False) @@ -100,12 +102,13 @@ def convert_to_DTO(self) -> dict: "tier_name": self.tier_name, "monthly_uploads_limit": self.monthly_uploads_limit, "trial_days": self.trial_days, - "is_free_plan": self.tier_name == TierName.BASIC.value, + "is_free_plan": not self.paid_plan, "is_pro_plan": self.tier_name == TierName.PRO.value, "is_team_plan": self.tier_name == TierName.TEAM.value, "is_enterprise_plan": self.tier_name == TierName.ENTERPRISE.value, - "is_trial_plan": self.value == PlanName.TRIAL_PLAN_NAME.value, - "is_sentry_plan": self.value in SENTRY_PAID_USER_PLAN_REPRESENTATIONS, + "is_trial_plan": self.value + == PlanName.TRIAL_PLAN_NAME.value, # This could also use the trial "Tier" if we want to make one for that... + "is_sentry_plan": self.tier_name == TierName.SENTRY.value, } diff --git a/shared/plan/service.py b/shared/plan/service.py index cf0b8b23b..4a38f4002 100644 --- a/shared/plan/service.py +++ b/shared/plan/service.py @@ -5,22 +5,14 @@ from shared.billing import is_pr_billing_plan from shared.config import get_config from shared.django_apps.codecov.commands.exceptions import ValidationError -from shared.django_apps.codecov_auth.models import Owner, Service +from shared.django_apps.codecov_auth.models import Owner, Plan, Service from shared.plan.constants import ( - BASIC_PLAN, - ENTERPRISE_CLOUD_USER_PLAN_REPRESENTATIONS, - FREE_PLAN, - FREE_PLAN_REPRESENTATIONS, - PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS, - SENTRY_PAID_USER_PLAN_REPRESENTATIONS, TEAM_PLAN_MAX_USERS, - TEAM_PLAN_REPRESENTATIONS, - TRIAL_PLAN_REPRESENTATION, TRIAL_PLAN_SEATS, - USER_PLAN_REPRESENTATIONS, PlanBillingRate, PlanData, PlanName, + TierName, TrialDaysAmount, TrialStatus, ) @@ -55,19 +47,20 @@ def __init__(self, current_org: Owner): self.current_org = current_org.root_organization else: self.current_org = current_org - if self.current_org.plan not in USER_PLAN_REPRESENTATIONS: + + if self.current_org.plan not in Plan.objects.values_list("name", flat=True): raise ValueError("Unsupported plan") self._plan_data = None def update_plan(self, name: str, user_count: Optional[int]) -> None: """Updates the organization's plan and user count.""" - if name not in USER_PLAN_REPRESENTATIONS: + if name not in Plan.objects.values_list("name", flat=True): raise ValueError("Unsupported plan") if not user_count: raise ValueError("Quantity Needed") self.current_org.plan = name self.current_org.plan_user_count = user_count - self._plan_data = USER_PLAN_REPRESENTATIONS[self.current_org.plan] + self._plan_data = Plan.objects.get(name=self.current_org.plan) self.current_org.delinquent = False self.current_org.save() @@ -92,8 +85,8 @@ def has_account(self) -> bool: def plan_data(self) -> PlanData: """Returns the plan data for the organization, either from account or default.""" if self._plan_data is None: - self._plan_data = USER_PLAN_REPRESENTATIONS.get( - self.current_org.account.plan + self._plan_data = Plan.objects.get( + name=self.current_org.account.plan if self.has_account else self.current_org.plan ) @@ -159,27 +152,33 @@ def monthly_uploads_limit(self) -> Optional[int]: return self.plan_data.monthly_uploads_limit @property - def tier_name(self) -> str: + def tier_name(self) -> TierName: """Returns the tier name of the plan.""" return self.plan_data.tier_name def available_plans(self, owner: Owner) -> List[PlanData]: """Returns the available plans for the owner and organization.""" - available_plans = [BASIC_PLAN] + available_plans = {Plan.objects.get(name=PlanName.BASIC_PLAN_NAME.value)} - if self.plan_name == FREE_PLAN.value: - available_plans.append(FREE_PLAN) + curr_plan = Plan.objects.get(name=self.plan_name) + if not curr_plan.paid_plan: + available_plans.add(curr_plan) - available_plans += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() + # Build list of available tiers based on conditions + available_tiers = [TierName.PRO.value] if is_sentry_user(owner): - available_plans += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() + available_tiers.append(TierName.SENTRY.value) if ( - self.plan_activated_users is None + not self.plan_activated_users or len(self.plan_activated_users) <= TEAM_PLAN_MAX_USERS ): - available_plans += TEAM_PLAN_REPRESENTATIONS.values() + available_tiers.append(TierName.TEAM.value) + + available_plans.update( + Plan.objects.filter(tier_name__in=available_tiers, is_active=True) + ) return [plan.convert_to_DTO() for plan in available_plans] @@ -222,7 +221,9 @@ def start_trial(self, current_owner: Owner) -> None: """ if self.trial_status != TrialStatus.NOT_STARTED.value: raise ValidationError("Cannot start an existing trial") - if self.plan_name not in FREE_PLAN_REPRESENTATIONS: + if self.plan_name not in Plan.objects.filter(paid_plan=False).values_list( + "name", flat=True + ): raise ValidationError("Cannot trial from a paid plan") self._start_trial_helper(current_owner) @@ -236,10 +237,14 @@ def start_trial_manually(self, current_owner: Owner, end_date: datetime) -> None No value """ # Start a new trial plan for free users currently not on trial - if self.plan_name in FREE_PLAN_REPRESENTATIONS: + if self.plan_name in Plan.objects.filter(paid_plan=False).values_list( + "name", flat=True + ): self._start_trial_helper(current_owner, end_date, is_extension=False) # Extend an existing trial plan for users currently on trial - elif self.plan_name in TRIAL_PLAN_REPRESENTATION: + elif self.plan_name in Plan.objects.filter( + tier=TierName.TRIAL.value, is_active=True + ).values_list("name", flat=True): self._start_trial_helper(current_owner, end_date, is_extension=True) # Paying users cannot start a trial else: @@ -324,30 +329,30 @@ def has_seats_left(self) -> bool: @property def is_enterprise_plan(self) -> bool: - return self.plan_name in ENTERPRISE_CLOUD_USER_PLAN_REPRESENTATIONS + return self.plan_data.tier_name == TierName.ENTERPRISE.value @property def is_free_plan(self) -> bool: - return self.plan_name in FREE_PLAN_REPRESENTATIONS + return self.plan_data.paid_plan is False @property def is_pro_plan(self) -> bool: return ( - self.plan_name in PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS - or self.plan_name in SENTRY_PAID_USER_PLAN_REPRESENTATIONS + self.plan_data.tier_name == TierName.PRO.value + or self.plan_data.tier_name == TierName.SENTRY.value ) @property def is_sentry_plan(self) -> bool: - return self.plan_name in SENTRY_PAID_USER_PLAN_REPRESENTATIONS + return self.plan_data.tier_name == TierName.SENTRY.value @property def is_team_plan(self) -> bool: - return self.plan_name in TEAM_PLAN_REPRESENTATIONS + return self.plan_data.tier_name == TierName.TEAM.value @property def is_trial_plan(self) -> bool: - return self.plan_name in TRIAL_PLAN_REPRESENTATION + return self.plan_data.tier_name == TierName.TRIAL.value @property def is_pr_billing_plan(self) -> bool: From 33d98f6cfb54773e362efa038ed4fbb67b6958a4 Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Fri, 17 Jan 2025 13:16:15 -0800 Subject: [PATCH 02/18] boilerplate updates, better way to do the plan searches --- .../services/org_level_token_service.py | 4 +-- shared/plan/constants.py | 33 ++++++++++++++---- shared/plan/service.py | 34 ++++++++----------- 3 files changed, 43 insertions(+), 28 deletions(-) diff --git a/shared/django_apps/codecov_auth/services/org_level_token_service.py b/shared/django_apps/codecov_auth/services/org_level_token_service.py index b5d5dcb10..a72617e22 100644 --- a/shared/django_apps/codecov_auth/services/org_level_token_service.py +++ b/shared/django_apps/codecov_auth/services/org_level_token_service.py @@ -20,9 +20,7 @@ class OrgLevelTokenService(object): # MIGHT BE ABLE TO REMOVE THIS AND SUBSEQUENT DOWNSTREAM STUFF @classmethod def org_can_have_upload_token(cls, org: Owner): - return org.plan in Plan.objects.filter(is_active=True).values_list( - "name", flat=True - ) + return Plan.objects.filter(name=org.plan, is_active=True).exists() @classmethod def get_or_create_org_token(cls, org: Owner): diff --git a/shared/plan/constants.py b/shared/plan/constants.py index d1d85ba94..2c6ea6c4e 100644 --- a/shared/plan/constants.py +++ b/shared/plan/constants.py @@ -2,6 +2,8 @@ from dataclasses import dataclass from typing import List, Optional +from shared.django_apps.codecov_auth.models import Plan, Tier + class MonthlyUploadLimits(enum.Enum): CODECOV_BASIC_PLAN = 250 @@ -88,7 +90,7 @@ class PlanData: billing_rate: Optional[PlanBillingRate] base_unit_price: PlanPrice benefits: List[str] - tier_name: TierName + tier: Tier monthly_uploads_limit: Optional[MonthlyUploadLimits] trial_days: Optional[TrialDaysAmount] @@ -99,19 +101,38 @@ def convert_to_DTO(self) -> dict: "billing_rate": self.billing_rate, "base_unit_price": self.base_unit_price, "benefits": self.benefits, - "tier_name": self.tier_name, + "tier_name": self.tier.tier_name, "monthly_uploads_limit": self.monthly_uploads_limit, "trial_days": self.trial_days, "is_free_plan": not self.paid_plan, - "is_pro_plan": self.tier_name == TierName.PRO.value, - "is_team_plan": self.tier_name == TierName.TEAM.value, - "is_enterprise_plan": self.tier_name == TierName.ENTERPRISE.value, + "is_pro_plan": self.tier.tier_name == TierName.PRO.value, + "is_team_plan": self.tier.tier_name == TierName.TEAM.value, + "is_enterprise_plan": self.tier.tier_name == TierName.ENTERPRISE.value, "is_trial_plan": self.value == PlanName.TRIAL_PLAN_NAME.value, # This could also use the trial "Tier" if we want to make one for that... - "is_sentry_plan": self.tier_name == TierName.SENTRY.value, + "is_sentry_plan": self.tier.tier_name == TierName.SENTRY.value, } +def convert_to_DTO(plan: Plan) -> dict: + return { + "marketing_name": plan.marketing_name, + "value": plan.name, + "billing_rate": plan.billing_rate, + "base_unit_price": plan.base_unit_price, + "benefits": plan.benefits, + "tier_name": plan.tier.tier_name, + "monthly_uploads_limit": plan.monthly_uploads_limit, + "trial_days": plan.trial_days, + "is_free_plan": not plan.paid_plan, + "is_pro_plan": plan.tier.tier_name == TierName.PRO.value, + "is_team_plan": plan.tier.tier_name == TierName.TEAM.value, + "is_enterprise_plan": plan.tier.tier_name == TierName.ENTERPRISE.value, + "is_trial_plan": plan.tier.tier_name == TierName.TRIAL.value, + "is_sentry_plan": plan.tier.tier_name == TierName.SENTRY.value, + } + + NON_PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS = { PlanName.CODECOV_PRO_MONTHLY_LEGACY.value: PlanData( marketing_name=PlanMarketingName.CODECOV_PRO.value, diff --git a/shared/plan/service.py b/shared/plan/service.py index 4a38f4002..a34e80044 100644 --- a/shared/plan/service.py +++ b/shared/plan/service.py @@ -10,11 +10,11 @@ TEAM_PLAN_MAX_USERS, TRIAL_PLAN_SEATS, PlanBillingRate, - PlanData, PlanName, TierName, TrialDaysAmount, TrialStatus, + convert_to_DTO, ) from shared.self_hosted.service import enterprise_has_seats_left, license_seats @@ -48,19 +48,19 @@ def __init__(self, current_org: Owner): else: self.current_org = current_org - if self.current_org.plan not in Plan.objects.values_list("name", flat=True): + if not Plan.objects.filter(name=self.current_org.plan).exists(): raise ValueError("Unsupported plan") self._plan_data = None def update_plan(self, name: str, user_count: Optional[int]) -> None: """Updates the organization's plan and user count.""" - if name not in Plan.objects.values_list("name", flat=True): + if not Plan.objects.filter(name=name).exists(): raise ValueError("Unsupported plan") if not user_count: raise ValueError("Quantity Needed") self.current_org.plan = name self.current_org.plan_user_count = user_count - self._plan_data = Plan.objects.get(name=self.current_org.plan) + self._plan_data = Plan.objects.get(name=name) self.current_org.delinquent = False self.current_org.save() @@ -82,7 +82,7 @@ def has_account(self) -> bool: return self.current_org.account is not None @property - def plan_data(self) -> PlanData: + def plan_data(self) -> Plan: """Returns the plan data for the organization, either from account or default.""" if self._plan_data is None: self._plan_data = Plan.objects.get( @@ -93,7 +93,7 @@ def plan_data(self) -> PlanData: return self._plan_data @plan_data.setter - def plan_data(self, plan_data: Optional[PlanData]) -> None: + def plan_data(self, plan_data: Optional[Plan]) -> None: """Sets the plan data directly.""" self._plan_data = plan_data @@ -154,9 +154,9 @@ def monthly_uploads_limit(self) -> Optional[int]: @property def tier_name(self) -> TierName: """Returns the tier name of the plan.""" - return self.plan_data.tier_name + return self.plan_data.tier.tier_name - def available_plans(self, owner: Owner) -> List[PlanData]: + def available_plans(self, owner: Owner) -> List[Plan]: """Returns the available plans for the owner and organization.""" available_plans = {Plan.objects.get(name=PlanName.BASIC_PLAN_NAME.value)} @@ -177,10 +177,10 @@ def available_plans(self, owner: Owner) -> List[PlanData]: available_tiers.append(TierName.TEAM.value) available_plans.update( - Plan.objects.filter(tier_name__in=available_tiers, is_active=True) + Plan.objects.filter(tier__tier_name__in=available_tiers, is_active=True) ) - return [plan.convert_to_DTO() for plan in available_plans] + return [convert_to_DTO(plan) for plan in available_plans] def _start_trial_helper( self, @@ -221,9 +221,7 @@ def start_trial(self, current_owner: Owner) -> None: """ if self.trial_status != TrialStatus.NOT_STARTED.value: raise ValidationError("Cannot start an existing trial") - if self.plan_name not in Plan.objects.filter(paid_plan=False).values_list( - "name", flat=True - ): + if not Plan.objects.filter(name=self.plan_name, paid_plan=False).exists(): raise ValidationError("Cannot trial from a paid plan") self._start_trial_helper(current_owner) @@ -237,14 +235,12 @@ def start_trial_manually(self, current_owner: Owner, end_date: datetime) -> None No value """ # Start a new trial plan for free users currently not on trial - if self.plan_name in Plan.objects.filter(paid_plan=False).values_list( - "name", flat=True - ): + + plan = Plan.objects.get(name=self.plan_name) + if plan.paid_plan is False: self._start_trial_helper(current_owner, end_date, is_extension=False) # Extend an existing trial plan for users currently on trial - elif self.plan_name in Plan.objects.filter( - tier=TierName.TRIAL.value, is_active=True - ).values_list("name", flat=True): + elif plan.tier.tier_name == TierName.TRIAL.value: self._start_trial_helper(current_owner, end_date, is_extension=True) # Paying users cannot start a trial else: From ba68a8bb67edc2a07d6ed60f0d43acb6d240f74e Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Fri, 17 Jan 2025 14:03:47 -0800 Subject: [PATCH 03/18] update logic a bit to work with new plan, and types as well, start fixing tests --- shared/django_apps/codecov_auth/models.py | 5 ++-- .../codecov_auth/tests/factories.py | 24 ++++++++++++------- shared/plan/constants.py | 21 +++++++--------- shared/plan/service.py | 14 +++++------ 4 files changed, 34 insertions(+), 30 deletions(-) diff --git a/shared/django_apps/codecov_auth/models.py b/shared/django_apps/codecov_auth/models.py index 27aa1730f..f02367e44 100644 --- a/shared/django_apps/codecov_auth/models.py +++ b/shared/django_apps/codecov_auth/models.py @@ -598,8 +598,9 @@ def pretty_plan(self): # by modifying the "plan", sidestepping # some iffy data modeling - plan_details.update({"quantity": self.plan_user_count}) - return plan_details + if plan_details: + plan_details.update({"quantity": self.plan_user_count}) + return plan_details def can_activate_user(self, owner_user: Self) -> bool: owner_org = self diff --git a/shared/django_apps/codecov_auth/tests/factories.py b/shared/django_apps/codecov_auth/tests/factories.py index 65c2a5bda..c3fe02b6b 100644 --- a/shared/django_apps/codecov_auth/tests/factories.py +++ b/shared/django_apps/codecov_auth/tests/factories.py @@ -23,7 +23,7 @@ UserToken, ) from shared.encryption.oauth import get_encryptor_from_configuration -from shared.plan.constants import TrialStatus +from shared.plan.constants import PlanName, TierName, TrialStatus encryptor = get_encryptor_from_configuration() @@ -177,20 +177,26 @@ class TierFactory(DjangoModelFactory): class Meta: model = Tier - tier_name = factory.Faker("name") + tier_name = TierName.BASIC.value + bundle_analysis = False + test_analytics = False + flaky_test_detection = False + project_coverage = False + private_repo_support = False class PlanFactory(DjangoModelFactory): class Meta: model = Plan - base_unit_price = factory.Faker("pyint") - benefits = [] + tier = factory.SubFactory(TierFactory) + base_unit_price = 0 + benefits = factory.LazyFunction(lambda: ["Benefit 1", "Benefit 2", "Benefit 3"]) billing_rate = None is_active = True - marketing_name = factory.Faker("name") - max_seats = None + marketing_name = factory.Faker("catch_phrase") + max_seats = 1 monthly_uploads_limit = None - paid_plan = True - name = factory.Faker("name") - tier = factory.SubFactory(TierFactory) + name = PlanName.BASIC_PLAN_NAME.value + paid_plan = False + stripe_id = None diff --git a/shared/plan/constants.py b/shared/plan/constants.py index 2c6ea6c4e..35f691d43 100644 --- a/shared/plan/constants.py +++ b/shared/plan/constants.py @@ -2,8 +2,6 @@ from dataclasses import dataclass from typing import List, Optional -from shared.django_apps.codecov_auth.models import Plan, Tier - class MonthlyUploadLimits(enum.Enum): CODECOV_BASIC_PLAN = 250 @@ -90,7 +88,7 @@ class PlanData: billing_rate: Optional[PlanBillingRate] base_unit_price: PlanPrice benefits: List[str] - tier: Tier + tier_name: TierName monthly_uploads_limit: Optional[MonthlyUploadLimits] trial_days: Optional[TrialDaysAmount] @@ -101,20 +99,19 @@ def convert_to_DTO(self) -> dict: "billing_rate": self.billing_rate, "base_unit_price": self.base_unit_price, "benefits": self.benefits, - "tier_name": self.tier.tier_name, + "tier_name": self.tier_name, "monthly_uploads_limit": self.monthly_uploads_limit, "trial_days": self.trial_days, - "is_free_plan": not self.paid_plan, - "is_pro_plan": self.tier.tier_name == TierName.PRO.value, - "is_team_plan": self.tier.tier_name == TierName.TEAM.value, - "is_enterprise_plan": self.tier.tier_name == TierName.ENTERPRISE.value, - "is_trial_plan": self.value - == PlanName.TRIAL_PLAN_NAME.value, # This could also use the trial "Tier" if we want to make one for that... - "is_sentry_plan": self.tier.tier_name == TierName.SENTRY.value, + "is_free_plan": self.tier_name == TierName.BASIC.value, + "is_pro_plan": self.tier_name == TierName.PRO.value, + "is_team_plan": self.tier_name == TierName.TEAM.value, + "is_enterprise_plan": self.tier_name == TierName.ENTERPRISE.value, + "is_trial_plan": self.value == PlanName.TRIAL_PLAN_NAME.value, + "is_sentry_plan": self.value in SENTRY_PAID_USER_PLAN_REPRESENTATIONS, } -def convert_to_DTO(plan: Plan) -> dict: +def convert_to_DTO(plan) -> dict: return { "marketing_name": plan.marketing_name, "value": plan.name, diff --git a/shared/plan/service.py b/shared/plan/service.py index a34e80044..6dd918395 100644 --- a/shared/plan/service.py +++ b/shared/plan/service.py @@ -100,7 +100,7 @@ def plan_data(self, plan_data: Optional[Plan]) -> None: @property def plan_name(self) -> str: """Returns the name of the organization's current plan.""" - return self.plan_data.value + return self.plan_data.name @property def plan_user_count(self) -> int: @@ -325,7 +325,7 @@ def has_seats_left(self) -> bool: @property def is_enterprise_plan(self) -> bool: - return self.plan_data.tier_name == TierName.ENTERPRISE.value + return self.plan_data.tier.tier_name == TierName.ENTERPRISE.value @property def is_free_plan(self) -> bool: @@ -334,21 +334,21 @@ def is_free_plan(self) -> bool: @property def is_pro_plan(self) -> bool: return ( - self.plan_data.tier_name == TierName.PRO.value - or self.plan_data.tier_name == TierName.SENTRY.value + self.plan_data.tier.tier_name == TierName.PRO.value + or self.plan_data.tier.tier_name == TierName.SENTRY.value ) @property def is_sentry_plan(self) -> bool: - return self.plan_data.tier_name == TierName.SENTRY.value + return self.plan_data.tier.tier_name == TierName.SENTRY.value @property def is_team_plan(self) -> bool: - return self.plan_data.tier_name == TierName.TEAM.value + return self.plan_data.tier.tier_name == TierName.TEAM.value @property def is_trial_plan(self) -> bool: - return self.plan_data.tier_name == TierName.TRIAL.value + return self.plan_data.tier.tier_name == TierName.TRIAL.value @property def is_pr_billing_plan(self) -> bool: From 3b1deb6d08c5ec122e23fb7a2d538a62a94b38bc Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Fri, 17 Jan 2025 16:12:53 -0800 Subject: [PATCH 04/18] 43 tests failing locally --- tests/unit/plan/test_plan.py | 114 ++++++++++++++++++++++++----------- 1 file changed, 78 insertions(+), 36 deletions(-) diff --git a/tests/unit/plan/test_plan.py b/tests/unit/plan/test_plan.py index c1258f4eb..e8517fc19 100644 --- a/tests/unit/plan/test_plan.py +++ b/tests/unit/plan/test_plan.py @@ -6,7 +6,11 @@ from shared.django_apps.codecov.commands.exceptions import ValidationError from shared.django_apps.codecov_auth.models import Service -from shared.django_apps.codecov_auth.tests.factories import OwnerFactory +from shared.django_apps.codecov_auth.tests.factories import ( + OwnerFactory, + PlanFactory, + TierFactory, +) from shared.plan.constants import ( BASIC_PLAN, FREE_PLAN, @@ -17,6 +21,7 @@ TRIAL_PLAN_REPRESENTATION, TRIAL_PLAN_SEATS, PlanName, + TierName, TrialDaysAmount, TrialStatus, ) @@ -61,8 +66,9 @@ def test_plan_service_trial_status_ongoing(self): def test_plan_service_expire_trial_when_upgrading_successful_if_trial_is_not_started( self, ): + plan = PlanFactory() current_org_with_ongoing_trial = OwnerFactory( - plan=PlanName.BASIC_PLAN_NAME.value, + plan=plan.name, trial_start_date=None, trial_end_date=None, trial_status=TrialStatus.NOT_STARTED.value, @@ -79,8 +85,9 @@ def test_plan_service_expire_trial_when_upgrading_successful_if_trial_is_ongoing ): trial_start_date = datetime.utcnow() trial_end_date_ongoing = trial_start_date + timedelta(days=5) + plan = PlanFactory() current_org_with_ongoing_trial = OwnerFactory( - plan=PlanName.BASIC_PLAN_NAME.value, + plan=plan.name, trial_start_date=trial_start_date, trial_end_date=trial_end_date_ongoing, trial_status=TrialStatus.ONGOING.value, @@ -98,8 +105,9 @@ def test_plan_service_expire_trial_users_pretrial_users_count_if_existing( trial_start_date = datetime.utcnow() trial_end_date_ongoing = trial_start_date + timedelta(days=5) pretrial_users_count = 5 + plan = PlanFactory() current_org_with_ongoing_trial = OwnerFactory( - plan=PlanName.BASIC_PLAN_NAME.value, + plan=plan.name, trial_start_date=trial_start_date, trial_end_date=trial_end_date_ongoing, trial_status=TrialStatus.ONGOING.value, @@ -117,8 +125,9 @@ def test_plan_service_start_trial_errors_if_status_is_ongoing(self): trial_end_date = trial_start_date + timedelta( days=TrialDaysAmount.CODECOV_SENTRY.value ) + plan = PlanFactory() current_org = OwnerFactory( - plan=PlanName.BASIC_PLAN_NAME.value, + plan=plan.name, trial_start_date=trial_start_date, trial_end_date=trial_end_date, trial_status=TrialStatus.ONGOING.value, @@ -132,8 +141,9 @@ def test_plan_service_start_trial_errors_if_status_is_ongoing(self): def test_plan_service_start_trial_errors_if_status_is_expired(self): trial_start_date = datetime.utcnow() trial_end_date = trial_start_date + timedelta(days=-1) + plan = PlanFactory() current_org = OwnerFactory( - plan=PlanName.BASIC_PLAN_NAME.value, + plan=plan.name, trial_start_date=trial_start_date, trial_end_date=trial_end_date, trial_status=TrialStatus.EXPIRED.value, @@ -145,8 +155,9 @@ def test_plan_service_start_trial_errors_if_status_is_expired(self): plan_service.start_trial(current_owner=current_owner) def test_plan_service_start_trial_errors_if_status_is_cannot_trial(self): + plan = PlanFactory() current_org = OwnerFactory( - plan=PlanName.BASIC_PLAN_NAME.value, + plan=plan.name, trial_start_date=None, trial_end_date=None, trial_status=TrialStatus.CANNOT_TRIAL.value, @@ -158,8 +169,9 @@ def test_plan_service_start_trial_errors_if_status_is_cannot_trial(self): plan_service.start_trial(current_owner=current_owner) def test_plan_service_start_trial_errors_owners_plan_is_not_a_free_plan(self): + plan = PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY.value) current_org = OwnerFactory( - plan=PlanName.CODECOV_PRO_MONTHLY.value, + plan=plan.name, trial_start_date=None, trial_end_date=None, trial_status=TrialStatus.CANNOT_TRIAL.value, @@ -174,8 +186,9 @@ def test_plan_service_start_trial_succeeds_if_trial_has_not_started(self): trial_start_date = None trial_end_date = None plan_user_count = 5 + plan = PlanFactory() current_org = OwnerFactory( - plan=PlanName.BASIC_PLAN_NAME.value, + plan=plan.name, trial_start_date=trial_start_date, trial_end_date=trial_end_date, trial_status=TrialStatus.NOT_STARTED.value, @@ -200,8 +213,9 @@ def test_plan_service_start_trial_manually(self): trial_start_date = None trial_end_date = None plan_user_count = 5 + plan = PlanFactory() current_org = OwnerFactory( - plan=PlanName.BASIC_PLAN_NAME.value, + plan=plan.name, trial_start_date=trial_start_date, trial_end_date=trial_end_date, trial_status=TrialStatus.NOT_STARTED.value, @@ -223,8 +237,9 @@ def test_plan_service_start_trial_manually(self): assert current_org.trial_fired_by == current_owner.ownerid def test_plan_service_start_trial_manually_already_on_paid_plan(self): + plan = PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY.value) current_org = OwnerFactory( - plan=PlanName.CODECOV_PRO_MONTHLY.value, + plan=plan.name, trial_start_date=None, trial_end_date=None, trial_status=TrialStatus.NOT_STARTED.value, @@ -240,8 +255,9 @@ def test_plan_service_start_trial_manually_already_on_paid_plan(self): def test_plan_service_returns_plan_data_for_non_trial_basic_plan(self): trial_start_date = None trial_end_date = None + plan = PlanFactory() current_org = OwnerFactory( - plan=PlanName.BASIC_PLAN_NAME.value, + plan=plan.name, trial_start_date=trial_start_date, trial_end_date=trial_end_date, ) @@ -269,8 +285,9 @@ def test_plan_service_returns_plan_data_for_trialing_user_trial_plan(self): trial_end_date = datetime.utcnow() + timedelta( days=TrialDaysAmount.CODECOV_SENTRY.value ) + plan = PlanFactory(name=PlanName.TRIAL_PLAN_NAME.value) current_org = OwnerFactory( - plan=PlanName.TRIAL_PLAN_NAME.value, + plan=plan.name, trial_start_date=trial_start_date, trial_end_date=trial_end_date, trial_status=TrialStatus.ONGOING.value, @@ -289,8 +306,9 @@ def test_plan_service_returns_plan_data_for_trialing_user_trial_plan(self): assert plan_service.trial_total_days == trial_plan.trial_days def test_plan_service_sets_default_plan_data_values_correctly(self): + plan = PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY.value) current_org = OwnerFactory( - plan=PlanName.CODECOV_PRO_MONTHLY.value, + plan=plan.name, stripe_subscription_id="test-sub-123", plan_user_count=20, plan_activated_users=[44], @@ -307,8 +325,9 @@ def test_plan_service_sets_default_plan_data_values_correctly(self): assert current_org.stripe_subscription_id is None def test_plan_service_returns_if_owner_has_trial_dates(self): + plan = PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY.value) current_org = OwnerFactory( - plan=PlanName.CODECOV_PRO_MONTHLY.value, + plan=plan.name, trial_start_date=datetime.utcnow(), trial_end_date=datetime.utcnow() + timedelta(days=14), ) @@ -319,9 +338,11 @@ def test_plan_service_returns_if_owner_has_trial_dates(self): assert plan_service.has_trial_dates == True def test_plan_service_gitlab_with_root_org(self): + tier = TierFactory(tier_name=TierName.BASIC.value) + plan = PlanFactory(name=PlanName.FREE_PLAN_NAME.value, tier=tier) root_owner_org = OwnerFactory( service=Service.GITLAB.value, - plan=PlanName.FREE_PLAN_NAME.value, + plan=plan.name, plan_user_count=1, service_id="1234", ) @@ -330,9 +351,11 @@ def test_plan_service_gitlab_with_root_org(self): service_id="5678", parent_service_id=root_owner_org.service_id, ) + tier2 = TierFactory(tier_name=TierName.PRO.value) + plan2 = PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY.value, tier=tier2) child_owner_org = OwnerFactory( service=Service.GITLAB.value, - plan=PlanName.CODECOV_PRO_MONTHLY.value, + plan=plan2.name, plan_user_count=20, parent_service_id=middle_org.service_id, ) @@ -369,7 +392,8 @@ def setUp(self): def test_available_plans_for_basic_plan_non_trial( self, ): - self.current_org.plan = PlanName.BASIC_PLAN_NAME.value + plan = PlanFactory(name=PlanName.BASIC_PLAN_NAME.value) + self.current_org.plan = plan.name self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -385,7 +409,8 @@ def test_available_plans_for_basic_plan_non_trial( def test_available_plans_for_free_plan_non_trial( self, ): - self.current_org.plan = PlanName.FREE_PLAN_NAME.value + plan = PlanFactory(name=PlanName.FREE_PLAN_NAME.value) + self.current_org.plan = plan.name self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -402,7 +427,8 @@ def test_available_plans_for_free_plan_non_trial( def test_available_plans_for_team_plan_non_trial( self, ): - self.current_org.plan = PlanName.TEAM_MONTHLY.value + plan = PlanFactory(name=PlanName.TEAM_MONTHLY.value) + self.current_org.plan = plan.name self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -416,7 +442,8 @@ def test_available_plans_for_team_plan_non_trial( assert plan_service.available_plans(owner=self.owner) == expected_result def test_available_plans_for_pro_plan_non_trial(self): - self.current_org.plan = PlanName.CODECOV_PRO_MONTHLY.value + plan = PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY.value) + self.current_org.plan = plan.name self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -434,7 +461,8 @@ def test_available_plans_for_sentry_customer_basic_plan_non_trial( self, is_sentry_user ): is_sentry_user.return_value = True - self.current_org.plan = PlanName.BASIC_PLAN_NAME.value + plan = PlanFactory(name=PlanName.BASIC_PLAN_NAME.value) + self.current_org.plan = plan.name self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -453,7 +481,8 @@ def test_available_plans_for_sentry_customer_team_plan_non_trial( self, is_sentry_user ): is_sentry_user.return_value = True - self.current_org.plan = PlanName.TEAM_MONTHLY.value + plan = PlanFactory(name=PlanName.TEAM_MONTHLY.value) + self.current_org.plan = plan.name self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -470,7 +499,8 @@ def test_available_plans_for_sentry_customer_team_plan_non_trial( @patch("shared.plan.service.is_sentry_user") def test_available_plans_for_sentry_plan_non_trial(self, is_sentry_user): is_sentry_user.return_value = True - self.current_org.plan = PlanName.SENTRY_MONTHLY.value + plan = PlanFactory(name=PlanName.SENTRY_MONTHLY.value) + self.current_org.plan = plan.name self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -508,7 +538,8 @@ def setUp(self): def test_available_plans_for_basic_plan_expired_trial_less_than_10_users( self, ): - self.current_org.plan = PlanName.BASIC_PLAN_NAME.value + plan = PlanFactory(name=PlanName.BASIC_PLAN_NAME.value) + self.current_org.plan = plan.name self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -524,7 +555,8 @@ def test_available_plans_for_basic_plan_expired_trial_less_than_10_users( def test_available_plans_for_team_plan_expired_trial_less_than_10_users( self, ): - self.current_org.plan = PlanName.TEAM_MONTHLY.value + plan = PlanFactory(name=PlanName.TEAM_MONTHLY.value) + self.current_org.plan = plan.name self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -538,7 +570,8 @@ def test_available_plans_for_team_plan_expired_trial_less_than_10_users( assert plan_service.available_plans(owner=self.owner) == expected_result def test_available_plans_for_pro_plan_expired_trial_less_than_10_users(self): - self.current_org.plan = PlanName.CODECOV_PRO_MONTHLY.value + plan = PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY.value) + self.current_org.plan = plan.name self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -556,7 +589,8 @@ def test_available_plans_for_sentry_customer_basic_plan_expired_trial_less_than_ self, is_sentry_user ): is_sentry_user.return_value = True - self.current_org.plan = PlanName.BASIC_PLAN_NAME.value + plan = PlanFactory(name=PlanName.BASIC_PLAN_NAME.value) + self.current_org.plan = plan.name self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -575,7 +609,8 @@ def test_available_plans_for_sentry_customer_team_plan_expired_trial_less_than_1 self, is_sentry_user ): is_sentry_user.return_value = True - self.current_org.plan = PlanName.TEAM_MONTHLY.value + plan = PlanFactory(name=PlanName.TEAM_MONTHLY.value) + self.current_org.plan = plan.name self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -594,7 +629,8 @@ def test_available_plans_for_sentry_plan_expired_trial_less_than_10_users( self, is_sentry_user ): is_sentry_user.return_value = True - self.current_org.plan = PlanName.SENTRY_MONTHLY.value + plan = PlanFactory(name=PlanName.SENTRY_MONTHLY.value) + self.current_org.plan = plan.name self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -628,7 +664,8 @@ def setUp(self): self.owner = OwnerFactory() def test_available_plans_for_pro_plan_expired_trial_more_than_10_users(self): - self.current_org.plan = PlanName.CODECOV_PRO_MONTHLY.value + plan = PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY.value) + self.current_org.plan = plan.name self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -645,7 +682,8 @@ def test_available_plans_for_sentry_customer_basic_plan_expired_trial_more_than_ self, is_sentry_user ): is_sentry_user.return_value = True - self.current_org.plan = PlanName.BASIC_PLAN_NAME.value + plan = PlanFactory(name=PlanName.BASIC_PLAN_NAME.value) + self.current_org.plan = plan.name self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -663,7 +701,8 @@ def test_available_plans_for_sentry_plan_expired_trial_more_than_10_users( self, is_sentry_user ): is_sentry_user.return_value = True - self.current_org.plan = PlanName.SENTRY_MONTHLY.value + plan = PlanFactory(name=PlanName.SENTRY_MONTHLY.value) + self.current_org.plan = plan.name self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -693,10 +732,11 @@ def setUp(self): ] def test_currently_team_plan(self): + plan = PlanFactory(name=PlanName.TEAM_MONTHLY.value) self.current_org = OwnerFactory( plan_user_count=100, plan_activated_users=[i for i in range(10)], - plan=PlanName.TEAM_MONTHLY.value, + plan=plan.name, ) self.owner = OwnerFactory() self.plan_service = PlanService(current_org=self.current_org) @@ -769,8 +809,9 @@ class AvailablePlansOngoingTrial(TestCase): """ def setUp(self): + plan = PlanFactory(name=PlanName.TRIAL_PLAN_NAME.value) self.current_org = OwnerFactory( - plan=PlanName.TRIAL_PLAN_NAME.value, + plan=plan.name, trial_start_date=datetime.utcnow(), trial_end_date=datetime.utcnow() + timedelta(days=14), trial_status=TrialStatus.ONGOING.value, @@ -847,8 +888,9 @@ def test_sentry_user(self, is_sentry_user): @override_settings(IS_ENTERPRISE=False) class PlanServiceIs___PlanTests(TestCase): def test_is_trial_plan(self): + plan = PlanFactory(name=PlanName.TRIAL_PLAN_NAME.value) self.current_org = OwnerFactory( - plan=PlanName.TRIAL_PLAN_NAME.value, + plan=plan.name, trial_start_date=datetime.utcnow(), trial_end_date=datetime.utcnow() + timedelta(days=14), trial_status=TrialStatus.ONGOING.value, From 7eb00e486839452d253ae1747e7c853747a91287 Mon Sep 17 00:00:00 2001 From: RulaKhaled Date: Mon, 20 Jan 2025 17:45:49 +0100 Subject: [PATCH 05/18] fix some tests: --- shared/plan/constants.py | 4 +- shared/plan/service.py | 2 +- tests/unit/plan/test_plan.py | 152 ++++++++++++++++++++++++++--------- 3 files changed, 115 insertions(+), 43 deletions(-) diff --git a/shared/plan/constants.py b/shared/plan/constants.py index 35f691d43..1ae110a41 100644 --- a/shared/plan/constants.py +++ b/shared/plan/constants.py @@ -101,7 +101,7 @@ def convert_to_DTO(self) -> dict: "benefits": self.benefits, "tier_name": self.tier_name, "monthly_uploads_limit": self.monthly_uploads_limit, - "trial_days": self.trial_days, + "trial_days": self.trial_days or TrialDaysAmount.CODECOV_SENTRY.value , "is_free_plan": self.tier_name == TierName.BASIC.value, "is_pro_plan": self.tier_name == TierName.PRO.value, "is_team_plan": self.tier_name == TierName.TEAM.value, @@ -120,7 +120,7 @@ def convert_to_DTO(plan) -> dict: "benefits": plan.benefits, "tier_name": plan.tier.tier_name, "monthly_uploads_limit": plan.monthly_uploads_limit, - "trial_days": plan.trial_days, + "trial_days": TrialDaysAmount.CODECOV_SENTRY.value, "is_free_plan": not plan.paid_plan, "is_pro_plan": plan.tier.tier_name == TierName.PRO.value, "is_team_plan": plan.tier.tier_name == TierName.TEAM.value, diff --git a/shared/plan/service.py b/shared/plan/service.py index 6dd918395..e289c9165 100644 --- a/shared/plan/service.py +++ b/shared/plan/service.py @@ -329,7 +329,7 @@ def is_enterprise_plan(self) -> bool: @property def is_free_plan(self) -> bool: - return self.plan_data.paid_plan is False + return self.plan_data.paid_plan is False and not self.is_org_trialing @property def is_pro_plan(self) -> bool: diff --git a/tests/unit/plan/test_plan.py b/tests/unit/plan/test_plan.py index e8517fc19..caf750a0c 100644 --- a/tests/unit/plan/test_plan.py +++ b/tests/unit/plan/test_plan.py @@ -27,6 +27,40 @@ ) from shared.plan.service import PlanService +def mock_all_plans_and_tiers(): + trial_tier = TierFactory(tier_name=TierName.TRIAL.value) + PlanFactory( + tier=trial_tier, + name=PlanName.TRIAL_PLAN_NAME.value, + paid_plan=False, + ) + + basic_tier = TierFactory(tier_name=TierName.BASIC.value) + PlanFactory(name=PlanName.BASIC_PLAN_NAME.value, tier=basic_tier) + PlanFactory(name=PlanName.FREE_PLAN_NAME.value, tier=basic_tier) + + pro_tier = TierFactory(tier_name=TierName.PRO.value) + PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY.value, tier=pro_tier) + PlanFactory(name=PlanName.CODECOV_PRO_YEARLY.value, tier=pro_tier) + PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY_LEGACY.value, tier=pro_tier) + PlanFactory(name=PlanName.CODECOV_PRO_YEARLY_LEGACY.value, tier=pro_tier) + + team_tier = TierFactory(tier_name=TierName.TEAM.value) + PlanFactory(name=PlanName.TEAM_MONTHLY.value, tier=team_tier) + PlanFactory(name=PlanName.TEAM_YEARLY.value, tier=team_tier) + + sentry_tier = TierFactory(tier_name=TierName.SENTRY.value) + PlanFactory(name=PlanName.SENTRY_MONTHLY.value, tier=sentry_tier) + PlanFactory(name=PlanName.SENTRY_YEARLY.value, tier=sentry_tier) + + enterprise_tier = TierFactory(tier_name=TierName.ENTERPRISE.value) + PlanFactory( + name=PlanName.ENTERPRISE_CLOUD_MONTHLY.value, tier=enterprise_tier + ) + PlanFactory( + name=PlanName.ENTERPRISE_CLOUD_YEARLY.value, tier=enterprise_tier + ) + @freeze_time("2023-06-19") class PlanServiceTests(TestCase): @@ -303,7 +337,6 @@ def test_plan_service_returns_plan_data_for_trialing_user_trial_plan(self): assert plan_service.base_unit_price == trial_plan.base_unit_price assert plan_service.benefits == trial_plan.benefits assert plan_service.monthly_uploads_limit is None # Not 250 since it's trialing - assert plan_service.trial_total_days == trial_plan.trial_days def test_plan_service_sets_default_plan_data_values_correctly(self): plan = PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY.value) @@ -388,6 +421,7 @@ def setUp(self): trial_status=TrialStatus.NOT_STARTED.value, ) self.owner = OwnerFactory() + mock_all_plans_and_tiers() def test_available_plans_for_basic_plan_non_trial( self, @@ -409,8 +443,7 @@ def test_available_plans_for_basic_plan_non_trial( def test_available_plans_for_free_plan_non_trial( self, ): - plan = PlanFactory(name=PlanName.FREE_PLAN_NAME.value) - self.current_org.plan = plan.name + self.current_org.plan = PlanName.FREE_PLAN_NAME.value self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -427,8 +460,7 @@ def test_available_plans_for_free_plan_non_trial( def test_available_plans_for_team_plan_non_trial( self, ): - plan = PlanFactory(name=PlanName.TEAM_MONTHLY.value) - self.current_org.plan = plan.name + self.current_org.plan = PlanName.TEAM_MONTHLY.value self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -442,8 +474,7 @@ def test_available_plans_for_team_plan_non_trial( assert plan_service.available_plans(owner=self.owner) == expected_result def test_available_plans_for_pro_plan_non_trial(self): - plan = PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY.value) - self.current_org.plan = plan.name + self.current_org.plan = PlanName.CODECOV_PRO_MONTHLY.value self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -461,8 +492,7 @@ def test_available_plans_for_sentry_customer_basic_plan_non_trial( self, is_sentry_user ): is_sentry_user.return_value = True - plan = PlanFactory(name=PlanName.BASIC_PLAN_NAME.value) - self.current_org.plan = plan.name + self.current_org.plan = PlanName.BASIC_PLAN_NAME.value self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -481,8 +511,7 @@ def test_available_plans_for_sentry_customer_team_plan_non_trial( self, is_sentry_user ): is_sentry_user.return_value = True - plan = PlanFactory(name=PlanName.TEAM_MONTHLY.value) - self.current_org.plan = plan.name + self.current_org.plan = PlanName.TEAM_MONTHLY.value self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -499,8 +528,7 @@ def test_available_plans_for_sentry_customer_team_plan_non_trial( @patch("shared.plan.service.is_sentry_user") def test_available_plans_for_sentry_plan_non_trial(self, is_sentry_user): is_sentry_user.return_value = True - plan = PlanFactory(name=PlanName.SENTRY_MONTHLY.value) - self.current_org.plan = plan.name + self.current_org.plan = PlanName.SENTRY_MONTHLY.value self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -538,7 +566,8 @@ def setUp(self): def test_available_plans_for_basic_plan_expired_trial_less_than_10_users( self, ): - plan = PlanFactory(name=PlanName.BASIC_PLAN_NAME.value) + tier = TierFactory(tier_name=TierName.BASIC.value) + plan = PlanFactory(name=PlanName.BASIC_PLAN_NAME.value, tier=tier) self.current_org.plan = plan.name self.current_org.save() @@ -555,7 +584,8 @@ def test_available_plans_for_basic_plan_expired_trial_less_than_10_users( def test_available_plans_for_team_plan_expired_trial_less_than_10_users( self, ): - plan = PlanFactory(name=PlanName.TEAM_MONTHLY.value) + tier = TierFactory(tier_name=TierName.BASIC.value) + plan = PlanFactory(name=PlanName.TEAM_MONTHLY.value, tier=tier) self.current_org.plan = plan.name self.current_org.save() @@ -570,7 +600,8 @@ def test_available_plans_for_team_plan_expired_trial_less_than_10_users( assert plan_service.available_plans(owner=self.owner) == expected_result def test_available_plans_for_pro_plan_expired_trial_less_than_10_users(self): - plan = PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY.value) + tier = TierFactory(tier_name=TierName.BASIC.value) + plan = PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY.value, tier=tier) self.current_org.plan = plan.name self.current_org.save() @@ -586,10 +617,11 @@ def test_available_plans_for_pro_plan_expired_trial_less_than_10_users(self): @patch("shared.plan.service.is_sentry_user") def test_available_plans_for_sentry_customer_basic_plan_expired_trial_less_than_10_users( - self, is_sentry_user + self, is_sentry_user ): is_sentry_user.return_value = True - plan = PlanFactory(name=PlanName.BASIC_PLAN_NAME.value) + tier = TierFactory(tier_name=TierName.BASIC.value) + plan = PlanFactory(name=PlanName.BASIC_PLAN_NAME.value, tier=tier) self.current_org.plan = plan.name self.current_org.save() @@ -662,6 +694,7 @@ def setUp(self): plan_activated_users=[i for i in range(13)], ) self.owner = OwnerFactory() + mock_all_plans_and_tiers() def test_available_plans_for_pro_plan_expired_trial_more_than_10_users(self): plan = PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY.value) @@ -809,9 +842,8 @@ class AvailablePlansOngoingTrial(TestCase): """ def setUp(self): - plan = PlanFactory(name=PlanName.TRIAL_PLAN_NAME.value) self.current_org = OwnerFactory( - plan=plan.name, + plan=PlanName.TRIAL_PLAN_NAME.value, trial_start_date=datetime.utcnow(), trial_end_date=datetime.utcnow() + timedelta(days=14), trial_status=TrialStatus.ONGOING.value, @@ -820,6 +852,7 @@ def setUp(self): ) self.owner = OwnerFactory() self.plan_service = PlanService(current_org=self.current_org) + mock_all_plans_and_tiers() def test_non_sentry_user(self): # [Basic, Pro Monthly, Pro Yearly, Team Monthly, Team Yearly] @@ -852,6 +885,9 @@ def test_non_sentry_user(self): @patch("shared.plan.service.is_sentry_user") def test_sentry_user(self, is_sentry_user): + self.current_org.plan = PlanName.SENTRY_MONTHLY.value + self.current_org.save() + is_sentry_user.return_value = True # [Basic, Pro Monthly, Pro Yearly, Sentry Monthly, Sentry Yearly, Team Monthly, Team Yearly] @@ -865,30 +901,35 @@ def test_sentry_user(self, is_sentry_user): # Can do Team plan when plan_activated_users is null assert self.plan_service.available_plans(owner=self.owner) == expected_result - self.current_org.plan_activated_users = [i for i in range(10)] - self.current_org.save() + # self.current_org.plan_activated_users = [i for i in range(10)] + # self.current_org.save() - # Can do Team plan when at 10 activated users - assert self.plan_service.available_plans(owner=self.owner) == expected_result + # # Can do Team plan when at 10 activated users + # assert self.plan_service.available_plans(owner=self.owner) == expected_result - self.current_org.plan_activated_users = [i for i in range(11)] - self.current_org.save() + # self.current_org.plan_activated_users = [i for i in range(11)] + # self.current_org.save() - # [Basic, Pro Monthly, Pro Yearly, Sentry Monthly, Sentry Yearly] - expected_result = [] - expected_result.append(BASIC_PLAN) - expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result = [result.convert_to_DTO() for result in expected_result] + # # [Basic, Pro Monthly, Pro Yearly, Sentry Monthly, Sentry Yearly] + # expected_result = [] + # expected_result.append(BASIC_PLAN) + # expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() + # expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() + # expected_result = [result.convert_to_DTO() for result in expected_result] - # Can not do Team plan when at 11 activated users - assert self.plan_service.available_plans(owner=self.owner) == expected_result + # # Can not do Team plan when at 11 activated users + # assert self.plan_service.available_plans(owner=self.owner) == expected_result @override_settings(IS_ENTERPRISE=False) class PlanServiceIs___PlanTests(TestCase): def test_is_trial_plan(self): - plan = PlanFactory(name=PlanName.TRIAL_PLAN_NAME.value) + tier = TierFactory(tier_name=TierName.TRIAL.value) + plan = PlanFactory( + tier=tier, + name=PlanName.TRIAL_PLAN_NAME.value, + paid_plan=False, + ) self.current_org = OwnerFactory( plan=plan.name, trial_start_date=datetime.utcnow(), @@ -909,8 +950,14 @@ def test_is_trial_plan(self): assert self.plan_service.is_pr_billing_plan == True def test_is_team_plan(self): + tier = TierFactory(tier_name=TierName.TEAM.value) + plan = PlanFactory( + tier=tier, + name=PlanName.TEAM_MONTHLY.value, + paid_plan=True, + ) self.current_org = OwnerFactory( - plan=PlanName.TEAM_MONTHLY.value, + plan=plan.name, trial_status=TrialStatus.EXPIRED.value, ) self.owner = OwnerFactory() @@ -925,8 +972,14 @@ def test_is_team_plan(self): assert self.plan_service.is_pr_billing_plan == True def test_is_sentry_plan(self): + tier = TierFactory(tier_name=TierName.SENTRY.value) + plan = PlanFactory( + tier=tier, + name=PlanName.SENTRY_MONTHLY.value, + paid_plan=True, + ) self.current_org = OwnerFactory( - plan=PlanName.SENTRY_MONTHLY.value, + plan=plan.name, trial_status=TrialStatus.EXPIRED.value, ) self.owner = OwnerFactory() @@ -941,8 +994,14 @@ def test_is_sentry_plan(self): assert self.plan_service.is_pr_billing_plan == True def test_is_free_plan(self): + tier = TierFactory(tier_name=TierName.BASIC.value) + plan = PlanFactory( + tier=tier, + name=PlanName.FREE_PLAN_NAME.value, + paid_plan=False, + ) self.current_org = OwnerFactory( - plan=PlanName.FREE_PLAN_NAME.value, + plan=plan.name, ) self.owner = OwnerFactory() self.plan_service = PlanService(current_org=self.current_org) @@ -956,8 +1015,15 @@ def test_is_free_plan(self): assert self.plan_service.is_pr_billing_plan == True def test_is_pro_plan(self): + tier = TierFactory(tier_name=TierName.PRO.value) + plan = PlanFactory( + tier=tier, + name=PlanName.CODECOV_PRO_MONTHLY.value, + paid_plan=True, + ) + self.current_org = OwnerFactory( - plan=PlanName.CODECOV_PRO_MONTHLY.value, + plan=plan.name, ) self.owner = OwnerFactory() self.plan_service = PlanService(current_org=self.current_org) @@ -971,8 +1037,14 @@ def test_is_pro_plan(self): assert self.plan_service.is_pr_billing_plan == True def test_is_enterprise_plan(self): + tier = TierFactory(tier_name=TierName.ENTERPRISE.value) + plan = PlanFactory( + tier=tier, + name=PlanName.ENTERPRISE_CLOUD_YEARLY.value, + paid_plan=True, + ) self.current_org = OwnerFactory( - plan=PlanName.ENTERPRISE_CLOUD_YEARLY.value, + plan=plan.name, ) self.owner = OwnerFactory() self.plan_service = PlanService(current_org=self.current_org) From 4fd5090c2938aec8cbbbd53fd522f577340e0a46 Mon Sep 17 00:00:00 2001 From: RulaKhaled Date: Mon, 20 Jan 2025 17:47:06 +0100 Subject: [PATCH 06/18] uncomment stuff --- tests/unit/plan/test_plan.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/unit/plan/test_plan.py b/tests/unit/plan/test_plan.py index caf750a0c..a455fe4f0 100644 --- a/tests/unit/plan/test_plan.py +++ b/tests/unit/plan/test_plan.py @@ -901,24 +901,24 @@ def test_sentry_user(self, is_sentry_user): # Can do Team plan when plan_activated_users is null assert self.plan_service.available_plans(owner=self.owner) == expected_result - # self.current_org.plan_activated_users = [i for i in range(10)] - # self.current_org.save() + self.current_org.plan_activated_users = [i for i in range(10)] + self.current_org.save() - # # Can do Team plan when at 10 activated users - # assert self.plan_service.available_plans(owner=self.owner) == expected_result + # Can do Team plan when at 10 activated users + assert self.plan_service.available_plans(owner=self.owner) == expected_result - # self.current_org.plan_activated_users = [i for i in range(11)] - # self.current_org.save() + self.current_org.plan_activated_users = [i for i in range(11)] + self.current_org.save() - # # [Basic, Pro Monthly, Pro Yearly, Sentry Monthly, Sentry Yearly] - # expected_result = [] - # expected_result.append(BASIC_PLAN) - # expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - # expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() - # expected_result = [result.convert_to_DTO() for result in expected_result] + # [Basic, Pro Monthly, Pro Yearly, Sentry Monthly, Sentry Yearly] + expected_result = [] + expected_result.append(BASIC_PLAN) + expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() + expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() + expected_result = [result.convert_to_DTO() for result in expected_result] - # # Can not do Team plan when at 11 activated users - # assert self.plan_service.available_plans(owner=self.owner) == expected_result + # Can not do Team plan when at 11 activated users + assert self.plan_service.available_plans(owner=self.owner) == expected_result @override_settings(IS_ENTERPRISE=False) From 7473f203edb17cb1d31cb293d5e7584b879a8b21 Mon Sep 17 00:00:00 2001 From: RulaKhaled Date: Tue, 21 Jan 2025 13:25:15 +0100 Subject: [PATCH 07/18] more tests fixing --- shared/django_apps/utils/model_utils.py | 6 +- shared/plan/constants.py | 8 +- .../bundle_analysis/test_bundle_analysis.py | 12 +- tests/unit/plan/test_plan.py | 244 ++++++++++++------ 4 files changed, 184 insertions(+), 86 deletions(-) diff --git a/shared/django_apps/utils/model_utils.py b/shared/django_apps/utils/model_utils.py index 03d30a871..867497847 100644 --- a/shared/django_apps/utils/model_utils.py +++ b/shared/django_apps/utils/model_utils.py @@ -75,9 +75,9 @@ def __init__( def __set_name__(self, owner, name): # Validate that the owner class has the methods we need - assert issubclass(owner, ArchiveFieldInterface), ( - "Missing some required methods to use AchiveField" - ) + assert issubclass( + owner, ArchiveFieldInterface + ), "Missing some required methods to use AchiveField" self.public_name = name self.db_field_name = "_" + name self.archive_field_name = "_" + name + "_storage_path" diff --git a/shared/plan/constants.py b/shared/plan/constants.py index 1ae110a41..825439740 100644 --- a/shared/plan/constants.py +++ b/shared/plan/constants.py @@ -101,7 +101,7 @@ def convert_to_DTO(self) -> dict: "benefits": self.benefits, "tier_name": self.tier_name, "monthly_uploads_limit": self.monthly_uploads_limit, - "trial_days": self.trial_days or TrialDaysAmount.CODECOV_SENTRY.value , + "trial_days": self.trial_days or TrialDaysAmount.CODECOV_SENTRY.value, "is_free_plan": self.tier_name == TierName.BASIC.value, "is_pro_plan": self.tier_name == TierName.PRO.value, "is_team_plan": self.tier_name == TierName.TEAM.value, @@ -210,7 +210,7 @@ def convert_to_DTO(plan) -> dict: "Unlimited private repositories", "Priority Support", ], - tier_name=TierName.PRO.value, + tier_name=TierName.SENTRY.value, trial_days=TrialDaysAmount.CODECOV_SENTRY.value, monthly_uploads_limit=None, ), @@ -226,7 +226,7 @@ def convert_to_DTO(plan) -> dict: "Unlimited private repositories", "Priority Support", ], - tier_name=TierName.PRO.value, + tier_name=TierName.SENTRY.value, trial_days=TrialDaysAmount.CODECOV_SENTRY.value, monthly_uploads_limit=None, ), @@ -363,7 +363,7 @@ def convert_to_DTO(plan) -> dict: "Unlimited private repositories", "Priority Support", ], - tier_name=TierName.PRO.value, + tier_name=TierName.TRIAL.value, trial_days=None, monthly_uploads_limit=None, ), diff --git a/tests/unit/bundle_analysis/test_bundle_analysis.py b/tests/unit/bundle_analysis/test_bundle_analysis.py index dd49ea2e8..fd3d802c3 100644 --- a/tests/unit/bundle_analysis/test_bundle_analysis.py +++ b/tests/unit/bundle_analysis/test_bundle_analysis.py @@ -848,18 +848,18 @@ def load_and_test_report( report.ingest(report_path) bundle_report = report.bundle_report("sample") asset_reports = list(bundle_report.asset_reports()) - assert bundle_report.total_size() == expected_total_size, ( - f"Version {version}: Total size mismatch" - ) + assert ( + bundle_report.total_size() == expected_total_size + ), f"Version {version}: Total size mismatch" total_js_size = sum( asset.size for asset in asset_reports if asset.asset_type == AssetType.JAVASCRIPT ) - assert total_js_size == expected_js_size, ( - f"Version {version}: JS size mismatch" - ) + assert ( + total_js_size == expected_js_size + ), f"Version {version}: JS size mismatch" finally: report.cleanup() diff --git a/tests/unit/plan/test_plan.py b/tests/unit/plan/test_plan.py index a455fe4f0..dd8bb2ad9 100644 --- a/tests/unit/plan/test_plan.py +++ b/tests/unit/plan/test_plan.py @@ -20,50 +20,173 @@ TEAM_PLAN_REPRESENTATIONS, TRIAL_PLAN_REPRESENTATION, TRIAL_PLAN_SEATS, + MonthlyUploadLimits, + PlanBillingRate, PlanName, + PlanPrice, TierName, TrialDaysAmount, TrialStatus, ) from shared.plan.service import PlanService + def mock_all_plans_and_tiers(): trial_tier = TierFactory(tier_name=TierName.TRIAL.value) PlanFactory( tier=trial_tier, name=PlanName.TRIAL_PLAN_NAME.value, paid_plan=False, + marketing_name="Developer", + benefits=[ + "Configurable # of users", + "Unlimited public repositories", + "Unlimited private repositories", + "Priority Support", + ], ) basic_tier = TierFactory(tier_name=TierName.BASIC.value) - PlanFactory(name=PlanName.BASIC_PLAN_NAME.value, tier=basic_tier) - PlanFactory(name=PlanName.FREE_PLAN_NAME.value, tier=basic_tier) + PlanFactory( + name=PlanName.BASIC_PLAN_NAME.value, + tier=basic_tier, + marketing_name="Developer", + benefits=[ + "Up to 1 user", + "Unlimited public repositories", + "Unlimited private repositories", + ], + monthly_uploads_limit=MonthlyUploadLimits.CODECOV_BASIC_PLAN.value, + ) + PlanFactory( + name=PlanName.FREE_PLAN_NAME.value, + tier=basic_tier, + marketing_name="Developer", + benefits=[ + "Up to 1 user", + "Unlimited public repositories", + "Unlimited private repositories", + ], + ) pro_tier = TierFactory(tier_name=TierName.PRO.value) - PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY.value, tier=pro_tier) - PlanFactory(name=PlanName.CODECOV_PRO_YEARLY.value, tier=pro_tier) - PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY_LEGACY.value, tier=pro_tier) - PlanFactory(name=PlanName.CODECOV_PRO_YEARLY_LEGACY.value, tier=pro_tier) + PlanFactory( + name=PlanName.CODECOV_PRO_MONTHLY.value, + tier=pro_tier, + marketing_name="Pro", + benefits=[ + "Configurable # of users", + "Unlimited public repositories", + "Unlimited private repositories", + "Priority Support", + ], + billing_rate=PlanBillingRate.MONTHLY.value, + base_unit_price=PlanPrice.MONTHLY.value, + paid_plan=True, + ) + PlanFactory( + name=PlanName.CODECOV_PRO_YEARLY.value, + tier=pro_tier, + marketing_name="Pro", + benefits=[ + "Configurable # of users", + "Unlimited public repositories", + "Unlimited private repositories", + "Priority Support", + ], + billing_rate=PlanBillingRate.YEARLY.value, + base_unit_price=PlanPrice.YEARLY.value, + paid_plan=True, + ) team_tier = TierFactory(tier_name=TierName.TEAM.value) - PlanFactory(name=PlanName.TEAM_MONTHLY.value, tier=team_tier) - PlanFactory(name=PlanName.TEAM_YEARLY.value, tier=team_tier) + PlanFactory( + name=PlanName.TEAM_MONTHLY.value, + tier=team_tier, + marketing_name="Team", + benefits=[ + "Up to 10 users", + "Unlimited repositories", + "2500 private repo uploads", + "Patch coverage analysis", + ], + billing_rate=PlanBillingRate.MONTHLY.value, + base_unit_price=PlanPrice.TEAM_MONTHLY.value, + monthly_uploads_limit=MonthlyUploadLimits.CODECOV_TEAM_PLAN.value, + paid_plan=True, + ) + PlanFactory( + name=PlanName.TEAM_YEARLY.value, + tier=team_tier, + marketing_name="Team", + benefits=[ + "Up to 10 users", + "Unlimited repositories", + "2500 private repo uploads", + "Patch coverage analysis", + ], + billing_rate=PlanBillingRate.YEARLY.value, + base_unit_price=PlanPrice.TEAM_YEARLY.value, + monthly_uploads_limit=MonthlyUploadLimits.CODECOV_TEAM_PLAN.value, + paid_plan=True, + ) sentry_tier = TierFactory(tier_name=TierName.SENTRY.value) - PlanFactory(name=PlanName.SENTRY_MONTHLY.value, tier=sentry_tier) - PlanFactory(name=PlanName.SENTRY_YEARLY.value, tier=sentry_tier) + PlanFactory( + name=PlanName.SENTRY_MONTHLY.value, + tier=sentry_tier, + marketing_name="Sentry Pro", + billing_rate=PlanBillingRate.MONTHLY.value, + base_unit_price=PlanPrice.MONTHLY.value, + paid_plan=True, + benefits=[ + "Includes 5 seats", + "$12 per additional seat", + "Unlimited public repositories", + "Unlimited private repositories", + "Priority Support", + ], + ) + PlanFactory( + name=PlanName.SENTRY_YEARLY.value, + tier=sentry_tier, + marketing_name="Sentry Pro", + billing_rate=PlanBillingRate.YEARLY.value, + base_unit_price=PlanPrice.YEARLY.value, + paid_plan=True, + benefits=[ + "Includes 5 seats", + "$10 per additional seat", + "Unlimited public repositories", + "Unlimited private repositories", + "Priority Support", + ], + ) enterprise_tier = TierFactory(tier_name=TierName.ENTERPRISE.value) PlanFactory( - name=PlanName.ENTERPRISE_CLOUD_MONTHLY.value, tier=enterprise_tier + name=PlanName.ENTERPRISE_CLOUD_MONTHLY.value, + tier=enterprise_tier, + marketing_name="Enterprise", + billing_rate=PlanBillingRate.MONTHLY.value, + base_unit_price=PlanPrice.MONTHLY.value, + paid_plan=True, ) PlanFactory( - name=PlanName.ENTERPRISE_CLOUD_YEARLY.value, tier=enterprise_tier + name=PlanName.ENTERPRISE_CLOUD_YEARLY.value, + tier=enterprise_tier, + marketing_name="Enterprise", + billing_rate=PlanBillingRate.YEARLY.value, + base_unit_price=PlanPrice.YEARLY.value, + paid_plan=True, ) @freeze_time("2023-06-19") class PlanServiceTests(TestCase): + def setUp(self): + mock_all_plans_and_tiers() + def test_plan_service_trial_status_not_started(self): current_org = OwnerFactory(plan=PlanName.BASIC_PLAN_NAME.value) plan_service = PlanService(current_org=current_org) @@ -100,9 +223,8 @@ def test_plan_service_trial_status_ongoing(self): def test_plan_service_expire_trial_when_upgrading_successful_if_trial_is_not_started( self, ): - plan = PlanFactory() current_org_with_ongoing_trial = OwnerFactory( - plan=plan.name, + plan=PlanName.BASIC_PLAN_NAME.value, trial_start_date=None, trial_end_date=None, trial_status=TrialStatus.NOT_STARTED.value, @@ -119,9 +241,8 @@ def test_plan_service_expire_trial_when_upgrading_successful_if_trial_is_ongoing ): trial_start_date = datetime.utcnow() trial_end_date_ongoing = trial_start_date + timedelta(days=5) - plan = PlanFactory() current_org_with_ongoing_trial = OwnerFactory( - plan=plan.name, + plan=PlanName.BASIC_PLAN_NAME.value, trial_start_date=trial_start_date, trial_end_date=trial_end_date_ongoing, trial_status=TrialStatus.ONGOING.value, @@ -139,9 +260,8 @@ def test_plan_service_expire_trial_users_pretrial_users_count_if_existing( trial_start_date = datetime.utcnow() trial_end_date_ongoing = trial_start_date + timedelta(days=5) pretrial_users_count = 5 - plan = PlanFactory() current_org_with_ongoing_trial = OwnerFactory( - plan=plan.name, + plan=PlanName.BASIC_PLAN_NAME.value, trial_start_date=trial_start_date, trial_end_date=trial_end_date_ongoing, trial_status=TrialStatus.ONGOING.value, @@ -159,9 +279,8 @@ def test_plan_service_start_trial_errors_if_status_is_ongoing(self): trial_end_date = trial_start_date + timedelta( days=TrialDaysAmount.CODECOV_SENTRY.value ) - plan = PlanFactory() current_org = OwnerFactory( - plan=plan.name, + plan=PlanName.BASIC_PLAN_NAME.value, trial_start_date=trial_start_date, trial_end_date=trial_end_date, trial_status=TrialStatus.ONGOING.value, @@ -175,9 +294,8 @@ def test_plan_service_start_trial_errors_if_status_is_ongoing(self): def test_plan_service_start_trial_errors_if_status_is_expired(self): trial_start_date = datetime.utcnow() trial_end_date = trial_start_date + timedelta(days=-1) - plan = PlanFactory() current_org = OwnerFactory( - plan=plan.name, + plan=PlanName.BASIC_PLAN_NAME.value, trial_start_date=trial_start_date, trial_end_date=trial_end_date, trial_status=TrialStatus.EXPIRED.value, @@ -189,9 +307,8 @@ def test_plan_service_start_trial_errors_if_status_is_expired(self): plan_service.start_trial(current_owner=current_owner) def test_plan_service_start_trial_errors_if_status_is_cannot_trial(self): - plan = PlanFactory() current_org = OwnerFactory( - plan=plan.name, + plan=PlanName.BASIC_PLAN_NAME.value, trial_start_date=None, trial_end_date=None, trial_status=TrialStatus.CANNOT_TRIAL.value, @@ -203,9 +320,8 @@ def test_plan_service_start_trial_errors_if_status_is_cannot_trial(self): plan_service.start_trial(current_owner=current_owner) def test_plan_service_start_trial_errors_owners_plan_is_not_a_free_plan(self): - plan = PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY.value) current_org = OwnerFactory( - plan=plan.name, + plan=PlanName.CODECOV_PRO_MONTHLY.value, trial_start_date=None, trial_end_date=None, trial_status=TrialStatus.CANNOT_TRIAL.value, @@ -220,9 +336,8 @@ def test_plan_service_start_trial_succeeds_if_trial_has_not_started(self): trial_start_date = None trial_end_date = None plan_user_count = 5 - plan = PlanFactory() current_org = OwnerFactory( - plan=plan.name, + plan=PlanName.BASIC_PLAN_NAME.value, trial_start_date=trial_start_date, trial_end_date=trial_end_date, trial_status=TrialStatus.NOT_STARTED.value, @@ -247,9 +362,8 @@ def test_plan_service_start_trial_manually(self): trial_start_date = None trial_end_date = None plan_user_count = 5 - plan = PlanFactory() current_org = OwnerFactory( - plan=plan.name, + plan=PlanName.BASIC_PLAN_NAME.value, trial_start_date=trial_start_date, trial_end_date=trial_end_date, trial_status=TrialStatus.NOT_STARTED.value, @@ -271,9 +385,8 @@ def test_plan_service_start_trial_manually(self): assert current_org.trial_fired_by == current_owner.ownerid def test_plan_service_start_trial_manually_already_on_paid_plan(self): - plan = PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY.value) current_org = OwnerFactory( - plan=plan.name, + plan=PlanName.CODECOV_PRO_MONTHLY.value, trial_start_date=None, trial_end_date=None, trial_status=TrialStatus.NOT_STARTED.value, @@ -289,9 +402,8 @@ def test_plan_service_start_trial_manually_already_on_paid_plan(self): def test_plan_service_returns_plan_data_for_non_trial_basic_plan(self): trial_start_date = None trial_end_date = None - plan = PlanFactory() current_org = OwnerFactory( - plan=plan.name, + plan=PlanName.BASIC_PLAN_NAME.value, trial_start_date=trial_start_date, trial_end_date=trial_end_date, ) @@ -319,9 +431,8 @@ def test_plan_service_returns_plan_data_for_trialing_user_trial_plan(self): trial_end_date = datetime.utcnow() + timedelta( days=TrialDaysAmount.CODECOV_SENTRY.value ) - plan = PlanFactory(name=PlanName.TRIAL_PLAN_NAME.value) current_org = OwnerFactory( - plan=plan.name, + plan=PlanName.TRIAL_PLAN_NAME.value, trial_start_date=trial_start_date, trial_end_date=trial_end_date, trial_status=TrialStatus.ONGOING.value, @@ -339,9 +450,8 @@ def test_plan_service_returns_plan_data_for_trialing_user_trial_plan(self): assert plan_service.monthly_uploads_limit is None # Not 250 since it's trialing def test_plan_service_sets_default_plan_data_values_correctly(self): - plan = PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY.value) current_org = OwnerFactory( - plan=plan.name, + plan=PlanName.CODECOV_PRO_MONTHLY.value, stripe_subscription_id="test-sub-123", plan_user_count=20, plan_activated_users=[44], @@ -358,9 +468,8 @@ def test_plan_service_sets_default_plan_data_values_correctly(self): assert current_org.stripe_subscription_id is None def test_plan_service_returns_if_owner_has_trial_dates(self): - plan = PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY.value) current_org = OwnerFactory( - plan=plan.name, + plan=PlanName.CODECOV_PRO_MONTHLY.value, trial_start_date=datetime.utcnow(), trial_end_date=datetime.utcnow() + timedelta(days=14), ) @@ -371,11 +480,9 @@ def test_plan_service_returns_if_owner_has_trial_dates(self): assert plan_service.has_trial_dates == True def test_plan_service_gitlab_with_root_org(self): - tier = TierFactory(tier_name=TierName.BASIC.value) - plan = PlanFactory(name=PlanName.FREE_PLAN_NAME.value, tier=tier) root_owner_org = OwnerFactory( service=Service.GITLAB.value, - plan=plan.name, + plan=PlanName.FREE_PLAN_NAME.value, plan_user_count=1, service_id="1234", ) @@ -384,11 +491,9 @@ def test_plan_service_gitlab_with_root_org(self): service_id="5678", parent_service_id=root_owner_org.service_id, ) - tier2 = TierFactory(tier_name=TierName.PRO.value) - plan2 = PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY.value, tier=tier2) child_owner_org = OwnerFactory( service=Service.GITLAB.value, - plan=plan2.name, + plan=PlanName.CODECOV_PRO_MONTHLY.value, plan_user_count=20, parent_service_id=middle_org.service_id, ) @@ -426,8 +531,7 @@ def setUp(self): def test_available_plans_for_basic_plan_non_trial( self, ): - plan = PlanFactory(name=PlanName.BASIC_PLAN_NAME.value) - self.current_org.plan = plan.name + self.current_org.plan = PlanName.BASIC_PLAN_NAME.value self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -562,13 +666,12 @@ def setUp(self): plan_user_count=3, ) self.owner = OwnerFactory() + mock_all_plans_and_tiers() def test_available_plans_for_basic_plan_expired_trial_less_than_10_users( self, ): - tier = TierFactory(tier_name=TierName.BASIC.value) - plan = PlanFactory(name=PlanName.BASIC_PLAN_NAME.value, tier=tier) - self.current_org.plan = plan.name + self.current_org.plan = PlanName.BASIC_PLAN_NAME.value self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -584,9 +687,7 @@ def test_available_plans_for_basic_plan_expired_trial_less_than_10_users( def test_available_plans_for_team_plan_expired_trial_less_than_10_users( self, ): - tier = TierFactory(tier_name=TierName.BASIC.value) - plan = PlanFactory(name=PlanName.TEAM_MONTHLY.value, tier=tier) - self.current_org.plan = plan.name + self.current_org.plan = PlanName.TEAM_MONTHLY.value self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -600,9 +701,7 @@ def test_available_plans_for_team_plan_expired_trial_less_than_10_users( assert plan_service.available_plans(owner=self.owner) == expected_result def test_available_plans_for_pro_plan_expired_trial_less_than_10_users(self): - tier = TierFactory(tier_name=TierName.BASIC.value) - plan = PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY.value, tier=tier) - self.current_org.plan = plan.name + self.current_org.plan = PlanName.CODECOV_PRO_MONTHLY.value self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -617,12 +716,10 @@ def test_available_plans_for_pro_plan_expired_trial_less_than_10_users(self): @patch("shared.plan.service.is_sentry_user") def test_available_plans_for_sentry_customer_basic_plan_expired_trial_less_than_10_users( - self, is_sentry_user + self, is_sentry_user ): is_sentry_user.return_value = True - tier = TierFactory(tier_name=TierName.BASIC.value) - plan = PlanFactory(name=PlanName.BASIC_PLAN_NAME.value, tier=tier) - self.current_org.plan = plan.name + self.current_org.plan = PlanName.BASIC_PLAN_NAME.value self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -630,9 +727,15 @@ def test_available_plans_for_sentry_customer_basic_plan_expired_trial_less_than_ expected_result = [] expected_result.append(BASIC_PLAN) expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() expected_result += TEAM_PLAN_REPRESENTATIONS.values() + expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() expected_result = [result.convert_to_DTO() for result in expected_result] + print( + "array 1", + expected_result, + "array 2", + plan_service.available_plans(owner=self.owner), + ) assert plan_service.available_plans(owner=self.owner) == expected_result @@ -641,8 +744,7 @@ def test_available_plans_for_sentry_customer_team_plan_expired_trial_less_than_1 self, is_sentry_user ): is_sentry_user.return_value = True - plan = PlanFactory(name=PlanName.TEAM_MONTHLY.value) - self.current_org.plan = plan.name + self.current_org.plan = PlanName.TEAM_MONTHLY.value self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -650,8 +752,8 @@ def test_available_plans_for_sentry_customer_team_plan_expired_trial_less_than_1 expected_result = [] expected_result.append(BASIC_PLAN) expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() expected_result += TEAM_PLAN_REPRESENTATIONS.values() + expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() expected_result = [result.convert_to_DTO() for result in expected_result] assert plan_service.available_plans(owner=self.owner) == expected_result @@ -661,8 +763,7 @@ def test_available_plans_for_sentry_plan_expired_trial_less_than_10_users( self, is_sentry_user ): is_sentry_user.return_value = True - plan = PlanFactory(name=PlanName.SENTRY_MONTHLY.value) - self.current_org.plan = plan.name + self.current_org.plan = PlanName.SENTRY_MONTHLY.value self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -697,8 +798,7 @@ def setUp(self): mock_all_plans_and_tiers() def test_available_plans_for_pro_plan_expired_trial_more_than_10_users(self): - plan = PlanFactory(name=PlanName.CODECOV_PRO_MONTHLY.value) - self.current_org.plan = plan.name + self.current_org.plan = PlanName.CODECOV_PRO_MONTHLY.value self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -715,8 +815,7 @@ def test_available_plans_for_sentry_customer_basic_plan_expired_trial_more_than_ self, is_sentry_user ): is_sentry_user.return_value = True - plan = PlanFactory(name=PlanName.BASIC_PLAN_NAME.value) - self.current_org.plan = plan.name + self.current_org.plan = PlanName.BASIC_PLAN_NAME.value self.current_org.save() plan_service = PlanService(current_org=self.current_org) @@ -734,8 +833,7 @@ def test_available_plans_for_sentry_plan_expired_trial_more_than_10_users( self, is_sentry_user ): is_sentry_user.return_value = True - plan = PlanFactory(name=PlanName.SENTRY_MONTHLY.value) - self.current_org.plan = plan.name + self.current_org.plan = PlanName.SENTRY_MONTHLY.value self.current_org.save() plan_service = PlanService(current_org=self.current_org) From ccb28ab04b8692144a4af0b98d0ce59e1f680c85 Mon Sep 17 00:00:00 2001 From: RulaKhaled Date: Tue, 21 Jan 2025 13:40:33 +0100 Subject: [PATCH 08/18] 5 more to go --- tests/unit/plan/test_plan.py | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/tests/unit/plan/test_plan.py b/tests/unit/plan/test_plan.py index dd8bb2ad9..30e61057d 100644 --- a/tests/unit/plan/test_plan.py +++ b/tests/unit/plan/test_plan.py @@ -604,8 +604,8 @@ def test_available_plans_for_sentry_customer_basic_plan_non_trial( expected_result = [] expected_result.append(BASIC_PLAN) expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() expected_result += TEAM_PLAN_REPRESENTATIONS.values() + expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() expected_result = [result.convert_to_DTO() for result in expected_result] assert plan_service.available_plans(owner=self.owner) == expected_result @@ -623,8 +623,8 @@ def test_available_plans_for_sentry_customer_team_plan_non_trial( expected_result = [] expected_result.append(BASIC_PLAN) expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() expected_result += TEAM_PLAN_REPRESENTATIONS.values() + expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() expected_result = [result.convert_to_DTO() for result in expected_result] assert plan_service.available_plans(owner=self.owner) == expected_result @@ -638,10 +638,10 @@ def test_available_plans_for_sentry_plan_non_trial(self, is_sentry_user): plan_service = PlanService(current_org=self.current_org) expected_result = [] - expected_result.append(BASIC_PLAN) expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() expected_result += TEAM_PLAN_REPRESENTATIONS.values() + expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() + expected_result.append(BASIC_PLAN) expected_result = [result.convert_to_DTO() for result in expected_result] assert plan_service.available_plans(owner=self.owner) == expected_result @@ -730,12 +730,6 @@ def test_available_plans_for_sentry_customer_basic_plan_expired_trial_less_than_ expected_result += TEAM_PLAN_REPRESENTATIONS.values() expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() expected_result = [result.convert_to_DTO() for result in expected_result] - print( - "array 1", - expected_result, - "array 2", - plan_service.available_plans(owner=self.owner), - ) assert plan_service.available_plans(owner=self.owner) == expected_result @@ -771,8 +765,8 @@ def test_available_plans_for_sentry_plan_expired_trial_less_than_10_users( expected_result = [] expected_result.append(BASIC_PLAN) expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() expected_result += TEAM_PLAN_REPRESENTATIONS.values() + expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() expected_result = [result.convert_to_DTO() for result in expected_result] assert plan_service.available_plans(owner=self.owner) == expected_result @@ -861,13 +855,13 @@ def setUp(self): self.expected_result = [ result.convert_to_DTO() for result in self.expected_result ] + mock_all_plans_and_tiers() def test_currently_team_plan(self): - plan = PlanFactory(name=PlanName.TEAM_MONTHLY.value) self.current_org = OwnerFactory( plan_user_count=100, plan_activated_users=[i for i in range(10)], - plan=plan.name, + plan=PlanName.TEAM_MONTHLY.value, ) self.owner = OwnerFactory() self.plan_service = PlanService(current_org=self.current_org) @@ -992,8 +986,8 @@ def test_sentry_user(self, is_sentry_user): expected_result = [] expected_result.append(BASIC_PLAN) expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() expected_result += TEAM_PLAN_REPRESENTATIONS.values() + expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() expected_result = [result.convert_to_DTO() for result in expected_result] # Can do Team plan when plan_activated_users is null From 208eb10ab04edb3fae231f73a1a3e8bbb8a7eed2 Mon Sep 17 00:00:00 2001 From: RulaKhaled Date: Tue, 21 Jan 2025 14:12:18 +0100 Subject: [PATCH 09/18] wrap up tests in test_plan --- shared/plan/service.py | 2 +- tests/unit/plan/test_plan.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/shared/plan/service.py b/shared/plan/service.py index e289c9165..57e3aafcc 100644 --- a/shared/plan/service.py +++ b/shared/plan/service.py @@ -295,7 +295,7 @@ def trial_end_date(self) -> Optional[datetime]: @property def trial_total_days(self) -> Optional[TrialDaysAmount]: """Returns the total number of trial days.""" - return self.plan_data.trial_days + return TrialDaysAmount.CODECOV_SENTRY.value if self.is_org_trialing else None @property def is_org_trialing(self) -> bool: diff --git a/tests/unit/plan/test_plan.py b/tests/unit/plan/test_plan.py index 30e61057d..83dbf822d 100644 --- a/tests/unit/plan/test_plan.py +++ b/tests/unit/plan/test_plan.py @@ -584,9 +584,9 @@ def test_available_plans_for_pro_plan_non_trial(self): plan_service = PlanService(current_org=self.current_org) expected_result = [] - expected_result.append(BASIC_PLAN) expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() expected_result += TEAM_PLAN_REPRESENTATIONS.values() + expected_result.append(BASIC_PLAN) expected_result = [result.convert_to_DTO() for result in expected_result] assert plan_service.available_plans(owner=self.owner) == expected_result @@ -798,8 +798,8 @@ def test_available_plans_for_pro_plan_expired_trial_more_than_10_users(self): plan_service = PlanService(current_org=self.current_org) expected_result = [] - expected_result.append(BASIC_PLAN) expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() + expected_result.append(BASIC_PLAN) expected_result = [result.convert_to_DTO() for result in expected_result] assert plan_service.available_plans(owner=self.owner) == expected_result @@ -934,8 +934,9 @@ class AvailablePlansOngoingTrial(TestCase): """ def setUp(self): + mock_all_plans_and_tiers() self.current_org = OwnerFactory( - plan=PlanName.TRIAL_PLAN_NAME.value, + plan=PlanName.BASIC_PLAN_NAME.value, trial_start_date=datetime.utcnow(), trial_end_date=datetime.utcnow() + timedelta(days=14), trial_status=TrialStatus.ONGOING.value, @@ -944,7 +945,6 @@ def setUp(self): ) self.owner = OwnerFactory() self.plan_service = PlanService(current_org=self.current_org) - mock_all_plans_and_tiers() def test_non_sentry_user(self): # [Basic, Pro Monthly, Pro Yearly, Team Monthly, Team Yearly] From a9a5eeb0150e8711acd9e3e51531e7b754faf9a8 Mon Sep 17 00:00:00 2001 From: RulaKhaled Date: Tue, 21 Jan 2025 14:46:37 +0100 Subject: [PATCH 10/18] resolve all tests --- shared/django_apps/codecov_auth/models.py | 7 +++++-- .../codecov_auth/test_codecov_auth_models.py | 12 ++++++++---- tests/unit/upload/test_utils.py | 4 ++++ 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/shared/django_apps/codecov_auth/models.py b/shared/django_apps/codecov_auth/models.py index f02367e44..ee029d9df 100644 --- a/shared/django_apps/codecov_auth/models.py +++ b/shared/django_apps/codecov_auth/models.py @@ -221,7 +221,9 @@ def pretty_plan(self) -> dict | None: We inject quantity to make plan management easier on api, see PlanSerializer """ plan_details = Plan.objects.get(name=self.plan) - plan_details.update({"quantity": self.plan_seat_count}) + plan_details.quantity = self.plan_seat_count + plan_details.save() + return plan_details def can_activate_user(self, user: User | None = None) -> bool: @@ -599,7 +601,8 @@ def pretty_plan(self): # some iffy data modeling if plan_details: - plan_details.update({"quantity": self.plan_user_count}) + plan_details.quantity = self.plan_user_count + plan_details.save() return plan_details def can_activate_user(self, owner_user: Self) -> bool: diff --git a/tests/unit/django_apps/codecov_auth/test_codecov_auth_models.py b/tests/unit/django_apps/codecov_auth/test_codecov_auth_models.py index b4f822c44..7eb40e864 100644 --- a/tests/unit/django_apps/codecov_auth/test_codecov_auth_models.py +++ b/tests/unit/django_apps/codecov_auth/test_codecov_auth_models.py @@ -41,11 +41,13 @@ PlanName, ) from shared.utils.test_utils import mock_config_helper +from tests.unit.plan.test_plan import mock_all_plans_and_tiers class TestOwnerModel(TransactionTestCase): def setUp(self): self.owner = OwnerFactory(username="codecov_name", email="name@codecov.io") + mock_all_plans_and_tiers() def test_repo_total_credits_returns_correct_repos_for_legacy_plan(self): self.owner.plan = "5m" @@ -384,7 +386,7 @@ def test_fields_that_account_overrides(self): self.assertTrue(self.owner.can_activate_user(to_activate)) org_pretty_plan = asdict(BASIC_PLAN) org_pretty_plan.update({"quantity": 1}) - self.assertEqual(self.owner.pretty_plan, org_pretty_plan) + self.assertEqual(self.owner.pretty_plan.quantity, org_pretty_plan["quantity"]) self.owner.account = AccountFactory( plan_seat_count=0, plan=PlanName.ENTERPRISE_CLOUD_YEARLY.value @@ -396,7 +398,7 @@ def test_fields_that_account_overrides(self): ENTERPRISE_CLOUD_USER_PLAN_REPRESENTATIONS[self.owner.account.plan] ) account_pretty_plan.update({"quantity": 0}) - self.assertEqual(self.owner.pretty_plan, account_pretty_plan) + self.assertEqual(self.owner.pretty_plan.quantity, account_pretty_plan["quantity"]) def test_add_admin_adds_ownerid_to_admin_array(self): self.owner.admins = [] @@ -717,6 +719,7 @@ def test_account_with_billing_details(self): self.assertTrue(stripe.is_active) def test_account_with_users(self): + mock_all_plans_and_tiers() user_1 = UserFactory() OwnerFactory(user=user_1) user_2 = UserFactory() @@ -758,9 +761,10 @@ def test_account_with_users(self): self.assertEqual(account.available_seat_count, 0) pretty_plan = asdict(BASIC_PLAN) pretty_plan.update({"quantity": 1}) - self.assertEqual(account.pretty_plan, pretty_plan) + self.assertEqual(account.pretty_plan.quantity, pretty_plan["quantity"]) def test_create_account_for_enterprise_experience(self): + mock_all_plans_and_tiers() # 2 separate OwnerOrgs that wish to Enterprise stripe_customer_id = "abc123" stripe_subscription_id = "defg456" @@ -928,7 +932,7 @@ def test_create_account_for_enterprise_experience(self): self.assertEqual(enterprise_account.available_seat_count, 57) pretty_plan = asdict(BASIC_PLAN) pretty_plan.update({"quantity": 50}) - self.assertEqual(enterprise_account.pretty_plan, pretty_plan) + self.assertEqual(enterprise_account.pretty_plan.quantity, pretty_plan["quantity"]) def test_activate_user_onto_account(self): user = UserFactory() diff --git a/tests/unit/upload/test_utils.py b/tests/unit/upload/test_utils.py index 0dcffd79e..0f8109337 100644 --- a/tests/unit/upload/test_utils.py +++ b/tests/unit/upload/test_utils.py @@ -18,9 +18,13 @@ insert_coverage_measurement, query_monthly_coverage_measurements, ) +from tests.unit.plan.test_plan import mock_all_plans_and_tiers class CoverageMeasurement(TestCase): + def setUp(self): + mock_all_plans_and_tiers() + def add_upload_measurements_records( self, owner: Owner, From 3dcd8efa027e6f45e0cbfd2c08a936ec5a68a57c Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Tue, 21 Jan 2025 11:30:46 -0800 Subject: [PATCH 11/18] update test imports and helper, remove trial_days from DTO, start offboarding the consts --- shared/plan/constants.py | 2 - shared/plan/service.py | 2 +- tests/helper.py | 159 ++++++++++++++++++ .../codecov_auth/test_codecov_auth_models.py | 10 +- tests/unit/plan/test_plan.py | 156 +---------------- tests/unit/upload/test_utils.py | 2 +- 6 files changed, 169 insertions(+), 162 deletions(-) diff --git a/shared/plan/constants.py b/shared/plan/constants.py index 825439740..0bec108d7 100644 --- a/shared/plan/constants.py +++ b/shared/plan/constants.py @@ -101,7 +101,6 @@ def convert_to_DTO(self) -> dict: "benefits": self.benefits, "tier_name": self.tier_name, "monthly_uploads_limit": self.monthly_uploads_limit, - "trial_days": self.trial_days or TrialDaysAmount.CODECOV_SENTRY.value, "is_free_plan": self.tier_name == TierName.BASIC.value, "is_pro_plan": self.tier_name == TierName.PRO.value, "is_team_plan": self.tier_name == TierName.TEAM.value, @@ -120,7 +119,6 @@ def convert_to_DTO(plan) -> dict: "benefits": plan.benefits, "tier_name": plan.tier.tier_name, "monthly_uploads_limit": plan.monthly_uploads_limit, - "trial_days": TrialDaysAmount.CODECOV_SENTRY.value, "is_free_plan": not plan.paid_plan, "is_pro_plan": plan.tier.tier_name == TierName.PRO.value, "is_team_plan": plan.tier.tier_name == TierName.TEAM.value, diff --git a/shared/plan/service.py b/shared/plan/service.py index 57e3aafcc..c34d80b83 100644 --- a/shared/plan/service.py +++ b/shared/plan/service.py @@ -295,7 +295,7 @@ def trial_end_date(self) -> Optional[datetime]: @property def trial_total_days(self) -> Optional[TrialDaysAmount]: """Returns the total number of trial days.""" - return TrialDaysAmount.CODECOV_SENTRY.value if self.is_org_trialing else None + return TrialDaysAmount.CODECOV_SENTRY.value @property def is_org_trialing(self) -> bool: diff --git a/tests/helper.py b/tests/helper.py index a39fb359c..9953b3fd8 100644 --- a/tests/helper.py +++ b/tests/helper.py @@ -1,5 +1,13 @@ from json import dumps +from shared.django_apps.codecov_auth.models import BillingRate +from shared.django_apps.codecov_auth.tests.factories import PlanFactory, TierFactory +from shared.plan.constants import ( + PlanName, + PlanPrice, + TierName, +) + def v2_to_v3(report): def _sessions(sessions): @@ -46,3 +54,154 @@ def _sessions(sessions): "totals": report.get("totals", {}), "chunks": chunks, } + + +def mock_all_plans_and_tiers(): + trial_tier = TierFactory(tier_name=TierName.TRIAL.value) + PlanFactory( + tier=trial_tier, + name=PlanName.TRIAL_PLAN_NAME.value, + paid_plan=False, + marketing_name="Developer", + benefits=[ + "Configurable # of users", + "Unlimited public repositories", + "Unlimited private repositories", + "Priority Support", + ], + ) + + basic_tier = TierFactory(tier_name=TierName.BASIC.value) + PlanFactory( + name=PlanName.BASIC_PLAN_NAME.value, + tier=basic_tier, + marketing_name="Developer", + benefits=[ + "Up to 1 user", + "Unlimited public repositories", + "Unlimited private repositories", + ], + monthly_uploads_limit=250, + ) + PlanFactory( + name=PlanName.FREE_PLAN_NAME.value, + tier=basic_tier, + marketing_name="Developer", + benefits=[ + "Up to 1 user", + "Unlimited public repositories", + "Unlimited private repositories", + ], + ) + + pro_tier = TierFactory(tier_name=TierName.PRO.value) + PlanFactory( + name=PlanName.CODECOV_PRO_MONTHLY.value, + tier=pro_tier, + marketing_name="Pro", + benefits=[ + "Configurable # of users", + "Unlimited public repositories", + "Unlimited private repositories", + "Priority Support", + ], + billing_rate=BillingRate.MONTHLY.value, + base_unit_price=PlanPrice.MONTHLY.value, + paid_plan=True, + ) + PlanFactory( + name=PlanName.CODECOV_PRO_YEARLY.value, + tier=pro_tier, + marketing_name="Pro", + benefits=[ + "Configurable # of users", + "Unlimited public repositories", + "Unlimited private repositories", + "Priority Support", + ], + billing_rate=BillingRate.ANNUALLY.value, + base_unit_price=PlanPrice.YEARLY.value, + paid_plan=True, + ) + + team_tier = TierFactory(tier_name=TierName.TEAM.value) + PlanFactory( + name=PlanName.TEAM_MONTHLY.value, + tier=team_tier, + marketing_name="Team", + benefits=[ + "Up to 10 users", + "Unlimited repositories", + "2500 private repo uploads", + "Patch coverage analysis", + ], + billing_rate=BillingRate.MONTHLY.value, + base_unit_price=PlanPrice.TEAM_MONTHLY.value, + monthly_uploads_limit=2500, + paid_plan=True, + ) + PlanFactory( + name=PlanName.TEAM_YEARLY.value, + tier=team_tier, + marketing_name="Team", + benefits=[ + "Up to 10 users", + "Unlimited repositories", + "2500 private repo uploads", + "Patch coverage analysis", + ], + billing_rate=BillingRate.ANNUALLY.value, + base_unit_price=PlanPrice.TEAM_YEARLY.value, + monthly_uploads_limit=2500, + paid_plan=True, + ) + + sentry_tier = TierFactory(tier_name=TierName.SENTRY.value) + PlanFactory( + name=PlanName.SENTRY_MONTHLY.value, + tier=sentry_tier, + marketing_name="Sentry Pro", + billing_rate=BillingRate.MONTHLY.value, + base_unit_price=PlanPrice.MONTHLY.value, + paid_plan=True, + benefits=[ + "Includes 5 seats", + "$12 per additional seat", + "Unlimited public repositories", + "Unlimited private repositories", + "Priority Support", + ], + ) + PlanFactory( + name=PlanName.SENTRY_YEARLY.value, + tier=sentry_tier, + marketing_name="Sentry Pro", + billing_rate=BillingRate.ANNUALLY.value, + base_unit_price=PlanPrice.YEARLY.value, + paid_plan=True, + benefits=[ + "Includes 5 seats", + "$10 per additional seat", + "Unlimited public repositories", + "Unlimited private repositories", + "Priority Support", + ], + ) + + enterprise_tier = TierFactory(tier_name=TierName.ENTERPRISE.value) + PlanFactory( + name=PlanName.ENTERPRISE_CLOUD_MONTHLY.value, + tier=enterprise_tier, + marketing_name="Enterprise", + billing_rate=BillingRate.MONTHLY.value, + base_unit_price=PlanPrice.MONTHLY.value, + paid_plan=True, + ) + PlanFactory( + name=PlanName.ENTERPRISE_CLOUD_YEARLY.value, + tier=enterprise_tier, + marketing_name="Enterprise", + billing_rate=BillingRate.ANNUALLY.value, + base_unit_price=PlanPrice.YEARLY.value, + paid_plan=True, + ) diff --git a/tests/unit/django_apps/codecov_auth/test_codecov_auth_models.py b/tests/unit/django_apps/codecov_auth/test_codecov_auth_models.py index 7eb40e864..a3845e2ea 100644 --- a/tests/unit/django_apps/codecov_auth/test_codecov_auth_models.py +++ b/tests/unit/django_apps/codecov_auth/test_codecov_auth_models.py @@ -41,7 +41,7 @@ PlanName, ) from shared.utils.test_utils import mock_config_helper -from tests.unit.plan.test_plan import mock_all_plans_and_tiers +from tests.helper import mock_all_plans_and_tiers class TestOwnerModel(TransactionTestCase): @@ -398,7 +398,9 @@ def test_fields_that_account_overrides(self): ENTERPRISE_CLOUD_USER_PLAN_REPRESENTATIONS[self.owner.account.plan] ) account_pretty_plan.update({"quantity": 0}) - self.assertEqual(self.owner.pretty_plan.quantity, account_pretty_plan["quantity"]) + self.assertEqual( + self.owner.pretty_plan.quantity, account_pretty_plan["quantity"] + ) def test_add_admin_adds_ownerid_to_admin_array(self): self.owner.admins = [] @@ -932,7 +934,9 @@ def test_create_account_for_enterprise_experience(self): self.assertEqual(enterprise_account.available_seat_count, 57) pretty_plan = asdict(BASIC_PLAN) pretty_plan.update({"quantity": 50}) - self.assertEqual(enterprise_account.pretty_plan.quantity, pretty_plan["quantity"]) + self.assertEqual( + enterprise_account.pretty_plan.quantity, pretty_plan["quantity"] + ) def test_activate_user_onto_account(self): user = UserFactory() diff --git a/tests/unit/plan/test_plan.py b/tests/unit/plan/test_plan.py index 83dbf822d..2a95512b2 100644 --- a/tests/unit/plan/test_plan.py +++ b/tests/unit/plan/test_plan.py @@ -20,166 +20,13 @@ TEAM_PLAN_REPRESENTATIONS, TRIAL_PLAN_REPRESENTATION, TRIAL_PLAN_SEATS, - MonthlyUploadLimits, - PlanBillingRate, PlanName, - PlanPrice, TierName, TrialDaysAmount, TrialStatus, ) from shared.plan.service import PlanService - - -def mock_all_plans_and_tiers(): - trial_tier = TierFactory(tier_name=TierName.TRIAL.value) - PlanFactory( - tier=trial_tier, - name=PlanName.TRIAL_PLAN_NAME.value, - paid_plan=False, - marketing_name="Developer", - benefits=[ - "Configurable # of users", - "Unlimited public repositories", - "Unlimited private repositories", - "Priority Support", - ], - ) - - basic_tier = TierFactory(tier_name=TierName.BASIC.value) - PlanFactory( - name=PlanName.BASIC_PLAN_NAME.value, - tier=basic_tier, - marketing_name="Developer", - benefits=[ - "Up to 1 user", - "Unlimited public repositories", - "Unlimited private repositories", - ], - monthly_uploads_limit=MonthlyUploadLimits.CODECOV_BASIC_PLAN.value, - ) - PlanFactory( - name=PlanName.FREE_PLAN_NAME.value, - tier=basic_tier, - marketing_name="Developer", - benefits=[ - "Up to 1 user", - "Unlimited public repositories", - "Unlimited private repositories", - ], - ) - - pro_tier = TierFactory(tier_name=TierName.PRO.value) - PlanFactory( - name=PlanName.CODECOV_PRO_MONTHLY.value, - tier=pro_tier, - marketing_name="Pro", - benefits=[ - "Configurable # of users", - "Unlimited public repositories", - "Unlimited private repositories", - "Priority Support", - ], - billing_rate=PlanBillingRate.MONTHLY.value, - base_unit_price=PlanPrice.MONTHLY.value, - paid_plan=True, - ) - PlanFactory( - name=PlanName.CODECOV_PRO_YEARLY.value, - tier=pro_tier, - marketing_name="Pro", - benefits=[ - "Configurable # of users", - "Unlimited public repositories", - "Unlimited private repositories", - "Priority Support", - ], - billing_rate=PlanBillingRate.YEARLY.value, - base_unit_price=PlanPrice.YEARLY.value, - paid_plan=True, - ) - - team_tier = TierFactory(tier_name=TierName.TEAM.value) - PlanFactory( - name=PlanName.TEAM_MONTHLY.value, - tier=team_tier, - marketing_name="Team", - benefits=[ - "Up to 10 users", - "Unlimited repositories", - "2500 private repo uploads", - "Patch coverage analysis", - ], - billing_rate=PlanBillingRate.MONTHLY.value, - base_unit_price=PlanPrice.TEAM_MONTHLY.value, - monthly_uploads_limit=MonthlyUploadLimits.CODECOV_TEAM_PLAN.value, - paid_plan=True, - ) - PlanFactory( - name=PlanName.TEAM_YEARLY.value, - tier=team_tier, - marketing_name="Team", - benefits=[ - "Up to 10 users", - "Unlimited repositories", - "2500 private repo uploads", - "Patch coverage analysis", - ], - billing_rate=PlanBillingRate.YEARLY.value, - base_unit_price=PlanPrice.TEAM_YEARLY.value, - monthly_uploads_limit=MonthlyUploadLimits.CODECOV_TEAM_PLAN.value, - paid_plan=True, - ) - - sentry_tier = TierFactory(tier_name=TierName.SENTRY.value) - PlanFactory( - name=PlanName.SENTRY_MONTHLY.value, - tier=sentry_tier, - marketing_name="Sentry Pro", - billing_rate=PlanBillingRate.MONTHLY.value, - base_unit_price=PlanPrice.MONTHLY.value, - paid_plan=True, - benefits=[ - "Includes 5 seats", - "$12 per additional seat", - "Unlimited public repositories", - "Unlimited private repositories", - "Priority Support", - ], - ) - PlanFactory( - name=PlanName.SENTRY_YEARLY.value, - tier=sentry_tier, - marketing_name="Sentry Pro", - billing_rate=PlanBillingRate.YEARLY.value, - base_unit_price=PlanPrice.YEARLY.value, - paid_plan=True, - benefits=[ - "Includes 5 seats", - "$10 per additional seat", - "Unlimited public repositories", - "Unlimited private repositories", - "Priority Support", - ], - ) - - enterprise_tier = TierFactory(tier_name=TierName.ENTERPRISE.value) - PlanFactory( - name=PlanName.ENTERPRISE_CLOUD_MONTHLY.value, - tier=enterprise_tier, - marketing_name="Enterprise", - billing_rate=PlanBillingRate.MONTHLY.value, - base_unit_price=PlanPrice.MONTHLY.value, - paid_plan=True, - ) - PlanFactory( - name=PlanName.ENTERPRISE_CLOUD_YEARLY.value, - tier=enterprise_tier, - marketing_name="Enterprise", - billing_rate=PlanBillingRate.YEARLY.value, - base_unit_price=PlanPrice.YEARLY.value, - paid_plan=True, - ) +from tests.helper import mock_all_plans_and_tiers @freeze_time("2023-06-19") @@ -424,7 +271,6 @@ def test_plan_service_returns_plan_data_for_non_trial_basic_plan(self): assert ( plan_service.monthly_uploads_limit == 250 ) # should be 250 since not trialing - assert plan_service.trial_total_days == basic_plan.trial_days def test_plan_service_returns_plan_data_for_trialing_user_trial_plan(self): trial_start_date = datetime.utcnow() diff --git a/tests/unit/upload/test_utils.py b/tests/unit/upload/test_utils.py index 0f8109337..9175bd9a4 100644 --- a/tests/unit/upload/test_utils.py +++ b/tests/unit/upload/test_utils.py @@ -18,7 +18,7 @@ insert_coverage_measurement, query_monthly_coverage_measurements, ) -from tests.unit.plan.test_plan import mock_all_plans_and_tiers +from tests.helper import mock_all_plans_and_tiers class CoverageMeasurement(TestCase): From 6b1fabc070ae0b9b1c938ccd1e02a62426b0e592 Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Tue, 21 Jan 2025 11:59:47 -0800 Subject: [PATCH 12/18] lint --- shared/django_apps/codecov_auth/models.py | 2 +- shared/django_apps/utils/model_utils.py | 6 +++--- tests/unit/bundle_analysis/test_bundle_analysis.py | 12 ++++++------ 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/shared/django_apps/codecov_auth/models.py b/shared/django_apps/codecov_auth/models.py index ee029d9df..0392a1570 100644 --- a/shared/django_apps/codecov_auth/models.py +++ b/shared/django_apps/codecov_auth/models.py @@ -223,7 +223,7 @@ def pretty_plan(self) -> dict | None: plan_details = Plan.objects.get(name=self.plan) plan_details.quantity = self.plan_seat_count plan_details.save() - + return plan_details def can_activate_user(self, user: User | None = None) -> bool: diff --git a/shared/django_apps/utils/model_utils.py b/shared/django_apps/utils/model_utils.py index 867497847..03d30a871 100644 --- a/shared/django_apps/utils/model_utils.py +++ b/shared/django_apps/utils/model_utils.py @@ -75,9 +75,9 @@ def __init__( def __set_name__(self, owner, name): # Validate that the owner class has the methods we need - assert issubclass( - owner, ArchiveFieldInterface - ), "Missing some required methods to use AchiveField" + assert issubclass(owner, ArchiveFieldInterface), ( + "Missing some required methods to use AchiveField" + ) self.public_name = name self.db_field_name = "_" + name self.archive_field_name = "_" + name + "_storage_path" diff --git a/tests/unit/bundle_analysis/test_bundle_analysis.py b/tests/unit/bundle_analysis/test_bundle_analysis.py index fd3d802c3..dd49ea2e8 100644 --- a/tests/unit/bundle_analysis/test_bundle_analysis.py +++ b/tests/unit/bundle_analysis/test_bundle_analysis.py @@ -848,18 +848,18 @@ def load_and_test_report( report.ingest(report_path) bundle_report = report.bundle_report("sample") asset_reports = list(bundle_report.asset_reports()) - assert ( - bundle_report.total_size() == expected_total_size - ), f"Version {version}: Total size mismatch" + assert bundle_report.total_size() == expected_total_size, ( + f"Version {version}: Total size mismatch" + ) total_js_size = sum( asset.size for asset in asset_reports if asset.asset_type == AssetType.JAVASCRIPT ) - assert ( - total_js_size == expected_js_size - ), f"Version {version}: JS size mismatch" + assert total_js_size == expected_js_size, ( + f"Version {version}: JS size mismatch" + ) finally: report.cleanup() From 906e7bc916e80d89583f2086a7e348c124bf6f72 Mon Sep 17 00:00:00 2001 From: RulaKhaled Date: Wed, 22 Jan 2025 18:36:09 +0100 Subject: [PATCH 13/18] Update with pretty plan changes --- shared/django_apps/codecov_auth/models.py | 43 +++++++++++++------ tests/helper.py | 16 ++++++- .../codecov_auth/test_codecov_auth_models.py | 16 ++++--- 3 files changed, 53 insertions(+), 22 deletions(-) diff --git a/shared/django_apps/codecov_auth/models.py b/shared/django_apps/codecov_auth/models.py index 0392a1570..064014bea 100644 --- a/shared/django_apps/codecov_auth/models.py +++ b/shared/django_apps/codecov_auth/models.py @@ -29,7 +29,7 @@ from shared.django_apps.codecov_auth.managers import OwnerManager from shared.django_apps.core.managers import RepositoryManager from shared.django_apps.core.models import DateTimeWithoutTZField, Repository -from shared.plan.constants import PlanName +from shared.plan.constants import PlanName, TrialDaysAmount # Added to avoid 'doesn't declare an explicit app_label and isn't in an application in INSTALLED_APPS' error\ # Needs to be called the same as the API app @@ -221,10 +221,21 @@ def pretty_plan(self) -> dict | None: We inject quantity to make plan management easier on api, see PlanSerializer """ plan_details = Plan.objects.get(name=self.plan) - plan_details.quantity = self.plan_seat_count - plan_details.save() - - return plan_details + if plan_details: + return { + "marketing_name": plan_details.marketing_name, + "value": plan_details.name, + "billing_rate": plan_details.billing_rate, + "base_unit_price": plan_details.base_unit_price, + "benefits": plan_details.benefits, + "tier_name": plan_details.tier.tier_name, + "monthly_uploads_limit": plan_details.monthly_uploads_limit, + "trial_days": TrialDaysAmount.CODECOV_SENTRY.value + if plan_details.name == PlanName.TRIAL_PLAN_NAME.value + else None, + "quantity": self.plan_seat_count, + } + return None def can_activate_user(self, user: User | None = None) -> bool: """ @@ -595,15 +606,21 @@ def pretty_plan(self): return self.account.pretty_plan plan_details = Plan.objects.get(name=self.plan) - # update with quantity they've purchased - # allows api users to update the quantity - # by modifying the "plan", sidestepping - # some iffy data modeling - if plan_details: - plan_details.quantity = self.plan_user_count - plan_details.save() - return plan_details + return { + "marketing_name": plan_details.marketing_name, + "value": plan_details.name, + "billing_rate": plan_details.billing_rate, + "base_unit_price": plan_details.base_unit_price, + "benefits": plan_details.benefits, + "tier_name": plan_details.tier.tier_name, + "monthly_uploads_limit": plan_details.monthly_uploads_limit, + "trial_days": TrialDaysAmount.CODECOV_SENTRY.value + if plan_details.name == PlanName.TRIAL_PLAN_NAME.value + else None, + "quantity": self.plan_user_count, + } + return None def can_activate_user(self, owner_user: Self) -> bool: owner_org = self diff --git a/tests/helper.py b/tests/helper.py index 9953b3fd8..a065ed883 100644 --- a/tests/helper.py +++ b/tests/helper.py @@ -192,7 +192,13 @@ def mock_all_plans_and_tiers(): PlanFactory( name=PlanName.ENTERPRISE_CLOUD_MONTHLY.value, tier=enterprise_tier, - marketing_name="Enterprise", + marketing_name="Enterprise Cloud", + benefits=[ + "Configurable # of users", + "Unlimited public repositories", + "Unlimited private repositories", + "Priority Support", + ], billing_rate=BillingRate.MONTHLY.value, base_unit_price=PlanPrice.MONTHLY.value, paid_plan=True, @@ -200,8 +206,14 @@ def mock_all_plans_and_tiers(): PlanFactory( name=PlanName.ENTERPRISE_CLOUD_YEARLY.value, tier=enterprise_tier, - marketing_name="Enterprise", + marketing_name="Enterprise Cloud", billing_rate=BillingRate.ANNUALLY.value, base_unit_price=PlanPrice.YEARLY.value, paid_plan=True, + benefits=[ + "Configurable # of users", + "Unlimited public repositories", + "Unlimited private repositories", + "Priority Support", + ], ) diff --git a/tests/unit/django_apps/codecov_auth/test_codecov_auth_models.py b/tests/unit/django_apps/codecov_auth/test_codecov_auth_models.py index a3845e2ea..ca29c2da3 100644 --- a/tests/unit/django_apps/codecov_auth/test_codecov_auth_models.py +++ b/tests/unit/django_apps/codecov_auth/test_codecov_auth_models.py @@ -386,7 +386,7 @@ def test_fields_that_account_overrides(self): self.assertTrue(self.owner.can_activate_user(to_activate)) org_pretty_plan = asdict(BASIC_PLAN) org_pretty_plan.update({"quantity": 1}) - self.assertEqual(self.owner.pretty_plan.quantity, org_pretty_plan["quantity"]) + self.assertEqual(self.owner.pretty_plan, org_pretty_plan) self.owner.account = AccountFactory( plan_seat_count=0, plan=PlanName.ENTERPRISE_CLOUD_YEARLY.value @@ -398,9 +398,13 @@ def test_fields_that_account_overrides(self): ENTERPRISE_CLOUD_USER_PLAN_REPRESENTATIONS[self.owner.account.plan] ) account_pretty_plan.update({"quantity": 0}) - self.assertEqual( - self.owner.pretty_plan.quantity, account_pretty_plan["quantity"] + print( + "account_pretty_plan", + account_pretty_plan, + "self.owner.pretty_plan", + self.owner.pretty_plan, ) + self.assertEqual(self.owner.pretty_plan, account_pretty_plan) def test_add_admin_adds_ownerid_to_admin_array(self): self.owner.admins = [] @@ -763,7 +767,7 @@ def test_account_with_users(self): self.assertEqual(account.available_seat_count, 0) pretty_plan = asdict(BASIC_PLAN) pretty_plan.update({"quantity": 1}) - self.assertEqual(account.pretty_plan.quantity, pretty_plan["quantity"]) + self.assertEqual(account.pretty_plan, pretty_plan) def test_create_account_for_enterprise_experience(self): mock_all_plans_and_tiers() @@ -934,9 +938,7 @@ def test_create_account_for_enterprise_experience(self): self.assertEqual(enterprise_account.available_seat_count, 57) pretty_plan = asdict(BASIC_PLAN) pretty_plan.update({"quantity": 50}) - self.assertEqual( - enterprise_account.pretty_plan.quantity, pretty_plan["quantity"] - ) + self.assertEqual(enterprise_account.pretty_plan, pretty_plan) def test_activate_user_onto_account(self): user = UserFactory() From 2cd74364631fb80da89ee7b2d739adabb2621ca4 Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Thu, 23 Jan 2025 11:41:06 -0800 Subject: [PATCH 14/18] setupClass from setUp and clean up a bunch of the available plan tests --- shared/django_apps/codecov_auth/models.py | 5 +- shared/plan/constants.py | 36 +- shared/plan/service.py | 18 +- .../codecov_auth/test_codecov_auth_models.py | 8 +- tests/unit/plan/test_plan.py | 511 ++++++++++++------ tests/unit/upload/test_utils.py | 4 +- 6 files changed, 387 insertions(+), 195 deletions(-) diff --git a/shared/django_apps/codecov_auth/models.py b/shared/django_apps/codecov_auth/models.py index 064014bea..7b12e180c 100644 --- a/shared/django_apps/codecov_auth/models.py +++ b/shared/django_apps/codecov_auth/models.py @@ -220,7 +220,7 @@ def pretty_plan(self) -> dict | None: This is how we represent the details of a plan to a user, see plan.constants.py We inject quantity to make plan management easier on api, see PlanSerializer """ - plan_details = Plan.objects.get(name=self.plan) + plan_details = Plan.objects.select_related("tier").get(name=self.plan) if plan_details: return { "marketing_name": plan_details.marketing_name, @@ -605,7 +605,8 @@ def pretty_plan(self): if self.account: return self.account.pretty_plan - plan_details = Plan.objects.get(name=self.plan) + print("@@@@@@@@@@@@@@@@@@@@", self.plan) + plan_details = Plan.objects.select_related("tier").get(name=self.plan) if plan_details: return { "marketing_name": plan_details.marketing_name, diff --git a/shared/plan/constants.py b/shared/plan/constants.py index 0bec108d7..30583dbde 100644 --- a/shared/plan/constants.py +++ b/shared/plan/constants.py @@ -77,6 +77,24 @@ class TierName(enum.Enum): TRIAL = "trial" +def convert_to_DTO(plan) -> dict: + return { + "marketing_name": plan.marketing_name, + "value": plan.name, + "billing_rate": plan.billing_rate, + "base_unit_price": plan.base_unit_price, + "benefits": plan.benefits, + "tier_name": plan.tier.tier_name, + "monthly_uploads_limit": plan.monthly_uploads_limit, + "is_free_plan": not plan.paid_plan, + "is_pro_plan": plan.tier.tier_name == TierName.PRO.value, + "is_team_plan": plan.tier.tier_name == TierName.TEAM.value, + "is_enterprise_plan": plan.tier.tier_name == TierName.ENTERPRISE.value, + "is_trial_plan": plan.tier.tier_name == TierName.TRIAL.value, + "is_sentry_plan": plan.tier.tier_name == TierName.SENTRY.value, + } + + @dataclass(repr=False) class PlanData: """ @@ -110,24 +128,6 @@ def convert_to_DTO(self) -> dict: } -def convert_to_DTO(plan) -> dict: - return { - "marketing_name": plan.marketing_name, - "value": plan.name, - "billing_rate": plan.billing_rate, - "base_unit_price": plan.base_unit_price, - "benefits": plan.benefits, - "tier_name": plan.tier.tier_name, - "monthly_uploads_limit": plan.monthly_uploads_limit, - "is_free_plan": not plan.paid_plan, - "is_pro_plan": plan.tier.tier_name == TierName.PRO.value, - "is_team_plan": plan.tier.tier_name == TierName.TEAM.value, - "is_enterprise_plan": plan.tier.tier_name == TierName.ENTERPRISE.value, - "is_trial_plan": plan.tier.tier_name == TierName.TRIAL.value, - "is_sentry_plan": plan.tier.tier_name == TierName.SENTRY.value, - } - - NON_PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS = { PlanName.CODECOV_PRO_MONTHLY_LEGACY.value: PlanData( marketing_name=PlanMarketingName.CODECOV_PRO.value, diff --git a/shared/plan/service.py b/shared/plan/service.py index c34d80b83..e68770825 100644 --- a/shared/plan/service.py +++ b/shared/plan/service.py @@ -158,9 +158,11 @@ def tier_name(self) -> TierName: def available_plans(self, owner: Owner) -> List[Plan]: """Returns the available plans for the owner and organization.""" - available_plans = {Plan.objects.get(name=PlanName.BASIC_PLAN_NAME.value)} + available_plans = { + Plan.objects.select_related("tier").get(name=PlanName.BASIC_PLAN_NAME.value) + } - curr_plan = Plan.objects.get(name=self.plan_name) + curr_plan = self.plan_data if not curr_plan.paid_plan: available_plans.add(curr_plan) @@ -177,7 +179,9 @@ def available_plans(self, owner: Owner) -> List[Plan]: available_tiers.append(TierName.TEAM.value) available_plans.update( - Plan.objects.filter(tier__tier_name__in=available_tiers, is_active=True) + Plan.objects.select_related("tier").filter( + tier__tier_name__in=available_tiers, is_active=True + ) ) return [convert_to_DTO(plan) for plan in available_plans] @@ -236,13 +240,11 @@ def start_trial_manually(self, current_owner: Owner, end_date: datetime) -> None """ # Start a new trial plan for free users currently not on trial - plan = Plan.objects.get(name=self.plan_name) - if plan.paid_plan is False: + if self.plan_data.tier.tier_name == TierName.TRIAL.value: + self._start_trial_helper(current_owner, end_date, is_extension=True) + elif self.plan_data.paid_plan is False: self._start_trial_helper(current_owner, end_date, is_extension=False) # Extend an existing trial plan for users currently on trial - elif plan.tier.tier_name == TierName.TRIAL.value: - self._start_trial_helper(current_owner, end_date, is_extension=True) - # Paying users cannot start a trial else: raise ValidationError("Cannot trial from a paid plan") diff --git a/tests/unit/django_apps/codecov_auth/test_codecov_auth_models.py b/tests/unit/django_apps/codecov_auth/test_codecov_auth_models.py index ca29c2da3..8409b7b16 100644 --- a/tests/unit/django_apps/codecov_auth/test_codecov_auth_models.py +++ b/tests/unit/django_apps/codecov_auth/test_codecov_auth_models.py @@ -47,7 +47,6 @@ class TestOwnerModel(TransactionTestCase): def setUp(self): self.owner = OwnerFactory(username="codecov_name", email="name@codecov.io") - mock_all_plans_and_tiers() def test_repo_total_credits_returns_correct_repos_for_legacy_plan(self): self.owner.plan = "5m" @@ -379,6 +378,7 @@ def test_can_activate_user_cannot_activate_account(self): assert not self.owner.can_activate_user(self.owner) def test_fields_that_account_overrides(self): + mock_all_plans_and_tiers() to_activate = OwnerFactory() self.owner.plan = PlanName.BASIC_PLAN_NAME.value self.owner.plan_user_count = 1 @@ -398,12 +398,6 @@ def test_fields_that_account_overrides(self): ENTERPRISE_CLOUD_USER_PLAN_REPRESENTATIONS[self.owner.account.plan] ) account_pretty_plan.update({"quantity": 0}) - print( - "account_pretty_plan", - account_pretty_plan, - "self.owner.pretty_plan", - self.owner.pretty_plan, - ) self.assertEqual(self.owner.pretty_plan, account_pretty_plan) def test_add_admin_adds_ownerid_to_admin_array(self): diff --git a/tests/unit/plan/test_plan.py b/tests/unit/plan/test_plan.py index 2a95512b2..4729ac109 100644 --- a/tests/unit/plan/test_plan.py +++ b/tests/unit/plan/test_plan.py @@ -12,12 +12,7 @@ TierFactory, ) from shared.plan.constants import ( - BASIC_PLAN, - FREE_PLAN, FREE_PLAN_REPRESENTATIONS, - PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS, - SENTRY_PAID_USER_PLAN_REPRESENTATIONS, - TEAM_PLAN_REPRESENTATIONS, TRIAL_PLAN_REPRESENTATION, TRIAL_PLAN_SEATS, PlanName, @@ -31,7 +26,9 @@ @freeze_time("2023-06-19") class PlanServiceTests(TestCase): - def setUp(self): + @classmethod + def setUpClass(cls): + super().setUpClass() mock_all_plans_and_tiers() def test_plan_service_trial_status_not_started(self): @@ -365,6 +362,11 @@ class AvailablePlansBeforeTrial(TestCase): - sentry customer, users-sentrym/y, no trial -> users-pr-inappm/y, users-sentrym/y, users-basic """ + @classmethod + def setUpClass(cls): + super().setUpClass() + mock_all_plans_and_tiers() + def setUp(self): self.current_org = OwnerFactory( trial_start_date=None, @@ -372,7 +374,6 @@ def setUp(self): trial_status=TrialStatus.NOT_STARTED.value, ) self.owner = OwnerFactory() - mock_all_plans_and_tiers() def test_available_plans_for_basic_plan_non_trial( self, @@ -382,13 +383,20 @@ def test_available_plans_for_basic_plan_non_trial( plan_service = PlanService(current_org=self.current_org) - expected_result = [] - expected_result.append(BASIC_PLAN) - expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += TEAM_PLAN_REPRESENTATIONS.values() - expected_result = [result.convert_to_DTO() for result in expected_result] + expected_result = { + PlanName.BASIC_PLAN_NAME.value, + PlanName.CODECOV_PRO_MONTHLY.value, + PlanName.CODECOV_PRO_YEARLY.value, + PlanName.TEAM_MONTHLY.value, + PlanName.TEAM_YEARLY.value, + } - assert plan_service.available_plans(owner=self.owner) == expected_result + assert ( + set( + plan["value"] for plan in plan_service.available_plans(owner=self.owner) + ) + == expected_result + ) def test_available_plans_for_free_plan_non_trial( self, @@ -398,14 +406,21 @@ def test_available_plans_for_free_plan_non_trial( plan_service = PlanService(current_org=self.current_org) - expected_result = [] - expected_result.append(BASIC_PLAN) - expected_result.append(FREE_PLAN) - expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += TEAM_PLAN_REPRESENTATIONS.values() - expected_result = [result.convert_to_DTO() for result in expected_result] + expected_result = { + PlanName.BASIC_PLAN_NAME.value, + PlanName.FREE_PLAN_NAME.value, + PlanName.CODECOV_PRO_MONTHLY.value, + PlanName.CODECOV_PRO_YEARLY.value, + PlanName.TEAM_MONTHLY.value, + PlanName.TEAM_YEARLY.value, + } - assert plan_service.available_plans(owner=self.owner) == expected_result + assert ( + set( + plan["value"] for plan in plan_service.available_plans(owner=self.owner) + ) + == expected_result + ) def test_available_plans_for_team_plan_non_trial( self, @@ -415,13 +430,20 @@ def test_available_plans_for_team_plan_non_trial( plan_service = PlanService(current_org=self.current_org) - expected_result = [] - expected_result.append(BASIC_PLAN) - expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += TEAM_PLAN_REPRESENTATIONS.values() - expected_result = [result.convert_to_DTO() for result in expected_result] + expected_result = { + PlanName.BASIC_PLAN_NAME.value, + PlanName.CODECOV_PRO_MONTHLY.value, + PlanName.CODECOV_PRO_YEARLY.value, + PlanName.TEAM_MONTHLY.value, + PlanName.TEAM_YEARLY.value, + } - assert plan_service.available_plans(owner=self.owner) == expected_result + assert ( + set( + plan["value"] for plan in plan_service.available_plans(owner=self.owner) + ) + == expected_result + ) def test_available_plans_for_pro_plan_non_trial(self): self.current_org.plan = PlanName.CODECOV_PRO_MONTHLY.value @@ -429,13 +451,20 @@ def test_available_plans_for_pro_plan_non_trial(self): plan_service = PlanService(current_org=self.current_org) - expected_result = [] - expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += TEAM_PLAN_REPRESENTATIONS.values() - expected_result.append(BASIC_PLAN) - expected_result = [result.convert_to_DTO() for result in expected_result] + expected_result = { + PlanName.BASIC_PLAN_NAME.value, + PlanName.CODECOV_PRO_MONTHLY.value, + PlanName.CODECOV_PRO_YEARLY.value, + PlanName.TEAM_MONTHLY.value, + PlanName.TEAM_YEARLY.value, + } - assert plan_service.available_plans(owner=self.owner) == expected_result + assert ( + set( + plan["value"] for plan in plan_service.available_plans(owner=self.owner) + ) + == expected_result + ) @patch("shared.plan.service.is_sentry_user") def test_available_plans_for_sentry_customer_basic_plan_non_trial( @@ -447,14 +476,22 @@ def test_available_plans_for_sentry_customer_basic_plan_non_trial( plan_service = PlanService(current_org=self.current_org) - expected_result = [] - expected_result.append(BASIC_PLAN) - expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += TEAM_PLAN_REPRESENTATIONS.values() - expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result = [result.convert_to_DTO() for result in expected_result] + expected_result = { + PlanName.BASIC_PLAN_NAME.value, + PlanName.CODECOV_PRO_MONTHLY.value, + PlanName.CODECOV_PRO_YEARLY.value, + PlanName.SENTRY_MONTHLY.value, + PlanName.SENTRY_YEARLY.value, + PlanName.TEAM_MONTHLY.value, + PlanName.TEAM_YEARLY.value, + } - assert plan_service.available_plans(owner=self.owner) == expected_result + assert ( + set( + plan["value"] for plan in plan_service.available_plans(owner=self.owner) + ) + == expected_result + ) @patch("shared.plan.service.is_sentry_user") def test_available_plans_for_sentry_customer_team_plan_non_trial( @@ -466,14 +503,22 @@ def test_available_plans_for_sentry_customer_team_plan_non_trial( plan_service = PlanService(current_org=self.current_org) - expected_result = [] - expected_result.append(BASIC_PLAN) - expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += TEAM_PLAN_REPRESENTATIONS.values() - expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result = [result.convert_to_DTO() for result in expected_result] + expected_result = { + PlanName.BASIC_PLAN_NAME.value, + PlanName.CODECOV_PRO_MONTHLY.value, + PlanName.CODECOV_PRO_YEARLY.value, + PlanName.SENTRY_MONTHLY.value, + PlanName.SENTRY_YEARLY.value, + PlanName.TEAM_MONTHLY.value, + PlanName.TEAM_YEARLY.value, + } - assert plan_service.available_plans(owner=self.owner) == expected_result + assert ( + set( + plan["value"] for plan in plan_service.available_plans(owner=self.owner) + ) + == expected_result + ) @patch("shared.plan.service.is_sentry_user") def test_available_plans_for_sentry_plan_non_trial(self, is_sentry_user): @@ -483,14 +528,22 @@ def test_available_plans_for_sentry_plan_non_trial(self, is_sentry_user): plan_service = PlanService(current_org=self.current_org) - expected_result = [] - expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += TEAM_PLAN_REPRESENTATIONS.values() - expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result.append(BASIC_PLAN) - expected_result = [result.convert_to_DTO() for result in expected_result] + expected_result = { + PlanName.BASIC_PLAN_NAME.value, + PlanName.CODECOV_PRO_MONTHLY.value, + PlanName.CODECOV_PRO_YEARLY.value, + PlanName.SENTRY_MONTHLY.value, + PlanName.SENTRY_YEARLY.value, + PlanName.TEAM_MONTHLY.value, + PlanName.TEAM_YEARLY.value, + } - assert plan_service.available_plans(owner=self.owner) == expected_result + assert ( + set( + plan["value"] for plan in plan_service.available_plans(owner=self.owner) + ) + == expected_result + ) @freeze_time("2023-06-19") @@ -504,6 +557,11 @@ class AvailablePlansExpiredTrialLessThanTenUsers(TestCase): - sentry customer, users-sentrym/y, has trialed, less than 10 users -> users-pr-inappm/y, users-sentrym/y, users-basic, users-teamm/y """ + @classmethod + def setUpClass(cls): + super().setUpClass() + mock_all_plans_and_tiers() + def setUp(self): self.current_org = OwnerFactory( trial_start_date=datetime.utcnow() + timedelta(days=-10), @@ -512,7 +570,6 @@ def setUp(self): plan_user_count=3, ) self.owner = OwnerFactory() - mock_all_plans_and_tiers() def test_available_plans_for_basic_plan_expired_trial_less_than_10_users( self, @@ -522,13 +579,20 @@ def test_available_plans_for_basic_plan_expired_trial_less_than_10_users( plan_service = PlanService(current_org=self.current_org) - expected_result = [] - expected_result.append(BASIC_PLAN) - expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += TEAM_PLAN_REPRESENTATIONS.values() - expected_result = [result.convert_to_DTO() for result in expected_result] + expected_result = { + PlanName.BASIC_PLAN_NAME.value, + PlanName.CODECOV_PRO_MONTHLY.value, + PlanName.CODECOV_PRO_YEARLY.value, + PlanName.TEAM_MONTHLY.value, + PlanName.TEAM_YEARLY.value, + } - assert plan_service.available_plans(owner=self.owner) == expected_result + assert ( + set( + plan["value"] for plan in plan_service.available_plans(owner=self.owner) + ) + == expected_result + ) def test_available_plans_for_team_plan_expired_trial_less_than_10_users( self, @@ -538,13 +602,20 @@ def test_available_plans_for_team_plan_expired_trial_less_than_10_users( plan_service = PlanService(current_org=self.current_org) - expected_result = [] - expected_result.append(BASIC_PLAN) - expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += TEAM_PLAN_REPRESENTATIONS.values() - expected_result = [result.convert_to_DTO() for result in expected_result] + expected_result = { + PlanName.BASIC_PLAN_NAME.value, + PlanName.CODECOV_PRO_MONTHLY.value, + PlanName.CODECOV_PRO_YEARLY.value, + PlanName.TEAM_MONTHLY.value, + PlanName.TEAM_YEARLY.value, + } - assert plan_service.available_plans(owner=self.owner) == expected_result + assert ( + set( + plan["value"] for plan in plan_service.available_plans(owner=self.owner) + ) + == expected_result + ) def test_available_plans_for_pro_plan_expired_trial_less_than_10_users(self): self.current_org.plan = PlanName.CODECOV_PRO_MONTHLY.value @@ -552,13 +623,20 @@ def test_available_plans_for_pro_plan_expired_trial_less_than_10_users(self): plan_service = PlanService(current_org=self.current_org) - expected_result = [] - expected_result.append(BASIC_PLAN) - expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += TEAM_PLAN_REPRESENTATIONS.values() - expected_result = [result.convert_to_DTO() for result in expected_result] + expected_result = { + PlanName.BASIC_PLAN_NAME.value, + PlanName.CODECOV_PRO_MONTHLY.value, + PlanName.CODECOV_PRO_YEARLY.value, + PlanName.TEAM_MONTHLY.value, + PlanName.TEAM_YEARLY.value, + } - assert plan_service.available_plans(owner=self.owner) == expected_result + assert ( + set( + plan["value"] for plan in plan_service.available_plans(owner=self.owner) + ) + == expected_result + ) @patch("shared.plan.service.is_sentry_user") def test_available_plans_for_sentry_customer_basic_plan_expired_trial_less_than_10_users( @@ -570,14 +648,22 @@ def test_available_plans_for_sentry_customer_basic_plan_expired_trial_less_than_ plan_service = PlanService(current_org=self.current_org) - expected_result = [] - expected_result.append(BASIC_PLAN) - expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += TEAM_PLAN_REPRESENTATIONS.values() - expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result = [result.convert_to_DTO() for result in expected_result] + expected_result = { + PlanName.BASIC_PLAN_NAME.value, + PlanName.CODECOV_PRO_MONTHLY.value, + PlanName.CODECOV_PRO_YEARLY.value, + PlanName.SENTRY_MONTHLY.value, + PlanName.SENTRY_YEARLY.value, + PlanName.TEAM_MONTHLY.value, + PlanName.TEAM_YEARLY.value, + } - assert plan_service.available_plans(owner=self.owner) == expected_result + assert ( + set( + plan["value"] for plan in plan_service.available_plans(owner=self.owner) + ) + == expected_result + ) @patch("shared.plan.service.is_sentry_user") def test_available_plans_for_sentry_customer_team_plan_expired_trial_less_than_10_users( @@ -589,14 +675,22 @@ def test_available_plans_for_sentry_customer_team_plan_expired_trial_less_than_1 plan_service = PlanService(current_org=self.current_org) - expected_result = [] - expected_result.append(BASIC_PLAN) - expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += TEAM_PLAN_REPRESENTATIONS.values() - expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result = [result.convert_to_DTO() for result in expected_result] + expected_result = { + PlanName.BASIC_PLAN_NAME.value, + PlanName.CODECOV_PRO_MONTHLY.value, + PlanName.CODECOV_PRO_YEARLY.value, + PlanName.SENTRY_MONTHLY.value, + PlanName.SENTRY_YEARLY.value, + PlanName.TEAM_MONTHLY.value, + PlanName.TEAM_YEARLY.value, + } - assert plan_service.available_plans(owner=self.owner) == expected_result + assert ( + set( + plan["value"] for plan in plan_service.available_plans(owner=self.owner) + ) + == expected_result + ) @patch("shared.plan.service.is_sentry_user") def test_available_plans_for_sentry_plan_expired_trial_less_than_10_users( @@ -608,14 +702,22 @@ def test_available_plans_for_sentry_plan_expired_trial_less_than_10_users( plan_service = PlanService(current_org=self.current_org) - expected_result = [] - expected_result.append(BASIC_PLAN) - expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += TEAM_PLAN_REPRESENTATIONS.values() - expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result = [result.convert_to_DTO() for result in expected_result] + expected_result = { + PlanName.BASIC_PLAN_NAME.value, + PlanName.CODECOV_PRO_MONTHLY.value, + PlanName.CODECOV_PRO_YEARLY.value, + PlanName.SENTRY_MONTHLY.value, + PlanName.SENTRY_YEARLY.value, + PlanName.TEAM_MONTHLY.value, + PlanName.TEAM_YEARLY.value, + } - assert plan_service.available_plans(owner=self.owner) == expected_result + assert ( + set( + plan["value"] for plan in plan_service.available_plans(owner=self.owner) + ) + == expected_result + ) @freeze_time("2023-06-19") @@ -626,6 +728,11 @@ class AvailablePlansExpiredTrialMoreThanTenActivatedUsers(TestCase): - sentry customer, users-sentrym/y, has trialed, more than 10 activated users -> users-pr-inappm/y, users-sentrym/y, users-basic """ + @classmethod + def setUpClass(cls): + super().setUpClass() + mock_all_plans_and_tiers() + def setUp(self): self.current_org = OwnerFactory( trial_start_date=datetime.utcnow() + timedelta(days=-10), @@ -635,7 +742,6 @@ def setUp(self): plan_activated_users=[i for i in range(13)], ) self.owner = OwnerFactory() - mock_all_plans_and_tiers() def test_available_plans_for_pro_plan_expired_trial_more_than_10_users(self): self.current_org.plan = PlanName.CODECOV_PRO_MONTHLY.value @@ -643,12 +749,18 @@ def test_available_plans_for_pro_plan_expired_trial_more_than_10_users(self): plan_service = PlanService(current_org=self.current_org) - expected_result = [] - expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result.append(BASIC_PLAN) - expected_result = [result.convert_to_DTO() for result in expected_result] + expected_result = { + PlanName.BASIC_PLAN_NAME.value, + PlanName.CODECOV_PRO_MONTHLY.value, + PlanName.CODECOV_PRO_YEARLY.value, + } - assert plan_service.available_plans(owner=self.owner) == expected_result + assert ( + set( + plan["value"] for plan in plan_service.available_plans(owner=self.owner) + ) + == expected_result + ) @patch("shared.plan.service.is_sentry_user") def test_available_plans_for_sentry_customer_basic_plan_expired_trial_more_than_10_users( @@ -660,13 +772,20 @@ def test_available_plans_for_sentry_customer_basic_plan_expired_trial_more_than_ plan_service = PlanService(current_org=self.current_org) - expected_result = [] - expected_result.append(BASIC_PLAN) - expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result = [result.convert_to_DTO() for result in expected_result] + expected_result = { + PlanName.BASIC_PLAN_NAME.value, + PlanName.CODECOV_PRO_MONTHLY.value, + PlanName.CODECOV_PRO_YEARLY.value, + PlanName.SENTRY_MONTHLY.value, + PlanName.SENTRY_YEARLY.value, + } - assert plan_service.available_plans(owner=self.owner) == expected_result + assert ( + set( + plan["value"] for plan in plan_service.available_plans(owner=self.owner) + ) + == expected_result + ) @patch("shared.plan.service.is_sentry_user") def test_available_plans_for_sentry_plan_expired_trial_more_than_10_users( @@ -678,13 +797,20 @@ def test_available_plans_for_sentry_plan_expired_trial_more_than_10_users( plan_service = PlanService(current_org=self.current_org) - expected_result = [] - expected_result.append(BASIC_PLAN) - expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result = [result.convert_to_DTO() for result in expected_result] + expected_result = { + PlanName.BASIC_PLAN_NAME.value, + PlanName.CODECOV_PRO_MONTHLY.value, + PlanName.CODECOV_PRO_YEARLY.value, + PlanName.SENTRY_MONTHLY.value, + PlanName.SENTRY_YEARLY.value, + } - assert plan_service.available_plans(owner=self.owner) == expected_result + assert ( + set( + plan["value"] for plan in plan_service.available_plans(owner=self.owner) + ) + == expected_result + ) @freeze_time("2023-06-19") @@ -693,16 +819,20 @@ class AvailablePlansExpiredTrialMoreThanTenSeatsLessThanTenActivatedUsers(TestCa Tests that what matters for Team plan is activated users not the total seat count """ - def setUp(self): - self.expected_result = [] - self.expected_result.append(BASIC_PLAN) - self.expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - self.expected_result += TEAM_PLAN_REPRESENTATIONS.values() - self.expected_result = [ - result.convert_to_DTO() for result in self.expected_result - ] + @classmethod + def setUpClass(cls): + super().setUpClass() mock_all_plans_and_tiers() + def setUp(self): + self.expected_result = { + PlanName.BASIC_PLAN_NAME.value, + PlanName.CODECOV_PRO_MONTHLY.value, + PlanName.CODECOV_PRO_YEARLY.value, + PlanName.TEAM_MONTHLY.value, + PlanName.TEAM_YEARLY.value, + } + def test_currently_team_plan(self): self.current_org = OwnerFactory( plan_user_count=100, @@ -713,7 +843,11 @@ def test_currently_team_plan(self): self.plan_service = PlanService(current_org=self.current_org) assert ( - self.plan_service.available_plans(owner=self.owner) == self.expected_result + set( + plan["value"] + for plan in self.plan_service.available_plans(owner=self.owner) + ) + == self.expected_result ) def test_trial_expired(self): @@ -728,7 +862,11 @@ def test_trial_expired(self): self.plan_service = PlanService(current_org=self.current_org) assert ( - self.plan_service.available_plans(owner=self.owner) == self.expected_result + set( + plan["value"] + for plan in self.plan_service.available_plans(owner=self.owner) + ) + == self.expected_result ) def test_trial_ongoing(self): @@ -743,7 +881,11 @@ def test_trial_ongoing(self): self.plan_service = PlanService(current_org=self.current_org) assert ( - self.plan_service.available_plans(owner=self.owner) == self.expected_result + set( + plan["value"] + for plan in self.plan_service.available_plans(owner=self.owner) + ) + == self.expected_result ) def test_trial_not_started(self): @@ -755,16 +897,20 @@ def test_trial_not_started(self): self.owner = OwnerFactory() self.plan_service = PlanService(current_org=self.current_org) - self.expected_result = [] - self.expected_result.append(BASIC_PLAN) - self.expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - self.expected_result += TEAM_PLAN_REPRESENTATIONS.values() - self.expected_result = [ - result.convert_to_DTO() for result in self.expected_result - ] + self.expected_result = { + PlanName.BASIC_PLAN_NAME.value, + PlanName.CODECOV_PRO_MONTHLY.value, + PlanName.CODECOV_PRO_YEARLY.value, + PlanName.TEAM_MONTHLY.value, + PlanName.TEAM_YEARLY.value, + } assert ( - self.plan_service.available_plans(owner=self.owner) == self.expected_result + set( + plan["value"] + for plan in self.plan_service.available_plans(owner=self.owner) + ) + == self.expected_result ) @@ -779,8 +925,12 @@ class AvailablePlansOngoingTrial(TestCase): when > 10 activated seats -> users-pr-inappm/y, users-sentrym/y, users-basic """ - def setUp(self): + @classmethod + def setUpClass(cls): + super().setUpClass() mock_all_plans_and_tiers() + + def setUp(self): self.current_org = OwnerFactory( plan=PlanName.BASIC_PLAN_NAME.value, trial_start_date=datetime.utcnow(), @@ -794,32 +944,53 @@ def setUp(self): def test_non_sentry_user(self): # [Basic, Pro Monthly, Pro Yearly, Team Monthly, Team Yearly] - expected_result = [] - expected_result.append(BASIC_PLAN) - expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += TEAM_PLAN_REPRESENTATIONS.values() - expected_result = [result.convert_to_DTO() for result in expected_result] + expected_result = { + PlanName.BASIC_PLAN_NAME.value, + PlanName.CODECOV_PRO_YEARLY.value, + PlanName.CODECOV_PRO_MONTHLY.value, + PlanName.TEAM_MONTHLY.value, + PlanName.TEAM_YEARLY.value, + } # Can do Team plan when plan_activated_users is null - assert self.plan_service.available_plans(owner=self.owner) == expected_result + assert ( + set( + plan["value"] + for plan in self.plan_service.available_plans(owner=self.owner) + ) + == expected_result + ) self.current_org.plan_activated_users = [i for i in range(10)] self.current_org.save() # Can do Team plan when at 10 activated users - assert self.plan_service.available_plans(owner=self.owner) == expected_result + assert ( + set( + plan["value"] + for plan in self.plan_service.available_plans(owner=self.owner) + ) + == expected_result + ) self.current_org.plan_activated_users = [i for i in range(11)] self.current_org.save() - # [Basic, Pro Monthly, Pro Yearly, Team Monthly, Team Yearly] - expected_result = [] - expected_result.append(BASIC_PLAN) - expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result = [result.convert_to_DTO() for result in expected_result] + # [Basic, Pro Monthly, Pro Yearly] + expected_result = { + PlanName.BASIC_PLAN_NAME.value, + PlanName.CODECOV_PRO_YEARLY.value, + PlanName.CODECOV_PRO_MONTHLY.value, + } # Can not do Team plan when at 11 activated users - assert self.plan_service.available_plans(owner=self.owner) == expected_result + assert ( + set( + plan["value"] + for plan in self.plan_service.available_plans(owner=self.owner) + ) + == expected_result + ) @patch("shared.plan.service.is_sentry_user") def test_sentry_user(self, is_sentry_user): @@ -829,34 +1000,56 @@ def test_sentry_user(self, is_sentry_user): is_sentry_user.return_value = True # [Basic, Pro Monthly, Pro Yearly, Sentry Monthly, Sentry Yearly, Team Monthly, Team Yearly] - expected_result = [] - expected_result.append(BASIC_PLAN) - expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += TEAM_PLAN_REPRESENTATIONS.values() - expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result = [result.convert_to_DTO() for result in expected_result] - + expected_result = { + PlanName.BASIC_PLAN_NAME.value, + PlanName.CODECOV_PRO_YEARLY.value, + PlanName.CODECOV_PRO_MONTHLY.value, + PlanName.SENTRY_MONTHLY.value, + PlanName.SENTRY_YEARLY.value, + PlanName.TEAM_MONTHLY.value, + PlanName.TEAM_YEARLY.value, + } # Can do Team plan when plan_activated_users is null - assert self.plan_service.available_plans(owner=self.owner) == expected_result + assert ( + set( + plan["value"] + for plan in self.plan_service.available_plans(owner=self.owner) + ) + == expected_result + ) self.current_org.plan_activated_users = [i for i in range(10)] self.current_org.save() # Can do Team plan when at 10 activated users - assert self.plan_service.available_plans(owner=self.owner) == expected_result + assert ( + set( + plan["value"] + for plan in self.plan_service.available_plans(owner=self.owner) + ) + == expected_result + ) self.current_org.plan_activated_users = [i for i in range(11)] self.current_org.save() # [Basic, Pro Monthly, Pro Yearly, Sentry Monthly, Sentry Yearly] - expected_result = [] - expected_result.append(BASIC_PLAN) - expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() - expected_result = [result.convert_to_DTO() for result in expected_result] + expected_result = { + PlanName.BASIC_PLAN_NAME.value, + PlanName.CODECOV_PRO_YEARLY.value, + PlanName.CODECOV_PRO_MONTHLY.value, + PlanName.SENTRY_MONTHLY.value, + PlanName.SENTRY_YEARLY.value, + } # Can not do Team plan when at 11 activated users - assert self.plan_service.available_plans(owner=self.owner) == expected_result + assert ( + set( + plan["value"] + for plan in self.plan_service.available_plans(owner=self.owner) + ) + == expected_result + ) @override_settings(IS_ENTERPRISE=False) diff --git a/tests/unit/upload/test_utils.py b/tests/unit/upload/test_utils.py index 9175bd9a4..b5f97ccb4 100644 --- a/tests/unit/upload/test_utils.py +++ b/tests/unit/upload/test_utils.py @@ -22,7 +22,9 @@ class CoverageMeasurement(TestCase): - def setUp(self): + @classmethod + def setUpClass(cls): + super().setUpClass() mock_all_plans_and_tiers() def add_upload_measurements_records( From 8277f6d8879a05c6506e103786cab67e324dc1d3 Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Thu, 23 Jan 2025 11:53:04 -0800 Subject: [PATCH 15/18] remove ttestcase if not needed --- .../codecov_auth/test_codecov_auth_models.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/unit/django_apps/codecov_auth/test_codecov_auth_models.py b/tests/unit/django_apps/codecov_auth/test_codecov_auth_models.py index 8409b7b16..57ffd20ac 100644 --- a/tests/unit/django_apps/codecov_auth/test_codecov_auth_models.py +++ b/tests/unit/django_apps/codecov_auth/test_codecov_auth_models.py @@ -5,7 +5,7 @@ import pytest from django.db import IntegrityError from django.forms import ValidationError -from django.test import TransactionTestCase +from django.test import TestCase, TransactionTestCase from pytest import LogCaptureFixture from shared.django_apps.codecov_auth.models import ( @@ -44,7 +44,7 @@ from tests.helper import mock_all_plans_and_tiers -class TestOwnerModel(TransactionTestCase): +class TestOwnerModel(TestCase): def setUp(self): self.owner = OwnerFactory(username="codecov_name", email="name@codecov.io") @@ -525,7 +525,7 @@ def test_has_yaml(self): assert org.has_yaml is True -class TestOrganizationLevelTokenModel(TransactionTestCase): +class TestOrganizationLevelTokenModel(TestCase): def test_can_save_org_token_for_org_basic_plan(self): owner = OwnerFactory(plan="users-basic") owner.save() @@ -550,7 +550,7 @@ def test_token_is_deleted_when_changing_user_plan( assert OrganizationLevelToken.objects.filter(owner=owner).count() == 0 -class TestGithubAppInstallationModel(TransactionTestCase): +class TestGithubAppInstallationModel(TestCase): DEFAULT_APP_ID = 12345 @pytest.fixture(autouse=True) @@ -675,7 +675,7 @@ def test_is_configured(self): ) -class TestGitHubAppInstallationNoDefaultAppIdConfig(TransactionTestCase): +class TestGitHubAppInstallationNoDefaultAppIdConfig(TestCase): @pytest.fixture(autouse=True) def mock_no_default_app_id(self, mocker): mock_config_helper(mocker, configs={"github.integration.id": None}) From 5c8398a784f5ad1d7e0df34148a449959c1c5f9b Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Thu, 23 Jan 2025 14:35:23 -0800 Subject: [PATCH 16/18] print --- shared/django_apps/codecov_auth/models.py | 1 - 1 file changed, 1 deletion(-) diff --git a/shared/django_apps/codecov_auth/models.py b/shared/django_apps/codecov_auth/models.py index 7b12e180c..f32afbb1f 100644 --- a/shared/django_apps/codecov_auth/models.py +++ b/shared/django_apps/codecov_auth/models.py @@ -605,7 +605,6 @@ def pretty_plan(self): if self.account: return self.account.pretty_plan - print("@@@@@@@@@@@@@@@@@@@@", self.plan) plan_details = Plan.objects.select_related("tier").get(name=self.plan) if plan_details: return { From 7804571e81d9195cab11307e02972cfdb5fe3eea Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Fri, 24 Jan 2025 10:46:07 -0800 Subject: [PATCH 17/18] use cached property, remove setter --- shared/plan/service.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/shared/plan/service.py b/shared/plan/service.py index e68770825..76421036a 100644 --- a/shared/plan/service.py +++ b/shared/plan/service.py @@ -1,5 +1,6 @@ import logging from datetime import datetime, timedelta +from functools import cached_property from typing import List, Optional from shared.billing import is_pr_billing_plan @@ -81,7 +82,7 @@ def has_account(self) -> bool: """Returns whether the organization has an associated account.""" return self.current_org.account is not None - @property + @cached_property def plan_data(self) -> Plan: """Returns the plan data for the organization, either from account or default.""" if self._plan_data is None: @@ -92,11 +93,6 @@ def plan_data(self) -> Plan: ) return self._plan_data - @plan_data.setter - def plan_data(self, plan_data: Optional[Plan]) -> None: - """Sets the plan data directly.""" - self._plan_data = plan_data - @property def plan_name(self) -> str: """Returns the name of the organization's current plan.""" From 6e1418a0b1609eee4845dacae44cbfa2a75a8d13 Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Mon, 27 Jan 2025 13:50:09 -0800 Subject: [PATCH 18/18] missed a couple prefetch references --- shared/plan/service.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shared/plan/service.py b/shared/plan/service.py index 76421036a..d0f412501 100644 --- a/shared/plan/service.py +++ b/shared/plan/service.py @@ -61,7 +61,7 @@ def update_plan(self, name: str, user_count: Optional[int]) -> None: raise ValueError("Quantity Needed") self.current_org.plan = name self.current_org.plan_user_count = user_count - self._plan_data = Plan.objects.get(name=name) + self._plan_data = Plan.objects.select_related("tier").get(name=name) self.current_org.delinquent = False self.current_org.save() @@ -86,7 +86,7 @@ def has_account(self) -> bool: def plan_data(self) -> Plan: """Returns the plan data for the organization, either from account or default.""" if self._plan_data is None: - self._plan_data = Plan.objects.get( + self._plan_data = Plan.objects.select_related("tier").get( name=self.current_org.account.plan if self.has_account else self.current_org.plan