-1

Okay so i have the following code. It works but it returns failed for each user. The first 2 users are supposed to fail but the last one should be success but it should only show all the failed attempts and then place it in a text file. This is what I have so far besides the output to a text file.

Import-Module ActiveDirectory
#$sam = read-host "Enter username"
#$user = Get-ADUser -filter {SamAccountName -eq $sam}

$user = @("user2","user3","olduser2")
foreach($sam in $user){
if(Get-Aduser $sam){
$Name = (Get-ADUser $sam -Properties cn).name
$path = "OU=Term,OU=test,DC=patel,DC=COM"
Get-ADUser $Name | Move-ADObject -TargetPath $path
}
if(!$sam){
Write-Host "$sam failed"
}

It would return user2 failed with a an error message because it cant be found

user3 failed with a an error message because it cant be found

olduser2 failed without error message.

Shiv Patel
  • 131
  • 1
  • 2
  • 11

2 Answers2

1

The iterator variable ($sam) in the ForEach goes out of scope when the ForEach loop exits. At that point, $sam -eq $null is true (equivalent to !$sam), and therefore you will get the failure message. Try

$user = @("user2","user3","olduser2")
foreach($sam in $user){
    if(Get-Aduser $sam){
        $Name = (Get-ADUser $sam -Properties cn).name
        $path = "OU=Term,OU=test,DC=patel,DC=COM"
        Get-ADUser $Name | Move-ADObject -TargetPath $path
    } else {
        Write-Host "$sam failed"
    }
}

and see if that gives you the results you want - and if you can understand why it does. There are other improvements you can make in the script, as well - but you should get it working first, then think about optimization.

Jeff Zeitlin
  • 9,773
  • 2
  • 21
  • 33
  • So it still gives me the errors but it works for olduser2 but it doesnt display user2 failed user3 failed – Shiv Patel Mar 15 '17 at 18:27
  • It says cannot find object with identity 'user 2' how do i get it display only that user2 failed and so on for all of them – Shiv Patel Mar 15 '17 at 18:41
  • 1
    Please paste the _exact_ and _entire_ text of the error message you are getting. Usually, the error message tells you which command is failing. – Jeff Zeitlin Mar 15 '17 at 19:13
  • Get-ADUser : Cannot find an object with identity: User 2' under: 'DC=Patel ,DC=COM'. At C:\Users\shpate\Desktop\Scripts\disabled_ou_move.ps1:10 char:11 + Get-ADUser <<<<  $Name | Move-ADObject -TargetPath $path     + CategoryInfo          : ObjectNotFound: (User 2:ADUser) [Get-ADUser], ADIdentityNotFoundException     + FullyQualifiedErrorId : Cannot find an object with identity: 'User 2' under: 'DC=patel,DC=COM'.,Microsoft.ActiveDirectory.Management.Commands.GetADUser – Shiv Patel Mar 15 '17 at 19:19
  • it then says that all three user2, user3, and olduser2 failed. It supposed to only say user2 and user3 failed so i can go back and change the ou manually – Shiv Patel Mar 15 '17 at 19:22
  • 1
    You need to rethink your error checking and logic - obviously, `Get-ADUser` throws a terminating error if it can't find the user you're looking for. At the point that the error is being thrown, you have clearly entered the 'then' section of the `if` construct; that's where I'd look for your problem. - You should also use parameter names, and not rely on positional parameters or aliases. Doing so makes your code clearer and easier to understand. – Jeff Zeitlin Mar 15 '17 at 19:30
  • @JeffZeitlin Remove the first `Get-ADUser` in the `if` block. Change the second to `Get-ADUser $sam | Move-ADObject -TargetPath $path` – BenH Mar 15 '17 at 19:30
  • @BenH - I was hoping the querent would do his own troubleshooting and arrive at that conclusion. That's a logic error that he should have caught almost immediately. Using the full parameter names, and reading up on the usage thereof, probably would have let him catch it. – Jeff Zeitlin Mar 15 '17 at 19:32
  • @ShivPatel `Get-ADUser` doesn't take `Name` as an `-Identity` which was the issue – BenH Mar 15 '17 at 19:36
1

As Jeff Zeitlin mentioned in the comments of the answer. This would be better to do error checking in a try catch loop.

$user = @("user2","user3","olduser2")
foreach($sam in $user) {
    try {
        $path = "OU=Term,OU=test,DC=patel,DC=COM"
        Get-ADUser $sam -ErrorAction Stop | Move-ADObject -TargetPath $path -ErrorAction Stop
    }
    catch {
        Write-Host "$sam failed"
    }
}
BenH
  • 9,766
  • 1
  • 22
  • 35