From 947954a837a37be667595f68b899c091d9b5bf52 Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Wed, 15 Jan 2025 09:40:34 -0800 Subject: [PATCH 01/19] starting transition, need the other PR merged to do imports and test updates --- api/internal/owner/serializers.py | 11 ++++------- billing/helpers.py | 7 +++++-- codecov_auth/admin.py | 3 +-- upload/helpers.py | 6 ++++-- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/api/internal/owner/serializers.py b/api/internal/owner/serializers.py index 7672f61d64..6eda78917a 100644 --- a/api/internal/owner/serializers.py +++ b/api/internal/owner/serializers.py @@ -8,9 +8,9 @@ from rest_framework.exceptions import PermissionDenied from shared.plan.constants import ( PAID_PLANS, - SENTRY_PAID_USER_PLAN_REPRESENTATIONS, TEAM_PLAN_MAX_USERS, TEAM_PLAN_REPRESENTATIONS, + TierName, ) from shared.plan.service import PlanService @@ -137,11 +137,6 @@ def validate_value(self, value: str) -> str: plan["value"] for plan in plan_service.available_plans(current_owner) ] if value not in plan_values: - if value in SENTRY_PAID_USER_PLAN_REPRESENTATIONS: - log.warning( - "Non-Sentry user attempted to transition to Sentry plan", - extra=dict(owner_id=current_owner.pk, plan=value), - ) raise serializers.ValidationError( f"Invalid value for plan: {value}; must be one of {plan_values}" ) @@ -342,7 +337,9 @@ def update(self, instance: Owner, validated_data: Dict[str, Any]) -> object: instance, desired_plan ) - if desired_plan["value"] in SENTRY_PAID_USER_PLAN_REPRESENTATIONS: + sentry_plans = Plan.objects.filter(tier=TierName.Sentry.value) + + if desired_plan["value"] in sentry_plans: current_owner = self.context["view"].request.current_owner send_sentry_webhook(current_owner, instance) diff --git a/billing/helpers.py b/billing/helpers.py index c66e6333cb..7d6c6af2ae 100644 --- a/billing/helpers.py +++ b/billing/helpers.py @@ -1,13 +1,16 @@ from django.conf import settings from django.db.models import QuerySet -from shared.plan.constants import ENTERPRISE_CLOUD_USER_PLAN_REPRESENTATIONS +from shared.plan.constants import TierName from codecov_auth.models import Owner def on_enterprise_plan(owner: Owner) -> bool: return settings.IS_ENTERPRISE or ( - owner.plan in ENTERPRISE_CLOUD_USER_PLAN_REPRESENTATIONS.keys() + owner.plan + in Plan.objects.filter(tier=TierName.ENTERPRISE.value).values_list( + "name", flat=True + ) ) diff --git a/codecov_auth/admin.py b/codecov_auth/admin.py index f07cebceb0..e61af46efa 100644 --- a/codecov_auth/admin.py +++ b/codecov_auth/admin.py @@ -21,7 +21,6 @@ StripeBilling, Tier, ) -from shared.plan.constants import USER_PLAN_REPRESENTATIONS from shared.plan.service import PlanService from codecov.admin import AdminMixin @@ -609,7 +608,7 @@ class OwnerAdmin(AdminMixin, admin.ModelAdmin): def get_form(self, request, obj=None, change=False, **kwargs): form = super().get_form(request, obj, change, **kwargs) - PLANS_CHOICES = [(x, x) for x in USER_PLAN_REPRESENTATIONS.keys()] + PLANS_CHOICES = [(x, x) for x in Plan.objects.values_list("name", flat=True)] form.base_fields["plan"].widget = Select( choices=BLANK_CHOICE_DASH + PLANS_CHOICES ) diff --git a/upload/helpers.py b/upload/helpers.py index bccd92e034..23379e3ee8 100644 --- a/upload/helpers.py +++ b/upload/helpers.py @@ -15,7 +15,6 @@ from redis import Redis from rest_framework.exceptions import NotFound, Throttled, ValidationError from shared.github import InvalidInstallationError -from shared.plan.constants import USER_PLAN_REPRESENTATIONS from shared.plan.service import PlanService from shared.reports.enums import UploadType from shared.torngit.base import TorngitBaseAdapter @@ -637,7 +636,10 @@ def validate_upload( owner = _determine_responsible_owner(repository) # If author is on per repo billing, check their repo credits - if owner.plan not in USER_PLAN_REPRESENTATIONS and owner.repo_credits <= 0: + if ( + owner.plan not in Plan.object.values_list("name", flat=True) + and owner.repo_credits <= 0 + ): raise ValidationError( "Sorry, but this team has no private repository credits left." ) From a06b8c83229cfb545ba3f0fbbbb161a7ba8f5a66 Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Wed, 15 Jan 2025 11:39:57 -0800 Subject: [PATCH 02/19] other api stuff --- .../commands/set_trial_status_values.py | 22 +++++++++---------- .../services/org_level_token_service.py | 3 +-- services/billing.py | 9 ++++---- 3 files changed, 16 insertions(+), 18 deletions(-) diff --git a/codecov_auth/management/commands/set_trial_status_values.py b/codecov_auth/management/commands/set_trial_status_values.py index 7872bbce57..0afa3b0db9 100644 --- a/codecov_auth/management/commands/set_trial_status_values.py +++ b/codecov_auth/management/commands/set_trial_status_values.py @@ -2,13 +2,11 @@ from typing import Any from django.core.management.base import BaseCommand, CommandParser -from django.db.models import Q +from django.db.models import Q, Subquery from shared.plan.constants import ( FREE_PLAN_REPRESENTATIONS, PLANS_THAT_CAN_TRIAL, - PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS, - SENTRY_PAID_USER_PLAN_REPRESENTATIONS, - PlanName, + TierName, TrialStatus, ) @@ -32,10 +30,12 @@ def handle(self, *args: Any, **options: Any) -> None: stripe_customer_id=None, ).update(trial_status=TrialStatus.NOT_STARTED.value) + sentry_plans = Plan.objects.filter(tier=TierName.SENTRY.value) + pro_plans = Plan.objects.filter(tier=TierName.PRO.value, paid_plan=True) # ONGOING if trial_status_type == "all" or trial_status_type == "ongoing": Owner.objects.filter( - plan__in=SENTRY_PAID_USER_PLAN_REPRESENTATIONS, + plan__in=Subquery(sentry_plans.values_list("name", flat=True)), trial_end_date__gt=datetime.now(), ).update(trial_status=TrialStatus.ONGOING.value) @@ -44,14 +44,14 @@ def handle(self, *args: Any, **options: Any) -> None: Owner.objects.filter( # Currently paying sentry customer with trial_end_date Q( - plan__in=SENTRY_PAID_USER_PLAN_REPRESENTATIONS, + plan__in=Subquery(sentry_plans.values_list("name", flat=True)), stripe_customer_id__isnull=False, stripe_subscription_id__isnull=False, trial_end_date__lte=datetime.now(), ) # Currently paying sentry customer without trial_end_date | Q( - plan__in=SENTRY_PAID_USER_PLAN_REPRESENTATIONS, + plan__in=Subquery(sentry_plans.values_list("name", flat=True)), stripe_customer_id__isnull=False, stripe_subscription_id__isnull=False, trial_start_date__isnull=True, @@ -59,7 +59,7 @@ def handle(self, *args: Any, **options: Any) -> None: ) # Previously paid but now back to basic with trial start/end dates | Q( - plan=PlanName.BASIC_PLAN_NAME.value, + plan="users-basic", stripe_customer_id__isnull=False, trial_start_date__isnull=False, trial_end_date__isnull=False, @@ -73,20 +73,20 @@ def handle(self, *args: Any, **options: Any) -> None: ~Q(plan__in=PLANS_THAT_CAN_TRIAL) # Previously paid but now back to basic without trial start/end dates | Q( - plan=PlanName.BASIC_PLAN_NAME.value, + plan="users-basic", stripe_customer_id__isnull=False, trial_start_date__isnull=True, trial_end_date__isnull=True, ) # Currently paying customer that isn't a sentry plan (they would be expired) | Q( - ~Q(plan__in=SENTRY_PAID_USER_PLAN_REPRESENTATIONS), + ~Q(plan__in=Subquery(sentry_plans.values_list("name", flat=True))), stripe_subscription_id__isnull=False, stripe_customer_id__isnull=False, ) # Invoiced customers without stripe info | Q( - Q(plan__in=PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS), + Q(plan__in=Subquery(pro_plans.values_list("name", flat=True))), stripe_subscription_id__isnull=True, stripe_customer_id__isnull=True, ) diff --git a/codecov_auth/services/org_level_token_service.py b/codecov_auth/services/org_level_token_service.py index 9cba8b48dc..d241fa8989 100644 --- a/codecov_auth/services/org_level_token_service.py +++ b/codecov_auth/services/org_level_token_service.py @@ -4,7 +4,6 @@ from django.db.models.signals import post_save from django.dispatch import receiver from django.forms import ValidationError -from shared.plan.constants import USER_PLAN_REPRESENTATIONS from codecov_auth.models import OrganizationLevelToken, Owner @@ -20,7 +19,7 @@ class OrgLevelTokenService(object): @classmethod def org_can_have_upload_token(cls, org: Owner): - return org.plan in USER_PLAN_REPRESENTATIONS + return org.plan in Plan.objects.values_list("name", flat=True) @classmethod def get_or_create_org_token(cls, org: Owner): diff --git a/services/billing.py b/services/billing.py index 825cd7ccab..77c2a53c0c 100644 --- a/services/billing.py +++ b/services/billing.py @@ -10,7 +10,6 @@ FREE_PLAN_REPRESENTATIONS, PAID_PLANS, TEAM_PLANS, - USER_PLAN_REPRESENTATIONS, PlanBillingRate, ) from shared.plan.service import PlanService @@ -454,8 +453,8 @@ def _is_extending_term(self, owner: Owner, desired_plan: dict) -> bool: """ Returns `True` if switching from monthly to yearly plan. """ - current_plan_info = USER_PLAN_REPRESENTATIONS.get(owner.plan) - desired_plan_info = USER_PLAN_REPRESENTATIONS.get(desired_plan["value"]) + current_plan_info = Plan.objects.get(name=owner.plan) + desired_plan_info = Plan.objects.get(name=desired_plan["value"]) return bool( current_plan_info @@ -468,8 +467,8 @@ def _is_similar_plan(self, owner: Owner, desired_plan: dict) -> bool: """ Returns `True` if switching to a plan with similar term and seats. """ - current_plan_info = USER_PLAN_REPRESENTATIONS.get(owner.plan) - desired_plan_info = USER_PLAN_REPRESENTATIONS.get(desired_plan["value"]) + current_plan_info = Plan.objects.get(name=owner.plan) + desired_plan_info = Plan.objects.get(name=desired_plan["value"]) is_same_term = ( current_plan_info From 1f2e63b84f6b80c62205c677dd47a58ea6e07028 Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Wed, 15 Jan 2025 12:02:43 -0800 Subject: [PATCH 03/19] revert trial status changes, update other instances of plan stuff --- api/internal/owner/serializers.py | 4 ++-- .../commands/set_trial_status_values.py | 22 +++++++++---------- graphql_api/types/owner/owner.py | 4 ++-- services/billing.py | 5 +++-- 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/api/internal/owner/serializers.py b/api/internal/owner/serializers.py index 6eda78917a..54193446c0 100644 --- a/api/internal/owner/serializers.py +++ b/api/internal/owner/serializers.py @@ -9,7 +9,6 @@ from shared.plan.constants import ( PAID_PLANS, TEAM_PLAN_MAX_USERS, - TEAM_PLAN_REPRESENTATIONS, TierName, ) from shared.plan.service import PlanService @@ -179,7 +178,8 @@ def validate(self, plan: Dict[str, Any]) -> Dict[str, Any]: "Quantity or plan for paid plan must be different from the existing one" ) if ( - plan["value"] in TEAM_PLAN_REPRESENTATIONS + plan["value"] + in Plan.objects.filter(tier=TierName.TEAM.value, is_active=True) and plan["quantity"] > TEAM_PLAN_MAX_USERS ): raise serializers.ValidationError( diff --git a/codecov_auth/management/commands/set_trial_status_values.py b/codecov_auth/management/commands/set_trial_status_values.py index 0afa3b0db9..7872bbce57 100644 --- a/codecov_auth/management/commands/set_trial_status_values.py +++ b/codecov_auth/management/commands/set_trial_status_values.py @@ -2,11 +2,13 @@ from typing import Any from django.core.management.base import BaseCommand, CommandParser -from django.db.models import Q, Subquery +from django.db.models import Q from shared.plan.constants import ( FREE_PLAN_REPRESENTATIONS, PLANS_THAT_CAN_TRIAL, - TierName, + PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS, + SENTRY_PAID_USER_PLAN_REPRESENTATIONS, + PlanName, TrialStatus, ) @@ -30,12 +32,10 @@ def handle(self, *args: Any, **options: Any) -> None: stripe_customer_id=None, ).update(trial_status=TrialStatus.NOT_STARTED.value) - sentry_plans = Plan.objects.filter(tier=TierName.SENTRY.value) - pro_plans = Plan.objects.filter(tier=TierName.PRO.value, paid_plan=True) # ONGOING if trial_status_type == "all" or trial_status_type == "ongoing": Owner.objects.filter( - plan__in=Subquery(sentry_plans.values_list("name", flat=True)), + plan__in=SENTRY_PAID_USER_PLAN_REPRESENTATIONS, trial_end_date__gt=datetime.now(), ).update(trial_status=TrialStatus.ONGOING.value) @@ -44,14 +44,14 @@ def handle(self, *args: Any, **options: Any) -> None: Owner.objects.filter( # Currently paying sentry customer with trial_end_date Q( - plan__in=Subquery(sentry_plans.values_list("name", flat=True)), + plan__in=SENTRY_PAID_USER_PLAN_REPRESENTATIONS, stripe_customer_id__isnull=False, stripe_subscription_id__isnull=False, trial_end_date__lte=datetime.now(), ) # Currently paying sentry customer without trial_end_date | Q( - plan__in=Subquery(sentry_plans.values_list("name", flat=True)), + plan__in=SENTRY_PAID_USER_PLAN_REPRESENTATIONS, stripe_customer_id__isnull=False, stripe_subscription_id__isnull=False, trial_start_date__isnull=True, @@ -59,7 +59,7 @@ def handle(self, *args: Any, **options: Any) -> None: ) # Previously paid but now back to basic with trial start/end dates | Q( - plan="users-basic", + plan=PlanName.BASIC_PLAN_NAME.value, stripe_customer_id__isnull=False, trial_start_date__isnull=False, trial_end_date__isnull=False, @@ -73,20 +73,20 @@ def handle(self, *args: Any, **options: Any) -> None: ~Q(plan__in=PLANS_THAT_CAN_TRIAL) # Previously paid but now back to basic without trial start/end dates | Q( - plan="users-basic", + plan=PlanName.BASIC_PLAN_NAME.value, stripe_customer_id__isnull=False, trial_start_date__isnull=True, trial_end_date__isnull=True, ) # Currently paying customer that isn't a sentry plan (they would be expired) | Q( - ~Q(plan__in=Subquery(sentry_plans.values_list("name", flat=True))), + ~Q(plan__in=SENTRY_PAID_USER_PLAN_REPRESENTATIONS), stripe_subscription_id__isnull=False, stripe_customer_id__isnull=False, ) # Invoiced customers without stripe info | Q( - Q(plan__in=Subquery(pro_plans.values_list("name", flat=True))), + Q(plan__in=PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS), stripe_subscription_id__isnull=True, stripe_customer_id__isnull=True, ) diff --git a/graphql_api/types/owner/owner.py b/graphql_api/types/owner/owner.py index a268472156..775a1ad710 100644 --- a/graphql_api/types/owner/owner.py +++ b/graphql_api/types/owner/owner.py @@ -8,7 +8,7 @@ from ariadne import ObjectType from django.conf import settings from graphql import GraphQLResolveInfo -from shared.plan.constants import FREE_PLAN_REPRESENTATIONS, PlanData, PlanName +from shared.plan.constants import PlanData, PlanName from shared.plan.service import PlanService import services.activation as activation @@ -112,7 +112,7 @@ def resolve_plan(owner: Owner, info: GraphQLResolveInfo) -> PlanService: @require_part_of_org def resolve_plan_representation(owner: Owner, info: GraphQLResolveInfo) -> PlanData: info.context["plan_service"] = PlanService(current_org=owner) - free_plan = FREE_PLAN_REPRESENTATIONS[PlanName.BASIC_PLAN_NAME.value] + free_plan = Plan.objects.get(name=PlanName.BASIC_PLAN_NAME.value) return free_plan.convert_to_DTO() diff --git a/services/billing.py b/services/billing.py index 77c2a53c0c..de669a7045 100644 --- a/services/billing.py +++ b/services/billing.py @@ -7,7 +7,6 @@ from dateutil.relativedelta import relativedelta from django.conf import settings from shared.plan.constants import ( - FREE_PLAN_REPRESENTATIONS, PAID_PLANS, TEAM_PLANS, PlanBillingRate, @@ -793,7 +792,9 @@ def update_plan(self, owner, desired_plan): on current state, might create a stripe checkout session and return the checkout session's ID, which is a string. Otherwise returns None. """ - if desired_plan["value"] in FREE_PLAN_REPRESENTATIONS: + if desired_plan["value"] in Plan.objects.filter(paid_plan=False).values_list( + "name", flat=True + ): if owner.stripe_subscription_id is not None: self.payment_service.delete_subscription(owner) else: From 5050704ed8837bc0b81ffeaf5e1746807abcdf92 Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Wed, 15 Jan 2025 12:06:48 -0800 Subject: [PATCH 04/19] fix a couple instances... still need to fix availablePlans --- api/internal/owner/serializers.py | 4 +++- codecov_auth/admin.py | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/api/internal/owner/serializers.py b/api/internal/owner/serializers.py index 54193446c0..3ad05b33b2 100644 --- a/api/internal/owner/serializers.py +++ b/api/internal/owner/serializers.py @@ -337,7 +337,9 @@ def update(self, instance: Owner, validated_data: Dict[str, Any]) -> object: instance, desired_plan ) - sentry_plans = Plan.objects.filter(tier=TierName.Sentry.value) + sentry_plans = Plan.objects.filter( + tier=TierName.SENTRY.value, is_active=True + ) if desired_plan["value"] in sentry_plans: current_owner = self.context["view"].request.current_owner diff --git a/codecov_auth/admin.py b/codecov_auth/admin.py index e61af46efa..e9771f009d 100644 --- a/codecov_auth/admin.py +++ b/codecov_auth/admin.py @@ -608,7 +608,10 @@ class OwnerAdmin(AdminMixin, admin.ModelAdmin): def get_form(self, request, obj=None, change=False, **kwargs): form = super().get_form(request, obj, change, **kwargs) - PLANS_CHOICES = [(x, x) for x in Plan.objects.values_list("name", flat=True)] + PLANS_CHOICES = [ + (x, x) + for x in Plan.objects.filter(is_active=True).values_list("name", flat=True) + ] form.base_fields["plan"].widget = Select( choices=BLANK_CHOICE_DASH + PLANS_CHOICES ) From 40abd2d3b01dc0003517e40d6094cee2213699cf Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Wed, 15 Jan 2025 15:36:50 -0800 Subject: [PATCH 05/19] a few more references for PAID and TEAM_PLANS --- api/internal/owner/serializers.py | 7 ++++--- services/billing.py | 14 +++++++++----- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/api/internal/owner/serializers.py b/api/internal/owner/serializers.py index 3ad05b33b2..c4ed296647 100644 --- a/api/internal/owner/serializers.py +++ b/api/internal/owner/serializers.py @@ -7,7 +7,6 @@ from rest_framework import serializers from rest_framework.exceptions import PermissionDenied from shared.plan.constants import ( - PAID_PLANS, TEAM_PLAN_MAX_USERS, TierName, ) @@ -149,7 +148,9 @@ def validate(self, plan: Dict[str, Any]) -> Dict[str, Any]: ) # Validate quantity here because we need access to whole plan object - if plan["value"] in PAID_PLANS: + if plan["value"] in Plan.objects.filter( + paid_plan=True, is_active=True + ).values_list("name", flat=True): if "quantity" not in plan: raise serializers.ValidationError( "Field 'quantity' required for updating to paid plans" @@ -214,7 +215,7 @@ def get_plan(self, phase: Dict[str, Any]) -> str: plan_name = list(stripe_plan_dict.keys())[ list(stripe_plan_dict.values()).index(plan_id) ] - marketing_plan_name = PAID_PLANS[plan_name].billing_rate + marketing_plan_name = Plan.objects.get(name=plan_name).marketing_name return marketing_plan_name def get_quantity(self, phase: Dict[str, Any]) -> int: diff --git a/services/billing.py b/services/billing.py index de669a7045..eb5aef2782 100644 --- a/services/billing.py +++ b/services/billing.py @@ -7,8 +7,6 @@ from dateutil.relativedelta import relativedelta from django.conf import settings from shared.plan.constants import ( - PAID_PLANS, - TEAM_PLANS, PlanBillingRate, ) from shared.plan.service import PlanService @@ -479,11 +477,15 @@ def _is_similar_plan(self, owner: Owner, desired_plan: dict) -> bool: owner.plan_user_count and owner.plan_user_count == desired_plan["quantity"] ) + team_plans = Plan.objects.filter( + tier=TierName.TEAM.value, is_active=True + ).values_list("name", flat=True) + # If from PRO to TEAM, then not a similar plan - if owner.plan not in TEAM_PLANS and desired_plan["value"] in TEAM_PLANS: + if owner.plan not in team_plans and desired_plan["value"] in team_plans: return False # If from TEAM to PRO, then considered a similar plan but really is an upgrade - elif owner.plan in TEAM_PLANS and desired_plan["value"] not in TEAM_PLANS: + elif owner.plan in team_plans and desired_plan["value"] not in team_plans: return True return bool(is_same_term and is_same_seats) @@ -800,7 +802,9 @@ def update_plan(self, owner, desired_plan): else: plan_service = PlanService(current_org=owner) plan_service.set_default_plan_data() - elif desired_plan["value"] in PAID_PLANS: + elif desired_plan["value"] in Plan.objects.filter( + paid_plan=True, is_active=True + ).values_list("name", flat=True): if owner.stripe_subscription_id is not None: self.payment_service.modify_subscription(owner, desired_plan) else: From 16f7fbac4564b3478ceb84433f065c49ae726903 Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Wed, 15 Jan 2025 16:07:58 -0800 Subject: [PATCH 06/19] missing s --- upload/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/upload/helpers.py b/upload/helpers.py index 23379e3ee8..461f513d15 100644 --- a/upload/helpers.py +++ b/upload/helpers.py @@ -637,7 +637,7 @@ def validate_upload( # If author is on per repo billing, check their repo credits if ( - owner.plan not in Plan.object.values_list("name", flat=True) + owner.plan not in Plan.objects.values_list("name", flat=True) and owner.repo_credits <= 0 ): raise ValidationError( From 3a5b3aff3eea17979ecf01f4d91d948a7467a14d Mon Sep 17 00:00:00 2001 From: RulaKhaled Date: Mon, 20 Jan 2025 12:51:02 +0100 Subject: [PATCH 07/19] add missing imports + refactor to call 1 query --- api/internal/owner/serializers.py | 19 +++++++++++++------ .../tests/views/test_account_viewset.py | 11 ++++++++++- billing/helpers.py | 2 +- .../services/org_level_token_service.py | 2 +- graphql_api/types/owner/owner.py | 1 + services/billing.py | 3 ++- upload/helpers.py | 1 + 7 files changed, 29 insertions(+), 10 deletions(-) diff --git a/api/internal/owner/serializers.py b/api/internal/owner/serializers.py index c4ed296647..81883a802c 100644 --- a/api/internal/owner/serializers.py +++ b/api/internal/owner/serializers.py @@ -12,7 +12,7 @@ ) from shared.plan.service import PlanService -from codecov_auth.models import Owner +from codecov_auth.models import Owner, Plan from services.billing import BillingService from services.sentry import send_user_webhook as send_sentry_webhook @@ -147,10 +147,18 @@ def validate(self, plan: Dict[str, Any]) -> Dict[str, Any]: detail="You cannot update your plan manually, for help or changes to plan, connect with sales@codecov.io" ) + active_plans = list( + Plan.objects.filter(paid_plan=True, is_active=True).values_list( + "name", "tier" + ) + ) + active_plan_names = {name for name, _ in active_plans} + team_tier_plans = { + name for name, tier in active_plans if tier == TierName.TEAM.value + } + # Validate quantity here because we need access to whole plan object - if plan["value"] in Plan.objects.filter( - paid_plan=True, is_active=True - ).values_list("name", flat=True): + if plan["value"] in active_plan_names: if "quantity" not in plan: raise serializers.ValidationError( "Field 'quantity' required for updating to paid plans" @@ -179,8 +187,7 @@ def validate(self, plan: Dict[str, Any]) -> Dict[str, Any]: "Quantity or plan for paid plan must be different from the existing one" ) if ( - plan["value"] - in Plan.objects.filter(tier=TierName.TEAM.value, is_active=True) + plan["value"] in team_tier_plans and plan["quantity"] > TEAM_PLAN_MAX_USERS ): raise serializers.ValidationError( diff --git a/api/internal/tests/views/test_account_viewset.py b/api/internal/tests/views/test_account_viewset.py index c183cfef24..4393ac2313 100644 --- a/api/internal/tests/views/test_account_viewset.py +++ b/api/internal/tests/views/test_account_viewset.py @@ -12,9 +12,11 @@ AccountFactory, InvoiceBillingFactory, OwnerFactory, + PlanFactory, + TierFactory, UserFactory, ) -from shared.plan.constants import PlanName, TrialStatus +from shared.plan.constants import PlanName, TierName, TrialStatus from stripe import StripeError from api.internal.tests.test_utils import GetAdminProviderAdapter @@ -187,6 +189,13 @@ def test_retrieve_account_gets_account_fields(self): def test_retrieve_account_gets_account_fields_when_there_are_scheduled_details( self, mock_retrieve_subscription, mock_retrieve_schedule ): + tier = TierFactory(tier_name=TierName.BASIC.value) + + PlanFactory( + name="users-basic", + tier=tier, + is_active=True, + ) owner = OwnerFactory( admins=[self.current_owner.ownerid], stripe_subscription_id="sub_123" ) diff --git a/billing/helpers.py b/billing/helpers.py index 7d6c6af2ae..8be859e4e8 100644 --- a/billing/helpers.py +++ b/billing/helpers.py @@ -2,7 +2,7 @@ from django.db.models import QuerySet from shared.plan.constants import TierName -from codecov_auth.models import Owner +from codecov_auth.models import Owner, Plan def on_enterprise_plan(owner: Owner) -> bool: diff --git a/codecov_auth/services/org_level_token_service.py b/codecov_auth/services/org_level_token_service.py index d241fa8989..bb20bfb36d 100644 --- a/codecov_auth/services/org_level_token_service.py +++ b/codecov_auth/services/org_level_token_service.py @@ -5,7 +5,7 @@ from django.dispatch import receiver from django.forms import ValidationError -from codecov_auth.models import OrganizationLevelToken, Owner +from codecov_auth.models import OrganizationLevelToken, Owner, Plan log = logging.getLogger(__name__) diff --git a/graphql_api/types/owner/owner.py b/graphql_api/types/owner/owner.py index 775a1ad710..61a844d9e8 100644 --- a/graphql_api/types/owner/owner.py +++ b/graphql_api/types/owner/owner.py @@ -22,6 +22,7 @@ Account, GithubAppInstallation, Owner, + Plan, ) from codecov_auth.views.okta_cloud import OKTA_SIGNED_IN_ACCOUNTS_SESSION_KEY from core.models import Repository diff --git a/services/billing.py b/services/billing.py index eb5aef2782..c8ac2fb55a 100644 --- a/services/billing.py +++ b/services/billing.py @@ -8,11 +8,12 @@ from django.conf import settings from shared.plan.constants import ( PlanBillingRate, + TierName, ) from shared.plan.service import PlanService from billing.constants import REMOVED_INVOICE_STATUSES -from codecov_auth.models import Owner +from codecov_auth.models import Owner, Plan log = logging.getLogger(__name__) diff --git a/upload/helpers.py b/upload/helpers.py index 461f513d15..9e6cae583a 100644 --- a/upload/helpers.py +++ b/upload/helpers.py @@ -28,6 +28,7 @@ SERVICE_GITHUB_ENTERPRISE, GithubAppInstallation, Owner, + Plan, ) from core.models import Commit, Repository from reports.models import CommitReport, ReportSession From d26259a2f9447c6d0ec43ca7167c66f113c9a7f3 Mon Sep 17 00:00:00 2001 From: RulaKhaled Date: Mon, 20 Jan 2025 14:07:57 +0100 Subject: [PATCH 08/19] update tests --- .../tests/views/test_account_viewset.py | 65 +++++++++++-------- 1 file changed, 39 insertions(+), 26 deletions(-) diff --git a/api/internal/tests/views/test_account_viewset.py b/api/internal/tests/views/test_account_viewset.py index 4393ac2313..31196a57e9 100644 --- a/api/internal/tests/views/test_account_viewset.py +++ b/api/internal/tests/views/test_account_viewset.py @@ -189,12 +189,14 @@ def test_retrieve_account_gets_account_fields(self): def test_retrieve_account_gets_account_fields_when_there_are_scheduled_details( self, mock_retrieve_subscription, mock_retrieve_schedule ): - tier = TierFactory(tier_name=TierName.BASIC.value) + pro_tier = TierFactory(tier_name=TierName.PRO.value) PlanFactory( - name="users-basic", - tier=tier, + name="users-pr-inappm", + tier=pro_tier, is_active=True, + billing_rate="monthly", + marketing_name="Pro", ) owner = OwnerFactory( admins=[self.current_owner.ownerid], stripe_subscription_id="sub_123" @@ -238,15 +240,22 @@ def test_retrieve_account_gets_account_fields_when_there_are_scheduled_details( kwargs={"service": owner.service, "owner_username": owner.username} ) assert response.status_code == status.HTTP_200_OK + print(response.data) assert response.data == { - "activated_user_count": 0, - "root_organization": None, "integration_id": owner.integration_id, - "plan_auto_activate": owner.plan_auto_activate, + "activated_student_count": 0, + "activated_user_count": 0, + "checkout_session_id": None, + "delinquent": None, + "email": owner.email, "inactive_user_count": 1, + "name": owner.name, + "nb_active_private_repos": 0, + "plan_auto_activate": True, + "plan_provider": owner.plan_provider, "plan": { "marketing_name": "Developer", - "value": PlanName.BASIC_PLAN_NAME.value, + "value": "users-basic", "billing_rate": None, "base_unit_price": 0, "benefits": [ @@ -256,6 +265,17 @@ def test_retrieve_account_gets_account_fields_when_there_are_scheduled_details( ], "quantity": 1, }, + "repo_total_credits": 99999999, + "root_organization": None, + "schedule_detail": { + "id": "123", + "scheduled_phase": { + "start_date": schedule_params["start_date"], + "plan": "Pro", + "quantity": schedule_params["quantity"], + }, + }, + "student_count": 0, "subscription_detail": { "latest_invoice": None, "default_payment_method": None, @@ -263,27 +283,10 @@ def test_retrieve_account_gets_account_fields_when_there_are_scheduled_details( "current_period_end": 1633512445, "customer": {"id": "cus_LK&*Hli8YLIO", "discount": None, "email": None}, "collection_method": "charge_automatically", - "trial_end": None, "tax_ids": None, - }, - "checkout_session_id": None, - "name": owner.name, - "email": owner.email, - "nb_active_private_repos": 0, - "repo_total_credits": 99999999, - "plan_provider": owner.plan_provider, - "activated_student_count": 0, - "student_count": 0, - "schedule_detail": { - "id": "123", - "scheduled_phase": { - "plan": "monthly", - "quantity": schedule_params["quantity"], - "start_date": schedule_params["start_date"], - }, + "trial_end": None, }, "uses_invoice": False, - "delinquent": None, } @patch("services.billing.stripe.SubscriptionSchedule.retrieve") @@ -291,6 +294,16 @@ def test_retrieve_account_gets_account_fields_when_there_are_scheduled_details( def test_retrieve_account_returns_last_phase_when_more_than_one_scheduled_phases( self, mock_retrieve_subscription, mock_retrieve_schedule ): + pro_tier = TierFactory(tier_name=TierName.PRO.value) + + PlanFactory( + name="users-pr-inappm", + tier=pro_tier, + is_active=True, + billing_rate="monthly", + marketing_name="Pro", + ) + owner = OwnerFactory( admins=[self.current_owner.ownerid], stripe_subscription_id="sub_2345687" ) @@ -380,7 +393,7 @@ def test_retrieve_account_returns_last_phase_when_more_than_one_scheduled_phases "schedule_detail": { "id": "123", "scheduled_phase": { - "plan": "monthly", + "plan": "Pro", "quantity": schedule_params["quantity"], "start_date": schedule_params["start_date"], }, From f21a5f1e5c99b58aef833d997aacadd9de711cfb Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Tue, 21 Jan 2025 13:42:46 -0800 Subject: [PATCH 09/19] testing locally, fix the tier checks and first test --- api/internal/owner/serializers.py | 2 +- api/internal/tests/test_pagination.py | 6 +++++- billing/helpers.py | 2 +- graphql_api/types/enums/tier_name.graphql | 2 ++ requirements.in | 2 +- requirements.txt | 2 +- 6 files changed, 11 insertions(+), 5 deletions(-) diff --git a/api/internal/owner/serializers.py b/api/internal/owner/serializers.py index 81883a802c..2702342c56 100644 --- a/api/internal/owner/serializers.py +++ b/api/internal/owner/serializers.py @@ -346,7 +346,7 @@ def update(self, instance: Owner, validated_data: Dict[str, Any]) -> object: ) sentry_plans = Plan.objects.filter( - tier=TierName.SENTRY.value, is_active=True + tier__tier_name=TierName.SENTRY.value, is_active=True ) if desired_plan["value"] in sentry_plans: diff --git a/api/internal/tests/test_pagination.py b/api/internal/tests/test_pagination.py index 3bde43a69e..ca972f18c2 100644 --- a/api/internal/tests/test_pagination.py +++ b/api/internal/tests/test_pagination.py @@ -1,6 +1,8 @@ from rest_framework.reverse import reverse from rest_framework.test import APITestCase +from shared.django_apps.codecov_auth.tests.factories import PlanFactory, TierFactory from shared.django_apps.core.tests.factories import OwnerFactory +from shared.plan.constants import TierName from utils.test_utils import Client @@ -8,7 +10,9 @@ class PageNumberPaginationTests(APITestCase): def setUp(self): self.client = Client() - self.owner = OwnerFactory(plan="users-free", plan_user_count=5) + tier = TierFactory(tier_name=TierName.BASIC.value) + plan = PlanFactory(tier=tier, is_active=True) + self.owner = OwnerFactory(plan=plan.name, plan_user_count=5) self.users = [ OwnerFactory(organizations=[self.owner.ownerid]), OwnerFactory(organizations=[self.owner.ownerid]), diff --git a/billing/helpers.py b/billing/helpers.py index 8be859e4e8..fb635b332a 100644 --- a/billing/helpers.py +++ b/billing/helpers.py @@ -8,7 +8,7 @@ def on_enterprise_plan(owner: Owner) -> bool: return settings.IS_ENTERPRISE or ( owner.plan - in Plan.objects.filter(tier=TierName.ENTERPRISE.value).values_list( + in Plan.objects.filter(tier__tier_name=TierName.ENTERPRISE.value).values_list( "name", flat=True ) ) diff --git a/graphql_api/types/enums/tier_name.graphql b/graphql_api/types/enums/tier_name.graphql index 24e323be03..aeb825d0d4 100644 --- a/graphql_api/types/enums/tier_name.graphql +++ b/graphql_api/types/enums/tier_name.graphql @@ -3,4 +3,6 @@ enum TierName { TEAM PRO ENTERPRISE + SENTRY + TRIAL } diff --git a/requirements.in b/requirements.in index 5bea32f01f..702d7f65a9 100644 --- a/requirements.in +++ b/requirements.in @@ -26,7 +26,7 @@ freezegun google-cloud-pubsub gunicorn>=22.0.0 https://github.com/codecov/opentelem-python/archive/refs/tags/v0.0.4a1.tar.gz#egg=codecovopentelem -https://github.com/codecov/shared/archive/fe16480b3646a616ff412d5c0a28cafd2c7104c1.tar.gz#egg=shared +https://github.com/codecov/shared/archive/3dcd8efa027e6f45e0cbfd2c08a936ec5a68a57c.tar.gz#egg=shared https://github.com/photocrowd/django-cursor-pagination/archive/f560902696b0c8509e4d95c10ba0d62700181d84.tar.gz idna>=3.7 minio diff --git a/requirements.txt b/requirements.txt index f88b5e98d5..5b82e91188 100644 --- a/requirements.txt +++ b/requirements.txt @@ -416,7 +416,7 @@ sentry-sdk[celery]==2.13.0 # shared setproctitle==1.1.10 # via -r requirements.in -shared @ https://github.com/codecov/shared/archive/fe16480b3646a616ff412d5c0a28cafd2c7104c1.tar.gz +shared @ https://github.com/codecov/shared/archive/3dcd8efa027e6f45e0cbfd2c08a936ec5a68a57c.tar.gz # via -r requirements.in simplejson==3.17.2 # via -r requirements.in From ba63895a8dd67b1a9c06b0d661256e3424fee081 Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Tue, 21 Jan 2025 15:47:25 -0800 Subject: [PATCH 10/19] starting to fix tests and some logic --- .../interactors/tests/test_cancel_trial.py | 16 ++++++- .../tests/test_get_uploads_number_per_user.py | 16 +++++-- .../interactors/tests/test_start_trial.py | 13 +++++ codecov_auth/tests/test_admin.py | 48 ++++++++++++------- .../services/test_org_level_token_service.py | 37 ++++++++++---- graphql_api/tests/test_plan_representation.py | 24 ++++++---- graphql_api/types/owner/owner.py | 6 ++- .../plan_representation.py | 2 + 8 files changed, 120 insertions(+), 42 deletions(-) diff --git a/codecov_auth/commands/owner/interactors/tests/test_cancel_trial.py b/codecov_auth/commands/owner/interactors/tests/test_cancel_trial.py index 138b04f64e..6aa62d59f8 100644 --- a/codecov_auth/commands/owner/interactors/tests/test_cancel_trial.py +++ b/codecov_auth/commands/owner/interactors/tests/test_cancel_trial.py @@ -5,8 +5,9 @@ from django.test import TransactionTestCase from freezegun import freeze_time from shared.django_apps.codecov.commands.exceptions import ValidationError +from shared.django_apps.codecov_auth.tests.factories import PlanFactory, TierFactory from shared.django_apps.core.tests.factories import OwnerFactory -from shared.plan.constants import PlanName, TrialStatus +from shared.plan.constants import PlanName, TierName, TrialStatus from codecov.commands.exceptions import Unauthorized from codecov.commands.exceptions import ValidationError as CodecovValidationError @@ -16,6 +17,10 @@ class CancelTrialInteractorTest(TransactionTestCase): + def setUp(self): + self.tier = TierFactory(tier_name=TierName.BASIC.value) + self.plan = PlanFactory(tier=self.tier) + @async_to_sync def execute(self, current_user, org_username=None): current_user = current_user @@ -27,6 +32,7 @@ def test_cancel_trial_raises_exception_when_owner_is_not_in_db(self): current_user = OwnerFactory( username="random-user-123", service="github", + plan=self.plan.name, ) with pytest.raises(CodecovValidationError): self.execute(current_user=current_user, org_username="some-other-username") @@ -35,10 +41,12 @@ def test_cancel_trial_raises_exception_when_current_user_not_part_of_org(self): current_user = OwnerFactory( username="random-user-123", service="github", + plan=self.plan.name, ) OwnerFactory( username="random-user-456", service="github", + plan=self.plan.name, ) with pytest.raises(Unauthorized): self.execute(current_user=current_user, org_username="random-user-456") @@ -54,6 +62,7 @@ def test_cancel_trial_raises_exception_when_owners_trial_status_is_not_started( service="github", trial_start_date=trial_start_date, trial_end_date=trial_end_date, + plan=self.plan.name, ) with pytest.raises(ValidationError): self.execute(current_user=current_user, org_username=current_user.username) @@ -68,6 +77,7 @@ def test_cancel_trial_raises_exception_when_owners_trial_status_is_expired(self) service="github", trial_start_date=trial_start_date, trial_end_date=trial_end_date, + plan=self.plan.name, ) with pytest.raises(ValidationError): self.execute(current_user=current_user, org_username=current_user.username) @@ -77,13 +87,15 @@ def test_cancel_trial_starts_trial_for_org_that_has_trial_ongoing(self): now = datetime.now() trial_start_date = now trial_end_date = now + timedelta(days=3) + trial_tier = TierFactory(tier_name=TierName.TRIAL.value) + trial_plan = PlanFactory(tier=trial_tier, name=PlanName.TRIAL_PLAN_NAME.value) current_user: Owner = OwnerFactory( username="random-user-123", service="github", trial_start_date=trial_start_date, trial_end_date=trial_end_date, trial_status=TrialStatus.ONGOING.value, - plan=PlanName.TRIAL_PLAN_NAME.value, + plan=trial_plan.name, ) self.execute(current_user=current_user, org_username=current_user.username) current_user.refresh_from_db() diff --git a/codecov_auth/commands/owner/interactors/tests/test_get_uploads_number_per_user.py b/codecov_auth/commands/owner/interactors/tests/test_get_uploads_number_per_user.py index 39eb4596e2..2b4cf1fd56 100644 --- a/codecov_auth/commands/owner/interactors/tests/test_get_uploads_number_per_user.py +++ b/codecov_auth/commands/owner/interactors/tests/test_get_uploads_number_per_user.py @@ -1,13 +1,14 @@ from datetime import datetime, timedelta from django.test import TransactionTestCase +from shared.django_apps.codecov_auth.tests.factories import PlanFactory, TierFactory from shared.django_apps.core.tests.factories import ( CommitFactory, OwnerFactory, RepositoryFactory, ) from shared.django_apps.reports.models import ReportType -from shared.plan.constants import TrialStatus +from shared.plan.constants import PlanName, TierName, TrialStatus from shared.upload.utils import UploaderType, insert_coverage_measurement from reports.tests.factories import CommitReportFactory, UploadFactory @@ -17,8 +18,10 @@ class GetUploadsNumberPerUserInteractorTest(TransactionTestCase): def setUp(self): - self.user_with_no_uploads = OwnerFactory() - self.user_with_uploads = OwnerFactory() + self.tier = TierFactory(tier_name=TierName.BASIC.value) + self.plan = PlanFactory(tier=self.tier, monthly_uploads_limit=250) + self.user_with_no_uploads = OwnerFactory(plan=self.plan.name) + self.user_with_uploads = OwnerFactory(plan=self.plan.name) repo = RepositoryFactory.create(author=self.user_with_uploads, private=True) commit = CommitFactory.create(repository=repo) report = CommitReportFactory.create( @@ -44,10 +47,17 @@ def setUp(self): report_within_40_days.save() # Trial Data + trial_tier = TierFactory(tier_name=TierName.TRIAL.value) + trial_plan = PlanFactory( + tier=trial_tier, + name=PlanName.TRIAL_PLAN_NAME.value, + monthly_uploads_limit=250, + ) self.trial_owner = OwnerFactory( trial_status=TrialStatus.EXPIRED.value, trial_start_date=datetime.now() + timedelta(days=-10), trial_end_date=datetime.now() + timedelta(days=-2), + plan=trial_plan.name, ) trial_repo = RepositoryFactory.create(author=self.trial_owner, private=True) trial_commit = CommitFactory.create(repository=trial_repo) diff --git a/codecov_auth/commands/owner/interactors/tests/test_start_trial.py b/codecov_auth/commands/owner/interactors/tests/test_start_trial.py index 3504e9d527..a95562c42a 100644 --- a/codecov_auth/commands/owner/interactors/tests/test_start_trial.py +++ b/codecov_auth/commands/owner/interactors/tests/test_start_trial.py @@ -5,10 +5,12 @@ from django.test import TransactionTestCase from freezegun import freeze_time from shared.django_apps.codecov.commands.exceptions import ValidationError +from shared.django_apps.codecov_auth.tests.factories import PlanFactory, TierFactory from shared.django_apps.core.tests.factories import OwnerFactory from shared.plan.constants import ( TRIAL_PLAN_SEATS, PlanName, + TierName, TrialDaysAmount, TrialStatus, ) @@ -21,6 +23,10 @@ class StartTrialInteractorTest(TransactionTestCase): + def setUp(self): + self.tier = TierFactory(tier_name=TierName.BASIC.value) + self.plan = PlanFactory(tier=self.tier, is_active=True) + @async_to_sync def execute(self, current_user, org_username=None): current_user = current_user @@ -32,6 +38,7 @@ def test_start_trial_raises_exception_when_owner_is_not_in_db(self): current_user = OwnerFactory( username="random-user-123", service="github", + plan=self.plan.name, ) with pytest.raises(CodecovValidationError): self.execute(current_user=current_user, org_username="some-other-username") @@ -40,10 +47,12 @@ def test_cancel_trial_raises_exception_when_current_user_not_part_of_org(self): current_user = OwnerFactory( username="random-user-123", service="github", + plan=self.plan.name, ) OwnerFactory( username="random-user-456", service="github", + plan=self.plan.name, ) with pytest.raises(Unauthorized): self.execute(current_user=current_user, org_username="random-user-456") @@ -59,6 +68,7 @@ def test_start_trial_raises_exception_when_owners_trial_status_is_ongoing(self): trial_start_date=trial_start_date, trial_end_date=trial_end_date, trial_status=TrialStatus.ONGOING.value, + plan=self.plan.name, ) with pytest.raises(ValidationError): self.execute(current_user=current_user, org_username=current_user.username) @@ -74,6 +84,7 @@ def test_start_trial_raises_exception_when_owners_trial_status_is_expired(self): trial_start_date=trial_start_date, trial_end_date=trial_end_date, trial_status=TrialStatus.EXPIRED.value, + plan=self.plan.name, ) with pytest.raises(ValidationError): self.execute(current_user=current_user, org_username=current_user.username) @@ -91,6 +102,7 @@ def test_start_trial_raises_exception_when_owners_trial_status_cannot_trial( trial_start_date=trial_start_date, trial_end_date=trial_end_date, trial_status=TrialStatus.CANNOT_TRIAL.value, + plan=self.plan.name, ) with pytest.raises(ValidationError): self.execute(current_user=current_user, org_username=current_user.username) @@ -105,6 +117,7 @@ def test_start_trial_starts_trial_for_org_that_has_not_started_trial_before_and_ trial_start_date=None, trial_end_date=None, trial_status=TrialStatus.NOT_STARTED.value, + plan=self.plan.name, ) self.execute(current_user=current_user, org_username=current_user.username) current_user.refresh_from_db() diff --git a/codecov_auth/tests/test_admin.py b/codecov_auth/tests/test_admin.py index bd9d0ca4e6..926f290be6 100644 --- a/codecov_auth/tests/test_admin.py +++ b/codecov_auth/tests/test_admin.py @@ -29,6 +29,7 @@ from shared.plan.constants import ( ENTERPRISE_CLOUD_USER_PLAN_REPRESENTATIONS, PlanName, + TierName, ) from codecov.commands.exceptions import ValidationError @@ -60,6 +61,17 @@ def setUp(self): admin_site.register(OrganizationLevelToken) self.owner_admin = OwnerAdmin(Owner, admin_site) + self.basic_tier = TierFactory(tier_name=TierName.BASIC.value) + self.basic_plan = PlanFactory( + tier=self.basic_tier, name=PlanName.BASIC_PLAN_NAME.value + ) + + self.enterprise_tier = TierFactory(tier_name=TierName.ENTERPRISE.value) + self.enterprise_plan = PlanFactory( + tier=self.enterprise_tier, + name=PlanName.ENTERPRISE_CLOUD_YEARLY.value, + ) + def test_owner_admin_detail_page(self): owner = OwnerFactory() response = self.client.get( @@ -68,8 +80,10 @@ def test_owner_admin_detail_page(self): self.assertEqual(response.status_code, 200) def test_owner_admin_impersonate_owner(self): - owner_to_impersonate = OwnerFactory(service="bitbucket") - other_owner = OwnerFactory() + owner_to_impersonate = OwnerFactory( + service="bitbucket", plan=self.basic_plan.name + ) + other_owner = OwnerFactory(plan=self.basic_plan.name) with self.subTest("more than one user selected"): response = self.client.post( @@ -103,7 +117,7 @@ def test_owner_admin_impersonate_owner(self): @patch("codecov_auth.admin.TaskService.delete_owner") def test_delete_queryset(self, delete_mock): - user_to_delete = OwnerFactory() + user_to_delete = OwnerFactory(plan=self.basic_plan.name) ownerid = user_to_delete.ownerid queryset = MagicMock() queryset.__iter__.return_value = [user_to_delete] @@ -114,14 +128,14 @@ def test_delete_queryset(self, delete_mock): @patch("codecov_auth.admin.TaskService.delete_owner") def test_delete_model(self, delete_mock): - user_to_delete = OwnerFactory() + user_to_delete = OwnerFactory(plan=self.basic_plan.name) ownerid = user_to_delete.ownerid self.owner_admin.delete_model(MagicMock(), user_to_delete) delete_mock.assert_called_once_with(ownerid=ownerid) @patch("codecov_auth.admin.admin.ModelAdmin.get_deleted_objects") def test_confirmation_deleted_objects(self, mocked_deleted_objs): - user_to_delete = OwnerFactory() + user_to_delete = OwnerFactory(plan=self.basic_plan.name) deleted_objs = [ 'Owner: {};'.format( user_to_delete.ownerid, user_to_delete @@ -141,7 +155,7 @@ def test_confirmation_deleted_objects(self, mocked_deleted_objs): @patch("codecov_auth.admin.admin.ModelAdmin.log_change") def test_prev_and_new_values_in_log_entry(self, mocked_super_log_change): - owner = OwnerFactory(staff=True) + owner = OwnerFactory(staff=True, plan=self.basic_plan.name) owner.save() owner.staff = False form = MagicMock() @@ -161,7 +175,7 @@ def test_prev_and_new_values_in_log_entry(self, mocked_super_log_change): ] def test_inline_orgwide_tokens_display(self): - owner = OwnerFactory() + owner = OwnerFactory(plan=self.basic_plan.name) request_url = reverse("admin:codecov_auth_owner_change", args=[owner.ownerid]) request = RequestFactory().get(request_url) request.user = self.staff_user @@ -170,7 +184,7 @@ def test_inline_orgwide_tokens_display(self): assert isinstance(inlines[0], OrgUploadTokenInline) def test_inline_orgwide_permissions(self): - owner_in_cloud_plan = OwnerFactory(plan="users-enterprisey") + owner_in_cloud_plan = OwnerFactory(plan=self.enterprise_plan.name) org_token = OrganizationLevelTokenFactory(owner=owner_in_cloud_plan) owner_in_cloud_plan.save() org_token.save() @@ -194,7 +208,7 @@ def test_inline_orgwide_permissions(self): def test_inline_orgwide_add_token_permission_no_token_and_user_in_enterprise_cloud_plan( self, ): - owner = OwnerFactory() + owner = OwnerFactory(plan=self.basic_plan.name) assert owner.plan not in ENTERPRISE_CLOUD_USER_PLAN_REPRESENTATIONS assert OrganizationLevelToken.objects.filter(owner=owner).count() == 0 request_url = reverse("admin:codecov_auth_owner_change", args=[owner.ownerid]) @@ -207,7 +221,7 @@ def test_inline_orgwide_add_token_permission_no_token_and_user_in_enterprise_clo def test_inline_orgwide_add_token_permission_no_token_user_not_in_enterprise_cloud_plan( self, ): - owner_in_cloud_plan = OwnerFactory(plan="users-enterprisey") + owner_in_cloud_plan = OwnerFactory(plan=self.enterprise_plan.name) assert ( OrganizationLevelToken.objects.filter(owner=owner_in_cloud_plan).count() == 0 @@ -227,7 +241,7 @@ def test_inline_orgwide_add_token_permission_no_token_user_not_in_enterprise_clo def test_org_token_refresh_request_calls_service_to_refresh_token( self, mock_refresh ): - owner_in_cloud_plan = OwnerFactory(plan="users-enterprisey") + owner_in_cloud_plan = OwnerFactory(plan=self.enterprise_plan.name) org_token = OrganizationLevelTokenFactory(owner=owner_in_cloud_plan) owner_in_cloud_plan.save() org_token.save() @@ -264,7 +278,7 @@ def test_org_token_refresh_request_calls_service_to_refresh_token( "codecov_auth.services.org_level_token_service.OrgLevelTokenService.refresh_token" ) def test_org_token_request_doesnt_call_service_to_refresh_token(self, mock_refresh): - owner_in_cloud_plan = OwnerFactory(plan="users-enterprisey") + owner_in_cloud_plan = OwnerFactory(plan=self.enterprise_plan.name) org_token = OrganizationLevelTokenFactory(owner=owner_in_cloud_plan) owner_in_cloud_plan.save() org_token.save() @@ -297,7 +311,7 @@ def test_org_token_request_doesnt_call_service_to_refresh_token(self, mock_refre mock_refresh.assert_not_called() def test_start_trial_ui_display(self): - owner = OwnerFactory() + owner = OwnerFactory(plan=self.basic_plan.name) res = self.client.post( reverse("admin:codecov_auth_owner_changelist"), @@ -312,7 +326,7 @@ def test_start_trial_ui_display(self): @patch("shared.plan.service.PlanService.start_trial_manually") def test_start_trial_action(self, mock_start_trial_service): mock_start_trial_service.return_value = None - org_to_be_trialed = OwnerFactory() + org_to_be_trialed = OwnerFactory(plan=self.basic_plan.name) res = self.client.post( reverse("admin:codecov_auth_owner_changelist"), @@ -329,7 +343,7 @@ def test_start_trial_action(self, mock_start_trial_service): @patch("shared.plan.service.PlanService._start_trial_helper") def test_extend_trial_action(self, mock_start_trial_service): mock_start_trial_service.return_value = None - org_to_be_trialed = OwnerFactory() + org_to_be_trialed = OwnerFactory(plan=self.basic_plan.name) org_to_be_trialed.plan = PlanName.TRIAL_PLAN_NAME.value org_to_be_trialed.save() @@ -352,7 +366,7 @@ def test_start_trial_paid_plan(self, mock_start_trial_service): "Cannot trial from a paid plan" ) - org_to_be_trialed = OwnerFactory() + org_to_be_trialed = OwnerFactory(plan=self.basic_plan.name) res = self.client.post( reverse("admin:codecov_auth_owner_changelist"), @@ -367,7 +381,7 @@ def test_start_trial_paid_plan(self, mock_start_trial_service): assert mock_start_trial_service.called def test_account_widget(self): - owner = OwnerFactory(user=UserFactory(), plan="users-enterprisey") + owner = OwnerFactory(user=UserFactory(), plan=self.enterprise_plan.name) rf = RequestFactory() get_request = rf.get(f"/admin/codecov_auth/owner/{owner.ownerid}/change/") get_request.user = self.staff_user diff --git a/codecov_auth/tests/unit/services/test_org_level_token_service.py b/codecov_auth/tests/unit/services/test_org_level_token_service.py index 16fc46daea..1c5d423c69 100644 --- a/codecov_auth/tests/unit/services/test_org_level_token_service.py +++ b/codecov_auth/tests/unit/services/test_org_level_token_service.py @@ -7,7 +7,10 @@ from shared.django_apps.codecov_auth.tests.factories import ( OrganizationLevelTokenFactory, OwnerFactory, + PlanFactory, + TierFactory, ) +from shared.plan.constants import PlanName, TierName from codecov_auth.models import OrganizationLevelToken from codecov_auth.services.org_level_token_service import OrgLevelTokenService @@ -20,7 +23,11 @@ def test_token_is_deleted_when_changing_user_plan(mocked_org_can_have_upload_tok # This should happen because of the signal consumer we have defined in # codecov_auth/services/org_upload_token_service.py > manage_org_tokens_if_owner_plan_changed mocked_org_can_have_upload_token.return_value = False - owner = OwnerFactory(plan="users-enterprisey") + enterprise_tier = TierFactory(tier_name=TierName.ENTERPRISE.value) + enterprise_plan = PlanFactory( + tier=enterprise_tier, name=PlanName.ENTERPRISE_CLOUD_YEARLY.value + ) + owner = OwnerFactory(plan=enterprise_plan.name) org_token = OrganizationLevelTokenFactory(owner=owner) owner.save() org_token.save() @@ -31,21 +38,33 @@ def test_token_is_deleted_when_changing_user_plan(mocked_org_can_have_upload_tok class TestOrgWideUploadTokenService(TransactionTestCase): + def setUp(self): + self.enterprise_tier = TierFactory(tier_name=TierName.ENTERPRISE.value) + self.enterprise_plan = PlanFactory( + tier=self.enterprise_tier, + name=PlanName.ENTERPRISE_CLOUD_YEARLY.value, + ) + self.basic_tier = TierFactory(tier_name=TierName.BASIC.value) + self.basic_plan = PlanFactory( + tier=self.basic_tier, + name=PlanName.BASIC_PLAN_NAME.value, + ) + self.owner = OwnerFactory(plan=self.enterprise_plan.name) + def test_get_org_token(self): # Check that if you try to create a token for an org that already has one you get the same token - owner = OwnerFactory(plan="users-enterprisey") - org_token = OrganizationLevelTokenFactory(owner=owner) - owner.save() + org_token = OrganizationLevelTokenFactory(owner=self.owner) + self.owner.save() org_token.save() - assert org_token == OrgLevelTokenService.get_or_create_org_token(owner) + assert org_token == OrgLevelTokenService.get_or_create_org_token(self.owner) def test_create_org_token(self): - user_in_enterprise_plan = OwnerFactory(plan="users-enterprisey") + user_in_enterprise_plan = OwnerFactory(plan=self.enterprise_plan.name) token = OrgLevelTokenService.get_or_create_org_token(user_in_enterprise_plan) assert isinstance(token.token, uuid.UUID) assert token.owner == user_in_enterprise_plan # Check that users not in enterprise plan can create org tokens - user_not_in_enterprise_plan = OwnerFactory(plan="users-basic") + user_not_in_enterprise_plan = OwnerFactory(plan=self.basic_plan.name) token = OrgLevelTokenService.get_or_create_org_token( user_not_in_enterprise_plan ) @@ -53,13 +72,13 @@ def test_create_org_token(self): assert token.owner == user_not_in_enterprise_plan def test_delete_token(self): - owner = OwnerFactory(plan="users-enterprisey") + owner = OwnerFactory(plan=self.enterprise_plan.name) OrgLevelTokenService.delete_org_token_if_exists(owner) with pytest.raises(OrganizationLevelToken.DoesNotExist): OrganizationLevelToken.objects.get(owner=owner) def test_refresh_token(self): - owner = OwnerFactory(plan="users-enterprisey") + owner = OwnerFactory(plan=self.enterprise_plan.name) org_token = OrganizationLevelTokenFactory(owner=owner) owner.save() org_token.save() diff --git a/graphql_api/tests/test_plan_representation.py b/graphql_api/tests/test_plan_representation.py index 2974b4ccee..760d7f3709 100644 --- a/graphql_api/tests/test_plan_representation.py +++ b/graphql_api/tests/test_plan_representation.py @@ -3,29 +3,39 @@ from django.test import TransactionTestCase from django.utils import timezone from freezegun import freeze_time +from shared.django_apps.codecov_auth.tests.factories import PlanFactory, TierFactory from shared.django_apps.core.tests.factories import OwnerFactory -from shared.plan.constants import PlanName, TrialStatus +from shared.plan.constants import PlanName, TierName, TrialStatus from .helper import GraphQLTestHelper class TestPlanRepresentationsType(GraphQLTestHelper, TransactionTestCase): def setUp(self): + self.tier = TierFactory(tier_name=TierName.BASIC.value) + self.plan = PlanFactory(tier=self.tier, is_active=True) self.current_org = OwnerFactory( username="random-plan-user", service="github", trial_start_date=timezone.now(), trial_end_date=timezone.now() + timedelta(days=14), + plan=self.plan.name, ) @freeze_time("2023-06-19") def test_owner_pretrial_plan_data_when_trialing(self): now = timezone.now() later = timezone.now() + timedelta(days=14) + trial_tier = TierFactory(tier_name=TierName.TRIAL.value) + trial_plan = PlanFactory( + tier=trial_tier, + is_active=True, + name=PlanName.TRIAL_PLAN_NAME.value, + ) current_org = OwnerFactory( username="random-plan-user", service="github", - plan=PlanName.TRIAL_PLAN_NAME.value, + plan=trial_plan.name, trial_start_date=now, trial_end_date=later, trial_status=TrialStatus.ONGOING.value, @@ -46,14 +56,10 @@ def test_owner_pretrial_plan_data_when_trialing(self): """ % (current_org.username) data = self.gql_request(query, owner=current_org) assert data["owner"]["pretrialPlan"] == { - "marketingName": "Developer", + "marketingName": self.plan.marketing_name, "value": "users-basic", "billingRate": None, "baseUnitPrice": 0, - "benefits": [ - "Up to 234 users", - "Unlimited public repositories", - "Unlimited private repositories", - ], - "monthlyUploadLimit": 250, + "benefits": ["Benefit 1", "Benefit 2", "Benefit 3"], + "monthlyUploadLimit": None, } diff --git a/graphql_api/types/owner/owner.py b/graphql_api/types/owner/owner.py index 61a844d9e8..52310f7e11 100644 --- a/graphql_api/types/owner/owner.py +++ b/graphql_api/types/owner/owner.py @@ -8,7 +8,7 @@ from ariadne import ObjectType from django.conf import settings from graphql import GraphQLResolveInfo -from shared.plan.constants import PlanData, PlanName +from shared.plan.constants import PlanData, PlanName, convert_to_DTO from shared.plan.service import PlanService import services.activation as activation @@ -111,14 +111,16 @@ def resolve_plan(owner: Owner, info: GraphQLResolveInfo) -> PlanService: @owner_bindable.field("pretrialPlan") @require_part_of_org +@sync_to_async def resolve_plan_representation(owner: Owner, info: GraphQLResolveInfo) -> PlanData: info.context["plan_service"] = PlanService(current_org=owner) free_plan = Plan.objects.get(name=PlanName.BASIC_PLAN_NAME.value) - return free_plan.convert_to_DTO() + return convert_to_DTO(free_plan) @owner_bindable.field("availablePlans") @require_part_of_org +@sync_to_async def resolve_available_plans(owner: Owner, info: GraphQLResolveInfo) -> List[PlanData]: plan_service = PlanService(current_org=owner) info.context["plan_service"] = plan_service diff --git a/graphql_api/types/plan_representation/plan_representation.py b/graphql_api/types/plan_representation/plan_representation.py index 8ef856265a..800dc637b3 100644 --- a/graphql_api/types/plan_representation/plan_representation.py +++ b/graphql_api/types/plan_representation/plan_representation.py @@ -4,6 +4,7 @@ from shared.plan.constants import PlanData from shared.plan.service import PlanService +from codecov.db import sync_to_async from graphql_api.helpers.ariadne import ariadne_load_local_graphql plan_representation = ariadne_load_local_graphql( @@ -33,6 +34,7 @@ def resolve_base_unit_price(plan_data: PlanData, info) -> int: @plan_representation_bindable.field("benefits") +@sync_to_async def resolve_benefits(plan_data: PlanData, info) -> List[str]: plan_service: PlanService = info.context["plan_service"] if plan_service.is_org_trialing: From 91ea64bd5e4dd1f2b372fd2f35feeeff2924dfcd Mon Sep 17 00:00:00 2001 From: RulaKhaled Date: Wed, 22 Jan 2025 18:39:20 +0100 Subject: [PATCH 11/19] update api with mocked plans --- billing/helpers.py | 156 +++++++++++++++++++++++++++++++++- billing/tests/test_helpers.py | 15 +++- 2 files changed, 168 insertions(+), 3 deletions(-) diff --git a/billing/helpers.py b/billing/helpers.py index fb635b332a..2405e0fd2c 100644 --- a/billing/helpers.py +++ b/billing/helpers.py @@ -1,6 +1,8 @@ from django.conf import settings from django.db.models import QuerySet -from shared.plan.constants import TierName +from shared.plan.constants import TierName, PlanName, PlanPrice +from shared.django_apps.codecov_auth.tests.factories import PlanFactory, TierFactory +from shared.django_apps.codecov_auth.models import BillingRate from codecov_auth.models import Owner, Plan @@ -26,3 +28,155 @@ def get_all_admins_for_owners(owners: QuerySet[Owner]): admins: QuerySet[Owner] = Owner.objects.filter(pk__in=admin_ids) return admins + + + +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, + ) \ No newline at end of file diff --git a/billing/tests/test_helpers.py b/billing/tests/test_helpers.py index f60974de34..c886c9a46c 100644 --- a/billing/tests/test_helpers.py +++ b/billing/tests/test_helpers.py @@ -1,7 +1,7 @@ from django.test import TestCase, override_settings from shared.django_apps.core.tests.factories import OwnerFactory -from shared.plan.constants import ENTERPRISE_CLOUD_USER_PLAN_REPRESENTATIONS - +from shared.plan.constants import ENTERPRISE_CLOUD_USER_PLAN_REPRESENTATIONS, TierName, PlanName +from shared.django_apps.codecov_auth.tests.factories import PlanFactory, TierFactory from billing.helpers import on_enterprise_plan @@ -12,6 +12,17 @@ def test_on_enterprise_plan_on_prem(self): assert on_enterprise_plan(owner) == True def test_on_enterprise_plan_enterpise_cloud(self): + enterprise_tier = TierFactory(tier_name=TierName.ENTERPRISE.value) + PlanFactory( + name=PlanName.ENTERPRISE_CLOUD_MONTHLY.value, + tier=enterprise_tier, + is_active=True, + ) + PlanFactory( + name=PlanName.ENTERPRISE_CLOUD_YEARLY.value, + tier=enterprise_tier, + is_active=True, + ) for plan in ENTERPRISE_CLOUD_USER_PLAN_REPRESENTATIONS.keys(): owner = OwnerFactory(plan=plan) assert on_enterprise_plan(owner) == True From 42488fbe44753c0dd0402e6b0de9a07e95c664e4 Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Wed, 22 Jan 2025 15:21:35 -0800 Subject: [PATCH 12/19] fix all tests except account_viewset --- billing/helpers.py | 7 +- billing/tests/test_helpers.py | 9 +- billing/tests/test_views.py | 23 +-- codecov_auth/tests/test_admin.py | 137 ++++++++---------- .../tests/mutation/test_cancel_trial.py | 9 +- graphql_api/tests/test_owner.py | 2 + graphql_api/tests/test_plan.py | 7 +- graphql_api/types/owner/owner.py | 1 + graphql_api/types/plan/plan.py | 2 + requirements.in | 2 +- requirements.txt | 2 +- services/billing.py | 2 +- services/tests/test_billing.py | 3 + upload/tests/test_helpers.py | 6 +- upload/tests/test_throttles.py | 2 + upload/tests/views/test_uploads.py | 15 +- 16 files changed, 127 insertions(+), 102 deletions(-) diff --git a/billing/helpers.py b/billing/helpers.py index 2405e0fd2c..3abfd9e583 100644 --- a/billing/helpers.py +++ b/billing/helpers.py @@ -1,8 +1,8 @@ from django.conf import settings from django.db.models import QuerySet -from shared.plan.constants import TierName, PlanName, PlanPrice -from shared.django_apps.codecov_auth.tests.factories import PlanFactory, TierFactory 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 from codecov_auth.models import Owner, Plan @@ -30,7 +30,6 @@ def get_all_admins_for_owners(owners: QuerySet[Owner]): return admins - def mock_all_plans_and_tiers(): trial_tier = TierFactory(tier_name=TierName.TRIAL.value) PlanFactory( @@ -179,4 +178,4 @@ def mock_all_plans_and_tiers(): billing_rate=BillingRate.ANNUALLY.value, base_unit_price=PlanPrice.YEARLY.value, paid_plan=True, - ) \ No newline at end of file + ) diff --git a/billing/tests/test_helpers.py b/billing/tests/test_helpers.py index c886c9a46c..dc255c5c29 100644 --- a/billing/tests/test_helpers.py +++ b/billing/tests/test_helpers.py @@ -1,7 +1,12 @@ from django.test import TestCase, override_settings -from shared.django_apps.core.tests.factories import OwnerFactory -from shared.plan.constants import ENTERPRISE_CLOUD_USER_PLAN_REPRESENTATIONS, TierName, PlanName from shared.django_apps.codecov_auth.tests.factories import PlanFactory, TierFactory +from shared.django_apps.core.tests.factories import OwnerFactory +from shared.plan.constants import ( + ENTERPRISE_CLOUD_USER_PLAN_REPRESENTATIONS, + PlanName, + TierName, +) + from billing.helpers import on_enterprise_plan diff --git a/billing/tests/test_views.py b/billing/tests/test_views.py index d1fbcc8da8..19e40b448a 100644 --- a/billing/tests/test_views.py +++ b/billing/tests/test_views.py @@ -11,6 +11,8 @@ from shared.django_apps.core.tests.factories import OwnerFactory, RepositoryFactory from shared.plan.constants import PlanName +from billing.helpers import mock_all_plans_and_tiers + from ..constants import StripeHTTPHeaders @@ -70,8 +72,10 @@ def __getitem__(self, key): class StripeWebhookHandlerTests(APITestCase): def setUp(self): + mock_all_plans_and_tiers() self.owner = OwnerFactory( - stripe_customer_id="cus_123", stripe_subscription_id="sub_123" + stripe_customer_id="cus_123", + stripe_subscription_id="sub_123", ) # Creates a second owner that shares billing details with self.owner. @@ -79,7 +83,8 @@ def setUp(self): # subscription in Stripe. def add_second_owner(self): self.other_owner = OwnerFactory( - stripe_customer_id="cus_123", stripe_subscription_id="sub_123" + stripe_customer_id="cus_123", + stripe_subscription_id="sub_123", ) def _send_event(self, payload, errorSig=None): @@ -467,7 +472,7 @@ def test_invoice_payment_failed_sends_email_to_admins_no_card( mocked_send_email.assert_has_calls(expected_calls) def test_customer_subscription_deleted_sets_plan_to_free(self): - self.owner.plan = "users-inappy" + self.owner.plan = PlanName.CODECOV_PRO_YEARLY.value self.owner.plan_user_count = 20 self.owner.save() @@ -492,10 +497,10 @@ def test_customer_subscription_deleted_sets_plan_to_free(self): def test_customer_subscription_deleted_sets_plan_to_free_mutliple_owner(self): self.add_second_owner() - self.owner.plan = "users-inappy" + self.owner.plan = PlanName.CODECOV_PRO_YEARLY.value self.owner.plan_user_count = 20 self.owner.save() - self.other_owner.plan = "users-inappy" + self.other_owner.plan = PlanName.CODECOV_PRO_YEARLY.value self.other_owner.plan_user_count = 20 self.other_owner.save() @@ -540,7 +545,7 @@ def test_customer_subscription_deleted_deactivates_all_repos(self): "object": { "id": self.owner.stripe_subscription_id, "customer": self.owner.stripe_customer_id, - "plan": {"name": "users-inappm"}, + "plan": {"name": PlanName.CODECOV_PRO_MONTHLY.value}, } }, } @@ -577,7 +582,7 @@ def test_customer_subscription_deleted_deactivates_all_repos_multiple_owner(self "object": { "id": self.owner.stripe_subscription_id, "customer": self.owner.stripe_customer_id, - "plan": {"name": "users-inappm"}, + "plan": {"name": PlanName.CODECOV_PRO_MONTHLY.value}, } }, } @@ -598,7 +603,7 @@ def test_customer_subscription_deleted_deactivates_all_repos_multiple_owner(self @patch("logging.Logger.info") def test_customer_subscription_deleted_no_customer(self, log_info_mock): - self.owner.plan = "users-inappy" + self.owner.plan = PlanName.CODECOV_PRO_MONTHLY.value self.owner.plan_user_count = 20 self.owner.save() @@ -688,7 +693,7 @@ def test_customer_subscription_created_sets_plan_info(self): stripe_subscription_id = "FOEKDCDEQ" stripe_customer_id = "sdo050493" - plan_name = "users-pr-inappy" + plan_name = PlanName.CODECOV_PRO_YEARLY.value quantity = 20 self._send_event( diff --git a/codecov_auth/tests/test_admin.py b/codecov_auth/tests/test_admin.py index 926f290be6..04f7c7fb2b 100644 --- a/codecov_auth/tests/test_admin.py +++ b/codecov_auth/tests/test_admin.py @@ -29,9 +29,9 @@ from shared.plan.constants import ( ENTERPRISE_CLOUD_USER_PLAN_REPRESENTATIONS, PlanName, - TierName, ) +from billing.helpers import mock_all_plans_and_tiers from codecov.commands.exceptions import ValidationError from codecov_auth.admin import ( AccountAdmin, @@ -55,23 +55,13 @@ class OwnerAdminTest(TestCase): def setUp(self): + mock_all_plans_and_tiers() self.staff_user = UserFactory(is_staff=True) self.client.force_login(user=self.staff_user) admin_site = AdminSite() admin_site.register(OrganizationLevelToken) self.owner_admin = OwnerAdmin(Owner, admin_site) - self.basic_tier = TierFactory(tier_name=TierName.BASIC.value) - self.basic_plan = PlanFactory( - tier=self.basic_tier, name=PlanName.BASIC_PLAN_NAME.value - ) - - self.enterprise_tier = TierFactory(tier_name=TierName.ENTERPRISE.value) - self.enterprise_plan = PlanFactory( - tier=self.enterprise_tier, - name=PlanName.ENTERPRISE_CLOUD_YEARLY.value, - ) - def test_owner_admin_detail_page(self): owner = OwnerFactory() response = self.client.get( @@ -81,9 +71,9 @@ def test_owner_admin_detail_page(self): def test_owner_admin_impersonate_owner(self): owner_to_impersonate = OwnerFactory( - service="bitbucket", plan=self.basic_plan.name + service="bitbucket", plan=PlanName.BASIC_PLAN_NAME.value ) - other_owner = OwnerFactory(plan=self.basic_plan.name) + other_owner = OwnerFactory(plan=PlanName.BASIC_PLAN_NAME.value) with self.subTest("more than one user selected"): response = self.client.post( @@ -117,7 +107,7 @@ def test_owner_admin_impersonate_owner(self): @patch("codecov_auth.admin.TaskService.delete_owner") def test_delete_queryset(self, delete_mock): - user_to_delete = OwnerFactory(plan=self.basic_plan.name) + user_to_delete = OwnerFactory(plan=PlanName.BASIC_PLAN_NAME.value) ownerid = user_to_delete.ownerid queryset = MagicMock() queryset.__iter__.return_value = [user_to_delete] @@ -128,14 +118,14 @@ def test_delete_queryset(self, delete_mock): @patch("codecov_auth.admin.TaskService.delete_owner") def test_delete_model(self, delete_mock): - user_to_delete = OwnerFactory(plan=self.basic_plan.name) + user_to_delete = OwnerFactory(plan=PlanName.BASIC_PLAN_NAME.value) ownerid = user_to_delete.ownerid self.owner_admin.delete_model(MagicMock(), user_to_delete) delete_mock.assert_called_once_with(ownerid=ownerid) @patch("codecov_auth.admin.admin.ModelAdmin.get_deleted_objects") def test_confirmation_deleted_objects(self, mocked_deleted_objs): - user_to_delete = OwnerFactory(plan=self.basic_plan.name) + user_to_delete = OwnerFactory(plan=PlanName.BASIC_PLAN_NAME.value) deleted_objs = [ 'Owner: {};'.format( user_to_delete.ownerid, user_to_delete @@ -155,7 +145,7 @@ def test_confirmation_deleted_objects(self, mocked_deleted_objs): @patch("codecov_auth.admin.admin.ModelAdmin.log_change") def test_prev_and_new_values_in_log_entry(self, mocked_super_log_change): - owner = OwnerFactory(staff=True, plan=self.basic_plan.name) + owner = OwnerFactory(staff=True, plan=PlanName.BASIC_PLAN_NAME.value) owner.save() owner.staff = False form = MagicMock() @@ -175,7 +165,7 @@ def test_prev_and_new_values_in_log_entry(self, mocked_super_log_change): ] def test_inline_orgwide_tokens_display(self): - owner = OwnerFactory(plan=self.basic_plan.name) + owner = OwnerFactory(plan=PlanName.BASIC_PLAN_NAME.value) request_url = reverse("admin:codecov_auth_owner_change", args=[owner.ownerid]) request = RequestFactory().get(request_url) request.user = self.staff_user @@ -184,7 +174,7 @@ def test_inline_orgwide_tokens_display(self): assert isinstance(inlines[0], OrgUploadTokenInline) def test_inline_orgwide_permissions(self): - owner_in_cloud_plan = OwnerFactory(plan=self.enterprise_plan.name) + owner_in_cloud_plan = OwnerFactory(plan=PlanName.ENTERPRISE_CLOUD_YEARLY.value) org_token = OrganizationLevelTokenFactory(owner=owner_in_cloud_plan) owner_in_cloud_plan.save() org_token.save() @@ -208,7 +198,7 @@ def test_inline_orgwide_permissions(self): def test_inline_orgwide_add_token_permission_no_token_and_user_in_enterprise_cloud_plan( self, ): - owner = OwnerFactory(plan=self.basic_plan.name) + owner = OwnerFactory(plan=PlanName.BASIC_PLAN_NAME.value) assert owner.plan not in ENTERPRISE_CLOUD_USER_PLAN_REPRESENTATIONS assert OrganizationLevelToken.objects.filter(owner=owner).count() == 0 request_url = reverse("admin:codecov_auth_owner_change", args=[owner.ownerid]) @@ -221,7 +211,7 @@ def test_inline_orgwide_add_token_permission_no_token_and_user_in_enterprise_clo def test_inline_orgwide_add_token_permission_no_token_user_not_in_enterprise_cloud_plan( self, ): - owner_in_cloud_plan = OwnerFactory(plan=self.enterprise_plan.name) + owner_in_cloud_plan = OwnerFactory(plan=PlanName.ENTERPRISE_CLOUD_YEARLY.value) assert ( OrganizationLevelToken.objects.filter(owner=owner_in_cloud_plan).count() == 0 @@ -241,7 +231,7 @@ def test_inline_orgwide_add_token_permission_no_token_user_not_in_enterprise_clo def test_org_token_refresh_request_calls_service_to_refresh_token( self, mock_refresh ): - owner_in_cloud_plan = OwnerFactory(plan=self.enterprise_plan.name) + owner_in_cloud_plan = OwnerFactory(plan=PlanName.ENTERPRISE_CLOUD_YEARLY.value) org_token = OrganizationLevelTokenFactory(owner=owner_in_cloud_plan) owner_in_cloud_plan.save() org_token.save() @@ -278,7 +268,7 @@ def test_org_token_refresh_request_calls_service_to_refresh_token( "codecov_auth.services.org_level_token_service.OrgLevelTokenService.refresh_token" ) def test_org_token_request_doesnt_call_service_to_refresh_token(self, mock_refresh): - owner_in_cloud_plan = OwnerFactory(plan=self.enterprise_plan.name) + owner_in_cloud_plan = OwnerFactory(plan=PlanName.ENTERPRISE_CLOUD_YEARLY.value) org_token = OrganizationLevelTokenFactory(owner=owner_in_cloud_plan) owner_in_cloud_plan.save() org_token.save() @@ -311,7 +301,7 @@ def test_org_token_request_doesnt_call_service_to_refresh_token(self, mock_refre mock_refresh.assert_not_called() def test_start_trial_ui_display(self): - owner = OwnerFactory(plan=self.basic_plan.name) + owner = OwnerFactory(plan=PlanName.BASIC_PLAN_NAME.value) res = self.client.post( reverse("admin:codecov_auth_owner_changelist"), @@ -326,7 +316,7 @@ def test_start_trial_ui_display(self): @patch("shared.plan.service.PlanService.start_trial_manually") def test_start_trial_action(self, mock_start_trial_service): mock_start_trial_service.return_value = None - org_to_be_trialed = OwnerFactory(plan=self.basic_plan.name) + org_to_be_trialed = OwnerFactory(plan=PlanName.BASIC_PLAN_NAME.value) res = self.client.post( reverse("admin:codecov_auth_owner_changelist"), @@ -343,7 +333,7 @@ def test_start_trial_action(self, mock_start_trial_service): @patch("shared.plan.service.PlanService._start_trial_helper") def test_extend_trial_action(self, mock_start_trial_service): mock_start_trial_service.return_value = None - org_to_be_trialed = OwnerFactory(plan=self.basic_plan.name) + org_to_be_trialed = OwnerFactory(plan=PlanName.BASIC_PLAN_NAME.value) org_to_be_trialed.plan = PlanName.TRIAL_PLAN_NAME.value org_to_be_trialed.save() @@ -366,7 +356,7 @@ def test_start_trial_paid_plan(self, mock_start_trial_service): "Cannot trial from a paid plan" ) - org_to_be_trialed = OwnerFactory(plan=self.basic_plan.name) + org_to_be_trialed = OwnerFactory(plan=PlanName.BASIC_PLAN_NAME.value) res = self.client.post( reverse("admin:codecov_auth_owner_changelist"), @@ -381,7 +371,9 @@ def test_start_trial_paid_plan(self, mock_start_trial_service): assert mock_start_trial_service.called def test_account_widget(self): - owner = OwnerFactory(user=UserFactory(), plan=self.enterprise_plan.name) + owner = OwnerFactory( + user=UserFactory(), plan=PlanName.ENTERPRISE_CLOUD_YEARLY.value + ) rf = RequestFactory() get_request = rf.get(f"/admin/codecov_auth/owner/{owner.ownerid}/change/") get_request.user = self.staff_user @@ -913,30 +905,28 @@ def setUp(self): admin_site = AdminSite() admin_site.register(Plan) + self.tier = TierFactory() + self.plan = PlanFactory(name=PlanName.BASIC_PLAN_NAME.value, tier=self.tier) + def test_plan_admin_modal_display(self): - plan = PlanFactory() response = self.client.get( - reverse("admin:codecov_auth_plan_change", args=[plan.pk]) + reverse("admin:codecov_auth_plan_change", args=[self.plan.pk]) ) self.assertEqual(response.status_code, 200) - self.assertContains(response, plan.name) + self.assertContains(response, self.plan.name) def test_plan_modal_tiers_display(self): - tier = TierFactory() - plan = PlanFactory(tier=tier) response = self.client.get( - reverse("admin:codecov_auth_plan_change", args=[plan.pk]) + reverse("admin:codecov_auth_plan_change", args=[self.plan.pk]) ) self.assertEqual(response.status_code, 200) - self.assertContains(response, tier.tier_name) + self.assertContains(response, self.tier.tier_name) def test_add_plans_modal_action(self): - plan = PlanFactory(base_unit_price=10, max_seats=5) - tier = TierFactory() data = { "action": "add_plans", - ACTION_CHECKBOX_NAME: [plan.pk], - "tier_id": tier.pk, + ACTION_CHECKBOX_NAME: [self.plan.pk], + "tier_id": self.tier.pk, } response = self.client.post( reverse("admin:codecov_auth_plan_changelist"), data=data @@ -945,9 +935,8 @@ def test_add_plans_modal_action(self): self.assertEqual(response.url, "/admin/codecov_auth/plan/") def test_plan_change_form(self): - plan = PlanFactory() response = self.client.get( - reverse("admin:codecov_auth_plan_change", args=[plan.pk]) + reverse("admin:codecov_auth_plan_change", args=[self.plan.pk]) ) self.assertEqual(response.status_code, 200) for field in [ @@ -965,49 +954,50 @@ def test_plan_change_form(self): self.assertContains(response, f"id_{field}") def test_plan_change_form_validation(self): - plan = PlanFactory(base_unit_price=-10) + self.plan.base_unit_price = -10 + self.plan.save() response = self.client.post( - reverse("admin:codecov_auth_plan_change", args=[plan.pk]), + reverse("admin:codecov_auth_plan_change", args=[self.plan.pk]), { - "tier": plan.tier_id, - "name": plan.name, - "marketing_name": plan.marketing_name, + "tier": self.tier.pk, + "name": self.plan.name, + "marketing_name": self.plan.marketing_name, "base_unit_price": -10, - "benefits": plan.benefits, - "is_active": plan.is_active, - "paid_plan": plan.paid_plan, + "benefits": self.plan.benefits, + "is_active": self.plan.is_active, + "paid_plan": self.plan.paid_plan, }, ) self.assertEqual(response.status_code, 200) self.assertContains(response, "Base unit price cannot be negative.") response = self.client.post( - reverse("admin:codecov_auth_plan_change", args=[plan.pk]), + reverse("admin:codecov_auth_plan_change", args=[self.plan.pk]), { - "tier": plan.tier_id, - "name": plan.name, - "marketing_name": plan.marketing_name, - "base_unit_price": plan.base_unit_price, - "benefits": plan.benefits, - "is_active": plan.is_active, + "tier": self.tier.pk, + "name": self.plan.name, + "marketing_name": self.plan.marketing_name, + "base_unit_price": self.plan.base_unit_price, + "benefits": self.plan.benefits, + "is_active": self.plan.is_active, "max_seats": -5, - "paid_plan": plan.paid_plan, + "paid_plan": self.plan.paid_plan, }, ) self.assertEqual(response.status_code, 200) self.assertContains(response, "Max seats cannot be negative.") response = self.client.post( - reverse("admin:codecov_auth_plan_change", args=[plan.pk]), + reverse("admin:codecov_auth_plan_change", args=[self.plan.pk]), { - "tier": plan.tier_id, - "name": plan.name, - "marketing_name": plan.marketing_name, - "benefits": plan.benefits, - "is_active": plan.is_active, + "tier": self.tier.pk, + "name": self.plan.name, + "marketing_name": self.plan.marketing_name, + "benefits": self.plan.benefits, + "is_active": self.plan.is_active, "monthly_uploads_limit": -5, - "paid_plan": plan.paid_plan, + "paid_plan": self.plan.paid_plan, }, ) self.assertEqual(response.status_code, 200) @@ -1021,21 +1011,21 @@ def setUp(self): admin_site = AdminSite() admin_site.register(Tier) + self.tier = TierFactory() + self.plan = PlanFactory(name=PlanName.BASIC_PLAN_NAME.value, tier=self.tier) + def test_tier_modal_plans_display(self): - tier = TierFactory() response = self.client.get( - reverse("admin:codecov_auth_tier_change", args=[tier.pk]) + reverse("admin:codecov_auth_tier_change", args=[self.tier.pk]) ) self.assertEqual(response.status_code, 200) - self.assertContains(response, tier.tier_name) + self.assertContains(response, self.tier.tier_name) def test_add_plans_modal_action(self): - tier = TierFactory() - plan = PlanFactory() data = { "action": "add_plans", - ACTION_CHECKBOX_NAME: [plan.pk], - "tier_id": tier.pk, + ACTION_CHECKBOX_NAME: [self.plan.pk], + "tier_id": self.tier.pk, } response = self.client.post( reverse("admin:codecov_auth_tier_changelist"), data=data @@ -1044,9 +1034,8 @@ def test_add_plans_modal_action(self): self.assertEqual(response.url, "/admin/codecov_auth/tier/") def test_tier_change_form(self): - tier = TierFactory() response = self.client.get( - reverse("admin:codecov_auth_tier_change", args=[tier.pk]) + reverse("admin:codecov_auth_tier_change", args=[self.tier.pk]) ) self.assertEqual(response.status_code, 200) for field in [ diff --git a/graphql_api/tests/mutation/test_cancel_trial.py b/graphql_api/tests/mutation/test_cancel_trial.py index 8ceab771e5..cb5d39dc8a 100644 --- a/graphql_api/tests/mutation/test_cancel_trial.py +++ b/graphql_api/tests/mutation/test_cancel_trial.py @@ -1,7 +1,8 @@ from django.test import TransactionTestCase from prometheus_client import REGISTRY +from shared.django_apps.codecov_auth.tests.factories import PlanFactory, TierFactory from shared.django_apps.core.tests.factories import OwnerFactory -from shared.plan.constants import PlanName, TrialStatus +from shared.plan.constants import PlanName, TierName, TrialStatus from graphql_api.tests.helper import GraphQLTestHelper from graphql_api.views import GQL_ERROR_COUNTER, GQL_HIT_COUNTER, GQL_REQUEST_LATENCIES @@ -61,9 +62,9 @@ def test_authenticated(self): labels={"operation_type": "mutation", "operation_name": "CancelTrialInput"}, ) trial_status = TrialStatus.ONGOING.value - owner = OwnerFactory( - trial_status=trial_status, plan=PlanName.TRIAL_PLAN_NAME.value - ) + tier = TierFactory(tier_name=TierName.TRIAL.value) + plan = PlanFactory(name=PlanName.TRIAL_PLAN_NAME.value, tier=tier) + owner = OwnerFactory(trial_status=trial_status, plan=plan.name) owner.save() assert self._request(owner=owner, org_username=owner.username) == { "cancelTrial": None diff --git a/graphql_api/tests/test_owner.py b/graphql_api/tests/test_owner.py index b626a07da6..40b131e131 100644 --- a/graphql_api/tests/test_owner.py +++ b/graphql_api/tests/test_owner.py @@ -23,6 +23,7 @@ from shared.plan.constants import PlanName, TrialStatus from shared.upload.utils import UploaderType, insert_coverage_measurement +from billing.helpers import mock_all_plans_and_tiers from codecov.commands.exceptions import ( MissingService, UnauthorizedGuestAccess, @@ -59,6 +60,7 @@ class TestOwnerType(GraphQLTestHelper, TransactionTestCase): def setUp(self): + mock_all_plans_and_tiers() self.account = AccountFactory() self.owner = OwnerFactory( username="codecov-user", service="github", account=self.account diff --git a/graphql_api/tests/test_plan.py b/graphql_api/tests/test_plan.py index ac136a521d..0c6b6734e1 100644 --- a/graphql_api/tests/test_plan.py +++ b/graphql_api/tests/test_plan.py @@ -11,6 +11,8 @@ from shared.plan.constants import PlanName, TrialStatus from shared.utils.test_utils import mock_config_helper +from billing.helpers import mock_all_plans_and_tiers + from .helper import GraphQLTestHelper @@ -20,6 +22,7 @@ def inject_mocker(request, mocker): request.mocker = mocker def setUp(self): + mock_all_plans_and_tiers() self.current_org = OwnerFactory( username="random-plan-user", service="github", @@ -72,10 +75,10 @@ def test_owner_plan_data_when_trialing(self): "trialStatus": "ONGOING", "trialEndDate": "2023-07-03T00:00:00", "trialStartDate": "2023-06-19T00:00:00", - "trialTotalDays": None, + "trialTotalDays": 14, "marketingName": "Developer", "value": "users-trial", - "tierName": "pro", + "tierName": "trial", "billingRate": None, "baseUnitPrice": 0, "benefits": [ diff --git a/graphql_api/types/owner/owner.py b/graphql_api/types/owner/owner.py index 52310f7e11..8fc15057d1 100644 --- a/graphql_api/types/owner/owner.py +++ b/graphql_api/types/owner/owner.py @@ -105,6 +105,7 @@ def resolve_yaml(owner: Owner, info: GraphQLResolveInfo) -> Optional[str]: @owner_bindable.field("plan") @require_part_of_org +@sync_to_async def resolve_plan(owner: Owner, info: GraphQLResolveInfo) -> PlanService: return PlanService(current_org=owner) diff --git a/graphql_api/types/plan/plan.py b/graphql_api/types/plan/plan.py index b10a84279e..3ad157a7e0 100644 --- a/graphql_api/types/plan/plan.py +++ b/graphql_api/types/plan/plan.py @@ -42,6 +42,7 @@ def resolve_marketing_name(plan_service: PlanService, info) -> str: @plan_bindable.field("value") +@sync_to_async def resolve_plan_name_as_value(plan_service: PlanService, info) -> str: return plan_service.plan_name @@ -71,6 +72,7 @@ def resolve_benefits(plan_service: PlanService, info) -> List[str]: @plan_bindable.field("pretrialUsersCount") +@sync_to_async def resolve_pretrial_users_count(plan_service: PlanService, info) -> Optional[int]: if plan_service.is_org_trialing: return plan_service.pretrial_users_count diff --git a/requirements.in b/requirements.in index 702d7f65a9..53ed3eea05 100644 --- a/requirements.in +++ b/requirements.in @@ -26,7 +26,7 @@ freezegun google-cloud-pubsub gunicorn>=22.0.0 https://github.com/codecov/opentelem-python/archive/refs/tags/v0.0.4a1.tar.gz#egg=codecovopentelem -https://github.com/codecov/shared/archive/3dcd8efa027e6f45e0cbfd2c08a936ec5a68a57c.tar.gz#egg=shared +https://github.com/codecov/shared/archive/906e7bc916e80d89583f2086a7e348c124bf6f72.tar.gz#egg=shared https://github.com/photocrowd/django-cursor-pagination/archive/f560902696b0c8509e4d95c10ba0d62700181d84.tar.gz idna>=3.7 minio diff --git a/requirements.txt b/requirements.txt index 5b82e91188..113e6f630e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -416,7 +416,7 @@ sentry-sdk[celery]==2.13.0 # shared setproctitle==1.1.10 # via -r requirements.in -shared @ https://github.com/codecov/shared/archive/3dcd8efa027e6f45e0cbfd2c08a936ec5a68a57c.tar.gz +shared @ https://github.com/codecov/shared/archive/906e7bc916e80d89583f2086a7e348c124bf6f72.tar.gz # via -r requirements.in simplejson==3.17.2 # via -r requirements.in diff --git a/services/billing.py b/services/billing.py index c8ac2fb55a..caa9a6543e 100644 --- a/services/billing.py +++ b/services/billing.py @@ -479,7 +479,7 @@ def _is_similar_plan(self, owner: Owner, desired_plan: dict) -> bool: ) team_plans = Plan.objects.filter( - tier=TierName.TEAM.value, is_active=True + tier__tier_name=TierName.TEAM.value, is_active=True ).values_list("name", flat=True) # If from PRO to TEAM, then not a similar plan diff --git a/services/tests/test_billing.py b/services/tests/test_billing.py index a2edd6640a..7b431206a3 100644 --- a/services/tests/test_billing.py +++ b/services/tests/test_billing.py @@ -9,6 +9,7 @@ from shared.plan.constants import PlanName from stripe import InvalidRequestError +from billing.helpers import mock_all_plans_and_tiers from codecov_auth.models import Service from services.billing import AbstractPaymentService, BillingService, StripeService @@ -177,6 +178,7 @@ def __getitem__(self, key): class StripeServiceTests(TestCase): def setUp(self): + mock_all_plans_and_tiers() self.user = OwnerFactory() self.stripe = StripeService(requesting_user=self.user) @@ -1873,6 +1875,7 @@ def create_setup_intent(self, owner): class BillingServiceTests(TestCase): def setUp(self): + mock_all_plans_and_tiers() self.mock_payment_service = MockPaymentService() self.billing_service = BillingService(payment_service=self.mock_payment_service) diff --git a/upload/tests/test_helpers.py b/upload/tests/test_helpers.py index e284d3d2ee..0f2e8c1b75 100644 --- a/upload/tests/test_helpers.py +++ b/upload/tests/test_helpers.py @@ -4,7 +4,7 @@ import jwt import pytest from django.conf import settings -from django.test import TransactionTestCase +from django.test import TestCase from rest_framework.exceptions import Throttled, ValidationError from shared.django_apps.core.tests.factories import ( CommitFactory, @@ -15,6 +15,7 @@ from shared.plan.constants import PlanName from shared.upload.utils import UploaderType, insert_coverage_measurement +from billing.helpers import mock_all_plans_and_tiers from codecov_auth.models import GithubAppInstallation, Service from reports.tests.factories import CommitReportFactory, UploadFactory from upload.helpers import ( @@ -27,7 +28,7 @@ ) -class TestGithubAppInstallationUsage(TransactionTestCase): +class TestGithubAppInstallationUsage(TestCase): def test_not_github_provider(self): repo = RepositoryFactory(author__service=Service.GITLAB.value) assert ghapp_installation_id_to_use(repo) is None @@ -169,6 +170,7 @@ def test_check_commit_constraints_settings_disabled(db, settings): def test_check_commit_constraints_settings_enabled(db, settings, mocker): settings.UPLOAD_THROTTLING_ENABLED = True + mock_all_plans_and_tiers() author = OwnerFactory.create(plan=PlanName.BASIC_PLAN_NAME.value) repository = RepositoryFactory.create(author=author, private=True) public_repository = RepositoryFactory.create(author=author, private=False) diff --git a/upload/tests/test_throttles.py b/upload/tests/test_throttles.py index 51c99471a0..818991b5ce 100644 --- a/upload/tests/test_throttles.py +++ b/upload/tests/test_throttles.py @@ -11,6 +11,7 @@ from shared.plan.constants import PlanName from shared.upload.utils import UploaderType, insert_coverage_measurement +from billing.helpers import mock_all_plans_and_tiers from reports.tests.factories import CommitReportFactory, UploadFactory from services.redis_configuration import get_redis_connection from upload.throttles import UploadsPerCommitThrottle, UploadsPerWindowThrottle @@ -18,6 +19,7 @@ class ThrottlesUnitTests(APITestCase): def setUp(self): + mock_all_plans_and_tiers() self.owner = OwnerFactory( plan=PlanName.BASIC_PLAN_NAME.value, max_upload_limit=150 ) diff --git a/upload/tests/views/test_uploads.py b/upload/tests/views/test_uploads.py index ec88613991..e55ea2576d 100644 --- a/upload/tests/views/test_uploads.py +++ b/upload/tests/views/test_uploads.py @@ -7,13 +7,16 @@ from rest_framework.exceptions import ValidationError from rest_framework.test import APIClient, APITestCase from shared.api_archive.archive import ArchiveService, MinioEndpoints +from shared.django_apps.codecov_auth.tests.factories import PlanFactory, TierFactory from shared.django_apps.core.tests.factories import ( CommitFactory, OwnerFactory, RepositoryFactory, ) +from shared.plan.constants import PlanName, TierName from shared.utils.test_utils import mock_config_helper +from billing.helpers import mock_all_plans_and_tiers from codecov_auth.authentication.repo_auth import OrgLevelTokenRepositoryAuth from codecov_auth.services.org_level_token_service import OrgLevelTokenService from reports.models import ( @@ -40,7 +43,9 @@ def test_upload_permission_class_pass(db, mocker): def test_upload_permission_orglevel_token(db, mocker): - owner = OwnerFactory(plan="users-enterprisem") + tier = TierFactory(tier_name=TierName.ENTERPRISE.value) + plan = PlanFactory(name=PlanName.ENTERPRISE_CLOUD_MONTHLY.value, tier=tier) + owner = OwnerFactory(plan=plan.name) owner.save() repo = RepositoryFactory(author=owner) repo.save() @@ -63,7 +68,9 @@ def test_upload_permission_class_fail(db, mocker): def test_upload_permission_orglevel_fail(db, mocker): - owner = OwnerFactory(plan="users-enterprisem") + tier = TierFactory(tier_name=TierName.ENTERPRISE.value) + plan = PlanFactory(name=PlanName.ENTERPRISE_CLOUD_MONTHLY.value, tier=tier) + owner = OwnerFactory(plan=plan.name) owner.save() repo = RepositoryFactory() # Not the same owner of the token repo.save() @@ -78,6 +85,7 @@ def test_upload_permission_orglevel_fail(db, mocker): def test_uploads_get_not_allowed(client, db, mocker): + mock_all_plans_and_tiers() mocker.patch.object( CanDoCoverageUploadsPermission, "has_permission", return_value=True ) @@ -193,6 +201,7 @@ def test_get_report_error(db): def test_uploads_post(db, mocker, mock_redis): + mock_all_plans_and_tiers() # TODO remove the mock object and test the flow with the permissions mocker.patch.object( CanDoCoverageUploadsPermission, "has_permission", return_value=True @@ -712,6 +721,7 @@ def test_uploads_post_github_oidc_auth( @override_settings(SHELTER_SHARED_SECRET="shelter-shared-secret") def test_uploads_post_shelter(db, mocker, mock_redis): + mock_all_plans_and_tiers() mocker.patch.object( CanDoCoverageUploadsPermission, "has_permission", return_value=True ) @@ -781,6 +791,7 @@ def test_uploads_post_shelter(db, mocker, mock_redis): def test_deactivated_repo(db, mocker, mock_redis): + mock_all_plans_and_tiers() mocker.patch.object( CanDoCoverageUploadsPermission, "has_permission", return_value=True ) From 1bfc6d88f2aeff323ff00a0c9604f0380c49e902 Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Thu, 23 Jan 2025 13:58:49 -0800 Subject: [PATCH 13/19] all tests passing now, fixed a couple small logic bugs too --- api/internal/owner/serializers.py | 12 +-- ..._fail_if_team_plan_and_too_many_users.yaml | 92 ++++++++++++++++++ ...urrently_team_plan_add_too_many_users.yaml | 95 +++++++++++++++++++ .../tests/views/test_account_viewset.py | 43 +++------ api/internal/tests/views/test_user_viewset.py | 6 +- billing/helpers.py | 24 +++-- billing/tests/test_helpers.py | 36 +++---- billing/tests/test_views.py | 6 +- codecov_auth/tests/test_admin.py | 6 +- graphql_api/tests/test_owner.py | 6 +- graphql_api/types/owner/owner.py | 4 +- requirements.in | 2 +- requirements.txt | 2 +- services/tests/test_billing.py | 12 ++- upload/tests/test_throttles.py | 6 +- 15 files changed, 277 insertions(+), 75 deletions(-) create mode 100644 api/internal/tests/views/cassetes/test_account_viewset/AccountViewSetTests/test_update_must_fail_if_team_plan_and_too_many_users.yaml create mode 100644 api/internal/tests/views/cassetes/test_account_viewset/AccountViewSetTests/test_update_team_plan_must_fail_if_currently_team_plan_add_too_many_users.yaml diff --git a/api/internal/owner/serializers.py b/api/internal/owner/serializers.py index 2702342c56..ac7a48189b 100644 --- a/api/internal/owner/serializers.py +++ b/api/internal/owner/serializers.py @@ -148,13 +148,13 @@ def validate(self, plan: Dict[str, Any]) -> Dict[str, Any]: ) active_plans = list( - Plan.objects.filter(paid_plan=True, is_active=True).values_list( - "name", "tier" - ) + Plan.objects.select_related("tier").filter(paid_plan=True, is_active=True) ) - active_plan_names = {name for name, _ in active_plans} + active_plan_names = {plan.name for plan in active_plans} team_tier_plans = { - name for name, tier in active_plans if tier == TierName.TEAM.value + plan.name + for plan in active_plans + if plan.tier.tier_name == TierName.TEAM.value } # Validate quantity here because we need access to whole plan object @@ -347,7 +347,7 @@ def update(self, instance: Owner, validated_data: Dict[str, Any]) -> object: sentry_plans = Plan.objects.filter( tier__tier_name=TierName.SENTRY.value, is_active=True - ) + ).values_list("name", flat=True) if desired_plan["value"] in sentry_plans: current_owner = self.context["view"].request.current_owner diff --git a/api/internal/tests/views/cassetes/test_account_viewset/AccountViewSetTests/test_update_must_fail_if_team_plan_and_too_many_users.yaml b/api/internal/tests/views/cassetes/test_account_viewset/AccountViewSetTests/test_update_must_fail_if_team_plan_and_too_many_users.yaml new file mode 100644 index 0000000000..2ee7c35712 --- /dev/null +++ b/api/internal/tests/views/cassetes/test_account_viewset/AccountViewSetTests/test_update_must_fail_if_team_plan_and_too_many_users.yaml @@ -0,0 +1,92 @@ +interactions: +- request: + body: billing_address_collection=required&payment_method_collection=if_required&client_reference_id=65&success_url=http%3A%2F%2Flocalhost%3A3000%2Fplan%2Fgh%2Fjustin47%3Fsuccess&cancel_url=http%3A%2F%2Flocalhost%3A3000%2Fplan%2Fgh%2Fjustin47%3Fcancel&customer=1000&mode=subscription&line_items[0][price]=price_1OCM0gGlVGuVgOrkWDYEBtSL&line_items[0][quantity]=11&subscription_data[metadata][service]=github&subscription_data[metadata][obo_organization]=65&subscription_data[metadata][username]=justin47&subscription_data[metadata][obo_name]=Kelly+Williams&subscription_data[metadata][obo_email]=christopher27%40lopez-welch.com&subscription_data[metadata][obo]=65&tax_id_collection[enabled]=True&customer_update[name]=auto&customer_update[address]=auto + headers: + Accept: + - '*/*' + Accept-Encoding: + - gzip, deflate + Connection: + - keep-alive + Content-Length: + - '744' + Content-Type: + - application/x-www-form-urlencoded + Idempotency-Key: + - 2e7e12e1-9051-4766-b947-1abe985b5e98 + Stripe-Version: + - 2024-12-18.acacia + User-Agent: + - Stripe/v1 PythonBindings/11.4.1 + X-Stripe-Client-User-Agent: + - '{"bindings_version": "11.4.1", "lang": "python", "publisher": "stripe", "httplib": + "requests", "lang_version": "3.12.8", "platform": "Linux-6.10.14-linuxkit-aarch64-with-glibc2.36", + "uname": "Linux 35c9e7c77efc 6.10.14-linuxkit #1 SMP Fri Nov 29 17:22:03 UTC + 2024 aarch64 "}' + method: POST + uri: https://api.stripe.com/v1/checkout/sessions + response: + body: + string: "{\n \"error\": {\n \"code\": \"resource_missing\",\n \"doc_url\": + \"https://stripe.com/docs/error-codes/resource-missing\",\n \"message\": + \"No such customer: '1000'\",\n \"param\": \"customer\",\n \"request_log_url\": + \"https://dashboard.stripe.com/test/logs/req_oevRZUbMiaT1kM?t=1737668618\",\n + \ \"type\": \"invalid_request_error\"\n }\n}\n" + headers: + Access-Control-Allow-Credentials: + - 'true' + Access-Control-Allow-Methods: + - GET, HEAD, PUT, PATCH, POST, DELETE + Access-Control-Allow-Origin: + - '*' + Access-Control-Expose-Headers: + - Request-Id, Stripe-Manage-Version, Stripe-Should-Retry, X-Stripe-External-Auth-Required, + X-Stripe-Privileged-Session-Required + Access-Control-Max-Age: + - '300' + Cache-Control: + - no-cache, no-store + Connection: + - keep-alive + Content-Length: + - '325' + Content-Security-Policy: + - base-uri 'none'; default-src 'none'; form-action 'none'; frame-ancestors 'none'; + img-src 'self'; script-src 'self' 'report-sample'; style-src 'self'; upgrade-insecure-requests; + report-uri https://q.stripe.com/csp-violation?q=ieXMYrsoTw4hsNfwL6RhDPN6zQnVwnAAc0QsFb8i8xtl4N7V94VZzDmgDhKzWxW8mHRRg08-d5GW6oHr + Content-Type: + - application/json + Cross-Origin-Opener-Policy-Report-Only: + - same-origin; report-to="coop" + Date: + - Thu, 23 Jan 2025 21:43:38 GMT + Idempotency-Key: + - 2e7e12e1-9051-4766-b947-1abe985b5e98 + Original-Request: + - req_oevRZUbMiaT1kM + Report-To: + - '{"group":"coop","max_age":8640,"endpoints":[{"url":"https://q.stripe.com/coop-report"}],"include_subdomains":true}' + Reporting-Endpoints: + - coop="https://q.stripe.com/coop-report" + Request-Id: + - req_oevRZUbMiaT1kM + Server: + - nginx + Strict-Transport-Security: + - max-age=63072000; includeSubDomains; preload + Stripe-Version: + - 2024-12-18.acacia + Vary: + - Origin + X-Content-Type-Options: + - nosniff + X-Stripe-Priority-Routing-Enabled: + - 'true' + X-Stripe-Routing-Context-Priority-Tier: + - api-testmode + X-Wc: + - AB + status: + code: 400 + message: Bad Request +version: 1 diff --git a/api/internal/tests/views/cassetes/test_account_viewset/AccountViewSetTests/test_update_team_plan_must_fail_if_currently_team_plan_add_too_many_users.yaml b/api/internal/tests/views/cassetes/test_account_viewset/AccountViewSetTests/test_update_team_plan_must_fail_if_currently_team_plan_add_too_many_users.yaml new file mode 100644 index 0000000000..f8c6ccc4d2 --- /dev/null +++ b/api/internal/tests/views/cassetes/test_account_viewset/AccountViewSetTests/test_update_team_plan_must_fail_if_currently_team_plan_add_too_many_users.yaml @@ -0,0 +1,95 @@ +interactions: +- request: + body: billing_address_collection=required&payment_method_collection=if_required&client_reference_id=93&success_url=http%3A%2F%2Flocalhost%3A3000%2Fplan%2Fgh%2Fjocelyn62%3Fsuccess&cancel_url=http%3A%2F%2Flocalhost%3A3000%2Fplan%2Fgh%2Fjocelyn62%3Fcancel&customer=1000&mode=subscription&line_items[0][price]=price_1OCM0gGlVGuVgOrkWDYEBtSL&line_items[0][quantity]=11&subscription_data[metadata][service]=github&subscription_data[metadata][obo_organization]=93&subscription_data[metadata][username]=jocelyn62&subscription_data[metadata][obo_name]=Crystal+Schmitt&subscription_data[metadata][obo_email]=smithamanda%40flowers.biz&subscription_data[metadata][obo]=93&tax_id_collection[enabled]=True&customer_update[name]=auto&customer_update[address]=auto + headers: + Accept: + - '*/*' + Accept-Encoding: + - gzip, deflate + Connection: + - keep-alive + Content-Length: + - '742' + Content-Type: + - application/x-www-form-urlencoded + Idempotency-Key: + - 26000f5a-feb8-4647-ac57-32a5e5ff8729 + Stripe-Version: + - 2024-12-18.acacia + User-Agent: + - Stripe/v1 PythonBindings/11.4.1 + X-Stripe-Client-Telemetry: + - '{"last_request_metrics": {"request_id": "req_AaY8IvHbbSDcvz", "request_duration_ms": + 2}}' + X-Stripe-Client-User-Agent: + - '{"bindings_version": "11.4.1", "lang": "python", "publisher": "stripe", "httplib": + "requests", "lang_version": "3.12.8", "platform": "Linux-6.10.14-linuxkit-aarch64-with-glibc2.36", + "uname": "Linux 35c9e7c77efc 6.10.14-linuxkit #1 SMP Fri Nov 29 17:22:03 UTC + 2024 aarch64 "}' + method: POST + uri: https://api.stripe.com/v1/checkout/sessions + response: + body: + string: "{\n \"error\": {\n \"code\": \"resource_missing\",\n \"doc_url\": + \"https://stripe.com/docs/error-codes/resource-missing\",\n \"message\": + \"No such customer: '1000'\",\n \"param\": \"customer\",\n \"request_log_url\": + \"https://dashboard.stripe.com/test/logs/req_k8lY68XdxWIFHo?t=1737668619\",\n + \ \"type\": \"invalid_request_error\"\n }\n}\n" + headers: + Access-Control-Allow-Credentials: + - 'true' + Access-Control-Allow-Methods: + - GET, HEAD, PUT, PATCH, POST, DELETE + Access-Control-Allow-Origin: + - '*' + Access-Control-Expose-Headers: + - Request-Id, Stripe-Manage-Version, Stripe-Should-Retry, X-Stripe-External-Auth-Required, + X-Stripe-Privileged-Session-Required + Access-Control-Max-Age: + - '300' + Cache-Control: + - no-cache, no-store + Connection: + - keep-alive + Content-Length: + - '325' + Content-Security-Policy: + - base-uri 'none'; default-src 'none'; form-action 'none'; frame-ancestors 'none'; + img-src 'self'; script-src 'self' 'report-sample'; style-src 'self'; upgrade-insecure-requests; + report-uri https://q.stripe.com/csp-violation?q=1vgSiZ6UPd0qoBXo1Mjsk02GXFGP3M7PXsjua2jiowWQKm8jByxTHbhTeRirsQsrZ7jscQLjXtdCc_sh + Content-Type: + - application/json + Cross-Origin-Opener-Policy-Report-Only: + - same-origin; report-to="coop" + Date: + - Thu, 23 Jan 2025 21:43:39 GMT + Idempotency-Key: + - 26000f5a-feb8-4647-ac57-32a5e5ff8729 + Original-Request: + - req_k8lY68XdxWIFHo + Report-To: + - '{"group":"coop","max_age":8640,"endpoints":[{"url":"https://q.stripe.com/coop-report"}],"include_subdomains":true}' + Reporting-Endpoints: + - coop="https://q.stripe.com/coop-report" + Request-Id: + - req_k8lY68XdxWIFHo + Server: + - nginx + Strict-Transport-Security: + - max-age=63072000; includeSubDomains; preload + Stripe-Version: + - 2024-12-18.acacia + Vary: + - Origin + X-Content-Type-Options: + - nosniff + X-Stripe-Priority-Routing-Enabled: + - 'true' + X-Stripe-Routing-Context-Priority-Tier: + - api-testmode + X-Wc: + - AB + status: + code: 400 + message: Bad Request +version: 1 diff --git a/api/internal/tests/views/test_account_viewset.py b/api/internal/tests/views/test_account_viewset.py index 31196a57e9..f686da909f 100644 --- a/api/internal/tests/views/test_account_viewset.py +++ b/api/internal/tests/views/test_account_viewset.py @@ -12,14 +12,13 @@ AccountFactory, InvoiceBillingFactory, OwnerFactory, - PlanFactory, - TierFactory, UserFactory, ) -from shared.plan.constants import PlanName, TierName, TrialStatus +from shared.plan.constants import PlanName, TrialStatus from stripe import StripeError from api.internal.tests.test_utils import GetAdminProviderAdapter +from billing.helpers import mock_all_plans_and_tiers from codecov_auth.models import Service from utils.test_utils import APIClient @@ -93,6 +92,11 @@ def _update(self, kwargs, data): def _destroy(self, kwargs): return self.client.delete(reverse("account_details-detail", kwargs=kwargs)) + @classmethod + def setUpClass(cls): + super().setUpClass() + mock_all_plans_and_tiers() + def setUp(self): self.service = "gitlab" self.current_owner = OwnerFactory( @@ -189,15 +193,6 @@ def test_retrieve_account_gets_account_fields(self): def test_retrieve_account_gets_account_fields_when_there_are_scheduled_details( self, mock_retrieve_subscription, mock_retrieve_schedule ): - pro_tier = TierFactory(tier_name=TierName.PRO.value) - - PlanFactory( - name="users-pr-inappm", - tier=pro_tier, - is_active=True, - billing_rate="monthly", - marketing_name="Pro", - ) owner = OwnerFactory( admins=[self.current_owner.ownerid], stripe_subscription_id="sub_123" ) @@ -294,16 +289,6 @@ def test_retrieve_account_gets_account_fields_when_there_are_scheduled_details( def test_retrieve_account_returns_last_phase_when_more_than_one_scheduled_phases( self, mock_retrieve_subscription, mock_retrieve_schedule ): - pro_tier = TierFactory(tier_name=TierName.PRO.value) - - PlanFactory( - name="users-pr-inappm", - tier=pro_tier, - is_active=True, - billing_rate="monthly", - marketing_name="Pro", - ) - owner = OwnerFactory( admins=[self.current_owner.ownerid], stripe_subscription_id="sub_2345687" ) @@ -541,13 +526,13 @@ def test_account_with_paid_user_plan_billed_monthly(self): } def test_account_with_paid_user_plan_billed_annually(self): - self.current_owner.plan = PlanName.CODECOV_PRO_YEARLY_LEGACY.value + self.current_owner.plan = PlanName.CODECOV_PRO_YEARLY.value self.current_owner.save() response = self._retrieve() assert response.status_code == status.HTTP_200_OK assert response.data["plan"] == { "marketing_name": "Pro", - "value": PlanName.CODECOV_PRO_YEARLY_LEGACY.value, + "value": PlanName.CODECOV_PRO_YEARLY.value, "billing_rate": "annually", "base_unit_price": 10, "benefits": [ @@ -707,7 +692,7 @@ def test_update_can_set_plan_auto_activate_on_org_with_account(self): assert response.data["plan_auto_activate"] is False def test_update_can_set_plan_to_users_basic(self): - self.current_owner.plan = PlanName.CODECOV_PRO_YEARLY_LEGACY.value + self.current_owner.plan = PlanName.CODECOV_PRO_YEARLY.value self.current_owner.save() response = self._update( @@ -1518,7 +1503,7 @@ def test_update_sentry_plan_monthly_with_users_org( @patch("api.internal.owner.serializers.send_sentry_webhook") @patch("services.billing.StripeService.modify_subscription") def test_update_sentry_plan_annual(self, modify_sub_mock, send_sentry_webhook): - desired_plan = {"value": "users-sentryy", "quantity": 12} + desired_plan = {"value": PlanName.SENTRY_YEARLY.value, "quantity": 12} self.current_owner.stripe_customer_id = "flsoe" self.current_owner.stripe_subscription_id = "djfos" self.current_owner.sentry_user_id = "sentry-user-id" @@ -1540,7 +1525,7 @@ def test_update_sentry_plan_annual(self, modify_sub_mock, send_sentry_webhook): def test_update_sentry_plan_annual_with_users_org( self, modify_sub_mock, send_sentry_webhook ): - desired_plan = {"value": "users-sentryy", "quantity": 12} + desired_plan = {"value": PlanName.SENTRY_YEARLY.value, "quantity": 12} org = OwnerFactory( service=Service.GITHUB.value, service_id="923836740", @@ -1648,7 +1633,7 @@ def test_update_apply_cancellation_discount_yearly( ): coupon_create_mock.return_value = MagicMock(id="test-coupon-id") - self.current_owner.plan = PlanName.CODECOV_PRO_YEARLY_LEGACY.value + self.current_owner.plan = PlanName.CODECOV_PRO_YEARLY.value self.current_owner.stripe_customer_id = "flsoe" self.current_owner.stripe_subscription_id = "djfos" self.current_owner.save() @@ -1701,7 +1686,7 @@ def test_retrieve_org_with_account(self): name="Hello World", plan_seat_count=5, free_seat_count=3, - plan="users-enterprisey", + plan=PlanName.ENTERPRISE_CLOUD_YEARLY.value, is_delinquent=False, ) InvoiceBillingFactory(is_active=True, account=account) diff --git a/api/internal/tests/views/test_user_viewset.py b/api/internal/tests/views/test_user_viewset.py index f62bb78310..3ce2c3a89f 100644 --- a/api/internal/tests/views/test_user_viewset.py +++ b/api/internal/tests/views/test_user_viewset.py @@ -4,11 +4,13 @@ from rest_framework import status from rest_framework.reverse import reverse from rest_framework.test import APITestCase +from shared.django_apps.codecov_auth.tests.factories import PlanFactory, TierFactory from shared.django_apps.core.tests.factories import ( OwnerFactory, PullFactory, RepositoryFactory, ) +from shared.plan.constants import PlanName, TierName from core.models import Pull from utils.test_utils import APIClient @@ -17,8 +19,10 @@ class UserViewSetTests(APITestCase): def setUp(self): non_org_active_user = OwnerFactory() + tier = TierFactory(tier_name=TierName.BASIC.value) + plan = PlanFactory(name=PlanName.BASIC_PLAN_NAME.value, tier=tier) self.current_owner = OwnerFactory( - plan="users-free", + plan=plan.name, plan_user_count=5, plan_activated_users=[non_org_active_user.ownerid], ) diff --git a/billing/helpers.py b/billing/helpers.py index 3abfd9e583..2bccb691ce 100644 --- a/billing/helpers.py +++ b/billing/helpers.py @@ -8,12 +8,8 @@ def on_enterprise_plan(owner: Owner) -> bool: - return settings.IS_ENTERPRISE or ( - owner.plan - in Plan.objects.filter(tier__tier_name=TierName.ENTERPRISE.value).values_list( - "name", flat=True - ) - ) + plan = Plan.objects.select_related("tier").get(name=owner.plan) + return settings.IS_ENTERPRISE or (plan.tier.tier_name == TierName.ENTERPRISE.value) def get_all_admins_for_owners(owners: QuerySet[Owner]): @@ -166,16 +162,28 @@ def mock_all_plans_and_tiers(): PlanFactory( name=PlanName.ENTERPRISE_CLOUD_MONTHLY.value, tier=enterprise_tier, - marketing_name="Enterprise", + marketing_name="Enterprise Cloud", billing_rate=BillingRate.MONTHLY.value, base_unit_price=PlanPrice.MONTHLY.value, paid_plan=True, + benefits=[ + "Configurable # of users", + "Unlimited public repositories", + "Unlimited private repositories", + "Priority Support", + ], ) 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/billing/tests/test_helpers.py b/billing/tests/test_helpers.py index dc255c5c29..f862b782e1 100644 --- a/billing/tests/test_helpers.py +++ b/billing/tests/test_helpers.py @@ -1,34 +1,28 @@ from django.test import TestCase, override_settings -from shared.django_apps.codecov_auth.tests.factories import PlanFactory, TierFactory -from shared.django_apps.core.tests.factories import OwnerFactory -from shared.plan.constants import ( - ENTERPRISE_CLOUD_USER_PLAN_REPRESENTATIONS, - PlanName, - TierName, -) +from shared.django_apps.codecov_auth.tests.factories import OwnerFactory +from shared.plan.constants import PlanName -from billing.helpers import on_enterprise_plan +from billing.helpers import mock_all_plans_and_tiers, on_enterprise_plan class HelpersTestCase(TestCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + mock_all_plans_and_tiers() + @override_settings(IS_ENTERPRISE=True) def test_on_enterprise_plan_on_prem(self): owner = OwnerFactory() assert on_enterprise_plan(owner) == True - def test_on_enterprise_plan_enterpise_cloud(self): - enterprise_tier = TierFactory(tier_name=TierName.ENTERPRISE.value) - PlanFactory( - name=PlanName.ENTERPRISE_CLOUD_MONTHLY.value, - tier=enterprise_tier, - is_active=True, - ) - PlanFactory( - name=PlanName.ENTERPRISE_CLOUD_YEARLY.value, - tier=enterprise_tier, - is_active=True, - ) - for plan in ENTERPRISE_CLOUD_USER_PLAN_REPRESENTATIONS.keys(): + def test_on_enterprise_plan_enterprise_cloud(self): + plan_names = [ + PlanName.ENTERPRISE_CLOUD_MONTHLY.value, + PlanName.ENTERPRISE_CLOUD_YEARLY.value, + ] + + for plan in plan_names: owner = OwnerFactory(plan=plan) assert on_enterprise_plan(owner) == True diff --git a/billing/tests/test_views.py b/billing/tests/test_views.py index 19e40b448a..ce05c6e911 100644 --- a/billing/tests/test_views.py +++ b/billing/tests/test_views.py @@ -71,8 +71,12 @@ def __getitem__(self, key): class StripeWebhookHandlerTests(APITestCase): - def setUp(self): + @classmethod + def setUpClass(cls): + super().setUpClass() mock_all_plans_and_tiers() + + def setUp(self): self.owner = OwnerFactory( stripe_customer_id="cus_123", stripe_subscription_id="sub_123", diff --git a/codecov_auth/tests/test_admin.py b/codecov_auth/tests/test_admin.py index 04f7c7fb2b..ec40c11f0d 100644 --- a/codecov_auth/tests/test_admin.py +++ b/codecov_auth/tests/test_admin.py @@ -54,8 +54,12 @@ class OwnerAdminTest(TestCase): - def setUp(self): + @classmethod + def setUpClass(cls): + super().setUpClass() mock_all_plans_and_tiers() + + def setUp(self): self.staff_user = UserFactory(is_staff=True) self.client.force_login(user=self.staff_user) admin_site = AdminSite() diff --git a/graphql_api/tests/test_owner.py b/graphql_api/tests/test_owner.py index 40b131e131..76ea0ea4be 100644 --- a/graphql_api/tests/test_owner.py +++ b/graphql_api/tests/test_owner.py @@ -59,8 +59,12 @@ class TestOwnerType(GraphQLTestHelper, TransactionTestCase): - def setUp(self): + @classmethod + def setUpClass(cls): + super().setUpClass() mock_all_plans_and_tiers() + + def setUp(self): self.account = AccountFactory() self.owner = OwnerFactory( username="codecov-user", service="github", account=self.account diff --git a/graphql_api/types/owner/owner.py b/graphql_api/types/owner/owner.py index 8fc15057d1..4ce4665327 100644 --- a/graphql_api/types/owner/owner.py +++ b/graphql_api/types/owner/owner.py @@ -115,7 +115,9 @@ def resolve_plan(owner: Owner, info: GraphQLResolveInfo) -> PlanService: @sync_to_async def resolve_plan_representation(owner: Owner, info: GraphQLResolveInfo) -> PlanData: info.context["plan_service"] = PlanService(current_org=owner) - free_plan = Plan.objects.get(name=PlanName.BASIC_PLAN_NAME.value) + free_plan = Plan.objects.select_related("tier").get( + name=PlanName.BASIC_PLAN_NAME.value + ) return convert_to_DTO(free_plan) diff --git a/requirements.in b/requirements.in index 53ed3eea05..612f26d281 100644 --- a/requirements.in +++ b/requirements.in @@ -26,7 +26,7 @@ freezegun google-cloud-pubsub gunicorn>=22.0.0 https://github.com/codecov/opentelem-python/archive/refs/tags/v0.0.4a1.tar.gz#egg=codecovopentelem -https://github.com/codecov/shared/archive/906e7bc916e80d89583f2086a7e348c124bf6f72.tar.gz#egg=shared +https://github.com/codecov/shared/archive/8277f6d8879a05c6506e103786cab67e324dc1d3.tar.gz#egg=shared https://github.com/photocrowd/django-cursor-pagination/archive/f560902696b0c8509e4d95c10ba0d62700181d84.tar.gz idna>=3.7 minio diff --git a/requirements.txt b/requirements.txt index 113e6f630e..dd087fcd3d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -416,7 +416,7 @@ sentry-sdk[celery]==2.13.0 # shared setproctitle==1.1.10 # via -r requirements.in -shared @ https://github.com/codecov/shared/archive/906e7bc916e80d89583f2086a7e348c124bf6f72.tar.gz +shared @ https://github.com/codecov/shared/archive/8277f6d8879a05c6506e103786cab67e324dc1d3.tar.gz # via -r requirements.in simplejson==3.17.2 # via -r requirements.in diff --git a/services/tests/test_billing.py b/services/tests/test_billing.py index 7b431206a3..bbb1e8fdbe 100644 --- a/services/tests/test_billing.py +++ b/services/tests/test_billing.py @@ -177,8 +177,12 @@ def __getitem__(self, key): class StripeServiceTests(TestCase): - def setUp(self): + @classmethod + def setUpClass(cls): + super().setUpClass() mock_all_plans_and_tiers() + + def setUp(self): self.user = OwnerFactory() self.stripe = StripeService(requesting_user=self.user) @@ -1874,8 +1878,12 @@ def create_setup_intent(self, owner): class BillingServiceTests(TestCase): - def setUp(self): + @classmethod + def setUpClass(cls): + super().setUpClass() mock_all_plans_and_tiers() + + def setUp(self): self.mock_payment_service = MockPaymentService() self.billing_service = BillingService(payment_service=self.mock_payment_service) diff --git a/upload/tests/test_throttles.py b/upload/tests/test_throttles.py index 818991b5ce..5dbeac5c63 100644 --- a/upload/tests/test_throttles.py +++ b/upload/tests/test_throttles.py @@ -18,9 +18,11 @@ class ThrottlesUnitTests(APITestCase): - def setUp(self): + @classmethod + def setUpClass(cls): + super().setUpClass() mock_all_plans_and_tiers() - self.owner = OwnerFactory( + cls.owner = OwnerFactory( plan=PlanName.BASIC_PLAN_NAME.value, max_upload_limit=150 ) From 99a63e113c944d6d3ed2639c12ccf2f7be579da6 Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Thu, 23 Jan 2025 14:14:40 -0800 Subject: [PATCH 14/19] fix this suite, when using ttestcase need to have setup vs setupClass --- graphql_api/tests/test_owner.py | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/graphql_api/tests/test_owner.py b/graphql_api/tests/test_owner.py index 76ea0ea4be..7a1546bdc5 100644 --- a/graphql_api/tests/test_owner.py +++ b/graphql_api/tests/test_owner.py @@ -59,12 +59,8 @@ class TestOwnerType(GraphQLTestHelper, TransactionTestCase): - @classmethod - def setUpClass(cls): - super().setUpClass() - mock_all_plans_and_tiers() - def setUp(self): + mock_all_plans_and_tiers() self.account = AccountFactory() self.owner = OwnerFactory( username="codecov-user", service="github", account=self.account @@ -1130,7 +1126,7 @@ def test_fetch_available_plans_is_enterprise_plan(self): current_org = OwnerFactory( username="random-plan-user", service="github", - plan=PlanName.FREE_PLAN_NAME.value, + plan=PlanName.BASIC_PLAN_NAME.value, ) query = """{ @@ -1160,15 +1156,6 @@ def test_fetch_available_plans_is_enterprise_plan(self): "isFreePlan": True, "isTrialPlan": False, }, - { - "value": "users-free", - "isEnterprisePlan": False, - "isProPlan": False, - "isTeamPlan": False, - "isSentryPlan": False, - "isFreePlan": True, - "isTrialPlan": False, - }, { "value": "users-pr-inappm", "isEnterprisePlan": False, From 49f2af828b876192c61da1af1a7a5ec7faf20be2 Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Thu, 23 Jan 2025 15:16:40 -0800 Subject: [PATCH 15/19] clean up the logic for update plan --- .../tests/views/test_account_viewset.py | 15 +++++----- services/billing.py | 28 +++++++++++-------- 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/api/internal/tests/views/test_account_viewset.py b/api/internal/tests/views/test_account_viewset.py index f686da909f..f9d608e7a7 100644 --- a/api/internal/tests/views/test_account_viewset.py +++ b/api/internal/tests/views/test_account_viewset.py @@ -126,7 +126,7 @@ def setUp(self): "description": "(10) users-pr-inappm", "amount": 120, "currency": "usd", - "plan_name": "users-pr-inappm", + "plan_name": PlanName.CODECOV_PRO_MONTHLY.value, "quantity": 1, "period": {"end": 1521326190, "start": 1518906990}, } @@ -235,7 +235,6 @@ def test_retrieve_account_gets_account_fields_when_there_are_scheduled_details( kwargs={"service": owner.service, "owner_username": owner.username} ) assert response.status_code == status.HTTP_200_OK - print(response.data) assert response.data == { "integration_id": owner.integration_id, "activated_student_count": 0, @@ -250,7 +249,7 @@ def test_retrieve_account_gets_account_fields_when_there_are_scheduled_details( "plan_provider": owner.plan_provider, "plan": { "marketing_name": "Developer", - "value": "users-basic", + "value": PlanName.BASIC_PLAN_NAME.value, "billing_rate": None, "base_unit_price": 0, "benefits": [ @@ -489,13 +488,13 @@ def test_retrieve_account_gets_account_students(self): } def test_account_with_free_user_plan(self): - self.current_owner.plan = "users-free" + self.current_owner.plan = PlanName.BASIC_PLAN_NAME.value self.current_owner.save() response = self._retrieve() assert response.status_code == status.HTTP_200_OK assert response.data["plan"] == { "marketing_name": "Developer", - "value": "users-free", + "value": PlanName.BASIC_PLAN_NAME.value, "billing_rate": None, "base_unit_price": 0, "benefits": [ @@ -507,13 +506,13 @@ def test_account_with_free_user_plan(self): } def test_account_with_paid_user_plan_billed_monthly(self): - self.current_owner.plan = "users-pr-inappm" + self.current_owner.plan = PlanName.CODECOV_PRO_MONTHLY.value self.current_owner.save() response = self._retrieve() assert response.status_code == status.HTTP_200_OK assert response.data["plan"] == { "marketing_name": "Pro", - "value": "users-pr-inappm", + "value": PlanName.CODECOV_PRO_MONTHLY.value, "billing_rate": "monthly", "base_unit_price": 12, "benefits": [ @@ -1581,7 +1580,7 @@ def test_update_apply_cancellation_discount( ): coupon_create_mock.return_value = MagicMock(id="test-coupon-id") - self.current_owner.plan = "users-pr-inappm" + self.current_owner.plan = PlanName.CODECOV_PRO_MONTHLY.value self.current_owner.stripe_customer_id = "flsoe" self.current_owner.stripe_subscription_id = "djfos" self.current_owner.save() diff --git a/services/billing.py b/services/billing.py index caa9a6543e..f3991222bb 100644 --- a/services/billing.py +++ b/services/billing.py @@ -795,26 +795,32 @@ def update_plan(self, owner, desired_plan): on current state, might create a stripe checkout session and return the checkout session's ID, which is a string. Otherwise returns None. """ - if desired_plan["value"] in Plan.objects.filter(paid_plan=False).values_list( - "name", flat=True - ): + try: + plan = Plan.objects.get(name=desired_plan["value"]) + except Plan.DoesNotExist: + log.warning( + f"Unable to find plan {desired_plan['value']} for owner {owner.ownerid}" + ) + return None + + if not plan.is_active: + log.warning( + f"Attempted to transition to non-existent or legacy plan: " + f"owner {owner.ownerid}, plan: {desired_plan}" + ) + return None + + if plan.paid_plan is False: if owner.stripe_subscription_id is not None: self.payment_service.delete_subscription(owner) else: plan_service = PlanService(current_org=owner) plan_service.set_default_plan_data() - elif desired_plan["value"] in Plan.objects.filter( - paid_plan=True, is_active=True - ).values_list("name", flat=True): + else: if owner.stripe_subscription_id is not None: self.payment_service.modify_subscription(owner, desired_plan) else: return self.payment_service.create_checkout_session(owner, desired_plan) - else: - log.warning( - f"Attempted to transition to non-existent or legacy plan: " - f"owner {owner.ownerid}, plan: {desired_plan}" - ) def update_payment_method(self, owner, payment_method): """ From 34b16776ca4d0d0413e666d2e3af83836831ccb9 Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Mon, 27 Jan 2025 11:23:35 -0800 Subject: [PATCH 16/19] rebase --- requirements.in | 2 +- requirements.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements.in b/requirements.in index 612f26d281..d720623d62 100644 --- a/requirements.in +++ b/requirements.in @@ -26,7 +26,7 @@ freezegun google-cloud-pubsub gunicorn>=22.0.0 https://github.com/codecov/opentelem-python/archive/refs/tags/v0.0.4a1.tar.gz#egg=codecovopentelem -https://github.com/codecov/shared/archive/8277f6d8879a05c6506e103786cab67e324dc1d3.tar.gz#egg=shared +https://github.com/codecov/shared/archive/0e2207ec1f892db82f687826669eb0fcf2d5c94d.tar.gz#egg=shared https://github.com/photocrowd/django-cursor-pagination/archive/f560902696b0c8509e4d95c10ba0d62700181d84.tar.gz idna>=3.7 minio diff --git a/requirements.txt b/requirements.txt index dd087fcd3d..4483684a28 100644 --- a/requirements.txt +++ b/requirements.txt @@ -416,7 +416,7 @@ sentry-sdk[celery]==2.13.0 # shared setproctitle==1.1.10 # via -r requirements.in -shared @ https://github.com/codecov/shared/archive/8277f6d8879a05c6506e103786cab67e324dc1d3.tar.gz +shared @ https://github.com/codecov/shared/archive/0e2207ec1f892db82f687826669eb0fcf2d5c94d.tar.gz # via -r requirements.in simplejson==3.17.2 # via -r requirements.in From 4fa07cacb7b5f68a2f7fd0a657df37cc4b41b318 Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Wed, 29 Jan 2025 15:22:05 -0800 Subject: [PATCH 17/19] staging check --- requirements.in | 2 +- requirements.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements.in b/requirements.in index d720623d62..fcde36de75 100644 --- a/requirements.in +++ b/requirements.in @@ -26,7 +26,7 @@ freezegun google-cloud-pubsub gunicorn>=22.0.0 https://github.com/codecov/opentelem-python/archive/refs/tags/v0.0.4a1.tar.gz#egg=codecovopentelem -https://github.com/codecov/shared/archive/0e2207ec1f892db82f687826669eb0fcf2d5c94d.tar.gz#egg=shared +https://github.com/codecov/shared/archive/a4bd8f76aa40b41772e59fb1993a7c7e5192f0df.tar.gz#egg=shared https://github.com/photocrowd/django-cursor-pagination/archive/f560902696b0c8509e4d95c10ba0d62700181d84.tar.gz idna>=3.7 minio diff --git a/requirements.txt b/requirements.txt index 4483684a28..4463b8964e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -416,7 +416,7 @@ sentry-sdk[celery]==2.13.0 # shared setproctitle==1.1.10 # via -r requirements.in -shared @ https://github.com/codecov/shared/archive/0e2207ec1f892db82f687826669eb0fcf2d5c94d.tar.gz +shared @ https://github.com/codecov/shared/archive/a4bd8f76aa40b41772e59fb1993a7c7e5192f0df.tar.gz # via -r requirements.in simplejson==3.17.2 # via -r requirements.in From b6ad27589ccb2b9da39ae1249e2464d493e28fc3 Mon Sep 17 00:00:00 2001 From: RulaKhaled Date: Thu, 30 Jan 2025 17:50:03 +0100 Subject: [PATCH 18/19] update with shared version --- requirements.in | 2 +- requirements.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements.in b/requirements.in index fcde36de75..fb46b3f820 100644 --- a/requirements.in +++ b/requirements.in @@ -26,7 +26,7 @@ freezegun google-cloud-pubsub gunicorn>=22.0.0 https://github.com/codecov/opentelem-python/archive/refs/tags/v0.0.4a1.tar.gz#egg=codecovopentelem -https://github.com/codecov/shared/archive/a4bd8f76aa40b41772e59fb1993a7c7e5192f0df.tar.gz#egg=shared +https://github.com/codecov/shared/archive/74c0888070699b69a4da73f54be502ad05fde7b6.tar.gz#egg=shared https://github.com/photocrowd/django-cursor-pagination/archive/f560902696b0c8509e4d95c10ba0d62700181d84.tar.gz idna>=3.7 minio diff --git a/requirements.txt b/requirements.txt index 4463b8964e..503ae882cd 100644 --- a/requirements.txt +++ b/requirements.txt @@ -416,7 +416,7 @@ sentry-sdk[celery]==2.13.0 # shared setproctitle==1.1.10 # via -r requirements.in -shared @ https://github.com/codecov/shared/archive/a4bd8f76aa40b41772e59fb1993a7c7e5192f0df.tar.gz +shared @ https://github.com/codecov/shared/archive/74c0888070699b69a4da73f54be502ad05fde7b6.tar.gz # via -r requirements.in simplejson==3.17.2 # via -r requirements.in From ea7582c1a1b858a8416f0622ef7fd9064333dbf7 Mon Sep 17 00:00:00 2001 From: Ajay Singh Date: Thu, 30 Jan 2025 09:57:16 -0800 Subject: [PATCH 19/19] PR comments --- api/internal/owner/serializers.py | 15 +++++++-------- services/billing.py | 21 ++++++++++++--------- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/api/internal/owner/serializers.py b/api/internal/owner/serializers.py index ac7a48189b..26584562f2 100644 --- a/api/internal/owner/serializers.py +++ b/api/internal/owner/serializers.py @@ -147,15 +147,14 @@ def validate(self, plan: Dict[str, Any]) -> Dict[str, Any]: detail="You cannot update your plan manually, for help or changes to plan, connect with sales@codecov.io" ) - active_plans = list( - Plan.objects.select_related("tier").filter(paid_plan=True, is_active=True) + active_plans = Plan.objects.select_related("tier").filter( + paid_plan=True, is_active=True ) - active_plan_names = {plan.name for plan in active_plans} - team_tier_plans = { - plan.name - for plan in active_plans - if plan.tier.tier_name == TierName.TEAM.value - } + + active_plan_names = set(active_plans.values_list("name", flat=True)) + team_tier_plans = active_plans.filter( + tier__tier_name=TierName.TEAM.value + ).values_list("name", flat=True) # Validate quantity here because we need access to whole plan object if plan["value"] in active_plan_names: diff --git a/services/billing.py b/services/billing.py index f3991222bb..c33c05bcd2 100644 --- a/services/billing.py +++ b/services/billing.py @@ -465,8 +465,10 @@ def _is_similar_plan(self, owner: Owner, desired_plan: dict) -> bool: """ Returns `True` if switching to a plan with similar term and seats. """ - current_plan_info = Plan.objects.get(name=owner.plan) - desired_plan_info = Plan.objects.get(name=desired_plan["value"]) + current_plan_info = Plan.objects.select_related("tier").get(name=owner.plan) + desired_plan_info = Plan.objects.select_related("tier").get( + name=desired_plan["value"] + ) is_same_term = ( current_plan_info @@ -477,16 +479,17 @@ def _is_similar_plan(self, owner: Owner, desired_plan: dict) -> bool: is_same_seats = ( owner.plan_user_count and owner.plan_user_count == desired_plan["quantity"] ) - - team_plans = Plan.objects.filter( - tier__tier_name=TierName.TEAM.value, is_active=True - ).values_list("name", flat=True) - # If from PRO to TEAM, then not a similar plan - if owner.plan not in team_plans and desired_plan["value"] in team_plans: + if ( + current_plan_info.tier.tier_name != TierName.TEAM.value + and desired_plan_info.tier.tier_name == TierName.TEAM.value + ): return False # If from TEAM to PRO, then considered a similar plan but really is an upgrade - elif owner.plan in team_plans and desired_plan["value"] not in team_plans: + elif ( + current_plan_info.tier.tier_name == TierName.TEAM.value + and desired_plan_info.tier.tier_name != TierName.TEAM.value + ): return True return bool(is_same_term and is_same_seats)