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

ref: Update shared + Add missing fields for plan representation #1068

Merged
merged 4 commits into from
Dec 23, 2024
Merged

ref: Update shared + Add missing fields for plan representation #1068

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
95b055b
inital setup for plan represntation
RulaKhaled Dec 20, 2024
c1f6d03
update with tests
RulaKhaled Dec 20, 2024
ff6c17f
update shared version, tests
RulaKhaled Dec 20, 2024
c23e09d
Merge branch 'main' into plan-repre
ajay-sentry Dec 20, 2024
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
6 changes: 2 additions & 4 deletions api/internal/owner/serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import logging
from dataclasses import asdict
from datetime import datetime

from dateutil.relativedelta import relativedelta
Expand Down Expand Up @@ -127,10 +126,9 @@ def validate_value(self, value):
current_owner = self.context["request"].current_owner

plan_service = PlanService(current_org=current_org)
available_plans = [
asdict(plan) for plan in plan_service.available_plans(current_owner)
plan_values = [
plan["value"] for plan in plan_service.available_plans(current_owner)
]
plan_values = [plan["value"] for plan in available_plans]
if value not in plan_values:
if value in SENTRY_PAID_USER_PLAN_REPRESENTATIONS:
log.warning(
Expand Down
83 changes: 83 additions & 0 deletions graphql_api/tests/test_owner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1078,3 +1078,86 @@ def test_fetch_activated_user_count_when_not_in_org_but_has_shared_account(self)
""" % (other_owner.username)
data = self.gql_request(query, owner=owner)
assert data["owner"]["activatedUserCount"] == 2

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,
},
]
}
}
3 changes: 2 additions & 1 deletion graphql_api/types/owner/owner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ def resolve_plan(owner: Owner, info: GraphQLResolveInfo) -> PlanService:
@require_part_of_org
def resolve_plan_representation(owner: Owner, info: GraphQLResolveInfo) -> PlanData:
info.context["plan_service"] = PlanService(current_org=owner)
return FREE_PLAN_REPRESENTATIONS[PlanName.BASIC_PLAN_NAME.value]
free_plan = FREE_PLAN_REPRESENTATIONS[PlanName.BASIC_PLAN_NAME.value]
return free_plan.convert_to_DTO()


@owner_bindable.field("availablePlans")
Expand Down
6 changes: 6 additions & 0 deletions graphql_api/types/plan_representation/plan_representation.graphql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,10 @@ type PlanRepresentation {
baseUnitPrice: Int!
benefits: [String!]!
monthlyUploadLimit: Int
isEnterprisePlan: Boolean!
isFreePlan: Boolean!
isProPlan: Boolean!
isTeamPlan: Boolean!
isSentryPlan: Boolean!
isTrialPlan: Boolean!
}
46 changes: 38 additions & 8 deletions graphql_api/types/plan_representation/plan_representation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,27 +14,27 @@

@plan_representation_bindable.field("marketingName")
def resolve_marketing_name(plan_data: PlanData, info) -> str:
Copy link
Contributor

Choose a reason for hiding this comment

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

Did we update the PlanData type somewhere to reflect this? Can't remember if that is included in shared

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh yes! shared changes included this, we added convet_to_DTO fn to the type

Copy link
Contributor

Choose a reason for hiding this comment

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

Right but that's just a return type of that function right? The PlanData type still reflects the explicit properties of the dataclass iirc

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't remember if we actually created an interface for convert_to_DTO

Copy link
Contributor

Choose a reason for hiding this comment

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

Overall, because Python typing isn't as strict as TS we won't have any runtime errors i think, but this is creating some additional cognitive burden / magic to unwind these types and see where the additional properties are coming from

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmmm yah it's a bit confusing because we're kinda calculating it on the fly within the class and actually returning a dict type after all, we could enhance this at some point to make the typing clearer, but it's not gonna effect things atm

return plan_data.marketing_name
return plan_data["marketing_name"]


@plan_representation_bindable.field("planName")
def resolve_plan_name(plan_data: PlanData, info) -> str:
return plan_data.value
return plan_data["value"]


@plan_representation_bindable.field("value")
def resolve_plan_value(plan_data: PlanData, info) -> str:
return plan_data.value
return plan_data["value"]


@plan_representation_bindable.field("billingRate")
def resolve_billing_rate(plan_data: PlanData, info) -> Optional[str]:
return plan_data.billing_rate
return plan_data["billing_rate"]


@plan_representation_bindable.field("baseUnitPrice")
def resolve_base_unit_price(plan_data: PlanData, info) -> int:
return plan_data.base_unit_price
return plan_data["base_unit_price"]


@plan_representation_bindable.field("benefits")
Expand All @@ -46,13 +46,43 @@
lambda benefit: benefit.replace(
"Up to 1 user", f"Up to {plan_service.pretrial_users_count} users"
),
plan_data.benefits,
plan_data["benefits"],
)
)
return benefits_with_pretrial_users
return plan_data.benefits
return plan_data["benefits"]

Check warning on line 53 in graphql_api/types/plan_representation/plan_representation.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphql_api/types/plan_representation/plan_representation.py#L53 

Added line #L53 was not covered by tests


@plan_representation_bindable.field("monthlyUploadLimit")
def resolve_monthly_uploads_limit(plan_data: PlanData, info) -> Optional[int]:
return plan_data.monthly_uploads_limit
return plan_data["monthly_uploads_limit"]


@plan_representation_bindable.field("isEnterprisePlan")
def resolve_is_enterprise(plan_data: PlanData, info) -> bool:
return plan_data["is_enterprise_plan"]


@plan_representation_bindable.field("isFreePlan")
def resolve_is_free(plan_data: PlanData, info) -> bool:
return plan_data["is_free_plan"]


@plan_representation_bindable.field("isProPlan")
def resolve_is_pro(plan_data: PlanData, info) -> bool:
return plan_data["is_pro_plan"]


@plan_representation_bindable.field("isTeamPlan")
def resolve_is_team(plan_data: PlanData, info) -> bool:
return plan_data["is_team_plan"]


@plan_representation_bindable.field("isSentryPlan")
def resolve_is_sentry(plan_data: PlanData, info) -> bool:
return plan_data["is_sentry_plan"]


@plan_representation_bindable.field("isTrialPlan")
def resolve_is_trial(plan_data: PlanData, info) -> bool:
return plan_data["is_trial_plan"]
2 changes: 1 addition & 1 deletion requirements.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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/1c4ca00e35d95d1281e0415ce1897f6dbbc6368a.tar.gz#egg=shared
https://github.com/codecov/shared/archive/abc6b36feceb05c3b754050061436574111058f9.tar.gz#egg=shared
https://github.com/photocrowd/django-cursor-pagination/archive/f560902696b0c8509e4d95c10ba0d62700181d84.tar.gz
idna>=3.7
minio
Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ sentry-sdk[celery]==2.13.0
# shared
setproctitle==1.1.10
# via -r requirements.in
shared @ https://github.com/codecov/shared/archive/1c4ca00e35d95d1281e0415ce1897f6dbbc6368a.tar.gz
shared @ https://github.com/codecov/shared/archive/abc6b36feceb05c3b754050061436574111058f9.tar.gz
# via -r requirements.in
simplejson==3.17.2
# via -r requirements.in
Expand Down
Loading