-
Notifications
You must be signed in to change notification settings - Fork 307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enhances entra user list
properties
#6056
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost good to go, let's do a few more modifications.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's working great, but let's do a few final additional changes before we merge it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! Made a few more tiny changes myself during the merge, but it works great!
if (options.type) { | ||
filter += filter.length > 0 ? ` and userType eq '${options.type}'` : `$filter=userType eq '${options.type}'`; | ||
if (type) { | ||
filter += filter.length > 0 ? ` and userType eq '${type}'` : `$filter=userType eq '${type}'`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now we're checking twice if there are any filters. I think it's a lot clearer if we throw all filters in a single array and use that array as a filter string.
catch (ex: any) { | ||
throw ex; | ||
|
||
const unknownOptions: Options = this.getUnknownOptions(args.options); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, we cannot type this as Options
. All properties declared in this interface will never be part of this object.
Merged manually, thank you for this addition! 👏 |
Closes #4667
I have also noticed that we added the option
type
with an autocomplete in a previous PR, but we didn't validate the options being passed, resulting in the command throwing an error when we used a user type other thanmember
orguest
.