Repository: getsentry/sentry
Pull request: feat(rule): Create NeglectedAlert table
Create a new `NeglectedAlert` table to store data about rules that haven't been clicked on in X period of time to schedule them for a migration that will disable the rule. 

Spec [here](https://www.notion.so/sentry/Disable-Neglected-Alerts-500e904502944d8e963fcd8a693e38bd) for more context.

Closes #56190 

Problem statement:
[BE] Write a migration to create the new `NeglectedAlert` table
- Make a new table `NeglectedAlert`
    - `rule_id` nullable False
    - `org_id`
    - `disable_date` will be “now” (when the script is run) + 14 days nullable False
    - `opted_out` boolean so when we re-run we don’t grab the same alert default False
    - `has_sent_initial_email` boolean default False
    - `has_sent_final_email` boolean default False

Hints from issue discussion:

Commit message: add org id, make model name singular, add to a test

Patch to review:
diff --git a/fixtures/backup/model_dependencies/detailed.json b/fixtures/backup/model_dependencies/detailed.json
index d464f821d230a0..d8394b4df50dfc 100644
--- a/fixtures/backup/model_dependencies/detailed.json
+++ b/fixtures/backup/model_dependencies/detailed.json
@@ -1802,6 +1802,23 @@
       "Region"
     ]
   },
