improved webhook semantics, coverage

api/features/feature_health/constants.py

@@ -1,5 +1,5 @@
 FEATURE_HEALTH_PROVIDER_CREATED_MESSAGE = "Health provider %s set up for project %s."
-FEATURE_HEALTH_PROVIDER_DELETED_MESSAGE = "Health provider %s removed for project %s."
+FEATURE_HEALTH_PROVIDER_DELETED_MESSAGE = "Health provider %s removed from project %s."
 
 FEATURE_HEALTH_EVENT_CREATED_MESSAGE = "Health status changed to %s for feature %s."
 FEATURE_HEALTH_EVENT_CREATED_FOR_ENVIRONMENT_MESSAGE = (

api/features/feature_health/services.py

@@ -39,7 +39,7 @@ def get_provider_from_webhook_path(path: str) -> FeatureHealthProvider | None:
     try:
         hex_string = _provider_webhook_signer.unsign_object(path)
     except signing.BadSignature:
-        logger.warning("invalid-webhook-path-requested", path=path)
+        logger.warning("invalid-feature-health-webhook-path-requested", path=path)
         return None
     feature_health_provider_uuid = uuid.UUID(hex_string)
     return FeatureHealthProvider.objects.filter(
@@ -53,36 +53,44 @@ def get_provider_response(
     if provider.name == FeatureHealthProviderName.SAMPLE.value:
         return sample.map_payload_to_provider_response(payload)
     logger.error(
-        "invalid-provider-requested",
+        "invalid-feature-health-provider-requested",
         provider_name=provider.name,
         provider_id=provider.uuid,
     )
     return None
 
 
-def create_feature_health_event_from_webhook(
-    path: str,
+def create_feature_health_event_from_provider(
+    provider: FeatureHealthProvider,
     payload: str,
 ) -> FeatureHealthEvent | None:
-    if provider := get_provider_from_webhook_path(path):
-        if response := get_provider_response(provider, payload):
-            project = provider.project
-            if feature := Feature.objects.filter(
-                project=provider.project, name=response.feature_name
-            ).first():
-                if response.environment_name:
-                    environment = Environment.objects.filter(
-                        project=project, name=response.environment_name
-                    ).first()
-                else:
-                    environment = None
-                return FeatureHealthEvent.objects.create(
-                    feature=feature,
-                    environment=environment,
-                    provider_name=provider.name,
-                    type=response.event_type,
-                    reason=response.reason,
-                )
+    try:
+        response = get_provider_response(provider, payload)
+    except Exception as exc:
+        logger.error(
+            "error-creating-feature-health-event",
+            provider_name=provider.name,
+            project_id=provider.project.id,
+            exc_info=exc,
+        )
+        return None
+    project = provider.project
+    if feature := Feature.objects.filter(
+        project=provider.project, name=response.feature_name
+    ).first():
+        if response.environment_name:
+            environment = Environment.objects.filter(
+                project=project, name=response.environment_name
+            ).first()
+        else:
+            environment = None
+        return FeatureHealthEvent.objects.create(
+            feature=feature,
+            environment=environment,
+            provider_name=provider.name,
+            type=response.event_type,
+            reason=response.reason,
+        )
     return None
 
 

api/features/feature_health/views.py

@@ -20,7 +20,8 @@
     FeatureHealthProviderSerializer,
 )
 from features.feature_health.services import (
-    create_feature_health_event_from_webhook,
+    create_feature_health_event_from_provider,
+    get_provider_from_webhook_path,
 )
 from projects.models import Project
 from projects.permissions import NestedProjectPermissions
@@ -102,7 +103,9 @@ def create(self, request: Request, *args, **kwargs) -> Response:
 @permission_classes([AllowAny])
 def feature_health_webhook(request: Request, **kwargs: typing.Any) -> Response:
     path = kwargs["path"]
+    if not (provider := get_provider_from_webhook_path(path)):
+        return Response(status=status.HTTP_404_NOT_FOUND)
     payload = request.body.decode("utf-8")
-    if create_feature_health_event_from_webhook(path=path, payload=payload):
+    if create_feature_health_event_from_provider(provider=provider, payload=payload):
         return Response(status=status.HTTP_200_OK)
-    return Response(status=status.HTTP_404_NOT_FOUND)
+    return Response(status=status.HTTP_400_BAD_REQUEST)

api/tests/integration/features/feature_health/conftest.py

@@ -1,3 +1,5 @@
+import json
+
 import pytest
 from django.urls import reverse
 from rest_framework.test import APIClient
@@ -11,3 +13,18 @@ def sample_feature_health_provider_webhook_url(
     url = reverse("api-v1:projects:feature-health-providers-list", args=[project])
     response = admin_client.post(url, data=feature_health_provider_data)
     return response.json()["webhook_url"]
+
+
+@pytest.fixture
+def unhealthy_feature(
+    sample_feature_health_provider_webhook_url: str,
+    feature_name: str,
+    feature: int,
+    api_client: APIClient,
+) -> int:
+    api_client.post(
+        sample_feature_health_provider_webhook_url,
+        data=json.dumps({"feature": feature_name, "status": "unhealthy"}),
+        content_type="application/json",
+    )
+    return feature

api/tests/integration/features/feature_health/test_views.py

@@ -1,12 +1,27 @@
 import json
+from datetime import datetime, timedelta
 
 import pytest
 from django.urls import reverse
+from freezegun import freeze_time
 from rest_framework.test import APIClient
 
 
+def test_webhook__invalid_path__expected_response(
+    api_client: APIClient,
+) -> None:
+    # Given
+    webhook_url = reverse("api-v1:feature-health-webhook", args=["invalid"])
+
+    # When
+    response = api_client.post(webhook_url)
+
+    # Then
+    assert response.status_code == 404
+
+
 @pytest.mark.freeze_time("2023-01-19T09:09:47.325132+00:00")
-def test_webhook__post__expected_feature_health_event_created__expected_tag_added(
+def test_webhook__sample_provider__post__expected_feature_health_event_created__expected_tag_added(
     feature: int,
     project: int,
     feature_name: str,
@@ -26,15 +41,15 @@ def test_webhook__post__expected_feature_health_event_created__expected_tag_adde
         "feature": feature_name,
         "status": "unhealthy",
     }
-    api_client.post(
+    response = api_client.post(
         sample_feature_health_provider_webhook_url,
         data=json.dumps(webhook_data),
         content_type="application/json",
     )
 
     # Then
-    response = admin_client.get(feature_health_events_url)
     assert response.status_code == 200
+    response = admin_client.get(feature_health_events_url)
     assert response.json() == [
         {
             "created_at": "2023-01-19T09:09:47.325132Z",
@@ -60,3 +75,123 @@ def test_webhook__post__expected_feature_health_event_created__expected_tag_adde
         if feature_data.get("id") == feature
     )
     assert tag_data["id"] in feature_data["tags"]
+
+
+@pytest.mark.freeze_time("2023-01-19T09:09:47.325132+00:00")
+def test_webhook__sample_provider__post_with_environment_expected_feature_health_event_created(
+    feature: int,
+    project: int,
+    environment: int,
+    feature_name: str,
+    environment_name: str,
+    sample_feature_health_provider_webhook_url: str,
+    api_client: APIClient,
+    admin_client: APIClient,
+) -> None:
+    # Given
+    feature_health_events_url = reverse(
+        "api-v1:projects:feature-health-events-list", args=[project]
+    )
+
+    # When
+    webhook_data = {
+        "feature": feature_name,
+        "environment": environment_name,
+        "status": "unhealthy",
+    }
+    response = api_client.post(
+        sample_feature_health_provider_webhook_url,
+        data=json.dumps(webhook_data),
+        content_type="application/json",
+    )
+
+    # Then
+    assert response.status_code == 200
+    response = admin_client.get(feature_health_events_url)
+    assert response.json() == [
+        {
+            "created_at": "2023-01-19T09:09:47.325132Z",
+            "environment": environment,
+            "feature": feature,
+            "provider_name": "Sample",
+            "reason": "",
+            "type": "UNHEALTHY",
+        }
+    ]
+
+
+@pytest.mark.freeze_time("2023-01-19T09:09:47.325132+00:00")
+def test_webhook__unhealthy_feature__post__expected_feature_health_event_created__expected_tag_removed(
+    unhealthy_feature: int,
+    project: int,
+    feature_name: str,
+    sample_feature_health_provider_webhook_url: str,
+    api_client: APIClient,
+    admin_client: APIClient,
+) -> None:
+    # Given
+    feature_health_events_url = reverse(
+        "api-v1:projects:feature-health-events-list", args=[project]
+    )
+    tags_url = reverse("api-v1:projects:tags-list", args=[project])
+    features_url = reverse("api-v1:projects:project-features-list", args=[project])
+
+    # When
+    webhook_data = {
+        "feature": feature_name,
+        "status": "healthy",
+    }
+    with freeze_time(datetime.now() + timedelta(seconds=1)):
+        response = api_client.post(
+            sample_feature_health_provider_webhook_url,
+            data=json.dumps(webhook_data),
+            content_type="application/json",
+        )
+
+    # Then
+    assert response.status_code == 200
+    response = admin_client.get(feature_health_events_url)
+    assert response.json() == [
+        {
+            "created_at": "2023-01-19T09:09:48.325132Z",
+            "environment": None,
+            "feature": unhealthy_feature,
+            "provider_name": "Sample",
+            "reason": "",
+            "type": "HEALTHY",
+        }
+    ]
+    response = admin_client.get(tags_url)
+    assert (
+        tag_data := next(
+            tag_data
+            for tag_data in response.json()["results"]
+            if tag_data.get("label") == "Unhealthy"
+        )
+    )
+    response = admin_client.get(features_url)
+    feature_data = next(
+        feature_data
+        for feature_data in response.json()["results"]
+        if feature_data.get("id") == unhealthy_feature
+    )
+    assert tag_data["id"] not in feature_data["tags"]
+
+
+@pytest.mark.parametrize(
+    "body", ["invalid", json.dumps({"status": "unhealthy", "feature": "non_existent"})]
+)
+def test_webhook__sample_provider__post__invalid_payload__expected_response(
+    sample_feature_health_provider_webhook_url: str,
+    api_client: APIClient,
+    body: str,
+) -> None:
+    # When
+    response = api_client.post(
+        sample_feature_health_provider_webhook_url,
+        data=body,
+        content_type="application/json",
+    )
+
+    # Then
+    assert response.status_code == 400

api/tests/unit/features/feature_health/conftest.py

@@ -0,0 +1,17 @@
+import pytest
+
+from features.feature_health.models import FeatureHealthProvider
+from projects.models import Project
+from users.models import FFAdminUser
+
+
+@pytest.fixture
+def feature_health_provider(
+    project: Project,
+    staff_user: FFAdminUser,
+) -> FeatureHealthProvider:
+    return FeatureHealthProvider.objects.create(
+        created_by=staff_user,
+        project=project,
+        name="Sample",
+    )