Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shared: Migrate to Plan / Tier Tables #479

Merged
merged 22 commits into from
Jan 30, 2025
Merged

Shared: Migrate to Plan / Tier Tables #479

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
fbbb396
shared stuff should be migrated with this, first try with available_p…
ajay-sentry Jan 16, 2025
52fdc2d
Merge branch 'main' into Ajay/milestone-3-migration
ajay-sentry Jan 17, 2025
33d98f6
boilerplate updates, better way to do the plan searches
ajay-sentry Jan 17, 2025
ba68a8b
update logic a bit to work with new plan, and types as well, start fi…
ajay-sentry Jan 17, 2025
3b1deb6
43 tests failing locally
ajay-sentry Jan 18, 2025
7eb00e4
fix some tests:
RulaKhaled Jan 20, 2025
4fd5090
uncomment stuff
RulaKhaled Jan 20, 2025
7473f20
more tests fixing
RulaKhaled Jan 21, 2025
ccb28ab
5 more to go
RulaKhaled Jan 21, 2025
208eb10
wrap up tests in test_plan
RulaKhaled Jan 21, 2025
a9a5eeb
resolve all tests
RulaKhaled Jan 21, 2025
3dcd8ef
update test imports and helper, remove trial_days from DTO, start off…
ajay-sentry Jan 21, 2025
6b1fabc
lint
ajay-sentry Jan 21, 2025
906e7bc
Update with pretty plan changes
RulaKhaled Jan 22, 2025
2cd7436
setupClass from setUp and clean up a bunch of the available plan tests
ajay-sentry Jan 23, 2025
8277f6d
remove ttestcase if not needed
ajay-sentry Jan 23, 2025
5c8398a
print
ajay-sentry Jan 23, 2025
d2c0af3
Merge branch 'main' into Ajay/milestone-3-migration
ajay-sentry Jan 24, 2025
7804571
use cached property, remove setter
ajay-sentry Jan 24, 2025
0e2207e
Merge branch 'main' into Ajay/milestone-3-migration
ajay-sentry Jan 27, 2025
6e1418a
missed a couple prefetch references
ajay-sentry Jan 27, 2025
a4bd8f7
Merge branch 'main' into Ajay/milestone-3-migration
ajay-sentry Jan 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 33 additions & 15 deletions shared/django_apps/codecov_auth/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import logging
import os
import uuid
from dataclasses import asdict
from datetime import datetime
from hashlib import md5
from typing import Optional, Self
Expand Down Expand Up @@ -30,7 +29,7 @@
from shared.django_apps.codecov_auth.managers import OwnerManager
from shared.django_apps.core.managers import RepositoryManager
from shared.django_apps.core.models import DateTimeWithoutTZField, Repository
from shared.plan.constants import USER_PLAN_REPRESENTATIONS, PlanName
from shared.plan.constants import PlanName, TrialDaysAmount

# Added to avoid 'doesn't declare an explicit app_label and isn't in an application in INSTALLED_APPS' error\
# Needs to be called the same as the API app
Expand Down Expand Up @@ -221,10 +220,22 @@ def pretty_plan(self) -> dict | None:
This is how we represent the details of a plan to a user, see plan.constants.py
We inject quantity to make plan management easier on api, see PlanSerializer
"""
if self.plan in USER_PLAN_REPRESENTATIONS:
plan_details = asdict(USER_PLAN_REPRESENTATIONS[self.plan])
plan_details.update({"quantity": self.plan_seat_count})
return plan_details
plan_details = Plan.objects.get(name=self.plan)
if plan_details:
return {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we make this a class method, like serialized plan or something

class Plan(BaseModel):
...

  def serialized_plan(self): -> <you could also type this into another dataclass or something if you want brownie points)
   return {...} # this is where you'd put this "pretty plan"

"marketing_name": plan_details.marketing_name,
"value": plan_details.name,
"billing_rate": plan_details.billing_rate,
"base_unit_price": plan_details.base_unit_price,
"benefits": plan_details.benefits,
"tier_name": plan_details.tier.tier_name,
"monthly_uploads_limit": plan_details.monthly_uploads_limit,
"trial_days": TrialDaysAmount.CODECOV_SENTRY.value
if plan_details.name == PlanName.TRIAL_PLAN_NAME.value
else None,
"quantity": self.plan_seat_count,
}
return None

def can_activate_user(self, user: User | None = None) -> bool:
"""
Expand Down Expand Up @@ -593,16 +604,23 @@ def avatar_url(self, size=DEFAULT_AVATAR_SIZE):
def pretty_plan(self):
if self.account:
return self.account.pretty_plan
if self.plan in USER_PLAN_REPRESENTATIONS:
plan_details = asdict(USER_PLAN_REPRESENTATIONS[self.plan])

