-
Notifications
You must be signed in to change notification settings - Fork 227
Set-SqlDscRSSmtpConfiguration: Add new command
#2411
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
Conversation
WalkthroughAdds a new public command Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Script
participant Cmd as Set-SqlDscRSSmtpConfiguration
participant SP as ShouldProcess
participant CIM as Invoke-RsCimMethod
participant Config as MSReportServer_ConfigurationSetting
participant ErrorHandler as Error Handler
User->>Cmd: Invoke with Configuration, SmtpServer, SenderEmailAddress
Cmd->>SP: Check ShouldProcess (or -Force)
alt Confirmed or -Force
SP-->>Cmd: Proceed
Cmd->>CIM: Invoke SetEmailConfiguration(SendUsingSMTPServer=1, SMTPServer, SenderEmailAddress)
CIM->>Config: Execute CIM method
alt Success
Config-->>CIM: Return success
CIM-->>Cmd: Return result
Cmd-->>User: Return Configuration if -PassThru
else Failure
Config-->>CIM: Throw exception
CIM-->>ErrorHandler: Capture error
ErrorHandler->>ErrorHandler: Wrap in New-Exception and New-ErrorRecord (SSRSSC0001)
ErrorHandler-->>Cmd: Terminate with error
Cmd-->>User: Terminating error
end
else Declined
SP-->>Cmd: Cancel
Cmd-->>User: No action taken
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (10)**/*.[Tt]ests.ps1📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-pester.instructions.md)
Files:
⚙️ CodeRabbit configuration file
Files:
tests/Unit/Public/*.[Tt]ests.ps1📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-pester.instructions.md)
Files:
tests/Unit/**/*.Tests.ps1📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
Files:
**/*.{ps1,psm1,psd1}📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
Files:
tests/[Uu]nit/**/*.[Tt]ests.ps1📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md)
Files:
⚙️ CodeRabbit configuration file
Files:
**/Public/**/*.ps1📄 CodeRabbit inference engine (.github/instructions/SqlServerDsc-guidelines.instructions.md)
Files:
**/tests/Unit/**/*.ps1📄 CodeRabbit inference engine (.github/instructions/SqlServerDsc-guidelines.instructions.md)
Files:
**⚙️ CodeRabbit configuration file
Files:
{**/*.ps1,**/*.psm1,**/*.psd1}⚙️ CodeRabbit configuration file
Files:
azure-pipelines.yml📄 CodeRabbit inference engine (.github/instructions/SqlServerDsc-guidelines.instructions.md)
Files:
🧠 Learnings (49)📓 Common learnings📚 Learning: 2025-12-30T15:15:36.079ZApplied to files:
📚 Learning: 2025-12-30T15:15:36.079ZApplied to files:
📚 Learning: 2026-01-01T11:57:15.298ZApplied to files:
📚 Learning: 2025-11-27T17:59:27.205ZApplied to files:
📚 Learning: 2025-11-27T17:59:27.205ZApplied to files:
📚 Learning: 2025-12-30T15:15:36.079ZApplied to files:
📚 Learning: 2025-11-27T17:59:27.205ZApplied to files:
📚 Learning: 2025-12-30T15:15:36.079ZApplied to files:
📚 Learning: 2026-01-16T13:36:13.522ZApplied to files:
📚 Learning: 2025-12-30T15:15:25.261ZApplied to files:
📚 Learning: 2025-11-27T17:59:27.205ZApplied to files:
📚 Learning: 2025-11-27T17:59:27.205ZApplied to files:
📚 Learning: 2025-11-27T17:59:27.205ZApplied to files:
📚 Learning: 2025-11-27T18:00:35.078ZApplied to files:
📚 Learning: 2025-11-27T17:59:27.205ZApplied to files:
📚 Learning: 2026-01-01T11:57:15.298ZApplied to files:
📚 Learning: 2026-01-01T11:57:15.298ZApplied to files:
📚 Learning: 2026-01-01T11:57:15.298ZApplied to files:
📚 Learning: 2025-08-17T10:13:30.079ZApplied to files:
📚 Learning: 2026-01-01T11:57:15.298ZApplied to files:
📚 Learning: 2025-11-27T17:59:27.205ZApplied to files:
📚 Learning: 2025-10-11T08:18:26.062ZApplied to files:
📚 Learning: 2025-10-11T08:35:56.141ZApplied to files:
📚 Learning: 2025-08-16T13:22:15.230ZApplied to files:
📚 Learning: 2025-08-17T10:15:48.194ZApplied to files:
📚 Learning: 2025-11-27T17:59:01.508ZApplied to files:
📚 Learning: 2025-11-27T17:59:27.205ZApplied to files:
📚 Learning: 2025-11-27T17:59:27.205ZApplied to files:
📚 Learning: 2025-11-27T17:59:27.205ZApplied to files:
📚 Learning: 2025-11-27T17:59:27.205ZApplied to files:
📚 Learning: 2025-11-27T17:59:27.205ZApplied to files:
📚 Learning: 2025-12-30T15:15:25.261ZApplied to files:
📚 Learning: 2026-01-16T13:28:29.138ZApplied to files:
📚 Learning: 2026-01-01T11:57:15.298ZApplied to files:
📚 Learning: 2025-11-27T17:59:27.205ZApplied to files:
📚 Learning: 2026-01-01T11:57:15.298ZApplied to files:
📚 Learning: 2025-12-30T15:15:25.261ZApplied to files:
📚 Learning: 2025-11-27T17:59:01.508ZApplied to files:
📚 Learning: 2025-11-27T18:56:46.759ZApplied to files:
📚 Learning: 2025-12-30T15:15:25.261ZApplied to files:
📚 Learning: 2026-01-01T11:57:15.298ZApplied to files:
📚 Learning: 2025-12-30T15:15:25.261ZApplied to files:
📚 Learning: 2025-12-30T15:15:25.261ZApplied to files:
📚 Learning: 2025-11-27T18:00:35.078ZApplied to files:
📚 Learning: 2026-01-16T13:36:13.522ZApplied to files:
📚 Learning: 2025-12-30T15:15:25.261ZApplied to files:
📚 Learning: 2026-01-01T11:57:15.298ZApplied to files:
📚 Learning: 2025-11-27T17:59:27.205ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (9)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2411 +/- ##
=====================================
Coverage 94% 94%
=====================================
Files 217 218 +1
Lines 10926 10981 +55
=====================================
+ Hits 10305 10358 +53
- Misses 621 623 +2
🚀 New features to boost your workflow:
|
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.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
source/Public/Restart-SqlDscRSService.ps1 (1)
133-140: Missing-Exceptionparameter in catch block.Per coding guidelines, catch blocks using
Write-Errorshould pass the original exception using-Exception. This preserves the original exception context for debugging and is consistent with other error handling in this PR.Proposed fix
catch { $errorMessage = $script:localizedData.Restart_SqlDscRSService_ServiceNotFound -f $targetServiceName - Write-Error -Message $errorMessage -Category ObjectNotFound -ErrorId 'RSRSRS0001' -TargetObject $targetServiceName + Write-Error -Message $errorMessage -Category ObjectNotFound -ErrorId 'RSRSRS0001' -TargetObject $targetServiceName -Exception $_.Exception return }
🤖 Fix all issues with AI agents
In `@azure-pipelines.yml`:
- Line 581: Add the missing integration test file
Set-SqlDscRSSmtpConfiguration.Integration.Tests.ps1 to the BI Report Server
stage in azure-pipelines.yml by inserting it into Group 3 immediately after
Restart-SqlDscRSService.Integration.Tests.ps1 so the BI Report Server stage has
the same coverage as the Reporting Services stage.
In
`@tests/Integration/Commands/Set-SqlDscRSSmtpConfiguration.Integration.Tests.ps1`:
- Around line 40-44: The It blocks call Set-SqlDscRSSmtpConfiguration and emit
unused output; suppress this by capturing the command output into $null in each
It block (e.g., replace direct invocation of Set-SqlDscRSSmtpConfiguration with
assignment to $null) for the tests at the locations referencing the
Set-SqlDscRSSmtpConfiguration calls so all It blocks (including the ones at the
other indicated ranges) remain quiet and follow Pester guidance.
- Around line 26-32: Add setup/teardown for the SQL engine connection inside the
existing BeforeAll/AfterAll blocks: in BeforeAll (where Import-Module is called)
invoke Connect-SqlDscDatabaseEngine to establish the DB session and ensure
errors stop the run, and in AfterAll call Disconnect-SqlDscDatabaseEngine to
close the session; keep the existing note about not using -Force/unloading
module, and ensure Connect-SqlDscDatabaseEngine is called before tests run and
Disconnect-SqlDscDatabaseEngine runs after all tests to comply with integration
test rules.
In `@tests/Unit/Public/Set-SqlDscRSSmtpConfiguration.Tests.ps1`:
- Around line 75-77: The tests are leaking command output because the
Set-SqlDscRSSmtpConfiguration calls (invoked on $mockCimInstance) are not
capturing their return values; change each invocation in the It blocks to assign
the command result to $null (for example, replace standalone "$mockCimInstance |
Set-SqlDscRSSmtpConfiguration ..." with "$null = $mockCimInstance |
Set-SqlDscRSSmtpConfiguration ...") so output is not written to the pipeline;
apply this same pattern to the other It blocks mentioned (the calls around the
119-121, 152-153, and 168-169 ranges) where the result is not asserted.
- Around line 4-24: The test data currently passed via -ForEach should be
declared inside the BeforeDiscovery block and referenced from the It block:
create a descriptive array variable (e.g. $testCases or $cases) inside
BeforeDiscovery, populate it with the data used by the -ForEach, and then remove
-ForEach from the It block and reference that array variable in the It block (or
use ForEach-Object over that variable) so the test becomes Pester data-driven
correctly; update the BeforeDiscovery, It and any additional occurrences noted
(lines ~48-52 equivalents) to use the new $testCases variable and remove inline
-ForEach usage.
- Around line 46-65: The test currently only asserts the parameter set string
for Set-SqlDscRSSmtpConfiguration; add a new Context that uses Get-Command -Name
'Set-SqlDscRSSmtpConfiguration' and inspects the ParameterSets/Parameters to
assert each parameter's attributes (e.g., Mandatory, ValueFromPipeline,
Position, and Type) for Configuration, SmtpServer, SenderEmailAddress, PassThru,
Force, WhatIf and Confirm; follow the existing Get-Command template pattern (use
ParameterSets, ParameterListAsString, and individual parameter property checks)
and add Should assertions verifying the expected Mandatory and ValueFromPipeline
flags and any specific Position or ParameterSet membership for those parameters.
- Around line 26-35: The BeforeAll block needs to load the SMO stub types before
importing the module and setting PSDefaultParameterValues; add a step inside the
BeforeAll (before Import-Module and the PSDefaultParameterValues assignments) to
load the SMO stubs (e.g., use Add-Type -Path 'SMO.cs' or equivalent to
compile/load the SMO.cs stubs), so that the tests have the SMO types available
when $script:moduleName is imported and when mocks/Shoulds run.
tests/Integration/Commands/Set-SqlDscRSSmtpConfiguration.Integration.Tests.ps1
Show resolved
Hide resolved
tests/Integration/Commands/Set-SqlDscRSSmtpConfiguration.Integration.Tests.ps1
Show resolved
Hide resolved
…t-SqlDscRSService
Pull Request (PR) description
Set-SqlDscRSSmtpConfigurationto configure SMTPsettings for Reporting Services email delivery. Wraps the
SetEmailConfigurationCIM method.
This Pull Request (PR) fixes the following issues
None.
Task list
file CHANGELOG.md. Entry should say what was changed and how that
affects users (if applicable), and reference the issue being resolved
(if applicable).
This change is