From cdcc788802730a7d192663132ba9dcc878d35101 Mon Sep 17 00:00:00 2001 From: novakzaballa Date: Wed, 8 May 2024 14:10:33 -0400 Subject: [PATCH 1/7] fix: Move call to GitHub integration tasks out from trigger_feature_state_change_webhooks --- api/features/signals.py | 34 ++++++++++++++++++++++++++++++++++ api/features/tasks.py | 37 ------------------------------------- 2 files changed, 34 insertions(+), 37 deletions(-) diff --git a/api/features/signals.py b/api/features/signals.py index 1fa2d864bdbf..ee202b85ef68 100644 --- a/api/features/signals.py +++ b/api/features/signals.py @@ -1,8 +1,14 @@ import logging +from dataclasses import asdict from django.db.models.signals import post_save from django.dispatch import receiver +from integrations.github.github import GithubData, generate_data +from integrations.github.tasks import call_github_app_webhook_for_feature_state +from organisations.models import Organisation +from webhooks.webhooks import WebhookEventType + # noinspection PyUnresolvedReferences from .models import FeatureState from .tasks import trigger_feature_state_change_webhooks @@ -12,6 +18,34 @@ @receiver(post_save, sender=FeatureState) def trigger_feature_state_change_webhooks_signal(instance, **kwargs): + # Enqueue Github integration tasks if feature has GitHub external resources + if ( + not instance.identity_id + and not instance.feature_segment + and instance.feature.external_resources.exists() + and instance.environment.project.github_project.exists() + and hasattr(instance.environment.project.organisation, "github_config") + ): + github_configuration = ( + Organisation.objects.prefetch_related("github_config") + .get(id=instance.environment.project.organisation_id) + .github_config.first() + ) + feature_states = [] + feature_states.append(instance) + + feature_data: GithubData = generate_data( + github_configuration=github_configuration, + feature_id=instance.feature.id, + feature_name=instance.feature.name, + type=WebhookEventType.FLAG_UPDATED.value, + feature_states=feature_states, + ) + + call_github_app_webhook_for_feature_state.delay( + args=(asdict(feature_data),), + ) + if instance.environment_feature_version_id: return trigger_feature_state_change_webhooks(instance) diff --git a/api/features/tasks.py b/api/features/tasks.py index 1d0c07feb904..95ceade3ed30 100644 --- a/api/features/tasks.py +++ b/api/features/tasks.py @@ -1,11 +1,7 @@ import logging -from dataclasses import asdict from environments.models import Webhook from features.models import Feature, FeatureState -from integrations.github.github import GithubData, generate_data -from integrations.github.tasks import call_github_app_webhook_for_feature_state -from organisations.models import Organisation from task_processor.decorators import register_task_handler from webhooks.constants import WEBHOOK_DATETIME_FORMAT from webhooks.webhooks import ( @@ -59,39 +55,6 @@ def trigger_feature_state_change_webhooks( ) ) - if ( - not instance.identity_id - and not instance.feature_segment - and instance.feature.external_resources.exists() - and instance.environment.project.github_project.exists() - and hasattr(instance.environment.project.organisation, "github_config") - ): - github_configuration = ( - Organisation.objects.prefetch_related("github_config") - .get(id=instance.environment.project.organisationn_id) - .github_config.first() - ) - feature_state = { - "environment_name": new_state["environment"]["name"], - "feature_value": new_state["enabled"], - } - feature_states = [] - feature_states.append(instance) - - feature_data: GithubData = generate_data( - github_configuration=github_configuration, - feature_id=history_instance.feature.id, - feature_name=history_instance.feature.name, - type=WebhookEventType.FLAG_UPDATED.value, - feature_states=feature_states, - ) - - feature_data.feature_states.append(feature_state) - - call_github_app_webhook_for_feature_state.delay( - args=(asdict(feature_data),), - ) - def _get_previous_state( instance: FeatureState, From 4751366ea3c8c230680fb93c88fabbd4927def41 Mon Sep 17 00:00:00 2001 From: Novak Zaballa Date: Thu, 9 May 2024 12:15:15 -0400 Subject: [PATCH 2/7] Move Github task from signal to hook --- api/features/models.py | 38 +++++++++++++++++++++++++++++++ api/features/signals.py | 34 --------------------------- api/integrations/github/github.py | 2 +- 3 files changed, 39 insertions(+), 35 deletions(-) diff --git a/api/features/models.py b/api/features/models.py index 71d31b9a7d60..442ad3f2d40b 100644 --- a/api/features/models.py +++ b/api/features/models.py @@ -5,6 +5,7 @@ import typing import uuid from copy import deepcopy +from dataclasses import asdict from core.models import ( AbstractBaseExportableModel, @@ -22,6 +23,7 @@ from django.utils.translation import ugettext_lazy as _ from django_lifecycle import ( AFTER_CREATE, + AFTER_SAVE, BEFORE_CREATE, BEFORE_SAVE, LifecycleModelMixin, @@ -72,6 +74,7 @@ STRING, ) from features.versioning.models import EnvironmentFeatureVersion +from organisations.models import Organisation from projects.models import Project from projects.tags.models import Tag @@ -989,6 +992,41 @@ def copy_identity_feature_states( # Save changes to target feature_state target_feature_state.save() + @hook(AFTER_SAVE) + def create_github_comment(self) -> None: + from integrations.github.github import GithubData, generate_data + from integrations.github.tasks import ( + call_github_app_webhook_for_feature_state, + ) + from webhooks.webhooks import WebhookEventType + + if ( + not self.identity_id + and not self.feature_segment + and self.feature.external_resources.exists() + and self.environment.project.github_project.exists() + and self.environment.project.organisation.github_config.exists() + ): + github_configuration = ( + Organisation.objects.prefetch_related("github_config") + .get(id=self.environment.project.organisation_id) + .github_config.first() + ) + feature_states = [] + feature_states.append(self) + + feature_data: GithubData = generate_data( + github_configuration=github_configuration, + feature_id=self.feature.id, + feature_name=self.feature.name, + type=WebhookEventType.FLAG_UPDATED.value, + feature_states=feature_states, + ) + + call_github_app_webhook_for_feature_state.delay( + args=(asdict(feature_data),), + ) + class FeatureStateValue( AbstractBaseFeatureValueModel, diff --git a/api/features/signals.py b/api/features/signals.py index ee202b85ef68..1fa2d864bdbf 100644 --- a/api/features/signals.py +++ b/api/features/signals.py @@ -1,14 +1,8 @@ import logging -from dataclasses import asdict from django.db.models.signals import post_save from django.dispatch import receiver -from integrations.github.github import GithubData, generate_data -from integrations.github.tasks import call_github_app_webhook_for_feature_state -from organisations.models import Organisation -from webhooks.webhooks import WebhookEventType - # noinspection PyUnresolvedReferences from .models import FeatureState from .tasks import trigger_feature_state_change_webhooks @@ -18,34 +12,6 @@ @receiver(post_save, sender=FeatureState) def trigger_feature_state_change_webhooks_signal(instance, **kwargs): - # Enqueue Github integration tasks if feature has GitHub external resources - if ( - not instance.identity_id - and not instance.feature_segment - and instance.feature.external_resources.exists() - and instance.environment.project.github_project.exists() - and hasattr(instance.environment.project.organisation, "github_config") - ): - github_configuration = ( - Organisation.objects.prefetch_related("github_config") - .get(id=instance.environment.project.organisation_id) - .github_config.first() - ) - feature_states = [] - feature_states.append(instance) - - feature_data: GithubData = generate_data( - github_configuration=github_configuration, - feature_id=instance.feature.id, - feature_name=instance.feature.name, - type=WebhookEventType.FLAG_UPDATED.value, - feature_states=feature_states, - ) - - call_github_app_webhook_for_feature_state.delay( - args=(asdict(feature_data),), - ) - if instance.environment_feature_version_id: return trigger_feature_state_change_webhooks(instance) diff --git a/api/integrations/github/github.py b/api/integrations/github/github.py index 5914258f6fc3..f750dc709919 100644 --- a/api/integrations/github/github.py +++ b/api/integrations/github/github.py @@ -55,7 +55,7 @@ def post_comment_to_github( url, json=payload, headers=headers, timeout=10 ) - return response.json() if response.status_code == 200 else None + return response.json() if response.status_code == 201 else None except requests.RequestException as e: logger.error(f" {e}") return None From 82bf75c72f5e9c131d763bec0bffa91aa5261dbc Mon Sep 17 00:00:00 2001 From: novakzaballa Date: Thu, 9 May 2024 14:35:29 -0400 Subject: [PATCH 3/7] test: Add test coverage for feature_state's AFTER_SAVE hook --- ...t_unit_feature_external_resources_views.py | 58 ++++++++++++++++++- 1 file changed, 55 insertions(+), 3 deletions(-) diff --git a/api/tests/unit/features/test_unit_feature_external_resources_views.py b/api/tests/unit/features/test_unit_feature_external_resources_views.py index 2f5330ac0dd4..03193bf71c80 100644 --- a/api/tests/unit/features/test_unit_feature_external_resources_views.py +++ b/api/tests/unit/features/test_unit_feature_external_resources_views.py @@ -1,4 +1,5 @@ import datetime +from unittest.mock import ANY import simplejson as json from django.core.serializers.json import DjangoJSONEncoder @@ -7,10 +8,14 @@ from rest_framework import status from rest_framework.test import APIClient +from environments.models import Environment +from environments.permissions.constants import UPDATE_FEATURE_STATE from features.feature_external_resources.models import FeatureExternalResource -from features.models import Feature +from features.models import Feature, FeatureState +from features.serializers import FeatureStateSerializerBasic from integrations.github.models import GithubConfiguration, GithubRepository from projects.models import Project +from tests.types import WithEnvironmentPermissionsCallable _django_json_encoder_default = DjangoJSONEncoder().default @@ -66,6 +71,7 @@ def test_create_feature_external_resource( url, data=feature_external_resource_data, format="json" ) + # Then github_request_mock.assert_called_with( "https://api.github.com/repos/repoowner/repo-name/issues/35/comments", json={ @@ -77,7 +83,6 @@ def test_create_feature_external_resource( }, timeout=10, ) - # Then assert response.status_code == status.HTTP_201_CREATED # assert that the payload has been save to the database feature_external_resources = FeatureExternalResource.objects.filter( @@ -85,7 +90,6 @@ def test_create_feature_external_resource( type=feature_external_resource_data["type"], url=feature_external_resource_data["url"], ).all() - assert len(feature_external_resources) == 1 assert feature_external_resources[0].metadata == json.dumps( feature_external_resource_data["metadata"], default=_django_json_encoder_default @@ -300,3 +304,51 @@ def test_get_feature_external_resource( assert response.data["id"] == feature_external_resource.id assert response.data["type"] == feature_external_resource.type assert response.data["url"] == feature_external_resource.url + + +def test_create_github_comment_on_feature_state_updated( + staff_client: APIClient, + with_environment_permissions: WithEnvironmentPermissionsCallable, + feature_external_resource: FeatureExternalResource, + feature: Feature, + project: Project, + github_configuration: GithubConfiguration, + github_repository: GithubRepository, + mocker, + environment: Environment, +) -> None: + # Given + with_environment_permissions([UPDATE_FEATURE_STATE]) + feature_state = FeatureState.objects.get( + feature=feature, environment=environment.id + ) + mock_generate_token = mocker.patch( + "integrations.github.github.generate_token", + ) + mock_generate_token.return_value = "mocked_token" + github_request_mock = mocker.patch( + "requests.post", side_effect=mocked_requests_post + ) + + payload = dict(FeatureStateSerializerBasic(instance=feature_state).data) + + payload["enabled"] = not feature_state.enabled + url = reverse( + viewname="api-v1:environments:environment-featurestates-detail", + kwargs={"environment_api_key": environment.api_key, "pk": feature_state.id}, + ) + + # When + response = staff_client.put(path=url, data=payload, format="json") + + # Then + assert response.status_code == status.HTTP_200_OK + github_request_mock.assert_called_with( + "https://api.github.com/repos/userexample/example-project-repo/issues/11/comments", + json=ANY, + headers={ + "Accept": "application/vnd.github.v3+json", + "Authorization": "Bearer mocked_token", + }, + timeout=10, + ) From 255dc4528ede2cabb942f13ffb03e9f3c0ff3bb4 Mon Sep 17 00:00:00 2001 From: novakzaballa Date: Thu, 9 May 2024 15:43:11 -0400 Subject: [PATCH 4/7] fix: simplify github_configuration query and update test for cover --- api/features/models.py | 8 ++--- ...t_unit_feature_external_resources_views.py | 30 +++++++++++++++++++ 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/api/features/models.py b/api/features/models.py index 442ad3f2d40b..3851c6012e9a 100644 --- a/api/features/models.py +++ b/api/features/models.py @@ -74,7 +74,7 @@ STRING, ) from features.versioning.models import EnvironmentFeatureVersion -from organisations.models import Organisation +from integrations.github.models import GithubConfiguration from projects.models import Project from projects.tags.models import Tag @@ -1007,10 +1007,8 @@ def create_github_comment(self) -> None: and self.environment.project.github_project.exists() and self.environment.project.organisation.github_config.exists() ): - github_configuration = ( - Organisation.objects.prefetch_related("github_config") - .get(id=self.environment.project.organisation_id) - .github_config.first() + github_configuration = GithubConfiguration.objects.get( + organisation_id=self.environment.project.organisation_id ) feature_states = [] feature_states.append(self) diff --git a/api/tests/unit/features/test_unit_feature_external_resources_views.py b/api/tests/unit/features/test_unit_feature_external_resources_views.py index 03193bf71c80..baccb7bb154a 100644 --- a/api/tests/unit/features/test_unit_feature_external_resources_views.py +++ b/api/tests/unit/features/test_unit_feature_external_resources_views.py @@ -13,9 +13,11 @@ from features.feature_external_resources.models import FeatureExternalResource from features.models import Feature, FeatureState from features.serializers import FeatureStateSerializerBasic +from integrations.github.github import GithubData from integrations.github.models import GithubConfiguration, GithubRepository from projects.models import Project from tests.types import WithEnvironmentPermissionsCallable +from webhooks.webhooks import WebhookEventType _django_json_encoder_default = DjangoJSONEncoder().default @@ -330,6 +332,27 @@ def test_create_github_comment_on_feature_state_updated( "requests.post", side_effect=mocked_requests_post ) + feature_state_value = feature_state.get_feature_state_value() + feature_env_data = {} + feature_env_data["feature_state_value"] = feature_state_value + feature_env_data["feature_state_value_type"] = ( + feature_state.get_feature_state_value_type(feature_state_value) + ) + feature_env_data["environment_name"] = environment.name + feature_env_data["feature_value"] = feature_state.enabled + + mock_generate_data = mocker.patch( + "integrations.github.github.generate_data", + return_value=GithubData( + installation_id=github_configuration.installation_id, + feature_id=feature.id, + feature_name=feature.name, + type=feature_external_resource.type, + feature_states=[feature_env_data], + url=feature_external_resource.url, + ), + ) + payload = dict(FeatureStateSerializerBasic(instance=feature_state).data) payload["enabled"] = not feature_state.enabled @@ -352,3 +375,10 @@ def test_create_github_comment_on_feature_state_updated( }, timeout=10, ) + mock_generate_data.assert_called_with( + github_configuration=github_configuration, + feature_id=feature.id, + feature_name=feature.name, + type=WebhookEventType.FLAG_UPDATED.value, + feature_states=[feature_state], + ) From c470f19aacc2936a537ccde3a4780992f0f54920 Mon Sep 17 00:00:00 2001 From: novakzaballa Date: Thu, 9 May 2024 18:43:23 -0400 Subject: [PATCH 5/7] Add Comment when feature was Deleted, and the test for cover it --- api/features/models.py | 24 +++-- api/integrations/github/constants.py | 7 +- api/integrations/github/github.py | 4 + api/integrations/github/tasks.py | 45 ++++++++-- ...t_unit_feature_external_resources_views.py | 90 ++++++++++++++----- 5 files changed, 128 insertions(+), 42 deletions(-) diff --git a/api/features/models.py b/api/features/models.py index 3851c6012e9a..09e5b1e8a96b 100644 --- a/api/features/models.py +++ b/api/features/models.py @@ -1013,13 +1013,23 @@ def create_github_comment(self) -> None: feature_states = [] feature_states.append(self) - feature_data: GithubData = generate_data( - github_configuration=github_configuration, - feature_id=self.feature.id, - feature_name=self.feature.name, - type=WebhookEventType.FLAG_UPDATED.value, - feature_states=feature_states, - ) + if self.deleted_at is None: + feature_data: GithubData = generate_data( + github_configuration=github_configuration, + feature_id=self.feature.id, + feature_name=self.feature.name, + type=WebhookEventType.FLAG_UPDATED.value, + feature_states=feature_states, + ) + + if self.deleted_at is not None: + feature_data: GithubData = generate_data( + github_configuration=github_configuration, + feature_id=self.feature.id, + feature_name=self.feature.name, + type=WebhookEventType.FLAG_DELETED.value, + feature_states=feature_states, + ) call_github_app_webhook_for_feature_state.delay( args=(asdict(feature_data),), diff --git a/api/integrations/github/constants.py b/api/integrations/github/constants.py index d88e60b64ad1..e5ca5be737ff 100644 --- a/api/integrations/github/constants.py +++ b/api/integrations/github/constants.py @@ -1,7 +1,8 @@ GITHUB_API_URL = "https://api.github.com/" GITHUB_API_VERSION = "2022-11-28" -LINK_FEATURE_TEXT = "### This pull request is linked to a Flagsmith Feature (%s):\n" -UNLINKED_FEATURE_TEXT = "### The feature flag %s was unlinked from the issue/PR" -UPDATED_FEATURE_TEXT = "### The Flagsmith Feature %s was updated in the environment " +LINK_FEATURE_TEXT = "### This pull request is linked to a Flagsmith Feature (`%s`):\n" +UNLINKED_FEATURE_TEXT = "### The feature flag `%s` was unlinked from the issue/PR" +UPDATED_FEATURE_TEXT = "### The Flagsmith Feature `%s` was updated in the environment " LAST_UPDATED_FEATURE_TEXT = "Last Updated %s" +DELETED_FEATURE_TEXT = "### The Feature Flag `%s` was deleted" diff --git a/api/integrations/github/github.py b/api/integrations/github/github.py index f750dc709919..fad30c2cd6c4 100644 --- a/api/integrations/github/github.py +++ b/api/integrations/github/github.py @@ -9,6 +9,7 @@ from features.models import FeatureState, FeatureStateValue from integrations.github.client import generate_token from integrations.github.constants import ( + DELETED_FEATURE_TEXT, GITHUB_API_URL, LAST_UPDATED_FEATURE_TEXT, LINK_FEATURE_TEXT, @@ -71,6 +72,9 @@ def generate_body_comment( is_removed = event_type == WebhookEventType.FEATURE_EXTERNAL_RESOURCE_REMOVED.value delete_text = UNLINKED_FEATURE_TEXT % (name,) + if event_type == WebhookEventType.FLAG_DELETED.value: + return DELETED_FEATURE_TEXT % (name,) + if is_removed: return delete_text diff --git a/api/integrations/github/tasks.py b/api/integrations/github/tasks.py index 3fd198881e7f..89e301f95d0e 100644 --- a/api/integrations/github/tasks.py +++ b/api/integrations/github/tasks.py @@ -31,7 +31,10 @@ def send_post_request(data: CallGithubData) -> None: installation_id = data.github_data.installation_id body = generate_body_comment(feature_name, event_type, feature_states) - if event_type == WebhookEventType.FLAG_UPDATED.value: + if ( + event_type == WebhookEventType.FLAG_UPDATED.value + or event_type == WebhookEventType.FLAG_DELETED.value + ): for resource in data.feature_external_resources: url = resource.get("url") pathname = urlparse(url).path @@ -61,8 +64,37 @@ def send_post_request(data: CallGithubData) -> None: @register_task_handler() def call_github_app_webhook_for_feature_state(event_data: dict[str, Any]) -> None: + from features.feature_external_resources.models import ( + FeatureExternalResource, + ) + github_event_data = GithubData.from_dict(event_data) + def generate_feature_external_resources( + feature_external_resources: FeatureExternalResource, + ) -> list[dict[str, Any]]: + return [ + { + "type": resource.type, + "url": resource.url, + } + for resource in feature_external_resources + ] + + if github_event_data.type == WebhookEventType.FLAG_DELETED.value: + feature_external_resources = generate_feature_external_resources( + FeatureExternalResource.objects.filter( + feature_id=github_event_data.feature_id + ) + ) + data = CallGithubData( + event_type=github_event_data.type, + github_data=github_event_data, + feature_external_resources=feature_external_resources, + ) + send_post_request(data) + return + if ( github_event_data.type == WebhookEventType.FEATURE_EXTERNAL_RESOURCE_REMOVED.value @@ -76,14 +108,9 @@ def call_github_app_webhook_for_feature_state(event_data: dict[str, Any]) -> Non return feature = Feature.objects.get(id=github_event_data.feature_id) - feature_external_resources = feature.external_resources.all() - feature_external_resources = [ - { - "type": resource.type, - "url": resource.url, - } - for resource in feature_external_resources - ] + feature_external_resources = generate_feature_external_resources( + feature.external_resources.all() + ) data = CallGithubData( event_type=github_event_data.type, github_data=github_event_data, diff --git a/api/tests/unit/features/test_unit_feature_external_resources_views.py b/api/tests/unit/features/test_unit_feature_external_resources_views.py index baccb7bb154a..34660ca658de 100644 --- a/api/tests/unit/features/test_unit_feature_external_resources_views.py +++ b/api/tests/unit/features/test_unit_feature_external_resources_views.py @@ -1,6 +1,6 @@ import datetime -from unittest.mock import ANY +import pytest import simplejson as json from django.core.serializers.json import DjangoJSONEncoder from django.urls import reverse @@ -77,7 +77,7 @@ def test_create_feature_external_resource( github_request_mock.assert_called_with( "https://api.github.com/repos/repoowner/repo-name/issues/35/comments", json={ - "body": f"### This pull request is linked to a Flagsmith Feature (feature_with_value):\n**Test Environment**\n- [ ] Disabled\nunicode\n```value```\n\nLast Updated {datetime_now.strftime('%dth %b %Y %I:%M%p')}" # noqa E501 + "body": f"### This pull request is linked to a Flagsmith Feature (`feature_with_value`):\n**Test Environment**\n- [ ] Disabled\nunicode\n```value```\n\nLast Updated {datetime_now.strftime('%dth %b %Y %I:%M%p')}" # noqa E501 }, headers={ "Accept": "application/vnd.github.v3+json", @@ -249,7 +249,7 @@ def test_delete_feature_external_resource( github_request_mock.assert_called_with( "https://api.github.com/repos/userexample/example-project-repo/issues/11/comments", json={ - "body": "### The feature flag Test Feature1 was unlinked from the issue/PR" + "body": "### The feature flag `Test Feature1` was unlinked from the issue/PR" }, headers={ "Accept": "application/vnd.github.v3+json", @@ -308,16 +308,23 @@ def test_get_feature_external_resource( assert response.data["url"] == feature_external_resource.url -def test_create_github_comment_on_feature_state_updated( +@pytest.mark.parametrize( + "event_type", + [ + ("update"), + ("delete"), + ], +) +def test_create_github_comment_on_feature_state_updated( # noqa: C901 staff_client: APIClient, with_environment_permissions: WithEnvironmentPermissionsCallable, feature_external_resource: FeatureExternalResource, feature: Feature, - project: Project, github_configuration: GithubConfiguration, github_repository: GithubRepository, mocker, environment: Environment, + event_type: str, ) -> None: # Given with_environment_permissions([UPDATE_FEATURE_STATE]) @@ -332,6 +339,15 @@ def test_create_github_comment_on_feature_state_updated( "requests.post", side_effect=mocked_requests_post ) + mock_generate_body_comment = mocker.patch( + "integrations.github.tasks.generate_body_comment", + ) + + if event_type == "update": + mock_generate_body_comment.return_value = "Flag updated" + elif event_type == "delete": + mock_generate_body_comment.return_value = "Flag deleted" + feature_state_value = feature_state.get_feature_state_value() feature_env_data = {} feature_env_data["feature_state_value"] = feature_state_value @@ -362,23 +378,51 @@ def test_create_github_comment_on_feature_state_updated( ) # When - response = staff_client.put(path=url, data=payload, format="json") + if event_type == "update": + response = staff_client.put(path=url, data=payload, format="json") + elif event_type == "delete": + response = staff_client.delete(path=url) # Then - assert response.status_code == status.HTTP_200_OK - github_request_mock.assert_called_with( - "https://api.github.com/repos/userexample/example-project-repo/issues/11/comments", - json=ANY, - headers={ - "Accept": "application/vnd.github.v3+json", - "Authorization": "Bearer mocked_token", - }, - timeout=10, - ) - mock_generate_data.assert_called_with( - github_configuration=github_configuration, - feature_id=feature.id, - feature_name=feature.name, - type=WebhookEventType.FLAG_UPDATED.value, - feature_states=[feature_state], - ) + if event_type == "update": + assert response.status_code == status.HTTP_200_OK + elif event_type == "delete": + assert response.status_code == status.HTTP_204_NO_CONTENT + + if event_type == "update": + github_request_mock.assert_called_with( + "https://api.github.com/repos/userexample/example-project-repo/issues/11/comments", + json={"body": "Flag updated"}, + headers={ + "Accept": "application/vnd.github.v3+json", + "Authorization": "Bearer mocked_token", + }, + timeout=10, + ) + elif event_type == "delete": + github_request_mock.assert_called_with( + "https://api.github.com/repos/userexample/example-project-repo/issues/11/comments", + json={"body": "Flag deleted"}, + headers={ + "Accept": "application/vnd.github.v3+json", + "Authorization": "Bearer mocked_token", + }, + timeout=10, + ) + if event_type == "update": + mock_generate_data.assert_called_with( + github_configuration=github_configuration, + feature_id=feature.id, + feature_name=feature.name, + type=WebhookEventType.FLAG_UPDATED.value, + feature_states=[feature_state], + ) + + elif event_type == "update": + mock_generate_data.assert_called_with( + github_configuration=github_configuration, + feature_id=feature.id, + feature_name=feature.name, + type=WebhookEventType.FLAG_UPDATED.value, + feature_states=[feature_state], + ) From 8299feeb75a42bc8d6e9d19596af9b929fd90500 Mon Sep 17 00:00:00 2001 From: Novak Zaballa Date: Thu, 9 May 2024 19:54:44 -0400 Subject: [PATCH 6/7] Correct test --- ...t_unit_feature_external_resources_views.py | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/api/tests/unit/features/test_unit_feature_external_resources_views.py b/api/tests/unit/features/test_unit_feature_external_resources_views.py index 34660ca658de..7e1f9647f57d 100644 --- a/api/tests/unit/features/test_unit_feature_external_resources_views.py +++ b/api/tests/unit/features/test_unit_feature_external_resources_views.py @@ -409,20 +409,20 @@ def test_create_github_comment_on_feature_state_updated( # noqa: C901 }, timeout=10, ) - if event_type == "update": - mock_generate_data.assert_called_with( - github_configuration=github_configuration, - feature_id=feature.id, - feature_name=feature.name, - type=WebhookEventType.FLAG_UPDATED.value, - feature_states=[feature_state], - ) - - elif event_type == "update": - mock_generate_data.assert_called_with( - github_configuration=github_configuration, - feature_id=feature.id, - feature_name=feature.name, - type=WebhookEventType.FLAG_UPDATED.value, - feature_states=[feature_state], - ) + if event_type == "update": + mock_generate_data.assert_called_with( + github_configuration=github_configuration, + feature_id=feature.id, + feature_name=feature.name, + type=WebhookEventType.FLAG_UPDATED.value, + feature_states=[feature_state], + ) + + elif event_type == "delete": + mock_generate_data.assert_called_with( + github_configuration=github_configuration, + feature_id=feature.id, + feature_name=feature.name, + type=WebhookEventType.FLAG_DELETED.value, + feature_states=[feature_state], + ) From eafb5f5e9e49381b579b6a3e15b666cdf8be7da1 Mon Sep 17 00:00:00 2001 From: Novak Zaballa Date: Thu, 9 May 2024 20:21:25 -0400 Subject: [PATCH 7/7] Cover the Flag deleted comment in the test --- ...t_unit_feature_external_resources_views.py | 47 +++++++------------ 1 file changed, 17 insertions(+), 30 deletions(-) diff --git a/api/tests/unit/features/test_unit_feature_external_resources_views.py b/api/tests/unit/features/test_unit_feature_external_resources_views.py index 7e1f9647f57d..6df4cc47393b 100644 --- a/api/tests/unit/features/test_unit_feature_external_resources_views.py +++ b/api/tests/unit/features/test_unit_feature_external_resources_views.py @@ -339,15 +339,6 @@ def test_create_github_comment_on_feature_state_updated( # noqa: C901 "requests.post", side_effect=mocked_requests_post ) - mock_generate_body_comment = mocker.patch( - "integrations.github.tasks.generate_body_comment", - ) - - if event_type == "update": - mock_generate_body_comment.return_value = "Flag updated" - elif event_type == "delete": - mock_generate_body_comment.return_value = "Flag deleted" - feature_state_value = feature_state.get_feature_state_value() feature_env_data = {} feature_env_data["feature_state_value"] = feature_state_value @@ -356,18 +347,23 @@ def test_create_github_comment_on_feature_state_updated( # noqa: C901 ) feature_env_data["environment_name"] = environment.name feature_env_data["feature_value"] = feature_state.enabled + if event_type == "update": + mock_generate_data = mocker.patch( + "integrations.github.github.generate_data", + return_value=GithubData( + installation_id=github_configuration.installation_id, + feature_id=feature.id, + feature_name=feature.name, + type=feature_external_resource.type, + feature_states=[feature_env_data], + url=feature_external_resource.url, + ), + ) - mock_generate_data = mocker.patch( - "integrations.github.github.generate_data", - return_value=GithubData( - installation_id=github_configuration.installation_id, - feature_id=feature.id, - feature_name=feature.name, - type=feature_external_resource.type, - feature_states=[feature_env_data], - url=feature_external_resource.url, - ), - ) + mocker.patch( + "integrations.github.tasks.generate_body_comment", + return_value="Flag updated", + ) payload = dict(FeatureStateSerializerBasic(instance=feature_state).data) @@ -402,7 +398,7 @@ def test_create_github_comment_on_feature_state_updated( # noqa: C901 elif event_type == "delete": github_request_mock.assert_called_with( "https://api.github.com/repos/userexample/example-project-repo/issues/11/comments", - json={"body": "Flag deleted"}, + json={"body": "### The Feature Flag `Test Feature1` was deleted"}, headers={ "Accept": "application/vnd.github.v3+json", "Authorization": "Bearer mocked_token", @@ -417,12 +413,3 @@ def test_create_github_comment_on_feature_state_updated( # noqa: C901 type=WebhookEventType.FLAG_UPDATED.value, feature_states=[feature_state], ) - - elif event_type == "delete": - mock_generate_data.assert_called_with( - github_configuration=github_configuration, - feature_id=feature.id, - feature_name=feature.name, - type=WebhookEventType.FLAG_DELETED.value, - feature_states=[feature_state], - )