diff --git a/api/conftest.py b/api/conftest.py index 583e60607713..31bda86c3a84 100644 --- a/api/conftest.py +++ b/api/conftest.py @@ -971,6 +971,7 @@ def feature_external_resource(feature: Feature) -> FeatureExternalResource: url="https://github.com/userexample/example-project-repo/issues/11", type="GITHUB_ISSUE", feature=feature, + metadata='{"status": "open"}', ) diff --git a/api/features/feature_external_resources/models.py b/api/features/feature_external_resources/models.py index 3421f18985dc..c41c81022bfe 100644 --- a/api/features/feature_external_resources/models.py +++ b/api/features/feature_external_resources/models.py @@ -1,5 +1,4 @@ import logging -from dataclasses import asdict from django.db import models from django_lifecycle import ( @@ -10,8 +9,7 @@ ) 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 integrations.github.github import call_github_task from organisations.models import Organisation from webhooks.webhooks import WebhookEventType @@ -50,43 +48,36 @@ class Meta: def execute_after_save_actions(self): # Add a comment to GitHub Issue/PR when feature is linked to the GH external resource if ( - github_configuration := Organisation.objects.prefetch_related( - "github_config" - ) + Organisation.objects.prefetch_related("github_config") .get(id=self.feature.project.organisation_id) .github_config.first() ): feature_states = FeatureState.objects.filter( feature_id=self.feature_id, identity_id__isnull=True ) - feature_data: GithubData = generate_data( - github_configuration=github_configuration, - feature=self.feature, + call_github_task( + organisation_id=self.feature.project.organisation_id, type=WebhookEventType.FEATURE_EXTERNAL_RESOURCE_ADDED.value, + feature=self.feature, + segment_name=None, + url=None, feature_states=feature_states, ) - call_github_app_webhook_for_feature_state.delay( - args=(asdict(feature_data),), - ) - @hook(BEFORE_DELETE) def execute_before_save_actions(self) -> None: # Add a comment to GitHub Issue/PR when feature is unlinked to the GH external resource if ( - github_configuration := Organisation.objects.prefetch_related( - "github_config" - ) + Organisation.objects.prefetch_related("github_config") .get(id=self.feature.project.organisation_id) .github_config.first() ): - feature_data: GithubData = generate_data( - github_configuration=github_configuration, - feature=self.feature, + + call_github_task( + organisation_id=self.feature.project.organisation_id, type=WebhookEventType.FEATURE_EXTERNAL_RESOURCE_REMOVED.value, + feature=self.feature, + segment_name=None, url=self.url, - ) - - call_github_app_webhook_for_feature_state.delay( - args=(asdict(feature_data),), + feature_states=None, ) diff --git a/api/features/feature_external_resources/views.py b/api/features/feature_external_resources/views.py index 822c3a07400e..f539877c31e8 100644 --- a/api/features/feature_external_resources/views.py +++ b/api/features/feature_external_resources/views.py @@ -8,6 +8,7 @@ from features.models import Feature from features.permissions import FeatureExternalResourcePermissions +from integrations.github.client import get_github_issue_pr_title_and_state from organisations.models import Organisation from .models import FeatureExternalResource @@ -25,6 +26,25 @@ def get_queryset(self): features_pk = self.kwargs["feature_pk"] return FeatureExternalResource.objects.filter(feature=features_pk) + # Override get list view to add github issue/pr name to each linked external resource + def list(self, request, *args, **kwargs) -> Response: + queryset = self.get_queryset() + serializer = self.get_serializer(queryset, many=True) + data = serializer.data + + # get organisation id from feature and get feature from validated data + organisation_id = get_object_or_404( + Feature.objects.filter(id=self.kwargs["feature_pk"]), + ).project.organisation_id + + for resource in data if isinstance(data, list) else []: + if resource_url := resource.get("url"): + resource["metadata"] = get_github_issue_pr_title_and_state( + organisation_id=organisation_id, resource_url=resource_url + ) + + return Response(data={"results": data}) + def create(self, request, *args, **kwargs): feature = get_object_or_404( Feature.objects.filter( diff --git a/api/features/models.py b/api/features/models.py index 326d6fc521f3..959e35d92530 100644 --- a/api/features/models.py +++ b/api/features/models.py @@ -5,7 +5,6 @@ import typing import uuid from copy import deepcopy -from dataclasses import asdict from core.models import ( AbstractBaseExportableModel, @@ -23,6 +22,7 @@ from django.utils import timezone from django_lifecycle import ( AFTER_CREATE, + AFTER_DELETE, AFTER_SAVE, BEFORE_CREATE, BEFORE_SAVE, @@ -74,7 +74,6 @@ STRING, ) from features.versioning.models import EnvironmentFeatureVersion -from integrations.github.models import GithubConfiguration from metadata.models import Metadata from projects.models import Project from projects.tags.models import Tag @@ -139,10 +138,7 @@ class Meta: @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 integrations.github.github import call_github_task from webhooks.webhooks import WebhookEventType if ( @@ -151,19 +147,14 @@ def create_github_comment(self) -> None: and self.project.organisation.github_config.exists() and self.deleted_at ): - github_configuration = GithubConfiguration.objects.get( - organisation_id=self.project.organisation_id - ) - feature_data: GithubData = generate_data( - github_configuration=github_configuration, - feature=self, + call_github_task( + organisation_id=self.project.organisation_id, type=WebhookEventType.FLAG_DELETED.value, - feature_states=[], - ) - - call_github_app_webhook_for_feature_state.delay( - args=(asdict(feature_data),), + feature=self, + segment_name=None, + url=None, + feature_states=None, ) @hook(AFTER_CREATE) @@ -219,6 +210,7 @@ def get_next_segment_priority(feature): class FeatureSegment( + LifecycleModelMixin, AbstractBaseExportableModel, OrderedModelBase, abstract_base_auditable_model_factory(["uuid"]), @@ -406,6 +398,26 @@ def get_delete_log_message(self, history_instance) -> typing.Optional[str]: def _get_environment(self) -> "Environment": return self.environment + @hook(AFTER_DELETE) + def create_github_comment(self) -> None: + from integrations.github.github import call_github_task + from webhooks.webhooks import WebhookEventType + + if ( + self.feature.external_resources.exists() + and self.feature.project.github_project.exists() + and self.feature.project.organisation.github_config.exists() + ): + + call_github_task( + self.feature.project.organisation_id, + WebhookEventType.SEGMENT_OVERRIDE_DELETED.value, + self.feature, + self.segment.name, + None, + None, + ) + class FeatureState( SoftDeleteExportableModel, diff --git a/api/features/serializers.py b/api/features/serializers.py index f026e0562e80..d98c4b70a14e 100644 --- a/api/features/serializers.py +++ b/api/features/serializers.py @@ -1,5 +1,4 @@ import typing -from dataclasses import asdict from datetime import datetime import django.core.exceptions @@ -13,9 +12,7 @@ from environments.sdk.serializers_mixins import ( HideSensitiveFieldsSerializerMixin, ) -from integrations.github.github import GithubData, generate_data -from integrations.github.models import GithubConfiguration -from integrations.github.tasks import call_github_app_webhook_for_feature_state +from integrations.github.github import call_github_task from metadata.serializers import MetadataSerializer, SerializerWithMetadata from projects.models import Project from users.serializers import ( @@ -474,23 +471,18 @@ def save(self, **kwargs): and feature_state.environment.project.github_project.exists() and feature_state.environment.project.organisation.github_config.exists() ): - github_configuration = GithubConfiguration.objects.get( - organisation_id=feature_state.environment.project.organisation_id - ) - feature_states = [] - feature_states.append(feature_state) - feature_data: GithubData = generate_data( - github_configuration=github_configuration, - feature=feature_state.feature, - type=WebhookEventType.FLAG_UPDATED.value, - feature_states=feature_states, - ) - call_github_app_webhook_for_feature_state.delay( - args=(asdict(feature_data),), + call_github_task( + organisation_id=feature_state.feature.project.organisation_id, + type=WebhookEventType.FLAG_UPDATED.value, + feature=feature_state.feature, + segment_name=None, + url=None, + feature_states=[feature_state], ) return response + except django.core.exceptions.ValidationError as e: raise serializers.ValidationError(str(e)) diff --git a/api/features/versioning/serializers.py b/api/features/versioning/serializers.py index 1b91b6047a69..5b145ba00873 100644 --- a/api/features/versioning/serializers.py +++ b/api/features/versioning/serializers.py @@ -2,7 +2,9 @@ from features.serializers import CreateSegmentOverrideFeatureStateSerializer from features.versioning.models import EnvironmentFeatureVersion +from integrations.github.github import call_github_task from users.models import FFAdminUser +from webhooks.webhooks import WebhookEventType class EnvironmentFeatureVersionFeatureStateSerializer( @@ -14,6 +16,28 @@ class Meta(CreateSegmentOverrideFeatureStateSerializer.Meta): + ("feature",) ) + def save(self, **kwargs): + response = super().save(**kwargs) + + feature_state = self.instance + if ( + not feature_state.identity_id + and feature_state.feature.external_resources.exists() + and feature_state.environment.project.github_project.exists() + and feature_state.environment.project.organisation.github_config.exists() + ): + + call_github_task( + organisation_id=feature_state.environment.project.organisation_id, + type=WebhookEventType.FLAG_UPDATED.value, + feature=feature_state.feature, + segment_name=None, + url=None, + feature_states=[feature_state], + ) + + return response + class EnvironmentFeatureVersionSerializer(serializers.ModelSerializer): class Meta: diff --git a/api/integrations/github/client.py b/api/integrations/github/client.py index 83330b272cdd..cb4d0565200e 100644 --- a/api/integrations/github/client.py +++ b/api/integrations/github/client.py @@ -1,8 +1,50 @@ +import logging +import typing +from enum import Enum + +import requests from django.conf import settings from github import Auth, Github +from rest_framework import status +from rest_framework.response import Response + +from integrations.github.constants import ( + GITHUB_API_CALLS_TIMEOUT, + GITHUB_API_URL, + GITHUB_API_VERSION, +) +from integrations.github.dataclasses import RepoQueryParams +from integrations.github.models import GithubConfiguration + +logger = logging.getLogger(__name__) + + +class ResourceType(Enum): + ISSUES = "issue" + PULL_REQUESTS = "pr" + + +def build_request_headers( + installation_id: str, use_jwt: bool = False +) -> dict[str, str]: + token = ( + generate_jwt_token(settings.GITHUB_APP_ID) + if use_jwt + else generate_token( + installation_id, + settings.GITHUB_APP_ID, + ) + ) + + return { + "X-GitHub-Api-Version": GITHUB_API_VERSION, + "Accept": "application/vnd.github.v3+json", + "Authorization": f"Bearer {token}", + } -def generate_token(installation_id: str, app_id: int) -> str: +# TODO: Add test coverage for this function +def generate_token(installation_id: str, app_id: int) -> str: # pragma: no cover auth: Auth.AppInstallationAuth = Auth.AppAuth( app_id=int(app_id), private_key=settings.GITHUB_PEM ).get_installation_auth( @@ -12,3 +54,142 @@ def generate_token(installation_id: str, app_id: int) -> str: Github(auth=auth) token = auth.token return token + + +# TODO: Add test coverage for this function +def generate_jwt_token(app_id: int) -> str: # pragma: no cover + github_auth: Auth.AppAuth = Auth.AppAuth( + app_id=app_id, + private_key=settings.GITHUB_PEM, + ) + token = github_auth.create_jwt() + return token + + +def post_comment_to_github( + installation_id: str, owner: str, repo: str, issue: str, body: str +) -> dict[str, typing.Any]: + + url = f"{GITHUB_API_URL}repos/{owner}/{repo}/issues/{issue}/comments" + headers = build_request_headers(installation_id) + payload = {"body": body} + response = response = requests.post( + url, json=payload, headers=headers, timeout=GITHUB_API_CALLS_TIMEOUT + ) + response.raise_for_status() + + return response.json() + + +def delete_github_installation(installation_id: str) -> requests.Response: + url = f"{GITHUB_API_URL}app/installations/{installation_id}" + headers = build_request_headers(installation_id, use_jwt=True) + response = requests.delete(url, headers=headers, timeout=GITHUB_API_CALLS_TIMEOUT) + response.raise_for_status() + return response + + +def fetch_github_resource( + resource_type: ResourceType, + organisation_id: int, + params: RepoQueryParams, +) -> dict[str, typing.Any]: + github_configuration = GithubConfiguration.objects.get( + organisation_id=organisation_id, deleted_at__isnull=True + ) + # Build Github search query + q = ["q="] + if params.search_text: + q.append(params.search_text) + q.append(f"repo:{params.repo_owner}/{params.repo_name}") + q.append(f"is:{resource_type.value}") + if params.state: + q.append(f"is:{params.state}") + q.append("in:title") + if params.search_in_body: + q.append("in:body") + if params.search_in_comments: + q.append("in:comments") + if params.author: + q.append(f"author:{params.author}") + if params.assignee: + q.append(f"assignee:{params.assignee}") + + url = ( + f"{GITHUB_API_URL}search/issues?" + + " ".join(q) + + f"&per_page={params.page_size}&page={params.page}" + ) + headers: dict[str, str] = build_request_headers( + github_configuration.installation_id + ) + response = requests.get(url, headers=headers, timeout=GITHUB_API_CALLS_TIMEOUT) + response.raise_for_status() + json_response = response.json() + results = [ + { + "html_url": i["html_url"], + "id": i["id"], + "title": i["title"], + "number": i["number"], + } + for i in json_response["items"] + ] + data = { + "results": results, + "count": json_response["total_count"], + "incomplete_results": json_response["incomplete_results"], + } + if response.links.get("prev"): + data["previous"] = response.links.get("prev") + + if response.links.get("next"): + data["next"] = response.links.get("next") + + return data + + +def fetch_github_repositories(installation_id: str) -> Response: + url = f"{GITHUB_API_URL}installation/repositories" + + headers: dict[str, str] = build_request_headers(installation_id) + + response = requests.get(url, headers=headers, timeout=GITHUB_API_CALLS_TIMEOUT) + json_response = response.json() + response.raise_for_status() + results = [ + { + "full_name": i["full_name"], + "id": i["id"], + "name": i["name"], + } + for i in json_response["repositories"] + ] + data = { + "repositories": results, + "total_count": json_response["total_count"], + } + return Response( + data=data, + content_type="application/json", + status=status.HTTP_200_OK, + ) + + +def get_github_issue_pr_title_and_state( + organisation_id: int, resource_url: str +) -> dict[str, str]: + url_parts = resource_url.split("/") + owner = url_parts[-4] + repo = url_parts[-3] + number = url_parts[-1] + installation_id = GithubConfiguration.objects.get( + organisation_id=organisation_id, deleted_at__isnull=True + ).installation_id + + url = f"{GITHUB_API_URL}repos/{owner}/{repo}/issues/{number}" + headers = build_request_headers(installation_id) + response = requests.get(url, headers=headers, timeout=GITHUB_API_CALLS_TIMEOUT) + response.raise_for_status() + response_json = response.json() + return {"title": response_json["title"], "state": response_json["state"]} diff --git a/api/integrations/github/constants.py b/api/integrations/github/constants.py index 713e6e73a0e9..929ea690b87e 100644 --- a/api/integrations/github/constants.py +++ b/api/integrations/github/constants.py @@ -10,4 +10,8 @@ UNLINKED_FEATURE_TEXT = "### The feature flag `%s` was unlinked from the issue/PR" UPDATED_FEATURE_TEXT = "Flagsmith Feature `%s` has been updated:\n" DELETED_FEATURE_TEXT = "### The Feature Flag `%s` was deleted" +DELETED_SEGMENT_OVERRIDE_TEXT = ( + "### The Segment Override `%s` for Feature Flag `%s` was deleted" +) FEATURE_ENVIRONMENT_URL = "%s/project/%s/environment/%s/features?feature=%s&tab=%s" +GITHUB_API_CALLS_TIMEOUT = 10 diff --git a/api/integrations/github/dataclasses.py b/api/integrations/github/dataclasses.py new file mode 100644 index 000000000000..f90c11ff52b5 --- /dev/null +++ b/api/integrations/github/dataclasses.py @@ -0,0 +1,44 @@ +import typing +from dataclasses import dataclass +from typing import Optional + + +@dataclass +class GithubData: + installation_id: str + feature_id: int + feature_name: str + type: str + feature_states: list[dict[str, typing.Any]] | None = None + url: str | None = None + project_id: int | None = None + segment_name: str | None = None + + @classmethod + def from_dict(cls, data_dict: dict[str, typing.Any]) -> "GithubData": + return cls(**data_dict) + + +@dataclass +class CallGithubData: + event_type: str + github_data: GithubData + feature_external_resources: list[dict[str, typing.Any]] + + +@dataclass +class RepoQueryParams: + repo_owner: str + repo_name: str + search_text: Optional[str] = None + page: Optional[int] = 1 + page_size: Optional[int] = 100 + state: Optional[str] = "open" + author: Optional[str] = None + assignee: Optional[str] = None + search_in_body: Optional[bool] = True + search_in_comments: Optional[bool] = False + + @classmethod + def from_dict(cls, data_dict: dict[str, typing.Any]) -> "RepoQueryParams": + return cls(**data_dict) diff --git a/api/integrations/github/github.py b/api/integrations/github/github.py index 6e6d5f7758dc..af967bc8f2a5 100644 --- a/api/integrations/github/github.py +++ b/api/integrations/github/github.py @@ -1,89 +1,53 @@ import logging import typing -from dataclasses import dataclass +from dataclasses import asdict -import requests from core.helpers import get_current_site_url -from django.conf import settings from django.utils.formats import get_format from features.models import Feature, FeatureState, FeatureStateValue -from integrations.github.client import generate_token from integrations.github.constants import ( DELETED_FEATURE_TEXT, + DELETED_SEGMENT_OVERRIDE_TEXT, FEATURE_ENVIRONMENT_URL, FEATURE_TABLE_HEADER, FEATURE_TABLE_ROW, - GITHUB_API_URL, LINK_FEATURE_TITLE, LINK_SEGMENT_TITLE, UNLINKED_FEATURE_TEXT, UPDATED_FEATURE_TEXT, ) +from integrations.github.dataclasses import GithubData from integrations.github.models import GithubConfiguration +from integrations.github.tasks import call_github_app_webhook_for_feature_state from webhooks.webhooks import WebhookEventType logger = logging.getLogger(__name__) -@dataclass -class GithubData: - installation_id: str - feature_id: int - feature_name: str - type: str - feature_states: typing.List[dict[str, typing.Any]] | None = None - url: str | None = None - project_id: int | None = None - - @classmethod - def from_dict(cls, data_dict: dict) -> "GithubData": - return cls(**data_dict) - - -def post_comment_to_github( - installation_id: str, owner: str, repo: str, issue: str, body: str -) -> typing.Optional[typing.Dict[str, typing.Any]]: - try: - token = generate_token( - installation_id, - settings.GITHUB_APP_ID, - ) - - url = f"{GITHUB_API_URL}repos/{owner}/{repo}/issues/{issue}/comments" - headers = { - "Accept": "application/vnd.github.v3+json", - "Authorization": f"Bearer {token}", - } - - payload = {"body": body} - response = response = requests.post( - url, json=payload, headers=headers, timeout=10 - ) - - return response.json() if response.status_code == 201 else None - except requests.RequestException as e: - logger.error(f" {e}") - return None - - def generate_body_comment( name: str, event_type: str, project_id: int, feature_id: int, - feature_states: typing.List[typing.Dict[str, typing.Any]], + feature_states: list[dict[str, typing.Any]], + segment_name: str | None = None, ) -> str: is_update = event_type == WebhookEventType.FLAG_UPDATED.value is_removed = event_type == WebhookEventType.FEATURE_EXTERNAL_RESOURCE_REMOVED.value - delete_text = UNLINKED_FEATURE_TEXT % (name,) + is_segment_override_deleted = ( + event_type == WebhookEventType.SEGMENT_OVERRIDE_DELETED.value + ) if event_type == WebhookEventType.FLAG_DELETED.value: - return DELETED_FEATURE_TEXT % (name,) + return DELETED_FEATURE_TEXT % (name) if is_removed: - return delete_text + return UNLINKED_FEATURE_TEXT % (name) + + if is_segment_override_deleted and segment_name is not None: + return DELETED_SEGMENT_OVERRIDE_TEXT % (segment_name, name) result = UPDATED_FEATURE_TEXT % (name) if is_update else LINK_FEATURE_TITLE % (name) last_segment_name = "" @@ -131,6 +95,7 @@ def generate_data( typing.Union[list[FeatureState], list[FeatureStateValue]] | None ) = None, url: str | None = None, + segment_name: str | None = None, ) -> GithubData: if feature_states: feature_states_list = [] @@ -171,4 +136,32 @@ def generate_data( ), feature_states=feature_states_list if feature_states else None, project_id=feature.project_id, + segment_name=segment_name, + ) + + +def call_github_task( + organisation_id: str, + type: str, + feature: Feature, + segment_name: str | None, + url: str | None, + feature_states: typing.Union[list[typing.Any], list[typing.Any]] | None, +) -> None: + + github_configuration = GithubConfiguration.objects.get( + organisation_id=organisation_id + ) + + feature_data: GithubData = generate_data( + github_configuration=github_configuration, + feature=feature, + type=type, + url=url, + segment_name=segment_name, + feature_states=feature_states, + ) + + call_github_app_webhook_for_feature_state.delay( + args=(asdict(feature_data),), ) diff --git a/api/integrations/github/serializers.py b/api/integrations/github/serializers.py index 0330096cae2d..88f74f85f022 100644 --- a/api/integrations/github/serializers.py +++ b/api/integrations/github/serializers.py @@ -1,7 +1,9 @@ from rest_framework import serializers from rest_framework.serializers import ModelSerializer +from rest_framework_dataclasses.serializers import DataclassSerializer -from .models import GithubConfiguration, GithubRepository +from integrations.github.dataclasses import RepoQueryParams +from integrations.github.models import GithubConfiguration, GithubRepository class GithubConfigurationSerializer(ModelSerializer): @@ -14,6 +16,7 @@ class Meta: class GithubRepositorySerializer(ModelSerializer): class Meta: model = GithubRepository + optional_fields = ("search_text", "page") fields = ( "id", "github_configuration", @@ -27,6 +30,8 @@ class Meta: ) -class RepoQuerySerializer(serializers.Serializer): - repo_owner = serializers.CharField(required=True) - repo_name = serializers.CharField(required=True) +class RepoQueryParamsSerializer(DataclassSerializer): + class Meta: + dataclass = RepoQueryParams + + search_in_body = serializers.BooleanField(required=False, default=True) diff --git a/api/integrations/github/tasks.py b/api/integrations/github/tasks.py index e241af521475..384bb88f8960 100644 --- a/api/integrations/github/tasks.py +++ b/api/integrations/github/tasks.py @@ -1,28 +1,19 @@ import logging -from dataclasses import dataclass -from typing import Any +from typing import Any, List from urllib.parse import urlparse from features.models import Feature -from integrations.github.github import ( - GithubData, - generate_body_comment, - post_comment_to_github, -) +from integrations.github.client import post_comment_to_github +from integrations.github.dataclasses import CallGithubData from task_processor.decorators import register_task_handler from webhooks.webhooks import WebhookEventType logger = logging.getLogger(__name__) -@dataclass -class CallGithubData: - event_type: str - github_data: GithubData - feature_external_resources: list[dict[str, Any]] - - def send_post_request(data: CallGithubData) -> None: + from integrations.github.github import generate_body_comment + feature_name = data.github_data.feature_name feature_id = data.github_data.feature_id project_id = data.github_data.project_id @@ -31,8 +22,9 @@ def send_post_request(data: CallGithubData) -> None: data.github_data.feature_states if data.github_data.feature_states else None ) installation_id = data.github_data.installation_id + segment_name: str | None = data.github_data.segment_name body = generate_body_comment( - feature_name, event_type, project_id, feature_id, feature_states + feature_name, event_type, project_id, feature_id, feature_states, segment_name ) if ( @@ -71,11 +63,12 @@ def call_github_app_webhook_for_feature_state(event_data: dict[str, Any]) -> Non from features.feature_external_resources.models import ( FeatureExternalResource, ) + from integrations.github.github import GithubData github_event_data = GithubData.from_dict(event_data) def generate_feature_external_resources( - feature_external_resources: FeatureExternalResource, + feature_external_resources: List[FeatureExternalResource], ) -> list[dict[str, Any]]: return [ { @@ -85,10 +78,15 @@ def generate_feature_external_resources( for resource in feature_external_resources ] - if github_event_data.type == WebhookEventType.FLAG_DELETED.value: + if ( + github_event_data.type == WebhookEventType.FLAG_DELETED.value + or github_event_data.type == WebhookEventType.SEGMENT_OVERRIDE_DELETED.value + ): feature_external_resources = generate_feature_external_resources( - FeatureExternalResource.objects.filter( - feature_id=github_event_data.feature_id + list( + FeatureExternalResource.objects.filter( + feature_id=github_event_data.feature_id + ) ) ) data = CallGithubData( diff --git a/api/integrations/github/views.py b/api/integrations/github/views.py index c62f49d60300..259f41d46ef9 100644 --- a/api/integrations/github/views.py +++ b/api/integrations/github/views.py @@ -1,6 +1,8 @@ import json +import logging import re from functools import wraps +from typing import Any, Callable import requests from django.conf import settings @@ -11,8 +13,13 @@ from rest_framework.permissions import AllowAny, IsAuthenticated from rest_framework.response import Response -from integrations.github.client import generate_token -from integrations.github.constants import GITHUB_API_URL, GITHUB_API_VERSION +from integrations.github.client import ( + ResourceType, + delete_github_installation, + fetch_github_repositories, + fetch_github_resource, +) +from integrations.github.dataclasses import RepoQueryParams from integrations.github.exceptions import DuplicateGitHubIntegration from integrations.github.helpers import github_webhook_payload_is_valid from integrations.github.models import GithubConfiguration, GithubRepository @@ -20,14 +27,14 @@ from integrations.github.serializers import ( GithubConfigurationSerializer, GithubRepositorySerializer, - RepoQuerySerializer, + RepoQueryParamsSerializer, ) -from organisations.models import Organisation from organisations.permissions.permissions import GithubIsAdminOrganisation +logger = logging.getLogger(__name__) -def github_auth_required(func): +def github_auth_required(func): @wraps(func) def wrapper(request, organisation_pk): @@ -46,6 +53,28 @@ def wrapper(request, organisation_pk): return wrapper +def github_api_call_error_handler( + error: str | None = None, +) -> Callable[..., Callable[..., Any]]: + def decorator(func): + @wraps(func) + def wrapper(*args, **kwargs) -> Response: + default_error = "Failed to retrieve requested information from GitHub API." + try: + return func(*args, **kwargs) + except requests.RequestException as e: + logger.error(f"{error or default_error} Error: {str(e)}", exc_info=e) + return Response( + data={"detail": (f"{error or default_error}" f" Error: {str(e)}")}, + content_type="application/json", + status=status.HTTP_502_BAD_GATEWAY, + ) + + return wrapper + + return decorator + + class GithubConfigurationViewSet(viewsets.ModelViewSet): permission_classes = ( IsAuthenticated, @@ -71,6 +100,11 @@ def create(self, request, *args, **kwargs): if re.search(r"Key \(organisation_id\)=\(\d+\) already exists", str(e)): raise DuplicateGitHubIntegration + @github_api_call_error_handler(error="Failed to delete GitHub Installation.") + def destroy(self, request, *args, **kwargs): + delete_github_installation(self.get_object().installation_id) + return super().destroy(request, *args, **kwargs) + class GithubRepositoryViewSet(viewsets.ModelViewSet): permission_classes = ( @@ -86,9 +120,12 @@ def perform_create(self, serializer): serializer.save(github_configuration_id=github_configuration_id) def get_queryset(self): - return GithubRepository.objects.filter( - github_configuration=self.kwargs["github_pk"] - ) + try: + if github_pk := self.kwargs.get("github_pk"): + int(github_pk) + return GithubRepository.objects.filter(github_configuration=github_pk) + except ValueError: + raise ValidationError({"github_pk": ["Must be an integer"]}) def create(self, request, *args, **kwargs): @@ -108,103 +145,64 @@ def create(self, request, *args, **kwargs): @api_view(["GET"]) @permission_classes([IsAuthenticated, HasPermissionToGithubConfiguration]) @github_auth_required -def fetch_pull_requests(request, organisation_pk) -> Response: - organisation = Organisation.objects.get(id=organisation_pk) - github_configuration = GithubConfiguration.objects.get( - organisation=organisation, deleted_at__isnull=True - ) - token = generate_token( - github_configuration.installation_id, - settings.GITHUB_APP_ID, - ) - - query_serializer = RepoQuerySerializer(data=request.query_params) +@github_api_call_error_handler(error="Failed to retrieve GitHub pull requests.") +def fetch_pull_requests(request, organisation_pk) -> Response | None: + query_serializer = RepoQueryParamsSerializer(data=request.query_params) if not query_serializer.is_valid(): return Response({"error": query_serializer.errors}, status=400) - repo_owner = query_serializer.validated_data.get("repo_owner") - repo_name = query_serializer.validated_data.get("repo_name") + query_params = RepoQueryParams.from_dict(query_serializer.validated_data.__dict__) - url = f"{GITHUB_API_URL}repos/{repo_owner}/{repo_name}/pulls" - - headers = { - "X-GitHub-Api-Version": GITHUB_API_VERSION, - "Accept": "application/vnd.github.v3+json", - "Authorization": f"Bearer {token}", - } - - try: - response = requests.get(url, headers=headers, timeout=10) - response.raise_for_status() - data = response.json() - return Response(data) - except requests.RequestException as e: - return Response({"error": str(e)}, status=500) + data = fetch_github_resource( + resource_type=ResourceType.PULL_REQUESTS, + organisation_id=organisation_pk, + params=query_params, + ) + return Response( + data=data, + content_type="application/json", + status=status.HTTP_200_OK, + ) @api_view(["GET"]) @permission_classes([IsAuthenticated, HasPermissionToGithubConfiguration]) @github_auth_required -def fetch_issues(request, organisation_pk) -> Response: - organisation = Organisation.objects.get(id=organisation_pk) - github_configuration = GithubConfiguration.objects.get( - organisation=organisation, deleted_at__isnull=True - ) - token = generate_token( - github_configuration.installation_id, - settings.GITHUB_APP_ID, - ) - - query_serializer = RepoQuerySerializer(data=request.query_params) +@github_api_call_error_handler(error="Failed to retrieve GitHub pull requests.") +def fetch_issues(request, organisation_pk) -> Response | None: + query_serializer = RepoQueryParamsSerializer(data=request.query_params) if not query_serializer.is_valid(): return Response({"error": query_serializer.errors}, status=400) - repo_owner = query_serializer.validated_data.get("repo_owner") - repo_name = query_serializer.validated_data.get("repo_name") + query_params = RepoQueryParams.from_dict(query_serializer.validated_data.__dict__) - url = f"{GITHUB_API_URL}repos/{repo_owner}/{repo_name}/issues" - - headers = { - "X-GitHub-Api-Version": GITHUB_API_VERSION, - "Accept": "application/vnd.github.v3+json", - "Authorization": f"Bearer {token}", - } - - try: - response = requests.get(url, headers=headers, timeout=10) - response.raise_for_status() - data = response.json() - filtered_data = [issue for issue in data if "pull_request" not in issue] - return Response(filtered_data) - except requests.RequestException as e: - return Response({"error": str(e)}, status=500) + data = fetch_github_resource( + resource_type=ResourceType.ISSUES, + organisation_id=organisation_pk, + params=query_params, + ) + return Response( + data=data, + content_type="application/json", + status=status.HTTP_200_OK, + ) @api_view(["GET"]) @permission_classes([IsAuthenticated, GithubIsAdminOrganisation]) -def fetch_repositories(request, organisation_pk: int) -> Response: +@github_api_call_error_handler(error="Failed to retrieve GitHub pull requests.") +def fetch_repositories(request, organisation_pk: int) -> Response | None: installation_id = request.GET.get("installation_id") - token = generate_token( - installation_id, - settings.GITHUB_APP_ID, - ) + if not installation_id: + return Response( + data={"detail": "Missing installation_id parameter"}, + content_type="application/json", + status=status.HTTP_400_BAD_REQUEST, + ) - url = f"{GITHUB_API_URL}installation/repositories" - - headers = { - "X-GitHub-Api-Version": GITHUB_API_VERSION, - "Accept": "application/vnd.github.v3+json", - "Authorization": f"Bearer {token}", - } - - try: - response = requests.get(url, headers=headers, timeout=10) - response.raise_for_status() - data = response.json() - return Response(data) - except requests.RequestException as e: - return Response({"error": str(e)}, status=500) + response: Response = fetch_github_repositories(installation_id) + return response @api_view(["POST"]) diff --git a/api/poetry.lock b/api/poetry.lock index b00250d648b0..9c8f26aa662a 100644 --- a/api/poetry.lock +++ b/api/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 1.8.3 and should not be changed by hand. +# This file is automatically @generated by Poetry 1.8.2 and should not be changed by hand. [[package]] name = "aiohttp" @@ -1239,6 +1239,25 @@ files = [ {file = "djangorestframework_api_key-2.2.0-py3-none-any.whl", hash = "sha256:c9884e52f2802994a02781eaba0a63a628a150ed55d58603d5c9c57a6657de43"}, ] +[[package]] +name = "djangorestframework-dataclasses" +version = "1.3.1" +description = "A dataclasses serializer for Django REST Framework" +optional = false +python-versions = ">=3.7" +files = [ + {file = "djangorestframework-dataclasses-1.3.1.tar.gz", hash = "sha256:d3796b5ce3f7266d525493c557ce7df9ffeae4367006250298ea4d94da4106c4"}, + {file = "djangorestframework_dataclasses-1.3.1-py3-none-any.whl", hash = "sha256:ca1aa1ca99b5306af874376f37355593bb3d1ac7d658d54e2790f9b303968065"}, +] + +[package.dependencies] +django = ">=2.0" +djangorestframework = ">=3.9" + +[package.extras] +dev = ["django-stubs", "djangorestframework-stubs", "mypy (>=1.5.1,<1.6.0)"] +test = ["coverage[toml]", "tox"] + [[package]] name = "djangorestframework-recursive" version = "0.1.2" @@ -4823,4 +4842,4 @@ requests = ">=2.7,<3.0" [metadata] lock-version = "2.0" python-versions = "~3.12" -content-hash = "5cca4c008090bced31ae5f4641f5593a520218bca48ed393690268cbc20c0ec4" +content-hash = "3a66d48c9a636de037518808ea68b3ead7fcf558e913c4271b5f72d54e3f255f" diff --git a/api/pyproject.toml b/api/pyproject.toml index 0b372f478542..aa1826c71f1e 100644 --- a/api/pyproject.toml +++ b/api/pyproject.toml @@ -114,6 +114,7 @@ python-gnupg = "^0.5.1" django-redis = "^5.4.0" pygithub = "2.1.1" hubspot-api-client = "^8.2.1" +djangorestframework-dataclasses = "^1.3.1" [tool.poetry.group.auth-controller] optional = true 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 84d4b169018f..f014ca26c6f6 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,3 +1,6 @@ +from datetime import datetime + +import responses import simplejson as json from django.core.serializers.json import DjangoJSONEncoder from django.urls import reverse @@ -9,14 +12,18 @@ 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, FeatureState +from features.models import Feature, FeatureSegment, FeatureState from features.serializers import ( FeatureStateSerializerBasic, WritableNestedFeatureStateSerializer, ) +from features.versioning.models import EnvironmentFeatureVersion +from integrations.github.constants import GITHUB_API_URL, GITHUB_API_VERSION from integrations.github.models import GithubConfiguration, GithubRepository from projects.models import Project +from segments.models import Segment from tests.types import WithEnvironmentPermissionsCallable +from users.models import FFAdminUser _django_json_encoder_default = DjangoJSONEncoder().default @@ -71,6 +78,7 @@ def json(self): return MockResponse(json_data={"data": "data"}, status_code=200) +@responses.activate def test_create_feature_external_resource( admin_client_new: APIClient, feature_with_value: Feature, @@ -83,7 +91,7 @@ def test_create_feature_external_resource( ) -> None: # Given mock_generate_token = mocker.patch( - "integrations.github.github.generate_token", + "integrations.github.client.generate_token", ) mock_generate_token.return_value = "mocked_token" @@ -91,16 +99,33 @@ def test_create_feature_external_resource( "requests.post", side_effect=mocked_requests_post ) - feature_state = FeatureState.objects.filter(feature=feature_with_value).first() - feature_state_updated_at = feature_state.updated_at.strftime( - get_format("DATETIME_INPUT_FORMATS")[0] + feature_external_resource_data = { + "type": "GITHUB_ISSUE", + "url": "https://github.com/repoowner/repo-name/issues/35", + "feature": feature_with_value.id, + "metadata": {"state": "open"}, + } + + url = reverse( + "api-v1:projects:feature-external-resources-list", + kwargs={"project_pk": project.id, "feature_pk": feature_with_value.id}, ) - segment_override_updated_at = ( - segment_featurestate_and_feature_with_value.updated_at.strftime( - get_format("DATETIME_INPUT_FORMATS")[0] - ) + + # When + response = admin_client_new.post( + url, data=feature_external_resource_data, format="json" ) + # Then + feature_state_update_at = ( + FeatureState.objects.filter(feature=feature_with_value) + .first() + .updated_at.strftime(get_format("DATETIME_INPUT_FORMATS")[0]) + ) + segment_override_updated_at = FeatureState.objects.get( + id=segment_featurestate_and_feature_with_value.id + ).updated_at.strftime(get_format("DATETIME_INPUT_FORMATS")[0]) + expected_comment_body = ( "**Flagsmith feature linked:** `feature_with_value`\n" + "Default Values:\n" @@ -108,7 +133,7 @@ def test_create_feature_external_resource( project.id, environment.api_key, feature_with_value.id, - feature_state_updated_at, + feature_state_update_at, ) + "\n" + expected_segment_comment_body( @@ -120,30 +145,12 @@ def test_create_feature_external_resource( "`value`", ) ) - - feature_external_resource_data = { - "type": "GITHUB_ISSUE", - "url": "https://github.com/repoowner/repo-name/issues/35", - "feature": feature_with_value.id, - "metadata": {"status": "open"}, - } - - url = reverse( - "api-v1:projects:feature-external-resources-list", - kwargs={"project_pk": project.id, "feature_pk": feature_with_value.id}, - ) - - # When - response = admin_client_new.post( - 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={"body": f"{expected_comment_body}"}, headers={ "Accept": "application/vnd.github.v3+json", + "X-GitHub-Api-Version": GITHUB_API_VERSION, "Authorization": "Bearer mocked_token", }, timeout=10, @@ -164,6 +171,12 @@ def test_create_feature_external_resource( assert feature_external_resources[0].url == feature_external_resource_data["url"] # And When + responses.add( + method="GET", + url=f"{GITHUB_API_URL}repos/repoowner/repo-name/issues/35", + status=200, + json={"title": "resource name", "state": "open"}, + ) url = reverse( "api-v1:projects:feature-external-resources-list", kwargs={"project_pk": project.id, "feature_pk": feature_with_value.id}, @@ -173,11 +186,13 @@ def test_create_feature_external_resource( # Then assert response.status_code == status.HTTP_200_OK - assert response.json()["count"] == 1 + assert len(response.json()["results"]) == 1 assert ( response.json()["results"][0]["type"] == feature_external_resource_data["type"] ) assert response.json()["results"][0]["url"] == feature_external_resource_data["url"] + feature_external_resource_data["metadata"]["title"] = "resource name" + assert ( response.json()["results"][0]["metadata"] == feature_external_resource_data["metadata"] @@ -294,7 +309,7 @@ def test_delete_feature_external_resource( ) -> None: # Given mock_generate_token = mocker.patch( - "integrations.github.github.generate_token", + "integrations.github.client.generate_token", ) mock_generate_token.return_value = "mocked_token" github_request_mock = mocker.patch( @@ -316,6 +331,7 @@ def test_delete_feature_external_resource( }, headers={ "Accept": "application/vnd.github.v3+json", + "X-GitHub-Api-Version": GITHUB_API_VERSION, "Authorization": "Bearer mocked_token", }, timeout=10, @@ -326,6 +342,7 @@ def test_delete_feature_external_resource( ).exists() +@responses.activate def test_get_feature_external_resources( admin_client_new: APIClient, feature_external_resource: FeatureExternalResource, @@ -333,13 +350,24 @@ def test_get_feature_external_resources( project: Project, github_configuration: GithubConfiguration, github_repository: GithubRepository, + mocker: MockerFixture, ) -> None: # Given + mocker.patch( + "integrations.github.client.generate_token", + ) url = reverse( "api-v1:projects:feature-external-resources-list", kwargs={"project_pk": project.id, "feature_pk": feature.id}, ) + responses.add( + method="GET", + url=f"{GITHUB_API_URL}repos/userexample/example-project-repo/issues/11", + status=200, + json={"title": "resource name", "state": "open"}, + ) + # When response = admin_client_new.get(url) @@ -372,6 +400,7 @@ def test_get_feature_external_resource( def test_create_github_comment_on_feature_state_updated( + staff_user: FFAdminUser, staff_client: APIClient, with_environment_permissions: WithEnvironmentPermissionsCallable, feature_external_resource: FeatureExternalResource, @@ -383,22 +412,34 @@ def test_create_github_comment_on_feature_state_updated( environment: Environment, ) -> None: # Given - with_environment_permissions([UPDATE_FEATURE_STATE]) + with_environment_permissions([UPDATE_FEATURE_STATE], environment.id, False) feature_state = FeatureState.objects.get( feature=feature, environment=environment.id ) mock_generate_token = mocker.patch( - "integrations.github.github.generate_token", + "integrations.github.client.generate_token", ) mock_generate_token.return_value = "mocked_token" github_request_mock = mocker.patch( "requests.post", side_effect=mocked_requests_post ) - feature_state_updated_at = feature_state.updated_at.strftime( - get_format("DATETIME_INPUT_FORMATS")[0] + 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 + feature_state_updated_at = FeatureState.objects.get( + id=feature_state.id + ).updated_at.strftime(get_format("DATETIME_INPUT_FORMATS")[0]) + expected_body_comment = ( "Flagsmith Feature `Test Feature1` has been updated:\n" + expected_default_body( @@ -411,18 +452,6 @@ def test_create_github_comment_on_feature_state_updated( ) ) - 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( @@ -430,6 +459,7 @@ def test_create_github_comment_on_feature_state_updated( json={"body": expected_body_comment}, headers={ "Accept": "application/vnd.github.v3+json", + "X-GitHub-Api-Version": GITHUB_API_VERSION, "Authorization": "Bearer mocked_token", }, timeout=10, @@ -448,7 +478,7 @@ def test_create_github_comment_on_feature_was_deleted( ) -> None: # Given mock_generate_token = mocker.patch( - "integrations.github.github.generate_token", + "integrations.github.client.generate_token", ) mock_generate_token.return_value = "mocked_token" @@ -472,6 +502,7 @@ def test_create_github_comment_on_feature_was_deleted( json={"body": "### The Feature Flag `Test Feature1` was deleted"}, headers={ "Accept": "application/vnd.github.v3+json", + "X-GitHub-Api-Version": GITHUB_API_VERSION, "Authorization": "Bearer mocked_token", }, timeout=10, @@ -492,7 +523,7 @@ def test_create_github_comment_on_segment_override_updated( # Given feature_state = segment_featurestate_and_feature_with_value mock_generate_token = mocker.patch( - "integrations.github.github.generate_token", + "integrations.github.client.generate_token", ) mock_generate_token.return_value = "mocked_token" github_request_mock = mocker.patch( @@ -509,11 +540,13 @@ def test_create_github_comment_on_segment_override_updated( kwargs={"pk": feature_state.id}, ) - segment_override_updated_at = ( - segment_featurestate_and_feature_with_value.updated_at.strftime( - get_format("DATETIME_INPUT_FORMATS")[0] - ) - ) + # When + response = admin_client.put(path=url, data=payload, format="json") + + # Then + segment_override_updated_at = FeatureState.objects.get( + id=segment_featurestate_and_feature_with_value.id + ).updated_at.strftime(get_format("DATETIME_INPUT_FORMATS")[0]) expected_comment_body = ( "Flagsmith Feature `feature_with_value` has been updated:\n" @@ -528,18 +561,195 @@ def test_create_github_comment_on_segment_override_updated( ) ) + 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={"body": expected_comment_body}, + headers={ + "Accept": "application/vnd.github.v3+json", + "X-GitHub-Api-Version": GITHUB_API_VERSION, + "Authorization": "Bearer mocked_token", + }, + timeout=10, + ) + + +def test_create_github_comment_on_segment_override_deleted( + segment_featurestate_and_feature_with_value: FeatureState, + feature_with_value_segment: FeatureSegment, + feature_with_value_external_resource: FeatureExternalResource, + github_configuration: GithubConfiguration, + github_repository: GithubRepository, + mocker: MockerFixture, + admin_client_new: APIClient, +) -> None: + # Given + mock_generate_token = mocker.patch( + "integrations.github.client.generate_token", + ) + mock_generate_token.return_value = "mocked_token" + github_request_mock = mocker.patch( + "requests.post", side_effect=mocked_requests_post + ) + + url = reverse( + viewname="api-v1:features:feature-segment-detail", + kwargs={"pk": feature_with_value_segment.id}, + ) + # When - response = admin_client.put(path=url, data=payload, format="json") + response = admin_client_new.delete(path=url, format="json") # Then - assert response.status_code == status.HTTP_200_OK + + assert response.status_code == status.HTTP_204_NO_CONTENT + + github_request_mock.assert_called_with( + "https://api.github.com/repos/userexample/example-project-repo/issues/11/comments", + json={ + "body": "### The Segment Override `segment` for Feature Flag `feature_with_value` was deleted" + }, + headers={ + "Accept": "application/vnd.github.v3+json", + "X-GitHub-Api-Version": GITHUB_API_VERSION, + "Authorization": "Bearer mocked_token", + }, + timeout=10, + ) + + +def test_create_github_comment_using_v2( + admin_client_new: APIClient, + feature_external_resource: FeatureExternalResource, + environment_v2_versioning: Environment, + segment: Segment, + feature: Feature, + environment: Environment, + project: Project, + github_configuration: GithubConfiguration, + github_repository: GithubRepository, + mocker: MockerFixture, +) -> None: + + # Given + mock_generate_token = mocker.patch( + "integrations.github.client.generate_token", + ) + mock_generate_token.return_value = "mocked_token" + + github_request_mock = mocker.patch( + "requests.post", side_effect=mocked_requests_post + ) + + environment_feature_version = EnvironmentFeatureVersion.objects.create( + environment=environment_v2_versioning, feature=feature + ) + + url = reverse( + "api-v1:versioning:environment-feature-version-featurestates-list", + args=[ + environment_v2_versioning.id, + feature.id, + environment_feature_version.uuid, + ], + ) + + data = { + "feature_segment": {"segment": segment.id}, + "enabled": True, + "feature_state_value": { + "string_value": "segment value!", + }, + } + + # When + response = admin_client_new.post( + url, data=json.dumps(data), content_type="application/json" + ) + response_data = response.json() + + # Then + format = "%Y-%m-%dT%H:%M:%S.%fZ" + formatted_updated_at = datetime.strptime( + response_data["updated_at"], format + ).strftime(get_format("DATETIME_INPUT_FORMATS")[0]) + expected_comment_body = ( + "Flagsmith Feature `Test Feature1` has been updated:\n" + + "\n" + + expected_segment_comment_body( + project.id, + environment.api_key, + feature.id, + formatted_updated_at, + "✅ Enabled", + "`segment value!`", + ) + ) github_request_mock.assert_called_with( "https://api.github.com/repos/userexample/example-project-repo/issues/11/comments", json={"body": expected_comment_body}, headers={ "Accept": "application/vnd.github.v3+json", + "X-GitHub-Api-Version": GITHUB_API_VERSION, "Authorization": "Bearer mocked_token", }, timeout=10, ) + + assert response.status_code == status.HTTP_201_CREATED + + +def test_create_github_comment_using_v2_fails_on_wrong_params( + admin_client_new: APIClient, + feature_external_resource: FeatureExternalResource, + environment_v2_versioning: Environment, + segment: Segment, + feature: Feature, + environment: Environment, + project: Project, + github_configuration: GithubConfiguration, + github_repository: GithubRepository, + mocker: MockerFixture, +) -> None: + + # Given + mock_generate_token = mocker.patch( + "integrations.github.client.generate_token", + ) + mock_generate_token.return_value = "mocked_token" + + github_request_mock = mocker.patch( + "requests.post", side_effect=mocked_requests_post + ) + + environment_feature_version = EnvironmentFeatureVersion.objects.create( + environment=environment_v2_versioning, feature=feature + ) + + url = reverse( + "api-v1:versioning:environment-feature-version-featurestates-list", + args=[ + environment_v2_versioning.id, + feature.id, + environment_feature_version.uuid, + ], + ) + + data = { + "feature_segment": {"segment": segment.id}, + "enabled": True, + "feature_state_value": { + "string_value": {"value": "wrong structure"}, + }, + } + + # When + response = admin_client_new.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + github_request_mock.assert_not_called() diff --git a/api/tests/unit/integrations/github/test_unit_github_views.py b/api/tests/unit/integrations/github/test_unit_github_views.py index c0b158a5abfd..3c2ad3b65ae8 100644 --- a/api/tests/unit/integrations/github/test_unit_github_views.py +++ b/api/tests/unit/integrations/github/test_unit_github_views.py @@ -1,6 +1,8 @@ import json import pytest +import requests +import responses from django.conf import settings from django.urls import reverse from pytest_lazyfixture import lazy_fixture @@ -9,6 +11,7 @@ from rest_framework.test import APIClient from features.feature_external_resources.models import FeatureExternalResource +from integrations.github.constants import GITHUB_API_URL from integrations.github.models import GithubConfiguration, GithubRepository from integrations.github.views import github_webhook_payload_is_valid from organisations.models import Organisation @@ -100,11 +103,13 @@ def test_cannot_create_github_configuration_when_the_organization_already_has_an ) +@responses.activate def test_delete_github_configuration( admin_client_new: APIClient, organisation: Organisation, github_configuration: GithubConfiguration, github_repository: GithubRepository, + mocker: MockerFixture, ) -> None: # Given url = reverse( @@ -114,10 +119,62 @@ def test_delete_github_configuration( github_configuration.id, ], ) + + mock_generate_token = mocker.patch( + "integrations.github.client.generate_jwt_token", + ) + mock_generate_token.return_value = "mocked_token" + responses.add( + method="DELETE", + url=f"{GITHUB_API_URL}app/installations/{github_configuration.installation_id}", + status=204, + ) + # When response = admin_client_new.delete(url) + # Then assert response.status_code == status.HTTP_204_NO_CONTENT + assert not GithubConfiguration.objects.filter(id=github_configuration.id).exists() + + +@responses.activate +def test_cannot_delete_github_configuration_when_delete_github_installation_response_was_404( + admin_client_new: APIClient, + organisation: Organisation, + github_configuration: GithubConfiguration, + github_repository: GithubRepository, + mocker: MockerFixture, +) -> None: + # Given + url = reverse( + "api-v1:organisations:integrations-github-detail", + args=[ + organisation.id, + github_configuration.id, + ], + ) + + mock_generate_token = mocker.patch( + "integrations.github.client.generate_jwt_token", + ) + mock_generate_token.return_value = "mocked_token" + responses.add( + method="DELETE", + url=f"{GITHUB_API_URL}app/installations/{github_configuration.installation_id}", + status=404, + json={"message": "not found"}, + ) + + # When + response = admin_client_new.delete(url) + # Then + assert response.status_code == status.HTTP_502_BAD_GATEWAY + assert ( + response.json()["detail"] + == "Failed to delete GitHub Installation. Error: 404 Client Error: Not Found for url: https://api.github.com/app/installations/1234567" # noqa: E501 + ) + assert GithubConfiguration.objects.filter(id=github_configuration.id).exists() def test_get_github_repository( @@ -136,6 +193,23 @@ def test_get_github_repository( assert response.status_code == status.HTTP_200_OK +def test_cannot_get_github_repository_when_github_pk_in_not_a_number( + admin_client_new: APIClient, + organisation: Organisation, + github_configuration: GithubConfiguration, +): + # Given + url = reverse( + "api-v1:organisations:repositories-list", + args=[organisation.id, "str"], + ) + # When + response = admin_client_new.get(url) + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json() == {"github_pk": ["Must be an integer"]} + + def test_create_github_repository( admin_client_new: APIClient, organisation: Organisation, @@ -228,7 +302,7 @@ def test_github_delete_repository( ) -> None: # Given mock_generate_token = mocker.patch( - "integrations.github.github.generate_token", + "integrations.github.client.generate_token", ) mock_generate_token.return_value = "mocked_token" url = reverse( @@ -245,19 +319,48 @@ def test_github_delete_repository( assert not FeatureExternalResource.objects.filter(feature=feature).exists() -def mocked_requests_get(*args, **kwargs): - class MockResponse: - def __init__(self, json_data, status_code): - self.json_data = json_data - self.status_code = status_code +class MockResponse: + def __init__(self, json_data, status_code): + self.json_data = json_data + self.status_code = status_code + self.links = { + "next": "https://example.com/next", + "prev": "https://example.com/prev", + } + + def raise_for_status(self) -> None: + if 400 <= self.status_code < 600: + raise requests.exceptions.HTTPError(f"HTTP Error {self.status_code}") + + def json(self): + return self.json_data + + +def mocked_requests_get_issues_and_pull_requests(*args, **kwargs): + json_data = { + "items": [ + { + "html_url": "https://example.com/1", + "id": 1, + "title": "Title 1", + "number": 101, + }, + ], + "total_count": 1, + "incomplete_results": 0, + } + status_code = 200 + response = MockResponse(json_data, status_code) + + return response - def raise_for_status(self) -> None: - pass - def json(self): - return self.json_data +def mocked_requests_get_error(*args, **kwargs): + json_data = {"detail": "Not found"} + status_code = 404 + response = MockResponse(json_data, status_code) - return MockResponse(json_data={"data": "data"}, status_code=200) + return response def test_fetch_pull_requests( @@ -265,15 +368,17 @@ def test_fetch_pull_requests( organisation: Organisation, github_configuration: GithubConfiguration, github_repository: GithubRepository, - mocker, + mocker: MockerFixture, ) -> None: # Given mock_generate_token = mocker.patch( - "integrations.github.views.generate_token", + "integrations.github.client.generate_token", ) mock_generate_token.return_value = "mocked_token" - github_request_mock = mocker.patch("requests.get", side_effect=mocked_requests_get) + github_request_mock = mocker.patch( + "requests.get", side_effect=mocked_requests_get_issues_and_pull_requests + ) url = reverse("api-v1:organisations:get-github-pulls", args=[organisation.id]) data = {"repo_owner": "owner", "repo_name": "repo"} @@ -283,11 +388,11 @@ def test_fetch_pull_requests( response_json = response.json() # Then - assert response.status_code == 200 - assert "data" in response_json + assert response.status_code == status.HTTP_200_OK + assert "results" in response_json github_request_mock.assert_called_with( - "https://api.github.com/repos/owner/repo/pulls", + "https://api.github.com/search/issues?q= repo:owner/repo is:pr is:open in:title in:body&per_page=100&page=1", # noqa: E501 headers={ "X-GitHub-Api-Version": "2022-11-28", "Accept": "application/vnd.github.v3+json", @@ -297,31 +402,40 @@ def test_fetch_pull_requests( ) -def test_fetch_issue( +def test_fetch_issues( admin_client_new: APIClient, organisation: Organisation, github_configuration: GithubConfiguration, github_repository: GithubRepository, - mocker, + mocker: MockerFixture, ) -> None: # Given mock_generate_token = mocker.patch( - "integrations.github.views.generate_token", + "integrations.github.client.generate_token", ) mock_generate_token.return_value = "mocked_token" - github_request_mock = mocker.patch("requests.get", side_effect=mocked_requests_get) + github_request_mock = mocker.patch( + "requests.get", side_effect=mocked_requests_get_issues_and_pull_requests + ) url = reverse("api-v1:organisations:get-github-issues", args=[organisation.id]) - data = {"repo_owner": "owner", "repo_name": "repo"} + data = { + "repo_owner": "owner", + "repo_name": "repo", + "search_text": "search text", + "search_in_comments": True, + "author": "author", + "assignee": "assignee", + } # When response = admin_client_new.get(url, data=data) # Then - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK response_json = response.json() - assert "data" in response_json + assert "results" in response_json github_request_mock.assert_called_with( - "https://api.github.com/repos/owner/repo/issues", + "https://api.github.com/search/issues?q= search text repo:owner/repo is:issue is:open in:title in:body in:comments author:author assignee:assignee&per_page=100&page=1", # noqa: E501 headers={ "X-GitHub-Api-Version": "2022-11-28", "Accept": "application/vnd.github.v3+json", @@ -331,39 +445,75 @@ def test_fetch_issue( ) +def test_fetch_issues_returns_error_on_bad_response_from_github( + admin_client_new: APIClient, + organisation: Organisation, + github_configuration: GithubConfiguration, + github_repository: GithubRepository, + mocker: MockerFixture, +) -> None: + # Given + mock_generate_token = mocker.patch( + "integrations.github.client.generate_token", + ) + mock_generate_token.return_value = "mocked_token" + mocker.patch("requests.get", side_effect=mocked_requests_get_error) + url = reverse("api-v1:organisations:get-github-issues", args=[organisation.id]) + data = {"repo_owner": "owner", "repo_name": "repo"} + # When + response = admin_client_new.get(url, data=data) + + # Then + assert response.status_code == status.HTTP_502_BAD_GATEWAY + response_json = response.json() + assert ( + "Failed to retrieve GitHub pull requests. Error: HTTP Error 404" + in response_json["detail"] + ) + + +@responses.activate def test_fetch_repositories( admin_client_new: APIClient, organisation: Organisation, github_configuration: GithubConfiguration, github_repository: GithubRepository, - mocker, + mocker: MockerFixture, ) -> None: # Given mock_generate_token = mocker.patch( - "integrations.github.views.generate_token", + "integrations.github.client.generate_token", ) mock_generate_token.return_value = "mocked_token" - github_request_mock = mocker.patch("requests.get", side_effect=mocked_requests_get) + responses.add( + method="GET", + url=f"{GITHUB_API_URL}installation/repositories", + status=status.HTTP_200_OK, + json={ + "repositories": [ + { + "full_name": "owner/repo-name", + "id": 1, + "name": "repo-name", + }, + ], + "total_count": 1, + }, + ) + url = reverse( "api-v1:organisations:get-github-installation-repos", args=[organisation.id] ) # When - response = admin_client_new.get(url) + response = admin_client_new.get( + url, data={"installation_id": github_configuration.installation_id} + ) # Then - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK response_json = response.json() - assert "data" in response_json - - github_request_mock.assert_called_with( - "https://api.github.com/installation/repositories", - headers={ - "X-GitHub-Api-Version": "2022-11-28", - "Accept": "application/vnd.github.v3+json", - "Authorization": "Bearer mocked_token", - }, - timeout=10, - ) + assert "repositories" in response_json + assert len(response_json["repositories"]) == 1 @pytest.mark.parametrize( @@ -386,7 +536,7 @@ def test_fetch_issues_and_pull_requests_fails_with_status_400_when_integration_n ) -> None: # Given mock_generate_token = mocker.patch( - "integrations.github.views.generate_token", + "integrations.github.client.generate_token", ) mock_generate_token.generate_token.return_value = "mocked_token" # When @@ -394,7 +544,7 @@ def test_fetch_issues_and_pull_requests_fails_with_status_400_when_integration_n response = client.get(url) # Then - assert response.status_code == 400 + assert response.status_code == status.HTTP_400_BAD_REQUEST @pytest.mark.parametrize( @@ -414,7 +564,7 @@ def test_cannot_fetch_issues_or_prs_when_does_not_have_permissions( ) -> None: # Given mock_generate_token = mocker.patch( - "integrations.github.views.generate_token", + "integrations.github.client.generate_token", ) mock_generate_token.generate_token.return_value = "mocked_token" @@ -480,7 +630,7 @@ def test_github_webhook_delete_installation( ) # Then - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK assert not GithubConfiguration.objects.filter(installation_id=1234567).exists() @@ -501,7 +651,7 @@ def test_github_webhook_fails_on_signature_header_missing( ) # Then - assert response.status_code == 400 + assert response.status_code == status.HTTP_400_BAD_REQUEST assert response.json() == {"error": "Invalid signature"} assert GithubConfiguration.objects.filter(installation_id=1234567).exists() @@ -524,7 +674,7 @@ def test_github_webhook_fails_on_bad_signature_header_missing( ) # Then - assert response.status_code == 400 + assert response.status_code == status.HTTP_400_BAD_REQUEST assert GithubConfiguration.objects.filter(installation_id=1234567).exists() assert response.json() == {"error": "Invalid signature"} @@ -547,5 +697,82 @@ def test_github_webhook_bypass_event( ) # Then - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK assert GithubConfiguration.objects.filter(installation_id=1234567).exists() + + +@responses.activate +def test_cannot_fetch_pull_requests_when_github_request_call_failed( + admin_client_new: APIClient, + organisation: Organisation, + github_configuration: GithubConfiguration, + github_repository: GithubRepository, + mocker, +) -> None: + + # Given + data = {"repo_owner": "owner", "repo_name": "repo"} + mock_generate_token = mocker.patch( + "integrations.github.client.generate_token", + ) + mock_generate_token.return_value = "mocked_token" + responses.add( + method="GET", + url=f"{GITHUB_API_URL}repos/{data['repo_owner']}/{data['repo_name']}/pulls", + status=404, + ) + + url = reverse("api-v1:organisations:get-github-pulls", args=[organisation.id]) + data = {"repo_owner": "owner", "repo_name": "repo"} + + # When + response = admin_client_new.get(url, data=data) + response_json = response.json() + + # Then + assert response.status_code == status.HTTP_502_BAD_GATEWAY + assert "Failed to retrieve GitHub pull requests." in response_json["detail"] + + +@responses.activate +def test_cannot_fetch_pulls_when_the_github_response_was_invalid( + admin_client_new: APIClient, + organisation: Organisation, + github_configuration: GithubConfiguration, + github_repository: GithubRepository, + mocker, +) -> None: + # Given + data = {"repo_owner": "owner", "repo_name": "repo"} + mock_generate_token = mocker.patch( + "integrations.github.client.generate_token", + ) + mock_generate_token.return_value = "mocked_token" + responses.add( + method="GET", + url=f"{GITHUB_API_URL}repos/{data['repo_owner']}/{data['repo_name']}/pulls", + status=200, + json={"details": "invalid"}, + ) + url = reverse("api-v1:organisations:get-github-issues", args=[organisation.id]) + data = {"repo_owner": "owner", "repo_name": "repo"} + # When + response = admin_client_new.get(url, data=data) + + # Then + assert response.status_code == status.HTTP_502_BAD_GATEWAY + + +def test_cannot_fetch_repositories_when_there_is_no_installation_id( + admin_client_new: APIClient, + organisation: Organisation, +) -> None: + # Given + url = reverse( + "api-v1:organisations:get-github-installation-repos", args=[organisation.id] + ) + # When + response = admin_client_new.get(url) + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json() == {"detail": "Missing installation_id parameter"} diff --git a/api/webhooks/webhooks.py b/api/webhooks/webhooks.py index 3b12200a1872..73e7a082d76d 100644 --- a/api/webhooks/webhooks.py +++ b/api/webhooks/webhooks.py @@ -41,6 +41,7 @@ class WebhookEventType(enum.Enum): NEW_VERSION_PUBLISHED = "NEW_VERSION_PUBLISHED" FEATURE_EXTERNAL_RESOURCE_ADDED = "FEATURE_EXTERNAL_RESOURCE_ADDED" FEATURE_EXTERNAL_RESOURCE_REMOVED = "FEATURE_EXTERNAL_RESOURCE_REMOVED" + SEGMENT_OVERRIDE_DELETED = "SEGMENT_OVERRIDE_DELETED" class WebhookType(enum.Enum):