Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 13 additions & 0 deletions api/organisations/chargebee/webhook_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,19 @@ def process_subscription(request: Request) -> Response: # noqa: C901
chargebee_subscription=subscription,
customer_email=customer["email"],
)

# `update_plan` only reads the plan's own metadata, so seats and API calls
# bought as addons never reach the subscription, and a subscription whose
# plan is unchanged is never updated at all. The extracted metadata accounts
# for both the plan and its addons, so use it as the source of truth.
if (
existing_subscription.max_seats != subscription_metadata.seats
or existing_subscription.max_api_calls != subscription_metadata.api_calls
):
existing_subscription.max_seats = subscription_metadata.seats
existing_subscription.max_api_calls = subscription_metadata.api_calls
existing_subscription.save()

osic_defaults = {
"chargebee_updated_at": timezone.now(),
"allowed_30d_api_calls": subscription_metadata.api_calls,
Expand Down
111 changes: 111 additions & 0 deletions api/tests/unit/organisations/test_unit_organisations_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1402,6 +1402,117 @@ def test_chargebee_webhook__plan_changed__updates_seats_and_api_calls( # type:
assert subscription_information_cache.chargebee_updated_at > updated_at


@mock.patch("organisations.models.get_plan_meta_data")
@mock.patch("organisations.chargebee.webhook_handlers.extract_subscription_metadata")
def test_chargebee_webhook__seats_added_to_same_plan__updates_seats(
mock_extract_subscription_metadata: MagicMock,
mock_get_plan_meta_data: MagicMock,
subscription: Subscription,
admin_client: APIClient,
organisation: Organisation,
) -> None:
# Given
chargebee_email = "chargebee@test.com"
url = reverse("api-v1:chargebee-webhook")

subscription.subscription_id = "sub-id"
subscription.plan = "scale-up-v2"
subscription.max_seats = 5
subscription.max_api_calls = 1_000_000
subscription.save()

# An additional seat addon raises the allowance without changing the plan.
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
Comment on lines +1425 to +1458

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 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-adjusted api_calls value and assert subscription.max_api_calls and subscription_information_cache.allowed_30d_api_calls.
  • api/tests/unit/organisations/test_unit_organisations_views.py#L1479-L1513: set extracted api_calls above 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



@mock.patch("organisations.models.get_plan_meta_data")
@mock.patch("organisations.chargebee.webhook_handlers.extract_subscription_metadata")
def test_chargebee_webhook__plan_changed_with_addons__updates_seats(
mock_extract_subscription_metadata: MagicMock,
mock_get_plan_meta_data: MagicMock,
subscription: Subscription,
admin_client: APIClient,
organisation: Organisation,
) -> None:
# Given
chargebee_email = "chargebee@test.com"
url = reverse("api-v1:chargebee-webhook")

subscription.subscription_id = "sub-id"
subscription.save()

# The plan on its own allows 5 seats, but the subscription also carries
# addons worth 3 more.
mock_get_plan_meta_data.return_value = {"seats": 5, "api_calls": 1_000_000}
mock_extract_subscription_metadata.return_value = ChargebeeObjMetadata(
seats=8,
api_calls=1_000_000,
projects=10,
chargebee_email=chargebee_email,
)

data = {
"content": {
"subscription": {
"status": "active",
"id": subscription.subscription_id,
"plan_id": "scale-up-v2",
},
"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

subscription.refresh_from_db()
assert subscription.plan == "scale-up-v2"
assert subscription.max_seats == 8

subscription_information_cache = (
OrganisationSubscriptionInformationCache.objects.get(organisation=organisation)
)
assert subscription_information_cache.allowed_seats == 8


def test_delete_organisation__other_org_exists__preserves_other_subscriptions( # type: ignore[no-untyped-def]
admin_client, admin_user, organisation, subscription
) -> None:
Expand Down
Loading