Skip to content
Closed
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion migrations_lockfile.txt
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ releases: 0004_cleanup_failed_safe_deletes

replays: 0007_organizationmember_replay_access

sentry: 1014_add_pkce_to_apigrant
sentry: 1015_backfill_on_command_phrase_trigger

social_auth: 0003_social_auth_json_field

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ def put(self, request: Request, organization: Organization) -> Response:
if updated_code_review_triggers is not None:
update_fields.append("code_review_triggers")

# Ensure ON_COMMAND_PHRASE is always a trigger
if CodeReviewTrigger.ON_COMMAND_PHRASE.value not in updated_code_review_triggers:
updated_code_review_triggers.append(CodeReviewTrigger.ON_COMMAND_PHRASE.value)

repositories = list(
Repository.objects.filter(
id__in=repository_ids,
Expand All @@ -93,7 +97,9 @@ def put(self, request: Request, organization: Organization) -> Response:

settings_to_upsert = []
for repo in repositories:
setting = existing_settings.get(repo.id) or RepositorySettings(repository=repo)
setting = existing_settings.get(repo.id) or RepositorySettings(
repository=repo, code_review_triggers=[CodeReviewTrigger.ON_COMMAND_PHRASE.value]
)

if updated_enabled_code_review is not None:
setting.enabled_code_review = updated_enabled_code_review
Expand Down
63 changes: 63 additions & 0 deletions src/sentry/migrations/1015_backfill_on_command_phrase_trigger.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# Generated by Django 5.2.8 on 2025-12-19 23:11

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 backfill_on_command_phrase_trigger(
apps: StateApps, schema_editor: BaseDatabaseSchemaEditor
) -> None:
"""Backfill on_command_phrase code review trigger for all existing RepositorySettings."""
RepositorySettings = apps.get_model("sentry", "RepositorySettings")

settings_to_update = []
batch_size = 1000

for setting in RepositorySettings.objects.all().iterator(chunk_size=batch_size):
triggers = list(setting.code_review_triggers or [])
if "on_command_phrase" not in triggers:
setting.code_review_triggers = triggers + ["on_command_phrase"]
settings_to_update.append(setting)

if len(settings_to_update) >= batch_size:
RepositorySettings.objects.bulk_update(
settings_to_update, ["code_review_triggers"], batch_size=batch_size
)
settings_to_update = []

# Update remaining items
if settings_to_update:
RepositorySettings.objects.bulk_update(
settings_to_update, ["code_review_triggers"], batch_size=batch_size
)


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", "1014_add_pkce_to_apigrant"),
]

operations = [
migrations.RunPython(
backfill_on_command_phrase_trigger,
reverse_code=migrations.RunPython.noop,
hints={"tables": ["sentry_repositorysettings"]},
),
]
10 changes: 10 additions & 0 deletions src/sentry/models/repositorysettings.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

from enum import StrEnum
from typing import Any

from django.contrib.postgres.fields.array import ArrayField
from django.db import models
Expand Down Expand Up @@ -41,3 +42,12 @@ class Meta:
db_table = "sentry_repositorysettings"

__repr__ = sane_repr("repository_id", "enabled_code_review")

def save(self, *args: Any, **kwargs: Any) -> None:
# Ensure ON_COMMAND_PHRASE is always a trigger
triggers = list(self.code_review_triggers or [])
if CodeReviewTrigger.ON_COMMAND_PHRASE.value not in triggers:
triggers.append(CodeReviewTrigger.ON_COMMAND_PHRASE.value)
self.code_review_triggers = triggers

super().save(*args, **kwargs)
Comment on lines +57 to +62
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How frequent are writes to this table? There will be a small time window (a few minutes) between your backfill running and this code being deployed to all servers. Any rows created in that time window will not have on_command_phrase stored. If you have a low volume of writes you could get lucky. If you want to completely avoid missed writes you'd need to make the migration a post_deploy instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I expect write freq to be low but if there's no downside to doing it post-deploy, I'll just do so for peace of mind!

Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ def test_expand_settings_with_settings(self) -> None:
assert response.data[0]["settings"]["codeReviewTriggers"] == [
"on_new_commit",
"on_ready_for_review",
"on_command_phrase",
]

def test_expand_settings_without_settings(self) -> None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ def test_get_repository_expand_settings(self) -> None:
assert response.data["settings"]["codeReviewTriggers"] == [
"on_new_commit",
"on_ready_for_review",
"on_command_phrase",
]

def test_get_repository_expand_settings_no_settings_exist(self) -> None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ def test_bulk_create_settings(self) -> None:
"enabledCodeReview": True,
"codeReviewTriggers": [
CodeReviewTrigger.ON_NEW_COMMIT,
CodeReviewTrigger.ON_READY_FOR_REVIEW,
],
},
format="json",
Expand All @@ -38,11 +37,16 @@ def test_bulk_create_settings(self) -> None:

