Skip to content

Commit 123c836

Browse files
authored
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](#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.
1 parent 0298846 commit 123c836

File tree

3 files changed

+8
-8
lines changed

3 files changed

+8
-8
lines changed

Diff for: static/app/types/alerts.tsx

-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ export interface IssueAlertRuleActionTemplate {
2828
enabled: boolean;
2929
id: string;
3030
label: string;
31-
name: string;
3231
actionType?: 'ticket' | 'sentryapp';
3332
formFields?:
3433
| {

Diff for: static/app/views/alerts/create.spec.tsx

-6
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,6 @@ describe('ProjectAlertsCreate', function () {
169169
conditions: [
170170
expect.objectContaining({
171171
id: 'sentry.rules.conditions.first_seen_event.FirstSeenEventCondition',
172-
name: 'A new issue is created',
173172
}),
174173
],
175174
filterMatch: 'all',
@@ -263,7 +262,6 @@ describe('ProjectAlertsCreate', function () {
263262
conditions: [
264263
expect.objectContaining({
265264
id: 'sentry.rules.conditions.first_seen_event.FirstSeenEventCondition',
266-
name: 'A new issue is created',
267265
}),
268266
],
269267
filterMatch: 'all',
@@ -321,7 +319,6 @@ describe('ProjectAlertsCreate', function () {
321319
conditions: [
322320
expect.objectContaining({
323321
id: 'sentry.rules.conditions.first_seen_event.FirstSeenEventCondition',
324-
name: 'A new issue is created',
325322
}),
326323
],
327324
actions: [],
@@ -371,7 +368,6 @@ describe('ProjectAlertsCreate', function () {
371368
conditions: [
372369
expect.objectContaining({
373370
id: 'sentry.rules.conditions.first_seen_event.FirstSeenEventCondition',
374-
name: 'A new issue is created',
375371
}),
376372
],
377373
filterMatch: 'all',
@@ -474,7 +470,6 @@ describe('ProjectAlertsCreate', function () {
474470
conditions: [
475471
expect.objectContaining({
476472
id: 'sentry.rules.conditions.first_seen_event.FirstSeenEventCondition',
477-
name: 'A new issue is created',
478473
}),
479474
],
480475
filterMatch: 'all',
@@ -522,7 +517,6 @@ describe('ProjectAlertsCreate', function () {
522517
conditions: [
523518
expect.objectContaining({
524519
id: 'sentry.rules.conditions.first_seen_event.FirstSeenEventCondition',
525-
name: 'A new issue is created',
526520
}),
527521
],
528522
filterMatch: 'all',

Diff for: static/app/views/alerts/rules/issue/index.tsx

+8-1
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,6 @@ class IssueRuleEditor extends DeprecatedAsyncView<Props, State> {
347347
{
348348
id,
349349
label: CHANGE_ALERT_PLACEHOLDERS_LABELS[id],
350-
name: 'A new issue is created',
351350
},
352351
]);
353352
}
@@ -603,6 +602,14 @@ class IssueRuleEditor extends DeprecatedAsyncView<Props, State> {
603602
if (actionName === 'SlackNotifyServiceAction') {
604603
transaction.setTag(actionName, true);
605604
}
605+
// to avoid storing inconsistent data in the db, don't pass the name fields
606+
delete action.name;
607+
}
608+
for (const condition of rule.conditions) {
609+
delete condition.name;
610+
}
611+
for (const filter of rule.filters) {
612+
delete filter.name;
606613
}
607614
transaction.setData('actions', rule.actions);
608615
}

0 commit comments

Comments
 (0)