+  "sentry.NeglectedRule": {
+    "foreign_keys": {
+      "organization_id": {
+        "kind": "FlexibleForeignKey",
+        "model": "sentry.Organization"
+      },
+      "rule_id": {
+        "kind": "FlexibleForeignKey",
+        "model": "sentry.Rule"
+      }
+    },
+    "model": "sentry.NeglectedRule",
+    "relocation_scope": "Organization",
+    "silos": [
+      "Region"
+    ]
+  },
   "sentry.NotificationAction": {
     "foreign_keys": {
       "integration_id": {
diff --git a/fixtures/backup/model_dependencies/flat.json b/fixtures/backup/model_dependencies/flat.json
index 7b3670c5774031..37ec2e99692551 100644
--- a/fixtures/backup/model_dependencies/flat.json
+++ b/fixtures/backup/model_dependencies/flat.json
@@ -406,6 +406,10 @@
     "sentry.Monitor"
   ],
   "sentry.MonitorLocation": [],
+  "sentry.NeglectedRule": [
+    "sentry.Organization",
+    "sentry.Rule"
+  ],
   "sentry.NotificationAction": [
     "sentry.Integration",
     "sentry.Organization",
diff --git a/fixtures/backup/model_dependencies/sorted.json b/fixtures/backup/model_dependencies/sorted.json
index ce01ea9cd917f4..bf689595d98fdc 100644
--- a/fixtures/backup/model_dependencies/sorted.json
+++ b/fixtures/backup/model_dependencies/sorted.json
@@ -176,6 +176,7 @@
   "sentry.ReleaseProjectEnvironment",
   "sentry.Rule",
   "sentry.RuleActivity",
+  "sentry.NeglectedRule",
   "sentry.RuleFireHistory",
   "sentry.ServiceHook",
   "sentry.UserReport",
diff --git a/migrations_lockfile.txt b/migrations_lockfile.txt
index 3cf2fef3a43123..de5b3a1aaad826 100644
--- a/migrations_lockfile.txt
+++ b/migrations_lockfile.txt
@@ -8,5 +8,5 @@ will then be regenerated, and you should be able to merge without conflicts.
 feedback: 0001_feedback
 nodestore: 0002_nodestore_no_dictfield
 replays: 0003_add_size_to_recording_segment
-sentry: 0551_drop_xactor_actor
+sentry: 0552_create_neglectedalert_table
 social_auth: 0002_default_auto_field
diff --git a/src/sentry/migrations/0552_create_neglectedalert_table.py b/src/sentry/migrations/0552_create_neglectedalert_table.py
new file mode 100644
index 00000000000000..a7d21de0fd6fc5
--- /dev/null
+++ b/src/sentry/migrations/0552_create_neglectedalert_table.py
@@ -0,0 +1,60 @@
+# Generated by Django 3.2.20 on 2023-09-13 21:17
+
+import django.db.models.deletion
+from django.db import migrations, models
+
+import sentry.db.models.fields.bounded
+import sentry.db.models.fields.foreignkey
+import sentry.models.rule
+from sentry.new_migrations.migrations import CheckedMigration
+
+
+class Migration(CheckedMigration):
+    # This flag is used to mark that a migration shouldn't be automatically run in production. For
+    # the most part, 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 dangerous:
+    # - Large data migrations. Typically we want these to be run manually by ops 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
+    #   have ops run this and not block the deploy. Note that while adding an index is a schema
+    #   change, it's completely safe to run the operation after the code has deployed.
+    is_dangerous = False
+
+    dependencies = [
+        ("sentry", "0551_drop_xactor_actor"),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name="NeglectedRule",
+            fields=[
+                (
+                    "id",
+                    sentry.db.models.fields.bounded.BoundedBigAutoField(
+                        primary_key=True, serialize=False
+                    ),
+                ),
+                ("disable_date", models.DateTimeField(default=sentry.models.rule.in_fourteen_days)),
+                ("opted_out", models.BooleanField(default=False)),
+                ("has_sent_initial_email", models.BooleanField(default=False)),
+                ("has_sent_final_email", models.BooleanField(default=False)),
+                (
+                    "organization_id",
+                    sentry.db.models.fields.foreignkey.FlexibleForeignKey(
+                        on_delete=django.db.models.deletion.CASCADE, to="sentry.organization"
+                    ),
+                ),
+                (
+                    "rule_id",
+                    sentry.db.models.fields.foreignkey.FlexibleForeignKey(
+                        on_delete=django.db.models.deletion.CASCADE, to="sentry.rule"
+                    ),
+                ),
+            ],
+            options={
+                "abstract": False,
+            },
+        ),
+    ]
diff --git a/src/sentry/models/rule.py b/src/sentry/models/rule.py
index 3f030a20ef956c..7f47b862e0ec44 100644
--- a/src/sentry/models/rule.py
+++ b/src/sentry/models/rule.py
@@ -1,3 +1,4 @@
+from datetime import timedelta
 from enum import Enum, IntEnum
 from typing import Sequence, Tuple
 
@@ -130,3 +131,19 @@ class RuleActivity(Model):
     class Meta:
         app_label = "sentry"
         db_table = "sentry_ruleactivity"
+
+
+def in_fourteen_days():
+    return timezone.now() + timedelta(days=14)
+
+
+@region_silo_only_model
+class NeglectedRule(Model):
+    __relocation_scope__ = RelocationScope.Organization
+
+    rule_id = FlexibleForeignKey("sentry.Rule")
+    organization_id = FlexibleForeignKey("sentry.Organization")
+    disable_date = models.DateTimeField(default=in_fourteen_days)
+    opted_out = models.BooleanField(default=False)
+    has_sent_initial_email = models.BooleanField(default=False)
+    has_sent_final_email = models.BooleanField(default=False)
diff --git a/tests/sentry/backup/test_models.py b/tests/sentry/backup/test_models.py
index 6fbcdfd9b284e8..c05f0404fcd23b 100644
--- a/tests/sentry/backup/test_models.py
+++ b/tests/sentry/backup/test_models.py
@@ -66,7 +66,7 @@
 from sentry.models.recentsearch import RecentSearch
 from sentry.models.relay import Relay, RelayUsage
 from sentry.models.repository import Repository
-from sentry.models.rule import Rule, RuleActivity, RuleActivityType
+from sentry.models.rule import NeglectedRule, Rule, RuleActivity, RuleActivityType
 from sentry.models.rulesnooze import RuleSnooze
 from sentry.models.savedsearch import SavedSearch, Visibility
 from sentry.models.search_common import SearchType
@@ -439,7 +439,7 @@ def test_relay(self):
         RelayUsage.objects.create(relay_id=relay_id, version="0.0.1", public_key=public_key)
         return self.import_export_then_validate()
 
-    @targets(mark(Rule, RuleActivity, RuleSnooze))
+    @targets(mark(Rule, RuleActivity, RuleSnooze, NeglectedRule))
     def test_rule(self):
         rule = self.create_project_rule(project=self.project)
         RuleActivity.objects.create(rule=rule, type=RuleActivityType.CREATED.value)


Code review comments:
diff_hunk: @@ -130,3 +131,19 @@ class RuleActivity(Model):
     class Meta:
         app_label = "sentry"
         db_table = "sentry_ruleactivity"
+
+
+def in_fourteen_days():
path: src/sentry/models/rule.py
text: @user1:
Maybe leave a comment that this is used in migrations and can't be removed/moved.

@author:
@user1 if for some reason we need to change it, we can edit the function itself right? It'd just be weird to have `in_fourteen_days` set 30 days (or some other number)?

@author:
Thought about it more and I'm just gonna remove it - I was doing it to make it a teeny bit easier when populating to not have to find 14 days in the future but it's not a big deal.

diff_hunk: @@ -0,0 +1,60 @@
+# Generated by Django 3.2.20 on 2023-09-13 21:17
+
+import django.db.models.deletion
+from django.db import migrations, models
+
+import sentry.db.models.fields.bounded
+import sentry.db.models.fields.foreignkey
+import sentry.models.rule
+from sentry.new_migrations.migrations import CheckedMigration
+
+
+class Migration(CheckedMigration):
+    # This flag is used to mark that a migration shouldn't be automatically run in production. For
+    # the most part, 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 dangerous:
+    # - Large data migrations. Typically we want these to be run manually by ops 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
+    #   have ops run this and not block the deploy. Note that while adding an index is a schema
+    #   change, it's completely safe to run the operation after the code has deployed.
+    is_dangerous = False
+
+    dependencies = [
+        ("sentry", "0551_drop_xactor_actor"),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name="NeglectedRule",
+            fields=[
+                (
+                    "id",
+                    sentry.db.models.fields.bounded.BoundedBigAutoField(
+                        primary_key=True, serialize=False
+                    ),
+                ),
+                ("disable_date", models.DateTimeField(default=sentry.models.rule.in_fourteen_days)),
path: src/sentry/migrations/0552_create_neglectedalert_table.py
text: @user1:
I would be careful with this. You can never move or rename that function as long as this migration lives.