feat: Add new users-developer plan #494
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #494 +/- ##
==========================================
- Coverage 90.34% 89.97% -0.37%
==========================================
Files 437 324 -113
Lines 12951 9045 -3906
Branches 2104 1599 -505
==========================================
- Hits 11700 8138 -3562
+ Misses 1133 846 -287
+ Partials 118 61 -57
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -311,9 +316,25 @@ def convert_to_DTO(self) -> dict: | |||
| monthly_uploads_limit=None, | |||
| ) | |||
|
|
|||
| DEVELOPER_PLAN = PlanData( | |||
There was a problem hiding this comment.
We shouldn't need these anymore right?
There was a problem hiding this comment.
unfortunately we do because API is still using FREE_PLAN_REPRESENTATIONS once we clean up API we can go back and remove all plans representations.
There was a problem hiding this comment.
Just double checked that reference and it's for a django command script that is "dead" :) I think we're good to not add it for that reason
| FREE_PLAN_REPRESENTATIONS = { | ||
| PlanName.FREE_PLAN_NAME.value: FREE_PLAN, | ||
| PlanName.BASIC_PLAN_NAME.value: BASIC_PLAN, | ||
| DEFAULT_FREE_PLAN: DEVELOPER_PLAN, |
There was a problem hiding this comment.
We shouldn't need this anymore right?
There was a problem hiding this comment.
same response, we can double confirm with @adrian-codecov but that's what he told me originally
shared/plan/constants.py
Outdated
| @@ -2,10 +2,13 @@ | |||
| from dataclasses import dataclass | |||
| from typing import List, Optional | |||
|
|
|||
| DEFAULT_FREE_PLAN = "users-developer" | |||
There was a problem hiding this comment.
Can we put this next to the plan names so it's easier to see?
There was a problem hiding this comment.
thoughts on renaming this to DEFAULT_PLAN_NAME as well? Since there could be a world where this plan isn't free, and this is technically just the name of the plan and not the full object
There was a problem hiding this comment.
I feel like the name "Free" makes more sense here. It'll be easier to look for "Free" references if someone wants to update it. "Default plan name" could potentially reference any other plan
There was a problem hiding this comment.
What about the concept of "name" vs. "plan"? Would you say it's still clear that this is just a reference to the plan name and not the plan object?
| trial_start_date=trial_start_date, | ||
| trial_end_date=trial_end_date, | ||
| ) | ||
| plan_service = PlanService(current_org=current_org) | ||
|
|
||
| basic_plan = FREE_PLAN_REPRESENTATIONS[PlanName.BASIC_PLAN_NAME.value] | ||
| developer_plan = FREE_PLAN_REPRESENTATIONS[DEFAULT_FREE_PLAN] | ||
| assert plan_service.current_org == current_org |
There was a problem hiding this comment.
A lot of these assertions probably aren't needed, we can probably just assert the plan name and call it a day
There was a problem hiding this comment.
yah we're doing that in a lot of places, good thing to consider refactoring in a separate PR
As part of milestone 4, we are introducing the new userrs-developer plan, which is basically a free version of a team plan.
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.