-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
ref(crons): Disable mute button when monitor has no environments (stage 5) #103568
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
ref(crons): Disable mute button when monitor has no environments (stage 5) #103568
Conversation
[NEW-564: There needs to be some way to mute the entire cron detector](https://linear.app/getsentry/issue/NEW-564/there-needs-to-be-some-way-to-mute-the-entire-cron-detector) Since muting now operates at the environment level, disallow muting a monitor when it has no environments (as it would do nothing). Shows a helpful tooltip explaining that muting is only available when there are monitor environments.
| muteDisableProps.title = t( | ||
| 'Muting is only available when there are monitor environments' | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: UI Masks True Reason for Disabled Button
When a monitor has no environments, the muteDisableProps.title is unconditionally overwritten, which loses the permission tooltip if the user lacks edit access. When both conditions are true (no permission and no environments), users see the environment message instead of the permission message, masking the actual reason the button is disabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's OK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree - the user can fix the env, then the permissions error will show up. might be nice if you could see both, but doesn't seem vital.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well it would be, the user creates an env, then they see they can't mute the monitor.
either way doesn't matter, this will go away in the future
saponifi3d
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
![]()
| muteDisableProps.title = t( | ||
| 'Muting is only available when there are monitor environments' | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree - the user can fix the env, then the permissions error will show up. might be nice if you could see both, but doesn't seem vital.
NEW-564: There needs to be some way to mute the entire cron detector
Since muting now operates at the environment level, disallow muting a monitor when it has no environments (as it would do nothing). Shows a helpful tooltip explaining that muting is only available when there are monitor environments.
Important
Not to be merged until the backend components are all released