ExcludeSystemUser correct logic in Get-DbaDbUser

[pollusb]

Type of Change

• Bug fix (non-breaking change, fixes # )
• New feature (non-breaking change, adds functionality, fixes # )
• Breaking change (affects multiple commands or functionality, fixes # )
• Ran manual Pester test and has passed (.\tests\manual.pester.ps1)
• Adding code coverage to existing functionality
• Pester test is included
• If new file reference added for test, has is been added to github.com/dataplat/appveyor-lab ?
• Unit test is included
• Documentation
• Build system

Purpose

Correct a logical error in Get-DbaDbUser

Approach

Commands to test

# Actual logic:
PS> (Get-DbaDbUser @splat).Count
15
PS> (Get-DbaDbUser @splat -ExcludeSystemUser).Count 
11
PS> (Get-DbaDbUser @splat -ExcludeSystemUser:$false).Count 
11 # logical error here
PS> (Get-DbaDbUser @splat -ExcludeSystemUser:$true).Count 
11

# Revised logic:
PS> (Get-DbaDbUser @splat).Count
15
PS> (Get-DbaDbUser @splat -ExcludeSystemUser).Count 
11
PS> (Get-DbaDbUser @splat -ExcludeSystemUser:$false).Count 
15
PS> (Get-DbaDbUser @splat -ExcludeSystemUser:$true).Count 
11

Screenshots

Learning

[niphlod]

LGTM, @andreasjordan ?

[andreasjordan]

Good catch. Maybe we have the same problem in other commands as well.

[pollusb]

Good catch. Maybe we have the same problem in other commands as well.

We do. Testing the presence of a switch parameter does not provide the right logic. Here's a list of a similar pattern:

PS C:\Users\me\Code\Repo\dbatools.fork.git\public> dir *.ps1|sls 'Test-Bound -ParameterName Exclude'

Get-DbaDbCheckConstraint.ps1:105:                    if ( (Test-Bound -ParameterName ExcludeSystemTable) -and
$tbl.IsSystemObject ) {
Get-DbaDbForeignKey.ps1:105:                    if ( (Test-Bound -ParameterName ExcludeSystemTable) -and
$tbl.IsSystemObject ) {
Get-DbaDbServiceBrokerQueue.ps1:92:                if ( (Test-Bound -ParameterName ExcludeSystemQueue) -and
$queue.IsSystemObject ) {
Get-DbaDbServiceBrokerService.ps1:92:                if ( (Test-Bound -ParameterName ExcludeSystemService) -and
$service.IsSystemObject ) {
Get-DbaDbStoredProcedure.ps1:185:                if ( (Test-Bound -ParameterName ExcludeSystemSp) -and
$proc.IsSystemObject ) {
Get-DbaDbUdf.ps1:122:                if (Test-Bound -ParameterName ExcludeSystemUdf) {
Get-DbaDbUser.ps1:124:                if (Test-Bound -ParameterName ExcludeSystemUser) {
Get-DbaDbView.ps1:170:            if (Test-Bound -ParameterName ExcludeSystemView) {
Get-DbaInstanceAudit.ps1:76:            if (Test-Bound -ParameterName ExcludeAudit) {
Set-DbaMaxDop.ps1:153:                    } elseif ((Test-Bound -Not -ParameterName AllDatabases) -and (Test-Bound
-ParameterName ExcludeDatabase)) {

I would probably send PR in the same ways for these. I mean with a testing scenario. Would that be OK?

[andreasjordan]

I would probably send PR in the same ways for these. I mean with a testing scenario. Would that be OK?

Sure.

[potatoqualitee]

Thanks everyone! 🏆