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.

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
Changes from 1 commit
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
5 changes: 3 additions & 2 deletions shared/django_apps/codecov_auth/models.py
Original file line number Diff line number Diff line change
@@ -220,7 +220,7 @@ def pretty_plan(self) -> dict | None:
This is how we represent the details of a plan to a user, see plan.constants.py
We inject quantity to make plan management easier on api, see PlanSerializer
"""
plan_details = Plan.objects.get(name=self.plan)
plan_details = Plan.objects.select_related("tier").get(name=self.plan)
if plan_details:
return {
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,
@@ -605,7 +605,8 @@ def pretty_plan(self):
if self.account:
return self.account.pretty_plan

plan_details = Plan.objects.get(name=self.plan)
print("@@@@@@@@@@@@@@@@@@@@", self.plan)
Copy link
Contributor

Choose a reason for hiding this comment

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

Another print 🖨️

plan_details = Plan.objects.select_related("tier").get(name=self.plan)
if plan_details:
return {
"marketing_name": plan_details.marketing_name,
36 changes: 18 additions & 18 deletions shared/plan/constants.py
Original file line number Diff line number Diff line change
@@ -77,6 +77,24 @@ class TierName(enum.Enum):
TRIAL = "trial"


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

Choose a reason for hiding this comment

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

@Swatinem this guy

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


@dataclass(repr=False)
class PlanData:
"""
@@ -110,24 +128,6 @@ def convert_to_DTO(self) -> dict:
}


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


NON_PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS = {
PlanName.CODECOV_PRO_MONTHLY_LEGACY.value: PlanData(
marketing_name=PlanMarketingName.CODECOV_PRO.value,
18 changes: 10 additions & 8 deletions shared/plan/service.py
Original file line number Diff line number Diff line change
@@ -158,9 +158,11 @@ def tier_name(self) -> TierName:

def available_plans(self, owner: Owner) -> List[Plan]:
"""Returns the available plans for the owner and organization."""
available_plans = {Plan.objects.get(name=PlanName.BASIC_PLAN_NAME.value)}
available_plans = {
Plan.objects.select_related("tier").get(name=PlanName.BASIC_PLAN_NAME.value)
}

curr_plan = Plan.objects.get(name=self.plan_name)
curr_plan = self.plan_data
if not curr_plan.paid_plan:
available_plans.add(curr_plan)

@@ -177,7 +179,9 @@ def available_plans(self, owner: Owner) -> List[Plan]:
available_tiers.append(TierName.TEAM.value)

available_plans.update(
Plan.objects.filter(tier__tier_name__in=available_tiers, is_active=True)
Plan.objects.select_related("tier").filter(
tier__tier_name__in=available_tiers, is_active=True
)
)

return [convert_to_DTO(plan) for plan in available_plans]
@@ -236,13 +240,11 @@ def start_trial_manually(self, current_owner: Owner, end_date: datetime) -> None
"""
# Start a new trial plan for free users currently not on trial

plan = Plan.objects.get(name=self.plan_name)
if plan.paid_plan is False:
if self.plan_data.tier.tier_name == TierName.TRIAL.value:
self._start_trial_helper(current_owner, end_date, is_extension=True)
elif self.plan_data.paid_plan is False:
self._start_trial_helper(current_owner, end_date, is_extension=False)
# Extend an existing trial plan for users currently on trial
elif plan.tier.tier_name == TierName.TRIAL.value:
self._start_trial_helper(current_owner, end_date, is_extension=True)
# Paying users cannot start a trial
else:
raise ValidationError("Cannot trial from a paid plan")

Original file line number Diff line number Diff line change
@@ -47,7 +47,6 @@
class TestOwnerModel(TransactionTestCase):
def setUp(self):
self.owner = OwnerFactory(username="codecov_name", email="[email protected]")
mock_all_plans_and_tiers()

def test_repo_total_credits_returns_correct_repos_for_legacy_plan(self):
self.owner.plan = "5m"
@@ -379,6 +378,7 @@ def test_can_activate_user_cannot_activate_account(self):
assert not self.owner.can_activate_user(self.owner)

def test_fields_that_account_overrides(self):
mock_all_plans_and_tiers()
to_activate = OwnerFactory()
self.owner.plan = PlanName.BASIC_PLAN_NAME.value
self.owner.plan_user_count = 1
@@ -398,12 +398,6 @@ def test_fields_that_account_overrides(self):
ENTERPRISE_CLOUD_USER_PLAN_REPRESENTATIONS[self.owner.account.plan]
)
account_pretty_plan.update({"quantity": 0})
print(
"account_pretty_plan",
account_pretty_plan,
"self.owner.pretty_plan",
self.owner.pretty_plan,
)
self.assertEqual(self.owner.pretty_plan, account_pretty_plan)

def test_add_admin_adds_ownerid_to_admin_array(self):
511 changes: 352 additions & 159 deletions tests/unit/plan/test_plan.py

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion tests/unit/upload/test_utils.py
Original file line number Diff line number Diff line change
@@ -22,7 +22,9 @@


class CoverageMeasurement(TestCase):
def setUp(self):
@classmethod
def setUpClass(cls):
super().setUpClass()
mock_all_plans_and_tiers()
Copy link
Contributor

@adrian-codecov adrian-codecov Jan 22, 2025

Choose a reason for hiding this comment

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

Why do we need that here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bc of this:

monthly_limit = plan_service.monthly_uploads_limit

admittedly could probably be more precise with the mock here


def add_upload_measurements_records(