Skip to content

Commit

Permalink
Fixes anomalies for 'tenant people profilecardproperty' commands
Browse files Browse the repository at this point in the history
  • Loading branch information
milanholemans committed Nov 5, 2023
1 parent 2970c86 commit 18d7ab4
Show file tree
Hide file tree
Showing 10 changed files with 75 additions and 94 deletions.
48 changes: 0 additions & 48 deletions docs/docs/cmd/tenant/people/people-profilecardproperty-add.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -66,54 +66,6 @@ m365 tenant people profilecardproperty add --name customAttribute1 --displayName

## Response

### Standard response

<Tabs>
<TabItem value="JSON">

```json
{
"directoryPropertyName": "UserPrincipalName",
"annotations": []
}
```

</TabItem>
<TabItem value="Text">

```text
annotations : []
directoryPropertyName: UserPrincipalName
```

</TabItem>
<TabItem value="CSV">

```csv
directoryPropertyName
UserPrincipalName
```

</TabItem>
<TabItem value="Markdown">

```md
# tenant people profilecardproperty add --name 'UserPrincipalName'

Date: 11/2/2023

Property | Value
---------|-------
directoryPropertyName | UserPrincipalName
```

</TabItem>
</Tabs>

### Response with a customAttribute

When we make use of one of the customAttributes, the response will differ.

<Tabs>
<TabItem value="JSON">

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,7 @@ m365 tenant people profilecardproperty get --name customAttribute1

</TabItem>
</Tabs>

## More information

- https://learn.microsoft.com/graph/add-properties-profilecard
20 changes: 6 additions & 14 deletions docs/docs/cmd/tenant/people/people-profilecardproperty-list.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ m365 tenant people profilecardproperty list
<TabItem value="JSON">

```json
{
"value": [
[
{
"directoryPropertyName": "customAttribute1",
"annotations": [
Expand All @@ -53,13 +52,8 @@ m365 tenant people profilecardproperty list
]
}
]
},
{
"directoryPropertyName": "Alias",
"annotations": []
}
]
}
```

</TabItem>
Expand All @@ -69,7 +63,6 @@ m365 tenant people profilecardproperty list
directoryPropertyName displayName displayName de
--------------------- -------------- --------------
customAttribute1 Cost center Kostenstelle
Alias
```

</TabItem>
Expand All @@ -78,7 +71,6 @@ m365 tenant people profilecardproperty list
```csv
directoryPropertyName,displayName,displayName de
customAttribute1,Cost center,Kostenstelle
Alias,,
```

</TabItem>
Expand All @@ -96,11 +88,11 @@ m365 tenant people profilecardproperty list
directoryPropertyName | customAttribute1
displayName | Cost center
displayName de | Kostenstelle

Property | Value
---------|-------
directoryPropertyName | Alias
```

</TabItem>
</Tabs>
</Tabs>

## More information

- https://learn.microsoft.com/graph/add-properties-profilecard
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ describe(commands.PEOPLE_PROFILECARDPROPERTY_ADD, () => {
throw `Invalid request ${opts.url}`;
});

await assert.doesNotReject(command.action(logger, { options: { name: 'userPrincipalName' } }));
await command.action(logger, { options: { name: 'userPrincipalName' } });
assert(loggerLogSpy.calledOnceWithExactly(propertyResponse));
});

Expand All @@ -154,7 +154,7 @@ describe(commands.PEOPLE_PROFILECARDPROPERTY_ADD, () => {
throw `Invalid request ${opts.url}`;
});

await assert.doesNotReject(command.action(logger, { options: { name: 'userPrincipalName', debug: true } }));
await command.action(logger, { options: { name: 'userPrincipalName', debug: true } });
assert(loggerLogSpy.calledOnceWithExactly(propertyResponse));
});

Expand All @@ -167,7 +167,7 @@ describe(commands.PEOPLE_PROFILECARDPROPERTY_ADD, () => {
throw `Invalid request ${opts.url}`;
});

await assert.doesNotReject(command.action(logger, { options: { name: 'fax' } }));
await command.action(logger, { options: { name: 'fax' } });
assert(loggerLogSpy.calledOnceWithExactly(propertyResponse));
});

Expand Down Expand Up @@ -249,6 +249,19 @@ describe(commands.PEOPLE_PROFILECARDPROPERTY_ADD, () => {
assert(loggerLogSpy.calledOnceWithExactly(customAttributePropertyTextResponse));
});

