Skip to content
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
3 changes: 2 additions & 1 deletion src/sentry/api/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 (
Expand Down
Empty file.
23 changes: 23 additions & 0 deletions src/sentry/issues/endpoints/bases/codeowners.py
Original file line number Diff line number Diff line change
@@ -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},
)
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,21 @@
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,
)
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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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


Expand Down Expand Up @@ -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",
)
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
191 changes: 191 additions & 0 deletions tests/sentry/issues/endpoints/test_project_codeowners_details.py
Original file line number Diff line number Diff line change
@@ -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("[email protected]", 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 [email protected]\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")
Loading