From 7da0d51525ab91a7974e0e86c41688aa22d24c24 Mon Sep 17 00:00:00 2001 From: Novak Zaballa Date: Wed, 4 Sep 2024 10:43:17 -0400 Subject: [PATCH 1/6] fix: GitHub integration tagging issues --- api/integrations/github/github.py | 30 ++++++++++++++----- api/integrations/github/models.py | 2 ++ .../github/test_unit_github_views.py | 5 +++- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/api/integrations/github/github.py b/api/integrations/github/github.py index 6b3573903dde..697efec2b18d 100644 --- a/api/integrations/github/github.py +++ b/api/integrations/github/github.py @@ -14,15 +14,17 @@ FEATURE_ENVIRONMENT_URL, FEATURE_TABLE_HEADER, FEATURE_TABLE_ROW, + GITHUB_TAG_COLOR, LINK_FEATURE_TITLE, LINK_SEGMENT_TITLE, UNLINKED_FEATURE_TEXT, UPDATED_FEATURE_TEXT, GitHubEventType, GitHubTag, + github_tag_description, ) from integrations.github.dataclasses import GithubData -from integrations.github.models import GithubConfiguration +from integrations.github.models import GithubConfiguration, GitHubRepository from integrations.github.tasks import call_github_app_webhook_for_feature_state from projects.tags.models import Tag, TagType @@ -46,7 +48,7 @@ def tag_feature_per_github_event( - event_type: str, action: str, metadata: dict[str, Any] + event_type: str, action: str, metadata: dict[str, Any], repo_full_name: str ) -> None: # Get Feature with external resource of type GITHUB and url matching the resource URL @@ -55,18 +57,28 @@ def tag_feature_per_github_event( | Q(external_resources__type="GITHUB_ISSUE"), external_resources__url=metadata.get("html_url"), ).first() + repo: list[str] = repo_full_name.split("/") + tagging_enabled = ( + GitHubRepository.objects.filter( + project=feature.project, repository_owner=repo[0], repository_name=repo[1] + ) + .first() + .tagging_enabled + ) - if feature: + if feature and tagging_enabled: 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( + # Get or create the corresponding project Tag to tag the feature + github_tag, _ = Tag.objects.get_or_create( + color=GITHUB_TAG_COLOR, + description=github_tag_description[tag_by_event_type[event_type][action]], label=tag_by_event_type[event_type][action], - project=feature.project_id, + project=feature.project, is_system_tag=True, type=TagType.GITHUB.value, ) @@ -100,8 +112,10 @@ def handle_github_webhook_event(event_type: str, payload: dict[str, Any]) -> Non 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) + if action in tag_by_event_type[event_type]: + repo_full_name = payload.get("repository", {}).get("full_name") + metadata = payload.get("issue", {}) or payload.get("pull_request", {}) + tag_feature_per_github_event(event_type, action, metadata, repo_full_name) def generate_body_comment( diff --git a/api/integrations/github/models.py b/api/integrations/github/models.py index 9b621d4c9b3f..069eb1dab08e 100644 --- a/api/integrations/github/models.py +++ b/api/integrations/github/models.py @@ -5,6 +5,7 @@ from django.db import models from django_lifecycle import ( AFTER_CREATE, + AFTER_UPDATE, BEFORE_DELETE, LifecycleModelMixin, hook, @@ -93,6 +94,7 @@ def delete_feature_external_resources( ).delete() @hook(AFTER_CREATE) + @hook(AFTER_UPDATE, when="tagging_enabled", has_changed=True, was=False) def create_github_tags( self, ) -> None: 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 ede44b4b6ddf..83988835b555 100644 --- a/api/tests/unit/integrations/github/test_unit_github_views.py +++ b/api/tests/unit/integrations/github/test_unit_github_views.py @@ -38,6 +38,9 @@ "html_url": "https://github.com/repositoryownertest/repositorynametest/issues/11", "merged": True, }, + "repository": { + "full_name": "repositoryownertest/repositorynametest", + }, "action": "closed", } ) @@ -49,7 +52,7 @@ WEBHOOK_SIGNATURE_WITHOUT_INSTALLATION_ID = ( "sha1=f99796bd3cebb902864e87ed960c5cca8772ff67" ) -WEBHOOK_MERGED_ACTION_SIGNATURE = "sha1=712ec7a5db14aad99d900da40738ebb9508ecad2" +WEBHOOK_MERGED_ACTION_SIGNATURE = "sha1=f3f7e1e9b43448d570451317447d3b4f8f8142de" WEBHOOK_SECRET = "secret-key" From 8f865b53a36a1617352af377d850b8c41673b70d Mon Sep 17 00:00:00 2001 From: Novak Zaballa Date: Thu, 5 Sep 2024 10:04:52 -0400 Subject: [PATCH 2/6] fix: Address the comments --- api/integrations/github/github.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/api/integrations/github/github.py b/api/integrations/github/github.py index 697efec2b18d..3ebace25c358 100644 --- a/api/integrations/github/github.py +++ b/api/integrations/github/github.py @@ -57,14 +57,12 @@ def tag_feature_per_github_event( | Q(external_resources__type="GITHUB_ISSUE"), external_resources__url=metadata.get("html_url"), ).first() - repo: list[str] = repo_full_name.split("/") - tagging_enabled = ( - GitHubRepository.objects.filter( - project=feature.project, repository_owner=repo[0], repository_name=repo[1] - ) - .first() - .tagging_enabled - ) + repository_owner, repository_name = repo_full_name.split(sep="/", maxsplit=1) + tagging_enabled = GitHubRepository.objects.get( + project=feature.project, + repository_owner=repository_owner, + repository_name=repository_name, + ).tagging_enabled if feature and tagging_enabled: if ( @@ -113,7 +111,7 @@ def handle_github_webhook_event(event_type: str, payload: dict[str, Any]) -> Non elif event_type in tag_by_event_type: action = str(payload.get("action")) if action in tag_by_event_type[event_type]: - repo_full_name = payload.get("repository", {}).get("full_name") + repo_full_name = payload["repository"]["full_name"] metadata = payload.get("issue", {}) or payload.get("pull_request", {}) tag_feature_per_github_event(event_type, action, metadata, repo_full_name) From d8ab013143506afe531f7c174b6b651e9bfe2cc1 Mon Sep 17 00:00:00 2001 From: Zach Aysan Date: Tue, 1 Oct 2024 14:25:17 +0000 Subject: [PATCH 3/6] Add unit test for missing feature --- .../integrations/github/test_unit_github_github.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 api/tests/unit/integrations/github/test_unit_github_github.py diff --git a/api/tests/unit/integrations/github/test_unit_github_github.py b/api/tests/unit/integrations/github/test_unit_github_github.py new file mode 100644 index 000000000000..ca5d3bfb206f --- /dev/null +++ b/api/tests/unit/integrations/github/test_unit_github_github.py @@ -0,0 +1,14 @@ +from integrations.github.github import tag_feature_per_github_event + + +def test_tag_feature_per_github_event_with_empty_feature(db: None) -> None: + # Given / When + result = tag_feature_per_github_event( + event_type="test", + action="closed", + metadata={}, + repo_full_name="test/repo", + ) + + # Then + assert result is None From af184201f9afad62809e08dfab158503625eff50 Mon Sep 17 00:00:00 2001 From: Zach Aysan Date: Tue, 1 Oct 2024 14:25:43 +0000 Subject: [PATCH 4/6] Check for feature and return if not present --- api/integrations/github/github.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/api/integrations/github/github.py b/api/integrations/github/github.py index 3ebace25c358..5c96171d77d2 100644 --- a/api/integrations/github/github.py +++ b/api/integrations/github/github.py @@ -57,6 +57,12 @@ def tag_feature_per_github_event( | Q(external_resources__type="GITHUB_ISSUE"), external_resources__url=metadata.get("html_url"), ).first() + + # Check to see if any feature objects match and if not return + # to allow the webhook processing complete. + if not feature: + return + repository_owner, repository_name = repo_full_name.split(sep="/", maxsplit=1) tagging_enabled = GitHubRepository.objects.get( project=feature.project, @@ -64,7 +70,7 @@ def tag_feature_per_github_event( repository_name=repository_name, ).tagging_enabled - if feature and tagging_enabled: + if tagging_enabled: if ( event_type == "pull_request" and action == "closed" From 2aebeeeaaf109173e631d760b30edbf72bfd0259 Mon Sep 17 00:00:00 2001 From: Zach Aysan Date: Wed, 9 Oct 2024 14:05:28 +0000 Subject: [PATCH 5/6] Fix unnecessary check for event type --- api/integrations/github/github.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/api/integrations/github/github.py b/api/integrations/github/github.py index 5c96171d77d2..d8af67b81ca7 100644 --- a/api/integrations/github/github.py +++ b/api/integrations/github/github.py @@ -113,13 +113,13 @@ 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")) - if action in tag_by_event_type[event_type]: - repo_full_name = payload["repository"]["full_name"] - metadata = payload.get("issue", {}) or payload.get("pull_request", {}) - tag_feature_per_github_event(event_type, action, metadata, repo_full_name) + return handle_installation_deleted(payload) + + action = str(payload.get("action")) + if action in tag_by_event_type[event_type]: + repo_full_name = payload["repository"]["full_name"] + metadata = payload.get("issue", {}) or payload.get("pull_request", {}) + tag_feature_per_github_event(event_type, action, metadata, repo_full_name) def generate_body_comment( From f92e23e8bc8ade36f1cf683ceb4dcefa6188d88d Mon Sep 17 00:00:00 2001 From: Zach Aysan Date: Wed, 9 Oct 2024 14:18:48 +0000 Subject: [PATCH 6/6] Add dequeued and fix any typing --- api/integrations/github/constants.py | 2 ++ api/integrations/github/github.py | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/api/integrations/github/constants.py b/api/integrations/github/constants.py index 897b3a2c6725..0f2595bcd544 100644 --- a/api/integrations/github/constants.py +++ b/api/integrations/github/constants.py @@ -43,6 +43,7 @@ class GitHubTag(Enum): PR_MERGED = "PR Merged" PR_CLOSED = "PR Closed" PR_DRAFT = "PR Draft" + PR_DEQUEUED = "PR Dequeued" ISSUE_OPEN = "Issue Open" ISSUE_CLOSED = "Issue Closed" @@ -52,6 +53,7 @@ class GitHubTag(Enum): 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.PR_DEQUEUED.value: "This feature has a linked PR dequeued", 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 d8af67b81ca7..cd7899fe8db2 100644 --- a/api/integrations/github/github.py +++ b/api/integrations/github/github.py @@ -36,6 +36,7 @@ "converted_to_draft": GitHubTag.PR_DRAFT.value, "opened": GitHubTag.PR_OPEN.value, "reopened": GitHubTag.PR_OPEN.value, + "dequeued": GitHubTag.PR_DEQUEUED.value, "ready_for_review": GitHubTag.PR_OPEN.value, "merged": GitHubTag.PR_MERGED.value, }, @@ -184,7 +185,7 @@ def generate_body_comment( return result -def check_not_none(value: any) -> bool: +def check_not_none(value: Any) -> bool: return value is not None