it('uses correct casing for name when incorrect casing is used', async () => {
const postStub = sinon.stub(request, 'post').callsFake(async (opts) => {
if (opts.url === `https://graph.microsoft.com/v1.0/admin/people/profileCardProperties`) {
return customAttributePropertyResponse;
}

throw 'Invalid Request';
});

await command.action(logger, { options: { name: 'ALIAS', output: 'json' } });
assert.strictEqual(postStub.lastCall.args[0].data.directoryPropertyName, 'Alias');
});

it('fails when the addition conflicts with an existing property', async () => {
sinon.stub(request, 'post').callsFake(async (opts) => {
if (opts.url === `https://graph.microsoft.com/v1.0/admin/people/profileCardProperties`) {
Expand Down
17 changes: 10 additions & 7 deletions src/m365/tenant/commands/people/people-profilecardproperty-add.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ interface Options extends GlobalOptions {
}

class TenantPeopleProfileCardPropertyAddCommand extends GraphCommand {
private excludeOptions: string[] = ['name', 'displayName', 'debug', 'verbose', 'output'];

public get name(): string {
return commands.PEOPLE_PROFILECARDPROPERTY_ADD;
}
Expand All @@ -35,8 +33,13 @@ class TenantPeopleProfileCardPropertyAddCommand extends GraphCommand {

#initTelemetry(): void {
this.telemetry.push((args: CommandArgs) => {
// Add unknown options to telemetry
const unknownOptions = Object.keys(this.getUnknownOptions(args.options));
const unknownOptionsObj = unknownOptions.reduce((obj, key) => ({ ...obj, [key]: true }), {});

Object.assign(this.telemetryProperties, {
displayName: typeof args.options.displayName !== 'undefined'
displayName: typeof args.options.displayName !== 'undefined',
...unknownOptionsObj
});
});
}
Expand All @@ -62,15 +65,15 @@ class TenantPeopleProfileCardPropertyAddCommand extends GraphCommand {
return `${args.options.name} is not a valid value for name. Allowed values are ${profileCardPropertyNames.join(', ')}`;
}

if (propertyName.startsWith("customattribute") && args.options.displayName === undefined) {
if (propertyName.startsWith('customattribute') && args.options.displayName === undefined) {
return `The option 'displayName' is required when adding customAttributes as profile card properties`;
}

if (!propertyName.startsWith("customattribute") && args.options.displayName !== undefined) {
if (!propertyName.startsWith('customattribute') && args.options.displayName !== undefined) {
return `The option 'displayName' can only be used when adding customAttributes as profile card properties`;
}

const unknownOptions = Object.keys(args.options).filter(key => !this.excludeOptions.includes(key));
const unknownOptions = Object.keys(this.getUnknownOptions(args.options));

if (!propertyName.startsWith('customattribute') && unknownOptions.length > 0) {
return `Unknown options like ${unknownOptions.join(', ')} are only supported with customAttributes`;
Expand Down Expand Up @@ -148,7 +151,7 @@ class TenantPeopleProfileCardPropertyAddCommand extends GraphCommand {
}

private getLocalizations(options: Options): { languageTag: string, displayName: string }[] {
const unknownOptions = Object.keys(options).filter(key => !this.excludeOptions.includes(key));
const unknownOptions = Object.keys(this.getUnknownOptions(options));

if (unknownOptions.length === 0) {
return [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,19 @@ describe(commands.PEOPLE_PROFILECARDPROPERTY_GET, () => {
assert(loggerLogSpy.calledOnceWith(textOutput));
});

it('uses correct casing for name when incorrect casing is used', async () => {
const getStub = sinon.stub(request, 'get').callsFake(async (opts) => {
if (opts.url === `https://graph.microsoft.com/v1.0/admin/people/profileCardProperties/${profileCardPropertyName}`) {
return response;
}

throw 'Invalid Request';
});

await command.action(logger, { options: { name: profileCardPropertyName.toUpperCase() } });
assert(getStub.called);
});

it('handles error when profile card property does not exist', async () => {
sinon.stub(request, 'get').rejects({
response: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,11 @@ class TenantPeopleProfileCardPropertyGetCommand extends GraphCommand {
await logger.logToStderr(`Retrieving information about profile card property '${args.options.name}'...`);
}

// 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}`,
headers: {
accept: 'application/json;odata.metadata=none'
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ describe(commands.PEOPLE_PROFILECARDPROPERTY_LIST, () => {
});

await command.action(logger, { options: { verbose: true } });
assert(loggerLogSpy.calledOnceWith(response));
assert(loggerLogSpy.calledOnceWith(response.value));
});

it('lists profile card properties information for other than json output', async () => {
Expand Down
20 changes: 4 additions & 16 deletions src/m365/tenant/commands/people/people-profilecardproperty-list.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { Logger } from '../../../../cli/Logger.js';
import GlobalOptions from '../../../../GlobalOptions.js';
import GraphCommand from '../../../base/GraphCommand.js';
import request, { CliRequestOptions } from '../../../../request.js';
import { ProfileCardProperty } from './profileCardProperties.js';
import commands from '../../commands.js';
import { odata } from '../../../../utils/odata.js';

interface CommandArgs {
options: GlobalOptions;
Expand All @@ -18,29 +18,17 @@ class TenantPeopleProfileCardPropertyListCommand extends GraphCommand {
return 'Lists all profile card properties';
}

constructor() {
super();
}

public async commandAction(logger: Logger, args: CommandArgs): Promise<void> {
try {
if (this.verbose) {
await logger.logToStderr(`Lists all profile card properties...`);
await logger.logToStderr(`Listing all profile card properties...`);
}

const requestOptions: CliRequestOptions = {
url: `${this.resource}/v1.0/admin/people/profileCardProperties`,
headers: {
accept: 'application/json;odata.metadata=none'
},
responseType: 'json'
};

const result = await request.get<ProfileCardProperty[]>(requestOptions);
const result = await odata.getAllItems<ProfileCardProperty>(`${this.resource}/v1.0/admin/people/profileCardProperties`);
let output: any = result;

if (args.options.output && args.options.output !== 'json') {
output = output.value.sort((n1: ProfileCardProperty, n2: ProfileCardProperty) => {
output = output.sort((n1: ProfileCardProperty, n2: ProfileCardProperty) => {
const localizations1 = n1.annotations[0]?.localizations?.length ?? 0;
const localizations2 = n2.annotations[0]?.localizations?.length ?? 0;
if (localizations1 > localizations2) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ describe(commands.PEOPLE_PROFILECARDPROPERTY_REMOVE, () => {
throw `Invalid request ${opts.url}`;
});

await assert.doesNotReject(command.action(logger, { options: { name: 'userPrincipalName' } }));
await command.action(logger, { options: { name: 'userPrincipalName' } });
});

it('correctly removes profile card property for userPrincipalName (debug)', async () => {
Expand All @@ -96,7 +96,7 @@ describe(commands.PEOPLE_PROFILECARDPROPERTY_REMOVE, () => {
throw `Invalid request ${opts.url}`;
});

await assert.doesNotReject(command.action(logger, { options: { name: 'userPrincipalName', debug: true } }));
await command.action(logger, { options: { name: 'userPrincipalName', debug: true } });
});

it('correctly removes profile card property for fax', async () => {
Expand All @@ -108,7 +108,7 @@ describe(commands.PEOPLE_PROFILECARDPROPERTY_REMOVE, () => {
throw `Invalid request ${opts.url}`;
});

await assert.doesNotReject(command.action(logger, { options: { name: 'fax' } }));
await command.action(logger, { options: { name: 'fax' } });
});

it('correctly removes profile card property for state with force', async () => {
Expand All @@ -120,7 +120,20 @@ describe(commands.PEOPLE_PROFILECARDPROPERTY_REMOVE, () => {
throw `Invalid request ${opts.url}`;
});

await assert.doesNotReject(command.action(logger, { options: { name: 'StateOrProvince', force: true } }));
await command.action(logger, { options: { name: 'StateOrProvince', force: true } });
});

it('uses correct casing for name when incorrect casing is used', async () => {
const deleteStub = sinon.stub(request, 'delete').callsFake(async (opts) => {
if (opts.url === `https://graph.microsoft.com/v1.0/admin/people/profileCardProperties/StateOrProvince`) {
return;
}

throw 'Invalid Request';
});

await command.action(logger, { options: { name: 'STATEORPROVINCE', force: true } });
assert(deleteStub.called);
});

it('fails when the removal runs into a property that is not found', async () => {
Expand Down

0 comments on commit 18d7ab4

Please sign in to comment.