Feature/alert muting part 2#642
Merged
erikdarlingdata merged 4 commits intoMar 23, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughParses alert DetailText into AlertMuteContext (database/query/wait/job) before creating or matching mute rules, adds a user-configurable default expiration for new mute rules (UI, persistence, and dialog defaulting), and flattens multi-line text before truncation in MainWindow. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Dashboard/Models/MuteRule.cs`:
- Around line 102-119: The XML doc for PopulateFromDetailText claims it extracts
"Job Name" but the method only parses Database, Query and Wait Type; either
update the summary to remove "Job Name" or add parsing logic for the Job Name
field. To fix, locate the PopulateFromDetailText method and either (A) change
the XML summary to list the three fields actually parsed (Database, Query, Wait
Type), or (B) implement parsing for a "Job Name: " line by checking
trimmed.StartsWith("Job Name: ", StringComparison.Ordinal) and assigning the
value to the JobName property (matching how DatabaseName, QueryText and WaitType
are handled). Ensure you reference the DatabaseName, QueryText, WaitType and
JobName symbols consistently.
In `@Lite/Models/MuteRule.cs`:
- Around line 102-119: The XML doc for PopulateFromDetailText incorrectly claims
it extracts "Job Name" while the method only sets DatabaseName, QueryText, and
WaitType; either update the summary to remove "Job Name" or extend
PopulateFromDetailText to parse "Job Name: " lines by adding a branch similar to
the others (check trimmed.StartsWith("Job Name: ", StringComparison.Ordinal) and
assign the value to the JobName property), referencing the
PopulateFromDetailText method and the JobName, DatabaseName, QueryText, and
WaitType members when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4c533fa7-5cea-4f35-aa55-ad4c09488fe3
📒 Files selected for processing (13)
Dashboard/Controls/AlertsHistoryContent.xaml.csDashboard/MainWindow.xaml.csDashboard/Models/MuteRule.csDashboard/Models/UserPreferences.csDashboard/MuteRuleDialog.xaml.csDashboard/SettingsWindow.xamlDashboard/SettingsWindow.xaml.csLite/App.xaml.csLite/Controls/AlertsHistoryTab.xaml.csLite/Models/MuteRule.csLite/Windows/MuteRuleDialog.xaml.csLite/Windows/SettingsWindow.xamlLite/Windows/SettingsWindow.xaml.cs
Contributor
Author
|
I'm just working through improving the query text being shown in the 'Query Text' Match Criteria in the 'Create Mute Rule' dialog box. |
Contributor
Author
|
Pushed three follow-up commits addressing the review feedback and a bug found during testing: 1. Add missing Job Name parsing (
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
Lite/Windows/MuteRuleDialog.xaml (2)
100-101: Wrap tooltip content for readability.The new guidance is helpful; consider using a wrapped
TextBlocktooltip so long text doesn’t render as a very wide single line.Suggested tooltip formatting
- <TextBox Grid.Row="3" Grid.Column="0" x:Name="QueryTextPatternBox" Margin="0,0,0,8" - ToolTip="Alerts whose query text contains this value (case-insensitive) will be muted. Use a distinctive fragment such as a table name or procedure call."/> + <TextBox Grid.Row="3" Grid.Column="0" x:Name="QueryTextPatternBox" Margin="0,0,0,8"> + <TextBox.ToolTip> + <TextBlock MaxWidth="320" + TextWrapping="Wrap" + Text="Alerts whose query text contains this value (case-insensitive) will be muted. Use a distinctive fragment such as a table name or procedure call."/> + </TextBox.ToolTip> + </TextBox>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Lite/Windows/MuteRuleDialog.xaml` around lines 100 - 101, The Tooltip on QueryTextPatternBox currently uses a plain string which renders as one long line; change it to a wrapped TextBlock so the tooltip wraps and limits width for readability by replacing the ToolTip attribute on QueryTextPatternBox with a ToolTip element containing a TextBlock (set TextWrapping="Wrap" and a reasonable MaxWidth, e.g., 300) and move the long guidance text into that TextBlock; update the XAML for the QueryTextPatternBox control accordingly.
45-50: Avoid hardcodedSelectedIndexfor expiration defaults.Line 45 hardcodes
SelectedIndex="3"(Never (permanent)), which can drift from the configurable default and from future item reordering. Prefer letting code-behind set the initial selection explicitly (with a safe fallback).Suggested XAML adjustment
- <ComboBox Grid.Row="5" x:Name="ExpirationCombo" SelectedIndex="3" Margin="0,0,0,12"> + <ComboBox Grid.Row="5" x:Name="ExpirationCombo" Margin="0,0,0,12">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Lite/Windows/MuteRuleDialog.xaml` around lines 45 - 50, Remove the hardcoded SelectedIndex="3" from the ExpirationCombo XAML and instead set the initial selection in the MuteRuleDialog code-behind (e.g., in the MuteRuleDialog constructor or Loaded handler). Read the configured/default expiration value (or index) from the existing settings API, map it to an ExpirationCombo item (by index or by matching ComboBoxItem.Content), and set ExpirationCombo.SelectedIndex with a safe fallback (e.g., if configured index is out of range, choose the last item or a sensible default). Ensure this uses the ExpirationCombo control name so future reordering of XAML items won’t break the default.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Lite/Windows/MuteRuleDialog.xaml`:
- Around line 100-101: The Tooltip on QueryTextPatternBox currently uses a plain
string which renders as one long line; change it to a wrapped TextBlock so the
tooltip wraps and limits width for readability by replacing the ToolTip
attribute on QueryTextPatternBox with a ToolTip element containing a TextBlock
(set TextWrapping="Wrap" and a reasonable MaxWidth, e.g., 300) and move the long
guidance text into that TextBlock; update the XAML for the QueryTextPatternBox
control accordingly.
- Around line 45-50: Remove the hardcoded SelectedIndex="3" from the
ExpirationCombo XAML and instead set the initial selection in the MuteRuleDialog
code-behind (e.g., in the MuteRuleDialog constructor or Loaded handler). Read
the configured/default expiration value (or index) from the existing settings
API, map it to an ExpirationCombo item (by index or by matching
ComboBoxItem.Content), and set ExpirationCombo.SelectedIndex with a safe
fallback (e.g., if configured index is out of range, choose the last item or a
sensible default). Ensure this uses the ExpirationCombo control name so future
reordering of XAML items won’t break the default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5103dbd7-7f34-4a51-b217-b3f29798a421
📒 Files selected for processing (2)
Dashboard/MuteRuleDialog.xamlLite/Windows/MuteRuleDialog.xaml
✅ Files skipped from review due to trivial changes (1)
- Dashboard/MuteRuleDialog.xaml
Owner
|
Hey @HannahVernon — this has merge conflicts with dev after the recent Velopack integration, data directory migration, and time slicer PRs. Could you rebase onto current dev when you get a chance? The conflicting files are mostly App.xaml.cs, MainWindow.xaml.cs, and SettingsWindow.xaml. Thanks! |
- Parse detail_text to extract Database, Query Text, and Wait Type when using 'Mute This Alert' from alert history (both editions) - Add PopulateFromDetailText() to AlertMuteContext for structured field extraction from the label: value format - Add 'Default expiration for new mute rules' dropdown to Settings in both editions (1 hour, 24 hours, 7 days, Never; default 24h) - MuteRuleDialog now selects the configured default expiration instead of always defaulting to 'Never' - Persist setting as mute_rule_default_expiration in settings.json (Lite) and preferences.json (Dashboard) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The XML doc claimed Job Name extraction but the parser did not implement it. Add the missing branch in both Dashboard and Lite editions so the behavior matches the documentation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Collapse newlines in Truncate/TruncateText so detail_text fields stay single-line in the label: value format - Handle multi-line query values in PopulateFromDetailText by accumulating continuation lines until the next indented field - Recognize variant query labels (Blocked Query, Blocking Query, Victim SQL) in addition to Query Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Explain that the field is a case-insensitive substring match and suggest entering a distinctive fragment like a table or procedure name. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
HannahVernon
force-pushed
the
feature/alert-muting-part-2
branch
from
b09adae to
b711025
Compare
Contributor
Author
|
@erikdarlingdata - rebased 👍 |
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Improves the Alert Muting feature by prefilling certain muting filter fields when adding a mute filter from the Alert History window. Automatically filled fields include the database name, the wait type, and query text. The filter fields available depend on the type of alert being muted; long-running-queries have different context than TempDB usage, for example. This PR also adds a configurable default expiration period for alerts.
Which component(s) does this affect?
How was this tested?
Tested on multiple SQL Server 2022 Enterprise and SQL Server 2019 VM-based instances inside and outside Azure.
Checklist
dotnet build -c Debug)Summary by CodeRabbit
New Features
Bug Fixes