ref: Remove references to plan and stripe consts #1130

ajay-sentry · 2025-02-05T00:24:52Z

Purpose/Motivation

This PR is meant to clean up the final few references of the "plan constants" that are no longer needed since migration to the Plan and Tiers tables.

As part of the cleanup, not only are we removing references to those plans but also to the "stripe id": "plan name" maps for dev/stage/prod which have also been ported over to the plan table.

I'm not sure if it would really matter but probably better to keep references to our plans, even if they're hashed values, away from the client as we modify or add more plans in the future. Plus, it's one less thing for us to mangle our way around and I felt like the queries on them for fetching are much easier to parse (much, much easier in some cases).

Tested:

Local upgrades / swaps / seat changes / and cancellations for plans
Double checked the plan reference maps in stage / prod with their associated stripe values in their console ✅

Links to relevant tickets

Closes codecov/engineering-team#3321

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.

ajay-sentry · 2025-02-05T00:25:23Z

api/internal/owner/serializers.py

-            list(stripe_plan_dict.values()).index(plan_id)
-        ]
-        marketing_plan_name = Plan.objects.get(name=plan_name).marketing_name
+        marketing_plan_name = Plan.objects.get(stripe_id=plan_id).marketing_name


This was so much cleaner to me

ya previous way was so annoying

ajay-sentry · 2025-02-05T00:25:40Z

billing/helpers.py

@@ -67,6 +58,7 @@ def mock_all_plans_and_tiers():
        billing_rate=BillingRate.MONTHLY.value,
        base_unit_price=PlanPrice.MONTHLY.value,
        paid_plan=True,
+        stripe_id="plan_pro",


mocked out the stripe_ids for us here for easy referencing

ajay-sentry · 2025-02-05T00:25:59Z

billing/tests/test_views.py

@@ -1326,12 +1328,11 @@ def test_subscription_schedule_released_updates_multiple_owners_with_existing_su

        self.owner.refresh_from_db()
        self.other_owner.refresh_from_db()
-        assert self.owner.plan == settings.STRIPE_PLAN_VALS[self.new_params["new_plan"]]
+
+        plan = Plan.objects.get(stripe_id=self.new_params["new_plan"])


had to change up the assertions a bit here to match

ajay-sentry · 2025-02-05T00:26:39Z

codecov_auth/management/commands/set_trial_status_values.py

@@ -1,114 +0,0 @@
-from datetime import datetime


Removing this command since it was a 1 time script and had some references in there that I didn't want to update

codecov · 2025-02-05T00:36:48Z

Codecov Report

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

Project coverage is 96.03%. Comparing base (4ed721d) to head (849c411).
Report is 6 commits behind head on main.

✅ All tests successful. No failed tests found.

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

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1130   +/-   ##
=======================================
  Coverage   96.02%   96.03%           
=======================================
  Files         837      837           
  Lines       19763    19761    -2     
=======================================
  Hits        18978    18978           
+ Misses        785      783    -2

Flag	Coverage Δ
unit	`95.92% <94.87%> (+0.01%)`	⬆️
unit-latest-uploader	`95.92% <94.87%> (+0.01%)`	⬆️

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-02-05T00:36:50Z

Codecov Report

Attention: Patch coverage is 94.87179% 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	91.30%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

codecov-qa · 2025-02-05T00:36:53Z

Codecov Report

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

Project coverage is 95.92%. Comparing base (4ed721d) to head (849c411).
Report is 6 commits behind head on main.

✅ All tests successful. No failed tests found.

github-actions · 2025-02-05T00:37:01Z

✅ All tests successful. No failed tests were found.

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

nora-codecov · 2025-02-05T01:57:10Z

codecov/settings_dev.py

-    "price_1Mj1mMGlVGuVgOrkC0ORc6iW": "users-sentryy",
-    "price_1OCM0gGlVGuVgOrkWDYEBtSL": "users-teamm",
-    "price_1OCM2cGlVGuVgOrkMWUFjPFz": "users-teamy",
-}


nora-codecov · 2025-02-05T01:59:07Z

services/billing.py

+                extra=dict(owner_id=owner.ownerid),
+            )
+            return
+


much better ☺️

adrian-codecov · 2025-02-05T18:37:46Z

codecov_auth/tests/test_admin.py

@@ -202,7 +201,6 @@ def test_inline_orgwide_add_token_permission_no_token_and_user_in_enterprise_clo
        self,
    ):
        owner = OwnerFactory(plan=DEFAULT_FREE_PLAN)
-        assert owner.plan not in ENTERPRISE_CLOUD_USER_PLAN_REPRESENTATIONS


Yeah I removed this assertion because the plan is defaulted to the free plan the line before, didn't really make sense to me

adrian-codecov · 2025-02-05T18:39:07Z

services/billing.py

