fix: Refactor existing Chargebee webhooks for subscriptions (#3047)

api/organisations/chargebee/serializers.py

@@ -1,5 +1,10 @@
 from rest_framework.exceptions import ValidationError
-from rest_framework.serializers import CharField, Serializer
+from rest_framework.serializers import (
+    CharField,
+    IntegerField,
+    ListField,
+    Serializer,
+)
 
 
 class PaymentFailedInvoiceSerializer(Serializer):
@@ -12,6 +17,37 @@ def validate_dunning_status(self, value):
         return value
 
 
+class ProcessSubscriptionCustomerSerializer(Serializer):
+    email = CharField(allow_null=False)
+
+
+class ProcessSubscriptionAddonsSerializer(Serializer):
+    id = CharField()
+    quantity = IntegerField()
+    unit_price = IntegerField()
+    amount = IntegerField()
+    object = CharField()
+
+
+class ProcessSubscriptionSubscriptionSerializer(Serializer):
+    id = CharField(allow_null=False)
+    status = CharField(allow_null=False)
+    plan_id = CharField(allow_null=True, required=False, default=None)
+    current_term_end = IntegerField(required=False, default=None)
+    addons = ListField(
+        child=ProcessSubscriptionAddonsSerializer(), required=False, default=list
+    )
+
+
+class ProcessSubscriptionContentSerializer(Serializer):
+    customer = ProcessSubscriptionCustomerSerializer(required=True)
+    subscription = ProcessSubscriptionSubscriptionSerializer(required=True)
+
+
+class ProcessSubscriptionSerializer(Serializer):
+    content = ProcessSubscriptionContentSerializer(required=True)
+
+
 class PaymentSucceededInvoiceSerializer(Serializer):
     subscription_id = CharField(allow_null=False)
 

api/organisations/chargebee/webhook_handlers.py

@@ -1,18 +1,28 @@
 import logging
+from datetime import datetime
 
+from django.utils import timezone
 from rest_framework import status
 from rest_framework.exceptions import ValidationError
 from rest_framework.request import Request
 from rest_framework.response import Response
 
 from organisations.chargebee.tasks import update_chargebee_cache
-from organisations.models import Subscription
+from organisations.models import (
+    OrganisationSubscriptionInformationCache,
+    Subscription,
+)
 from organisations.subscriptions.constants import (
     SUBSCRIPTION_BILLING_STATUS_ACTIVE,
     SUBSCRIPTION_BILLING_STATUS_DUNNING,
 )
 
-from .serializers import PaymentFailedSerializer, PaymentSucceededSerializer
+from .chargebee import extract_subscription_metadata
+from .serializers import (
+    PaymentFailedSerializer,
+    PaymentSucceededSerializer,
+    ProcessSubscriptionSerializer,
+)
 
 logger = logging.getLogger(__name__)
 
@@ -91,3 +101,59 @@ def payment_succeeded(request: Request) -> Response:
     subscription.save()
 
     return Response(status=status.HTTP_200_OK)
+
+
+def process_subscription(request: Request) -> Response:
+    serializer = ProcessSubscriptionSerializer(data=request.data)
+
+    # Since this function is a catchall, we're not surprised if
+    # other webhook events fail to process.
+    try:
+        serializer.is_valid(raise_exception=True)
+    except ValidationError:
+        return Response(status=status.HTTP_200_OK)
+
+    subscription = serializer.validated_data["content"]["subscription"]
+    customer = serializer.validated_data["content"]["customer"]
+    try:
+        existing_subscription = Subscription.objects.get(
+            subscription_id=subscription["id"]
+        )
+    except (Subscription.DoesNotExist, Subscription.MultipleObjectsReturned):
+        logger.warning(
+            f"Couldn't get unique subscription for ChargeBee id {subscription['id']}"
+        )
+        return Response(status=status.HTTP_200_OK)
+
+    if subscription["status"] in ("non_renewing", "cancelled"):
+        existing_subscription.cancel(
+            datetime.fromtimestamp(subscription.get("current_term_end")).replace(
+                tzinfo=timezone.utc
+            ),
+            update_chargebee=False,
+        )
+        return Response(status=status.HTTP_200_OK)
+
+    if subscription["status"] != "active":
+        # Nothing to do, so return early.
+        return Response(status=status.HTTP_200_OK)
+
+    if subscription["plan_id"] != existing_subscription.plan:
+        existing_subscription.update_plan(subscription["plan_id"])
+
+    subscription_metadata = extract_subscription_metadata(
+        chargebee_subscription=subscription,
+        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,
+        },
+    )
+    return Response(status=status.HTTP_200_OK)

api/organisations/views.py

@@ -2,14 +2,12 @@
 from __future__ import unicode_literals
 
 import logging
-from datetime import datetime
 
 from app_analytics.influxdb_wrapper import (
     get_events_for_organisation,
     get_multiple_event_list_for_organisation,
 )
 from core.helpers import get_current_site_url
-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,9 +23,7 @@
 from organisations.models import (
     Organisation,
     OrganisationRole,
-    OrganisationSubscriptionInformationCache,
     OrganisationWebhook,
-    Subscription,
 )
 from organisations.permissions.models import OrganisationPermissionModel
 from organisations.permissions.permissions import (
@@ -55,8 +51,6 @@
 from webhooks.mixins import TriggerSampleWebhookMixin
 from webhooks.webhooks import WebhookType
 
-from .chargebee import extract_subscription_metadata
-
 logger = logging.getLogger(__name__)
 
 
@@ -281,50 +275,8 @@ def chargebee_webhook(request: Request) -> Response:
     if event_type in webhook_event_types.CACHE_REBUILD_TYPES:
         return webhook_handlers.cache_rebuild_event(request)
 
-    if request.data.get("content") and "subscription" in request.data.get("content"):
-        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: str = (
-                "Couldn't get unique subscription for ChargeBee id %s"
-                % subscription_data.get("id")
-            )
-            logger.warning(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")
-                ).replace(tzinfo=timezone.utc),
-                update_chargebee=False,
-            )
-
-    return Response(status=status.HTTP_200_OK)
+    # Catchall handlers for finding subscription related processing data.
+    return webhook_handlers.process_subscription(request)
 
 
 class OrganisationWebhookViewSet(viewsets.ModelViewSet, TriggerSampleWebhookMixin):

api/tests/unit/organisations/test_unit_organisations_views.py

@@ -11,6 +11,7 @@
 from django.core import mail
 from django.db.models import Model
 from django.urls import reverse
+from django.utils import timezone
 from freezegun import freeze_time
 from pytest_mock import MockerFixture
 from pytz import UTC
@@ -588,7 +589,9 @@ def setUp(self) -> None:
         )
         self.subscription = Subscription.objects.get(organisation=self.organisation)
 
-    @mock.patch("organisations.views.extract_subscription_metadata")
+    @mock.patch(
+        "organisations.chargebee.webhook_handlers.extract_subscription_metadata"
+    )
     def test_chargebee_webhook(
         self, mock_extract_subscription_metadata: MagicMock
     ) -> None:
@@ -633,12 +636,13 @@ def test_when_subscription_is_set_to_non_renewing_then_cancellation_date_set_and
     ):
         # Given
         cancellation_date = datetime.now(tz=UTC) + timedelta(days=1)
+        current_term_end = int(datetime.timestamp(cancellation_date))
         data = {
             "content": {
                 "subscription": {
                     "status": "non_renewing",
                     "id": self.subscription_id,
-                    "current_term_end": datetime.timestamp(cancellation_date),
+                    "current_term_end": current_term_end,
                 },
                 "customer": {"email": self.cb_user.email},
             }
@@ -651,7 +655,9 @@ def test_when_subscription_is_set_to_non_renewing_then_cancellation_date_set_and
 
         # Then
         self.subscription.refresh_from_db()
-        assert self.subscription.cancellation_date == cancellation_date
+        assert self.subscription.cancellation_date == datetime.utcfromtimestamp(
+            current_term_end
+        ).replace(tzinfo=timezone.utc)
 
         # and
         assert len(mail.outbox) == 1
@@ -662,12 +668,13 @@ def test_when_subscription_is_cancelled_then_cancellation_date_set_and_alert_sen
     ):
         # Given
         cancellation_date = datetime.now(tz=UTC) + timedelta(days=1)