# update with quantity they've purchased
# allows api users to update the quantity
# by modifying the "plan", sidestepping
# some iffy data modeling

plan_details.update({"quantity": self.plan_user_count})
return plan_details
plan_details = Plan.objects.get(name=self.plan)
if plan_details:
return {
"marketing_name": plan_details.marketing_name,
"value": plan_details.name,
"billing_rate": plan_details.billing_rate,
"base_unit_price": plan_details.base_unit_price,
"benefits": plan_details.benefits,
"tier_name": plan_details.tier.tier_name,
"monthly_uploads_limit": plan_details.monthly_uploads_limit,
"trial_days": TrialDaysAmount.CODECOV_SENTRY.value
if plan_details.name == PlanName.TRIAL_PLAN_NAME.value
else None,
"quantity": self.plan_user_count,
}
return None

def can_activate_user(self, owner_user: Self) -> bool:
owner_org = self
Expand Down
6 changes: 3 additions & 3 deletions shared/django_apps/codecov_auth/services/org_level_token_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
from django.dispatch import receiver
from django.forms import ValidationError

from shared.django_apps.codecov_auth.models import OrganizationLevelToken, Owner
from shared.plan.constants import USER_PLAN_REPRESENTATIONS
from shared.django_apps.codecov_auth.models import OrganizationLevelToken, Owner, Plan

log = logging.getLogger(__name__)

