From dea835f1a81985a73c7745b688a6cc2fe231676c Mon Sep 17 00:00:00 2001 From: srest2021 Date: Thu, 8 Jan 2026 15:19:02 -0800 Subject: [PATCH 01/16] only remove from org option --- migrations_lockfile.txt | 2 +- src/sentry/constants.py | 4 ++ .../core/endpoints/organization_details.py | 6 ++ .../1016_remove_on_command_phrase_trigger.py | 58 +++++++++++++++++++ .../overwatch/endpoints/overwatch_rpc.py | 4 +- .../overwatch/endpoints/test_overwatch_rpc.py | 6 +- 6 files changed, 74 insertions(+), 6 deletions(-) create mode 100644 src/sentry/migrations/1016_remove_on_command_phrase_trigger.py diff --git a/migrations_lockfile.txt b/migrations_lockfile.txt index abfcae6343459d..414f4f3f26fe29 100644 --- a/migrations_lockfile.txt +++ b/migrations_lockfile.txt @@ -31,7 +31,7 @@ releases: 0004_cleanup_failed_safe_deletes replays: 0007_organizationmember_replay_access -sentry: 1015_backfill_self_hosted_sentry_app_emails +sentry: 1016_remove_on_command_phrase_trigger social_auth: 0003_social_auth_json_field diff --git a/src/sentry/constants.py b/src/sentry/constants.py index 596ccad0267b57..ae928067761799 100644 --- a/src/sentry/constants.py +++ b/src/sentry/constants.py @@ -725,6 +725,10 @@ class InsightModules(Enum): AUTO_ENABLE_CODE_REVIEW = False # Seer Org level default for code review triggers DEFAULT_CODE_REVIEW_TRIGGERS: list[str] = [ + "on_ready_for_review", + "on_new_commit", +] +SEER_DEFAULT_CODE_REVIEW_TRIGGERS: list[str] = [ "on_command_phrase", "on_ready_for_review", "on_new_commit", diff --git a/src/sentry/core/endpoints/organization_details.py b/src/sentry/core/endpoints/organization_details.py index c0e31d74c65bd8..3e3f48e600c243 100644 --- a/src/sentry/core/endpoints/organization_details.py +++ b/src/sentry/core/endpoints/organization_details.py @@ -1489,3 +1489,9 @@ def old_value(model, field_name): if value is DEFERRED: return None return model.__data.get(field_name) + if model.__data is UNSAVED: + return None + value = model.__data.get(field_name) + if value is DEFERRED: + return None + return model.__data.get(field_name) diff --git a/src/sentry/migrations/1016_remove_on_command_phrase_trigger.py b/src/sentry/migrations/1016_remove_on_command_phrase_trigger.py new file mode 100644 index 00000000000000..6393d0eae4be8e --- /dev/null +++ b/src/sentry/migrations/1016_remove_on_command_phrase_trigger.py @@ -0,0 +1,58 @@ +# Generated by Django 5.2.8 on 2026-01-08 18:18 + + +from django.db import migrations +from django.db.backends.base.schema import BaseDatabaseSchemaEditor +from django.db.migrations.state import StateApps + +from sentry.new_migrations.migrations import CheckedMigration + + +def remove_on_command_phrase_from_triggers( + apps: StateApps, schema_editor: BaseDatabaseSchemaEditor +) -> None: + """Remove 'on_command_phrase' from organization defaultCodeReviewTriggers.""" + OrganizationOption = apps.get_model("sentry", "OrganizationOption") + + org_options_to_update = OrganizationOption.objects.filter( + key="sentry:default_code_review_triggers" + ) + + for org_option in org_options_to_update: + if ( + org_option.value + and isinstance(org_option.value, list) + and "on_command_phrase" in org_option.value + ): + org_option.value = [ + trigger for trigger in org_option.value if trigger != "on_command_phrase" + ] + org_option.save(update_fields=["value"]) + + +class Migration(CheckedMigration): + # This flag is used to mark that a migration shouldn't be automatically run in production. + # This should only be used for operations where it's safe to run the migration after your + # code has deployed. So this should not be used for most operations that alter the schema + # of a table. + # Here are some things that make sense to mark as post deployment: + # - Large data migrations. Typically we want these to be run manually so that they can be + # monitored and not block the deploy for a long period of time while they run. + # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to + # run this outside deployments so that we don't block them. Note that while adding an index + # is a schema change, it's completely safe to run the operation after the code has deployed. + # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment + + is_post_deployment = False + + dependencies = [ + ("sentry", "1015_backfill_self_hosted_sentry_app_emails"), + ] + + operations = [ + migrations.RunPython( + remove_on_command_phrase_from_triggers, + reverse_code=migrations.RunPython.noop, + hints={"tables": ["sentry_organizationoptions"]}, + ) + ] diff --git a/src/sentry/overwatch/endpoints/overwatch_rpc.py b/src/sentry/overwatch/endpoints/overwatch_rpc.py index 4e7962078513a6..c4569b606b4ab3 100644 --- a/src/sentry/overwatch/endpoints/overwatch_rpc.py +++ b/src/sentry/overwatch/endpoints/overwatch_rpc.py @@ -16,7 +16,7 @@ from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.authentication import AuthenticationSiloLimit, StandardAuthentication from sentry.api.base import Endpoint, region_silo_endpoint -from sentry.constants import DEFAULT_CODE_REVIEW_TRIGGERS, ObjectStatus +from sentry.constants import SEER_DEFAULT_CODE_REVIEW_TRIGGERS, ObjectStatus from sentry.integrations.services.integration import integration_service from sentry.models.organization import Organization from sentry.models.repository import Repository @@ -216,7 +216,7 @@ def get(self, request: Request) -> Response: return Response( { "enabledCodeReview": True, - "codeReviewTriggers": DEFAULT_CODE_REVIEW_TRIGGERS, + "codeReviewTriggers": SEER_DEFAULT_CODE_REVIEW_TRIGGERS, } ) diff --git a/tests/sentry/overwatch/endpoints/test_overwatch_rpc.py b/tests/sentry/overwatch/endpoints/test_overwatch_rpc.py index bbd6a00c863195..b467f573874c2f 100644 --- a/tests/sentry/overwatch/endpoints/test_overwatch_rpc.py +++ b/tests/sentry/overwatch/endpoints/test_overwatch_rpc.py @@ -5,7 +5,7 @@ from django.urls import reverse -from sentry.constants import DEFAULT_CODE_REVIEW_TRIGGERS, DataCategory, ObjectStatus +from sentry.constants import SEER_DEFAULT_CODE_REVIEW_TRIGGERS, DataCategory, ObjectStatus from sentry.models.organizationcontributors import OrganizationContributors from sentry.models.repositorysettings import RepositorySettings from sentry.prevent.models import PreventAIConfiguration @@ -574,7 +574,7 @@ def test_returns_enabled_with_default_triggers_when_code_review_beta_flag(self): assert resp.status_code == 200 assert resp.data == { "enabledCodeReview": True, - "codeReviewTriggers": DEFAULT_CODE_REVIEW_TRIGGERS, + "codeReviewTriggers": SEER_DEFAULT_CODE_REVIEW_TRIGGERS, } @patch( @@ -598,7 +598,7 @@ def test_returns_enabled_with_default_triggers_when_pr_review_test_generation_en assert resp.status_code == 200 assert resp.data == { "enabledCodeReview": True, - "codeReviewTriggers": DEFAULT_CODE_REVIEW_TRIGGERS, + "codeReviewTriggers": SEER_DEFAULT_CODE_REVIEW_TRIGGERS, } @patch( From 639391af1734b67e1407f2798c62494e8ecb118c Mon Sep 17 00:00:00 2001 From: srest2021 Date: Thu, 8 Jan 2026 15:19:44 -0800 Subject: [PATCH 02/16] fix cursor slop --- src/sentry/core/endpoints/organization_details.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/sentry/core/endpoints/organization_details.py b/src/sentry/core/endpoints/organization_details.py index 3e3f48e600c243..c0e31d74c65bd8 100644 --- a/src/sentry/core/endpoints/organization_details.py +++ b/src/sentry/core/endpoints/organization_details.py @@ -1489,9 +1489,3 @@ def old_value(model, field_name): if value is DEFERRED: return None return model.__data.get(field_name) - if model.__data is UNSAVED: - return None - value = model.__data.get(field_name) - if value is DEFERRED: - return None - return model.__data.get(field_name) From 2332ff1edb39c96d9c0f8d00ef9d71ec6d9ca931 Mon Sep 17 00:00:00 2001 From: srest2021 Date: Thu, 8 Jan 2026 15:22:21 -0800 Subject: [PATCH 03/16] fix order --- src/sentry/constants.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sentry/constants.py b/src/sentry/constants.py index ae928067761799..43275cd982e7b8 100644 --- a/src/sentry/constants.py +++ b/src/sentry/constants.py @@ -724,12 +724,12 @@ class InsightModules(Enum): # Seer Org level setting to automatically enable code review for all new GitHub repo's that become connected AUTO_ENABLE_CODE_REVIEW = False # Seer Org level default for code review triggers -DEFAULT_CODE_REVIEW_TRIGGERS: list[str] = [ +SEER_DEFAULT_CODE_REVIEW_TRIGGERS: list[str] = [ + "on_command_phrase", "on_ready_for_review", "on_new_commit", ] -SEER_DEFAULT_CODE_REVIEW_TRIGGERS: list[str] = [ - "on_command_phrase", +DEFAULT_CODE_REVIEW_TRIGGERS: list[str] = [ "on_ready_for_review", "on_new_commit", ] From 628c7fa82748747c23bc7f2c4750d309c22d8367 Mon Sep 17 00:00:00 2001 From: srest2021 Date: Thu, 8 Jan 2026 15:28:51 -0800 Subject: [PATCH 04/16] remove from serializer --- src/sentry/apidocs/examples/organization_examples.py | 1 - src/sentry/core/endpoints/organization_details.py | 4 +--- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/sentry/apidocs/examples/organization_examples.py b/src/sentry/apidocs/examples/organization_examples.py index b274a069fd0cab..d9c7c23532b425 100644 --- a/src/sentry/apidocs/examples/organization_examples.py +++ b/src/sentry/apidocs/examples/organization_examples.py @@ -297,7 +297,6 @@ class OrganizationExamples: "defaultAutofixAutomationTuning": AutofixAutomationTuningSettings.OFF, "autoEnableCodeReview": True, "defaultCodeReviewTriggers": [ - "on_command_phrase", "on_ready_for_review", "on_new_commit", ], diff --git a/src/sentry/core/endpoints/organization_details.py b/src/sentry/core/endpoints/organization_details.py index c0e31d74c65bd8..f32ac237878f0b 100644 --- a/src/sentry/core/endpoints/organization_details.py +++ b/src/sentry/core/endpoints/organization_details.py @@ -369,9 +369,7 @@ class OrganizationSerializer(BaseOrganizationSerializer): autoOpenPrs = serializers.BooleanField(required=False) autoEnableCodeReview = serializers.BooleanField(required=False) defaultCodeReviewTriggers = serializers.ListField( - child=serializers.ChoiceField( - choices=["on_command_phrase", "on_ready_for_review", "on_new_commit"] - ), + child=serializers.ChoiceField(choices=["on_ready_for_review", "on_new_commit"]), required=False, allow_empty=True, help_text="The default code review triggers for new repositories.", From 411a245e2ab1d04d377c94f6062492fcd11ecd92 Mon Sep 17 00:00:00 2001 From: srest2021 Date: Thu, 8 Jan 2026 15:33:31 -0800 Subject: [PATCH 05/16] make post deployment --- src/sentry/migrations/1016_remove_on_command_phrase_trigger.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/migrations/1016_remove_on_command_phrase_trigger.py b/src/sentry/migrations/1016_remove_on_command_phrase_trigger.py index 6393d0eae4be8e..08f896d5cf5777 100644 --- a/src/sentry/migrations/1016_remove_on_command_phrase_trigger.py +++ b/src/sentry/migrations/1016_remove_on_command_phrase_trigger.py @@ -43,7 +43,7 @@ class Migration(CheckedMigration): # is a schema change, it's completely safe to run the operation after the code has deployed. # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment - is_post_deployment = False + is_post_deployment = True dependencies = [ ("sentry", "1015_backfill_self_hosted_sentry_app_emails"), From 6e410ee7ac7d726d1a6ada6dccf9192bb197e8c7 Mon Sep 17 00:00:00 2001 From: srest2021 Date: Thu, 8 Jan 2026 15:47:15 -0800 Subject: [PATCH 06/16] dont need to make this a constant --- src/sentry/constants.py | 5 ----- src/sentry/overwatch/endpoints/overwatch_rpc.py | 4 ++-- tests/sentry/overwatch/endpoints/test_overwatch_rpc.py | 6 +++--- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/sentry/constants.py b/src/sentry/constants.py index 43275cd982e7b8..035e388d62ce66 100644 --- a/src/sentry/constants.py +++ b/src/sentry/constants.py @@ -724,11 +724,6 @@ class InsightModules(Enum): # Seer Org level setting to automatically enable code review for all new GitHub repo's that become connected AUTO_ENABLE_CODE_REVIEW = False # Seer Org level default for code review triggers -SEER_DEFAULT_CODE_REVIEW_TRIGGERS: list[str] = [ - "on_command_phrase", - "on_ready_for_review", - "on_new_commit", -] DEFAULT_CODE_REVIEW_TRIGGERS: list[str] = [ "on_ready_for_review", "on_new_commit", diff --git a/src/sentry/overwatch/endpoints/overwatch_rpc.py b/src/sentry/overwatch/endpoints/overwatch_rpc.py index c4569b606b4ab3..e464c0c0baeb0a 100644 --- a/src/sentry/overwatch/endpoints/overwatch_rpc.py +++ b/src/sentry/overwatch/endpoints/overwatch_rpc.py @@ -16,7 +16,7 @@ from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.authentication import AuthenticationSiloLimit, StandardAuthentication from sentry.api.base import Endpoint, region_silo_endpoint -from sentry.constants import SEER_DEFAULT_CODE_REVIEW_TRIGGERS, ObjectStatus +from sentry.constants import DEFAULT_CODE_REVIEW_TRIGGERS, ObjectStatus from sentry.integrations.services.integration import integration_service from sentry.models.organization import Organization from sentry.models.repository import Repository @@ -216,7 +216,7 @@ def get(self, request: Request) -> Response: return Response( { "enabledCodeReview": True, - "codeReviewTriggers": SEER_DEFAULT_CODE_REVIEW_TRIGGERS, + "codeReviewTriggers": DEFAULT_CODE_REVIEW_TRIGGERS + ["on_command_phrase"], } ) diff --git a/tests/sentry/overwatch/endpoints/test_overwatch_rpc.py b/tests/sentry/overwatch/endpoints/test_overwatch_rpc.py index b467f573874c2f..56901a501b036c 100644 --- a/tests/sentry/overwatch/endpoints/test_overwatch_rpc.py +++ b/tests/sentry/overwatch/endpoints/test_overwatch_rpc.py @@ -5,7 +5,7 @@ from django.urls import reverse -from sentry.constants import SEER_DEFAULT_CODE_REVIEW_TRIGGERS, DataCategory, ObjectStatus +from sentry.constants import DEFAULT_CODE_REVIEW_TRIGGERS, DataCategory, ObjectStatus from sentry.models.organizationcontributors import OrganizationContributors from sentry.models.repositorysettings import RepositorySettings from sentry.prevent.models import PreventAIConfiguration @@ -574,7 +574,7 @@ def test_returns_enabled_with_default_triggers_when_code_review_beta_flag(self): assert resp.status_code == 200 assert resp.data == { "enabledCodeReview": True, - "codeReviewTriggers": SEER_DEFAULT_CODE_REVIEW_TRIGGERS, + "codeReviewTriggers": DEFAULT_CODE_REVIEW_TRIGGERS + ["on_command_phrase"], } @patch( @@ -598,7 +598,7 @@ def test_returns_enabled_with_default_triggers_when_pr_review_test_generation_en assert resp.status_code == 200 assert resp.data == { "enabledCodeReview": True, - "codeReviewTriggers": SEER_DEFAULT_CODE_REVIEW_TRIGGERS, + "codeReviewTriggers": DEFAULT_CODE_REVIEW_TRIGGERS + ["on_command_phrase"], } @patch( From 286fc403a5ac291fa85bb319fd5626365033d35f Mon Sep 17 00:00:00 2001 From: srest2021 Date: Thu, 8 Jan 2026 15:56:07 -0800 Subject: [PATCH 07/16] remove from repositorysettings --- src/sentry/models/repositorysettings.py | 1 - .../overwatch/endpoints/overwatch_rpc.py | 4 ++- src/sentry/seer/code_review/utils.py | 9 ++++-- .../code_review/webhooks/issue_comment.py | 5 ++-- .../seer/code_review/webhooks/pull_request.py | 9 +++--- src/sentry/seer/code_review/webhooks/task.py | 5 ++-- .../test_organization_repository_settings.py | 8 ++--- .../sentry/models/test_repositorysettings.py | 6 ++-- .../overwatch/endpoints/test_overwatch_rpc.py | 30 ++++++++++--------- .../sentry/seer/code_review/test_preflight.py | 4 +-- tests/sentry/seer/code_review/test_utils.py | 14 ++++----- 11 files changed, 50 insertions(+), 45 deletions(-) diff --git a/src/sentry/models/repositorysettings.py b/src/sentry/models/repositorysettings.py index 5a80c398cf21ab..2a084031579d47 100644 --- a/src/sentry/models/repositorysettings.py +++ b/src/sentry/models/repositorysettings.py @@ -11,7 +11,6 @@ class CodeReviewTrigger(StrEnum): - ON_COMMAND_PHRASE = "on_command_phrase" ON_NEW_COMMIT = "on_new_commit" ON_READY_FOR_REVIEW = "on_ready_for_review" diff --git a/src/sentry/overwatch/endpoints/overwatch_rpc.py b/src/sentry/overwatch/endpoints/overwatch_rpc.py index e464c0c0baeb0a..3435c9cf77ff5a 100644 --- a/src/sentry/overwatch/endpoints/overwatch_rpc.py +++ b/src/sentry/overwatch/endpoints/overwatch_rpc.py @@ -227,10 +227,12 @@ def get(self, request: Request) -> Response: } ) + triggers = set(repo_settings.code_review_triggers) + triggers.add("on_command_phrase") return Response( { "enabledCodeReview": repo_settings.enabled_code_review, - "codeReviewTriggers": repo_settings.code_review_triggers, + "codeReviewTriggers": list(triggers), } ) diff --git a/src/sentry/seer/code_review/utils.py b/src/sentry/seer/code_review/utils.py index d6194232c1b9fb..3bba9bdd1ead90 100644 --- a/src/sentry/seer/code_review/utils.py +++ b/src/sentry/seer/code_review/utils.py @@ -11,7 +11,6 @@ from sentry.integrations.services.integration.model import RpcIntegration from sentry.models.organization import Organization from sentry.models.repository import Repository -from sentry.models.repositorysettings import CodeReviewTrigger from sentry.net.http import connection_from_url from sentry.seer.signed_seer_api import make_signed_seer_api_request @@ -27,6 +26,12 @@ class ClientError(Exception): pass +class SeerCodeReviewTrigger(StrEnum): + ON_COMMAND_PHRASE = "on_command_phrase" + ON_NEW_COMMIT = "on_new_commit" + ON_READY_FOR_REVIEW = "on_ready_for_review" + + # These values need to match the value defined in the Seer API. class SeerEndpoint(StrEnum): # https://github.com/getsentry/seer/blob/main/src/seer/routes/automation_request.py#L57 @@ -194,7 +199,7 @@ def transform_webhook_to_codegen_request( organization: Organization, repo: Repository, target_commit_sha: str, - trigger: CodeReviewTrigger, + trigger: SeerCodeReviewTrigger, ) -> dict[str, Any] | None: """ Transform a GitHub webhook payload into CodecovTaskRequest format for Seer. diff --git a/src/sentry/seer/code_review/webhooks/issue_comment.py b/src/sentry/seer/code_review/webhooks/issue_comment.py index 3a6b9b96a8cdb7..82c190e224ba72 100644 --- a/src/sentry/seer/code_review/webhooks/issue_comment.py +++ b/src/sentry/seer/code_review/webhooks/issue_comment.py @@ -14,10 +14,9 @@ from sentry.integrations.services.integration import RpcIntegration from sentry.models.organization import Organization from sentry.models.repository import Repository -from sentry.models.repositorysettings import CodeReviewTrigger from sentry.utils import metrics -from ..utils import _get_target_commit_sha +from ..utils import SeerCodeReviewTrigger, _get_target_commit_sha from .config import get_direct_to_seer_gh_orgs logger = logging.getLogger(__name__) @@ -118,5 +117,5 @@ def handle_issue_comment_event( organization=organization, repo=repo, target_commit_sha=target_commit_sha, - trigger=CodeReviewTrigger.ON_COMMAND_PHRASE, + trigger=SeerCodeReviewTrigger.ON_COMMAND_PHRASE, ) diff --git a/src/sentry/seer/code_review/webhooks/pull_request.py b/src/sentry/seer/code_review/webhooks/pull_request.py index f48e2c9c406676..a3733a99e693fe 100644 --- a/src/sentry/seer/code_review/webhooks/pull_request.py +++ b/src/sentry/seer/code_review/webhooks/pull_request.py @@ -14,10 +14,9 @@ from sentry.integrations.services.integration import RpcIntegration from sentry.models.organization import Organization from sentry.models.repository import Repository -from sentry.models.repositorysettings import CodeReviewTrigger from sentry.utils import metrics -from ..utils import _get_target_commit_sha +from ..utils import SeerCodeReviewTrigger, _get_target_commit_sha from .config import get_direct_to_seer_gh_orgs logger = logging.getLogger(__name__) @@ -79,12 +78,12 @@ class PullRequestAction(enum.StrEnum): } -def _get_trigger_for_action(action: PullRequestAction) -> CodeReviewTrigger: +def _get_trigger_for_action(action: PullRequestAction) -> SeerCodeReviewTrigger: match action: case PullRequestAction.OPENED | PullRequestAction.READY_FOR_REVIEW: - return CodeReviewTrigger.ON_READY_FOR_REVIEW + return SeerCodeReviewTrigger.ON_READY_FOR_REVIEW case PullRequestAction.SYNCHRONIZE: - return CodeReviewTrigger.ON_NEW_COMMIT + return SeerCodeReviewTrigger.ON_NEW_COMMIT case _: raise ValueError(f"Unsupported pull request action: {action}") diff --git a/src/sentry/seer/code_review/webhooks/task.py b/src/sentry/seer/code_review/webhooks/task.py index a50ce30efdb8ed..ea5cccc7d7b23a 100644 --- a/src/sentry/seer/code_review/webhooks/task.py +++ b/src/sentry/seer/code_review/webhooks/task.py @@ -11,7 +11,6 @@ from sentry.integrations.github.webhook_types import GithubWebhookType from sentry.models.organization import Organization from sentry.models.repository import Repository -from sentry.models.repositorysettings import CodeReviewTrigger from sentry.seer.code_review.utils import ( get_webhook_option_key, transform_webhook_to_codegen_request, @@ -23,7 +22,7 @@ from sentry.taskworker.state import current_task from sentry.utils import metrics -from ..utils import get_seer_endpoint_for_event, make_seer_request +from ..utils import SeerCodeReviewTrigger, get_seer_endpoint_for_event, make_seer_request from .config import get_direct_to_seer_gh_orgs logger = logging.getLogger(__name__) @@ -42,7 +41,7 @@ def schedule_task( organization: Organization, repo: Repository, target_commit_sha: str, - trigger: CodeReviewTrigger, + trigger: SeerCodeReviewTrigger, ) -> None: """Transform and forward a webhook event to Seer for processing.""" from .task import process_github_webhook_event diff --git a/tests/sentry/integrations/api/endpoints/test_organization_repository_settings.py b/tests/sentry/integrations/api/endpoints/test_organization_repository_settings.py index 5c0ceb7e04deaa..ba1141c5637ca5 100644 --- a/tests/sentry/integrations/api/endpoints/test_organization_repository_settings.py +++ b/tests/sentry/integrations/api/endpoints/test_organization_repository_settings.py @@ -55,7 +55,7 @@ def test_bulk_update_existing_settings(self) -> None: self.create_repository_settings( repository=repo1, enabled_code_review=False, - code_review_triggers=[CodeReviewTrigger.ON_COMMAND_PHRASE], + code_review_triggers=[CodeReviewTrigger.ON_READY_FOR_REVIEW], ) self.create_repository_settings( repository=repo2, @@ -168,7 +168,7 @@ def test_partial_bulk_update_enabled_code_review_preserves_triggers(self) -> Non repository=repo1, enabled_code_review=False, code_review_triggers=[ - CodeReviewTrigger.ON_COMMAND_PHRASE, + CodeReviewTrigger.ON_READY_FOR_REVIEW, CodeReviewTrigger.ON_NEW_COMMIT, ], ) @@ -191,7 +191,7 @@ def test_partial_bulk_update_enabled_code_review_preserves_triggers(self) -> Non settings1 = RepositorySettings.objects.get(repository=repo1) assert settings1.enabled_code_review is True - assert settings1.code_review_triggers == ["on_command_phrase", "on_new_commit"] + assert settings1.code_review_triggers == ["on_ready_for_review", "on_new_commit"] settings2 = RepositorySettings.objects.get(repository=repo2) assert settings2.enabled_code_review is True @@ -204,7 +204,7 @@ def test_partial_bulk_update_code_review_triggers_preserves_enabled(self) -> Non self.create_repository_settings( repository=repo1, enabled_code_review=True, - code_review_triggers=[CodeReviewTrigger.ON_COMMAND_PHRASE], + code_review_triggers=[CodeReviewTrigger.ON_NEW_COMMIT], ) self.create_repository_settings( repository=repo2, diff --git a/tests/sentry/models/test_repositorysettings.py b/tests/sentry/models/test_repositorysettings.py index f1bc181ace2fc1..a9af49f0a21af4 100644 --- a/tests/sentry/models/test_repositorysettings.py +++ b/tests/sentry/models/test_repositorysettings.py @@ -11,7 +11,7 @@ class TestCodeReviewSettings(TestCase): def test_initialization(self) -> None: - triggers = [CodeReviewTrigger.ON_COMMAND_PHRASE, CodeReviewTrigger.ON_NEW_COMMIT] + triggers = [CodeReviewTrigger.ON_READY_FOR_REVIEW, CodeReviewTrigger.ON_NEW_COMMIT] settings = CodeReviewSettings(enabled=True, triggers=triggers) assert settings.enabled is True @@ -38,7 +38,7 @@ def test_get_code_review_settings_with_enabled_and_triggers(self) -> None: repository=self.repo, enabled_code_review=True, code_review_triggers=[ - CodeReviewTrigger.ON_COMMAND_PHRASE.value, + CodeReviewTrigger.ON_NEW_COMMIT.value, CodeReviewTrigger.ON_READY_FOR_REVIEW.value, ], ) @@ -47,7 +47,7 @@ def test_get_code_review_settings_with_enabled_and_triggers(self) -> None: assert settings.enabled is True assert len(settings.triggers) == 2 - assert CodeReviewTrigger.ON_COMMAND_PHRASE in settings.triggers + assert CodeReviewTrigger.ON_NEW_COMMIT in settings.triggers assert CodeReviewTrigger.ON_READY_FOR_REVIEW in settings.triggers def test_get_code_review_settings_converts_string_triggers_to_enum(self) -> None: diff --git a/tests/sentry/overwatch/endpoints/test_overwatch_rpc.py b/tests/sentry/overwatch/endpoints/test_overwatch_rpc.py index 56901a501b036c..939d670ded1970 100644 --- a/tests/sentry/overwatch/endpoints/test_overwatch_rpc.py +++ b/tests/sentry/overwatch/endpoints/test_overwatch_rpc.py @@ -620,7 +620,7 @@ def test_returns_repo_settings_when_exist(self): RepositorySettings.objects.create( repository=repo, enabled_code_review=True, - code_review_triggers=["on_command_phrase", "on_ready_for_review"], + code_review_triggers=["on_new_commit"], ) url = reverse("sentry-api-0-code-review-repo-settings") @@ -632,10 +632,11 @@ def test_returns_repo_settings_when_exist(self): auth = self._auth_header_for_get(url, params, "test-secret") resp = self.client.get(url, params, HTTP_AUTHORIZATION=auth) assert resp.status_code == 200 - assert resp.data == { - "enabledCodeReview": True, - "codeReviewTriggers": ["on_command_phrase", "on_ready_for_review"], - } + assert resp.data["enabledCodeReview"] is True + triggers = resp.data["codeReviewTriggers"] + assert "on_new_commit" in triggers + assert "on_command_phrase" in triggers + assert len(triggers) == 2 @patch( "sentry.overwatch.endpoints.overwatch_rpc.settings.OVERWATCH_RPC_SHARED_SECRET", @@ -658,7 +659,7 @@ def test_filters_inactive_repositories(self): RepositorySettings.objects.create( repository=repo, enabled_code_review=True, - code_review_triggers=["on_command_phrase"], + code_review_triggers=["on_new_commit"], ) url = reverse("sentry-api-0-code-review-repo-settings") @@ -715,10 +716,11 @@ def test_scopes_by_organization(self): auth = self._auth_header_for_get(url, params1, "test-secret") resp1 = self.client.get(url, params1, HTTP_AUTHORIZATION=auth) assert resp1.status_code == 200 - assert resp1.data == { - "enabledCodeReview": True, - "codeReviewTriggers": ["on_new_commit"], - } + assert resp1.data["enabledCodeReview"] is True + triggers = resp1.data["codeReviewTriggers"] + assert "on_new_commit" in triggers + assert "on_command_phrase" in triggers + assert len(triggers) == 2 # Request for org2 should return defaults (no settings created) params2 = { @@ -864,7 +866,7 @@ def test_returns_false_when_code_review_enabled_but_no_contributor_exists(self): RepositorySettings.objects.create( repository=repo, enabled_code_review=True, - code_review_triggers=["on_command_phrase"], + code_review_triggers=["on_new_commit"], ) url = reverse("sentry-api-0-prevent-pr-review-eligibility") @@ -905,7 +907,7 @@ def test_returns_false_when_quota_check_fails(self, mock_check_quota): RepositorySettings.objects.create( repository=repo, enabled_code_review=True, - code_review_triggers=["on_command_phrase"], + code_review_triggers=["on_new_commit"], ) OrganizationContributors.objects.create( @@ -954,7 +956,7 @@ def test_returns_true_when_code_review_enabled_and_quota_available(self, mock_ch RepositorySettings.objects.create( repository=repo, enabled_code_review=True, - code_review_triggers=["on_command_phrase"], + code_review_triggers=["on_new_commit"], ) contributor = OrganizationContributors.objects.create( @@ -1021,7 +1023,7 @@ def test_returns_true_when_any_org_is_eligible(self, mock_check_quota): RepositorySettings.objects.create( repository=repo2, enabled_code_review=True, - code_review_triggers=["on_command_phrase"], + code_review_triggers=["on_new_commit"], ) OrganizationContributors.objects.create( organization=org2, diff --git a/tests/sentry/seer/code_review/test_preflight.py b/tests/sentry/seer/code_review/test_preflight.py index 1e6bcf28d85966..a6fbbc0f2eba78 100644 --- a/tests/sentry/seer/code_review/test_preflight.py +++ b/tests/sentry/seer/code_review/test_preflight.py @@ -181,7 +181,7 @@ def test_returns_repo_settings_when_allowed(self, mock_check_quota: MagicMock) - repository=self.repo, enabled_code_review=True, code_review_triggers=[ - CodeReviewTrigger.ON_COMMAND_PHRASE.value, + CodeReviewTrigger.ON_NEW_COMMIT.value, CodeReviewTrigger.ON_READY_FOR_REVIEW.value, ], ) @@ -198,7 +198,7 @@ def test_returns_repo_settings_when_allowed(self, mock_check_quota: MagicMock) - assert result.allowed is True assert result.settings is not None assert result.settings.enabled is True - assert CodeReviewTrigger.ON_COMMAND_PHRASE in result.settings.triggers + assert CodeReviewTrigger.ON_NEW_COMMIT in result.settings.triggers assert CodeReviewTrigger.ON_READY_FOR_REVIEW in result.settings.triggers @patch("sentry.seer.code_review.billing.quotas.backend.check_seer_quota") diff --git a/tests/sentry/seer/code_review/test_utils.py b/tests/sentry/seer/code_review/test_utils.py index 0f04b894e274f1..de499ba8e7dcaf 100644 --- a/tests/sentry/seer/code_review/test_utils.py +++ b/tests/sentry/seer/code_review/test_utils.py @@ -6,8 +6,8 @@ from sentry.models.organization import Organization from sentry.models.project import Project from sentry.models.repository import Repository -from sentry.models.repositorysettings import CodeReviewTrigger from sentry.seer.code_review.utils import ( + SeerCodeReviewTrigger, _get_target_commit_sha, _get_trigger_metadata, transform_webhook_to_codegen_request, @@ -179,7 +179,7 @@ def test_pull_request_event( organization, repo, "abc123sha", - CodeReviewTrigger.ON_READY_FOR_REVIEW, + SeerCodeReviewTrigger.ON_READY_FOR_REVIEW, ) expected_repo = { @@ -203,7 +203,7 @@ def test_pull_request_event( } assert result["data"]["config"] == { "features": {"bug_prediction": True}, - "trigger": CodeReviewTrigger.ON_READY_FOR_REVIEW.value, + "trigger": SeerCodeReviewTrigger.ON_READY_FOR_REVIEW.value, } | {k: v for k, v in result["data"]["config"].items() if k not in ("features", "trigger")} def test_issue_comment_on_pr( @@ -229,14 +229,14 @@ def test_issue_comment_on_pr( organization, repo, "def456sha", - CodeReviewTrigger.ON_COMMAND_PHRASE, + SeerCodeReviewTrigger.ON_NEW_COMMIT, ) assert isinstance(result, dict) data = result["data"] config = data["config"] assert data["pr_id"] == 42 - assert config["trigger"] == CodeReviewTrigger.ON_COMMAND_PHRASE.value + assert config["trigger"] == SeerCodeReviewTrigger.ON_NEW_COMMIT.value assert config["trigger_comment_id"] == 12345 assert config["trigger_user"] == "commenter" assert config["trigger_comment_type"] == "issue_comment" @@ -256,7 +256,7 @@ def test_issue_comment_on_regular_issue_returns_none( organization, repo, "somesha", - CodeReviewTrigger.ON_COMMAND_PHRASE, + SeerCodeReviewTrigger.ON_NEW_COMMIT, ) assert result is None @@ -280,5 +280,5 @@ def test_invalid_repo_name_format_raises( organization, bad_repo, "sha123", - CodeReviewTrigger.ON_READY_FOR_REVIEW, + SeerCodeReviewTrigger.ON_READY_FOR_REVIEW, ) From bd700fff94c84122c263381649c1de2fe726c299 Mon Sep 17 00:00:00 2001 From: srest2021 Date: Thu, 8 Jan 2026 16:00:58 -0800 Subject: [PATCH 08/16] add migration --- migrations_lockfile.txt | 2 +- ..._on_command_phrase_trigger_reposettings.py | 72 +++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 src/sentry/migrations/1017_remove_on_command_phrase_trigger_reposettings.py diff --git a/migrations_lockfile.txt b/migrations_lockfile.txt index 414f4f3f26fe29..fd87de909a49f6 100644 --- a/migrations_lockfile.txt +++ b/migrations_lockfile.txt @@ -31,7 +31,7 @@ releases: 0004_cleanup_failed_safe_deletes replays: 0007_organizationmember_replay_access -sentry: 1016_remove_on_command_phrase_trigger +sentry: 1017_remove_on_command_phrase_trigger_reposettings social_auth: 0003_social_auth_json_field diff --git a/src/sentry/migrations/1017_remove_on_command_phrase_trigger_reposettings.py b/src/sentry/migrations/1017_remove_on_command_phrase_trigger_reposettings.py new file mode 100644 index 00000000000000..226d5ffb80e93d --- /dev/null +++ b/src/sentry/migrations/1017_remove_on_command_phrase_trigger_reposettings.py @@ -0,0 +1,72 @@ +# Generated by Django 5.2.8 on 2026-01-08 23:56 + +import django.contrib.postgres.fields +from django.db import migrations, models +from django.db.backends.base.schema import BaseDatabaseSchemaEditor +from django.db.migrations.state import StateApps + +from sentry.new_migrations.migrations import CheckedMigration + + +def remove_on_command_phrase_from_repository_settings( + apps: StateApps, schema_editor: BaseDatabaseSchemaEditor +) -> None: + """Remove 'on_command_phrase' from RepositorySettings.code_review_triggers.""" + RepositorySettings = apps.get_model("sentry", "RepositorySettings") + + settings_to_update = RepositorySettings.objects.all() + + for settings in settings_to_update: + if ( + settings.code_review_triggers + and isinstance(settings.code_review_triggers, list) + and "on_command_phrase" in settings.code_review_triggers + ): + settings.code_review_triggers = [ + trigger + for trigger in settings.code_review_triggers + if trigger != "on_command_phrase" + ] + settings.save(update_fields=["code_review_triggers"]) + + +class Migration(CheckedMigration): + # This flag is used to mark that a migration shouldn't be automatically run in production. + # This should only be used for operations where it's safe to run the migration after your + # code has deployed. So this should not be used for most operations that alter the schema + # of a table. + # Here are some things that make sense to mark as post deployment: + # - Large data migrations. Typically we want these to be run manually so that they can be + # monitored and not block the deploy for a long period of time while they run. + # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to + # run this outside deployments so that we don't block them. Note that while adding an index + # is a schema change, it's completely safe to run the operation after the code has deployed. + # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment + + is_post_deployment = False + + dependencies = [ + ("sentry", "1016_remove_on_command_phrase_trigger"), + ] + + operations = [ + migrations.RunPython( + remove_on_command_phrase_from_repository_settings, + reverse_code=migrations.RunPython.noop, + hints={"tables": ["sentry_repositorysettings"]}, + ), + migrations.AlterField( + model_name="repositorysettings", + name="code_review_triggers", + field=django.contrib.postgres.fields.ArrayField( + base_field=models.CharField( + choices=[ + ("on_new_commit", "on_new_commit"), + ("on_ready_for_review", "on_ready_for_review"), + ], + max_length=32, + ), + default=list, + ), + ), + ] From 7970184f0fbb2a0e5b4d969d99fed7b262a4df58 Mon Sep 17 00:00:00 2001 From: srest2021 Date: Thu, 8 Jan 2026 20:57:00 -0800 Subject: [PATCH 09/16] optimize migration --- .../1016_remove_on_command_phrase_trigger.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/sentry/migrations/1016_remove_on_command_phrase_trigger.py b/src/sentry/migrations/1016_remove_on_command_phrase_trigger.py index 08f896d5cf5777..3324b391cf43e2 100644 --- a/src/sentry/migrations/1016_remove_on_command_phrase_trigger.py +++ b/src/sentry/migrations/1016_remove_on_command_phrase_trigger.py @@ -14,10 +14,11 @@ def remove_on_command_phrase_from_triggers( """Remove 'on_command_phrase' from organization defaultCodeReviewTriggers.""" OrganizationOption = apps.get_model("sentry", "OrganizationOption") - org_options_to_update = OrganizationOption.objects.filter( - key="sentry:default_code_review_triggers" + org_options_to_update = list( + OrganizationOption.objects.filter(key="sentry:default_code_review_triggers") ) + updated_org_options = [] for org_option in org_options_to_update: if ( org_option.value @@ -27,7 +28,10 @@ def remove_on_command_phrase_from_triggers( org_option.value = [ trigger for trigger in org_option.value if trigger != "on_command_phrase" ] - org_option.save(update_fields=["value"]) + updated_org_options.append(org_option) + + if updated_org_options: + OrganizationOption.objects.bulk_update(updated_org_options, fields=["value"]) class Migration(CheckedMigration): From 363f975130e388252f884d5724dcb5c9146793d8 Mon Sep 17 00:00:00 2001 From: srest2021 Date: Thu, 8 Jan 2026 21:10:39 -0800 Subject: [PATCH 10/16] combine migrations --- migrations_lockfile.txt | 2 +- .../1016_remove_on_command_phrase_trigger.py | 57 +++++++++++++-- ..._on_command_phrase_trigger_reposettings.py | 72 ------------------- ...t_1016_remove_on_command_phrase_trigger.py | 69 ++++++++++++++++++ 4 files changed, 121 insertions(+), 79 deletions(-) delete mode 100644 src/sentry/migrations/1017_remove_on_command_phrase_trigger_reposettings.py create mode 100644 tests/sentry/migrations/test_1016_remove_on_command_phrase_trigger.py diff --git a/migrations_lockfile.txt b/migrations_lockfile.txt index fd87de909a49f6..414f4f3f26fe29 100644 --- a/migrations_lockfile.txt +++ b/migrations_lockfile.txt @@ -31,7 +31,7 @@ releases: 0004_cleanup_failed_safe_deletes replays: 0007_organizationmember_replay_access -sentry: 1017_remove_on_command_phrase_trigger_reposettings +sentry: 1016_remove_on_command_phrase_trigger social_auth: 0003_social_auth_json_field diff --git a/src/sentry/migrations/1016_remove_on_command_phrase_trigger.py b/src/sentry/migrations/1016_remove_on_command_phrase_trigger.py index 3324b391cf43e2..53d6df3936dcaf 100644 --- a/src/sentry/migrations/1016_remove_on_command_phrase_trigger.py +++ b/src/sentry/migrations/1016_remove_on_command_phrase_trigger.py @@ -1,14 +1,14 @@ # Generated by Django 5.2.8 on 2026-01-08 18:18 - -from django.db import migrations +import django.contrib.postgres.fields +from django.db import migrations, models from django.db.backends.base.schema import BaseDatabaseSchemaEditor from django.db.migrations.state import StateApps from sentry.new_migrations.migrations import CheckedMigration -def remove_on_command_phrase_from_triggers( +def remove_on_command_phrase_from_org_options( apps: StateApps, schema_editor: BaseDatabaseSchemaEditor ) -> None: """Remove 'on_command_phrase' from organization defaultCodeReviewTriggers.""" @@ -34,6 +34,32 @@ def remove_on_command_phrase_from_triggers( OrganizationOption.objects.bulk_update(updated_org_options, fields=["value"]) +def remove_on_command_phrase_from_repository_settings( + apps: StateApps, schema_editor: BaseDatabaseSchemaEditor +) -> None: + """Remove 'on_command_phrase' from RepositorySettings.code_review_triggers.""" + RepositorySettings = apps.get_model("sentry", "RepositorySettings") + + all_settings = list(RepositorySettings.objects.all()) + + updated_settings = [] + for settings in all_settings: + if ( + settings.code_review_triggers + and isinstance(settings.code_review_triggers, list) + and "on_command_phrase" in settings.code_review_triggers + ): + settings.code_review_triggers = [ + trigger + for trigger in settings.code_review_triggers + if trigger != "on_command_phrase" + ] + updated_settings.append(settings) + + if updated_settings: + RepositorySettings.objects.bulk_update(updated_settings, fields=["code_review_triggers"]) + + class Migration(CheckedMigration): # This flag is used to mark that a migration shouldn't be automatically run in production. # This should only be used for operations where it's safe to run the migration after your @@ -47,7 +73,7 @@ class Migration(CheckedMigration): # is a schema change, it's completely safe to run the operation after the code has deployed. # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment - is_post_deployment = True + is_post_deployment = False dependencies = [ ("sentry", "1015_backfill_self_hosted_sentry_app_emails"), @@ -55,8 +81,27 @@ class Migration(CheckedMigration): operations = [ migrations.RunPython( - remove_on_command_phrase_from_triggers, + remove_on_command_phrase_from_org_options, reverse_code=migrations.RunPython.noop, hints={"tables": ["sentry_organizationoptions"]}, - ) + ), + migrations.RunPython( + remove_on_command_phrase_from_repository_settings, + reverse_code=migrations.RunPython.noop, + hints={"tables": ["sentry_repositorysettings"]}, + ), + migrations.AlterField( + model_name="repositorysettings", + name="code_review_triggers", + field=django.contrib.postgres.fields.ArrayField( + base_field=models.CharField( + choices=[ + ("on_new_commit", "on_new_commit"), + ("on_ready_for_review", "on_ready_for_review"), + ], + max_length=32, + ), + default=list, + ), + ), ] diff --git a/src/sentry/migrations/1017_remove_on_command_phrase_trigger_reposettings.py b/src/sentry/migrations/1017_remove_on_command_phrase_trigger_reposettings.py deleted file mode 100644 index 226d5ffb80e93d..00000000000000 --- a/src/sentry/migrations/1017_remove_on_command_phrase_trigger_reposettings.py +++ /dev/null @@ -1,72 +0,0 @@ -# Generated by Django 5.2.8 on 2026-01-08 23:56 - -import django.contrib.postgres.fields -from django.db import migrations, models -from django.db.backends.base.schema import BaseDatabaseSchemaEditor -from django.db.migrations.state import StateApps - -from sentry.new_migrations.migrations import CheckedMigration - - -def remove_on_command_phrase_from_repository_settings( - apps: StateApps, schema_editor: BaseDatabaseSchemaEditor -) -> None: - """Remove 'on_command_phrase' from RepositorySettings.code_review_triggers.""" - RepositorySettings = apps.get_model("sentry", "RepositorySettings") - - settings_to_update = RepositorySettings.objects.all() - - for settings in settings_to_update: - if ( - settings.code_review_triggers - and isinstance(settings.code_review_triggers, list) - and "on_command_phrase" in settings.code_review_triggers - ): - settings.code_review_triggers = [ - trigger - for trigger in settings.code_review_triggers - if trigger != "on_command_phrase" - ] - settings.save(update_fields=["code_review_triggers"]) - - -class Migration(CheckedMigration): - # This flag is used to mark that a migration shouldn't be automatically run in production. - # This should only be used for operations where it's safe to run the migration after your - # code has deployed. So this should not be used for most operations that alter the schema - # of a table. - # Here are some things that make sense to mark as post deployment: - # - Large data migrations. Typically we want these to be run manually so that they can be - # monitored and not block the deploy for a long period of time while they run. - # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to - # run this outside deployments so that we don't block them. Note that while adding an index - # is a schema change, it's completely safe to run the operation after the code has deployed. - # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment - - is_post_deployment = False - - dependencies = [ - ("sentry", "1016_remove_on_command_phrase_trigger"), - ] - - operations = [ - migrations.RunPython( - remove_on_command_phrase_from_repository_settings, - reverse_code=migrations.RunPython.noop, - hints={"tables": ["sentry_repositorysettings"]}, - ), - migrations.AlterField( - model_name="repositorysettings", - name="code_review_triggers", - field=django.contrib.postgres.fields.ArrayField( - base_field=models.CharField( - choices=[ - ("on_new_commit", "on_new_commit"), - ("on_ready_for_review", "on_ready_for_review"), - ], - max_length=32, - ), - default=list, - ), - ), - ] diff --git a/tests/sentry/migrations/test_1016_remove_on_command_phrase_trigger.py b/tests/sentry/migrations/test_1016_remove_on_command_phrase_trigger.py new file mode 100644 index 00000000000000..326a9f8c38385f --- /dev/null +++ b/tests/sentry/migrations/test_1016_remove_on_command_phrase_trigger.py @@ -0,0 +1,69 @@ +from sentry.models.options.organization_option import OrganizationOption +from sentry.testutils.cases import TestMigrations + + +class RemoveOnCommandPhraseTriggerTest(TestMigrations): + migrate_from = "1015_backfill_self_hosted_sentry_app_emails" + migrate_to = "1016_remove_on_command_phrase_trigger" + + def setup_initial_state(self) -> None: + self.org_with_trigger = self.create_organization(name="org_with_trigger") + self.org_without_trigger = self.create_organization(name="org_without_trigger") + self.org_with_different_key = self.create_organization(name="org_with_different_key") + self.org_with_none_value = self.create_organization(name="org_with_none_value") + + # Org with on_command_phrase that should be removed + OrganizationOption.objects.set_value( + self.org_with_trigger, + "sentry:default_code_review_triggers", + ["on_command_phrase", "on_ready_for_review", "on_new_commit"], + ) + + # Org without on_command_phrase, should remain unchanged + OrganizationOption.objects.set_value( + self.org_without_trigger, + "sentry:default_code_review_triggers", + ["on_ready_for_review", "on_new_commit"], + ) + + # Org with different key that contains on_command_phrase, should remain unchanged + OrganizationOption.objects.set_value( + self.org_with_different_key, + "sentry:some_other_key", + ["on_command_phrase", "other_value"], + ) + + # Org with None value, should remain unchanged + OrganizationOption.objects.set_value( + self.org_with_none_value, + "sentry:default_code_review_triggers", + None, + ) + + def test(self) -> None: + # Query directly from database to bypass cache + # Verify org with trigger: on_command_phrase removed, others remain + org_option = OrganizationOption.objects.get( + organization=self.org_with_trigger, key="sentry:default_code_review_triggers" + ) + assert org_option.value == ["on_ready_for_review", "on_new_commit"] + assert "on_command_phrase" not in org_option.value + + # Verify org without trigger: unchanged + org_option = OrganizationOption.objects.get( + organization=self.org_without_trigger, key="sentry:default_code_review_triggers" + ) + assert org_option.value == ["on_ready_for_review", "on_new_commit"] + assert "on_command_phrase" not in org_option.value + + # Verify org with different key: unchanged (migration only affects specific key) + org_option = OrganizationOption.objects.get( + organization=self.org_with_different_key, key="sentry:some_other_key" + ) + assert org_option.value == ["on_command_phrase", "other_value"] + + # Verify org with None value: unchanged + org_option = OrganizationOption.objects.get( + organization=self.org_with_none_value, key="sentry:default_code_review_triggers" + ) + assert org_option.value is None From 8a5d0c06173b2829b16936eae04e534af133003a Mon Sep 17 00:00:00 2001 From: srest2021 Date: Thu, 8 Jan 2026 21:13:16 -0800 Subject: [PATCH 11/16] remove migration test for now --- ...t_1016_remove_on_command_phrase_trigger.py | 69 ------------------- 1 file changed, 69 deletions(-) delete mode 100644 tests/sentry/migrations/test_1016_remove_on_command_phrase_trigger.py diff --git a/tests/sentry/migrations/test_1016_remove_on_command_phrase_trigger.py b/tests/sentry/migrations/test_1016_remove_on_command_phrase_trigger.py deleted file mode 100644 index 326a9f8c38385f..00000000000000 --- a/tests/sentry/migrations/test_1016_remove_on_command_phrase_trigger.py +++ /dev/null @@ -1,69 +0,0 @@ -from sentry.models.options.organization_option import OrganizationOption -from sentry.testutils.cases import TestMigrations - - -class RemoveOnCommandPhraseTriggerTest(TestMigrations): - migrate_from = "1015_backfill_self_hosted_sentry_app_emails" - migrate_to = "1016_remove_on_command_phrase_trigger" - - def setup_initial_state(self) -> None: - self.org_with_trigger = self.create_organization(name="org_with_trigger") - self.org_without_trigger = self.create_organization(name="org_without_trigger") - self.org_with_different_key = self.create_organization(name="org_with_different_key") - self.org_with_none_value = self.create_organization(name="org_with_none_value") - - # Org with on_command_phrase that should be removed - OrganizationOption.objects.set_value( - self.org_with_trigger, - "sentry:default_code_review_triggers", - ["on_command_phrase", "on_ready_for_review", "on_new_commit"], - ) - - # Org without on_command_phrase, should remain unchanged - OrganizationOption.objects.set_value( - self.org_without_trigger, - "sentry:default_code_review_triggers", - ["on_ready_for_review", "on_new_commit"], - ) - - # Org with different key that contains on_command_phrase, should remain unchanged - OrganizationOption.objects.set_value( - self.org_with_different_key, - "sentry:some_other_key", - ["on_command_phrase", "other_value"], - ) - - # Org with None value, should remain unchanged - OrganizationOption.objects.set_value( - self.org_with_none_value, - "sentry:default_code_review_triggers", - None, - ) - - def test(self) -> None: - # Query directly from database to bypass cache - # Verify org with trigger: on_command_phrase removed, others remain - org_option = OrganizationOption.objects.get( - organization=self.org_with_trigger, key="sentry:default_code_review_triggers" - ) - assert org_option.value == ["on_ready_for_review", "on_new_commit"] - assert "on_command_phrase" not in org_option.value - - # Verify org without trigger: unchanged - org_option = OrganizationOption.objects.get( - organization=self.org_without_trigger, key="sentry:default_code_review_triggers" - ) - assert org_option.value == ["on_ready_for_review", "on_new_commit"] - assert "on_command_phrase" not in org_option.value - - # Verify org with different key: unchanged (migration only affects specific key) - org_option = OrganizationOption.objects.get( - organization=self.org_with_different_key, key="sentry:some_other_key" - ) - assert org_option.value == ["on_command_phrase", "other_value"] - - # Verify org with None value: unchanged - org_option = OrganizationOption.objects.get( - organization=self.org_with_none_value, key="sentry:default_code_review_triggers" - ) - assert org_option.value is None From 4066ced5022b7cf3d7dde9b2d60f00fdb193ab88 Mon Sep 17 00:00:00 2001 From: srest2021 Date: Thu, 8 Jan 2026 21:25:42 -0800 Subject: [PATCH 12/16] don't need to modify overwatch rpc --- .../overwatch/endpoints/overwatch_rpc.py | 9 ++----- .../overwatch/endpoints/test_overwatch_rpc.py | 24 +++++++++---------- 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/src/sentry/overwatch/endpoints/overwatch_rpc.py b/src/sentry/overwatch/endpoints/overwatch_rpc.py index 3435c9cf77ff5a..56d539defddc8a 100644 --- a/src/sentry/overwatch/endpoints/overwatch_rpc.py +++ b/src/sentry/overwatch/endpoints/overwatch_rpc.py @@ -214,10 +214,7 @@ def get(self, request: Request) -> Response: ) ): return Response( - { - "enabledCodeReview": True, - "codeReviewTriggers": DEFAULT_CODE_REVIEW_TRIGGERS + ["on_command_phrase"], - } + {"enabledCodeReview": True, "codeReviewTriggers": DEFAULT_CODE_REVIEW_TRIGGERS} ) return Response( @@ -227,12 +224,10 @@ def get(self, request: Request) -> Response: } ) - triggers = set(repo_settings.code_review_triggers) - triggers.add("on_command_phrase") return Response( { "enabledCodeReview": repo_settings.enabled_code_review, - "codeReviewTriggers": list(triggers), + "codeReviewTriggers": repo_settings.code_review_triggers, } ) diff --git a/tests/sentry/overwatch/endpoints/test_overwatch_rpc.py b/tests/sentry/overwatch/endpoints/test_overwatch_rpc.py index 939d670ded1970..4cf7e6d48c0e60 100644 --- a/tests/sentry/overwatch/endpoints/test_overwatch_rpc.py +++ b/tests/sentry/overwatch/endpoints/test_overwatch_rpc.py @@ -574,7 +574,7 @@ def test_returns_enabled_with_default_triggers_when_code_review_beta_flag(self): assert resp.status_code == 200 assert resp.data == { "enabledCodeReview": True, - "codeReviewTriggers": DEFAULT_CODE_REVIEW_TRIGGERS + ["on_command_phrase"], + "codeReviewTriggers": DEFAULT_CODE_REVIEW_TRIGGERS, } @patch( @@ -598,7 +598,7 @@ def test_returns_enabled_with_default_triggers_when_pr_review_test_generation_en assert resp.status_code == 200 assert resp.data == { "enabledCodeReview": True, - "codeReviewTriggers": DEFAULT_CODE_REVIEW_TRIGGERS + ["on_command_phrase"], + "codeReviewTriggers": DEFAULT_CODE_REVIEW_TRIGGERS, } @patch( @@ -620,7 +620,7 @@ def test_returns_repo_settings_when_exist(self): RepositorySettings.objects.create( repository=repo, enabled_code_review=True, - code_review_triggers=["on_new_commit"], + code_review_triggers=["on_new_commit", "on_ready_for_review"], ) url = reverse("sentry-api-0-code-review-repo-settings") @@ -632,11 +632,10 @@ def test_returns_repo_settings_when_exist(self): auth = self._auth_header_for_get(url, params, "test-secret") resp = self.client.get(url, params, HTTP_AUTHORIZATION=auth) assert resp.status_code == 200 - assert resp.data["enabledCodeReview"] is True - triggers = resp.data["codeReviewTriggers"] - assert "on_new_commit" in triggers - assert "on_command_phrase" in triggers - assert len(triggers) == 2 + assert resp.data == { + "enabledCodeReview": True, + "codeReviewTriggers": ["on_new_commit", "on_ready_for_review"], + } @patch( "sentry.overwatch.endpoints.overwatch_rpc.settings.OVERWATCH_RPC_SHARED_SECRET", @@ -716,11 +715,10 @@ def test_scopes_by_organization(self): auth = self._auth_header_for_get(url, params1, "test-secret") resp1 = self.client.get(url, params1, HTTP_AUTHORIZATION=auth) assert resp1.status_code == 200 - assert resp1.data["enabledCodeReview"] is True - triggers = resp1.data["codeReviewTriggers"] - assert "on_new_commit" in triggers - assert "on_command_phrase" in triggers - assert len(triggers) == 2 + assert resp1.data == { + "enabledCodeReview": True, + "codeReviewTriggers": ["on_new_commit"], + } # Request for org2 should return defaults (no settings created) params2 = { From e726f5c366e3d9c890b67a93915593d0c815719d Mon Sep 17 00:00:00 2001 From: srest2021 Date: Thu, 8 Jan 2026 21:26:23 -0800 Subject: [PATCH 13/16] minimize diff --- src/sentry/overwatch/endpoints/overwatch_rpc.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/sentry/overwatch/endpoints/overwatch_rpc.py b/src/sentry/overwatch/endpoints/overwatch_rpc.py index 56d539defddc8a..4e7962078513a6 100644 --- a/src/sentry/overwatch/endpoints/overwatch_rpc.py +++ b/src/sentry/overwatch/endpoints/overwatch_rpc.py @@ -214,7 +214,10 @@ def get(self, request: Request) -> Response: ) ): return Response( - {"enabledCodeReview": True, "codeReviewTriggers": DEFAULT_CODE_REVIEW_TRIGGERS} + { + "enabledCodeReview": True, + "codeReviewTriggers": DEFAULT_CODE_REVIEW_TRIGGERS, + } ) return Response( From a3886f19cef355a499781e093f2e3dcd518cbe10 Mon Sep 17 00:00:00 2001 From: srest2021 Date: Fri, 9 Jan 2026 11:32:40 -0800 Subject: [PATCH 14/16] add cpomment to SeerCodeReviewTrigger --- src/sentry/seer/code_review/utils.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/sentry/seer/code_review/utils.py b/src/sentry/seer/code_review/utils.py index 3bba9bdd1ead90..e1b159d94a1419 100644 --- a/src/sentry/seer/code_review/utils.py +++ b/src/sentry/seer/code_review/utils.py @@ -27,6 +27,13 @@ class ClientError(Exception): class SeerCodeReviewTrigger(StrEnum): + """ + Internal code review trigger type used for Seer flows. + + This includes all user-configurable CodeReviewTrigger values, plus on_command_phrase, + which is always enabled and cannot be turned off by users. + """ + ON_COMMAND_PHRASE = "on_command_phrase" ON_NEW_COMMIT = "on_new_commit" ON_READY_FOR_REVIEW = "on_ready_for_review" From cac411c1b372eefec96178be1219cecaf3b2f911 Mon Sep 17 00:00:00 2001 From: srest2021 Date: Fri, 9 Jan 2026 11:41:31 -0800 Subject: [PATCH 15/16] remove org option migration; add migration --- .../1016_remove_on_command_phrase_trigger.py | 31 ------------- ...t_1016_remove_on_command_phrase_trigger.py | 45 +++++++++++++++++++ 2 files changed, 45 insertions(+), 31 deletions(-) create mode 100644 tests/sentry/migrations/test_1016_remove_on_command_phrase_trigger.py diff --git a/src/sentry/migrations/1016_remove_on_command_phrase_trigger.py b/src/sentry/migrations/1016_remove_on_command_phrase_trigger.py index 53d6df3936dcaf..269cb5b1146a70 100644 --- a/src/sentry/migrations/1016_remove_on_command_phrase_trigger.py +++ b/src/sentry/migrations/1016_remove_on_command_phrase_trigger.py @@ -8,32 +8,6 @@ from sentry.new_migrations.migrations import CheckedMigration -def remove_on_command_phrase_from_org_options( - apps: StateApps, schema_editor: BaseDatabaseSchemaEditor -) -> None: - """Remove 'on_command_phrase' from organization defaultCodeReviewTriggers.""" - OrganizationOption = apps.get_model("sentry", "OrganizationOption") - - org_options_to_update = list( - OrganizationOption.objects.filter(key="sentry:default_code_review_triggers") - ) - - updated_org_options = [] - for org_option in org_options_to_update: - if ( - org_option.value - and isinstance(org_option.value, list) - and "on_command_phrase" in org_option.value - ): - org_option.value = [ - trigger for trigger in org_option.value if trigger != "on_command_phrase" - ] - updated_org_options.append(org_option) - - if updated_org_options: - OrganizationOption.objects.bulk_update(updated_org_options, fields=["value"]) - - def remove_on_command_phrase_from_repository_settings( apps: StateApps, schema_editor: BaseDatabaseSchemaEditor ) -> None: @@ -80,11 +54,6 @@ class Migration(CheckedMigration): ] operations = [ - migrations.RunPython( - remove_on_command_phrase_from_org_options, - reverse_code=migrations.RunPython.noop, - hints={"tables": ["sentry_organizationoptions"]}, - ), migrations.RunPython( remove_on_command_phrase_from_repository_settings, reverse_code=migrations.RunPython.noop, diff --git a/tests/sentry/migrations/test_1016_remove_on_command_phrase_trigger.py b/tests/sentry/migrations/test_1016_remove_on_command_phrase_trigger.py new file mode 100644 index 00000000000000..4af89882d4e37f --- /dev/null +++ b/tests/sentry/migrations/test_1016_remove_on_command_phrase_trigger.py @@ -0,0 +1,45 @@ +from sentry.models.repositorysettings import RepositorySettings +from sentry.testutils.cases import TestMigrations + + +class RemoveOnCommandPhraseTriggerTest(TestMigrations): + migrate_from = "1015_backfill_self_hosted_sentry_app_emails" + migrate_to = "1016_remove_on_command_phrase_trigger" + + def setup_initial_state(self) -> None: + org = self.create_organization() + self.project1 = self.create_project(organization=org) + self.repo1 = self.create_repo(project=self.project1, name="org/repo1") + self.repo_settings1 = RepositorySettings.objects.create( + repository=self.repo1, + enabled_code_review=True, + code_review_triggers=["on_command_phrase", "on_ready_for_review", "on_new_commit"], + ) + + self.project2 = self.create_project(organization=org) + self.repo2 = self.create_repo(project=self.project2, name="org/repo2") + self.repo_settings2 = RepositorySettings.objects.create( + repository=self.repo2, + enabled_code_review=True, + code_review_triggers=["on_ready_for_review", "on_new_commit"], + ) + + self.project3 = self.create_project(organization=org) + self.repo3 = self.create_repo(project=self.project3, name="org/repo3") + self.repo_settings3 = RepositorySettings.objects.create( + repository=self.repo3, + enabled_code_review=True, + code_review_triggers=[], + ) + + def test(self) -> None: + repo_settings = RepositorySettings.objects.get(id=self.repo_settings1.id) + assert repo_settings.code_review_triggers == ["on_ready_for_review", "on_new_commit"] + assert "on_command_phrase" not in repo_settings.code_review_triggers + + repo_settings = RepositorySettings.objects.get(id=self.repo_settings2.id) + assert repo_settings.code_review_triggers == ["on_ready_for_review", "on_new_commit"] + assert "on_command_phrase" not in repo_settings.code_review_triggers + + repo_settings = RepositorySettings.objects.get(id=self.repo_settings3.id) + assert repo_settings.code_review_triggers == [] From ac0aa40d1bf26e3ebd4ba731ef414bfdbf5541ed Mon Sep 17 00:00:00 2001 From: srest2021 Date: Fri, 9 Jan 2026 11:51:42 -0800 Subject: [PATCH 16/16] make migration test more robust --- .../migrations/test_1016_remove_on_command_phrase_trigger.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/sentry/migrations/test_1016_remove_on_command_phrase_trigger.py b/tests/sentry/migrations/test_1016_remove_on_command_phrase_trigger.py index 4af89882d4e37f..d7db84110b764c 100644 --- a/tests/sentry/migrations/test_1016_remove_on_command_phrase_trigger.py +++ b/tests/sentry/migrations/test_1016_remove_on_command_phrase_trigger.py @@ -27,9 +27,7 @@ def setup_initial_state(self) -> None: self.project3 = self.create_project(organization=org) self.repo3 = self.create_repo(project=self.project3, name="org/repo3") self.repo_settings3 = RepositorySettings.objects.create( - repository=self.repo3, - enabled_code_review=True, - code_review_triggers=[], + repository=self.repo3, enabled_code_review=True ) def test(self) -> None: