Skip to content

Commit dece80b

Browse files
JoshFergepriscilawebdev
authored andcommitted
ref: move codeowners endpoints into issues module (#97916)
- Move codeowners endpoints into the issues module - Get rid of the Mixin and use a base endpoint instead - Move serializers - Adjust tests and mocks
1 parent d8848b3 commit dece80b

11 files changed

+226
-41
lines changed

src/sentry/api/urls.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,8 @@
311311
from sentry.issues.endpoints.organization_issues_resolved_in_release import (
312312
OrganizationIssuesResolvedInReleaseEndpoint,
313313
)
314+
from sentry.issues.endpoints.project_codeowners_details import ProjectCodeOwnersDetailsEndpoint
315+
from sentry.issues.endpoints.project_codeowners_index import ProjectCodeOwnersEndpoint
314316
from sentry.issues.endpoints.project_grouping_configs import ProjectGroupingConfigsEndpoint
315317
from sentry.issues.endpoints.project_issues_resolved_in_release import (
316318
ProjectIssuesResolvedInReleaseEndpoint,
@@ -590,7 +592,6 @@
590592
from .endpoints.catchall import CatchallEndpoint
591593
from .endpoints.check_am2_compatibility import CheckAM2CompatibilityEndpoint
592594
from .endpoints.chunk import ChunkUploadEndpoint
593-
from .endpoints.codeowners import ProjectCodeOwnersDetailsEndpoint, ProjectCodeOwnersEndpoint
594595
from .endpoints.custom_rules import CustomRulesEndpoint
595596
from .endpoints.data_scrubbing_selector_suggestions import DataScrubbingSelectorSuggestionsEndpoint
596597
from .endpoints.debug_files import (

src/sentry/issues/endpoints/bases/__init__.py

Whitespace-only changes.
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
from rest_framework.request import Request
2+
3+
from sentry import features
4+
from sentry.api.bases.project import ProjectEndpoint
5+
from sentry.models.project import Project
6+
from sentry.utils import metrics
7+
8+
9+
class ProjectCodeOwnersBase(ProjectEndpoint):
10+
def has_feature(self, request: Request, project: Project) -> bool:
11+
return bool(
12+
features.has(
13+
"organizations:integrations-codeowners", project.organization, actor=request.user
14+
)
15+
)
16+
17+
def track_response_code(self, type: str, status: int | str) -> None:
18+
if type in ["create", "update"]:
19+
metrics.incr(
20+
f"codeowners.{type}.http_response",
21+
sample_rate=1.0,
22+
tags={"status": status},
23+
)

src/sentry/issues/endpoints/organization_group_search_view_details.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
from sentry.api.serializers.base import serialize
1313
from sentry.api.serializers.models.groupsearchview import GroupSearchViewSerializer
1414
from sentry.api.serializers.rest_framework.groupsearchview import ViewValidator
15-
from sentry.issues.endpoints.bases import GroupSearchViewPermission
15+
from sentry.issues.endpoints.bases.group_search_view import GroupSearchViewPermission
1616
from sentry.issues.endpoints.organization_group_search_views import pick_default_project
1717
from sentry.models.groupsearchview import GroupSearchView
1818
from sentry.models.groupsearchviewlastvisited import GroupSearchViewLastVisited

src/sentry/api/endpoints/codeowners/details.py renamed to src/sentry/issues/endpoints/project_codeowners_details.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,20 +13,19 @@
1313
from sentry.api.api_owners import ApiOwner
1414
from sentry.api.api_publish_status import ApiPublishStatus
1515
from sentry.api.base import region_silo_endpoint
16-
from sentry.api.bases.project import ProjectEndpoint
1716
from sentry.api.exceptions import ResourceDoesNotExist
1817
from sentry.api.serializers import serialize
1918
from sentry.api.serializers.models import projectcodeowners as projectcodeowners_serializers
19+
from sentry.issues.endpoints.bases.codeowners import ProjectCodeOwnersBase
20+
from sentry.issues.endpoints.serializers import ProjectCodeOwnerSerializer
2021
from sentry.models.project import Project
2122
from sentry.models.projectcodeowners import ProjectCodeOwners
2223

23-
from . import ProjectCodeOwnerSerializer, ProjectCodeOwnersMixin
24-
2524
logger = logging.getLogger(__name__)
2625

2726

2827
@region_silo_endpoint
29-
class ProjectCodeOwnersDetailsEndpoint(ProjectEndpoint, ProjectCodeOwnersMixin):
28+
class ProjectCodeOwnersDetailsEndpoint(ProjectCodeOwnersBase):
3029
owner = ApiOwner.ISSUES
3130
publish_status = {
3231
"DELETE": ApiPublishStatus.PRIVATE,

src/sentry/api/endpoints/codeowners/index.py renamed to src/sentry/issues/endpoints/project_codeowners_index.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,21 @@
99
from sentry.api.api_owners import ApiOwner
1010
from sentry.api.api_publish_status import ApiPublishStatus
1111
from sentry.api.base import region_silo_endpoint
12-
from sentry.api.bases.project import ProjectEndpoint
1312
from sentry.api.serializers import serialize
1413
from sentry.api.serializers.models import projectcodeowners as projectcodeowners_serializers
1514
from sentry.api.validators.project_codeowners import validate_codeowners_associations
15+
from sentry.issues.endpoints.bases.codeowners import ProjectCodeOwnersBase
16+
from sentry.issues.endpoints.serializers import ProjectCodeOwnerSerializer
1617
from sentry.issues.ownership.grammar import (
1718
convert_codeowners_syntax,
1819
create_schema_from_issue_owners,
1920
)
2021
from sentry.models.project import Project
2122
from sentry.models.projectcodeowners import ProjectCodeOwners
2223

23-
from . import ProjectCodeOwnerSerializer, ProjectCodeOwnersMixin
24-
2524

2625
@region_silo_endpoint
27-
class ProjectCodeOwnersEndpoint(ProjectEndpoint, ProjectCodeOwnersMixin):
26+
class ProjectCodeOwnersEndpoint(ProjectCodeOwnersBase):
2827
owner = ApiOwner.ISSUES
2928
publish_status = {
3029
"GET": ApiPublishStatus.PRIVATE,

src/sentry/api/endpoints/codeowners/__init__.py renamed to src/sentry/issues/endpoints/serializers.py

Lines changed: 1 addition & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,8 @@
66
import sentry_sdk
77
from rest_framework import serializers
88
from rest_framework.exceptions import ValidationError
9-
from rest_framework.request import Request
109

11-
from sentry import analytics, features
10+
from sentry import analytics
1211
from sentry.analytics.events.codeowners_max_length_exceeded import CodeOwnersMaxLengthExceeded
1312
from sentry.api.serializers.rest_framework.base import CamelSnakeModelSerializer
1413
from sentry.api.validators.project_codeowners import validate_codeowners_associations
@@ -17,9 +16,7 @@
1716
convert_codeowners_syntax,
1817
create_schema_from_issue_owners,
1918
)
20-
from sentry.models.project import Project
2119
from sentry.models.projectcodeowners import ProjectCodeOwners
22-
from sentry.utils import metrics
2320
from sentry.utils.codeowners import MAX_RAW_LENGTH
2421

2522

@@ -124,29 +121,3 @@ def update(
124121
setattr(instance, key, value)
125122
instance.save()
126123
return instance
127-
128-
129-
class ProjectCodeOwnersMixin:
130-
def has_feature(self, request: Request, project: Project) -> bool:
131-
return bool(
132-
features.has(
133-
"organizations:integrations-codeowners", project.organization, actor=request.user
134-
)
135-
)
136-
137-
def track_response_code(self, type: str, status: int | str) -> None:
138-
if type in ["create", "update"]:
139-
metrics.incr(
140-
f"codeowners.{type}.http_response",
141-
sample_rate=1.0,
142-
tags={"status": status},
143-
)
144-
145-
146-
from .details import ProjectCodeOwnersDetailsEndpoint
147-
from .index import ProjectCodeOwnersEndpoint
148-
149-
__all__ = (
150-
"ProjectCodeOwnersEndpoint",
151-
"ProjectCodeOwnersDetailsEndpoint",
152-
)

tests/sentry/api/endpoints/test_project_codeowners_details.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,8 @@ def test_codeowners_email_update(self) -> None:
146146
@patch("sentry.analytics.record")
147147
def test_codeowners_max_raw_length(self, mock_record: MagicMock) -> None:
148148
with mock.patch(
149-
"sentry.api.endpoints.codeowners.MAX_RAW_LENGTH", len(self.data["raw"]) + 1
149+
"sentry.issues.endpoints.serializers.MAX_RAW_LENGTH",
150+
len(self.data["raw"]) + 1,
150151
):
151152
data = {
152153
"raw": f"# cool stuff comment\n*.js {self.user.email}\n# good comment"
Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
from datetime import datetime, timezone
2+
from unittest import mock
3+
from unittest.mock import MagicMock, patch
4+
5+
from django.urls import reverse
6+
from rest_framework.exceptions import ErrorDetail
7+
8+
from sentry.analytics.events.codeowners_max_length_exceeded import CodeOwnersMaxLengthExceeded
9+
from sentry.models.projectcodeowners import ProjectCodeOwners
10+
from sentry.testutils.cases import APITestCase
11+
from sentry.testutils.helpers.analytics import assert_last_analytics_event
12+
13+
14+
class ProjectCodeOwnersDetailsEndpointTestCase(APITestCase):
15+
def setUp(self) -> None:
16+
self.user = self.create_user("[email protected]", is_superuser=True)
17+
18+
self.login_as(user=self.user)
19+
20+
self.team = self.create_team(
21+
organization=self.organization, slug="tiger-team", members=[self.user]
22+
)
23+
24+
self.project = self.project = self.create_project(
25+
organization=self.organization, teams=[self.team], slug="bengal"
26+
)
27+
28+
self.code_mapping = self.create_code_mapping(project=self.project)
29+
self.external_user = self.create_external_user(
30+
external_name="@NisanthanNanthakumar", integration=self.integration
31+
)
32+
self.external_team = self.create_external_team(integration=self.integration)
33+
self.data = {
34+
"raw": "docs/* @NisanthanNanthakumar @getsentry/ecosystem\n",
35+
}
36+
self.codeowners = self.create_codeowners(
37+
project=self.project, code_mapping=self.code_mapping
38+
)
39+
self.url = reverse(
40+
"sentry-api-0-project-codeowners-details",
41+
kwargs={
42+
"organization_id_or_slug": self.organization.slug,
43+
"project_id_or_slug": self.project.slug,
44+
"codeowners_id": self.codeowners.id,
45+
},
46+
)
47+
48+
# Mock the external HTTP request to prevent real network calls
49+
self.codeowner_patcher = patch(
50+
"sentry.integrations.source_code_management.repository.RepositoryIntegration.get_codeowner_file",
51+
return_value={
52+
"html_url": "https://github.com/test/CODEOWNERS",
53+
"filepath": "CODEOWNERS",
54+
"raw": "test content",
55+
},
56+
)
57+
self.codeowner_mock = self.codeowner_patcher.start()
58+
self.addCleanup(self.codeowner_patcher.stop)
59+
60+
def test_basic_delete(self) -> None:
61+
with self.feature({"organizations:integrations-codeowners": True}):
62+
response = self.client.delete(self.url)
63+
assert response.status_code == 204
64+
assert not ProjectCodeOwners.objects.filter(id=str(self.codeowners.id)).exists()
65+
66+
@patch("django.utils.timezone.now")
67+
def test_basic_update(self, mock_timezone_now: MagicMock) -> None:
68+
self.create_external_team(external_name="@getsentry/frontend", integration=self.integration)
69+
self.create_external_team(external_name="@getsentry/docs", integration=self.integration)
70+
date = datetime(2023, 10, 3, tzinfo=timezone.utc)
71+
mock_timezone_now.return_value = date
72+
raw = "\n# cool stuff comment\n*.js @getsentry/frontend @NisanthanNanthakumar\n# good comment\n\n\n docs/* @getsentry/docs @getsentry/ecosystem\n\n"
73+
data = {
74+
"raw": raw,
75+
}
76+
77+
# Reset call count to verify this specific test's calls
78+
self.codeowner_mock.reset_mock()
79+
80+
with self.feature({"organizations:integrations-codeowners": True}):
81+
response = self.client.put(self.url, data)
82+
83+
# Verify our mock was called instead of making real HTTP requests
84+
assert (
85+
self.codeowner_mock.called
86+
), "Mock should have been called - no external HTTP requests made"
87+
88+
assert response.status_code == 200
89+
assert response.data["id"] == str(self.codeowners.id)
90+
assert response.data["raw"] == raw.strip()
91+
codeowner = ProjectCodeOwners.objects.filter(id=self.codeowners.id)[0]
92+
assert codeowner.date_updated == date
93+
94+
def test_wrong_codeowners_id(self) -> None:
95+
self.url = reverse(
96+
"sentry-api-0-project-codeowners-details",
97+
kwargs={
98+
"organization_id_or_slug": self.organization.slug,
99+
"project_id_or_slug": self.project.slug,
100+
"codeowners_id": 1000,
101+
},
102+
)
103+
with self.feature({"organizations:integrations-codeowners": True}):
104+
response = self.client.put(self.url, self.data)
105+
assert response.status_code == 404
106+
assert response.data == {"detail": "The requested resource does not exist"}
107+
108+
def test_missing_external_associations_update(self) -> None:
109+
data = {
110+
"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"
111+
}
112+
with self.feature({"organizations:integrations-codeowners": True}):
113+
response = self.client.put(self.url, data)
114+
assert response.status_code == 200
115+
assert response.data["id"] == str(self.codeowners.id)
116+
assert response.data["codeMappingId"] == str(self.code_mapping.id)
117+
118+
errors = response.data["errors"]
119+
assert set(errors["missing_external_teams"]) == {"@getsentry/frontend", "@getsentry/docs"}
120+
assert set(errors["missing_external_users"]) == {"@AnotherUser"}
121+
assert errors["missing_user_emails"] == []
122+
assert errors["teams_without_access"] == []
123+
assert errors["users_without_access"] == []
124+
125+
def test_invalid_code_mapping_id_update(self) -> None:
126+
with self.feature({"organizations:integrations-codeowners": True}):
127+
response = self.client.put(self.url, {"codeMappingId": 500})
128+
assert response.status_code == 400
129+
assert response.data == {"codeMappingId": ["This code mapping does not exist."]}
130+
131+
def test_no_duplicates_code_mappings(self) -> None:
132+
new_code_mapping = self.create_code_mapping(project=self.project, stack_root="blah")
133+
self.create_codeowners(project=self.project, code_mapping=new_code_mapping)
134+
with self.feature({"organizations:integrations-codeowners": True}):
135+
response = self.client.put(self.url, {"codeMappingId": new_code_mapping.id})
136+
assert response.status_code == 400
137+
assert response.data == {"codeMappingId": ["This code mapping is already in use."]}
138+
139+
def test_codeowners_email_update(self) -> None:
140+
data = {"raw": f"\n# cool stuff comment\n*.js {self.user.email}\n# good comment\n\n\n"}
141+
with self.feature({"organizations:integrations-codeowners": True}):
142+
response = self.client.put(self.url, data)
143+
assert response.status_code == 200
144+
assert response.data["raw"] == "# cool stuff comment\n*.js [email protected]\n# good comment"
145+
146+
@patch("sentry.analytics.record")
147+
def test_codeowners_max_raw_length(self, mock_record: MagicMock) -> None:
148+
with mock.patch(
149+
"sentry.issues.endpoints.serializers.MAX_RAW_LENGTH", len(self.data["raw"]) + 1
150+
):
151+
data = {
152+
"raw": f"# cool stuff comment\n*.js {self.user.email}\n# good comment"
153+
}
154+
155+
with self.feature({"organizations:integrations-codeowners": True}):
156+
response = self.client.put(self.url, data)
157+
assert response.status_code == 400
158+
assert response.data == {
159+
"raw": [
160+
ErrorDetail(
161+
string=f"Raw needs to be <= {len(self.data['raw']) + 1} characters in length",
162+
code="invalid",
163+
)
164+
]
165+
}
166+
167+
assert_last_analytics_event(
168+
mock_record,
169+
CodeOwnersMaxLengthExceeded(
170+
organization_id=self.organization.id,
171+
),
172+
)
173+
# Test that we allow this to be modified for existing large rows
174+
code_mapping = self.create_code_mapping(project=self.project, stack_root="/")
175+
codeowners = self.create_codeowners(
176+
project=self.project,
177+
code_mapping=code_mapping,
178+
raw=f"*.py test@localhost #{self.team.slug}",
179+
)
180+
url = reverse(
181+
"sentry-api-0-project-codeowners-details",
182+
kwargs={
183+
"organization_id_or_slug": self.organization.slug,
184+
"project_id_or_slug": self.project.slug,
185+
"codeowners_id": codeowners.id,
186+
},
187+
)
188+
with self.feature({"organizations:integrations-codeowners": True}):
189+
response = self.client.put(url, data)
190+
191+
assert ProjectCodeOwners.objects.get(id=codeowners.id).raw == data.get("raw")

0 commit comments

Comments
 (0)