Skip to content
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

Implement disambiguation prompt in missing places. Closes #5490 #5568

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/m365/base/AppCommand.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ describe('AppCommand', () => {
sinonUtil.restore([
fs.existsSync,
fs.readFileSync,
Cli.prompt
Cli.prompt,
Cli.handleMultipleResultsFound
]);
});

Expand Down Expand Up @@ -115,7 +116,7 @@ describe('AppCommand', () => {
}
]
}));
const cliPromptStub = sinon.stub(Cli, 'prompt').callsFake(async () => (
const cliPromptStub = sinon.stub(Cli, 'handleMultipleResultsFound').callsFake(async () => (
{ appIdIndex: 0 }
));
await assert.rejects(cmd.action(logger, { options: {} }));
Expand All @@ -136,7 +137,7 @@ describe('AppCommand', () => {
}
]
}));
sinon.stub(Cli, 'prompt').resolves({ appIdIndex: 1 });
sinon.stub(Cli, 'handleMultipleResultsFound').resolves({ appIdIndex: 1 });
sinon.stub(Command.prototype, 'action').resolves();

try {
Expand Down
16 changes: 3 additions & 13 deletions src/m365/base/AppCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import Command, { CommandArgs, CommandError } from '../../Command.js';
import GlobalOptions from '../../GlobalOptions.js';
import { validation } from '../../utils/validation.js';
import { M365RcJson, M365RcJsonApp } from './M365RcJson.js';
import { formatting } from '../../utils/formatting.js';

export interface AppCommandArgs {
options: AppCommandOptions;
Expand Down Expand Up @@ -89,19 +90,8 @@ export default abstract class AppCommand extends Command {
}

if (this.m365rcJson.apps.length > 1) {
const result = await Cli.prompt<{ appIdIndex: number }>({
message: `Multiple Azure AD apps found in ${m365rcJsonPath}. Which app would you like to use?`,
type: 'list',
choices: this.m365rcJson.apps.map((app, i) => {
return {
name: `${app.name} (${app.appId})`,
value: i
};
}),
default: 0,
name: 'appIdIndex'
});

const resultAsKeyValuePair = formatting.convertArrayToHashTable('appIdIndex', this.m365rcJson.apps);
const result = await Cli.handleMultipleResultsFound<{ appIdIndex: number }>(`Multiple Azure AD apps found in ${m365rcJsonPath}.`, resultAsKeyValuePair);
this.appId = ((this.m365rcJson as M365RcJson).apps as M365RcJsonApp[])[result.appIdIndex].appId;
await super.action(logger, args);
}
Expand Down
50 changes: 48 additions & 2 deletions src/m365/teams/commands/app/app-update.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ describe(commands.APP_UPDATE, () => {
request.put,
fs.readFileSync,
fs.existsSync,
cli.getSettingWithDefaultValue
cli.getSettingWithDefaultValue,
Cli.handleMultipleResultsFound
]);
});

Expand Down Expand Up @@ -175,6 +176,14 @@ describe(commands.APP_UPDATE, () => {
});

it('handles error when multiple Teams apps with the specified name found', async () => {
sinon.stub(cli, 'getSettingWithDefaultValue').callsFake((settingName, defaultValue) => {
if (settingName === settingsNames.prompt) {
return false;
}

return defaultValue;
});

sinon.stub(request, 'get').callsFake(async (opts) => {
if ((opts.url as string).indexOf(`/v1.0/appCatalogs/teamsApps?$filter=displayName eq '`) > -1) {
return {
Expand All @@ -199,7 +208,44 @@ describe(commands.APP_UPDATE, () => {
name: 'Test app',
filePath: 'teamsapp.zip'
}
} as any), new CommandError('Multiple Teams apps with name Test app found. Please choose one of these ids: e3e29acb-8c79-412b-b746-e6c39ff4cd22, 5b31c38c-2584-42f0-aa47-657fb3a84230'));
} as any), new CommandError('Multiple Teams apps with name Test app found. Found: e3e29acb-8c79-412b-b746-e6c39ff4cd22, 5b31c38c-2584-42f0-aa47-657fb3a84230.'));
});

it('handles selecting single result when multiple Teams apps found with the specified name', async () => {
sinon.stub(request, 'get').callsFake(async (opts) => {
if ((opts.url as string).indexOf(`/v1.0/appCatalogs/teamsApps?$filter=displayName eq '`) > -1) {
return {
"value": [
{
"id": "e3e29acb-8c79-412b-b746-e6c39ff4cd22",
"displayName": "Test app"
},
{
"id": "5b31c38c-2584-42f0-aa47-657fb3a84230",
"displayName": "Test app"
}
]
};
}
throw 'Invalid request';
});

sinon.stub(Cli, 'handleMultipleResultsFound').resolves({ id: '5b31c38c-2584-42f0-aa47-657fb3a84230' });

let updateTeamsAppCalled = false;
sinon.stub(request, 'put').callsFake(async (opts) => {
if (opts.url === `https://graph.microsoft.com/v1.0/appCatalogs/teamsApps/5b31c38c-2584-42f0-aa47-657fb3a84230`) {
updateTeamsAppCalled = true;
return;
}

throw 'Invalid request';
});

sinon.stub(fs, 'readFileSync').callsFake(() => '123');

await command.action(logger, { options: { filePath: 'teamsapp.zip', name: 'Test app' } });
assert(updateTeamsAppCalled);
});

it('update Teams app in the tenant app catalog by id', async () => {
Expand Down
15 changes: 9 additions & 6 deletions src/m365/teams/commands/app/app-update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { formatting } from '../../../../utils/formatting.js';
import { validation } from '../../../../utils/validation.js';
import GraphCommand from '../../../base/GraphCommand.js';
import commands from '../../commands.js';
import { Cli } from '../../../../cli/Cli.js';

interface CommandArgs {
options: Options;
Expand Down Expand Up @@ -89,7 +90,7 @@ class TeamsAppUpdateCommand extends GraphCommand {
const { filePath } = args.options;

try {
const appId: string = await this.getAppId(args);
const appId: string = await this.getAppId(args.options);
const fullPath: string = path.resolve(filePath);
if (this.verbose) {
await logger.logToStderr(`Updating app with id '${appId}' and file '${fullPath}' in the app catalog...`);
Expand All @@ -110,13 +111,13 @@ class TeamsAppUpdateCommand extends GraphCommand {
}
}

private async getAppId(args: CommandArgs): Promise<string> {
if (args.options.id) {
return args.options.id;
private async getAppId(options: Options): Promise<string> {
if (options.id) {
return options.id;
}

const requestOptions: CliRequestOptions = {
url: `${this.resource}/v1.0/appCatalogs/teamsApps?$filter=displayName eq '${formatting.encodeQueryParameter(args.options.name as string)}'`,
url: `${this.resource}/v1.0/appCatalogs/teamsApps?$filter=displayName eq '${formatting.encodeQueryParameter(options.name as string)}'`,
headers: {
accept: 'application/json;odata.metadata=none'
},
Expand All @@ -131,7 +132,9 @@ class TeamsAppUpdateCommand extends GraphCommand {
}

if (response.value.length > 1) {
throw `Multiple Teams apps with name ${args.options.name} found. Please choose one of these ids: ${response.value.map(x => x.id).join(', ')}`;
const resultAsKeyValuePair = formatting.convertArrayToHashTable('id', response.value);
const result = await Cli.handleMultipleResultsFound<{ id: string; }>(`Multiple Teams apps with name ${options.name} found.`, resultAsKeyValuePair);
return result.id;
}

return app.id;
Expand Down