Author Posts

February 16, 2017 at 8:20 pm

I'm writing a script that will check if certain users in an OU are enabled, if they are an email will be sent out alerting so.

I want it to look like

'The following users are still active:
User1
User2
User3
'

$users = Get-ADUser -ErrorAction stop -filter * -SearchBase "OU=VPN Users,OU=Vendor & Visitor Accounts,DC=domain,DC=com"
$enabled=@()
foreach($user in $users)
{
	if($user.enabled -eq $true)
	{
		$enabled += $user.name
	}
}
$letterbody="The following users are still active:`r`n"
$enabled | foreach{$letterbody += "$_`r`n"}

The above works, but I was wondering if there was a more eloquent way of doing so (I'm SURE there is..)?

I would love to see any other examples that might help me in the future.

Thanks!

February 16, 2017 at 9:41 pm

Here's one way:

$LetterBody = "The following users are still active:"

$Users | Where-Object { $_.Enabled } | ForEach-Object {
	$LetterBody += "`r`n$($_.Name)"
}

February 16, 2017 at 9:47 pm

I like that! Also like your name ;D

February 16, 2017 at 11:24 pm

That's pretty much the way to do it: iterate over the array and act on each member.

But, taking the entire thing and making it even more elegant, assuming you don't need to do anything else with the users after this, don't even pick up the disabled users from AD in the first place:

$users = Get-ADUser -ErrorAction stop -filter {Enabled -eq $true} -SearchBase "OU=VPN Users,OU=Vendor & Visitor Accounts,DC=domain,DC=com"

$letterbody = "The following users are still active:`n"
$users | foreach{$letterbody += "$_`n"}

Also, you don't really need to use both a return and a newline...just a newline is enough (the `n character).

If you do plan to use the disabled users later, you could do something like this (using the Where() and ForEach() methods instead of the cmdlets (just because they're fun to use)):

$letterbody = "The following users are still active:`n"
$users.Where({$_.Enabled -eq $true}).ForEach({$letterbody += "$_`n"})

Or, let's take it a step further and move the $letterbody outside of the foreach loop entirely:

$letterbody = "The following users are still active:"
$letterbody += $users.Where({$_.Enabled -eq $true}).ForEach({"`n$_"})

Note that I had to move the newline character to make that work properly, though that does have the added benefit of not leaving an extra, unneeded newline at the end of the email.

But if we're going to do that, why not go all the way and set it all in one go? Not quite as readable, but definitely elegant:

$letterbody = "The following users are still active:$($users.Where({$_.Enabled -eq $true}).ForEach({"`n$_"}))"

Anyway, I'm not posting these specifically as recommendations, so much as playing around with different ways to accomplish this, and hope at least one of them is helpful or useful to you.

February 17, 2017 at 8:53 am

I aim to please 😉