Skip to content

Commit

Permalink
fix: Solve API GitHub integration issues (#4502)
Browse files Browse the repository at this point in the history
  • Loading branch information
novakzaballa authored Aug 19, 2024
1 parent 7034fa4 commit 19bc58e
Show file tree
Hide file tree
Showing 9 changed files with 130 additions and 59 deletions.
6 changes: 3 additions & 3 deletions api/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
from features.value_types import STRING
from features.versioning.tasks import enable_v2_versioning
from features.workflows.core.models import ChangeRequest
from integrations.github.models import GithubConfiguration, GithubRepository
from integrations.github.models import GithubConfiguration, GitHubRepository
from metadata.models import (
Metadata,
MetadataField,
Expand Down Expand Up @@ -1079,8 +1079,8 @@ def github_configuration(organisation: Organisation) -> GithubConfiguration:
def github_repository(
github_configuration: GithubConfiguration,
project: Project,
) -> GithubRepository:
return GithubRepository.objects.create(
) -> GitHubRepository:
return GitHubRepository.objects.create(
github_configuration=github_configuration,
repository_owner="repositoryownertest",
repository_name="repositorynametest",
Expand Down
15 changes: 13 additions & 2 deletions api/features/feature_external_resources/models.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import json
import logging
import re

from django.db import models
from django.db.models import Q
Expand All @@ -14,7 +15,7 @@
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 integrations.github.models import GitHubRepository
from organisations.models import Organisation
from projects.tags.models import Tag, TagType

Expand Down Expand Up @@ -79,9 +80,19 @@ def execute_after_save_actions(self):
.get(id=self.feature.project.organisation_id)
.github_config.first()
):
github_repo = GithubRepository.objects.get(
if self.type == "GITHUB_PR":
pattern = r"github.com/([^/]+)/([^/]+)/pull/\d+$"
elif self.type == "GITHUB_ISSUE":
pattern = r"github.com/([^/]+)/([^/]+)/issues/\d+$"

url_match = re.search(pattern, self.url)
owner, repo = url_match.groups()

github_repo = GitHubRepository.objects.get(
github_configuration=github_configuration.id,
project=self.feature.project,
repository_owner=owner,
repository_name=repo,
)
if github_repo.tagging_enabled:
github_tag = Tag.objects.get(
Expand Down
4 changes: 2 additions & 2 deletions api/features/feature_external_resources/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
get_github_issue_pr_title_and_state,
label_github_issue_pr,
)
from integrations.github.models import GithubRepository
from integrations.github.models import GitHubRepository
from organisations.models import Organisation

from .models import FeatureExternalResource
Expand Down Expand Up @@ -85,7 +85,7 @@ def create(self, request, *args, **kwargs):
url_match = re.search(pattern, url)
if url_match:
owner, repo, issue = url_match.groups()
if GithubRepository.objects.get(
if GitHubRepository.objects.get(
github_configuration=github_configuration,
repository_owner=owner,
repository_name=repo,
Expand Down
20 changes: 17 additions & 3 deletions api/integrations/github/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,23 @@ def fetch_search_github_resource(
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()
try:
response = requests.get(url, headers=headers, timeout=GITHUB_API_CALLS_TIMEOUT)
response.raise_for_status()
json_response = response.json()

except HTTPError:
response_content = response.content.decode("utf-8")
error_message = (
"The resources do not exist or you do not have permission to view them"
)
error_data = json.loads(response_content)
if error_data.get("message", "") == "Validation Failed" and any(
error.get("code", "") == "invalid" for error in error_data.get("errors", [])
):
logger.warning(error_message)
raise ValueError(error_message)

results = [
{
"html_url": i["html_url"],
Expand Down
2 changes: 1 addition & 1 deletion api/integrations/github/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class Meta:
ordering = ("id",)


class GithubRepository(LifecycleModelMixin, SoftDeleteExportableModel):
class GitHubRepository(LifecycleModelMixin, SoftDeleteExportableModel):
github_configuration = models.ForeignKey(
GithubConfiguration, related_name="repository_config", on_delete=models.CASCADE
)
Expand Down
4 changes: 2 additions & 2 deletions api/integrations/github/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
PaginatedQueryParams,
RepoQueryParams,
)
from integrations.github.models import GithubConfiguration, GithubRepository
from integrations.github.models import GithubConfiguration, GitHubRepository


class GithubConfigurationSerializer(ModelSerializer):
Expand All @@ -19,7 +19,7 @@ class Meta:

class GithubRepositorySerializer(ModelSerializer):
class Meta:
model = GithubRepository
model = GitHubRepository
optional_fields = ("search_text", "page")
fields = (
"id",
Expand Down
6 changes: 3 additions & 3 deletions api/integrations/github/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
tag_by_event_type,
)
from integrations.github.helpers import github_webhook_payload_is_valid
from integrations.github.models import GithubConfiguration, GithubRepository
from integrations.github.models import GithubConfiguration, GitHubRepository
from integrations.github.permissions import HasPermissionToGithubConfiguration
from integrations.github.serializers import (
GithubConfigurationSerializer,
Expand Down Expand Up @@ -126,7 +126,7 @@ class GithubRepositoryViewSet(viewsets.ModelViewSet):
GithubIsAdminOrganisation,
)
serializer_class = GithubRepositorySerializer
model_class = GithubRepository
model_class = GitHubRepository

def perform_create(self, serializer):
github_configuration_id = self.kwargs["github_pk"]
Expand All @@ -136,7 +136,7 @@ def get_queryset(self):
try:
if github_pk := self.kwargs.get("github_pk"):
int(github_pk)
return GithubRepository.objects.filter(github_configuration=github_pk)
return GitHubRepository.objects.filter(github_configuration=github_pk)
except ValueError:
raise ValidationError({"github_pk": ["Must be an integer"]})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
)
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 integrations.github.models import GithubConfiguration, GitHubRepository
from projects.models import Project
from segments.models import Segment
from tests.types import WithEnvironmentPermissionsCallable
Expand Down Expand Up @@ -73,7 +73,7 @@ def test_create_feature_external_resource(
environment: Environment,
project: Project,
github_configuration: GithubConfiguration,
github_repository: GithubRepository,
github_repository: GitHubRepository,
post_request_mock: MagicMock,
mock_github_client_generate_token: MagicMock,
) -> None:
Expand Down Expand Up @@ -186,7 +186,7 @@ def test_cannot_create_feature_external_resource_with_an_invalid_gh_url(
feature: Feature,
project: Project,
github_configuration: GithubConfiguration,
github_repository: GithubRepository,
github_repository: GitHubRepository,
) -> None:
# Given
feature_external_resource_data = {
Expand Down Expand Up @@ -216,7 +216,7 @@ def test_cannot_create_feature_external_resource_with_an_incorrect_gh_type(
feature: Feature,
project: Project,
github_configuration: GithubConfiguration,
github_repository: GithubRepository,
github_repository: GitHubRepository,
) -> None:
# Given
feature_external_resource_data = {
Expand Down Expand Up @@ -316,7 +316,7 @@ def test_cannot_create_feature_external_resource_due_to_unique_constraint(
feature: Feature,
project: Project,
github_configuration: GithubConfiguration,
github_repository: GithubRepository,
github_repository: GitHubRepository,
feature_external_resource: FeatureExternalResource,
) -> None:
# Given
Expand Down Expand Up @@ -346,7 +346,7 @@ def test_update_feature_external_resource(
feature_external_resource: FeatureExternalResource,
project: Project,
github_configuration: GithubConfiguration,
github_repository: GithubRepository,
github_repository: GitHubRepository,
post_request_mock: MagicMock,
mocker: MockerFixture,
) -> None:
Expand All @@ -357,7 +357,7 @@ def test_update_feature_external_resource(
mock_generate_token.return_value = "mocked_token"
feature_external_resource_data = {
"type": "GITHUB_ISSUE",
"url": "https://github.com/userexample/example-project-repo/issues/12",
"url": f"https://github.com/{github_repository.repository_owner}/{github_repository.repository_name}/issues/12",
"feature": feature.id,
"metadata": '{"state": "open"}',
}
Expand All @@ -378,7 +378,7 @@ def test_delete_feature_external_resource(
feature: Feature,
project: Project,
github_configuration: GithubConfiguration,
github_repository: GithubRepository,
github_repository: GitHubRepository,
feature_external_resource: FeatureExternalResource,
post_request_mock: MagicMock,
mocker: MockerFixture,
Expand Down Expand Up @@ -417,7 +417,7 @@ def test_get_feature_external_resources(
feature: Feature,
project: Project,
github_configuration: GithubConfiguration,
github_repository: GithubRepository,
github_repository: GitHubRepository,
feature_external_resource: FeatureExternalResource,
mock_github_client_generate_token: MagicMock,
) -> None:
Expand Down Expand Up @@ -446,7 +446,7 @@ def test_get_feature_external_resource(
feature: Feature,
project: Project,
github_configuration: GithubConfiguration,
github_repository: GithubRepository,
github_repository: GitHubRepository,
feature_external_resource: FeatureExternalResource,
) -> None:
# Given
Expand All @@ -472,7 +472,7 @@ def test_create_github_comment_on_feature_state_updated(
feature: Feature,
project: Project,
github_configuration: GithubConfiguration,
github_repository: GithubRepository,
github_repository: GitHubRepository,
post_request_mock: MagicMock,
mocker: MockerFixture,
environment: Environment,
Expand Down Expand Up @@ -532,7 +532,7 @@ def test_create_github_comment_on_feature_was_deleted(
feature: Feature,
project: Project,
github_configuration: GithubConfiguration,
github_repository: GithubRepository,
github_repository: GitHubRepository,
feature_external_resource: FeatureExternalResource,
post_request_mock: MagicMock,
mock_github_client_generate_token: MagicMock,
Expand Down Expand Up @@ -566,7 +566,7 @@ def test_create_github_comment_on_segment_override_updated(
segment_override_for_feature_with_value: FeatureState,
project: Project,
github_configuration: GithubConfiguration,
github_repository: GithubRepository,
github_repository: GitHubRepository,
post_request_mock: MagicMock,
environment: Environment,
admin_client: APIClient,
Expand Down Expand Up @@ -624,7 +624,7 @@ def test_create_github_comment_on_segment_override_deleted(
segment_override_for_feature_with_value: FeatureState,
feature_with_value_segment: FeatureSegment,
github_configuration: GithubConfiguration,
github_repository: GithubRepository,
github_repository: GitHubRepository,
post_request_mock: MagicMock,
admin_client_new: APIClient,
feature_with_value_external_resource: FeatureExternalResource,
Expand Down Expand Up @@ -665,7 +665,7 @@ def test_create_github_comment_using_v2(
environment: Environment,
project: Project,
github_configuration: GithubConfiguration,
github_repository: GithubRepository,
github_repository: GitHubRepository,
feature_external_resource: FeatureExternalResource,
post_request_mock: MagicMock,
mocker: MockerFixture,
Expand Down Expand Up @@ -738,7 +738,7 @@ def test_create_github_comment_using_v2_fails_on_wrong_params(
environment: Environment,
project: Project,
github_configuration: GithubConfiguration,
github_repository: GithubRepository,
github_repository: GitHubRepository,
feature_external_resource: FeatureExternalResource,
post_request_mock: MagicMock,
mocker: MockerFixture,
Expand Down Expand Up @@ -781,7 +781,7 @@ def test_create_feature_external_resource_on_environment_with_v2(
admin_client_new: APIClient,
project: Project,
github_configuration: GithubConfiguration,
github_repository: GithubRepository,
github_repository: GitHubRepository,
segment_override_for_feature_with_value: FeatureState,
environment_v2_versioning: Environment,
post_request_mock: MagicMock,
Expand Down Expand Up @@ -858,7 +858,7 @@ def test_cannot_create_feature_external_resource_for_the_same_feature_and_resour
feature: Feature,
project: Project,
github_configuration: GithubConfiguration,
github_repository: GithubRepository,
github_repository: GitHubRepository,
feature_external_resource_gh_pr: FeatureExternalResource,
) -> None:
# Given
Expand Down
Loading

0 comments on commit 19bc58e

Please sign in to comment.