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.

Sign up for GitHub

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

Jump to bottom

[Set-DbaAgentAlert] - Added ShouldProcess to the Rename to fix #8137 #8138

Merged
merged 4 commits into from
Feb 4, 2022
Merged

[Set-DbaAgentAlert] - Added ShouldProcess to the Rename to fix #8137 #8138

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2ae6d61
Added ShouldProcess to the Rename
MikeyBronowski Feb 3, 2022
8f5e2e6
Fixes for multiple alerts supplied
MikeyBronowski Feb 3, 2022
45ae9db
Copy-Paste is wrong
MikeyBronowski Feb 3, 2022
3fde480
Reverted the change by formatter
MikeyBronowski Feb 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 5 additions & 4 deletions functions/Set-DbaAgentAlert.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,9 @@ function Set-DbaAgentAlert {
#region alert options
# Settings the options for the alert
if ($NewName) {
Write-Message -Message "Setting alert name to $NewName" -Level Verbose
$currentAlert.Rename($NewName)
if ($Pscmdlet.ShouldProcess($server, "Setting alert name to $NewName for $currentAlert")) {
$currentAlert.Rename($NewName)
}
}

if ($Enabled) {
Expand All @@ -149,9 +150,9 @@ function Set-DbaAgentAlert {
#endregion alert options

# Execute
if ($PSCmdlet.ShouldProcess($SqlInstance, "Changing the alert $a")) {
if ($PSCmdlet.ShouldProcess($SqlInstance, "Committing changes for alert $a")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Like in line 135, this should not be $a but $currentAlert.

try {
Write-Message -Message "Changing the alert" -Level Verbose
Write-Message -Message "Committing changes for alert $a" -Level Verbose
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need this line, as ShouldProcess also has verbose output. (But if you still like to have the line: Like in line 135, this should not be $a but $currentAlert.)


# Change the alert
$currentAlert.Alter()
Expand Down