From 42049fcca8372dc32b4dab0fb350b9d8dc15ab34 Mon Sep 17 00:00:00 2001 From: Novak Zaballa <41410593+novakzaballa@users.noreply.github.com> Date: Tue, 22 Aug 2023 09:43:25 -0400 Subject: [PATCH] feat: Use get-metadata-subscription to get max_api_calls (#2279) --- api/conftest.py | 4 +- api/organisations/chargebee/__init__.py | 3 +- api/organisations/chargebee/chargebee.py | 54 +++-- ...iptioninformationcache_allowed_projects.py | 18 ++ .../migrations/0045_auto_20230802_1956.py | 23 ++ api/organisations/models.py | 24 ++- api/organisations/subscription_info_cache.py | 22 +- api/organisations/subscriptions/constants.py | 7 + api/organisations/subscriptions/metadata.py | 4 +- .../subscriptions/subscription_service.py | 6 +- api/organisations/tasks.py | 13 +- api/organisations/tests/test_models.py | 20 +- api/organisations/tests/test_views.py | 203 +++++++++++++----- api/organisations/views.py | 27 ++- .../templates/sales_dashboard/home.html | 10 +- api/sales_dashboard/urls.py | 11 +- api/sales_dashboard/views.py | 32 ++- .../unit/organisations/chargebee/conftest.py | 38 ++++ .../test_unit_chargebee_chargebee.py | 92 ++++++-- .../test_unit_subscription_service.py | 2 +- ...t_organisations_subscription_info_cache.py | 10 +- frontend/web/components/App.js | 12 +- 22 files changed, 482 insertions(+), 153 deletions(-) create mode 100644 api/organisations/migrations/0044_organisationsubscriptioninformationcache_allowed_projects.py create mode 100644 api/organisations/migrations/0045_auto_20230802_1956.py diff --git a/api/conftest.py b/api/conftest.py index d5743d2cfb87..eb1dff40d5a2 100644 --- a/api/conftest.py +++ b/api/conftest.py @@ -1,4 +1,4 @@ -import typing +from typing import Tuple import pytest from django.contrib.contenttypes.models import ContentType @@ -292,7 +292,7 @@ def environment_api_key(environment): @pytest.fixture() -def master_api_key(organisation) -> typing.Tuple[MasterAPIKey, str]: +def master_api_key(organisation) -> Tuple[MasterAPIKey, str]: master_api_key, key = MasterAPIKey.objects.create_key( name="test_key", organisation=organisation ) diff --git a/api/organisations/chargebee/__init__.py b/api/organisations/chargebee/__init__.py index 1ab68c9c69c7..663a9b137aa2 100644 --- a/api/organisations/chargebee/__init__.py +++ b/api/organisations/chargebee/__init__.py @@ -1,5 +1,6 @@ from .chargebee import ( # noqa add_single_seat, + extract_subscription_metadata, get_customer_id_from_subscription_id, get_hosted_page_url_for_subscription_upgrade, get_max_api_calls_for_plan, @@ -7,5 +8,5 @@ get_plan_meta_data, get_portal_url, get_subscription_data_from_hosted_page, - get_subscription_metadata, + get_subscription_metadata_from_id, ) diff --git a/api/organisations/chargebee/chargebee.py b/api/organisations/chargebee/chargebee.py index f3113dbd0e85..919a566a4a24 100644 --- a/api/organisations/chargebee/chargebee.py +++ b/api/organisations/chargebee/chargebee.py @@ -103,7 +103,28 @@ def get_hosted_page_url_for_subscription_upgrade( return checkout_existing_response.hosted_page.url -def get_subscription_metadata( +def extract_subscription_metadata( + chargebee_subscription: dict, + customer_email: str, +) -> ChargebeeObjMetadata: + chargebee_addons = chargebee_subscription.get("addons", []) + chargebee_cache = ChargebeeCache() + subscription_metadata: ChargebeeObjMetadata = chargebee_cache.plans[ + chargebee_subscription["plan_id"] + ] + subscription_metadata.chargebee_email = customer_email + + for addon in chargebee_addons: + quantity = getattr(addon, "quantity", None) or 1 + addon_metadata: ChargebeeObjMetadata = ( + chargebee_cache.addons[addon["id"]] * quantity + ) + subscription_metadata = subscription_metadata + addon_metadata + + return subscription_metadata + + +def get_subscription_metadata_from_id( subscription_id: str, ) -> typing.Optional[ChargebeeObjMetadata]: if not (subscription_id and subscription_id.strip() != ""): @@ -112,20 +133,13 @@ def get_subscription_metadata( with suppress(ChargebeeAPIError): chargebee_result = chargebee.Subscription.retrieve(subscription_id) - subscription = chargebee_result.subscription - addons = subscription.addons or [] - - chargebee_cache = ChargebeeCache() - plan_metadata = chargebee_cache.plans[subscription.plan_id] - subscription_metadata = plan_metadata - subscription_metadata.chargebee_email = chargebee_result.customer.email - - for addon in addons: - quantity = getattr(addon, "quantity", None) or 1 - addon_metadata = chargebee_cache.addons[addon.id] * quantity - subscription_metadata = subscription_metadata + addon_metadata + chargebee_subscription = _convert_chargebee_subscription_to_dictionary( + chargebee_result.subscription + ) - return subscription_metadata + return extract_subscription_metadata( + chargebee_subscription, chargebee_result.customer.email + ) def cancel_subscription(subscription_id: str): @@ -169,3 +183,15 @@ def add_single_seat(subscription_id: str): ) logger.error(msg) raise UpgradeSeatsError(msg) from e + + +def _convert_chargebee_subscription_to_dictionary( + chargebee_subscription: chargebee.Subscription, +) -> dict: + chargebee_subscription_dict = vars(chargebee_subscription) + # convert the addons into a list of dictionaries since vars don't do it recursively + chargebee_subscription_dict["addons"] = [ + vars(addon) for addon in chargebee_subscription.addons + ] + + return chargebee_subscription_dict diff --git a/api/organisations/migrations/0044_organisationsubscriptioninformationcache_allowed_projects.py b/api/organisations/migrations/0044_organisationsubscriptioninformationcache_allowed_projects.py new file mode 100644 index 000000000000..84233b54c27c --- /dev/null +++ b/api/organisations/migrations/0044_organisationsubscriptioninformationcache_allowed_projects.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.20 on 2023-07-14 16:59 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('organisations', '0043_add_created_at_and_updated_at_to_organisationwebhook'), + ] + + operations = [ + migrations.AddField( + model_name='organisationsubscriptioninformationcache', + name='allowed_projects', + field=models.IntegerField(default=1), + ), + ] diff --git a/api/organisations/migrations/0045_auto_20230802_1956.py b/api/organisations/migrations/0045_auto_20230802_1956.py new file mode 100644 index 000000000000..26ff8b279669 --- /dev/null +++ b/api/organisations/migrations/0045_auto_20230802_1956.py @@ -0,0 +1,23 @@ +# Generated by Django 3.2.20 on 2023-08-02 19:56 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('organisations', '0044_organisationsubscriptioninformationcache_allowed_projects'), + ] + + operations = [ + migrations.AddField( + model_name='organisationsubscriptioninformationcache', + name='chargebee_updated_at', + field=models.DateTimeField(null=True), + ), + migrations.AddField( + model_name='organisationsubscriptioninformationcache', + name='influx_updated_at', + field=models.DateTimeField(null=True), + ), + ] diff --git a/api/organisations/models.py b/api/organisations/models.py index c0de2827bd3d..dbea4a7fa885 100644 --- a/api/organisations/models.py +++ b/api/organisations/models.py @@ -20,12 +20,13 @@ get_max_seats_for_plan, get_plan_meta_data, get_portal_url, - get_subscription_metadata, + get_subscription_metadata_from_id, ) from organisations.chargebee.chargebee import add_single_seat from organisations.chargebee.chargebee import ( cancel_subscription as cancel_chargebee_subscription, ) +from organisations.chargebee.metadata import ChargebeeObjMetadata from organisations.subscriptions.constants import ( CHARGEBEE, FREE_PLAN_ID, @@ -95,6 +96,11 @@ def num_seats(self): def has_subscription(self) -> bool: return hasattr(self, "subscription") and bool(self.subscription.subscription_id) + def has_subscription_information_cache(self) -> bool: + return hasattr(self, "subscription_information_cache") and bool( + self.subscription_information_cache + ) + @property def is_paid(self): return self.has_subscription() and self.subscription.cancellation_date is None @@ -211,14 +217,23 @@ def get_portal_url(self, redirect_url): def get_subscription_metadata(self) -> BaseSubscriptionMetadata: metadata = None - if self.subscription_id == TRIAL_SUBSCRIPTION_ID: metadata = BaseSubscriptionMetadata( seats=self.max_seats, api_calls=self.max_api_calls ) if self.payment_method == CHARGEBEE and self.subscription_id: - metadata = get_subscription_metadata(self.subscription_id) + if self.organisation.has_subscription_information_cache(): + # Getting the data from the subscription information cache because + # data is guaranteed to be up to date by using a Chargebee webhook. + metadata = ChargebeeObjMetadata( + seats=self.organisation.subscription_information_cache.allowed_seats, + api_calls=self.organisation.subscription_information_cache.allowed_30d_api_calls, + projects=self.organisation.subscription_information_cache.allowed_projects, + chargebee_email=self.organisation.subscription_information_cache.chargebee_email, + ) + else: + metadata = get_subscription_metadata_from_id(self.subscription_id) elif self.payment_method == XERO and self.subscription_id: metadata = XeroSubscriptionMetadata( seats=self.max_seats, api_calls=self.max_api_calls, projects=None @@ -271,6 +286,8 @@ class OrganisationSubscriptionInformationCache(models.Model): on_delete=models.CASCADE, ) updated_at = models.DateTimeField(auto_now=True) + chargebee_updated_at = models.DateTimeField(auto_now=False, null=True) + influx_updated_at = models.DateTimeField(auto_now=False, null=True) api_calls_24h = models.IntegerField(default=0) api_calls_7d = models.IntegerField(default=0) @@ -278,5 +295,6 @@ class OrganisationSubscriptionInformationCache(models.Model): allowed_seats = models.IntegerField(default=1) allowed_30d_api_calls = models.IntegerField(default=50000) + allowed_projects = models.IntegerField(default=1) chargebee_email = models.EmailField(blank=True, max_length=254, null=True) diff --git a/api/organisations/subscription_info_cache.py b/api/organisations/subscription_info_cache.py index 97db83c658f8..b4e69d98980c 100644 --- a/api/organisations/subscription_info_cache.py +++ b/api/organisations/subscription_info_cache.py @@ -2,20 +2,19 @@ from app_analytics.influxdb_wrapper import get_top_organisations from django.conf import settings -from django.utils import timezone -from .chargebee import get_subscription_metadata +from .chargebee import get_subscription_metadata_from_id from .models import Organisation, OrganisationSubscriptionInformationCache -from .subscriptions.constants import CHARGEBEE +from .subscriptions.constants import CHARGEBEE, SubscriptionCacheEntity OrganisationSubscriptionInformationCacheDict = typing.Dict[ int, OrganisationSubscriptionInformationCache ] -def update_caches(): +def update_caches(update_cache_entities: typing.Tuple[SubscriptionCacheEntity, ...]): """ - Update the cache objects for all active organisations in the database. + Update the cache objects for an update_cache_entity in the database. """ organisations = Organisation.objects.select_related( @@ -30,14 +29,16 @@ def update_caches(): for org in organisations } - _update_caches_with_influx_data(organisation_info_cache_dict) - _update_caches_with_chargebee_data(organisations, organisation_info_cache_dict) + if SubscriptionCacheEntity.INFLUX in update_cache_entities: + _update_caches_with_influx_data(organisation_info_cache_dict) + + if SubscriptionCacheEntity.CHARGEBEE in update_cache_entities: + _update_caches_with_chargebee_data(organisations, organisation_info_cache_dict) to_update = [] to_create = [] for subscription_info_cache in organisation_info_cache_dict.values(): - subscription_info_cache.updated_at = timezone.now() if subscription_info_cache.id: to_update.append(subscription_info_cache) else: @@ -53,7 +54,8 @@ def update_caches(): "allowed_seats", "allowed_30d_api_calls", "chargebee_email", - "updated_at", + "chargebee_updated_at", + "influx_updated_at", ], ) @@ -99,7 +101,7 @@ def _update_caches_with_chargebee_data( ): continue - metadata = get_subscription_metadata(subscription.subscription_id) + metadata = get_subscription_metadata_from_id(subscription.subscription_id) if not metadata: continue diff --git a/api/organisations/subscriptions/constants.py b/api/organisations/subscriptions/constants.py index e259b506f73d..e0aa61f724b8 100644 --- a/api/organisations/subscriptions/constants.py +++ b/api/organisations/subscriptions/constants.py @@ -1,3 +1,5 @@ +from enum import Enum + from organisations.subscriptions.metadata import BaseSubscriptionMetadata MAX_SEATS_IN_FREE_PLAN = 1 @@ -24,3 +26,8 @@ projects=MAX_PROJECTS_IN_FREE_PLAN, ) FREE_PLAN_ID = "free" + + +class SubscriptionCacheEntity(Enum): + INFLUX = "INFLUX" + CHARGEBEE = "CHARGEBEE" diff --git a/api/organisations/subscriptions/metadata.py b/api/organisations/subscriptions/metadata.py index 387100ec8af7..91ca6d5545e0 100644 --- a/api/organisations/subscriptions/metadata.py +++ b/api/organisations/subscriptions/metadata.py @@ -42,7 +42,9 @@ def __str__(self): return ( "%s Subscription Metadata (seats: %d, api_calls: %d, projects: %s, chargebee_email: %s)" % ( - self.payment_source.title(), + self.payment_source.title() + if self.payment_source is not None + else "unknown payment source", self.seats, self.api_calls, str(self.projects) if self.projects is not None else "no limit", diff --git a/api/organisations/subscriptions/subscription_service.py b/api/organisations/subscriptions/subscription_service.py index 8f2d806028b9..eb888e94126f 100644 --- a/api/organisations/subscriptions/subscription_service.py +++ b/api/organisations/subscriptions/subscription_service.py @@ -1,6 +1,4 @@ -from organisations.chargebee import ( - get_subscription_metadata as get_chargebee_subscription_metadata, -) +from organisations.chargebee import get_subscription_metadata_from_id from organisations.models import Organisation from organisations.subscriptions.xero.metadata import XeroSubscriptionMetadata @@ -14,7 +12,7 @@ def get_subscription_metadata(organisation: Organisation) -> BaseSubscriptionMet seats=max_seats, api_calls=max_api_calls, projects=max_projects ) if organisation.subscription.payment_method == CHARGEBEE: - chargebee_subscription_metadata = get_chargebee_subscription_metadata( + chargebee_subscription_metadata = get_subscription_metadata_from_id( organisation.subscription.subscription_id ) if chargebee_subscription_metadata is not None: diff --git a/api/organisations/tasks.py b/api/organisations/tasks.py index d483ca0be5c2..17726da44912 100644 --- a/api/organisations/tasks.py +++ b/api/organisations/tasks.py @@ -6,6 +6,8 @@ from task_processor.decorators import register_task_handler from users.models import FFAdminUser +from .subscriptions.constants import SubscriptionCacheEntity + ALERT_EMAIL_MESSAGE = ( "Organisation %s has used %d seats which is over their plan limit of %d (plan: %s)" ) @@ -30,5 +32,12 @@ def send_org_over_limit_alert(organisation_id): @register_task_handler() -def update_organisation_subscription_information_caches(): - subscription_info_cache.update_caches() +def update_organisation_subscription_information_influx_cache(): + subscription_info_cache.update_caches((SubscriptionCacheEntity.INFLUX,)) + + +@register_task_handler() +def update_organisation_subscription_information_cache(): + subscription_info_cache.update_caches( + (SubscriptionCacheEntity.CHARGEBEE, SubscriptionCacheEntity.INFLUX) + ) diff --git a/api/organisations/tests/test_models.py b/api/organisations/tests/test_models.py index ec3e87305218..5a29ae6a2fbd 100644 --- a/api/organisations/tests/test_models.py +++ b/api/organisations/tests/test_models.py @@ -243,26 +243,30 @@ def test_organisation_is_paid_returns_false_if_cancelled_subscription_exists( def test_subscription_get_subscription_metadata_returns_cb_metadata_for_cb_subscription( + organisation, mocker, ): # Given - subscription = Subscription( - payment_method=CHARGEBEE, subscription_id="cb-subscription" + seats = 10 + api_calls = 50000000 + OrganisationSubscriptionInformationCache.objects.create( + organisation=organisation, allowed_seats=seats, allowed_30d_api_calls=api_calls ) - expected_metadata = ChargebeeObjMetadata(seats=10, api_calls=50000000, projects=10) + expected_metadata = ChargebeeObjMetadata( + seats=seats, api_calls=api_calls, projects=10 + ) mock_cb_get_subscription_metadata = mocker.patch( - "organisations.models.get_subscription_metadata" + "organisations.models.Subscription.get_subscription_metadata" ) mock_cb_get_subscription_metadata.return_value = expected_metadata # When - subscription_metadata = subscription.get_subscription_metadata() + subscription_metadata = organisation.subscription.get_subscription_metadata() # Then - mock_cb_get_subscription_metadata.assert_called_once_with( - subscription.subscription_id - ) + mock_cb_get_subscription_metadata.assert_called_once_with() + assert subscription_metadata == expected_metadata diff --git a/api/organisations/tests/test_views.py b/api/organisations/tests/test_views.py index ffd98a52c688..5920afc99047 100644 --- a/api/organisations/tests/test_views.py +++ b/api/organisations/tests/test_views.py @@ -7,6 +7,7 @@ from django.contrib.auth import get_user_model from django.core import mail from django.urls import reverse +from freezegun import freeze_time from pytz import UTC from rest_framework import status from rest_framework.test import APIClient, override_settings @@ -19,6 +20,7 @@ from organisations.models import ( Organisation, OrganisationRole, + OrganisationSubscriptionInformationCache, OrganisationWebhook, Subscription, ) @@ -580,43 +582,6 @@ def setUp(self) -> None: ) self.subscription = Subscription.objects.get(organisation=self.organisation) - @mock.patch("organisations.models.get_plan_meta_data") - def test_when_subscription_plan_is_changed_max_seats_and_max_api_calls_are_updated( - self, mock_get_plan_meta_data - ): - # Given - new_plan_id = "new-plan-id" - new_max_seats = 3 - new_max_api_calls = 100 - mock_get_plan_meta_data.return_value = { - "seats": new_max_seats, - "api_calls": new_max_api_calls, - } - - data = { - "content": { - "subscription": { - "status": "active", - "id": self.subscription_id, - "plan_id": new_plan_id, - } - } - } - - # When - res = self.client.post( - self.url, data=json.dumps(data), content_type="application/json" - ) - - # Then - assert res.status_code == status.HTTP_200_OK - - # and - self.subscription.refresh_from_db() - assert self.subscription.plan == new_plan_id - assert self.subscription.max_seats == new_max_seats - assert self.subscription.max_api_calls == new_max_api_calls - @mock.patch("organisations.models.cancel_chargebee_subscription") def test_when_subscription_is_set_to_non_renewing_then_cancellation_date_set_and_alert_sent( self, mocked_cancel_chargebee_subscription @@ -629,7 +594,8 @@ def test_when_subscription_is_set_to_non_renewing_then_cancellation_date_set_and "status": "non_renewing", "id": self.subscription_id, "current_term_end": datetime.timestamp(cancellation_date), - } + }, + "customer": {"email": self.cb_user.email}, } } @@ -657,7 +623,8 @@ def test_when_subscription_is_cancelled_then_cancellation_date_set_and_alert_sen "status": "cancelled", "id": self.subscription_id, "current_term_end": datetime.timestamp(cancellation_date), - } + }, + "customer": {"email": self.cb_user.email}, } } @@ -673,20 +640,29 @@ def test_when_subscription_is_cancelled_then_cancellation_date_set_and_alert_sen # and assert len(mail.outbox) == 1 + @mock.patch("organisations.views.extract_subscription_metadata") def test_when_cancelled_subscription_is_renewed_then_subscription_activated_and_no_cancellation_email_sent( self, + mock_extract_subscription_metadata, ): # Given self.subscription.cancellation_date = datetime.now(tz=UTC) - timedelta(days=1) self.subscription.save() mail.outbox.clear() + mock_extract_subscription_metadata.return_value = ChargebeeObjMetadata( + seats=3, + api_calls=100, + projects=1, + chargebee_email=self.cb_user.email, + ) data = { "content": { "subscription": { "status": "active", "id": self.subscription_id, - } + }, + "customer": {"email": self.cb_user.email}, } } @@ -707,7 +683,10 @@ def test_when_chargebee_webhook_received_with_unknown_subscription_id_then_404( ): # Given data = { - "content": {"subscription": {"status": "active", "id": "some-random-id"}} + "content": { + "subscription": {"status": "active", "id": "some-random-id"}, + "customer": {"email": self.cb_user.email}, + } } # When @@ -793,7 +772,44 @@ def test_trigger_sample_webhook_calls_trigger_sample_webhook_method_with_correct assert args[0].url == self.valid_webhook_url -def test_get_subscription_metadata( +def test_get_subscription_metadata_when_subscription_information_cache_exist( + organisation, admin_client, chargebee_subscription +): + # Given + expected_seats = 10 + expected_projects = 5 + expected_projects = 3 + expected_api_calls = 100 + expected_chargebee_email = "test@example.com" + + OrganisationSubscriptionInformationCache.objects.create( + organisation=organisation, + allowed_seats=expected_seats, + allowed_projects=expected_projects, + allowed_30d_api_calls=expected_api_calls, + chargebee_email=expected_chargebee_email, + ) + + url = reverse( + "api-v1:organisations:organisation-get-subscription-metadata", + args=[organisation.pk], + ) + + # When + response = admin_client.get(url) + + # Then + assert response.status_code == status.HTTP_200_OK + assert response.json() == { + "max_seats": expected_seats, + "max_projects": expected_projects, + "max_api_calls": expected_api_calls, + "payment_source": CHARGEBEE, + "chargebee_email": expected_chargebee_email, + } + + +def test_get_subscription_metadata_when_subscription_information_cache_does_not_exist( mocker, organisation, admin_client, chargebee_subscription ): # Given @@ -803,7 +819,7 @@ def test_get_subscription_metadata( expected_chargebee_email = "test@example.com" get_subscription_metadata = mocker.patch( - "organisations.models.get_subscription_metadata", + "organisations.models.get_subscription_metadata_from_id", return_value=ChargebeeObjMetadata( seats=expected_seats, projects=expected_projects, @@ -839,7 +855,7 @@ def test_get_subscription_metadata_returns_404_if_the_organisation_have_no_subsc ): # Given get_subscription_metadata = mocker.patch( - "organisations.models.get_subscription_metadata" + "organisations.models.get_subscription_metadata_from_id" ) url = reverse( @@ -856,14 +872,9 @@ def test_get_subscription_metadata_returns_404_if_the_organisation_have_no_subsc def test_get_subscription_metadata_returns_defaults_if_chargebee_error( - mocker, organisation, admin_client, chargebee_subscription + organisation, admin_client, chargebee_subscription ): # Given - get_subscription_metadata = mocker.patch( - "organisations.models.get_subscription_metadata" - ) - get_subscription_metadata.return_value = None - url = reverse( "api-v1:organisations:organisation-get-subscription-metadata", args=[organisation.pk], @@ -874,9 +885,7 @@ def test_get_subscription_metadata_returns_defaults_if_chargebee_error( # Then assert response.status_code == status.HTTP_200_OK - get_subscription_metadata.assert_called_once_with( - chargebee_subscription.subscription_id - ) + assert response.json() == { "max_seats": MAX_SEATS_IN_FREE_PLAN, "max_api_calls": MAX_API_CALLS_IN_FREE_PLAN, @@ -952,6 +961,94 @@ def test_organisation_get_influx_data( assert response.json() == {"events_list": []} +@freeze_time("2023-07-31 12:00:00") +@pytest.mark.parametrize( + "plan_id, max_seats, max_api_calls, max_projects, is_updated", + [ + ("plan-id", 3, 100, 3, False), + ("updated-plan-id", 5, 500, 10, True), + ], +) +@mock.patch("organisations.models.get_plan_meta_data") +@mock.patch("organisations.views.extract_subscription_metadata") +def test_when_plan_is_changed_max_seats_and_max_api_calls_are_updated( + mock_extract_subscription_metadata, + mock_get_plan_meta_data, + subscription, + admin_client, + organisation, + plan_id, + max_seats, + max_api_calls, + max_projects, + is_updated, +): + # Given + chargebee_email = "chargebee@test.com" + url = reverse("api-v1:chargebee-webhook") + updated_at = datetime.now(tz=UTC) - timedelta( + days=1 + ) # The timestamp representing the last update time, one day ago from the current time. + + mock_get_plan_meta_data.return_value = { + "seats": max_seats, + "api_calls": max_api_calls, + } + mock_extract_subscription_metadata.return_value = ChargebeeObjMetadata( + seats=max_seats, + api_calls=max_api_calls, + projects=max_projects, + chargebee_email=chargebee_email, + ) + + data = { + "content": { + "subscription": { + "status": "active", + "id": subscription.subscription_id, + "plan_id": plan_id, + }, + "customer": {"email": chargebee_email}, + } + } + + if is_updated: + subscription_information_cache = ( + OrganisationSubscriptionInformationCache.objects.create( + organisation=organisation, + allowed_seats=1, + allowed_30d_api_calls=10, + allowed_projects=1, + chargebee_email=chargebee_email, + chargebee_updated_at=updated_at, + influx_updated_at=None, + ) + ) + + # When + res = admin_client.post(url, data=json.dumps(data), content_type="application/json") + + subscription_information_cache = ( + OrganisationSubscriptionInformationCache.objects.get(organisation=organisation) + ) + + subscription.refresh_from_db() + # Then + assert res.status_code == status.HTTP_200_OK + assert subscription.plan == plan_id + assert subscription.max_seats == max_seats + assert subscription.max_api_calls == max_api_calls + + assert subscription_information_cache.allowed_seats == max_seats + assert subscription_information_cache.allowed_30d_api_calls == max_api_calls + assert subscription_information_cache.allowed_projects == max_projects + assert subscription_information_cache.chargebee_email == chargebee_email + assert subscription_information_cache.chargebee_updated_at + assert subscription_information_cache.influx_updated_at is None + if is_updated: + assert subscription_information_cache.chargebee_updated_at > updated_at + + def test_delete_organisation_does_not_delete_all_subscriptions_from_the_database( admin_client, admin_user, organisation, subscription ): diff --git a/api/organisations/views.py b/api/organisations/views.py index 998a16373857..76f054335770 100644 --- a/api/organisations/views.py +++ b/api/organisations/views.py @@ -9,6 +9,7 @@ get_multiple_event_list_for_organisation, ) from django.contrib.sites.shortcuts import get_current_site +from django.utils import timezone from drf_yasg.utils import swagger_auto_schema from rest_framework import status, viewsets from rest_framework.authentication import BasicAuthentication @@ -25,6 +26,7 @@ from organisations.models import ( Organisation, OrganisationRole, + OrganisationSubscriptionInformationCache, OrganisationWebhook, Subscription, ) @@ -54,6 +56,8 @@ from webhooks.mixins import TriggerSampleWebhookMixin from webhooks.webhooks import WebhookType +from .chargebee import extract_subscription_metadata + logger = logging.getLogger(__name__) @@ -184,6 +188,7 @@ def get_subscription_metadata(self, request, pk): subscription_details = organisation.subscription.get_subscription_metadata() serializer = self.get_serializer(instance=subscription_details) + return Response(serializer.data) @action(detail=True, methods=["GET"], url_path="portal-url") @@ -269,24 +274,40 @@ def chargebee_webhook(request): """ if request.data.get("content") and "subscription" in request.data.get("content"): - subscription_data = request.data["content"]["subscription"] + subscription_data: dict = request.data["content"]["subscription"] + customer_email: str = request.data["content"]["customer"]["email"] try: existing_subscription = Subscription.objects.get( subscription_id=subscription_data.get("id") ) except (Subscription.DoesNotExist, Subscription.MultipleObjectsReturned): - error_message = ( + error_message: str = ( "Couldn't get unique subscription for ChargeBee id %s" % subscription_data.get("id") ) logger.error(error_message) return Response(status=status.HTTP_200_OK) - subscription_status = subscription_data.get("status") if subscription_status == "active": if subscription_data.get("plan_id") != existing_subscription.plan: existing_subscription.update_plan(subscription_data.get("plan_id")) + subscription_metadata = extract_subscription_metadata( + chargebee_subscription=subscription_data, + customer_email=customer_email, + ) + OrganisationSubscriptionInformationCache.objects.update_or_create( + organisation_id=existing_subscription.organisation_id, + defaults={ + "chargebee_updated_at": timezone.now(), + "allowed_30d_api_calls": subscription_metadata.api_calls, + "allowed_seats": subscription_metadata.seats, + "organisation_id": existing_subscription.organisation_id, + "allowed_projects": subscription_metadata.projects, + "chargebee_email": subscription_metadata.chargebee_email, + }, + ) + elif subscription_status in ("non_renewing", "cancelled"): existing_subscription.cancel( datetime.fromtimestamp(subscription_data.get("current_term_end")), diff --git a/api/sales_dashboard/templates/sales_dashboard/home.html b/api/sales_dashboard/templates/sales_dashboard/home.html index 82b7abf1b10e..24a37f5a991d 100644 --- a/api/sales_dashboard/templates/sales_dashboard/home.html +++ b/api/sales_dashboard/templates/sales_dashboard/home.html @@ -12,17 +12,17 @@

Organisations

-
+ {% csrf_token %} - +
-
+ {% csrf_token %} - +
- Last updated at: {{ subscription_information_caches_updated_at|default:"never" }} + Last Influx data updated at: {{ subscription_information_caches_influx_updated_at|default:"never" }}
diff --git a/api/sales_dashboard/urls.py b/api/sales_dashboard/urls.py index ef1511800541..ffd53040684c 100644 --- a/api/sales_dashboard/urls.py +++ b/api/sales_dashboard/urls.py @@ -39,9 +39,14 @@ name="download-org-data", ), path( - "update-organisation-subscription-information-caches", - views.trigger_update_organisation_subscription_information_caches, - name="update-organisation-subscription-information-caches", + "update-organisation-subscription-information-influx-cache", + views.trigger_update_organisation_subscription_information_influx_cache, + name="update-organisation-subscription-information-influx-cache", + ), + path( + "update-organisation-subscription-information-cache", + views.trigger_update_organisation_subscription_information_cache, + name="update-organisation-subscription-information-cache", ), path( "update-chargebee-cache", diff --git a/api/sales_dashboard/views.py b/api/sales_dashboard/views.py index 115391f16565..f2ec1937ff04 100644 --- a/api/sales_dashboard/views.py +++ b/api/sales_dashboard/views.py @@ -32,7 +32,8 @@ get_subscription_metadata, ) from organisations.tasks import ( - update_organisation_subscription_information_caches, + update_organisation_subscription_information_cache, + update_organisation_subscription_information_influx_cache, ) from .forms import EmailUsageForm, MaxAPICallsForm, MaxSeatsForm @@ -87,20 +88,23 @@ def get_context_data(self, **kwargs): data["sort_field"] = self.request.GET.get("sort_field") data["sort_direction"] = self.request.GET.get("sort_direction") - # Use the most recent OrganisationSubscriptionInformationCache object to determine when the caches - # were last updated. + # Use the most recent "influx_updated_at" in + # OrganisationSubscriptionInformationCache object to determine + # when the caches were last updated. try: - subscription_information_caches_updated_at = ( - OrganisationSubscriptionInformationCache.objects.order_by("-updated_at") + subscription_information_caches_influx_updated_at = ( + OrganisationSubscriptionInformationCache.objects.order_by( + "-influx_updated_at" + ) .first() - .updated_at.strftime("%H:%M:%S %d/%m/%Y") + .influx_updated_at.strftime("%H:%M:%S %d/%m/%Y") ) except AttributeError: - subscription_information_caches_updated_at = None + subscription_information_caches_influx_updated_at = None data[ - "subscription_information_caches_updated_at" - ] = subscription_information_caches_updated_at + "subscription_information_caches_influx_updated_at" + ] = subscription_information_caches_influx_updated_at return data @@ -218,8 +222,14 @@ def download_org_data(request, organisation_id): @staff_member_required() -def trigger_update_organisation_subscription_information_caches(request): - update_organisation_subscription_information_caches.delay() +def trigger_update_organisation_subscription_information_influx_cache(request): + update_organisation_subscription_information_influx_cache.delay() + return HttpResponseRedirect(reverse("sales_dashboard:index")) + + +@staff_member_required() +def trigger_update_organisation_subscription_information_cache(request): + update_organisation_subscription_information_cache.delay() return HttpResponseRedirect(reverse("sales_dashboard:index")) diff --git a/api/tests/unit/organisations/chargebee/conftest.py b/api/tests/unit/organisations/chargebee/conftest.py index 6f08c6712cc1..46f14ba09c4c 100644 --- a/api/tests/unit/organisations/chargebee/conftest.py +++ b/api/tests/unit/organisations/chargebee/conftest.py @@ -1,4 +1,9 @@ import pytest +from pytest_mock import MockerFixture +from tests.unit.organisations.chargebee.test_unit_chargebee_chargebee import ( + MockChargeBeeAddOn, + MockChargeBeeSubscriptionResponse, +) from organisations.chargebee.metadata import ChargebeeObjMetadata @@ -6,3 +11,36 @@ @pytest.fixture def chargebee_object_metadata(): return ChargebeeObjMetadata(seats=10, api_calls=100, projects=10) + + +@pytest.fixture +def mock_subscription_response( + mocker: MockerFixture, + chargebee_object_metadata: ChargebeeObjMetadata, +) -> MockChargeBeeSubscriptionResponse: + # Given + plan_id = "plan-id" + addon_id = "addon-id" + subscription_id = "subscription-id" + customer_email = "test@example.com" + + # Let's create a (mocked) subscription object + mock_subscription_response = MockChargeBeeSubscriptionResponse( + subscription_id=subscription_id, + plan_id=plan_id, + customer_email=customer_email, + addons=[MockChargeBeeAddOn(addon_id=addon_id, quantity=1)], + ) + mocked_chargebee = mocker.patch("organisations.chargebee.chargebee.chargebee") + + # tie that subscription object to the mocked chargebee object + mocked_chargebee.Subscription.retrieve.return_value = mock_subscription_response + + # now, let's mock chargebee cache object + mocked_chargebee_cache = mocker.patch( + "organisations.chargebee.chargebee.ChargebeeCache", autospec=True + ) + mocked_chargebee_cache.return_value.plans = {plan_id: chargebee_object_metadata} + mocked_chargebee_cache.return_value.addons = {addon_id: chargebee_object_metadata} + + return mock_subscription_response diff --git a/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py b/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py index 86e78fc8647a..2e4d7ab22a0c 100644 --- a/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py +++ b/api/tests/unit/organisations/chargebee/test_unit_chargebee_chargebee.py @@ -8,6 +8,7 @@ from organisations.chargebee import ( add_single_seat, + extract_subscription_metadata, get_customer_id_from_subscription_id, get_hosted_page_url_for_subscription_upgrade, get_max_api_calls_for_plan, @@ -15,10 +16,11 @@ get_plan_meta_data, get_portal_url, get_subscription_data_from_hosted_page, - get_subscription_metadata, + get_subscription_metadata_from_id, ) from organisations.chargebee.chargebee import cancel_subscription from organisations.chargebee.constants import ADDITIONAL_SEAT_ADDON_ID +from organisations.chargebee.metadata import ChargebeeObjMetadata from organisations.subscriptions.exceptions import ( CannotCancelChargebeeSubscription, UpgradeSeatsError, @@ -260,35 +262,81 @@ def test_get_hosted_page_url_for_subscription_upgrade(self): ) -@pytest.mark.parametrize("addon_quantity", (None, 1)) -def test_get_subscription_metadata(mocker, chargebee_object_metadata, addon_quantity): +def test_extract_subscription_metadata( + mock_subscription_response, chargebee_object_metadata: ChargebeeObjMetadata +): # Given + status = "status" plan_id = "plan-id" addon_id = "addon-id" subscription_id = "subscription-id" customer_email = "test@example.com" - # Let's create a (mocked) subscription object - mock_subscription_response = MockChargeBeeSubscriptionResponse( - subscription_id=subscription_id, - plan_id=plan_id, - customer_email=customer_email, - addons=[MockChargeBeeAddOn(addon_id=addon_id, quantity=addon_quantity)], + subscription = { + "status": status, + "id": subscription_id, + "plan_id": plan_id, + "addons": [ + { + "id": addon_id, + "quantity": 2, + "unit_price": 0, + "amount": 0, + } + ], + } + + # When + subscription_metadata = extract_subscription_metadata( + subscription, + customer_email, ) - mocked_chargebee = mocker.patch("organisations.chargebee.chargebee.chargebee") - # tie that subscription object to the mocked chargebee object - mocked_chargebee.Subscription.retrieve.return_value = mock_subscription_response + # Then + assert subscription_metadata.seats == chargebee_object_metadata.seats * 2 + assert subscription_metadata.api_calls == chargebee_object_metadata.api_calls * 2 + assert subscription_metadata.projects == chargebee_object_metadata.projects * 2 + assert subscription_metadata.chargebee_email == customer_email + - # now, let's mock chargebee cache object - mocked_chargebee_cache = mocker.patch( - "organisations.chargebee.chargebee.ChargebeeCache", autospec=True +def test_extract_subscription_metadata_when_addon_list_is_empty( + mock_subscription_response, chargebee_object_metadata: ChargebeeObjMetadata +): + # Given + status = "status" + plan_id = "plan-id" + subscription_id = "subscription-id" + customer_email = "test@example.com" + + subscription = { + "status": status, + "id": subscription_id, + "plan_id": plan_id, + "addons": [], + } + + # When + subscription_metadata = extract_subscription_metadata( + subscription, + customer_email, ) - mocked_chargebee_cache.return_value.plans = {plan_id: chargebee_object_metadata} - mocked_chargebee_cache.return_value.addons = {addon_id: chargebee_object_metadata} + + # Then + assert subscription_metadata.seats == chargebee_object_metadata.seats + assert subscription_metadata.api_calls == chargebee_object_metadata.api_calls + assert subscription_metadata.projects == chargebee_object_metadata.projects + assert subscription_metadata.chargebee_email == customer_email + + +def test_get_subscription_metadata_from_id( + mock_subscription_response, chargebee_object_metadata: ChargebeeObjMetadata +): + # Given + customer_email = "test@example.com" + subscription_id = mock_subscription_response.subscription.id # When - subscription_metadata = get_subscription_metadata(subscription_id) + subscription_metadata = get_subscription_metadata_from_id(subscription_id) # Then assert subscription_metadata.seats == chargebee_object_metadata.seats * 2 @@ -341,7 +389,7 @@ class MockException(Exception): ) -def test_get_subscription_metadata_returns_null_if_chargebee_error( +def test_get_subscription_metadata_from_id_returns_null_if_chargebee_error( mocker, chargebee_object_metadata ): # Given @@ -353,7 +401,7 @@ def test_get_subscription_metadata_returns_null_if_chargebee_error( subscription_id = "foo" # arbitrary subscription id # When - subscription_metadata = get_subscription_metadata(subscription_id) + subscription_metadata = get_subscription_metadata_from_id(subscription_id) # Then assert subscription_metadata is None @@ -363,14 +411,14 @@ def test_get_subscription_metadata_returns_null_if_chargebee_error( "subscription_id", [None, "", " "], ) -def test_get_subscription_metadata_returns_none_for_invalid_subscription_id( +def test_get_subscription_metadata_from_id_returns_none_for_invalid_subscription_id( mocker, chargebee_object_metadata, subscription_id ): # Given mocked_chargebee = mocker.patch("organisations.chargebee.chargebee.chargebee") # When - subscription_metadata = get_subscription_metadata(subscription_id) + subscription_metadata = get_subscription_metadata_from_id(subscription_id) # Then mocked_chargebee.Subscription.retrieve.assert_not_called() diff --git a/api/tests/unit/organisations/subscriptions/test_unit_subscription_service.py b/api/tests/unit/organisations/subscriptions/test_unit_subscription_service.py index c31c8143b1d8..57d57fd39b50 100644 --- a/api/tests/unit/organisations/subscriptions/test_unit_subscription_service.py +++ b/api/tests/unit/organisations/subscriptions/test_unit_subscription_service.py @@ -32,7 +32,7 @@ def test_get_subscription_metadata_uses_chargebee_data_if_chargebee_subscription projects = 20 api_calls = 30 mocked_get_chargebee_subscription_metadata = mocker.patch( - "organisations.subscriptions.subscription_service.get_chargebee_subscription_metadata", + "organisations.subscriptions.subscription_service.get_subscription_metadata_from_id", autospec=True, return_value=ChargebeeObjMetadata( seats=seats, projects=projects, api_calls=api_calls diff --git a/api/tests/unit/organisations/test_unit_organisations_subscription_info_cache.py b/api/tests/unit/organisations/test_unit_organisations_subscription_info_cache.py index 01296dd10ec8..6c3edf1d8b7f 100644 --- a/api/tests/unit/organisations/test_unit_organisations_subscription_info_cache.py +++ b/api/tests/unit/organisations/test_unit_organisations_subscription_info_cache.py @@ -1,5 +1,6 @@ from organisations.chargebee.metadata import ChargebeeObjMetadata from organisations.subscription_info_cache import update_caches +from organisations.subscriptions.constants import SubscriptionCacheEntity from task_processor.task_run_method import TaskRunMethod @@ -19,15 +20,18 @@ def test_update_caches(mocker, organisation, chargebee_subscription, settings): chargebee_metadata = ChargebeeObjMetadata(seats=15, api_calls=1000000) mocked_get_subscription_metadata = mocker.patch( - "organisations.subscription_info_cache.get_subscription_metadata" + "organisations.subscription_info_cache.get_subscription_metadata_from_id" ) mocked_get_subscription_metadata.return_value = chargebee_metadata # When - update_caches() + subscription_cache_entities = ( + SubscriptionCacheEntity.INFLUX, + SubscriptionCacheEntity.CHARGEBEE, + ) + update_caches(subscription_cache_entities) # Then - assert organisation.subscription_information_cache.updated_at assert ( organisation.subscription_information_cache.api_calls_24h == organisation_usage["24h"] diff --git a/frontend/web/components/App.js b/frontend/web/components/App.js index 34e2115a9545..c987b0e7b3e0 100644 --- a/frontend/web/components/App.js +++ b/frontend/web/components/App.js @@ -38,6 +38,7 @@ const App = class extends Component { state = { activeOrganisation: 0, asideIsVisible: !isMobile, + maxApiCalls: 0, pin: '', totalApiCalls: 0, } @@ -202,7 +203,7 @@ const App = class extends Component { } const { location } = this.props const pathname = location.pathname - const { asideIsVisible, totalApiCalls } = this.state + const { asideIsVisible, maxApiCalls, totalApiCalls } = this.state const match = matchPath(pathname, { exact: false, path: '/project/:projectId/environment/:environmentId', @@ -340,8 +341,7 @@ const App = class extends Component { style={ Utils.calculaterRemainingCallsPercentage( totalApiCalls, - organisation.subscription - ?.max_api_calls, + maxApiCalls, ) && Utils.getFlagsmithHasFeature( 'max_api_calls_alert', @@ -365,8 +365,7 @@ const App = class extends Component { > {Utils.calculaterRemainingCallsPercentage( totalApiCalls, - organisation.subscription - ?.max_api_calls, + maxApiCalls, ) && Utils.getFlagsmithHasFeature( 'max_api_calls_alert', @@ -376,8 +375,7 @@ const App = class extends Component { {`You used ${Format.shortenNumber( totalApiCalls, )}/${Format.shortenNumber( - organisation.subscription - ?.max_api_calls, + maxApiCalls, )} requests. Click to`}{' '} {'Upgrade'}