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

API: Migrate to Plan / Tier Tables #1099

Merged
merged 22 commits into from
Jan 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
947954a
starting transition, need the other PR merged to do imports and test …
ajay-sentry Jan 15, 2025
a06b8c8
other api stuff
ajay-sentry Jan 15, 2025
1f2e63b
revert trial status changes, update other instances of plan stuff
ajay-sentry Jan 15, 2025
5050704
fix a couple instances... still need to fix availablePlans
ajay-sentry Jan 15, 2025
40abd2d
a few more references for PAID and TEAM_PLANS
ajay-sentry Jan 15, 2025
16f7fba
missing s
ajay-sentry Jan 16, 2025
3a5b3af
add missing imports + refactor to call 1 query
RulaKhaled Jan 20, 2025
d26259a
update tests
RulaKhaled Jan 20, 2025
f21a5f1
testing locally, fix the tier checks and first test
ajay-sentry Jan 21, 2025
ba63895
starting to fix tests and some logic
ajay-sentry Jan 21, 2025
91ea64b
update api with mocked plans
RulaKhaled Jan 22, 2025
42488fb
fix all tests except account_viewset
ajay-sentry Jan 22, 2025
1bfc6d8
all tests passing now, fixed a couple small logic bugs too
ajay-sentry Jan 23, 2025
99a63e1
fix this suite, when using ttestcase need to have setup vs setupClass
ajay-sentry Jan 23, 2025
49f2af8
clean up the logic for update plan
ajay-sentry Jan 23, 2025
e2560c5
Merge branch 'main' into Ajay/milestone-3-migration
ajay-sentry Jan 24, 2025
34b1677
rebase
ajay-sentry Jan 27, 2025
3f7fe6f
Merge branch 'main' into Ajay/milestone-3-migration
ajay-sentry Jan 29, 2025
4fa07ca
staging check
ajay-sentry Jan 29, 2025
b6ad275
update with shared version
RulaKhaled Jan 30, 2025
ea7582c
PR comments
ajay-sentry Jan 30, 2025
e9662a2
Merge branch 'main' into Ajay/milestone-3-migration
ajay-sentry Jan 30, 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
32 changes: 19 additions & 13 deletions api/internal/owner/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,12 @@
from rest_framework import serializers
from rest_framework.exceptions import PermissionDenied
from shared.plan.constants import (
PAID_PLANS,
SENTRY_PAID_USER_PLAN_REPRESENTATIONS,
TEAM_PLAN_MAX_USERS,
TEAM_PLAN_REPRESENTATIONS,
TierName,
)
from shared.plan.service import PlanService

from codecov_auth.models import Owner
from codecov_auth.models import Owner, Plan
from services.billing import BillingService
from services.sentry import send_user_webhook as send_sentry_webhook

Expand Down Expand Up @@ -137,11 +135,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:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

log.warning(
"Non-Sentry user attempted to transition to Sentry plan",
extra=dict(owner_id=current_owner.pk, plan=value),
)
raise serializers.ValidationError(
f"Invalid value for plan: {value}; must be one of {plan_values}"
)
Expand All @@ -154,8 +147,17 @@ def validate(self, plan: Dict[str, Any]) -> Dict[str, Any]:
detail="You cannot update your plan manually, for help or changes to plan, connect with [email protected]"
)

active_plans = Plan.objects.select_related("tier").filter(
paid_plan=True, is_active=True
)
nora-codecov marked this conversation as resolved.
Show resolved Hide resolved

active_plan_names = set(active_plans.values_list("name", flat=True))
team_tier_plans = active_plans.filter(
tier__tier_name=TierName.TEAM.value
).values_list("name", flat=True)

# Validate quantity here because we need access to whole plan object
if plan["value"] in PAID_PLANS:
if plan["value"] in active_plan_names:
if "quantity" not in plan:
raise serializers.ValidationError(
"Field 'quantity' required for updating to paid plans"
Expand Down Expand Up @@ -184,7 +186,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
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and plan["quantity"] > TEAM_PLAN_MAX_USERS
):
raise serializers.ValidationError(
Expand Down Expand Up @@ -219,7 +221,7 @@ def get_plan(self, phase: Dict[str, Any]) -> str:
plan_name = list(stripe_plan_dict.keys())[
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

list(stripe_plan_dict.values()).index(plan_id)
]
marketing_plan_name = PAID_PLANS[plan_name].billing_rate
marketing_plan_name = Plan.objects.get(name=plan_name).marketing_name
return marketing_plan_name

def get_quantity(self, phase: Dict[str, Any]) -> int:
Expand Down Expand Up @@ -342,7 +344,11 @@ def update(self, instance: Owner, validated_data: Dict[str, Any]) -> object:
instance, desired_plan
)

if desired_plan["value"] in SENTRY_PAID_USER_PLAN_REPRESENTATIONS:
sentry_plans = Plan.objects.filter(
tier__tier_name=TierName.SENTRY.value, is_active=True
).values_list("name", flat=True)

if desired_plan["value"] in sentry_plans:
current_owner = self.context["view"].request.current_owner
send_sentry_webhook(current_owner, instance)