settings1 = RepositorySettings.objects.get(repository=repo1)
assert settings1.enabled_code_review is True
assert settings1.code_review_triggers == ["on_new_commit", "on_ready_for_review"]

assert settings1.code_review_triggers == [
CodeReviewTrigger.ON_NEW_COMMIT,
CodeReviewTrigger.ON_COMMAND_PHRASE,
]
settings2 = RepositorySettings.objects.get(repository=repo2)
assert settings2.enabled_code_review is True
assert settings2.code_review_triggers == ["on_new_commit", "on_ready_for_review"]
assert settings2.code_review_triggers == [
CodeReviewTrigger.ON_NEW_COMMIT,
CodeReviewTrigger.ON_COMMAND_PHRASE,
]

for repo_data in response.data:
assert "settings" in repo_data
Expand All @@ -55,7 +59,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,
Expand All @@ -77,11 +81,17 @@ def test_bulk_update_existing_settings(self) -> None:

settings1 = RepositorySettings.objects.get(repository=repo1)
assert settings1.enabled_code_review is True
assert settings1.code_review_triggers == ["on_new_commit"]
assert settings1.code_review_triggers == [
CodeReviewTrigger.ON_NEW_COMMIT,
CodeReviewTrigger.ON_COMMAND_PHRASE,
]

settings2 = RepositorySettings.objects.get(repository=repo2)
assert settings2.enabled_code_review is True
assert settings2.code_review_triggers == ["on_new_commit"]
assert settings2.code_review_triggers == [
CodeReviewTrigger.ON_NEW_COMMIT,
CodeReviewTrigger.ON_COMMAND_PHRASE,
]

def test_repository_not_found(self) -> None:
response = self.client.put(
Expand Down Expand Up @@ -168,7 +178,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,
],
)
Expand All @@ -191,11 +201,15 @@ 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 == [
CodeReviewTrigger.ON_READY_FOR_REVIEW,
CodeReviewTrigger.ON_NEW_COMMIT,
CodeReviewTrigger.ON_COMMAND_PHRASE,
]

settings2 = RepositorySettings.objects.get(repository=repo2)
assert settings2.enabled_code_review is True
assert settings2.code_review_triggers == []
assert settings2.code_review_triggers == [CodeReviewTrigger.ON_COMMAND_PHRASE]

def test_partial_bulk_update_code_review_triggers_preserves_enabled(self) -> None:
repo1 = Repository.objects.create(name="repo1", organization_id=self.org.id)
Expand All @@ -204,7 +218,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,
Expand All @@ -228,13 +242,21 @@ def test_partial_bulk_update_code_review_triggers_preserves_enabled(self) -> Non

settings1 = RepositorySettings.objects.get(repository=repo1)
assert settings1.enabled_code_review is True
assert settings1.code_review_triggers == ["on_new_commit", "on_ready_for_review"]
assert settings1.code_review_triggers == [
CodeReviewTrigger.ON_NEW_COMMIT,
CodeReviewTrigger.ON_READY_FOR_REVIEW,
CodeReviewTrigger.ON_COMMAND_PHRASE,
]

settings2 = RepositorySettings.objects.get(repository=repo2)
assert settings2.enabled_code_review is False
assert settings2.code_review_triggers == ["on_new_commit", "on_ready_for_review"]
assert settings2.code_review_triggers == [
CodeReviewTrigger.ON_NEW_COMMIT,
CodeReviewTrigger.ON_READY_FOR_REVIEW,
CodeReviewTrigger.ON_COMMAND_PHRASE,
]

def test_partial_bulk_create_enabled_code_review_uses_default(self) -> None:
def test_partial_bulk_create_code_review_triggers_uses_default(self) -> None:
repo1 = Repository.objects.create(name="repo1", organization_id=self.org.id)
repo2 = Repository.objects.create(name="repo2", organization_id=self.org.id)

Expand All @@ -251,13 +273,13 @@ def test_partial_bulk_create_enabled_code_review_uses_default(self) -> None:

settings1 = RepositorySettings.objects.get(repository=repo1)
assert settings1.enabled_code_review is True
assert settings1.code_review_triggers == []
assert settings1.code_review_triggers == [CodeReviewTrigger.ON_COMMAND_PHRASE]