Expand All @@ -18,9 +17,10 @@ class OrgLevelTokenService(object):
-- only 1 token per Owner
"""

# MIGHT BE ABLE TO REMOVE THIS AND SUBSEQUENT DOWNSTREAM STUFF
@classmethod
def org_can_have_upload_token(cls, org: Owner):
return org.plan in USER_PLAN_REPRESENTATIONS
return Plan.objects.filter(name=org.plan, is_active=True).exists()

@classmethod
def get_or_create_org_token(cls, org: Owner):
Expand Down
24 changes: 15 additions & 9 deletions shared/django_apps/codecov_auth/tests/factories.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
UserToken,
)
from shared.encryption.oauth import get_encryptor_from_configuration
from shared.plan.constants import TrialStatus
from shared.plan.constants import PlanName, TierName, TrialStatus

encryptor = get_encryptor_from_configuration()

Expand Down Expand Up @@ -177,20 +177,26 @@ class TierFactory(DjangoModelFactory):
class Meta:
model = Tier

tier_name = factory.Faker("name")
tier_name = TierName.BASIC.value
bundle_analysis = False
test_analytics = False
flaky_test_detection = False
project_coverage = False
private_repo_support = False


class PlanFactory(DjangoModelFactory):
class Meta:
model = Plan

base_unit_price = factory.Faker("pyint")
benefits = []
tier = factory.SubFactory(TierFactory)
base_unit_price = 0
benefits = factory.LazyFunction(lambda: ["Benefit 1", "Benefit 2", "Benefit 3"])
billing_rate = None
is_active = True
marketing_name = factory.Faker("name")
max_seats = None
marketing_name = factory.Faker("catch_phrase")
max_seats = 1
monthly_uploads_limit = None
paid_plan = True
name = factory.Faker("name")
tier = factory.SubFactory(TierFactory)
name = PlanName.BASIC_PLAN_NAME.value
paid_plan = False
stripe_id = None
27 changes: 23 additions & 4 deletions shared/plan/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ class TierName(enum.Enum):
TEAM = "team"
PRO = "pro"
ENTERPRISE = "enterprise"
SENTRY = "sentry"
TRIAL = "trial"


@dataclass(repr=False)
Expand All @@ -99,7 +101,6 @@ def convert_to_DTO(self) -> dict:
"benefits": self.benefits,
"tier_name": self.tier_name,
"monthly_uploads_limit": self.monthly_uploads_limit,
"trial_days": self.trial_days,
"is_free_plan": self.tier_name == TierName.BASIC.value,
"is_pro_plan": self.tier_name == TierName.PRO.value,
"is_team_plan": self.tier_name == TierName.TEAM.value,
Expand All @@ -109,6 +110,24 @@ def convert_to_DTO(self) -> dict:
}


def convert_to_DTO(plan) -> dict:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on typing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was getting a circular dependency trying to import the Plan class and type it that way, is there another way we can do it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So why is this a circular dependency? I suppose cause you'd import Plan, and plan imports from constants.

So, this being the only thing that imports from Plan, could this belong in plan instead? You'd have to make an instance of Plan to use this wherever you do though

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah exactly, I kinda wanted to avoid having to bind this function to a class when it's really just a "transformer" from our plan object to the plan object that the planService and subsequently GQL expects

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I'll leave the implementation up to you, I think though that not typing a function because of circular imports feels a bit odd, but you're getting of the fn soon right? If so I wouldn't say it's a big deal

return {
"marketing_name": plan.marketing_name,
"value": plan.name,
"billing_rate": plan.billing_rate,
"base_unit_price": plan.base_unit_price,
"benefits": plan.benefits,
"tier_name": plan.tier.tier_name,
"monthly_uploads_limit": plan.monthly_uploads_limit,
"is_free_plan": not plan.paid_plan,
"is_pro_plan": plan.tier.tier_name == TierName.PRO.value,
"is_team_plan": plan.tier.tier_name == TierName.TEAM.value,
"is_enterprise_plan": plan.tier.tier_name == TierName.ENTERPRISE.value,
"is_trial_plan": plan.tier.tier_name == TierName.TRIAL.value,
"is_sentry_plan": plan.tier.tier_name == TierName.SENTRY.value,
}


NON_PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS = {
PlanName.CODECOV_PRO_MONTHLY_LEGACY.value: PlanData(
marketing_name=PlanMarketingName.CODECOV_PRO.value,
Expand Down Expand Up @@ -189,7 +208,7 @@ def convert_to_DTO(self) -> dict:
"Unlimited private repositories",
"Priority Support",
],
tier_name=TierName.PRO.value,
tier_name=TierName.SENTRY.value,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: These consts are all disappearing soon and aren't / shouldn't be a SOT for anything anymore

trial_days=TrialDaysAmount.CODECOV_SENTRY.value,
monthly_uploads_limit=None,
),
Expand All @@ -205,7 +224,7 @@ def convert_to_DTO(self) -> dict:
"Unlimited private repositories",
"Priority Support",
],
tier_name=TierName.PRO.value,
tier_name=TierName.SENTRY.value,
trial_days=TrialDaysAmount.CODECOV_SENTRY.value,
monthly_uploads_limit=None,
),
Expand Down Expand Up @@ -342,7 +361,7 @@ def convert_to_DTO(self) -> dict:
"Unlimited private repositories",
"Priority Support",
],
tier_name=TierName.PRO.value,
tier_name=TierName.TRIAL.value,
trial_days=None,
monthly_uploads_limit=None,
),
Expand Down
Loading
Loading