From 5205e70ca26634985f7b04c8549310ccdd59247e Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Tue, 9 Dec 2025 21:24:06 -0800 Subject: [PATCH 1/6] feat(code-review): Handle auto enable code review on repoCreated --- src/sentry/models/repository.py | 41 +++++++- tests/sentry/models/test_repository.py | 136 +++++++++++++++++++++++++ 2 files changed, 176 insertions(+), 1 deletion(-) diff --git a/src/sentry/models/repository.py b/src/sentry/models/repository.py index 9bdf41f3cd9574..15a8ebb4f34bd3 100644 --- a/src/sentry/models/repository.py +++ b/src/sentry/models/repository.py @@ -4,7 +4,7 @@ from django.contrib.postgres.fields.array import ArrayField from django.db import models -from django.db.models.signals import pre_delete +from django.db.models.signals import post_save, pre_delete from django.utils import timezone from sentry.backup.dependencies import NormalizedModelName, get_model_name @@ -24,6 +24,8 @@ rename_on_pending_deletion, reset_pending_deletion_field_names, ) +from sentry.models import OrganizationOption +from sentry.models.repository_settings import RepositorySettings from sentry.organizations.services.organization.service import organization_service from sentry.signals import pending_delete from sentry.users.services.user import RpcUser @@ -186,3 +188,40 @@ def handle_exception(e): sender=Repository, weak=False, ) + + +def handle_auto_enable_code_review(instance: Repository) -> None: + """ + When a new repository is created, auto enable code review if applicable. + """ + + SUPPORTED_PROVIDERS = {"integrations:github"} + + if instance.provider not in SUPPORTED_PROVIDERS: + return + + if OrganizationOption.objects.get_value( + organization=instance.organization_id, + key="sentry:auto_enable_code_review", + default=False, + ): + triggers = OrganizationOption.objects.get_value( + organization=instance.organization_id, + key="sentry:default_code_review_triggers", + default=[], + ) + if not isinstance(triggers, list): + triggers = [] + + RepositorySettings.objects.get_or_create( + repository_id=instance.id, + defaults={"enabled_code_review": True, "code_review_triggers": triggers}, + ) + + +def on_repository_created(sender, instance, created, **kwargs): + if created: + handle_auto_enable_code_review(instance) + + +post_save.connect(on_repository_created, sender=Repository, weak=False) diff --git a/tests/sentry/models/test_repository.py b/tests/sentry/models/test_repository.py index c1c7f5d6b2c0e2..555ab2dc90af2f 100644 --- a/tests/sentry/models/test_repository.py +++ b/tests/sentry/models/test_repository.py @@ -1,6 +1,8 @@ from django.core import mail +from sentry.models import OrganizationOption from sentry.models.repository import Repository +from sentry.models.repository_settings import RepositorySettings from sentry.plugins.providers.dummy import DummyRepositoryProvider from sentry.testutils.cases import TestCase from sentry.testutils.helpers.features import with_feature @@ -64,3 +66,137 @@ def test_generate_delete_fail_email(self) -> None: assert msg.context["repo"] == self.repo assert msg.context["error_message"] == "Test error message" assert msg.context["provider_name"] == "Example" + + +class RepositoryCodeReviewSettingsTest(TestCase): + """Tests for the post_save signal that creates RepositorySettings on repository creation.""" + + def test_no_settings_created_when_auto_enable_disabled(self): + """When auto_enable_code_review is False, no RepositorySettings should be created.""" + org = self.create_organization() + + # Ensure auto_enable is not set (defaults to False) + repo = Repository.objects.create( + organization_id=org.id, + name="test-repo", + provider="integrations:github", + ) + + # No settings should be created + assert not RepositorySettings.objects.filter(repository=repo).exists() + + def test_settings_created_when_auto_enable_enabled(self): + """When auto_enable_code_review is True, RepositorySettings should be created.""" + org = self.create_organization() + + # Enable auto code review for the organization + OrganizationOption.objects.set_value( + organization=org, + key="sentry:auto_enable_code_review", + value=True, + ) + + repo = Repository.objects.create( + organization_id=org.id, + name="test-repo", + provider="integrations:github", + ) + + # Settings should be created with code review enabled + settings = RepositorySettings.objects.get(repository=repo) + assert settings.enabled_code_review is True + assert settings.code_review_triggers == [] + + def test_settings_created_with_triggers(self): + """When triggers are configured, they should be copied to RepositorySettings.""" + org = self.create_organization() + + # Enable auto code review with triggers + OrganizationOption.objects.set_value( + organization=org, + key="sentry:auto_enable_code_review", + value=True, + ) + OrganizationOption.objects.set_value( + organization=org, + key="sentry:default_code_review_triggers", + value=["on_new_commit", "on_ready_for_review"], + ) + + repo = Repository.objects.create( + organization_id=org.id, + name="test-repo", + provider="integrations:github", + ) + + settings = RepositorySettings.objects.get(repository=repo) + assert settings.enabled_code_review is True + assert settings.code_review_triggers == ["on_new_commit", "on_ready_for_review"] + + def test_no_settings_for_unsupported_provider(self): + """Repositories with unsupported providers should not get settings.""" + org = self.create_organization() + + # Enable auto code review + OrganizationOption.objects.set_value( + organization=org, + key="sentry:auto_enable_code_review", + value=True, + ) + + repo = Repository.objects.create( + organization_id=org.id, + name="test-repo", + provider="unsupported_provider", + ) + + # No settings should be created for unsupported provider + assert not RepositorySettings.objects.filter(repository=repo).exists() + + def test_invalid_triggers_type_defaults_to_empty_list(self): + """When triggers is not a list, it should default to empty list.""" + org = self.create_organization() + + OrganizationOption.objects.set_value( + organization=org, + key="sentry:auto_enable_code_review", + value=True, + ) + # Set invalid triggers type (string instead of list) + OrganizationOption.objects.set_value( + organization=org, + key="sentry:default_code_review_triggers", + value="invalid_string", + ) + + repo = Repository.objects.create( + organization_id=org.id, + name="test-repo", + provider="integrations:github", + ) + + settings = RepositorySettings.objects.get(repository=repo) + assert settings.code_review_triggers == [] + + def test_settings_not_duplicated_on_update(self): + """Updating a repository should not create duplicate settings.""" + org = self.create_organization() + + OrganizationOption.objects.set_value( + organization=org, + key="sentry:auto_enable_code_review", + value=True, + ) + + repo = Repository.objects.create( + organization_id=org.id, + name="test-repo", + provider="integrations:github", + ) + + # Update the repository + repo.name = "updated-repo" + repo.save() + + # Should still have exactly one settings record + assert RepositorySettings.objects.filter(repository=repo).count() == 1 From a7175c18fb52fbdb2016d1f3b847f6a37c616bf4 Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Wed, 10 Dec 2025 18:16:05 -0800 Subject: [PATCH 2/6] update trigger defaults --- src/sentry/models/repository.py | 8 ++++---- tests/sentry/models/test_repository.py | 7 ++++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/sentry/models/repository.py b/src/sentry/models/repository.py index 15a8ebb4f34bd3..d99f7eb0696d14 100644 --- a/src/sentry/models/repository.py +++ b/src/sentry/models/repository.py @@ -10,7 +10,7 @@ from sentry.backup.dependencies import NormalizedModelName, get_model_name from sentry.backup.sanitize import SanitizableField, Sanitizer from sentry.backup.scopes import RelocationScope -from sentry.constants import ObjectStatus +from sentry.constants import DEFAULT_CODE_REVIEW_TRIGGERS, ObjectStatus from sentry.db.models import ( BoundedBigIntegerField, BoundedPositiveIntegerField, @@ -24,8 +24,8 @@ rename_on_pending_deletion, reset_pending_deletion_field_names, ) -from sentry.models import OrganizationOption -from sentry.models.repository_settings import RepositorySettings +from sentry.models.options.organization_option import OrganizationOption +from sentry.models.repositorysettings import RepositorySettings from sentry.organizations.services.organization.service import organization_service from sentry.signals import pending_delete from sentry.users.services.user import RpcUser @@ -211,7 +211,7 @@ def handle_auto_enable_code_review(instance: Repository) -> None: default=[], ) if not isinstance(triggers, list): - triggers = [] + triggers = DEFAULT_CODE_REVIEW_TRIGGERS RepositorySettings.objects.get_or_create( repository_id=instance.id, diff --git a/tests/sentry/models/test_repository.py b/tests/sentry/models/test_repository.py index 555ab2dc90af2f..1dec141cd95183 100644 --- a/tests/sentry/models/test_repository.py +++ b/tests/sentry/models/test_repository.py @@ -1,8 +1,9 @@ from django.core import mail -from sentry.models import OrganizationOption +from sentry.constants import DEFAULT_CODE_REVIEW_TRIGGERS +from sentry.models.options.organization_option import OrganizationOption from sentry.models.repository import Repository -from sentry.models.repository_settings import RepositorySettings +from sentry.models.repositorysettings import RepositorySettings from sentry.plugins.providers.dummy import DummyRepositoryProvider from sentry.testutils.cases import TestCase from sentry.testutils.helpers.features import with_feature @@ -176,7 +177,7 @@ def test_invalid_triggers_type_defaults_to_empty_list(self): ) settings = RepositorySettings.objects.get(repository=repo) - assert settings.code_review_triggers == [] + assert settings.code_review_triggers == DEFAULT_CODE_REVIEW_TRIGGERS def test_settings_not_duplicated_on_update(self): """Updating a repository should not create duplicate settings.""" From adc5dfb86b6d89feac4f5a9e8e394c2969cef7e9 Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Thu, 11 Dec 2025 13:13:36 -0800 Subject: [PATCH 3/6] incorporate pr comments --- src/sentry/models/repository.py | 2 +- tests/sentry/models/test_repository.py | 15 --------------- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/src/sentry/models/repository.py b/src/sentry/models/repository.py index d99f7eb0696d14..aaada1f57eedec 100644 --- a/src/sentry/models/repository.py +++ b/src/sentry/models/repository.py @@ -208,7 +208,7 @@ def handle_auto_enable_code_review(instance: Repository) -> None: triggers = OrganizationOption.objects.get_value( organization=instance.organization_id, key="sentry:default_code_review_triggers", - default=[], + default=DEFAULT_CODE_REVIEW_TRIGGERS, ) if not isinstance(triggers, list): triggers = DEFAULT_CODE_REVIEW_TRIGGERS diff --git a/tests/sentry/models/test_repository.py b/tests/sentry/models/test_repository.py index 1dec141cd95183..5d29660de0ba28 100644 --- a/tests/sentry/models/test_repository.py +++ b/tests/sentry/models/test_repository.py @@ -73,24 +73,19 @@ class RepositoryCodeReviewSettingsTest(TestCase): """Tests for the post_save signal that creates RepositorySettings on repository creation.""" def test_no_settings_created_when_auto_enable_disabled(self): - """When auto_enable_code_review is False, no RepositorySettings should be created.""" org = self.create_organization() - # Ensure auto_enable is not set (defaults to False) repo = Repository.objects.create( organization_id=org.id, name="test-repo", provider="integrations:github", ) - # No settings should be created assert not RepositorySettings.objects.filter(repository=repo).exists() def test_settings_created_when_auto_enable_enabled(self): - """When auto_enable_code_review is True, RepositorySettings should be created.""" org = self.create_organization() - # Enable auto code review for the organization OrganizationOption.objects.set_value( organization=org, key="sentry:auto_enable_code_review", @@ -103,16 +98,13 @@ def test_settings_created_when_auto_enable_enabled(self): provider="integrations:github", ) - # Settings should be created with code review enabled settings = RepositorySettings.objects.get(repository=repo) assert settings.enabled_code_review is True assert settings.code_review_triggers == [] def test_settings_created_with_triggers(self): - """When triggers are configured, they should be copied to RepositorySettings.""" org = self.create_organization() - # Enable auto code review with triggers OrganizationOption.objects.set_value( organization=org, key="sentry:auto_enable_code_review", @@ -135,10 +127,8 @@ def test_settings_created_with_triggers(self): assert settings.code_review_triggers == ["on_new_commit", "on_ready_for_review"] def test_no_settings_for_unsupported_provider(self): - """Repositories with unsupported providers should not get settings.""" org = self.create_organization() - # Enable auto code review OrganizationOption.objects.set_value( organization=org, key="sentry:auto_enable_code_review", @@ -151,11 +141,9 @@ def test_no_settings_for_unsupported_provider(self): provider="unsupported_provider", ) - # No settings should be created for unsupported provider assert not RepositorySettings.objects.filter(repository=repo).exists() def test_invalid_triggers_type_defaults_to_empty_list(self): - """When triggers is not a list, it should default to empty list.""" org = self.create_organization() OrganizationOption.objects.set_value( @@ -180,7 +168,6 @@ def test_invalid_triggers_type_defaults_to_empty_list(self): assert settings.code_review_triggers == DEFAULT_CODE_REVIEW_TRIGGERS def test_settings_not_duplicated_on_update(self): - """Updating a repository should not create duplicate settings.""" org = self.create_organization() OrganizationOption.objects.set_value( @@ -195,9 +182,7 @@ def test_settings_not_duplicated_on_update(self): provider="integrations:github", ) - # Update the repository repo.name = "updated-repo" repo.save() - # Should still have exactly one settings record assert RepositorySettings.objects.filter(repository=repo).count() == 1 From fb4bcc0f40ae7537c5c25770bf71fa69fb6ea377 Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Thu, 11 Dec 2025 13:57:18 -0800 Subject: [PATCH 4/6] missed a spot --- tests/sentry/models/test_repository.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/sentry/models/test_repository.py b/tests/sentry/models/test_repository.py index 5d29660de0ba28..e53800c91f0350 100644 --- a/tests/sentry/models/test_repository.py +++ b/tests/sentry/models/test_repository.py @@ -100,7 +100,7 @@ def test_settings_created_when_auto_enable_enabled(self): settings = RepositorySettings.objects.get(repository=repo) assert settings.enabled_code_review is True - assert settings.code_review_triggers == [] + assert settings.code_review_triggers == DEFAULT_CODE_REVIEW_TRIGGERS def test_settings_created_with_triggers(self): org = self.create_organization() From a7331b7e9d226e3ec9974626a6844a9b706fb8ef Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Thu, 11 Dec 2025 15:33:59 -0800 Subject: [PATCH 5/6] override model save instead --- src/sentry/models/repository.py | 82 ++++++++++++++------------ tests/sentry/models/test_repository.py | 28 ++++++++- 2 files changed, 71 insertions(+), 39 deletions(-) diff --git a/src/sentry/models/repository.py b/src/sentry/models/repository.py index aaada1f57eedec..fca0bf21331084 100644 --- a/src/sentry/models/repository.py +++ b/src/sentry/models/repository.py @@ -1,10 +1,11 @@ from __future__ import annotations +import logging from typing import Any from django.contrib.postgres.fields.array import ArrayField from django.db import models -from django.db.models.signals import post_save, pre_delete +from django.db.models.signals import pre_delete from django.utils import timezone from sentry.backup.dependencies import NormalizedModelName, get_model_name @@ -31,6 +32,8 @@ from sentry.users.services.user import RpcUser from sentry.utils.email import MessageBuilder +_default_logger = logging.getLogger(__name__) + @region_silo_model class Repository(Model): @@ -149,6 +152,46 @@ def sanitize_relocation_json( sanitizer.set_string(json, SanitizableField(model_name, "provider")) json["fields"]["languages"] = "[]" + def save(self, *args: Any, **kwargs: Any) -> None: + is_new = self.pk is None + super().save(*args, **kwargs) + if is_new: + try: + self._handle_auto_enable_code_review() + except Exception: + _default_logger.exception( + "Failed to auto-enable code review on repository creation", + extra={"repository_id": self.id}, + ) + pass + + def _handle_auto_enable_code_review(self) -> None: + """ + When a new repository is created, auto enable code review if applicable. + """ + SUPPORTED_PROVIDERS = {"integrations:github"} + + if self.provider not in SUPPORTED_PROVIDERS: + return + + if OrganizationOption.objects.get_value( + organization=self.organization_id, + key="sentry:auto_enable_code_review", + default=False, + ): + triggers = OrganizationOption.objects.get_value( + organization=self.organization_id, + key="sentry:default_code_review_triggers", + default=DEFAULT_CODE_REVIEW_TRIGGERS, + ) + if not isinstance(triggers, list): + triggers = DEFAULT_CODE_REVIEW_TRIGGERS + + RepositorySettings.objects.get_or_create( + repository_id=self.id, + defaults={"enabled_code_review": True, "code_review_triggers": triggers}, + ) + def on_delete(instance, actor: RpcUser | None = None, **kwargs): """ @@ -188,40 +231,3 @@ def handle_exception(e): sender=Repository, weak=False, ) - - -def handle_auto_enable_code_review(instance: Repository) -> None: - """ - When a new repository is created, auto enable code review if applicable. - """ - - SUPPORTED_PROVIDERS = {"integrations:github"} - - if instance.provider not in SUPPORTED_PROVIDERS: - return - - if OrganizationOption.objects.get_value( - organization=instance.organization_id, - key="sentry:auto_enable_code_review", - default=False, - ): - triggers = OrganizationOption.objects.get_value( - organization=instance.organization_id, - key="sentry:default_code_review_triggers", - default=DEFAULT_CODE_REVIEW_TRIGGERS, - ) - if not isinstance(triggers, list): - triggers = DEFAULT_CODE_REVIEW_TRIGGERS - - RepositorySettings.objects.get_or_create( - repository_id=instance.id, - defaults={"enabled_code_review": True, "code_review_triggers": triggers}, - ) - - -def on_repository_created(sender, instance, created, **kwargs): - if created: - handle_auto_enable_code_review(instance) - - -post_save.connect(on_repository_created, sender=Repository, weak=False) diff --git a/tests/sentry/models/test_repository.py b/tests/sentry/models/test_repository.py index e53800c91f0350..2d2c8be65905b0 100644 --- a/tests/sentry/models/test_repository.py +++ b/tests/sentry/models/test_repository.py @@ -1,3 +1,5 @@ +from unittest.mock import patch + from django.core import mail from sentry.constants import DEFAULT_CODE_REVIEW_TRIGGERS @@ -70,7 +72,7 @@ def test_generate_delete_fail_email(self) -> None: class RepositoryCodeReviewSettingsTest(TestCase): - """Tests for the post_save signal that creates RepositorySettings on repository creation.""" + """Tests for auto-enabling code review settings on repository creation.""" def test_no_settings_created_when_auto_enable_disabled(self): org = self.create_organization() @@ -186,3 +188,27 @@ def test_settings_not_duplicated_on_update(self): repo.save() assert RepositorySettings.objects.filter(repository=repo).count() == 1 + + def test_repository_saved_even_if_auto_enable_fails(self): + org = self.create_organization() + + OrganizationOption.objects.set_value( + organization=org, + key="sentry:auto_enable_code_review", + value=True, + ) + + with patch.object( + Repository, + "_handle_auto_enable_code_review", + side_effect=Exception("Test exception"), + ): + repo = Repository.objects.create( + organization_id=org.id, + name="test-repo", + provider="integrations:github", + ) + + # Repository should still be saved + assert Repository.objects.filter(id=repo.id).exists() + assert repo.name == "test-repo" From 231ce861615eb09b2f878ab90bea57a2059c5bf9 Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Thu, 11 Dec 2025 16:29:12 -0800 Subject: [PATCH 6/6] incorporate pr changes --- src/sentry/models/repository.py | 17 +++------- tests/sentry/models/test_repository.py | 43 +++++++++++++++++++++----- 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/src/sentry/models/repository.py b/src/sentry/models/repository.py index fca0bf21331084..d609af39363baa 100644 --- a/src/sentry/models/repository.py +++ b/src/sentry/models/repository.py @@ -1,10 +1,9 @@ from __future__ import annotations -import logging from typing import Any from django.contrib.postgres.fields.array import ArrayField -from django.db import models +from django.db import models, router, transaction from django.db.models.signals import pre_delete from django.utils import timezone @@ -32,8 +31,6 @@ from sentry.users.services.user import RpcUser from sentry.utils.email import MessageBuilder -_default_logger = logging.getLogger(__name__) - @region_silo_model class Repository(Model): @@ -154,16 +151,10 @@ def sanitize_relocation_json( def save(self, *args: Any, **kwargs: Any) -> None: is_new = self.pk is None - super().save(*args, **kwargs) - if is_new: - try: + with transaction.atomic(router.db_for_write(Repository)): + super().save(*args, **kwargs) + if is_new: self._handle_auto_enable_code_review() - except Exception: - _default_logger.exception( - "Failed to auto-enable code review on repository creation", - extra={"repository_id": self.id}, - ) - pass def _handle_auto_enable_code_review(self) -> None: """ diff --git a/tests/sentry/models/test_repository.py b/tests/sentry/models/test_repository.py index 2d2c8be65905b0..70dae05805b801 100644 --- a/tests/sentry/models/test_repository.py +++ b/tests/sentry/models/test_repository.py @@ -1,5 +1,6 @@ from unittest.mock import patch +import pytest from django.core import mail from sentry.constants import DEFAULT_CODE_REVIEW_TRIGGERS @@ -189,7 +190,7 @@ def test_settings_not_duplicated_on_update(self): assert RepositorySettings.objects.filter(repository=repo).count() == 1 - def test_repository_saved_even_if_auto_enable_fails(self): + def test_transaction_rollback_on_auto_enable_failure(self): org = self.create_organization() OrganizationOption.objects.set_value( @@ -198,17 +199,43 @@ def test_repository_saved_even_if_auto_enable_fails(self): value=True, ) + initial_repo_count = Repository.objects.filter(organization_id=org.id).count() + with patch.object( Repository, "_handle_auto_enable_code_review", side_effect=Exception("Test exception"), ): - repo = Repository.objects.create( - organization_id=org.id, - name="test-repo", - provider="integrations:github", - ) + with pytest.raises(Exception, match="Test exception"): + Repository.objects.create( + organization_id=org.id, + name="test-repo", + provider="integrations:github", + ) + + # Neither Repository nor RepositorySettings should be saved due to transaction rollback + assert Repository.objects.filter(organization_id=org.id).count() == initial_repo_count + assert not RepositorySettings.objects.filter(repository__organization_id=org.id).exists() + + def test_both_repository_and_settings_saved_atomically(self): + org = self.create_organization() - # Repository should still be saved + OrganizationOption.objects.set_value( + organization=org, + key="sentry:auto_enable_code_review", + value=True, + ) + + repo = Repository.objects.create( + organization_id=org.id, + name="test-repo", + provider="integrations:github", + ) + + # Both should exist assert Repository.objects.filter(id=repo.id).exists() - assert repo.name == "test-repo" + assert RepositorySettings.objects.filter(repository=repo).exists() + + # Verify the settings are correct + settings = RepositorySettings.objects.get(repository=repo) + assert settings.enabled_code_review is True