Skip to content

Commit a7223b8

Browse files
authored
fix(seer): Don't override when bulk updating settings (#104918)
followup: #104912 Previously we were requiring both `enabledCodeReview` and `codeReviewTriggers` when bulk updating. Here we allow either or both, and don't override if a field isn't present.
1 parent 82754d4 commit a7223b8

File tree

2 files changed

+148
-40
lines changed

2 files changed

+148
-40
lines changed

src/sentry/integrations/api/endpoints/organization_repository_settings.py

Lines changed: 36 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -21,21 +21,19 @@ class RepositorySettingsSerializer(serializers.Serializer):
2121
help_text="List of repository IDs to update settings for. Maximum 1000 repositories.",
2222
)
2323
enabledCodeReview = serializers.BooleanField(
24-
required=True,
24+
required=False,
2525
help_text="Whether code review is enabled for these repositories",
2626
)
2727
codeReviewTriggers = serializers.ListField(
2828
child=serializers.ChoiceField(choices=[trigger.value for trigger in CodeReviewTrigger]),
29-
required=True,
29+
required=False,
3030
help_text="List of triggers for code review",
3131
)
3232

3333
def validate(self, data):
34-
if data.get("enabledCodeReview") and not data.get("codeReviewTriggers"):
34+
if "enabledCodeReview" not in data and "codeReviewTriggers" not in data:
3535
raise serializers.ValidationError(
36-
{
37-
"codeReviewTriggers": "At least one trigger is required when code review is enabled."
38-
}
36+
"At least one of 'enabledCodeReview' or 'codeReviewTriggers' must be provided."
3937
)
4038
return data
4139

@@ -64,39 +62,55 @@ def put(self, request: Request, organization: Organization) -> Response:
6462

6563
data = serializer.validated_data
6664
repository_ids = data["repositoryIds"]
67-
enabled_code_review = data["enabledCodeReview"]
68-
code_review_triggers = data["codeReviewTriggers"]
6965