@@ -404,6 +413,14 @@ def _modify_subscription_schedule(
        current_plan = subscription_item["plan"]["id"]
        current_quantity = subscription_item["quantity"]

+        plan = Plan.objects.filter(name=desired_plan["value"]).first()


Would add some tests to both of these if possible

codecov-public-qa · 2025-02-05T22:43:45Z

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
2738	1	2737	6

View the top 1 failed tests by shortest run time

services/tests/test_billing.py::StripeServiceTests::test_modify_subscription_no_plan_found

Stack Traces | 0.04s run time

self = <MagicMock name='error' id='140269164617728'>
args = ('Plan invalid plan not found',), kwargs = {'extra': {'owner_id': 1854}}
msg = 'Expected \'error\' to be called once. Called 2 times.\nCalls: [call(\'Internal error in sentry_sdk\', exc_info=(<clas...tream"), <traceback object at 0x7f92f4785a40>)),\n call(\'Plan invalid plan not found\', extra={\'owner_id\': 1854})].'

    def assert_called_once_with(self, /, *args, **kwargs):
        """assert that the mock was called exactly once and that that call was
        with the specified arguments."""
        if not self.call_count == 1:
            msg = ("Expected '%s' to be called once. Called %s times.%s"
                   % (self._mock_name or 'mock',
                      self.call_count,
                      self._calls_repr()))
>           raise AssertionError(msg)
E           AssertionError: Expected 'error' to be called once. Called 2 times.
E           Calls: [call('Internal error in sentry_sdk', exc_info=(<class 'django.http.request.RawPostDataException'>, RawPostDataException("You cannot access body after reading from request's data stream"), <traceback object at 0x7f92f4785a40>)),
E            call('Plan invalid plan not found', extra={'owner_id': 1854})].

.../local/lib/python3.12/unittest/mock.py:960: AssertionError

During handling of the above exception, another exception occurred:

self = <services.tests.test_billing.StripeServiceTests testMethod=test_modify_subscription_no_plan_found>
log_error_mock = <MagicMock name='error' id='140269164617728'>

    @patch("logging.Logger.error")
    def test_modify_subscription_no_plan_found(
        self,
        log_error_mock,
    ):
        original_plan = PlanName.CODECOV_PRO_MONTHLY.value
        original_user_count = 10
        owner = OwnerFactory(
            plan=original_plan,
            plan_user_count=original_user_count,
            stripe_subscription_id="33043sdf",
        )
    
        desired_plan_name = "invalid plan"
        desired_user_count = 10
        desired_plan = {"value": desired_plan_name, "quantity": desired_user_count}
        self.stripe.modify_subscription(owner, desired_plan)
    
        owner.refresh_from_db()
        assert owner.plan == original_plan
        assert owner.plan_user_count == original_user_count
>       log_error_mock.assert_called_once_with(
            f"Plan {desired_plan_name} not found",
            extra=dict(owner_id=owner.ownerid),
        )
E       AssertionError: Expected 'error' to be called once. Called 2 times.
E       Calls: [call('Internal error in sentry_sdk', exc_info=(<class 'django.http.request.RawPostDataException'>, RawPostDataException("You cannot access body after reading from request's data stream"), <traceback object at 0x7f92f4785a40>)),
E        call('Plan invalid plan not found', extra={'owner_id': 1854})].

services/tests/test_billing.py:718: AssertionError

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

…ations

remove all old plan representation and stripe id references, fix tests

7218333

ajay-sentry changed the title ~~remove all old plan representation and stripe id references, fix tests~~ ref: Remove references to plan and stripe consts Feb 5, 2025

ajay-sentry commented Feb 5, 2025

View reviewed changes

nora-codecov approved these changes Feb 5, 2025

View reviewed changes

adrian-codecov reviewed Feb 5, 2025

View reviewed changes

ajay-sentry and others added 2 commits February 5, 2025 14:12

ref: Plan query optimizations (#1131)

4330f01

add test

aff49b3

adrian-codecov approved these changes Feb 5, 2025

View reviewed changes

ajay-sentry added 2 commits February 5, 2025 14:49

robustness for flaky

a2a853a

Merge branch 'main' into Ajay/remove-references-to-old-plan-represent…

849c411

…ations

ajay-sentry temporarily deployed to staging February 7, 2025 22:43 — with GitHub Actions Inactive

ajay-sentry added this pull request to the merge queue Feb 10, 2025

Merged via the queue into main with commit 66e1efd Feb 10, 2025
28 of 32 checks passed

ajay-sentry deleted the Ajay/remove-references-to-old-plan-representations branch February 10, 2025 18:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ref: Remove references to plan and stripe consts #1130

ref: Remove references to plan and stripe consts #1130

ajay-sentry commented Feb 5, 2025 •

edited

Loading

ajay-sentry Feb 5, 2025

adrian-codecov Feb 5, 2025

ajay-sentry Feb 5, 2025

ajay-sentry Feb 5, 2025

ajay-sentry Feb 5, 2025

codecov bot commented Feb 5, 2025 •

edited

Loading

codecov-notifications bot commented Feb 5, 2025 •

edited

Loading

codecov-qa bot commented Feb 5, 2025 •

edited

Loading

github-actions bot commented Feb 5, 2025

nora-codecov Feb 5, 2025

nora-codecov Feb 5, 2025

adrian-codecov Feb 5, 2025

ajay-sentry Feb 5, 2025

adrian-codecov Feb 5, 2025

codecov-public-qa bot commented Feb 5, 2025

ref: Remove references to plan and stripe consts #1130

ref: Remove references to plan and stripe consts #1130

Conversation

ajay-sentry commented Feb 5, 2025 • edited Loading

Purpose/Motivation

Links to relevant tickets

Notes to Reviewer

Legal Boilerplate

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

codecov bot commented Feb 5, 2025 • edited Loading

Codecov Report

codecov-notifications bot commented Feb 5, 2025 • edited Loading

Codecov Report

codecov-qa bot commented Feb 5, 2025 • edited Loading

Codecov Report

github-actions bot commented Feb 5, 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

codecov-public-qa bot commented Feb 5, 2025

❌ 1 Tests Failed:

ajay-sentry commented Feb 5, 2025 •

edited

Loading

codecov bot commented Feb 5, 2025 •

edited

Loading

codecov-notifications bot commented Feb 5, 2025 •

edited

Loading

codecov-qa bot commented Feb 5, 2025 •

edited

Loading