Expand Down
6 changes: 5 additions & 1 deletion api/internal/tests/test_pagination.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
from rest_framework.reverse import reverse
from rest_framework.test import APITestCase
from shared.django_apps.codecov_auth.tests.factories import PlanFactory, TierFactory
from shared.django_apps.core.tests.factories import OwnerFactory
from shared.plan.constants import TierName

from utils.test_utils import Client


class PageNumberPaginationTests(APITestCase):
def setUp(self):
self.client = Client()
self.owner = OwnerFactory(plan="users-free", plan_user_count=5)
tier = TierFactory(tier_name=TierName.BASIC.value)
plan = PlanFactory(tier=tier, is_active=True)
self.owner = OwnerFactory(plan=plan.name, plan_user_count=5)
self.users = [
OwnerFactory(organizations=[self.owner.ownerid]),
OwnerFactory(organizations=[self.owner.ownerid]),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
interactions:
- request:
body: billing_address_collection=required&payment_method_collection=if_required&client_reference_id=65&success_url=http%3A%2F%2Flocalhost%3A3000%2Fplan%2Fgh%2Fjustin47%3Fsuccess&cancel_url=http%3A%2F%2Flocalhost%3A3000%2Fplan%2Fgh%2Fjustin47%3Fcancel&customer=1000&mode=subscription&line_items[0][price]=price_1OCM0gGlVGuVgOrkWDYEBtSL&line_items[0][quantity]=11&subscription_data[metadata][service]=github&subscription_data[metadata][obo_organization]=65&subscription_data[metadata][username]=justin47&subscription_data[metadata][obo_name]=Kelly+Williams&subscription_data[metadata][obo_email]=christopher27%40lopez-welch.com&subscription_data[metadata][obo]=65&tax_id_collection[enabled]=True&customer_update[name]=auto&customer_update[address]=auto
headers:
Accept:
- '*/*'
Accept-Encoding:
- gzip, deflate
Connection:
- keep-alive
Content-Length:
- '744'
Content-Type:
- application/x-www-form-urlencoded
Idempotency-Key:
- 2e7e12e1-9051-4766-b947-1abe985b5e98
Stripe-Version:
- 2024-12-18.acacia
User-Agent:
- Stripe/v1 PythonBindings/11.4.1
X-Stripe-Client-User-Agent:
- '{"bindings_version": "11.4.1", "lang": "python", "publisher": "stripe", "httplib":
"requests", "lang_version": "3.12.8", "platform": "Linux-6.10.14-linuxkit-aarch64-with-glibc2.36",
"uname": "Linux 35c9e7c77efc 6.10.14-linuxkit #1 SMP Fri Nov 29 17:22:03 UTC
2024 aarch64 "}'
method: POST
uri: https://api.stripe.com/v1/checkout/sessions
response:
body:
string: "{\n \"error\": {\n \"code\": \"resource_missing\",\n \"doc_url\":
\"https://stripe.com/docs/error-codes/resource-missing\",\n \"message\":
\"No such customer: '1000'\",\n \"param\": \"customer\",\n \"request_log_url\":
\"https://dashboard.stripe.com/test/logs/req_oevRZUbMiaT1kM?t=1737668618\",\n
\ \"type\": \"invalid_request_error\"\n }\n}\n"
headers:
Access-Control-Allow-Credentials:
- 'true'
Access-Control-Allow-Methods:
- GET, HEAD, PUT, PATCH, POST, DELETE
Access-Control-Allow-Origin:
- '*'
Access-Control-Expose-Headers:
- Request-Id, Stripe-Manage-Version, Stripe-Should-Retry, X-Stripe-External-Auth-Required,
X-Stripe-Privileged-Session-Required
Access-Control-Max-Age:
- '300'
Cache-Control:
- no-cache, no-store
Connection:
- keep-alive
Content-Length:
- '325'
Content-Security-Policy:
- base-uri 'none'; default-src 'none'; form-action 'none'; frame-ancestors 'none';
img-src 'self'; script-src 'self' 'report-sample'; style-src 'self'; upgrade-insecure-requests;
report-uri https://q.stripe.com/csp-violation?q=ieXMYrsoTw4hsNfwL6RhDPN6zQnVwnAAc0QsFb8i8xtl4N7V94VZzDmgDhKzWxW8mHRRg08-d5GW6oHr
Content-Type:
- application/json
Cross-Origin-Opener-Policy-Report-Only:
- same-origin; report-to="coop"
Date:
- Thu, 23 Jan 2025 21:43:38 GMT
Idempotency-Key:
- 2e7e12e1-9051-4766-b947-1abe985b5e98
Original-Request:
- req_oevRZUbMiaT1kM
Report-To:
- '{"group":"coop","max_age":8640,"endpoints":[{"url":"https://q.stripe.com/coop-report"}],"include_subdomains":true}'
Reporting-Endpoints:
- coop="https://q.stripe.com/coop-report"
Request-Id:
- req_oevRZUbMiaT1kM
Server:
- nginx
Strict-Transport-Security:
- max-age=63072000; includeSubDomains; preload
Stripe-Version:
- 2024-12-18.acacia
Vary:
- Origin
X-Content-Type-Options:
- nosniff
X-Stripe-Priority-Routing-Enabled:
- 'true'
X-Stripe-Routing-Context-Priority-Tier:
- api-testmode
X-Wc:
- AB
status:
code: 400
message: Bad Request
version: 1
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
interactions:
- request:
body: billing_address_collection=required&payment_method_collection=if_required&client_reference_id=93&success_url=http%3A%2F%2Flocalhost%3A3000%2Fplan%2Fgh%2Fjocelyn62%3Fsuccess&cancel_url=http%3A%2F%2Flocalhost%3A3000%2Fplan%2Fgh%2Fjocelyn62%3Fcancel&customer=1000&mode=subscription&line_items[0][price]=price_1OCM0gGlVGuVgOrkWDYEBtSL&line_items[0][quantity]=11&subscription_data[metadata][service]=github&subscription_data[metadata][obo_organization]=93&subscription_data[metadata][username]=jocelyn62&subscription_data[metadata][obo_name]=Crystal+Schmitt&subscription_data[metadata][obo_email]=smithamanda%40flowers.biz&subscription_data[metadata][obo]=93&tax_id_collection[enabled]=True&customer_update[name]=auto&customer_update[address]=auto
headers:
Accept:
- '*/*'
Accept-Encoding:
- gzip, deflate
Connection:
- keep-alive
Content-Length:
- '742'
Content-Type:
- application/x-www-form-urlencoded
Idempotency-Key:
- 26000f5a-feb8-4647-ac57-32a5e5ff8729
Stripe-Version:
- 2024-12-18.acacia
User-Agent:
- Stripe/v1 PythonBindings/11.4.1
X-Stripe-Client-Telemetry:
- '{"last_request_metrics": {"request_id": "req_AaY8IvHbbSDcvz", "request_duration_ms":
2}}'
X-Stripe-Client-User-Agent:
- '{"bindings_version": "11.4.1", "lang": "python", "publisher": "stripe", "httplib":
"requests", "lang_version": "3.12.8", "platform": "Linux-6.10.14-linuxkit-aarch64-with-glibc2.36",
"uname": "Linux 35c9e7c77efc 6.10.14-linuxkit #1 SMP Fri Nov 29 17:22:03 UTC
2024 aarch64 "}'
method: POST
uri: https://api.stripe.com/v1/checkout/sessions
response:
body:
string: "{\n \"error\": {\n \"code\": \"resource_missing\",\n \"doc_url\":
\"https://stripe.com/docs/error-codes/resource-missing\",\n \"message\":
\"No such customer: '1000'\",\n \"param\": \"customer\",\n \"request_log_url\":
\"https://dashboard.stripe.com/test/logs/req_k8lY68XdxWIFHo?t=1737668619\",\n
\ \"type\": \"invalid_request_error\"\n }\n}\n"
headers:
Access-Control-Allow-Credentials:
- 'true'
Access-Control-Allow-Methods:
- GET, HEAD, PUT, PATCH, POST, DELETE
Access-Control-Allow-Origin:
- '*'
Access-Control-Expose-Headers:
- Request-Id, Stripe-Manage-Version, Stripe-Should-Retry, X-Stripe-External-Auth-Required,
X-Stripe-Privileged-Session-Required
Access-Control-Max-Age:
- '300'
Cache-Control:
- no-cache, no-store
Connection:
- keep-alive
Content-Length:
- '325'
Content-Security-Policy:
- base-uri 'none'; default-src 'none'; form-action 'none'; frame-ancestors 'none';
img-src 'self'; script-src 'self' 'report-sample'; style-src 'self'; upgrade-insecure-requests;
report-uri https://q.stripe.com/csp-violation?q=1vgSiZ6UPd0qoBXo1Mjsk02GXFGP3M7PXsjua2jiowWQKm8jByxTHbhTeRirsQsrZ7jscQLjXtdCc_sh
Content-Type:
- application/json
Cross-Origin-Opener-Policy-Report-Only:
- same-origin; report-to="coop"
Date:
- Thu, 23 Jan 2025 21:43:39 GMT
Idempotency-Key:
- 26000f5a-feb8-4647-ac57-32a5e5ff8729
Original-Request:
- req_k8lY68XdxWIFHo
Report-To:
- '{"group":"coop","max_age":8640,"endpoints":[{"url":"https://q.stripe.com/coop-report"}],"include_subdomains":true}'
Reporting-Endpoints:
- coop="https://q.stripe.com/coop-report"
Request-Id:
- req_k8lY68XdxWIFHo
Server:
- nginx
Strict-Transport-Security:
- max-age=63072000; includeSubDomains; preload
Stripe-Version:
- 2024-12-18.acacia
Vary:
- Origin
X-Content-Type-Options:
- nosniff
X-Stripe-Priority-Routing-Enabled:
- 'true'
X-Stripe-Routing-Context-Priority-Tier:
- api-testmode
X-Wc:
- AB
status:
code: 400
message: Bad Request
version: 1
Loading
Loading