Fixes anomalies for 'tenant people profilecardproperty' commands #5635
Conversation
milanholemans
force-pushed
the
pcp-fix
branch
2 times, most recently
from
7df61d3 to
18d7ab4
Compare
src/m365/tenant/commands/people/people-profilecardproperty-remove.spec.ts
Show resolved
Hide resolved
milanholemans
force-pushed
the
pcp-fix
branch
from
18d7ab4 to
ef5c1ba
Compare
| // Get the right casing for the profile card property name | ||
| const profileCardProperty = profileCardPropertyNames.find(p => p.toLowerCase() === args.options.name.toLowerCase()); | ||
|
|
||
| const requestOptions: CliRequestOptions = { | ||
| url: `${this.resource}/v1.0/admin/people/profileCardProperties/${args.options.name}`, | ||
| url: `${this.resource}/v1.0/admin/people/profileCardProperties/${profileCardProperty}`, |
There was a problem hiding this comment.
Should we really enforce the casing for the get command? Folks could always add properties straight to the graph API with their own casing. If they then want to get the property through the CLI they'll get an empty result while it exists with different casing.
The same goes for the remove and set commands.
There was a problem hiding this comment.
Yes, but on the other hand if they specify for example FAX in the add command and use the same value for the remove or get command, they will receive an error that the property doesn't exist, which is weird.
In my opinion we do it everywhere or nowhere at all, the commands should work seamlessly together. It's a mess that we have to deal with this indeed.
There was a problem hiding this comment.
I agree to keep consistent behavior across the commands.
|
Ready to merge 🚀 |
|
merged manually. Thank you for your awesome work👏 |
In this PR I made the following changes for
tenant people profilecardpropertycommands.addcommand docs. We only use multiple responses if the output of the command changes. In this case, an empty array is not a structural output change.More informationdocs section to every page.listcommand outputs an array instead of an object.listcommand, removed extra response objects. We only display 1 object in the response preview.addcommand to use a built-in function to get unknown properties.getcommand.addcommand.