MethodsAndPractices / vsteam

PowerShell module for accessing Azure DevOps Services and Azure DevOps Server (formerly VSTS or TFS)
https://methodsandpractices.github.io/vsteam-docs/
MIT License
447 stars 155 forks source link

Ambiguity or bug in Update-VSTeamUserEntitlement #393

Closed wembleyford closed 3 years ago

wembleyford commented 3 years ago

Steps to reproduce

update-VSTeamUserEntitlement -id $userDescriptor -license Stakeholder

Expected behavior

The license for the specified user should be updated. It can be updated by using the following format

Get-VSTeamUserEntitlement -Id $userDescriptor|Update-VSTeamUserEntitlement -License StakeHolder

Actual behavior

Update-VSTeamUserEntitlement: Parameter set cannot be resolved using the specified named parameters. One or more parameters issued cannot be used together or an insufficient number of parameters were provided.

There's no detail in the get-help documentation for this particular cmdlet specifying any examples, other than to indicate that the combination of id and license should be valid.

It's also unclear as to what input that this command is accepting via the pipeline in the second example which does work.

Environment data

> Get-VSTeamAPIVersion

Billing                     : 5.1-preview.1
Build                       : 5.1
Core                        : 5.1
DistributedTask             : 6.0-preview
DistributedTaskReleased     : 5.1
ExtensionsManagement        : 6.0-preview
Git                         : 5.1
Graph                       : 6.0-preview
HierarchyQuery              : 5.1-preview
MemberEntitlementManagement : 6.0-preview
Packaging                   : 6.0-preview
Pipelines                   : 5.1-preview
Policy                      : 5.1
Processes                   : 6.0-preview
Release                     : 5.1
ServiceEndpoints            : 5.0-preview
TaskGroups                  : 6.0-preview
Tfvc                        : 5.1
VariableGroups              : 5.1-preview.1
Version                     : VSTS
> $PSVersionTable
Name                           Value
----                           -----
PSVersion                      7.1.3
PSEdition                      Core
GitCommitId                    7.1.3
OS                             Linux 5.10.27-gentoo-x86_64 #1 SMP Fri Apr 2 16:48:21 BST 2021
Platform                       Unix
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0
SebastianSchuetze commented 3 years ago

just to clarify

Is this correct?

wembleyford commented 3 years ago

Apologies for being unclear.

SebastianSchuetze commented 3 years ago

Gonna check this, when I find time. Thanks!

If you are willing to make a PR we would also be happy to review.

SebastianSchuetze commented 3 years ago

@wembleyford: I have checked it and the parameter set is with the name "ByIdLicenseOnly" should make it possible to only use id and license but it still expects the parameter LicensingSource. Then it is working.

@DarqueWarrior or @smurawski I think something must be wrong with all the parameter sets. I have not enough knowledge maybe one of you know what could be wrong?

This is the cmdlet: https://github.com/MethodsAndPractices/vsteam/blob/trunk/Source/Public/Update-VSTeamUserEntitlement.ps1

smurawski commented 3 years ago

@SebastianSchuetze My guess (which should be confirmable with a Trace-Command watching for parameter binding) is that the engine can't differentiate between ByIdLicenseOnly and ByIdWithSource.

My suggestion would be that you don't need the two parameter sets - just a ById and make the behavior conditional on whether or not a Source was provided. You may have similar challenges with the ByEmail variant.

If you look at the command metadata in the engine - it builds the appropriate parameter sets, but parameter sets need something that distinguishes them from all other parameter sets. The absence of one parameter doesn't differentiate enough. It's not the same as method overloads because there are runtime behaviors for binding, transformation, and user prompting.

SebastianSchuetze commented 3 years ago

makes completely sense. The set's looked too complicated to me anyways. Maybe I try to simplify without braking any existing scripts using this cmdlet.

SebastianSchuetze commented 3 years ago

@smurawski would you be so kind to quickly check for me if you think the change would not break existing scripts using the cmdlet? I basically just merged parameter sets. By this at max one parameter is not mandatory anymore.

This is the PR #400