Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 15, 2023

Conversation

ceorourke
Copy link
Member

@ceorourke ceorourke commented Aug 14, 2023

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) 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. 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:

Screenshot 2023-08-14 at 4 35 15 PM and here: Screenshot 2023-08-14 at 4 35 48 PM

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.

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Aug 14, 2023
@ceorourke ceorourke marked this pull request as ready for review August 15, 2023 00:00
@ceorourke ceorourke requested a review from a team as a code owner August 15, 2023 00:00
@ceorourke ceorourke merged commit 123c836 into master Aug 15, 2023
@ceorourke ceorourke deleted the ceorourke/rm-name-data-alert-rule branch August 15, 2023 16:40
ceorourke added a commit that referenced this pull request Aug 16, 2023
Backend piece to #54739 to make
sure we aren't saving the `name` field to the `Rule` table's `data`
column.
ceorourke added a commit that referenced this pull request Aug 29, 2023
This is a follow up to #54739
and #54746 to normalize the
issue alert rule data to not have the unnecessary "name" field.

We have 1,413,825 rows to check so I'm marking it as dangerous.

Closes #55115
@github-actions github-actions bot locked and limited conversation to collaborators Aug 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants