Skip to content

Commit

Permalink
fix: GitHub repos unique constraint and delete (#4037)
Browse files Browse the repository at this point in the history
  • Loading branch information
novakzaballa authored May 28, 2024
1 parent 3b61f83 commit 7454e4a
Show file tree
Hide file tree
Showing 8 changed files with 226 additions and 122 deletions.
41 changes: 38 additions & 3 deletions api/conftest.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import os
import typing
from unittest.mock import MagicMock

import boto3
import pytest
Expand All @@ -11,6 +12,7 @@
from moto import mock_dynamodb
from mypy_boto3_dynamodb.service_resource import DynamoDBServiceResource, Table
from pytest_django.plugin import blocking_manager_key
from pytest_mock import MockerFixture
from rest_framework.authtoken.models import Token
from rest_framework.test import APIClient
from urllib3.connectionpool import HTTPConnectionPool
Expand Down Expand Up @@ -85,6 +87,25 @@ def pytest_sessionstart(session: pytest.Session) -> None:
fix_issue_3869()


@pytest.fixture()
def post_request_mock(mocker: MockerFixture) -> MagicMock:
def mocked_request(*args, **kwargs) -> None:
class MockResponse:
def __init__(self, json_data: str, status_code: int) -> None:
self.json_data = json_data
self.status_code = status_code

def raise_for_status(self) -> None:
pass

def json(self) -> str:
return self.json_data

return MockResponse(json_data={"data": "data"}, status_code=200)

return mocker.patch("requests.post", side_effect=mocked_request)


@pytest.hookimpl(trylast=True)
def pytest_configure(config: pytest.Config) -> None:
if (
Expand Down Expand Up @@ -966,9 +987,16 @@ def flagsmith_environments_v2_table(dynamodb: DynamoDBServiceResource) -> Table:


@pytest.fixture()
def feature_external_resource(feature: Feature) -> FeatureExternalResource:
def feature_external_resource(
feature: Feature, post_request_mock: MagicMock, mocker: MockerFixture
) -> FeatureExternalResource:
mocker.patch(
"integrations.github.client.generate_token",
return_value="mocked_token",
)

return FeatureExternalResource.objects.create(
url="https://github.com/userexample/example-project-repo/issues/11",
url="https://github.com/repositoryownertest/repositorynametest/issues/11",
type="GITHUB_ISSUE",
feature=feature,
metadata='{"status": "open"}',
Expand All @@ -978,9 +1006,16 @@ def feature_external_resource(feature: Feature) -> FeatureExternalResource:
@pytest.fixture()
def feature_with_value_external_resource(
feature_with_value: Feature,
post_request_mock: MagicMock,
mocker: MockerFixture,
) -> FeatureExternalResource:
mocker.patch(
"integrations.github.client.generate_token",
return_value="mocked_token",
)

return FeatureExternalResource.objects.create(
url="https://github.com/userexample/example-project-repo/issues/11",
url="https://github.com/repositoryownertest/repositorynametest/issues/11",
type="GITHUB_ISSUE",
feature=feature_with_value,
)
Expand Down
11 changes: 6 additions & 5 deletions api/features/feature_external_resources/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@
logger = logging.getLogger(__name__)


class FeatureExternalResource(LifecycleModelMixin, models.Model):
class ResourceType(models.TextChoices):
# GitHub external resource types
GITHUB_ISSUE = "GITHUB_ISSUE", "GitHub Issue"
GITHUB_PR = "GITHUB_PR", "GitHub PR"
class ResourceType(models.TextChoices):
# GitHub external resource types
GITHUB_ISSUE = "GITHUB_ISSUE", "GitHub Issue"
GITHUB_PR = "GITHUB_PR", "GitHub PR"


class FeatureExternalResource(LifecycleModelMixin, models.Model):
url = models.URLField()
type = models.CharField(max_length=20, choices=ResourceType.choices)

Expand Down
19 changes: 19 additions & 0 deletions api/integrations/github/github.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import logging
import typing
from dataclasses import asdict
from typing import Any

from core.helpers import get_current_site_url
from django.utils.formats import get_format
Expand All @@ -25,6 +26,24 @@
logger = logging.getLogger(__name__)


def handle_installation_deleted(payload: dict[str, Any]) -> None:
installation_id = payload.get("installation", {}).get("id")
if installation_id is not None:
try:
GithubConfiguration.objects.get(installation_id=installation_id).delete()
except GithubConfiguration.DoesNotExist:
logger.error(
f"GitHub Configuration with installation_id {installation_id} does not exist"
)
else:
logger.error(f"The installation_id is not present in the payload: {payload}")


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)


def generate_body_comment(
name: str,
event_type: str,
Expand Down
29 changes: 29 additions & 0 deletions api/integrations/github/migrations/0003_auto_20240528_0640.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Generated by Django 3.2.25 on 2024-05-28 06:40

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('github', '0002_auto_20240502_1949'),
]

operations = [
migrations.AlterModelOptions(
name='githubconfiguration',
options={'ordering': ('id',)},
),
migrations.AlterModelOptions(
name='githubrepository',
options={'ordering': ('id',)},
),
migrations.RemoveConstraint(
model_name='githubrepository',
name='unique_repository_data',
),
migrations.AddConstraint(
model_name='githubrepository',
constraint=models.UniqueConstraint(condition=models.Q(('deleted_at__isnull', True)), fields=('github_configuration', 'project', 'repository_owner', 'repository_name'), name='unique_repository_data'),
),
]
13 changes: 11 additions & 2 deletions api/integrations/github/models.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import logging
import re

from core.models import SoftDeleteExportableModel
from django.db import models
Expand Down Expand Up @@ -31,6 +32,7 @@ class Meta:
condition=models.Q(deleted_at__isnull=True),
)
]
ordering = ("id",)


class GithubRepository(LifecycleModelMixin, SoftDeleteExportableModel):
Expand All @@ -57,21 +59,28 @@ class Meta:
"repository_name",
],
name="unique_repository_data",
condition=models.Q(deleted_at__isnull=True),
)
]
ordering = ("id",)

@hook(BEFORE_DELETE)
def delete_feature_external_resources(
self,
) -> None:
from features.feature_external_resources.models import (
FeatureExternalResource,
ResourceType,
)

pattern = re.escape(f"/{self.repository_owner}/{self.repository_name}/")

FeatureExternalResource.objects.filter(
feature_id__in=self.project.features.values_list("id", flat=True),
type__in=[
FeatureExternalResource.ResourceType.GITHUB_ISSUE,
FeatureExternalResource.ResourceType.GITHUB_PR,
ResourceType.GITHUB_ISSUE,
ResourceType.GITHUB_PR,
],
# Filter by url containing the repository owner and name
url__regex=pattern,
).delete()
8 changes: 3 additions & 5 deletions api/integrations/github/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
fetch_search_github_resource,
)
from integrations.github.exceptions import DuplicateGitHubIntegration
from integrations.github.github import handle_github_webhook_event
from integrations.github.helpers import github_webhook_payload_is_valid
from integrations.github.models import GithubConfiguration, GithubRepository
from integrations.github.permissions import HasPermissionToGithubConfiguration
Expand Down Expand Up @@ -249,11 +250,8 @@ def github_webhook(request) -> Response:
payload_body=payload, secret_token=secret, signature_header=signature
):
data = json.loads(payload.decode("utf-8"))
# handle GitHub Webhook "installation" event with action type "deleted"
if github_event == "installation" and data["action"] == "deleted":
GithubConfiguration.objects.filter(
installation_id=data["installation"]["id"]
).delete()
if github_event == "installation":
handle_github_webhook_event(event_type=github_event, payload=data)
return Response({"detail": "Event processed"}, status=200)
else:
return Response({"detail": "Event bypassed"}, status=200)
Expand Down
Loading

0 comments on commit 7454e4a

Please sign in to comment.