diff --git a/api/conftest.py b/api/conftest.py index ab0aa198e9e3..ff6732ee44d3 100644 --- a/api/conftest.py +++ b/api/conftest.py @@ -1018,14 +1018,20 @@ def flagsmith_environments_v2_table(dynamodb: DynamoDBServiceResource) -> Table: @pytest.fixture() -def feature_external_resource( - feature: Feature, post_request_mock: MagicMock, mocker: MockerFixture -) -> FeatureExternalResource: - mocker.patch( +def mock_github_client_generate_token(mocker: MockerFixture) -> MagicMock: + return mocker.patch( "integrations.github.client.generate_token", return_value="mocked_token", ) + +@pytest.fixture() +def feature_external_resource( + feature: Feature, + post_request_mock: MagicMock, + mocker: MockerFixture, + mock_github_client_generate_token: MagicMock, +) -> FeatureExternalResource: return FeatureExternalResource.objects.create( url="https://github.com/repositoryownertest/repositorynametest/issues/11", type="GITHUB_ISSUE", @@ -1035,16 +1041,26 @@ def feature_external_resource( @pytest.fixture() -def feature_with_value_external_resource( - feature_with_value: Feature, +def feature_external_resource_gh_pr( + feature: Feature, post_request_mock: MagicMock, mocker: MockerFixture, + mock_github_client_generate_token: MagicMock, ) -> FeatureExternalResource: - mocker.patch( - "integrations.github.client.generate_token", - return_value="mocked_token", + return FeatureExternalResource.objects.create( + url="https://github.com/repositoryownertest/repositorynametest/pull/1", + type="GITHUB_PR", + feature=feature, + metadata='{"status": "open"}', ) + +@pytest.fixture() +def feature_with_value_external_resource( + feature_with_value: Feature, + post_request_mock: MagicMock, + mock_github_client_generate_token: MagicMock, +) -> FeatureExternalResource: return FeatureExternalResource.objects.create( url="https://github.com/repositoryownertest/repositorynametest/issues/11", type="GITHUB_ISSUE", @@ -1069,6 +1085,7 @@ def github_repository( repository_owner="repositoryownertest", repository_name="repositorynametest", project=project, + tagging_enabled=True, ) @@ -1120,3 +1137,10 @@ def handle(self, record: logging.LogRecord) -> None: self.messages.append(self.format(record)) return InspectingHandler() + + +@pytest.fixture +def set_github_webhook_secret() -> None: + from django.conf import settings + + settings.GITHUB_WEBHOOK_SECRET = "secret-key" diff --git a/api/features/feature_external_resources/models.py b/api/features/feature_external_resources/models.py index 6dcfc4d99a7f..8df2428e3158 100644 --- a/api/features/feature_external_resources/models.py +++ b/api/features/feature_external_resources/models.py @@ -1,3 +1,4 @@ +import json import logging from django.db import models @@ -11,9 +12,11 @@ from environments.models import Environment from features.models import Feature, FeatureState +from integrations.github.constants import GitHubEventType, GitHubTag from integrations.github.github import call_github_task +from integrations.github.models import GithubRepository from organisations.models import Organisation -from webhooks.webhooks import WebhookEventType +from projects.tags.models import Tag, TagType logger = logging.getLogger(__name__) @@ -24,6 +27,20 @@ class ResourceType(models.TextChoices): GITHUB_PR = "GITHUB_PR", "GitHub PR" +tag_by_type_and_state = { + ResourceType.GITHUB_ISSUE.value: { + "open": GitHubTag.ISSUE_OPEN.value, + "closed": GitHubTag.ISSUE_CLOSED.value, + }, + ResourceType.GITHUB_PR.value: { + "open": GitHubTag.PR_OPEN.value, + "closed": GitHubTag.PR_CLOSED.value, + "merged": GitHubTag.PR_MERGED.value, + "draft": GitHubTag.PR_DRAFT.value, + }, +} + + class FeatureExternalResource(LifecycleModelMixin, models.Model): url = models.URLField() type = models.CharField(max_length=20, choices=ResourceType.choices) @@ -49,12 +66,33 @@ class Meta: @hook(AFTER_SAVE) def execute_after_save_actions(self): + # Tag the feature with the external resource type + metadata = json.loads(self.metadata) if self.metadata else {} + state = metadata.get("state", "open") + # Add a comment to GitHub Issue/PR when feature is linked to the GH external resource + # and tag the feature with the corresponding tag if tagging is enabled if ( - Organisation.objects.prefetch_related("github_config") + github_configuration := Organisation.objects.prefetch_related( + "github_config" + ) .get(id=self.feature.project.organisation_id) .github_config.first() ): + github_repo = GithubRepository.objects.get( + github_configuration=github_configuration.id, + project=self.feature.project, + ) + if github_repo.tagging_enabled: + github_tag = Tag.objects.get( + label=tag_by_type_and_state[self.type][state], + project=self.feature.project, + is_system_tag=True, + type=TagType.GITHUB.value, + ) + self.feature.tags.add(github_tag) + self.feature.save() + feature_states: list[FeatureState] = [] environments = Environment.objects.filter( @@ -74,7 +112,7 @@ def execute_after_save_actions(self): call_github_task( organisation_id=self.feature.project.organisation_id, - type=WebhookEventType.FEATURE_EXTERNAL_RESOURCE_ADDED.value, + type=GitHubEventType.FEATURE_EXTERNAL_RESOURCE_ADDED.value, feature=self.feature, segment_name=None, url=None, @@ -92,7 +130,7 @@ def execute_before_save_actions(self) -> None: call_github_task( organisation_id=self.feature.project.organisation_id, - type=WebhookEventType.FEATURE_EXTERNAL_RESOURCE_REMOVED.value, + type=GitHubEventType.FEATURE_EXTERNAL_RESOURCE_REMOVED.value, feature=self.feature, segment_name=None, url=self.url, diff --git a/api/features/feature_external_resources/views.py b/api/features/feature_external_resources/views.py index c9636bba1132..641074215fe6 100644 --- a/api/features/feature_external_resources/views.py +++ b/api/features/feature_external_resources/views.py @@ -1,10 +1,16 @@ +import re + from django.shortcuts import get_object_or_404 from rest_framework import status, viewsets from rest_framework.response import Response from features.models import Feature from features.permissions import FeatureExternalResourcePermissions -from integrations.github.client import get_github_issue_pr_title_and_state +from integrations.github.client import ( + get_github_issue_pr_title_and_state, + label_github_issue_pr, +) +from integrations.github.models import GithubRepository from organisations.models import Organisation from .models import FeatureExternalResource @@ -48,14 +54,13 @@ def create(self, request, *args, **kwargs): ), ) - if not ( - ( - Organisation.objects.prefetch_related("github_config") - .get(id=feature.project.organisation_id) - .github_config.first() - ) - or not hasattr(feature.project, "github_project") - ): + github_configuration = ( + Organisation.objects.prefetch_related("github_config") + .get(id=feature.project.organisation_id) + .github_config.first() + ) + + if not github_configuration or not hasattr(feature.project, "github_project"): return Response( data={ "detail": "This Project doesn't have a valid GitHub integration configuration" @@ -63,7 +68,42 @@ def create(self, request, *args, **kwargs): content_type="application/json", status=status.HTTP_400_BAD_REQUEST, ) - return super().create(request, *args, **kwargs) + + # Get repository owner and name, and issue/PR number from the external resource URL + url = request.data.get("url") + if request.data.get("type") == "GITHUB_PR": + pattern = r"github.com/([^/]+)/([^/]+)/pull/(\d+)$" + elif request.data.get("type") == "GITHUB_ISSUE": + pattern = r"github.com/([^/]+)/([^/]+)/issues/(\d+)$" + else: + return Response( + data={"detail": "Incorrect GitHub type"}, + content_type="application/json", + status=status.HTTP_400_BAD_REQUEST, + ) + + match = re.search(pattern, url) + if match: + owner, repo, issue = match.groups() + if GithubRepository.objects.get( + github_configuration=github_configuration, + repository_owner=owner, + repository_name=repo, + ).tagging_enabled: + label_github_issue_pr( + installation_id=github_configuration.installation_id, + owner=owner, + repo=repo, + issue=issue, + ) + response = super().create(request, *args, **kwargs) + return response + else: + return Response( + data={"detail": "Invalid GitHub Issue/PR URL"}, + content_type="application/json", + status=status.HTTP_400_BAD_REQUEST, + ) def perform_update(self, serializer): external_resource_id = int(self.kwargs["pk"]) diff --git a/api/features/models.py b/api/features/models.py index 6a80c5c90251..1493a54c4849 100644 --- a/api/features/models.py +++ b/api/features/models.py @@ -74,6 +74,7 @@ STRING, ) from features.versioning.models import EnvironmentFeatureVersion +from integrations.github.constants import GitHubEventType from metadata.models import Metadata from projects.models import Project from projects.tags.models import Tag @@ -139,7 +140,6 @@ class Meta: @hook(AFTER_SAVE) def create_github_comment(self) -> None: from integrations.github.github import call_github_task - from webhooks.webhooks import WebhookEventType if ( self.external_resources.exists() @@ -150,7 +150,7 @@ def create_github_comment(self) -> None: call_github_task( organisation_id=self.project.organisation_id, - type=WebhookEventType.FLAG_DELETED.value, + type=GitHubEventType.FLAG_DELETED.value, feature=self, segment_name=None, url=None, @@ -406,7 +406,6 @@ def _get_environment(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() @@ -416,7 +415,7 @@ def create_github_comment(self) -> None: call_github_task( self.feature.project.organisation_id, - WebhookEventType.SEGMENT_OVERRIDE_DELETED.value, + GitHubEventType.SEGMENT_OVERRIDE_DELETED.value, self.feature, self.segment.name, None, diff --git a/api/features/serializers.py b/api/features/serializers.py index a2a297238637..5d4de0005517 100644 --- a/api/features/serializers.py +++ b/api/features/serializers.py @@ -19,6 +19,7 @@ from environments.sdk.serializers_mixins import ( HideSensitiveFieldsSerializerMixin, ) +from integrations.github.constants import GitHubEventType from integrations.github.github import call_github_task from metadata.serializers import MetadataSerializer, SerializerWithMetadata from projects.models import Project @@ -30,7 +31,6 @@ from util.drf_writable_nested.serializers import ( DeleteBeforeUpdateWritableNestedModelSerializer, ) -from webhooks.webhooks import WebhookEventType from .constants import INTERSECTION, UNION from .feature_segments.serializers import ( @@ -478,7 +478,7 @@ def save(self, **kwargs): call_github_task( organisation_id=feature_state.feature.project.organisation_id, - type=WebhookEventType.FLAG_UPDATED.value, + type=GitHubEventType.FLAG_UPDATED.value, feature=feature_state.feature, segment_name=None, url=None, diff --git a/api/features/versioning/serializers.py b/api/features/versioning/serializers.py index ec21cdfae5ae..90295f222b8a 100644 --- a/api/features/versioning/serializers.py +++ b/api/features/versioning/serializers.py @@ -8,10 +8,10 @@ CustomCreateSegmentOverrideFeatureStateSerializer, ) from features.versioning.models import EnvironmentFeatureVersion +from integrations.github.constants import GitHubEventType from integrations.github.github import call_github_task from segments.models import Segment from users.models import FFAdminUser -from webhooks.webhooks import WebhookEventType class CustomEnvironmentFeatureVersionFeatureStateSerializer( @@ -36,7 +36,7 @@ def save(self, **kwargs): call_github_task( organisation_id=feature_state.environment.project.organisation_id, - type=WebhookEventType.FLAG_UPDATED.value, + type=GitHubEventType.FLAG_UPDATED.value, feature=feature_state.feature, segment_name=None, url=None, diff --git a/api/integrations/github/client.py b/api/integrations/github/client.py index b4307fd736f9..4d4293b30a48 100644 --- a/api/integrations/github/client.py +++ b/api/integrations/github/client.py @@ -1,3 +1,4 @@ +import json import logging from enum import Enum from typing import Any @@ -5,11 +6,15 @@ import requests from django.conf import settings from github import Auth, Github +from requests.exceptions import HTTPError from integrations.github.constants import ( GITHUB_API_CALLS_TIMEOUT, GITHUB_API_URL, GITHUB_API_VERSION, + GITHUB_FLAGSMITH_LABEL, + GITHUB_FLAGSMITH_LABEL_COLOR, + GITHUB_FLAGSMITH_LABEL_DESCRIPTION, ) from integrations.github.dataclasses import ( IssueQueryParams, @@ -159,6 +164,9 @@ def fetch_search_github_resource( "id": i["id"], "title": i["title"], "number": i["number"], + "state": i["state"], + "merged": i.get("merged", False), + "draft": i.get("draft", False), } for i in json_response["items"] ] @@ -244,3 +252,45 @@ def fetch_github_repo_contributors( ] return build_paginated_response(results, response) + + +def create_flagsmith_flag_label( + installation_id: str, owner: str, repo: str +) -> dict[str, Any]: + # Create "Flagsmith Flag" label in linked repo + url = f"{GITHUB_API_URL}repos/{owner}/{repo}/labels" + headers = build_request_headers(installation_id) + payload = { + "name": GITHUB_FLAGSMITH_LABEL, + "color": GITHUB_FLAGSMITH_LABEL_COLOR, + "description": GITHUB_FLAGSMITH_LABEL_DESCRIPTION, + } + try: + response = requests.post( + url, json=payload, headers=headers, timeout=GITHUB_API_CALLS_TIMEOUT + ) + response.raise_for_status() + return response.json() + + except HTTPError: + response_content = response.content.decode("utf-8") + error_data = json.loads(response_content) + if any( + error["code"] == "already_exists" for error in error_data.get("errors", []) + ): + logger.warning("Label already exists") + return {"message": "Label already exists"}, 200 + + +def label_github_issue_pr( + installation_id: str, owner: str, repo: str, issue: str +) -> dict[str, Any]: + # Label linked GitHub Issue or PR with the "Flagsmith Flag" label + url = f"{GITHUB_API_URL}repos/{owner}/{repo}/issues/{issue}/labels" + headers = build_request_headers(installation_id) + payload = [GITHUB_FLAGSMITH_LABEL] + response = requests.post( + url, json=payload, headers=headers, timeout=GITHUB_API_CALLS_TIMEOUT + ) + response.raise_for_status() + return response.json() diff --git a/api/integrations/github/constants.py b/api/integrations/github/constants.py index 929ea690b87e..897b3a2c6725 100644 --- a/api/integrations/github/constants.py +++ b/api/integrations/github/constants.py @@ -1,3 +1,5 @@ +from enum import Enum + GITHUB_API_URL = "https://api.github.com/" GITHUB_API_VERSION = "2022-11-28" @@ -7,11 +9,49 @@ | :--- | :----- | :------ | :------ |\n""" FEATURE_TABLE_ROW = "| [%s](%s) | %s | %s | %s |\n" LINK_SEGMENT_TITLE = "Segment `%s` values:\n" -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" +UNLINKED_FEATURE_TEXT = "**The feature flag `%s` was unlinked from the issue/PR**" +UPDATED_FEATURE_TEXT = "**Flagsmith Feature `%s` has been updated:**\n" +FEATURE_UPDATED_FROM_GHA_TEXT = ( + "**Flagsmith Feature `%s` has been updated from GHA:**\n" +) +DELETED_FEATURE_TEXT = "**The Feature Flag `%s` was deleted**" DELETED_SEGMENT_OVERRIDE_TEXT = ( - "### The Segment Override `%s` for Feature Flag `%s` was deleted" + "**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 + +GITHUB_TAG_COLOR = "#838992" +GITHUB_FLAGSMITH_LABEL = "Flagsmith Flag" +GITHUB_FLAGSMITH_LABEL_DESCRIPTION = ( + "This GitHub Issue/PR is linked to a Flagsmith Feature Flag" +) +GITHUB_FLAGSMITH_LABEL_COLOR = "6633FF" + + +class GitHubEventType(Enum): + FLAG_UPDATED = "FLAG_UPDATED" + FLAG_DELETED = "FLAG_DELETED" + FLAG_UPDATED_FROM_GHA = "FLAG_UPDATED_FROM_GHA" + FEATURE_EXTERNAL_RESOURCE_ADDED = "FEATURE_EXTERNAL_RESOURCE_ADDED" + FEATURE_EXTERNAL_RESOURCE_REMOVED = "FEATURE_EXTERNAL_RESOURCE_REMOVED" + SEGMENT_OVERRIDE_DELETED = "SEGMENT_OVERRIDE_DELETED" + + +class GitHubTag(Enum): + PR_OPEN = "PR Open" + PR_MERGED = "PR Merged" + PR_CLOSED = "PR Closed" + PR_DRAFT = "PR Draft" + ISSUE_OPEN = "Issue Open" + ISSUE_CLOSED = "Issue Closed" + + +github_tag_description = { + GitHubTag.PR_OPEN.value: "This feature has a linked PR open", + GitHubTag.PR_MERGED.value: "This feature has a linked PR merged", + GitHubTag.PR_CLOSED.value: "This feature has a linked PR closed", + GitHubTag.PR_DRAFT.value: "This feature has a linked PR draft", + GitHubTag.ISSUE_OPEN.value: "This feature has a linked issue open", + GitHubTag.ISSUE_CLOSED.value: "This feature has a linked issue closed", +} diff --git a/api/integrations/github/github.py b/api/integrations/github/github.py index ddbbdcc04698..6b3573903dde 100644 --- a/api/integrations/github/github.py +++ b/api/integrations/github/github.py @@ -4,6 +4,7 @@ from typing import Any from core.helpers import get_current_site_url +from django.db.models import Q from django.utils.formats import get_format from features.models import Feature, FeatureState, FeatureStateValue @@ -17,14 +18,69 @@ LINK_SEGMENT_TITLE, UNLINKED_FEATURE_TEXT, UPDATED_FEATURE_TEXT, + GitHubEventType, + GitHubTag, ) 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 +from projects.tags.models import Tag, TagType logger = logging.getLogger(__name__) +tag_by_event_type = { + "pull_request": { + "closed": GitHubTag.PR_CLOSED.value, + "converted_to_draft": GitHubTag.PR_DRAFT.value, + "opened": GitHubTag.PR_OPEN.value, + "reopened": GitHubTag.PR_OPEN.value, + "ready_for_review": GitHubTag.PR_OPEN.value, + "merged": GitHubTag.PR_MERGED.value, + }, + "issues": { + "closed": GitHubTag.ISSUE_CLOSED.value, + "opened": GitHubTag.ISSUE_OPEN.value, + "reopened": GitHubTag.ISSUE_OPEN.value, + }, +} + + +def tag_feature_per_github_event( + event_type: str, action: str, metadata: dict[str, Any] +) -> None: + + # Get Feature with external resource of type GITHUB and url matching the resource URL + feature = Feature.objects.filter( + Q(external_resources__type="GITHUB_PR") + | Q(external_resources__type="GITHUB_ISSUE"), + external_resources__url=metadata.get("html_url"), + ).first() + + if feature: + if ( + event_type == "pull_request" + and action == "closed" + and metadata.get("merged") + ): + action = "merged" + # Get corresponding project Tag to tag the feature + github_tag = Tag.objects.get( + label=tag_by_event_type[event_type][action], + project=feature.project_id, + is_system_tag=True, + type=TagType.GITHUB.value, + ) + tag_label_pattern = "Issue" if event_type == "issues" else "PR" + # Remove all GITHUB tags from the feature which label starts with issue or pr depending on event_type + feature.tags.remove( + *feature.tags.filter( + Q(type=TagType.GITHUB.value) & Q(label__startswith=tag_label_pattern) + ) + ) + + feature.tags.add(github_tag) + feature.save() + def handle_installation_deleted(payload: dict[str, Any]) -> None: installation_id = payload.get("installation", {}).get("id") @@ -42,6 +98,10 @@ def handle_installation_deleted(payload: dict[str, Any]) -> None: def handle_github_webhook_event(event_type: str, payload: dict[str, Any]) -> None: if event_type == "installation" and payload.get("action") == "deleted": handle_installation_deleted(payload) + elif event_type in tag_by_event_type: + action = str(payload.get("action")) + metadata = payload.get("issue", {}) or payload.get("pull_request", {}) + tag_feature_per_github_event(event_type, action, metadata) def generate_body_comment( @@ -53,13 +113,12 @@ def generate_body_comment( segment_name: str | None = None, ) -> str: - is_update = event_type == WebhookEventType.FLAG_UPDATED.value - is_removed = event_type == WebhookEventType.FEATURE_EXTERNAL_RESOURCE_REMOVED.value + is_removed = event_type == GitHubEventType.FEATURE_EXTERNAL_RESOURCE_REMOVED.value is_segment_override_deleted = ( - event_type == WebhookEventType.SEGMENT_OVERRIDE_DELETED.value + event_type == GitHubEventType.SEGMENT_OVERRIDE_DELETED.value ) - if event_type == WebhookEventType.FLAG_DELETED.value: + if event_type == GitHubEventType.FLAG_DELETED.value: return DELETED_FEATURE_TEXT % (name) if is_removed: @@ -68,7 +127,12 @@ def generate_body_comment( 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) + result = "" + if event_type == GitHubEventType.FLAG_UPDATED.value: + result = UPDATED_FEATURE_TEXT % (name) + else: + result = LINK_FEATURE_TITLE % (name) + last_segment_name = "" if len(feature_states) > 0 and not feature_states[0].get("segment_name"): result += FEATURE_TABLE_HEADER @@ -125,7 +189,7 @@ def generate_data( if check_not_none(feature_state_value): feature_env_data["feature_state_value"] = feature_state_value - if type is not WebhookEventType.FEATURE_EXTERNAL_RESOURCE_REMOVED.value: + if type is not GitHubEventType.FEATURE_EXTERNAL_RESOURCE_REMOVED.value: feature_env_data["environment_name"] = feature_state.environment.name feature_env_data["enabled"] = feature_state.enabled feature_env_data["last_updated"] = feature_state.updated_at.strftime( @@ -150,7 +214,7 @@ def generate_data( type=type, url=( url - if type == WebhookEventType.FEATURE_EXTERNAL_RESOURCE_REMOVED.value + if type == GitHubEventType.FEATURE_EXTERNAL_RESOURCE_REMOVED.value else None ), feature_states=feature_states_list if feature_states else None, diff --git a/api/integrations/github/migrations/0004_githubrepository_tagging_enabled.py b/api/integrations/github/migrations/0004_githubrepository_tagging_enabled.py new file mode 100644 index 000000000000..a3ded07330d2 --- /dev/null +++ b/api/integrations/github/migrations/0004_githubrepository_tagging_enabled.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.25 on 2024-08-07 17:47 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('github', '0003_auto_20240528_0640'), + ] + + operations = [ + migrations.AddField( + model_name='githubrepository', + name='tagging_enabled', + field=models.BooleanField(default=False), + ), + ] diff --git a/api/integrations/github/models.py b/api/integrations/github/models.py index 532e0760b9ff..546b009d601e 100644 --- a/api/integrations/github/models.py +++ b/api/integrations/github/models.py @@ -3,8 +3,14 @@ from core.models import SoftDeleteExportableModel from django.db import models -from django_lifecycle import BEFORE_DELETE, LifecycleModelMixin, hook +from django_lifecycle import ( + AFTER_CREATE, + BEFORE_DELETE, + LifecycleModelMixin, + hook, +) +from integrations.github.constants import GITHUB_TAG_COLOR from organisations.models import Organisation logger: logging.Logger = logging.getLogger(name=__name__) @@ -48,6 +54,7 @@ class GithubRepository(LifecycleModelMixin, SoftDeleteExportableModel): null=False, on_delete=models.CASCADE, ) + tagging_enabled = models.BooleanField(default=False) class Meta: constraints = [ @@ -84,3 +91,23 @@ def delete_feature_external_resources( # Filter by url containing the repository owner and name url__regex=pattern, ).delete() + + @hook(AFTER_CREATE) + def create_github_tags( + self, + ) -> None: + from integrations.github.constants import ( + GitHubTag, + github_tag_description, + ) + from projects.tags.models import Tag, TagType + + for tag_label in GitHubTag: + tag, created = Tag.objects.get_or_create( + color=GITHUB_TAG_COLOR, + description=github_tag_description[tag_label.value], + label=tag_label.value, + project=self.project, + is_system_tag=True, + type=TagType.GITHUB.value, + ) diff --git a/api/integrations/github/serializers.py b/api/integrations/github/serializers.py index b3dc96c5263e..9d1cf3a81635 100644 --- a/api/integrations/github/serializers.py +++ b/api/integrations/github/serializers.py @@ -27,6 +27,7 @@ class Meta: "project", "repository_owner", "repository_name", + "tagging_enabled", ) read_only_fields = ( "id", diff --git a/api/integrations/github/tasks.py b/api/integrations/github/tasks.py index 8e94c9007b5d..970067daf0c4 100644 --- a/api/integrations/github/tasks.py +++ b/api/integrations/github/tasks.py @@ -6,8 +6,8 @@ from features.models import Feature from integrations.github.client import post_comment_to_github +from integrations.github.constants import GitHubEventType from integrations.github.dataclasses import CallGithubData -from webhooks.webhooks import WebhookEventType logger = logging.getLogger(__name__) @@ -29,8 +29,9 @@ def send_post_request(data: CallGithubData) -> None: ) if ( - event_type == WebhookEventType.FLAG_UPDATED.value - or event_type == WebhookEventType.FLAG_DELETED.value + event_type == GitHubEventType.FLAG_UPDATED.value + or event_type == GitHubEventType.FLAG_DELETED.value + or event_type == GitHubEventType.FLAG_UPDATED_FROM_GHA.value ): for resource in data.feature_external_resources: url = resource.get("url") @@ -40,7 +41,7 @@ def send_post_request(data: CallGithubData) -> None: installation_id, split_url[1], split_url[2], split_url[4], body ) - elif event_type == WebhookEventType.FEATURE_EXTERNAL_RESOURCE_REMOVED.value: + elif event_type == GitHubEventType.FEATURE_EXTERNAL_RESOURCE_REMOVED.value: url = data.github_data.url pathname = urlparse(url).path split_url = pathname.split("/") @@ -80,8 +81,8 @@ def generate_feature_external_resources( ] if ( - github_event_data.type == WebhookEventType.FLAG_DELETED.value - or github_event_data.type == WebhookEventType.SEGMENT_OVERRIDE_DELETED.value + github_event_data.type == GitHubEventType.FLAG_DELETED.value + or github_event_data.type == GitHubEventType.SEGMENT_OVERRIDE_DELETED.value ): feature_external_resources = generate_feature_external_resources( list( @@ -100,7 +101,7 @@ def generate_feature_external_resources( if ( github_event_data.type - == WebhookEventType.FEATURE_EXTERNAL_RESOURCE_REMOVED.value + == GitHubEventType.FEATURE_EXTERNAL_RESOURCE_REMOVED.value ): data = CallGithubData( event_type=github_event_data.type, diff --git a/api/integrations/github/views.py b/api/integrations/github/views.py index 4cc2f3efea57..8d48a17dfa84 100644 --- a/api/integrations/github/views.py +++ b/api/integrations/github/views.py @@ -15,13 +15,17 @@ from integrations.github.client import ( ResourceType, + create_flagsmith_flag_label, delete_github_installation, fetch_github_repo_contributors, fetch_github_repositories, fetch_search_github_resource, ) from integrations.github.exceptions import DuplicateGitHubIntegration -from integrations.github.github import handle_github_webhook_event +from integrations.github.github import ( + handle_github_webhook_event, + tag_by_event_type, +) from integrations.github.helpers import github_webhook_payload_is_valid from integrations.github.models import GithubConfiguration, GithubRepository from integrations.github.permissions import HasPermissionToGithubConfiguration @@ -136,10 +140,20 @@ def get_queryset(self): except ValueError: raise ValidationError({"github_pk": ["Must be an integer"]}) - def create(self, request, *args, **kwargs): + def create(self, request, *args, **kwargs) -> Response | None: try: - return super().create(request, *args, **kwargs) + response: Response = super().create(request, *args, **kwargs) + github_configuration: GithubConfiguration = GithubConfiguration.objects.get( + id=self.kwargs["github_pk"] + ) + if request.data.get("tagging_enabled", False): + create_flagsmith_flag_label( + installation_id=github_configuration.installation_id, + owner=request.data.get("repository_owner"), + repo=request.data.get("repository_name"), + ) + return response except IntegrityError as e: if re.search( @@ -150,6 +164,19 @@ def create(self, request, *args, **kwargs): detail="Duplication error. The GitHub repository already linked" ) + def update(self, request, *args, **kwargs) -> Response | None: + response: Response = super().update(request, *args, **kwargs) + github_configuration: GithubConfiguration = GithubConfiguration.objects.get( + id=self.kwargs["github_pk"] + ) + if request.data.get("tagging_enabled", False): + create_flagsmith_flag_label( + installation_id=github_configuration.installation_id, + owner=request.data.get("repository_owner"), + repo=request.data.get("repository_name"), + ) + return response + @api_view(["GET"]) @permission_classes([IsAuthenticated, HasPermissionToGithubConfiguration]) @@ -250,7 +277,7 @@ def github_webhook(request) -> Response: payload_body=payload, secret_token=secret, signature_header=signature ): data = json.loads(payload.decode("utf-8")) - if github_event == "installation": + if github_event == "installation" or github_event in tag_by_event_type: handle_github_webhook_event(event_type=github_event, payload=data) return Response({"detail": "Event processed"}, status=200) else: diff --git a/api/projects/tags/migrations/0006_alter_tag_type.py b/api/projects/tags/migrations/0006_alter_tag_type.py new file mode 100644 index 000000000000..84736c31bd3f --- /dev/null +++ b/api/projects/tags/migrations/0006_alter_tag_type.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.25 on 2024-05-27 15:03 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('tags', '0005_add_tag_fields_for_stale_flags_logic'), + ] + + operations = [ + migrations.AlterField( + model_name='tag', + name='type', + field=models.CharField(choices=[('NONE', 'None'), ('STALE', 'Stale'), ('GITHUB', 'Github')], default='NONE', help_text='Field used to provide a consistent identifier for the FE and API to use for business logic.', max_length=100), + ), + ] diff --git a/api/projects/tags/models.py b/api/projects/tags/models.py index 597bebdd0d9a..35c75eecfb7e 100644 --- a/api/projects/tags/models.py +++ b/api/projects/tags/models.py @@ -7,6 +7,7 @@ class TagType(models.Choices): NONE = "NONE" STALE = "STALE" + GITHUB = "GITHUB" class Tag(AbstractBaseExportableModel): 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 901454b6add6..d69b05766c0e 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 @@ -75,17 +75,15 @@ def test_create_feature_external_resource( github_configuration: GithubConfiguration, github_repository: GithubRepository, post_request_mock: MagicMock, - mocker: MockerFixture, + mock_github_client_generate_token: MagicMock, ) -> None: # Given - mocker.patch( - "integrations.github.client.generate_token", - return_value="mocked_token", + repository_owner_name = ( + f"{github_repository.repository_owner}/{github_repository.repository_name}" ) - feature_external_resource_data = { "type": "GITHUB_ISSUE", - "url": "https://github.com/repoowner/repo-name/issues/35", + "url": f"https://github.com/{repository_owner_name}/issues/35", "feature": feature_with_value.id, "metadata": {"state": "open"}, } @@ -130,7 +128,7 @@ def test_create_feature_external_resource( ) ) post_request_mock.assert_called_with( - "https://api.github.com/repos/repoowner/repo-name/issues/35/comments", + f"https://api.github.com/repos/{repository_owner_name}/issues/35/comments", json={"body": f"{expected_comment_body}"}, headers={ "Accept": "application/vnd.github.v3+json", @@ -157,7 +155,7 @@ def test_create_feature_external_resource( # And When responses.add( method="GET", - url=f"{GITHUB_API_URL}repos/repoowner/repo-name/issues/35", + url=f"{GITHUB_API_URL}repos/{repository_owner_name}/issues/35", status=200, json={"title": "resource name", "state": "open"}, ) @@ -183,6 +181,66 @@ def test_create_feature_external_resource( ) +def test_cannot_create_feature_external_resource_with_an_invalid_gh_url( + admin_client_new: APIClient, + feature: Feature, + project: Project, + github_configuration: GithubConfiguration, + github_repository: GithubRepository, +) -> None: + # Given + feature_external_resource_data = { + "type": "GITHUB_ISSUE", + "url": "https://github.com/repoowner/repo-name/pull/1", + "feature": feature.id, + "metadata": {"state": "open"}, + } + + url = reverse( + "api-v1:projects:feature-external-resources-list", + kwargs={"project_pk": project.id, "feature_pk": feature.id}, + ) + + # When + response = admin_client_new.post( + url, data=feature_external_resource_data, format="json" + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["detail"] == "Invalid GitHub Issue/PR URL" + + +def test_cannot_create_feature_external_resource_with_an_incorrect_gh_type( + admin_client_new: APIClient, + feature: Feature, + project: Project, + github_configuration: GithubConfiguration, + github_repository: GithubRepository, +) -> None: + # Given + feature_external_resource_data = { + "type": "GITHUB_INCORRECT_TYPE", + "url": "https://github.com/repoowner/repo-name/pull/1", + "feature": feature.id, + "metadata": {"state": "open"}, + } + + url = reverse( + "api-v1:projects:feature-external-resources-list", + kwargs={"project_pk": project.id, "feature_pk": feature.id}, + ) + + # When + response = admin_client_new.post( + url, data=feature_external_resource_data, format="json" + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["detail"] == "Incorrect GitHub type" + + def test_cannot_create_feature_external_resource_when_doesnt_have_a_valid_github_integration( admin_client_new: APIClient, feature: Feature, @@ -297,11 +355,11 @@ def test_update_feature_external_resource( "integrations.github.client.generate_token", ) mock_generate_token.return_value = "mocked_token" - mock_generate_token.return_value = "mocked_token" feature_external_resource_data = { "type": "GITHUB_ISSUE", "url": "https://github.com/userexample/example-project-repo/issues/12", "feature": feature.id, + "metadata": '{"state": "open"}', } url = reverse( "api-v1:projects:feature-external-resources-detail", @@ -338,7 +396,7 @@ def test_delete_feature_external_resource( post_request_mock.assert_called_with( "https://api.github.com/repos/repositoryownertest/repositorynametest/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", @@ -361,12 +419,9 @@ def test_get_feature_external_resources( github_configuration: GithubConfiguration, github_repository: GithubRepository, feature_external_resource: FeatureExternalResource, - mocker: MockerFixture, + mock_github_client_generate_token: MagicMock, ) -> 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}, @@ -446,7 +501,7 @@ def test_create_github_comment_on_feature_state_updated( ).updated_at.strftime(get_format("DATETIME_INPUT_FORMATS")[0]) expected_body_comment = ( - "Flagsmith Feature `Test Feature1` has been updated:\n" + "**Flagsmith Feature `Test Feature1` has been updated:**\n" + expected_default_body( project.id, environment.api_key, @@ -480,14 +535,9 @@ def test_create_github_comment_on_feature_was_deleted( github_repository: GithubRepository, feature_external_resource: FeatureExternalResource, post_request_mock: MagicMock, - mocker: MockerFixture, + mock_github_client_generate_token: MagicMock, ) -> None: # Given - mocker.patch( - "integrations.github.client.generate_token", - return_value="mocked_token", - ) - url = reverse( viewname="api-v1:projects:project-features-detail", kwargs={"project_pk": project.id, "pk": feature.id}, @@ -501,7 +551,7 @@ def test_create_github_comment_on_feature_was_deleted( post_request_mock.assert_called_with( "https://api.github.com/repos/repositoryownertest/repositorynametest/issues/11/comments", - json={"body": "### The Feature Flag `Test Feature1` 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, @@ -518,18 +568,13 @@ def test_create_github_comment_on_segment_override_updated( github_configuration: GithubConfiguration, github_repository: GithubRepository, post_request_mock: MagicMock, - mocker: MockerFixture, environment: Environment, admin_client: APIClient, feature_with_value_external_resource: FeatureExternalResource, + mock_github_client_generate_token: MagicMock, ) -> None: # Given feature_state = segment_override_for_feature_with_value - mocker.patch( - "integrations.github.client.generate_token", - return_value="mocked_token", - ) - payload = dict(WritableNestedFeatureStateSerializer(instance=feature_state).data) payload["enabled"] = not feature_state.enabled @@ -549,7 +594,7 @@ def test_create_github_comment_on_segment_override_updated( ).updated_at.strftime(get_format("DATETIME_INPUT_FORMATS")[0]) expected_comment_body = ( - "Flagsmith Feature `feature_with_value` has been updated:\n" + "**Flagsmith Feature `feature_with_value` has been updated:**\n" + "\n" + expected_segment_comment_body( project.id, @@ -581,16 +626,11 @@ def test_create_github_comment_on_segment_override_deleted( github_configuration: GithubConfiguration, github_repository: GithubRepository, post_request_mock: MagicMock, - mocker: MockerFixture, admin_client_new: APIClient, feature_with_value_external_resource: FeatureExternalResource, + mock_github_client_generate_token: MagicMock, ) -> None: # Given - mocker.patch( - "integrations.github.client.generate_token", - return_value="mocked_token", - ) - url = reverse( viewname="api-v1:features:feature-segment-detail", kwargs={"pk": feature_with_value_segment.id}, @@ -606,7 +646,7 @@ def test_create_github_comment_on_segment_override_deleted( post_request_mock.assert_called_with( "https://api.github.com/repos/repositoryownertest/repositorynametest/issues/11/comments", json={ - "body": "### The Segment Override `segment` for Feature Flag `feature_with_value` was deleted" + "body": "**The Segment Override `segment` for Feature Flag `feature_with_value` was deleted**" }, headers={ "Accept": "application/vnd.github.v3+json", @@ -664,7 +704,7 @@ def test_create_github_comment_using_v2( response_data["updated_at"], format ).strftime(get_format("DATETIME_INPUT_FORMATS")[0]) expected_comment_body = ( - "Flagsmith Feature `Test Feature1` has been updated:\n" + "**Flagsmith Feature `Test Feature1` has been updated:**\n" + "\n" + expected_segment_comment_body( project.id, @@ -745,19 +785,17 @@ def test_create_feature_external_resource_on_environment_with_v2( segment_override_for_feature_with_value: FeatureState, environment_v2_versioning: Environment, post_request_mock: MagicMock, - mocker: MockerFixture, + mock_github_client_generate_token: MagicMock, ) -> None: # Given feature_id = segment_override_for_feature_with_value.feature_id - - mocker.patch( - "integrations.github.client.generate_token", - return_value="mocked_token", + repository_owner_name = ( + f"{github_repository.repository_owner}/{github_repository.repository_name}" ) feature_external_resource_data = { "type": "GITHUB_ISSUE", - "url": "https://github.com/repoowner/repo-name/issues/35", + "url": f"https://github.com/{repository_owner_name}/issues/35", "feature": feature_id, "metadata": {"state": "open"}, } @@ -804,7 +842,7 @@ def test_create_feature_external_resource_on_environment_with_v2( assert response.status_code == status.HTTP_201_CREATED post_request_mock.assert_called_with( - "https://api.github.com/repos/repoowner/repo-name/issues/35/comments", + f"https://api.github.com/repos/{repository_owner_name}/issues/35/comments", json={"body": f"{expected_comment_body}"}, headers={ "Accept": "application/vnd.github.v3+json", @@ -813,3 +851,37 @@ def test_create_feature_external_resource_on_environment_with_v2( }, timeout=10, ) + + +def test_cannot_create_feature_external_resource_for_the_same_feature_and_resource_uri( + admin_client_new: APIClient, + feature: Feature, + project: Project, + github_configuration: GithubConfiguration, + github_repository: GithubRepository, + feature_external_resource_gh_pr: FeatureExternalResource, +) -> None: + # Given + feature_external_resource_data = { + "type": "GITHUB_PR", + "url": "https://github.com/repositoryownertest/repositorynametest/pull/1", + "feature": feature.id, + "metadata": {"state": "open"}, + } + + url = reverse( + "api-v1:projects:feature-external-resources-list", + kwargs={"project_pk": project.id, "feature_pk": feature.id}, + ) + + # When + response = admin_client_new.post( + url, data=feature_external_resource_data, format="json" + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert ( + response.json()["non_field_errors"][0] + == "The fields feature, url must make a unique set." + ) 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 07d3e9660f14..a1d9add23f36 100644 --- a/api/tests/unit/integrations/github/test_unit_github_views.py +++ b/api/tests/unit/integrations/github/test_unit_github_views.py @@ -1,10 +1,10 @@ import json from typing import Any +from unittest.mock import MagicMock import pytest import requests import responses -from django.conf import settings from django.urls import reverse from pytest_lazyfixture import lazy_fixture from pytest_mock import MockerFixture @@ -12,7 +12,9 @@ from rest_framework.response import Response from rest_framework.test import APIClient +from environments.models import Environment from features.feature_external_resources.models import FeatureExternalResource +from features.models import Feature from integrations.github.constants import GITHUB_API_URL from integrations.github.models import GithubConfiguration, GithubRepository from integrations.github.views import ( @@ -29,6 +31,17 @@ WEBHOOK_PAYLOAD_WITHOUT_INSTALLATION_ID = json.dumps( {"installation": {"test": 765432}, "action": "deleted"} ) +WEBHOOK_PAYLOAD_MERGED = json.dumps( + { + "pull_request": { + "id": 1234567, + "html_url": "https://github.com/repositoryownertest/repositorynametest/issues/11", + "merged": True, + }, + "action": "closed", + } +) + WEBHOOK_SIGNATURE = "sha1=57a1426e19cdab55dd6d0c191743e2958e50ccaa" WEBHOOK_SIGNATURE_WITH_AN_INVALID_INSTALLATION_ID = ( "sha1=081eef49d04df27552587d5df1c6b76e0fe20d21" @@ -36,6 +49,7 @@ WEBHOOK_SIGNATURE_WITHOUT_INSTALLATION_ID = ( "sha1=f99796bd3cebb902864e87ed960c5cca8772ff67" ) +WEBHOOK_MERGED_ACTION_SIGNATURE = "sha1=712ec7a5db14aad99d900da40738ebb9508ecad2" WEBHOOK_SECRET = "secret-key" @@ -246,11 +260,14 @@ def test_cannot_get_github_repository_when_github_pk_in_not_a_number( assert response.json() == {"github_pk": ["Must be an integer"]} +@responses.activate def test_create_github_repository( admin_client_new: APIClient, organisation: Organisation, github_configuration: GithubConfiguration, project: Project, + mocker: MockerFixture, + mock_github_client_generate_token: MagicMock, ) -> None: # Given data = { @@ -258,8 +275,16 @@ def test_create_github_repository( "repository_owner": "repositoryowner", "repository_name": "repositoryname", "project": project.id, + "tagging_enabled": True, } + responses.add( + method="POST", + url=f"{GITHUB_API_URL}repos/repositoryowner/repositoryname/labels", + status=status.HTTP_200_OK, + json={}, + ) + url = reverse( "api-v1:organisations:repositories-list", args=[organisation.id, github_configuration.id], @@ -272,6 +297,53 @@ def test_create_github_repository( assert GithubRepository.objects.filter(repository_owner="repositoryowner").exists() +@responses.activate +def test_create_github_repository_and_label_already_Existe( + admin_client_new: APIClient, + organisation: Organisation, + github_configuration: GithubConfiguration, + project: Project, + mocker: MockerFixture, + mock_github_client_generate_token: MagicMock, +) -> None: + # Given + mocker_logger = mocker.patch("integrations.github.client.logger") + + data = { + "github_configuration": github_configuration.id, + "repository_owner": "repositoryowner", + "repository_name": "repositoryname", + "project": project.id, + "tagging_enabled": True, + } + + mock_response = { + "message": "Validation Failed", + "errors": [{"resource": "Label", "code": "already_exists", "field": "name"}], + "documentation_url": "https://docs.github.com/rest/issues/labels#create-a-label", + "status": "422", + } + + responses.add( + method="POST", + url=f"{GITHUB_API_URL}repos/repositoryowner/repositoryname/labels", + status=status.HTTP_422_UNPROCESSABLE_ENTITY, + json=mock_response, + ) + + url = reverse( + "api-v1:organisations:repositories-list", + args=[organisation.id, github_configuration.id], + ) + # When + response = admin_client_new.post(url, data) + + # Then + mocker_logger.warning.assert_called_once_with("Label already exists") + assert response.status_code == status.HTTP_201_CREATED + assert GithubRepository.objects.filter(repository_owner="repositoryowner").exists() + + def test_cannot_create_github_repository_when_does_not_have_permissions( test_user_client: APIClient, organisation: Organisation, @@ -334,13 +406,9 @@ def test_github_delete_repository( github_configuration: GithubConfiguration, github_repository: GithubRepository, feature_external_resource: FeatureExternalResource, - mocker: MockerFixture, + mock_github_client_generate_token: MagicMock, ) -> None: # Given - mock_generate_token = mocker.patch( - "integrations.github.client.generate_token", - ) - mock_generate_token.return_value = "mocked_token" url = reverse( "api-v1:organisations:repositories-detail", args=[organisation.id, github_configuration.id, github_repository.id], @@ -409,14 +477,10 @@ def test_fetch_pull_requests( organisation: Organisation, github_configuration: GithubConfiguration, github_repository: GithubRepository, + mock_github_client_generate_token: MagicMock, 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.get", side_effect=mocked_requests_get_issues_and_pull_requests ) @@ -448,13 +512,10 @@ def test_fetch_issues( organisation: Organisation, github_configuration: GithubConfiguration, github_repository: GithubRepository, + mock_github_client_generate_token: MagicMock, 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.get", side_effect=mocked_requests_get_issues_and_pull_requests ) @@ -491,13 +552,10 @@ def test_fetch_issues_returns_error_on_bad_response_from_github( organisation: Organisation, github_configuration: GithubConfiguration, github_repository: GithubRepository, + mock_github_client_generate_token: MagicMock, 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"} @@ -519,13 +577,9 @@ def test_fetch_repositories( organisation: Organisation, github_configuration: GithubConfiguration, github_repository: GithubRepository, - mocker: MockerFixture, + mock_github_client_generate_token: MagicMock, ) -> None: # Given - 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}installation/repositories", @@ -573,13 +627,11 @@ def test_fetch_repositories( ], ) def test_fetch_issues_and_pull_requests_fails_with_status_400_when_integration_not_configured( - client: APIClient, organisation: Organisation, reverse_url: str, mocker + client: APIClient, + organisation: Organisation, + reverse_url: str, + mock_github_client_generate_token: MagicMock, ) -> None: - # Given - mock_generate_token = mocker.patch( - "integrations.github.client.generate_token", - ) - mock_generate_token.generate_token.return_value = "mocked_token" # When url = reverse(reverse_url, args=[organisation.id]) response = client.get(url) @@ -600,15 +652,9 @@ def test_cannot_fetch_issues_or_prs_when_does_not_have_permissions( organisation: Organisation, github_configuration: GithubConfiguration, github_repository: GithubRepository, - mocker, + mock_github_client_generate_token: MagicMock, reverse_url: str, ) -> None: - # Given - mock_generate_token = mocker.patch( - "integrations.github.client.generate_token", - ) - mock_generate_token.generate_token.return_value = "mocked_token" - # When url = reverse(reverse_url, args=[organisation.id]) response = test_user_client.get(url) @@ -656,9 +702,9 @@ def test_verify_github_webhook_payload_returns_false_on_no_signature_header() -> def test_github_webhook_delete_installation( api_client: APIClient, github_configuration: GithubConfiguration, + set_github_webhook_secret, ) -> None: # Given - settings.GITHUB_WEBHOOK_SECRET = WEBHOOK_SECRET url = reverse("api-v1:github-webhook") # When @@ -675,63 +721,88 @@ def test_github_webhook_delete_installation( assert not GithubConfiguration.objects.filter(installation_id=1234567).exists() -def test_github_webhook_with_non_existing_installation( +def test_github_webhook_merged_a_pull_request( api_client: APIClient, + feature: Feature, github_configuration: GithubConfiguration, + github_repository: GithubRepository, + feature_external_resource: FeatureExternalResource, + set_github_webhook_secret, +) -> None: + # Given + url = reverse("api-v1:github-webhook") + + # When + response = api_client.post( + path=url, + data=WEBHOOK_PAYLOAD_MERGED, + content_type="application/json", + HTTP_X_HUB_SIGNATURE=WEBHOOK_MERGED_ACTION_SIGNATURE, + HTTP_X_GITHUB_EVENT="pull_request", + ) + + # Then + feature.refresh_from_db() + assert response.status_code == status.HTTP_200_OK + assert feature.tags.first().label == "PR Merged" + + +def test_github_webhook_without_installation_id( + api_client: APIClient, mocker: MockerFixture, + set_github_webhook_secret, ) -> None: # Given - settings.GITHUB_WEBHOOK_SECRET = WEBHOOK_SECRET url = reverse("api-v1:github-webhook") mocker_logger = mocker.patch("integrations.github.github.logger") # When response = api_client.post( path=url, - data=WEBHOOK_PAYLOAD_WITH_AN_INVALID_INSTALLATION_ID, + data=WEBHOOK_PAYLOAD_WITHOUT_INSTALLATION_ID, content_type="application/json", - HTTP_X_HUB_SIGNATURE=WEBHOOK_SIGNATURE_WITH_AN_INVALID_INSTALLATION_ID, + HTTP_X_HUB_SIGNATURE=WEBHOOK_SIGNATURE_WITHOUT_INSTALLATION_ID, HTTP_X_GITHUB_EVENT="installation", ) # Then mocker_logger.error.assert_called_once_with( - "GitHub Configuration with installation_id 765432 does not exist" + "The installation_id is not present in the payload: {'installation': {'test': 765432}, 'action': 'deleted'}" ) assert response.status_code == status.HTTP_200_OK -def test_github_webhook_without_installation_id( +def test_github_webhook_with_non_existing_installation( api_client: APIClient, github_configuration: GithubConfiguration, mocker: MockerFixture, + set_github_webhook_secret, ) -> None: # Given - settings.GITHUB_WEBHOOK_SECRET = WEBHOOK_SECRET url = reverse("api-v1:github-webhook") mocker_logger = mocker.patch("integrations.github.github.logger") # When response = api_client.post( path=url, - data=WEBHOOK_PAYLOAD_WITHOUT_INSTALLATION_ID, + data=WEBHOOK_PAYLOAD_WITH_AN_INVALID_INSTALLATION_ID, content_type="application/json", - HTTP_X_HUB_SIGNATURE=WEBHOOK_SIGNATURE_WITHOUT_INSTALLATION_ID, + HTTP_X_HUB_SIGNATURE=WEBHOOK_SIGNATURE_WITH_AN_INVALID_INSTALLATION_ID, HTTP_X_GITHUB_EVENT="installation", ) # Then mocker_logger.error.assert_called_once_with( - "The installation_id is not present in the payload: {'installation': {'test': 765432}, 'action': 'deleted'}" + "GitHub Configuration with installation_id 765432 does not exist" ) assert response.status_code == status.HTTP_200_OK def test_github_webhook_fails_on_signature_header_missing( github_configuration: GithubConfiguration, + set_github_webhook_secret, ) -> None: # Given - settings.GITHUB_WEBHOOK_SECRET = WEBHOOK_SECRET url = reverse("api-v1:github-webhook") # When @@ -751,9 +822,9 @@ def test_github_webhook_fails_on_signature_header_missing( def test_github_webhook_fails_on_bad_signature_header_missing( github_configuration: GithubConfiguration, + set_github_webhook_secret, ) -> None: # Given - settings.GITHUB_WEBHOOK_SECRET = WEBHOOK_SECRET url = reverse("api-v1:github-webhook") # When @@ -774,9 +845,9 @@ def test_github_webhook_fails_on_bad_signature_header_missing( def test_github_webhook_bypass_event( github_configuration: GithubConfiguration, + set_github_webhook_secret, ) -> None: # Given - settings.GITHUB_WEBHOOK_SECRET = WEBHOOK_SECRET url = reverse("api-v1:github-webhook") # When @@ -800,15 +871,10 @@ def test_cannot_fetch_pull_requests_when_github_request_call_failed( organisation: Organisation, github_configuration: GithubConfiguration, github_repository: GithubRepository, - mocker, + mock_github_client_generate_token: MagicMock, ) -> 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", @@ -833,14 +899,10 @@ def test_cannot_fetch_pulls_when_the_github_response_was_invalid( organisation: Organisation, github_configuration: GithubConfiguration, github_repository: GithubRepository, - mocker, + mock_github_client_generate_token: MagicMock, ) -> 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", @@ -877,7 +939,7 @@ def test_fetch_github_repo_contributors( organisation: Organisation, github_configuration: GithubConfiguration, github_repository: GithubRepository, - mocker: MockerFixture, + mock_github_client_generate_token: MagicMock, ) -> None: # Given url = reverse( @@ -905,11 +967,6 @@ def test_fetch_github_repo_contributors( expected_response = {"results": mocked_github_response} - mock_generate_token = mocker.patch( - "integrations.github.client.generate_token", - ) - mock_generate_token.return_value = "mocked_token" - # Add response for endpoint being tested responses.add( method=responses.GET, @@ -1063,3 +1120,85 @@ def test_send_the_invalid_type_page_or_page_size_param_returns_400( assert response.status_code == status.HTTP_400_BAD_REQUEST response_json = response.json() assert response_json == error_response + + +@responses.activate +def test_label_and_tags_no_added_when_tagging_is_disabled( + admin_client_new: APIClient, + project: Project, + environment: Environment, + github_repository: GithubRepository, + feature_with_value: Feature, + mock_github_client_generate_token: MagicMock, + post_request_mock: MagicMock, +) -> None: + # Given + github_repository.tagging_enabled = False + github_repository.save() + repository_owner_name = ( + f"{github_repository.repository_owner}/{github_repository.repository_name}" + ) + + feature_external_resource_data = { + "type": "GITHUB_ISSUE", + "url": f"https://github.com/{repository_owner_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}, + ) + + # When + response = admin_client_new.post( + url, data=feature_external_resource_data, format="json" + ) + + # Then + assert response.status_code == status.HTTP_201_CREATED + assert feature_with_value.tags.count() == 0 + + +@responses.activate +def test_update_github_repository( + admin_client_new: APIClient, + organisation: Organisation, + github_configuration: GithubConfiguration, + github_repository: GithubRepository, + project: Project, + mocker: MockerFixture, + mock_github_client_generate_token: MagicMock, +) -> None: + # Given + github_repository.tagging_enabled = False + github_repository.save() + data = { + "github_configuration": github_configuration.id, + "repository_owner": "repositoryowner", + "repository_name": "repositoryname", + "project": project.id, + "tagging_enabled": True, + } + + responses.add( + method="POST", + url=f"{GITHUB_API_URL}repos/repositoryowner/repositoryname/labels", + status=status.HTTP_200_OK, + json={}, + ) + + url = reverse( + "api-v1:organisations:repositories-detail", + args=[organisation.id, github_configuration.id, github_repository.id], + ) + # When + response = admin_client_new.put(url, data) + + # Then + assert response.status_code == status.HTTP_200_OK + assert GithubRepository.objects.filter(repository_owner="repositoryowner").exists() + assert GithubRepository.objects.get( + repository_owner="repositoryowner" + ).tagging_enabled diff --git a/api/webhooks/webhooks.py b/api/webhooks/webhooks.py index 75684e886614..e314b5976f9f 100644 --- a/api/webhooks/webhooks.py +++ b/api/webhooks/webhooks.py @@ -39,9 +39,6 @@ class WebhookEventType(enum.Enum): FLAG_DELETED = "FLAG_DELETED" AUDIT_LOG_CREATED = "AUDIT_LOG_CREATED" 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):