API: Migrate to Plan / Tier Tables #1099

ajay-sentry · 2025-01-15T17:40:57Z

Purpose/Motivation

This PR relies on this version of shared: codecov/shared#479.

The main objective of this PR is to migrate all Backend Service logic to use the new plan and tiers tables instead of the Plan constants. You can read more about milestone 3 here.

A core change introduced in this PR is the creation of mocks for plans and tiers, which are used in most test setup functions. To ensure efficient testing, we use setUpClass to set up these mocks once per test class, avoiding the need to recreate them for each test.

Links to relevant tickets

Closes codecov/engineering-team#3252

What does this PR do?

Include a brief description of the changes in this PR. Bullet points are your friend.

Notes to Reviewer

Anything to note to the team? Any tips on how to review, or where to start?

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

codecov · 2025-01-15T17:51:56Z

Codecov Report

Attention: Patch coverage is 96.77419% with 2 lines in your changes missing coverage. Please review.

Project coverage is 96.07%. Comparing base (e06c5d8) to head (e9662a2).
Report is 4 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
services/billing.py	87.50%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1099      +/-   ##
==========================================
- Coverage   96.11%   96.07%   -0.04%     
==========================================
  Files         835      836       +1     
  Lines       19572    19611      +39     
==========================================
+ Hits        18812    18842      +30     
- Misses        760      769       +9