settings2 = RepositorySettings.objects.get(repository=repo2)
assert settings2.enabled_code_review is True
assert settings2.code_review_triggers == []
assert settings2.code_review_triggers == [CodeReviewTrigger.ON_COMMAND_PHRASE]

def test_partial_bulk_create_code_review_triggers_uses_default(self) -> None:
def test_partial_bulk_create_enabled_code_review_uses_default(self) -> None:
repo1 = Repository.objects.create(name="repo1", organization_id=self.org.id)
repo2 = Repository.objects.create(name="repo2", organization_id=self.org.id)

Expand All @@ -277,11 +299,19 @@ def test_partial_bulk_create_code_review_triggers_uses_default(self) -> None:

settings1 = RepositorySettings.objects.get(repository=repo1)
assert settings1.enabled_code_review is False
assert settings1.code_review_triggers == ["on_new_commit", "on_ready_for_review"]
assert settings1.code_review_triggers == [
CodeReviewTrigger.ON_NEW_COMMIT,
CodeReviewTrigger.ON_READY_FOR_REVIEW,
CodeReviewTrigger.ON_COMMAND_PHRASE,
]

settings2 = RepositorySettings.objects.get(repository=repo2)
assert settings2.enabled_code_review is False
assert settings2.code_review_triggers == ["on_new_commit", "on_ready_for_review"]
assert settings2.code_review_triggers == [
CodeReviewTrigger.ON_NEW_COMMIT,
CodeReviewTrigger.ON_READY_FOR_REVIEW,
CodeReviewTrigger.ON_COMMAND_PHRASE,
]

def test_audit_log_created_on_update(self) -> None:
repo1 = Repository.objects.create(name="repo1", organization_id=self.org.id)
Expand Down Expand Up @@ -309,4 +339,7 @@ def test_audit_log_created_on_update(self) -> None:
assert audit_log.data["repository_count"] == 2
assert set(audit_log.data["repository_ids"]) == {repo1.id, repo2.id}
assert audit_log.data["enabled_code_review"] is True
assert audit_log.data["code_review_triggers"] == [CodeReviewTrigger.ON_NEW_COMMIT]
assert audit_log.data["code_review_triggers"] == [
CodeReviewTrigger.ON_NEW_COMMIT,
CodeReviewTrigger.ON_COMMAND_PHRASE,
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import pytest

from sentry.models.repository import Repository
from sentry.models.repositorysettings import RepositorySettings
from sentry.testutils.cases import TestMigrations


@pytest.mark.migrations
class BackfillOnCommandPhraseTriggerTest(TestMigrations):
migrate_from = "1014_add_pkce_to_apigrant"
migrate_to = "1015_backfill_on_command_phrase_trigger"

def setup_initial_state(self) -> None:
self.org = self.create_organization()

self.repo_empty = Repository.objects.create(
organization_id=self.org.id,
name="test-repo",
provider="integrations:github",
)

self.repo_with_triggers = Repository.objects.create(
organization_id=self.org.id,
name="test-repo-2",
provider="integrations:github",
)

self.repo_already_has = Repository.objects.create(
organization_id=self.org.id,
name="test-repo-3",
provider="integrations:github",
)

def setup_before_migration(self, apps) -> None:
RepositorySettings = apps.get_model("sentry", "RepositorySettings")

self.setting_empty = RepositorySettings.objects.create(
repository_id=self.repo_empty.id,
enabled_code_review=True,
code_review_triggers=[],
)

self.setting_with_triggers = RepositorySettings.objects.create(
repository_id=self.repo_with_triggers.id,
enabled_code_review=True,
code_review_triggers=["on_new_commit", "on_ready_for_review"],
)

self.setting_already_has = RepositorySettings.objects.create(
repository_id=self.repo_already_has.id,
enabled_code_review=True,
code_review_triggers=["on_command_phrase", "on_new_commit"],
)

def test_backfills_on_command_phrase_trigger(self) -> None:
setting_empty = RepositorySettings.objects.get(id=self.setting_empty.id)
assert set(setting_empty.code_review_triggers) == {"on_command_phrase"}
assert len(setting_empty.code_review_triggers) == 1

setting_with_triggers = RepositorySettings.objects.get(id=self.setting_with_triggers.id)
assert set(setting_with_triggers.code_review_triggers) == {
"on_command_phrase",
"on_new_commit",
"on_ready_for_review",
}
assert len(setting_with_triggers.code_review_triggers) == 3

setting_already_has = RepositorySettings.objects.get(id=self.setting_already_has.id)
assert setting_already_has.code_review_triggers.count("on_command_phrase") == 1
assert set(setting_already_has.code_review_triggers) == {
"on_command_phrase",
"on_new_commit",
}
Loading
Loading