Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Delete feature external resources when GitHub integration was deleted #3836

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Generated by Django 3.2.25 on 2024-04-24 18:55

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('feature_external_resources', '0001_initial'),
]

operations = [
migrations.AddIndex(
model_name='featureexternalresource',
index=models.Index(fields=['type'], name='feature_ext_type_2b2068_idx'),
),
]
3 changes: 3 additions & 0 deletions api/features/feature_external_resources/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ class Meta:
fields=["feature", "url"], name="unique_feature_url_constraint"
)
]
indexes = [
models.Index(fields=["type"]),
]

@hook(AFTER_SAVE)
def exectute_after_save_actions(self):
Expand Down
19 changes: 18 additions & 1 deletion api/integrations/github/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from core.models import SoftDeleteExportableModel
from django.db import models
from django_lifecycle import BEFORE_DELETE, LifecycleModelMixin, hook

from organisations.models import Organisation

Expand All @@ -21,7 +22,7 @@ def has_github_configuration(organisation_id: int) -> bool:
).exists()


class GithubRepository(SoftDeleteExportableModel):
class GithubRepository(LifecycleModelMixin, SoftDeleteExportableModel):
github_configuration = models.ForeignKey(
GithubConfiguration, related_name="repository_config", on_delete=models.CASCADE
)
Expand All @@ -47,3 +48,19 @@ class Meta:
name="unique_repository_data",
)
]

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

FeatureExternalResource.objects.filter(
feature_id__in=self.project.features.values_list("id", flat=True),
type__in=[
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we need to add an index on FeatureExternalResource.type?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, I added an index

FeatureExternalResource.ResourceType.GITHUB_ISSUE,
FeatureExternalResource.ResourceType.GITHUB_PR,
],
).delete()
12 changes: 12 additions & 0 deletions api/tests/unit/integrations/github/test_unit_github_views.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import pytest
from django.urls import reverse
from pytest_lazyfixture import lazy_fixture
from pytest_mock import MockerFixture
from rest_framework import status
from rest_framework.test import APIClient

from features.feature_external_resources.models import FeatureExternalResource
from integrations.github.models import GithubConfiguration, GithubRepository
from organisations.models import Organisation
from projects.models import Project
Expand Down Expand Up @@ -191,18 +193,28 @@ def test_cannot_create_github_repository_due_to_unique_constraint(
def test_github_delete_repository(
client: APIClient,
organisation: Organisation,
feature_external_resource: FeatureExternalResource,
github_configuration: GithubConfiguration,
github_repository: GithubRepository,
mocker: MockerFixture,
) -> None:
# Given
mock_generate_token = mocker.patch(
"integrations.github.github.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],
)
for feature in github_repository.project.features.all():
assert FeatureExternalResource.objects.filter(feature=feature).exists()
# When
response = client.delete(url)
# Then
assert response.status_code == status.HTTP_204_NO_CONTENT
for feature in github_repository.project.features.all():
assert not FeatureExternalResource.objects.filter(feature=feature).exists()


def mocked_requests_get(*args, **kwargs):
Expand Down