Flag	Coverage Δ
unit	`95.98% <96.77%> (-0.05%)`	⬇️
unit-latest-uploader	`95.98% <96.77%> (-0.05%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

codecov-notifications · 2025-01-15T17:52:07Z

Codecov Report

Attention: Patch coverage is 96.77419% with 2 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
services/billing.py	87.50%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

codecov-qa · 2025-01-15T17:52:11Z

Codecov Report

Attention: Patch coverage is 96.82540% with 2 lines in your changes missing coverage. Please review.

Project coverage is 95.98%. Comparing base (ae83c2f) to head (b6ad275).
Report is 2 commits behind head on main.

✅ All tests successful. No failed tests found.

codecov-public-qa · 2025-01-15T17:52:19Z

❌ 5 Tests Failed:

Tests completed	Failed	Passed	Skipped
2711	5	2706	6

View the top 3 failed tests by shortest run time

graphql_api/tests/test_owner.py::TestOwnerType::test_fetch_available_plans_is_enterprise_plan

Stack Traces | 0.442s run time

self = <graphql_api.tests.test_owner.TestOwnerType testMethod=test_fetch_available_plans_is_enterprise_plan>

    def test_fetch_available_plans_is_enterprise_plan(self):
        current_org = OwnerFactory(
            username="random-plan-user",
            service="github",
            plan=PlanName.FREE_PLAN_NAME.value,
        )
    
        query = """{
            owner(username: "%s") {
                availablePlans {
                    value
                    isEnterprisePlan
                    isProPlan
                    isTeamPlan
                    isSentryPlan
                    isFreePlan
                    isTrialPlan
                }
            }
        }
        """ % (current_org.username)
        data = self.gql_request(query, owner=current_org)
>       assert data == {
            "owner": {
                "availablePlans": [
                    {
                        "value": "users-basic",
                        "isEnterprisePlan": False,
                        "isProPlan": False,
                        "isTeamPlan": False,
                        "isSentryPlan": False,
                        "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,
                        "isProPlan": True,
                        "isTeamPlan": False,
                        "isSentryPlan": False,
                        "isFreePlan": False,
                        "isTrialPlan": False,
                    },
                    {
                        "value": "users-pr-inappy",
                        "isEnterprisePlan": False,
                        "isProPlan": True,
                        "isTeamPlan": False,
                        "isSentryPlan": False,
                        "isFreePlan": False,
                        "isTrialPlan": False,
                    },
                    {
                        "value": "users-teamm",
                        "isEnterprisePlan": False,
                        "isProPlan": False,
                        "isTeamPlan": True,
                        "isSentryPlan": False,
                        "isFreePlan": False,
                        "isTrialPlan": False,
                    },
                    {
                        "value": "users-teamy",
                        "isEnterprisePlan": False,
                        "isProPlan": False,
                        "isTeamPlan": True,
                        "isSentryPlan": False,
                        "isFreePlan": False,
                        "isTrialPlan": False,
                    },
                ]
            }
        }
E       AssertionError: assert {'owner': {'a...Plans': None}} == {'owner': {'a...False, ...}]}}
E         
E         Differing items:
E         {'owner': {'availablePlans': None}} != {'owner': {'availablePlans': [{'isEnterprisePlan': False, 'isFreePlan': True, 'isProPlan': False, 'isSentryPlan': Fals...Plan': False, ...}, {'isEnterprisePlan': False, 'isFreePlan': False, 'isProPlan': False, 'isSentryPlan': False, ...}]}}
E         Use -v to get more diff

graphql_api/tests/test_owner.py:1151: AssertionError

graphql_api/tests/test_owner.py::TestOwnerType::test_resolve_number_of_uploads_per_user

Stack Traces | 0.625s run time

self = <graphql_api.tests.test_owner.TestOwnerType testMethod=test_resolve_number_of_uploads_per_user>

    def test_resolve_number_of_uploads_per_user(self):
        query_uploads_number = """{
            owner(username: "%s") {
               numberOfUploads
            }
        }
        """
        repository = RepositoryFactory.create(
            author__plan=PlanName.BASIC_PLAN_NAME.value, author=self.owner
        )
        first_commit = CommitFactory.create(repository=repository)
        first_report = CommitReportFactory.create(
            commit=first_commit, report_type=ReportType.COVERAGE.value
        )
        for i in range(150):
            upload = UploadFactory.create(report=first_report)
            insert_coverage_measurement(
                owner_id=self.owner.ownerid,
                repo_id=repository.repoid,
                commit_id=first_commit.id,
                upload_id=upload.id,
                uploader_used=UploaderType.CLI.value,
                private_repo=repository.private,
                report_type=first_report.report_type,
            )
        query = query_uploads_number % (repository.author.username)
        data = self.gql_request(query, owner=self.owner)
>       assert data["owner"]["numberOfUploads"] == 150
E       assert None == 150

graphql_api/tests/test_owner.py:383: AssertionError

graphql_api/tests/test_owner.py::TestOwnerType::test_owner_plan_status

Stack Traces | 1.22s run time

self = <graphql_api.tests.test_owner.TestOwnerType testMethod=test_owner_plan_status>

    @freeze_time("2023-06-19")
    def test_owner_plan_status(self):
        current_org = OwnerFactory(
            username="random-plan-user",
            service="github",
            trial_start_date=timezone.now(),
            trial_end_date=timezone.now() + timedelta(days=14),
            trial_status=TrialStatus.ONGOING.value,
        )
        query = """{
            owner(username: "%s") {
                plan {
                    trialStatus
                }
            }
        }
        """ % (current_org.username)
        data = self.gql_request(query, owner=current_org)
>       assert data["owner"]["plan"] == {
            "trialStatus": "ONGOING",
        }
E       AssertionError: assert None == {'trialStatus': 'ONGOING'}

graphql_api/tests/test_owner.py:644: AssertionError

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

github-actions · 2025-01-15T17:52:19Z

✅ All tests successful. No failed tests were found.

📣 Thoughts on this report? Let Codecov know! | Powered by Codecov

ajay-sentry · 2025-01-15T21:09:03Z

api/internal/owner/serializers.py

@@ -131,11 +130,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:


Removed this because it would now just be an extra DB call for a log lol

api/internal/owner/serializers.py

…updates

billing/helpers.py

github-actions · 2025-01-21T21:43:11Z

This PR includes changes to shared. Please review them here: codecov/shared@fe16480...0e2207e

ajay-sentry · 2025-01-23T23:01:50Z

api/internal/owner/serializers.py

@@ -219,7 +222,7 @@ def get_plan(self, phase: Dict[str, Any]) -> str:
        plan_name = list(stripe_plan_dict.keys())[


This can probably be further cleaned up by doing a get on the stripe_id 🤔

ajay-sentry · 2025-01-23T23:20:04Z

upload/helpers.py

@@ -637,7 +637,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.objects.values_list("name", flat=True)


This is some mega legacy code, I don't think anyone is on per repo billing anymore and can probably be deleted tbh

RulaKhaled · 2025-01-24T11:07:22Z

billing/tests/test_views.py

@@ -69,17 +71,24 @@ def __getitem__(self, key):


 class StripeWebhookHandlerTests(APITestCase):
+    @classmethod
+    def setUpClass(cls):


ajay-sentry · 2025-01-27T22:32:03Z

graphql_api/types/owner/owner.py

 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]
-    return free_plan.convert_to_DTO()
+    free_plan = Plan.objects.select_related("tier").get(


direct fetch of the basic plan info, will swap to new dev plan when we do that integration

ajay-sentry · 2025-01-27T22:33:58Z

services/billing.py

@@ -794,22 +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 FREE_PLAN_REPRESENTATIONS:
+        try:
+            plan = Plan.objects.get(name=desired_plan["value"])


This is a bit of a logic update for what I interpreted the other const to be doing; check if the plan exists AND check if the plan is active; otherwise its an old plan we don't want users to transition to anymore

ajay-sentry · 2025-01-27T22:35:14Z

services/billing.py

+            )
+            return None
+
+        if plan.paid_plan is False:


a logic "reversal" here because we've already ruled out the non-existent/old plan case. So we can check if the plan is a free plan and set that plan data, otherwise do the paid plan logic

nora-codecov · 2025-01-28T18:56:38Z

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


I don't see this commit on the branch in shared - can you update to the most recent commit on your branch?

You can find the commit at codecov/shared#479. We need the changes from that version and will update to the new shared version once it's merged. Feel free to review the rest of the files—we did the same for the worker PR

github-actions · 2025-01-29T23:10:13Z

This PR includes changes to shared. Please review them here: codecov/shared@fe16480...a4bd8f7

github-actions · 2025-01-30T16:50:25Z

This PR includes changes to shared. Please review them here: codecov/shared@fe16480...74c0888

api/internal/owner/serializers.py

nora-codecov · 2025-01-30T17:15:33Z

api/internal/owner/serializers.py

+        active_plans = list(
+            Plan.objects.select_related("tier").filter(paid_plan=True, is_active=True)
+        )
+        active_plan_names = {plan.name for plan in active_plans}


Suggested change

active_plan_names = {plan.name for plan in active_plans}

active_plan_names = set(active_plans.values_list("name", flat=True))

written from memory so there may be typos, but instead of iterating through, just return the values you want

It's cool that django can keep those sets in memory and do additional filters if needed

nora-codecov · 2025-01-30T17:16:31Z

api/internal/owner/serializers.py

+            plan.name
+            for plan in active_plans
+            if plan.tier.tier_name == TierName.TEAM.value
+        }


do a filter on the qset to achieve plan.tier.tier_name == TierName.TEAM.value rather than iterating and checking. then do a values_list to get the names

Updated ✔️ Thanks for the suggestion

nora-codecov · 2025-01-30T17:19:11Z

api/internal/owner/serializers.py

@@ -184,7 +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 TEAM_PLAN_REPRESENTATIONS
+                plan["value"] in team_tier_plans


I thought that with the new structuring of plans, instead of doing a comparison like this, you would get the Plan object that plan["value"] references, and then you could do, if plan.tier.tier_name == TierName.TEAM.value

the plan should have these attributes on it, so you don't need to compile a list of other plans and ask is this one in that list - just check the attribute.

Yep totally on board! Unfortunately the frontend is the thing that sends us these values right now and it's super naive in how it's being done. We could fetch the plan based off the name here maybe and do a similar check, but ultimately its about the same

From here:
https://github.com/codecov/gazebo/blob/0d1dd111140f3ac278bdf50631ec14bfbdfe4b03/src/services/account/useUpgradePlan.ts#L30-L35

nora-codecov · 2025-01-30T17:33:36Z

services/billing.py

            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:


same question on these checks - could you not do current_plan_info.tier_name?

yeah fair point, makes sense

nora-codecov · 2025-01-30T19:08:21Z

services/billing.py

+        elif (
+            current_plan_info.tier.tier_name == TierName.TEAM.value
+            and desired_plan_info.tier.tier_name != TierName.TEAM.value
+        ):


_is_extending_term and _is_similar_plan - they are both getting Plans from the db, but they are both only called once, from the same place (_get_proration_params)

could do the db calls in _get_proration_params, then pass the Plan objs or the relevant fields to _is_extending_term and _is_similar_plan to cut db hits in half

suejung-sentry

approving on behalf of @nora-codecov who is awaiting getting added to this repo

ajay-sentry requested a review from a team as a code owner January 15, 2025 17:40

ajay-sentry requested a review from a team as a code owner January 15, 2025 20:02

ajay-sentry commented Jan 15, 2025

View reviewed changes

RulaKhaled reviewed Jan 16, 2025

View reviewed changes

api/internal/owner/serializers.py Outdated Show resolved Hide resolved

ajay-sentry added 6 commits January 20, 2025 10:28

starting transition, need the other PR merged to do imports and test …

947954a

…updates

other api stuff

a06b8c8

revert trial status changes, update other instances of plan stuff

1f2e63b

fix a couple instances... still need to fix availablePlans

5050704

a few more references for PAID and TEAM_PLANS

40abd2d

missing s

16f7fba

RulaKhaled reviewed Jan 20, 2025

View reviewed changes

billing/helpers.py Outdated Show resolved Hide resolved

add missing imports + refactor to call 1 query

3a5b3af

RulaKhaled force-pushed the Ajay/milestone-3-migration branch from bef1fa7 to 3a5b3af Compare January 20, 2025 11:51

RulaKhaled and others added 2 commits January 20, 2025 14:07

update tests

d26259a

testing locally, fix the tier checks and first test

f21a5f1

ajay-sentry and others added 3 commits January 21, 2025 15:47

starting to fix tests and some logic

ba63895

update api with mocked plans

91ea64b

fix all tests except account_viewset

42488fb

ajay-sentry changed the title ~~[WIP] Plan Milestone 3 Transitions~~ API: Migrate to Plan / Tier Tables Jan 23, 2025

ajay-sentry added 2 commits January 23, 2025 13:58

all tests passing now, fixed a couple small logic bugs too

1bfc6d8

fix this suite, when using ttestcase need to have setup vs setupClass

99a63e1

ajay-sentry mentioned this pull request Jan 23, 2025

Shared: Migrate to Plan / Tier Tables codecov/shared#479

Merged

ajay-sentry commented Jan 23, 2025

View reviewed changes

clean up the logic for update plan

49f2af8

ajay-sentry commented Jan 23, 2025

View reviewed changes

RulaKhaled reviewed Jan 24, 2025

View reviewed changes

ajay-sentry added 2 commits January 24, 2025 10:41

Merge branch 'main' into Ajay/milestone-3-migration

e2560c5

rebase

34b1677

ajay-sentry commented Jan 27, 2025

View reviewed changes

nora-codecov reviewed Jan 28, 2025

View reviewed changes

Merge branch 'main' into Ajay/milestone-3-migration

3f7fe6f

staging check

4fa07ca

ajay-sentry temporarily deployed to staging January 29, 2025 23:37 — with GitHub Actions Inactive

update with shared version

b6ad275

RulaKhaled enabled auto-merge January 30, 2025 17:03

RulaKhaled disabled auto-merge January 30, 2025 17:05

nora-codecov reviewed Jan 30, 2025

View reviewed changes

api/internal/owner/serializers.py Show resolved Hide resolved

nora-codecov reviewed Jan 30, 2025

View reviewed changes

ajay-sentry added 2 commits January 30, 2025 09:57

PR comments

ea7582c

Merge branch 'main' into Ajay/milestone-3-migration

e9662a2

nora-codecov reviewed Jan 30, 2025

View reviewed changes

nora-codecov approved these changes Jan 30, 2025

View reviewed changes

suejung-sentry approved these changes Jan 30, 2025

View reviewed changes

ajay-sentry added this pull request to the merge queue Jan 30, 2025

Merged via the queue into main with commit 354b640 Jan 30, 2025
17 of 19 checks passed

ajay-sentry deleted the Ajay/milestone-3-migration branch January 30, 2025 19:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API: Migrate to Plan / Tier Tables #1099

API: Migrate to Plan / Tier Tables #1099

ajay-sentry commented Jan 15, 2025 •

edited by RulaKhaled

Loading

codecov bot commented Jan 15, 2025 •

edited

Loading

codecov-notifications bot commented Jan 15, 2025 •

edited

Loading

codecov-qa bot commented Jan 15, 2025 •

edited

Loading

codecov-public-qa bot commented Jan 15, 2025 •

edited

Loading

github-actions bot commented Jan 15, 2025 •

edited

Loading

ajay-sentry Jan 15, 2025

github-actions bot commented Jan 21, 2025 •

edited

Loading

ajay-sentry Jan 23, 2025

ajay-sentry Jan 23, 2025

RulaKhaled Jan 24, 2025

ajay-sentry Jan 27, 2025 •

edited

Loading

ajay-sentry Jan 27, 2025

ajay-sentry Jan 27, 2025

nora-codecov Jan 28, 2025

RulaKhaled Jan 29, 2025

github-actions bot commented Jan 29, 2025 •

edited

Loading

github-actions bot commented Jan 30, 2025

nora-codecov Jan 30, 2025

ajay-sentry Jan 30, 2025

nora-codecov Jan 30, 2025

ajay-sentry Jan 30, 2025

nora-codecov Jan 30, 2025

ajay-sentry Jan 30, 2025

ajay-sentry Jan 30, 2025

nora-codecov Jan 30, 2025

ajay-sentry Jan 30, 2025

nora-codecov Jan 30, 2025

suejung-sentry left a comment

		@@ -219,7 +222,7 @@ def get_plan(self, phase: Dict[str, Any]) -> str:
		plan_name = list(stripe_plan_dict.keys())[

	active_plan_names = {plan.name for plan in active_plans}
	active_plan_names = set(active_plans.values_list("name", flat=True))

API: Migrate to Plan / Tier Tables #1099

API: Migrate to Plan / Tier Tables #1099

Conversation

ajay-sentry commented Jan 15, 2025 • edited by RulaKhaled Loading

Purpose/Motivation

Links to relevant tickets

What does this PR do?

Notes to Reviewer

Legal Boilerplate

codecov bot commented Jan 15, 2025 • edited Loading

Codecov Report

codecov-notifications bot commented Jan 15, 2025 • edited Loading

Codecov Report

codecov-qa bot commented Jan 15, 2025 • edited Loading

Codecov Report

codecov-public-qa bot commented Jan 15, 2025 • edited Loading

❌ 5 Tests Failed:

github-actions bot commented Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajay-sentry Jan 27, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 29, 2025 • edited Loading

github-actions bot commented Jan 30, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

suejung-sentry left a comment

Choose a reason for hiding this comment

ajay-sentry commented Jan 15, 2025 •

edited by RulaKhaled

Loading

codecov bot commented Jan 15, 2025 •

edited

Loading

codecov-notifications bot commented Jan 15, 2025 •

edited

Loading

codecov-qa bot commented Jan 15, 2025 •

edited

Loading

codecov-public-qa bot commented Jan 15, 2025 •

edited

Loading

github-actions bot commented Jan 15, 2025 •

edited

Loading

github-actions bot commented Jan 21, 2025 •

edited

Loading

ajay-sentry Jan 27, 2025 •

edited

Loading

github-actions bot commented Jan 29, 2025 •

edited

Loading