GetUserProfileProperty: add ByClaims, ByCurrentUser and ByUser parametersets without tenant admin

[fowl2]

Because it doesn't actually require tenant admin to read the user profiles. :)

Examples

# Get the user profile of a user in claims format (Tenant admin site)
Get-PnPUserProfileProperties -Claims 'i:0#.f|membership|[email protected]'

# Get the user profile of a user in claims format (Tenant admin site) (Current site)
Get-PnPUserProfileProperties -Claims 'i:0#.f|membership|[email protected]' -FromTenantAdminSite

# Get the user profile of user on current site by ID
Get-PnPUserProfileProperties -User 361

# Get the user profile of user on current site by User
New-PnPUser -LoginName '[email protected]' | Get-PnPUserProfileProperties

# Get the user profiles of all users on current site
Get-PnPUser | Get-PnPUserProfileProperties

# Get the user profile of user on current site by Title
Get-PnPUserProfileProperties -User 'Smith, Jane'

# Get the user profile of a user in claims format (Current site)
Get-PnPUserProfileProperties -Claims 'i:0#.f|membership|[email protected]'

# Get the user profile of a user in claims format (Tenant admin site)
Get-PnPUserProfileProperties -Claims 'i:0#.f|membership|[email protected]' -FromTenantAdminSite

# Get current user's user profile (Current site)
Get-PnPUserProfileProperties   

# Get current user's user profile (Tenant admin site)
Get-PnPUserProfileProperties -FromTenantAdminSite 

Type

• [x] New Feature

[KoenZomers]

Looks impressive @fowl2! Two questions from my side:

1. Could you please update the documentation on Get-PnUserProfileProperty to reflect the things you're adding?
2. Can you elaborate why you changed PnPAdminCmdlet, what advantages it brings and if it will be backwards compatible for all cmdlets inheriting from this base class?

[fowl2]

1. Ahh the docs are manually written now instead of generated by attribute? I'll take a look next week

2. Yes the changes are designed to have no behavior difference unless the new protected virtual property is overridden, which I've done only in this cmdlet.

It seemed the least amount of changes required to allow a cmdlet to conditionally determine which site to use. We don't want to unconditionally use the tenant admin site because not everyone is tenant admin (eg. I'm not).

Some alternatives might be to split the cmdlet into two or to perhaps to split admin connection into a seperate property and then audit all uses in the admin cmdlets but that seems user unfriendly and error prone, respectively.

[KoenZomers]

Correct, documentation has been completely put aside since version 1 to allow for more flexibility in creating tailored documentation and to allow non developers to contribute to the documentation as well. You can easily locate the matching documentation file by searching for Get-PnPUserProfileProperty.md.

If I understand you correctly, since the changes you're proposing to Get-PnPUserProfileProperty, based on the parameters/paremsets used, may no longer require the admin context, but as the current way PnP PowerShell has been built either an entire cmdlet requires the admin context or it does not, decided by its inheritance, you have implemented the changes to accommodate both of these scenarions into one cmdlet, correct?

[fowl2]

That's correct. :)

[KoenZomers]

Thanks for confirming. Let me take this within our core team to see if we all agree on this being the best solution to this scenario.

[KoenZomers]

I'm sorry but I'm really having a hard time fully understanding your code changes. Can you confirm that nothing you're proposing to change here will cause a non-backwards compatible scenario with existing scripts utilizing this cmdlet?

[fowl2]

The changes to PnPAdminCmdlet are designed to have no effect unless the cmdlet overrides ImplicitAdminContextSwitch and returns true, in which case it needs to call SwitchToAdminClientContext() if it needs to run as admin.

The new parameters/parameter set for Get-PnPUserProfileProperty are all additive, with the default set to the existing set. If you only pass -Account, it will get the data from admin site just like today.

Does that clarify things?

(I've also rebased/fixed merge conflicts)

[KoenZomers]

I'm reluctant to merge this as the impact could be big and I cannot oversee what it does. @gautamdsheth, what's your take on this one?

[fowl2]

A colleague ran into a situation today where this functionality would've been really helpful.

Is there something I could do to improve the confidence here?

Would an alternative, more localized, approach of reworking this cmdlet away fromPnPAdminCmdlet be more acceptable?

[KoenZomers]

Same for this one I'm afraid @fowl2. You must have spent a great amount of time into making this PR, so I'm sorry. We've discussed this one a few times in the team, but could never get to a complete consensus about the estimated impact of it. I really would like to clean ship in the PRs and in all honesty if we didn't manage to reach a consensus in the past 1+ year, it's not going to happen anymore. Going to have to reject this PR.