diff --git a/api/integrations/github/client.py b/api/integrations/github/client.py index 97a0480ee831..b4307fd736f9 100644 --- a/api/integrations/github/client.py +++ b/api/integrations/github/client.py @@ -1,19 +1,21 @@ import logging -import typing from enum import Enum +from typing import Any import requests from django.conf import settings from github import Auth, Github -from rest_framework import status -from rest_framework.response import Response from integrations.github.constants import ( GITHUB_API_CALLS_TIMEOUT, GITHUB_API_URL, GITHUB_API_VERSION, ) -from integrations.github.dataclasses import RepoQueryParams +from integrations.github.dataclasses import ( + IssueQueryParams, + PaginatedQueryParams, + RepoQueryParams, +) from integrations.github.models import GithubConfiguration logger = logging.getLogger(__name__) @@ -66,9 +68,34 @@ def generate_jwt_token(app_id: int) -> str: # pragma: no cover return token +def build_paginated_response( + results: list[dict[str, Any]], + response: requests.Response, + total_count: int | None = None, + incomplete_results: bool | None = None, +) -> dict[str, Any]: + data: dict[str, Any] = { + "results": results, + } + + if response.links.get("prev"): + data["previous"] = response.links.get("prev") + + if response.links.get("next"): + data["next"] = response.links.get("next") + + if total_count: + data["total_count"] = total_count + + if incomplete_results: + data["incomplete_results"] = incomplete_results + + return data + + def post_comment_to_github( installation_id: str, owner: str, repo: str, issue: str, body: str -) -> dict[str, typing.Any]: +) -> dict[str, Any]: url = f"{GITHUB_API_URL}repos/{owner}/{repo}/issues/{issue}/comments" headers = build_request_headers(installation_id) @@ -89,11 +116,11 @@ def delete_github_installation(installation_id: str) -> requests.Response: return response -def fetch_github_resource( +def fetch_search_github_resource( resource_type: ResourceType, organisation_id: int, - params: RepoQueryParams, -) -> dict[str, typing.Any]: + params: IssueQueryParams, +) -> dict[str, Any]: github_configuration = GithubConfiguration.objects.get( organisation_id=organisation_id, deleted_at__isnull=True ) @@ -135,22 +162,23 @@ def fetch_github_resource( } for i in json_response["items"] ] - data = { - "results": results, - "count": json_response["total_count"], - "incomplete_results": json_response["incomplete_results"], - } - if response.links.get("prev"): - data["previous"] = response.links.get("prev") - if response.links.get("next"): - data["next"] = response.links.get("next") - - return data + return build_paginated_response( + results=results, + response=response, + total_count=json_response["total_count"], + incomplete_results=json_response["incomplete_results"], + ) -def fetch_github_repositories(installation_id: str) -> Response: - url = f"{GITHUB_API_URL}installation/repositories" +def fetch_github_repositories( + installation_id: str, + params: PaginatedQueryParams, +) -> dict[str, Any]: + url = ( + f"{GITHUB_API_URL}installation/repositories?" + + f"&per_page={params.page_size}&page={params.page}" + ) headers: dict[str, str] = build_request_headers(installation_id) @@ -165,15 +193,8 @@ def fetch_github_repositories(installation_id: str) -> Response: } for i in json_response["repositories"] ] - data = { - "repositories": results, - "total_count": json_response["total_count"], - } - return Response( - data=data, - content_type="application/json", - status=status.HTTP_200_OK, - ) + + return build_paginated_response(results, response, json_response["total_count"]) def get_github_issue_pr_title_and_state( @@ -191,5 +212,35 @@ def get_github_issue_pr_title_and_state( headers = build_request_headers(installation_id) response = requests.get(url, headers=headers, timeout=GITHUB_API_CALLS_TIMEOUT) response.raise_for_status() - response_json = response.json() - return {"title": response_json["title"], "state": response_json["state"]} + json_response = response.json() + return {"title": json_response["title"], "state": json_response["state"]} + + +def fetch_github_repo_contributors( + organisation_id: int, + params: RepoQueryParams, +) -> dict[str, Any]: + installation_id = GithubConfiguration.objects.get( + organisation_id=organisation_id, deleted_at__isnull=True + ).installation_id + + url = ( + f"{GITHUB_API_URL}repos/{params.repo_owner}/{params.repo_name}/contributors?" + + f"&per_page={params.page_size}&page={params.page}" + ) + + headers = build_request_headers(installation_id) + response = requests.get(url, headers=headers, timeout=GITHUB_API_CALLS_TIMEOUT) + response.raise_for_status() + json_response = response.json() + + results = [ + { + "login": i["login"], + "avatar_url": i["avatar_url"], + "contributions": i["contributions"], + } + for i in json_response + ] + + return build_paginated_response(results, response) diff --git a/api/integrations/github/dataclasses.py b/api/integrations/github/dataclasses.py index f90c11ff52b5..39ce51d999a4 100644 --- a/api/integrations/github/dataclasses.py +++ b/api/integrations/github/dataclasses.py @@ -1,21 +1,21 @@ -import typing -from dataclasses import dataclass -from typing import Optional +from dataclasses import dataclass, field +from typing import Any, Optional +# Base Dataclasses @dataclass class GithubData: installation_id: str feature_id: int feature_name: str type: str - feature_states: list[dict[str, typing.Any]] | None = None + feature_states: list[dict[str, Any]] | None = None url: str | None = None project_id: int | None = None segment_name: str | None = None @classmethod - def from_dict(cls, data_dict: dict[str, typing.Any]) -> "GithubData": + def from_dict(cls, data_dict: dict[str, Any]) -> "GithubData": return cls(**data_dict) @@ -23,22 +23,33 @@ def from_dict(cls, data_dict: dict[str, typing.Any]) -> "GithubData": class CallGithubData: event_type: str github_data: GithubData - feature_external_resources: list[dict[str, typing.Any]] + feature_external_resources: list[dict[str, Any]] +# Dataclasses for external calls to GitHub API @dataclass -class RepoQueryParams: +class PaginatedQueryParams: + page: int = field(default=1, kw_only=True) + page_size: int = field(default=100, kw_only=True) + + def __post_init__(self): + if self.page < 1: + raise ValueError("Page must be greater or equal than 1") + if self.page_size < 1 or self.page_size > 100: + raise ValueError("Page size must be an integer between 1 and 100") + + +@dataclass +class RepoQueryParams(PaginatedQueryParams): repo_owner: str repo_name: str + + +@dataclass +class IssueQueryParams(RepoQueryParams): search_text: Optional[str] = None - page: Optional[int] = 1 - page_size: Optional[int] = 100 state: Optional[str] = "open" author: Optional[str] = None assignee: Optional[str] = None search_in_body: Optional[bool] = True search_in_comments: Optional[bool] = False - - @classmethod - def from_dict(cls, data_dict: dict[str, typing.Any]) -> "RepoQueryParams": - return cls(**data_dict) diff --git a/api/integrations/github/serializers.py b/api/integrations/github/serializers.py index 88f74f85f022..b3dc96c5263e 100644 --- a/api/integrations/github/serializers.py +++ b/api/integrations/github/serializers.py @@ -2,7 +2,11 @@ from rest_framework.serializers import ModelSerializer from rest_framework_dataclasses.serializers import DataclassSerializer -from integrations.github.dataclasses import RepoQueryParams +from integrations.github.dataclasses import ( + IssueQueryParams, + PaginatedQueryParams, + RepoQueryParams, +) from integrations.github.models import GithubConfiguration, GithubRepository @@ -30,8 +34,18 @@ class Meta: ) +class PaginatedQueryParamsSerializer(DataclassSerializer): + class Meta: + dataclass = PaginatedQueryParams + + class RepoQueryParamsSerializer(DataclassSerializer): class Meta: dataclass = RepoQueryParams + +class IssueQueryParamsSerializer(DataclassSerializer): + class Meta: + dataclass = IssueQueryParams + search_in_body = serializers.BooleanField(required=False, default=True) diff --git a/api/integrations/github/views.py b/api/integrations/github/views.py index 259f41d46ef9..67063bcc34ba 100644 --- a/api/integrations/github/views.py +++ b/api/integrations/github/views.py @@ -16,10 +16,10 @@ from integrations.github.client import ( ResourceType, delete_github_installation, + fetch_github_repo_contributors, fetch_github_repositories, - fetch_github_resource, + fetch_search_github_resource, ) -from integrations.github.dataclasses import RepoQueryParams from integrations.github.exceptions import DuplicateGitHubIntegration from integrations.github.helpers import github_webhook_payload_is_valid from integrations.github.models import GithubConfiguration, GithubRepository @@ -27,6 +27,8 @@ from integrations.github.serializers import ( GithubConfigurationSerializer, GithubRepositorySerializer, + IssueQueryParamsSerializer, + PaginatedQueryParamsSerializer, RepoQueryParamsSerializer, ) from organisations.permissions.permissions import GithubIsAdminOrganisation @@ -62,6 +64,12 @@ def wrapper(*args, **kwargs) -> Response: default_error = "Failed to retrieve requested information from GitHub API." try: return func(*args, **kwargs) + except ValueError as e: + return Response( + data={"detail": (f"{error or default_error}" f" Error: {str(e)}")}, + content_type="application/json", + status=status.HTTP_400_BAD_REQUEST, + ) except requests.RequestException as e: logger.error(f"{error or default_error} Error: {str(e)}", exc_info=e) return Response( @@ -146,17 +154,15 @@ def create(self, request, *args, **kwargs): @permission_classes([IsAuthenticated, HasPermissionToGithubConfiguration]) @github_auth_required @github_api_call_error_handler(error="Failed to retrieve GitHub pull requests.") -def fetch_pull_requests(request, organisation_pk) -> Response | None: - query_serializer = RepoQueryParamsSerializer(data=request.query_params) +def fetch_pull_requests(request, organisation_pk) -> Response: + query_serializer = IssueQueryParamsSerializer(data=request.query_params) if not query_serializer.is_valid(): return Response({"error": query_serializer.errors}, status=400) - query_params = RepoQueryParams.from_dict(query_serializer.validated_data.__dict__) - - data = fetch_github_resource( + data = fetch_search_github_resource( resource_type=ResourceType.PULL_REQUESTS, organisation_id=organisation_pk, - params=query_params, + params=query_serializer.validated_data, ) return Response( data=data, @@ -168,18 +174,16 @@ def fetch_pull_requests(request, organisation_pk) -> Response | None: @api_view(["GET"]) @permission_classes([IsAuthenticated, HasPermissionToGithubConfiguration]) @github_auth_required -@github_api_call_error_handler(error="Failed to retrieve GitHub pull requests.") +@github_api_call_error_handler(error="Failed to retrieve GitHub issues.") def fetch_issues(request, organisation_pk) -> Response | None: - query_serializer = RepoQueryParamsSerializer(data=request.query_params) + query_serializer = IssueQueryParamsSerializer(data=request.query_params) if not query_serializer.is_valid(): return Response({"error": query_serializer.errors}, status=400) - query_params = RepoQueryParams.from_dict(query_serializer.validated_data.__dict__) - - data = fetch_github_resource( + data = fetch_search_github_resource( resource_type=ResourceType.ISSUES, organisation_id=organisation_pk, - params=query_params, + params=query_serializer.validated_data, ) return Response( data=data, @@ -190,8 +194,11 @@ def fetch_issues(request, organisation_pk) -> Response | None: @api_view(["GET"]) @permission_classes([IsAuthenticated, GithubIsAdminOrganisation]) -@github_api_call_error_handler(error="Failed to retrieve GitHub pull requests.") +@github_api_call_error_handler(error="Failed to retrieve GitHub repositories.") def fetch_repositories(request, organisation_pk: int) -> Response | None: + query_serializer = PaginatedQueryParamsSerializer(data=request.query_params) + if not query_serializer.is_valid(): + return Response({"error": query_serializer.errors}, status=400) installation_id = request.GET.get("installation_id") if not installation_id: @@ -201,8 +208,34 @@ def fetch_repositories(request, organisation_pk: int) -> Response | None: status=status.HTTP_400_BAD_REQUEST, ) - response: Response = fetch_github_repositories(installation_id) - return response + data = fetch_github_repositories( + installation_id=installation_id, params=query_serializer.validated_data + ) + return Response( + data=data, + content_type="application/json", + status=status.HTTP_200_OK, + ) + + +@api_view(["GET"]) +@permission_classes([IsAuthenticated, HasPermissionToGithubConfiguration]) +@github_auth_required +@github_api_call_error_handler(error="Failed to retrieve GitHub repo contributors.") +def fetch_repo_contributors(request, organisation_pk) -> Response: + query_serializer = RepoQueryParamsSerializer(data=request.query_params) + if not query_serializer.is_valid(): + return Response({"error": query_serializer.errors}, status=400) + + response = fetch_github_repo_contributors( + organisation_id=organisation_pk, params=query_serializer.validated_data + ) + + return Response( + data=response, + content_type="application/json", + status=status.HTTP_200_OK, + ) @api_view(["POST"]) diff --git a/api/organisations/urls.py b/api/organisations/urls.py index 5acd955b0dde..5504a0afa24f 100644 --- a/api/organisations/urls.py +++ b/api/organisations/urls.py @@ -13,6 +13,7 @@ GithubRepositoryViewSet, fetch_issues, fetch_pull_requests, + fetch_repo_contributors, fetch_repositories, ) from metadata.views import MetaDataModelFieldViewSet @@ -124,6 +125,11 @@ fetch_issues, name="get-github-issues", ), + path( + "/github/repo-contributors/", + fetch_repo_contributors, + name="get-github-repo-contributors", + ), path( "/github/pulls/", fetch_pull_requests, 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 3c2ad3b65ae8..16d8df6f3c30 100644 --- a/api/tests/unit/integrations/github/test_unit_github_views.py +++ b/api/tests/unit/integrations/github/test_unit_github_views.py @@ -1,4 +1,5 @@ import json +from typing import Any import pytest import requests @@ -8,12 +9,16 @@ from pytest_lazyfixture import lazy_fixture from pytest_mock import MockerFixture from rest_framework import status +from rest_framework.response import Response from rest_framework.test import APIClient from features.feature_external_resources.models import FeatureExternalResource from integrations.github.constants import GITHUB_API_URL from integrations.github.models import GithubConfiguration, GithubRepository -from integrations.github.views import github_webhook_payload_is_valid +from integrations.github.views import ( + github_api_call_error_handler, + github_webhook_payload_is_valid, +) from organisations.models import Organisation from projects.models import Project @@ -347,7 +352,7 @@ def mocked_requests_get_issues_and_pull_requests(*args, **kwargs): }, ], "total_count": 1, - "incomplete_results": 0, + "incomplete_results": True, } status_code = 200 response = MockResponse(json_data, status_code) @@ -467,7 +472,7 @@ def test_fetch_issues_returns_error_on_bad_response_from_github( assert response.status_code == status.HTTP_502_BAD_GATEWAY response_json = response.json() assert ( - "Failed to retrieve GitHub pull requests. Error: HTTP Error 404" + "Failed to retrieve GitHub issues. Error: HTTP Error 404" in response_json["detail"] ) @@ -512,8 +517,8 @@ def test_fetch_repositories( # Then assert response.status_code == status.HTTP_200_OK response_json = response.json() - assert "repositories" in response_json - assert len(response_json["repositories"]) == 1 + assert "results" in response_json + assert len(response_json["results"]) == 1 @pytest.mark.parametrize( @@ -776,3 +781,197 @@ def test_cannot_fetch_repositories_when_there_is_no_installation_id( # Then assert response.status_code == status.HTTP_400_BAD_REQUEST assert response.json() == {"detail": "Missing installation_id parameter"} + + +@responses.activate +def test_fetch_github_repo_contributors( + admin_client_new: APIClient, + organisation: Organisation, + github_configuration: GithubConfiguration, + github_repository: GithubRepository, + mocker: MockerFixture, +) -> None: + # Given + url = reverse( + viewname="api-v1:organisations:get-github-repo-contributors", + args=[organisation.id], + ) + + mocked_github_response = [ + { + "login": "contributor1", + "avatar_url": "https://example.com/avatar1", + "contributions": 150, + }, + { + "login": "contributor2", + "avatar_url": "https://example.com/avatar2", + "contributions": 110, + }, + { + "login": "contributor3", + "avatar_url": "https://example.com/avatar3", + "contributions": 12, + }, + ] + + 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, + url=( + f"{GITHUB_API_URL}repos/{github_repository.repository_owner}/{github_repository.repository_name}/" + "contributors?&per_page=100&page=1" + ), + json=mocked_github_response, + status=200, + ) + + # When + response = admin_client_new.get( + path=url, + data={ + "repo_owner": github_repository.repository_owner, + "repo_name": github_repository.repository_name, + }, + ) + + # Then + assert response.status_code == status.HTTP_200_OK + assert response.json() == expected_response + + +def test_fetch_github_repo_contributors_with_invalid_query_params( + admin_client_new: APIClient, + organisation: Organisation, + github_configuration: GithubConfiguration, + github_repository: GithubRepository, +) -> None: + # Given + url = reverse( + viewname="api-v1:organisations:get-github-repo-contributors", + args=[organisation.id], + ) + + # When + response = admin_client_new.get( + path=url, + data={ + "repo_owner": github_repository.repository_owner, + }, + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json() == {"error": {"repo_name": ["This field is required."]}} + + +def test_github_api_call_error_handler_with_value_error( + mocker: MockerFixture, +) -> None: + # Given + @github_api_call_error_handler() + def test_view(request): + raise ValueError("Invalid parameter") + + # When + response = test_view(None) + + # Then + assert isinstance(response, Response) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.data == { + "detail": "Failed to retrieve requested information from GitHub API. Error: Invalid parameter" + } + + +@pytest.mark.parametrize( + "page, page_size, error_detail", + [ + ( + 1, + 103, + "Failed to retrieve GitHub repositories. Error: Page size must be an integer between 1 and 100", + ), + ( + 0, + 100, + "Failed to retrieve GitHub repositories. Error: Page must be greater or equal than 1", + ), + ], +) +def test_send_the_invalid_number_page_or_page_size_param_returns_400( + admin_client: APIClient, + organisation: Organisation, + github_configuration: GithubConfiguration, + github_repository: GithubRepository, + page: int, + page_size: int, + error_detail: str, +) -> None: + # Given + data: dict[str, str | int] = { + "installation_id": github_configuration.installation_id, + "page": page, + "page_size": page_size, + } + + url = reverse( + "api-v1:organisations:get-github-installation-repos", args=[organisation.id] + ) + # When + response = admin_client.get(url, data) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + response_json = response.json() + assert response_json == {"detail": error_detail} + + +@pytest.mark.parametrize( + "page, page_size, error_response", + [ + ( + 1, + "string", + {"error": {"page_size": ["A valid integer is required."]}}, + ), + ( + "string", + 100, + {"error": {"page": ["A valid integer is required."]}}, + ), + ], +) +def test_send_the_invalid_type_page_or_page_size_param_returns_400( + admin_client: APIClient, + organisation: Organisation, + github_configuration: GithubConfiguration, + github_repository: GithubRepository, + page: int, + page_size: int, + error_response: dict[str, Any], +) -> None: + # Given + data: dict[str, str | int] = { + "installation_id": github_configuration.installation_id, + "page": page, + "page_size": page_size, + } + + url = reverse( + "api-v1:organisations:get-github-installation-repos", args=[organisation.id] + ) + # When + response = admin_client.get(url, data) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + response_json = response.json() + assert response_json == error_response