+        current_term_end = int(datetime.timestamp(cancellation_date))
         data = {
             "content": {
                 "subscription": {
                     "status": "cancelled",
                     "id": self.subscription_id,
-                    "current_term_end": datetime.timestamp(cancellation_date),
+                    "current_term_end": current_term_end,
                 },
                 "customer": {"email": self.cb_user.email},
             }
@@ -680,12 +687,16 @@ def test_when_subscription_is_cancelled_then_cancellation_date_set_and_alert_sen
 
         # Then
         self.subscription.refresh_from_db()
-        assert self.subscription.cancellation_date == cancellation_date
+        assert self.subscription.cancellation_date == datetime.utcfromtimestamp(
+            current_term_end
+        ).replace(tzinfo=timezone.utc)
 
         # and
         assert len(mail.outbox) == 1
 
-    @mock.patch("organisations.views.extract_subscription_metadata")
+    @mock.patch(
+        "organisations.chargebee.webhook_handlers.extract_subscription_metadata"
+    )
     def test_when_cancelled_subscription_is_renewed_then_subscription_activated_and_no_cancellation_email_sent(
         self,
         mock_extract_subscription_metadata,
@@ -726,7 +737,7 @@ def test_when_cancelled_subscription_is_renewed_then_subscription_activated_and_
 
 def test_when_chargebee_webhook_received_with_unknown_subscription_id_then_200(
     api_client: APIClient, caplog: LogCaptureFixture, django_user_model: Type[Model]
-):
+) -> None:
     # Given
     subscription_id = "some-random-id"
     cb_user = django_user_model.objects.create(email="test@example.com", is_staff=True)
@@ -748,7 +759,7 @@ def test_when_chargebee_webhook_received_with_unknown_subscription_id_then_200(
 
     assert len(caplog.records) == 1
     assert caplog.record_tuples[0] == (
-        "organisations.views",
+        "organisations.chargebee.webhook_handlers",
         30,
         f"Couldn't get unique subscription for ChargeBee id {subscription_id}",
     )
@@ -1036,7 +1047,7 @@ def test_organisation_get_influx_data(
     ],
 )
 @mock.patch("organisations.models.get_plan_meta_data")
-@mock.patch("organisations.views.extract_subscription_metadata")
+@mock.patch("organisations.chargebee.webhook_handlers.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,
@@ -1066,6 +1077,8 @@ def test_when_plan_is_changed_max_seats_and_max_api_calls_are_updated(
         projects=max_projects,
         chargebee_email=chargebee_email,
     )
+    subscription.subscription_id = "sub-id"
+    subscription.save()
 
     data = {
         "content": {
@@ -1433,12 +1446,16 @@ def test_when_subscription_is_cancelled_then_remove_all_but_the_first_user(
 ):
     # Given
     cancellation_date = datetime.now(tz=UTC)
+    current_term_end = int(datetime.timestamp(cancellation_date))
+    subscription.subscription_id = "subscription_id23"
+    subscription.save()
+
     data = {
         "content": {
             "subscription": {
                 "status": "cancelled",
                 "id": subscription.subscription_id,
-                "current_term_end": datetime.timestamp(cancellation_date),
+                "current_term_end": current_term_end,
             },
             "customer": {
                 "email": "chargebee@bullet-train.io",
@@ -1458,7 +1475,9 @@ def test_when_subscription_is_cancelled_then_remove_all_but_the_first_user(
     assert response.status_code == 200
 
     subscription.refresh_from_db()
-    assert subscription.cancellation_date == cancellation_date
+    assert subscription.cancellation_date == datetime.utcfromtimestamp(
+        current_term_end
+    ).replace(tzinfo=timezone.utc)
     organisation.refresh_from_db()
     assert organisation.num_seats == 1