-
Notifications
You must be signed in to change notification settings - Fork 28
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
fix: available plans for users while trialing (#731) #230
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #230 +/- ##
==========================================
- Coverage 95.67% 95.67% -0.01%
==========================================
Files 605 605
Lines 15475 15469 -6
==========================================
- Hits 14806 14800 -6
Misses 669 669
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Codecov Report
@@ Coverage Diff @@
## main #230 +/- ##
=======================================
Coverage ? 95.67%
=======================================
Files ? 603
Lines ? 15456
Branches ? 0
=======================================
Hits ? 14787
Misses ? 669
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more.
|
plan/test_plan.py
Outdated
@@ -628,9 +629,37 @@ def test_available_plans_for_sentry_customer_users_trial_plan_ongoing_trial_more | |||
expected_result.append(BASIC_PLAN) | |||
expected_result += PR_AUTHOR_PAID_USER_PLAN_REPRESENTATIONS.values() | |||
expected_result += SENTRY_PAID_USER_PLAN_REPRESENTATIONS.values() | |||
expected_result += TEAM_PLAN_REPRESENTATIONS.values() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
plan/test_plan.py
Outdated
self.current_org.save() | ||
|
||
# Can not do Team plan when at 11 activated users | ||
assert plan_service.available_plans(owner=self.owner) == expected_result[:3] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we adjust the expected_result[:3]
to perhaps another list? expected_result_without_team
or something? If we add another plan eventually, we'll have to refactor this test then
plan/test_plan.py
Outdated
|
||
assert plan_service.available_plans(owner=self.owner) == expected_result | ||
|
||
def test_available_plans_for_more_than_10_activated_users(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And can you indicate in the test title this is for trialing users too? test_available_plans_for_trialing_users_with_more_than_10_activated_users
or something like that.
I'm okay with splitting this into two tests too, as this says more than 10 activated users, but you test a scenario with < 10 first, then > 10
Codecov Report
@@ Coverage Diff @@
## main #230 +/- ##
=======================================
+ Coverage 95.58 95.59 +0.01
=======================================
Files 719 717 -2
Lines 15882 15936 +54
=======================================
+ Hits 15180 15234 +54
Misses 702 702
Flags with carried forward coverage won't be shown. Click here to find out more.
|
* Test ats run (#229) * Test ats run * Test ats run * Test ats run * Test ats run * Test ats run * Test ats run * Test ats run * Test ats run * feat: Emit sync repo message to Shelter on repo update (#215) * feat: add prometheus instrumentation to requests, db accesses, and models (#202) * feat: add prometheus instrumentation to requests, db accesses, and models * switch to multiprocessing mode * add nullchecking for gunicorn worker/pid * only call multiprocess.mark_process_dead if prometheus multiprocess is enabled * add comment about public prometheus route --------- Co-authored-by: Trent Schmidt <trent@codecov.io> * fix: make GraphQL types matches nullability with django models (#220) Audit all the GraphQL API types to make sure that the nullabilitity (nullable or not nullable) matches that of the database columns (via Django ORM). * feat: Add index on pulls (repoid, id) (#217) * feat: Add index on pulls (repoid, id) * Fix migration conflict * Fix XSS vulnerability by removing ref_type from Response (#211) * fix: available plans for users while trialing (#731) (#230) Fix a bug where a user while trialing does not have Team plans available to upgrade to. This happens because while trialing we set plan_user_count to 1000 while trialing, so we will check plan_activated_users while user is trialing. * fix: Skip login if GitHub OAuth does not return access_token (#236) * Send github oauth access token expiry with cookie (#237) * initial commit * remove method * lint * sort imports * reintroduce variable * Update small badge (#238) * chore: Moves the small badge mask over to leave space for 100% while still looking good with 2 digits * fix small badge expected svg util * fix small badge tests * fix: Upgrade Django version to fix security vulnerabilities (#239) Fixes Django security vulnerabilities by updating to the fix version: Django Denial-of-service in django.utils.text.Truncator Django potential denial of service vulnerability in UsernameField on Windows Django Denial of service vulnerability in django.utils.encoding.uri_to_iri * feat: Modify billing for Team plan (#637) (#225) - When signing up to a new paid plan, it will have Team plan choice, and checks for plan user limit. - Also update limit from 1000 to 2500 and refactor a variable name - Add isPublic option to measurements filter - Change marketing name of Pro Team to Pro * fix: Handle Shelter storage paths in upload download handler (#234) * fix: Handle shetler storage paths in upload download handler * fix: Handle shetler storage paths in upload download handler * Add test for shelter storage path * Updating workflows (#242) * Fix urllib and opentelemetry-instrumentation dependencies (#213) * Update urllib3 dependency * Fix opentelementry-instrumentation dependency Fixes: codecov/internal-issues#103 Fixes: codecov/internal-issues#104 Signed-off-by: joseph-sentry <joseph.sawaya@sentry.io> * Make migration info log less confusing (#221) Clearly state whether migrations failed or succeded. Co-authored-by: Trent Schmidt <trent@codecov.io> * Use alpine 3.18 --------- Signed-off-by: joseph-sentry <joseph.sawaya@sentry.io> Co-authored-by: scott-codecov <scott@codecov.io> Co-authored-by: matt-codecov <137832199+matt-codecov@users.noreply.github.com> Co-authored-by: JerrySentry <142266253+JerrySentry@users.noreply.github.com> Co-authored-by: joseph-sentry <136376984+joseph-sentry@users.noreply.github.com> Co-authored-by: Rohit Vinnakota <148245014+rohitvinnakota-codecov@users.noreply.github.com> Co-authored-by: Terry <87824812+terry-codecov@users.noreply.github.com> Co-authored-by: Andreas Bergmeier <51448674+AndreasBergmeier6176@users.noreply.github.com>
Purpose/Motivation
What is the feature? Why is this being done?
Fix a bug where a user while trialing does not have Team plans available to upgrade to.
This happens because while trialing we set
plan_user_count
to 1000 while trialing, so we will checkplan_activated_users
while user is trialing.Links to relevant tickets
codecov/engineering-team#731
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.