fix(billing): max seats stays out of sync after plan upgrades - #8278
fix(billing): max seats stays out of sync after plan upgrades#8278bardock-2393 wants to merge 1 commit into
Conversation
Seats and API calls bought as Chargebee addons only ever reached the subscription information cache. The subscription record itself was updated from the plan's own metadata, and only when the plan id changed, so buying extra seats left `max_seats` stale indefinitely. Take the seat and API call allowances from the metadata extracted from the webhook payload, which accounts for the plan and its addons.
|
@bardock-2393 is attempting to deploy a commit to the Flagsmith Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthrough
Estimated code review effort: 2 (Simple) | ~10 minutes ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8278 +/- ##
==========================================
- Coverage 98.73% 98.59% -0.15%
==========================================
Files 1567 1567
Lines 62379 62414 +35
==========================================
- Hits 61591 61536 -55
- Misses 788 878 +90 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a2472028-d4a5-471e-9cd5-c39baf5c37a0
📒 Files selected for processing (2)
api/organisations/chargebee/webhook_handlers.pyapi/tests/unit/organisations/test_unit_organisations_views.py
| mock_extract_subscription_metadata.return_value = ChargebeeObjMetadata( | ||
| seats=6, | ||
| api_calls=1_000_000, | ||
| projects=10, | ||
| chargebee_email=chargebee_email, | ||
| ) | ||
|
|
||
| data = { | ||
| "content": { | ||
| "subscription": { | ||
| "status": "active", | ||
| "id": subscription.subscription_id, | ||
| "plan_id": subscription.plan, | ||
| }, | ||
| "customer": {"email": chargebee_email}, | ||
| } | ||
| } | ||
|
|
||
| # When | ||
| response = admin_client.post( | ||
| url, data=json.dumps(data), content_type="application/json" | ||
| ) | ||
|
|
||
| # Then | ||
| assert response.status_code == status.HTTP_200_OK | ||
| mock_get_plan_meta_data.assert_not_called() | ||
|
|
||
| subscription.refresh_from_db() | ||
| assert subscription.max_seats == 6 | ||
|
|
||
| subscription_information_cache = ( | ||
| OrganisationSubscriptionInformationCache.objects.get(organisation=organisation) | ||
| ) | ||
| assert subscription_information_cache.allowed_seats == 6 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Test addon-aware API-call allowances.
Both tests use api_calls=1_000_000, which equals the plan allowance. Neither test checks the persisted API-call allowance. A defect in the max_api_calls synchronisation would pass these tests.
api/tests/unit/organisations/test_unit_organisations_views.py#L1425-L1458: use an addon-adjustedapi_callsvalue and assertsubscription.max_api_callsandsubscription_information_cache.allowed_30d_api_calls.api/tests/unit/organisations/test_unit_organisations_views.py#L1479-L1513: set extractedapi_callsabove the mocked plan value and assert both persisted values.
🧰 Tools
🪛 ast-grep (0.45.1)
[info] 1444-1444: use jsonify instead of json.dumps for JSON output
Context: json.dumps(data)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
📍 Affects 1 file
api/tests/unit/organisations/test_unit_organisations_views.py#L1425-L1458(this comment)api/tests/unit/organisations/test_unit_organisations_views.py#L1479-L1513
1 participant
Seat allowances bought as Chargebee addons never reached the organisation's subscription record, so the seat count shown on Users and Permissions stayed at the old value after a seat upgrade while Organisation Billing showed the correct one. Anyone who buys extra seats sees a stale — and lower — limit on the page they use to manage their team.
The subscription record was only refreshed when the Chargebee plan ID itself changed, and even then it took its allowances from the plan's own metadata, which knows nothing about addons. Since seat upgrades are sold as addons, the record was never updated. The subscription information cache behind the billing page was already deriving its figures from the plan and its addons, which is why the two pages disagreed. The webhook now takes the seat and API call allowances from that same addon-aware source.
Changes
Closes #8276
Review effort: 2/5
How did you test this code?
Automated. Two tests were added to
test_unit_organisations_views.py, both of which fail onmainand pass with this change:test_chargebee_webhook__seats_added_to_same_plan__updates_seats— an addon raises the seat count while the plan is unchanged. Fails without the fix withassert 5 == 6.test_chargebee_webhook__plan_changed_with_addons__updates_seats— a plan change where addons add seats on top of the plan's own allowance. Fails without the fix withassert 5 == 8, the plan's figure winning over the real one.tests/unit/organisationsandtests/unit/sales_dashboardpass in full (370 passed, 1 skipped), as domake typecheckon the changed files and the pre-commit suite.One thing to flag
This keeps the record in sync from here on: an organisation's seat count corrects itself the next time Chargebee sends a subscription webhook for it. Organisations whose record is already stale today are not backfilled, so they will keep showing the old number until their subscription next changes.
_update_caches_with_chargebee_data, the periodic reconciliation task, already holds both the subscription and the addon-aware metadata and could heal every stale record in a few lines. I have left it out because backfilling live billing data seemed like your call rather than mine — happy to add it if you would like it in this PR.