70-
repositories = Repository.objects.filter(
71-
id__in=repository_ids,
72-
organization_id=organization.id,
66+
updated_enabled_code_review = data.get("enabledCodeReview")
67+
updated_code_review_triggers = data.get("codeReviewTriggers")
68+
69+
update_fields = []
70+
if updated_enabled_code_review is not None:
71+
update_fields.append("enabled_code_review")
72+
if updated_code_review_triggers is not None:
73+
update_fields.append("code_review_triggers")
74+
75+
repositories = list(
76+
Repository.objects.filter(
77+
id__in=repository_ids,
78+
organization_id=organization.id,
79+
)
7380
)
7481

75-
if repositories.count() != len(repository_ids):
82+
if len(repositories) != len(repository_ids):
7683
return Response(
7784
{"detail": "One or more repositories were not found in this organization."},
7885
status=400,
7986
)
8087

81-
settings_to_upsert = [
82-
RepositorySettings(
83-
repository=repo,
84-
enabled_code_review=enabled_code_review,
85-
code_review_triggers=code_review_triggers,
86-
)
87-
for repo in repositories
88-
]
88+
existing_settings = {
89+
setting.repository_id: setting
90+
for setting in RepositorySettings.objects.filter(repository_id__in=repository_ids)
91+
}
92+
93+
settings_to_upsert = []
94+
for repo in repositories:
95+
setting = existing_settings.get(repo.id) or RepositorySettings(repository=repo)
96+
97+
if updated_enabled_code_review is not None:
98+
setting.enabled_code_review = updated_enabled_code_review
99+
if updated_code_review_triggers is not None:
100+
setting.code_review_triggers = updated_code_review_triggers
101+
102+
settings_to_upsert.append(setting)
89103

90104
RepositorySettings.objects.bulk_create(
91105
settings_to_upsert,
92106
update_conflicts=True,
93107
unique_fields=["repository"],
94-
update_fields=["enabled_code_review", "code_review_triggers"],
108+
update_fields=update_fields,
95109
)
96110

97111
return Response(
98112
serialize(
99-
list(repositories),
113+
repositories,
100114
request.user,
101115
RepositorySerializer(expand=["settings"]),
102116
),

tests/sentry/integrations/api/endpoints/test_organization_repository_settings.py

Lines changed: 112 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -135,52 +135,146 @@ def test_missing_required_fields(self) -> None:
135135
)
136136

137137
assert response.status_code == 400, response.content
138-
assert "enabledCodeReview" in response.data
139-
assert "codeReviewTriggers" in response.data
138+
assert (
139+
"At least one of 'enabledCodeReview' or 'codeReviewTriggers' must be provided."
140+
in response.data["non_field_errors"]
141+
)
140142

141-
def test_enabled_code_review_requires_triggers(self) -> None:
143+
def test_partial_repository_ids_not_found(self) -> None:
142144
repo = Repository.objects.create(name="repo", organization_id=self.org.id)
143145

144146
response = self.client.put(
145147
self.url,
146148
data={
147-
"repositoryIds": [repo.id],
148-
"enabledCodeReview": True,
149+
"repositoryIds": [repo.id, 99999],
150+
"enabledCodeReview": False,
149151
"codeReviewTriggers": [],
150152
},
151153
format="json",
152154
)
153155

154156
assert response.status_code == 400, response.content
155-
assert "codeReviewTriggers" in response.data
157+
assert "not found" in response.data["detail"]
156158

157-
def test_disabled_code_review_allows_empty_triggers(self) -> None:
158-
repo = Repository.objects.create(name="repo", organization_id=self.org.id)
159+
def test_partial_bulk_update_enabled_code_review_preserves_triggers(self) -> None:
160+
repo1 = Repository.objects.create(name="repo1", organization_id=self.org.id)
161+
repo2 = Repository.objects.create(name="repo2", organization_id=self.org.id)
162+
163+
self.create_repository_settings(
164+
repository=repo1,
165+
enabled_code_review=False,
166+
code_review_triggers=[
167+
CodeReviewTrigger.ON_COMMAND_PHRASE,
168+
CodeReviewTrigger.ON_NEW_COMMIT,
169+
],
170+
)
171+
self.create_repository_settings(
172+
repository=repo2,
173+
enabled_code_review=False,
174+
code_review_triggers=[],
175+
)
159176

160177
response = self.client.put(
161178
self.url,
162179
data={
163-
"repositoryIds": [repo.id],
164-
"enabledCodeReview": False,
165-
"codeReviewTriggers": [],
180+
"repositoryIds": [repo1.id, repo2.id],
181+
"enabledCodeReview": True,
166182
},
167183
format="json",
168184
)
169185

170186
assert response.status_code == 200, response.content
171187

172-
def test_partial_repository_ids_not_found(self) -> None:
173-
repo = Repository.objects.create(name="repo", organization_id=self.org.id)
188+
settings1 = RepositorySettings.objects.get(repository=repo1)
189+
assert settings1.enabled_code_review is True
190+
assert settings1.code_review_triggers == ["on_command_phrase", "on_new_commit"]
191+
192+
settings2 = RepositorySettings.objects.get(repository=repo2)
193+
assert settings2.enabled_code_review is True
194+
assert settings2.code_review_triggers == []
195+
196+
def test_partial_bulk_update_code_review_triggers_preserves_enabled(self) -> None:
197+
repo1 = Repository.objects.create(name="repo1", organization_id=self.org.id)
198+
repo2 = Repository.objects.create(name="repo2", organization_id=self.org.id)
199+
200+
self.create_repository_settings(
201+
repository=repo1,
202+
enabled_code_review=True,
203+
code_review_triggers=[CodeReviewTrigger.ON_COMMAND_PHRASE],
204+
)
205+
self.create_repository_settings(
206+
repository=repo2,
207+
enabled_code_review=False,
208+
code_review_triggers=[],
209+
)
174210

175211
response = self.client.put(
176212
self.url,
177213
data={
178-
"repositoryIds": [repo.id, 99999],
179-
"enabledCodeReview": False,
180-
"codeReviewTriggers": [],
214+
"repositoryIds": [repo1.id, repo2.id],
215+
"codeReviewTriggers": [
216+
CodeReviewTrigger.ON_NEW_COMMIT,
217+
CodeReviewTrigger.ON_READY_FOR_REVIEW,
218+
],
181219
},
182220
format="json",
183221
)
184222

185-
assert response.status_code == 400, response.content
186-
assert "not found" in response.data["detail"]
223+
assert response.status_code == 200, response.content
224+
225+
settings1 = RepositorySettings.objects.get(repository=repo1)
226+
assert settings1.enabled_code_review is True
227+
assert settings1.code_review_triggers == ["on_new_commit", "on_ready_for_review"]
228+
229+
settings2 = RepositorySettings.objects.get(repository=repo2)
230+
assert settings2.enabled_code_review is False
231+
assert settings2.code_review_triggers == ["on_new_commit", "on_ready_for_review"]
232+
233+
def test_partial_bulk_create_enabled_code_review_uses_default(self) -> None:
234+
repo1 = Repository.objects.create(name="repo1", organization_id=self.org.id)
235+
repo2 = Repository.objects.create(name="repo2", organization_id=self.org.id)
236+
237+
response = self.client.put(
238+
self.url,
239+
data={
240+
"repositoryIds": [repo1.id, repo2.id],
241+
"enabledCodeReview": True,
242+
},
243+
format="json",
244+
)
245+
246+
assert response.status_code == 200, response.content
247+
248+
settings1 = RepositorySettings.objects.get(repository=repo1)
249+
assert settings1.enabled_code_review is True
250+
assert settings1.code_review_triggers == []
251+
252+
settings2 = RepositorySettings.objects.get(repository=repo2)
253+
assert settings2.enabled_code_review is True
254+
assert settings2.code_review_triggers == []
255+
256+
def test_partial_bulk_create_code_review_triggers_uses_default(self) -> None:
257+
repo1 = Repository.objects.create(name="repo1", organization_id=self.org.id)
258+
repo2 = Repository.objects.create(name="repo2", organization_id=self.org.id)
259+
260+
response = self.client.put(
261+
self.url,
262+
data={
263+
"repositoryIds": [repo1.id, repo2.id],
264+
"codeReviewTriggers": [
265+
CodeReviewTrigger.ON_NEW_COMMIT,
266+
CodeReviewTrigger.ON_READY_FOR_REVIEW,
267+
],
268+
},
269+
format="json",
270+
)
271+
272+
assert response.status_code == 200, response.content
273+
274+
settings1 = RepositorySettings.objects.get(repository=repo1)
275+
assert settings1.enabled_code_review is False
276+
assert settings1.code_review_triggers == ["on_new_commit", "on_ready_for_review"]
277+
278+
settings2 = RepositorySettings.objects.get(repository=repo2)
279+
assert settings2.enabled_code_review is False
280+
assert settings2.code_review_triggers == ["on_new_commit", "on_ready_for_review"]

0 commit comments

Comments
 (0)