Skip to content
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

chore(alerts): Remove fe code for activated alerts #81218

Merged
merged 3 commits into from
Jan 6, 2025

Conversation

ceorourke
Copy link
Member

@ceorourke ceorourke commented Nov 22, 2024

Remove the front end for activated alerts as it's being deprecated. I can't merge this until we clean up a couple live alerts but I wanted to get the PRs ready. The gigantic backend counterpart is here #81095

Before
Screenshot 2024-11-25 at 1 25 54 PM

After
Screenshot 2024-11-25 at 1 27 26 PM

@ceorourke ceorourke requested a review from a team as a code owner November 22, 2024 22:12
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Nov 22, 2024
Copy link

codecov bot commented Nov 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #81218      +/-   ##
==========================================
+ Coverage   80.25%   80.34%   +0.08%     
==========================================
  Files        7447     7218     -229     
  Lines      329627   319477   -10150     
  Branches    20779    20765      -14     
==========================================
- Hits       264540   256681    -7859     
+ Misses      64693    62402    -2291     
  Partials      394      394              

Copy link
Contributor

@saponifi3d saponifi3d left a comment

Choose a reason for hiding this comment

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

code changes lgtm - just the question about the visual.

just fyi - for ui PRs it's also helpful to attach before / after screen shots to illustrate the changes.

@getsantry
Copy link
Contributor

getsantry bot commented Dec 26, 2024

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you add the label WIP, I will leave it alone unless WIP is removed ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry bot added Stale and removed Stale labels Dec 26, 2024
@ceorourke ceorourke merged commit 8ff2196 into master Jan 6, 2025
49 checks passed
@ceorourke ceorourke deleted the ceorourke/rm-fe-activated-alerts branch January 6, 2025 19:55
ceorourke added a commit that referenced this pull request Jan 6, 2025
Another chunk of https://github.com/getsentry/sentry/pull/81095/files
that removes activated alerts from serializers.

This can't be merged until
#81218 is merged.
ceorourke added a commit that referenced this pull request Jan 6, 2025
Another chunk of #81095 that
removes the notification code for activated alerts.

This can't be merged until
#81218 is merged.
ceorourke added a commit that referenced this pull request Jan 7, 2025
I unintentionally removed the interval select field on metric alerts
when removing the monitor type in
#81218 this PR puts it back

<img width="1077" alt="Screenshot 2025-01-07 at 9 45 10 AM"
src="https://github.com/user-attachments/assets/d249600f-665a-43f3-bcfa-d6f2c567cfdd"
/>

---------

Co-authored-by: Nate Moore <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Do Not Merge Don't merge 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