diff --git a/src/sentry/api/urls.py b/src/sentry/api/urls.py index b9c1a1baa79e39..0ac94221d93841 100644 --- a/src/sentry/api/urls.py +++ b/src/sentry/api/urls.py @@ -311,6 +311,8 @@ from sentry.issues.endpoints.organization_issues_resolved_in_release import ( OrganizationIssuesResolvedInReleaseEndpoint, ) +from sentry.issues.endpoints.project_codeowners_details import ProjectCodeOwnersDetailsEndpoint +from sentry.issues.endpoints.project_codeowners_index import ProjectCodeOwnersEndpoint from sentry.issues.endpoints.project_grouping_configs import ProjectGroupingConfigsEndpoint from sentry.issues.endpoints.project_issues_resolved_in_release import ( ProjectIssuesResolvedInReleaseEndpoint, @@ -590,7 +592,6 @@ from .endpoints.catchall import CatchallEndpoint from .endpoints.check_am2_compatibility import CheckAM2CompatibilityEndpoint from .endpoints.chunk import ChunkUploadEndpoint -from .endpoints.codeowners import ProjectCodeOwnersDetailsEndpoint, ProjectCodeOwnersEndpoint from .endpoints.custom_rules import CustomRulesEndpoint from .endpoints.data_scrubbing_selector_suggestions import DataScrubbingSelectorSuggestionsEndpoint from .endpoints.debug_files import ( diff --git a/src/sentry/issues/endpoints/bases/__init__.py b/src/sentry/issues/endpoints/bases/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/src/sentry/issues/endpoints/bases/codeowners.py b/src/sentry/issues/endpoints/bases/codeowners.py new file mode 100644 index 00000000000000..7caa45086ea0ae --- /dev/null +++ b/src/sentry/issues/endpoints/bases/codeowners.py @@ -0,0 +1,23 @@ +from rest_framework.request import Request + +from sentry import features +from sentry.api.bases.project import ProjectEndpoint +from sentry.models.project import Project +from sentry.utils import metrics + + +class ProjectCodeOwnersBase(ProjectEndpoint): + def has_feature(self, request: Request, project: Project) -> bool: + return bool( + features.has( + "organizations:integrations-codeowners", project.organization, actor=request.user + ) + ) + + def track_response_code(self, type: str, status: int | str) -> None: + if type in ["create", "update"]: + metrics.incr( + f"codeowners.{type}.http_response", + sample_rate=1.0, + tags={"status": status}, + ) diff --git a/src/sentry/issues/endpoints/bases.py b/src/sentry/issues/endpoints/bases/group_search_view.py similarity index 100% rename from src/sentry/issues/endpoints/bases.py rename to src/sentry/issues/endpoints/bases/group_search_view.py diff --git a/src/sentry/issues/endpoints/organization_group_search_view_details.py b/src/sentry/issues/endpoints/organization_group_search_view_details.py index a0b874f4519935..a60be9c2ab7702 100644 --- a/src/sentry/issues/endpoints/organization_group_search_view_details.py +++ b/src/sentry/issues/endpoints/organization_group_search_view_details.py @@ -12,7 +12,7 @@ from sentry.api.serializers.base import serialize from sentry.api.serializers.models.groupsearchview import GroupSearchViewSerializer from sentry.api.serializers.rest_framework.groupsearchview import ViewValidator -from sentry.issues.endpoints.bases import GroupSearchViewPermission +from sentry.issues.endpoints.bases.group_search_view import GroupSearchViewPermission from sentry.issues.endpoints.organization_group_search_views import pick_default_project from sentry.models.groupsearchview import GroupSearchView from sentry.models.groupsearchviewlastvisited import GroupSearchViewLastVisited diff --git a/src/sentry/api/endpoints/codeowners/details.py b/src/sentry/issues/endpoints/project_codeowners_details.py similarity index 94% rename from src/sentry/api/endpoints/codeowners/details.py rename to src/sentry/issues/endpoints/project_codeowners_details.py index 8bb4bf1a93a564..7b84c434b363d6 100644 --- a/src/sentry/api/endpoints/codeowners/details.py +++ b/src/sentry/issues/endpoints/project_codeowners_details.py @@ -13,20 +13,19 @@ from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint -from sentry.api.bases.project import ProjectEndpoint from sentry.api.exceptions import ResourceDoesNotExist from sentry.api.serializers import serialize from sentry.api.serializers.models import projectcodeowners as projectcodeowners_serializers +from sentry.issues.endpoints.bases.codeowners import ProjectCodeOwnersBase +from sentry.issues.endpoints.serializers import ProjectCodeOwnerSerializer from sentry.models.project import Project from sentry.models.projectcodeowners import ProjectCodeOwners -from . import ProjectCodeOwnerSerializer, ProjectCodeOwnersMixin - logger = logging.getLogger(__name__) @region_silo_endpoint -class ProjectCodeOwnersDetailsEndpoint(ProjectEndpoint, ProjectCodeOwnersMixin): +class ProjectCodeOwnersDetailsEndpoint(ProjectCodeOwnersBase): owner = ApiOwner.ISSUES publish_status = { "DELETE": ApiPublishStatus.PRIVATE, diff --git a/src/sentry/api/endpoints/codeowners/index.py b/src/sentry/issues/endpoints/project_codeowners_index.py similarity index 96% rename from src/sentry/api/endpoints/codeowners/index.py rename to src/sentry/issues/endpoints/project_codeowners_index.py index 2c0270accf1fbd..b9162be45c02cc 100644 --- a/src/sentry/api/endpoints/codeowners/index.py +++ b/src/sentry/issues/endpoints/project_codeowners_index.py @@ -9,10 +9,11 @@ from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint -from sentry.api.bases.project import ProjectEndpoint from sentry.api.serializers import serialize from sentry.api.serializers.models import projectcodeowners as projectcodeowners_serializers from sentry.api.validators.project_codeowners import validate_codeowners_associations +from sentry.issues.endpoints.bases.codeowners import ProjectCodeOwnersBase +from sentry.issues.endpoints.serializers import ProjectCodeOwnerSerializer from sentry.issues.ownership.grammar import ( convert_codeowners_syntax, create_schema_from_issue_owners, @@ -20,11 +21,9 @@ from sentry.models.project import Project from sentry.models.projectcodeowners import ProjectCodeOwners -from . import ProjectCodeOwnerSerializer, ProjectCodeOwnersMixin - @region_silo_endpoint -class ProjectCodeOwnersEndpoint(ProjectEndpoint, ProjectCodeOwnersMixin): +class ProjectCodeOwnersEndpoint(ProjectCodeOwnersBase): owner = ApiOwner.ISSUES publish_status = { "GET": ApiPublishStatus.PRIVATE, diff --git a/src/sentry/api/endpoints/codeowners/__init__.py b/src/sentry/issues/endpoints/serializers.py similarity index 85% rename from src/sentry/api/endpoints/codeowners/__init__.py rename to src/sentry/issues/endpoints/serializers.py index a32b0bbf255287..de08d573654cae 100644 --- a/src/sentry/api/endpoints/codeowners/__init__.py +++ b/src/sentry/issues/endpoints/serializers.py @@ -6,9 +6,8 @@ import sentry_sdk from rest_framework import serializers from rest_framework.exceptions import ValidationError -from rest_framework.request import Request -from sentry import analytics, features +from sentry import analytics from sentry.analytics.events.codeowners_max_length_exceeded import CodeOwnersMaxLengthExceeded from sentry.api.serializers.rest_framework.base import CamelSnakeModelSerializer from sentry.api.validators.project_codeowners import validate_codeowners_associations @@ -17,9 +16,7 @@ convert_codeowners_syntax, create_schema_from_issue_owners, ) -from sentry.models.project import Project from sentry.models.projectcodeowners import ProjectCodeOwners -from sentry.utils import metrics from sentry.utils.codeowners import MAX_RAW_LENGTH @@ -124,29 +121,3 @@ def update( setattr(instance, key, value) instance.save() return instance - - -class ProjectCodeOwnersMixin: - def has_feature(self, request: Request, project: Project) -> bool: - return bool( - features.has( - "organizations:integrations-codeowners", project.organization, actor=request.user - ) - ) - - def track_response_code(self, type: str, status: int | str) -> None: - if type in ["create", "update"]: - metrics.incr( - f"codeowners.{type}.http_response", - sample_rate=1.0, - tags={"status": status}, - ) - - -from .details import ProjectCodeOwnersDetailsEndpoint -from .index import ProjectCodeOwnersEndpoint - -__all__ = ( - "ProjectCodeOwnersEndpoint", - "ProjectCodeOwnersDetailsEndpoint", -) diff --git a/tests/sentry/api/endpoints/test_project_codeowners_details.py b/tests/sentry/api/endpoints/test_project_codeowners_details.py index 82f3594dc88115..e625070f744bcb 100644 --- a/tests/sentry/api/endpoints/test_project_codeowners_details.py +++ b/tests/sentry/api/endpoints/test_project_codeowners_details.py @@ -146,7 +146,8 @@ def test_codeowners_email_update(self) -> None: @patch("sentry.analytics.record") def test_codeowners_max_raw_length(self, mock_record: MagicMock) -> None: with mock.patch( - "sentry.api.endpoints.codeowners.MAX_RAW_LENGTH", len(self.data["raw"]) + 1 + "sentry.issues.endpoints.serializers.MAX_RAW_LENGTH", + len(self.data["raw"]) + 1, ): data = { "raw": f"# cool stuff comment\n*.js {self.user.email}\n# good comment" diff --git a/tests/sentry/issues/endpoints/test_project_codeowners_details.py b/tests/sentry/issues/endpoints/test_project_codeowners_details.py new file mode 100644 index 00000000000000..34f0cb37321fee --- /dev/null +++ b/tests/sentry/issues/endpoints/test_project_codeowners_details.py @@ -0,0 +1,191 @@ +from datetime import datetime, timezone +from unittest import mock +from unittest.mock import MagicMock, patch + +from django.urls import reverse +from rest_framework.exceptions import ErrorDetail + +from sentry.analytics.events.codeowners_max_length_exceeded import CodeOwnersMaxLengthExceeded +from sentry.models.projectcodeowners import ProjectCodeOwners +from sentry.testutils.cases import APITestCase +from sentry.testutils.helpers.analytics import assert_last_analytics_event + + +class ProjectCodeOwnersDetailsEndpointTestCase(APITestCase): + def setUp(self) -> None: + self.user = self.create_user("admin@sentry.io", is_superuser=True) + + self.login_as(user=self.user) + + self.team = self.create_team( + organization=self.organization, slug="tiger-team", members=[self.user] + ) + + self.project = self.project = self.create_project( + organization=self.organization, teams=[self.team], slug="bengal" + ) + + self.code_mapping = self.create_code_mapping(project=self.project) + self.external_user = self.create_external_user( + external_name="@NisanthanNanthakumar", integration=self.integration + ) + self.external_team = self.create_external_team(integration=self.integration) + self.data = { + "raw": "docs/* @NisanthanNanthakumar @getsentry/ecosystem\n", + } + self.codeowners = self.create_codeowners( + project=self.project, code_mapping=self.code_mapping + ) + self.url = reverse( + "sentry-api-0-project-codeowners-details", + kwargs={ + "organization_id_or_slug": self.organization.slug, + "project_id_or_slug": self.project.slug, + "codeowners_id": self.codeowners.id, + }, + ) + + # Mock the external HTTP request to prevent real network calls + self.codeowner_patcher = patch( + "sentry.integrations.source_code_management.repository.RepositoryIntegration.get_codeowner_file", + return_value={ + "html_url": "https://github.com/test/CODEOWNERS", + "filepath": "CODEOWNERS", + "raw": "test content", + }, + ) + self.codeowner_mock = self.codeowner_patcher.start() + self.addCleanup(self.codeowner_patcher.stop) + + def test_basic_delete(self) -> None: + with self.feature({"organizations:integrations-codeowners": True}): + response = self.client.delete(self.url) + assert response.status_code == 204 + assert not ProjectCodeOwners.objects.filter(id=str(self.codeowners.id)).exists() + + @patch("django.utils.timezone.now") + def test_basic_update(self, mock_timezone_now: MagicMock) -> None: + self.create_external_team(external_name="@getsentry/frontend", integration=self.integration) + self.create_external_team(external_name="@getsentry/docs", integration=self.integration) + date = datetime(2023, 10, 3, tzinfo=timezone.utc) + mock_timezone_now.return_value = date + raw = "\n# cool stuff comment\n*.js @getsentry/frontend @NisanthanNanthakumar\n# good comment\n\n\n docs/* @getsentry/docs @getsentry/ecosystem\n\n" + data = { + "raw": raw, + } + + # Reset call count to verify this specific test's calls + self.codeowner_mock.reset_mock() + + with self.feature({"organizations:integrations-codeowners": True}): + response = self.client.put(self.url, data) + + # Verify our mock was called instead of making real HTTP requests + assert ( + self.codeowner_mock.called + ), "Mock should have been called - no external HTTP requests made" + + assert response.status_code == 200 + assert response.data["id"] == str(self.codeowners.id) + assert response.data["raw"] == raw.strip() + codeowner = ProjectCodeOwners.objects.filter(id=self.codeowners.id)[0] + assert codeowner.date_updated == date + + def test_wrong_codeowners_id(self) -> None: + self.url = reverse( + "sentry-api-0-project-codeowners-details", + kwargs={ + "organization_id_or_slug": self.organization.slug, + "project_id_or_slug": self.project.slug, + "codeowners_id": 1000, + }, + ) + with self.feature({"organizations:integrations-codeowners": True}): + response = self.client.put(self.url, self.data) + assert response.status_code == 404 + assert response.data == {"detail": "The requested resource does not exist"} + + def test_missing_external_associations_update(self) -> None: + data = { + "raw": "\n# cool stuff comment\n*.js @getsentry/frontend @NisanthanNanthakumar\n# good comment\n\n\n docs/* @getsentry/docs @getsentry/ecosystem\nsrc/sentry/* @AnotherUser\n\n" + } + with self.feature({"organizations:integrations-codeowners": True}): + response = self.client.put(self.url, data) + assert response.status_code == 200 + assert response.data["id"] == str(self.codeowners.id) + assert response.data["codeMappingId"] == str(self.code_mapping.id) + + errors = response.data["errors"] + assert set(errors["missing_external_teams"]) == {"@getsentry/frontend", "@getsentry/docs"} + assert set(errors["missing_external_users"]) == {"@AnotherUser"} + assert errors["missing_user_emails"] == [] + assert errors["teams_without_access"] == [] + assert errors["users_without_access"] == [] + + def test_invalid_code_mapping_id_update(self) -> None: + with self.feature({"organizations:integrations-codeowners": True}): + response = self.client.put(self.url, {"codeMappingId": 500}) + assert response.status_code == 400 + assert response.data == {"codeMappingId": ["This code mapping does not exist."]} + + def test_no_duplicates_code_mappings(self) -> None: + new_code_mapping = self.create_code_mapping(project=self.project, stack_root="blah") + self.create_codeowners(project=self.project, code_mapping=new_code_mapping) + with self.feature({"organizations:integrations-codeowners": True}): + response = self.client.put(self.url, {"codeMappingId": new_code_mapping.id}) + assert response.status_code == 400 + assert response.data == {"codeMappingId": ["This code mapping is already in use."]} + + def test_codeowners_email_update(self) -> None: + data = {"raw": f"\n# cool stuff comment\n*.js {self.user.email}\n# good comment\n\n\n"} + with self.feature({"organizations:integrations-codeowners": True}): + response = self.client.put(self.url, data) + assert response.status_code == 200 + assert response.data["raw"] == "# cool stuff comment\n*.js admin@sentry.io\n# good comment" + + @patch("sentry.analytics.record") + def test_codeowners_max_raw_length(self, mock_record: MagicMock) -> None: + with mock.patch( + "sentry.issues.endpoints.serializers.MAX_RAW_LENGTH", len(self.data["raw"]) + 1 + ): + data = { + "raw": f"# cool stuff comment\n*.js {self.user.email}\n# good comment" + } + + with self.feature({"organizations:integrations-codeowners": True}): + response = self.client.put(self.url, data) + assert response.status_code == 400 + assert response.data == { + "raw": [ + ErrorDetail( + string=f"Raw needs to be <= {len(self.data['raw']) + 1} characters in length", + code="invalid", + ) + ] + } + + assert_last_analytics_event( + mock_record, + CodeOwnersMaxLengthExceeded( + organization_id=self.organization.id, + ), + ) + # Test that we allow this to be modified for existing large rows + code_mapping = self.create_code_mapping(project=self.project, stack_root="/") + codeowners = self.create_codeowners( + project=self.project, + code_mapping=code_mapping, + raw=f"*.py test@localhost #{self.team.slug}", + ) + url = reverse( + "sentry-api-0-project-codeowners-details", + kwargs={ + "organization_id_or_slug": self.organization.slug, + "project_id_or_slug": self.project.slug, + "codeowners_id": codeowners.id, + }, + ) + with self.feature({"organizations:integrations-codeowners": True}): + response = self.client.put(url, data) + + assert ProjectCodeOwners.objects.get(id=codeowners.id).raw == data.get("raw") diff --git a/tests/sentry/api/endpoints/test_project_codeowners.py b/tests/sentry/issues/endpoints/test_project_codeowners_index.py similarity index 100% rename from tests/sentry/api/endpoints/test_project_codeowners.py rename to tests/sentry/issues/endpoints/test_project_codeowners_index.py