Browse Source

ref(alerts): Don't pass "name" field (#54739)

We have inconsistent data stored for issue alert rule data because we
are sometimes saving the "name" field for filters, conditions, and
actions and sometimes not. This is making it difficult to prevent
creation of exact duplicate alerts (which I'm working on
[here](https://github.com/getsentry/sentry/pull/54515)) and makes
queries more complicated since the data stored for the same thing has
variance.

The first case where we store the "name" field is on
`FirstSeenEventCondition` because it is
[hardcoded](https://github.com/getsentry/sentry/blob/master/static/app/views/alerts/rules/issue/index.tsx#L350).
Removing it does not affect the display on the alert details or edit
page.

The second case is when you duplicate an alert. We do a GET request to
the `ProjectRuleDetailsEndpoint` and the serializer adds the "name"
fields so the front end can populate the data shown here:

<img width="360" alt="Screenshot 2023-08-14 at 4 35 15 PM"
src="https://github.com/getsentry/sentry/assets/29959063/d6bc87ac-6bb5-4e3a-a2a3-b2a9ed020d89">
and here:
<img width="666" alt="Screenshot 2023-08-14 at 4 35 48 PM"
src="https://github.com/getsentry/sentry/assets/29959063/a8670b60-d591-48a9-8f99-857052e8e1ad">
 

 We end up with data like:
```
[
    {
        'id': 'sentry.rules.conditions.first_seen_event.FirstSeenEventCondition',
    }, {
       'id': 'sentry.rules.filters.latest_release.LatestReleaseFilter'
    }
]
```
versus
```
[
    {
        'id': 'sentry.rules.conditions.first_seen_event.FirstSeenEventCondition', 
        'name': 'A new issue is created'
    }, {
        'id': 'sentry.rules.filters.latest_release.LatestReleaseFilter',
        'name': 'The event is from the latest release'
    }
]
```
stored in the database which makes it hard to query and compare. We
should not send the "name" field to the backend when creating,
duplicating, or updating an issue alert rule to avoid this.


We have a few places in the backend that are hardcoding this as well
that I'll remove in a separate PR, and then I'll write a migration to
remove it from any existing db rows so we have consistent data.
Colleen O'Rourke 1 year ago
parent
commit
123c8360cf

+ 0 - 1
static/app/types/alerts.tsx

@@ -28,7 +28,6 @@ export interface IssueAlertRuleActionTemplate {
   enabled: boolean;
   id: string;
   label: string;
-  name: string;
   actionType?: 'ticket' | 'sentryapp';
   formFields?:
     | {

+ 0 - 6
static/app/views/alerts/create.spec.tsx

@@ -169,7 +169,6 @@ describe('ProjectAlertsCreate', function () {
               conditions: [
                 expect.objectContaining({
                   id: 'sentry.rules.conditions.first_seen_event.FirstSeenEventCondition',
-                  name: 'A new issue is created',
                 }),
               ],
               filterMatch: 'all',
@@ -263,7 +262,6 @@ describe('ProjectAlertsCreate', function () {
               conditions: [
                 expect.objectContaining({
                   id: 'sentry.rules.conditions.first_seen_event.FirstSeenEventCondition',
-                  name: 'A new issue is created',
                 }),
               ],
               filterMatch: 'all',
@@ -321,7 +319,6 @@ describe('ProjectAlertsCreate', function () {
               conditions: [
                 expect.objectContaining({
                   id: 'sentry.rules.conditions.first_seen_event.FirstSeenEventCondition',
-                  name: 'A new issue is created',
                 }),
               ],
               actions: [],
@@ -371,7 +368,6 @@ describe('ProjectAlertsCreate', function () {
               conditions: [
                 expect.objectContaining({
                   id: 'sentry.rules.conditions.first_seen_event.FirstSeenEventCondition',
-                  name: 'A new issue is created',
                 }),
               ],
               filterMatch: 'all',
@@ -474,7 +470,6 @@ describe('ProjectAlertsCreate', function () {
               conditions: [
                 expect.objectContaining({
                   id: 'sentry.rules.conditions.first_seen_event.FirstSeenEventCondition',
-                  name: 'A new issue is created',
                 }),
               ],
               filterMatch: 'all',
@@ -522,7 +517,6 @@ describe('ProjectAlertsCreate', function () {
               conditions: [
                 expect.objectContaining({
                   id: 'sentry.rules.conditions.first_seen_event.FirstSeenEventCondition',
-                  name: 'A new issue is created',
                 }),
               ],
               filterMatch: 'all',

+ 8 - 1
static/app/views/alerts/rules/issue/index.tsx

@@ -347,7 +347,6 @@ class IssueRuleEditor extends DeprecatedAsyncView<Props, State> {
         {
           id,
           label: CHANGE_ALERT_PLACEHOLDERS_LABELS[id],
-          name: 'A new issue is created',
         },
       ]);
     }
@@ -603,6 +602,14 @@ class IssueRuleEditor extends DeprecatedAsyncView<Props, State> {
           if (actionName === 'SlackNotifyServiceAction') {
             transaction.setTag(actionName, true);
           }
+          // to avoid storing inconsistent data in the db, don't pass the name fields
+          delete action.name;
+        }
+        for (const condition of rule.conditions) {
+          delete condition.name;
+        }
+        for (const filter of rule.filters) {
+          delete filter.name;
         }
         transaction.setData('actions', rule.actions);
       }