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

Enhancement: Run "pa app remove" as admin. Closes #5519 #5610

Closed
Closed
Show file tree
Hide file tree
Changes from 4 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
14 changes: 14 additions & 0 deletions docs/docs/cmd/pa/app/app-remove.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ m365 pa app remove [options]

`-f, --force`
: Don't prompt for confirmation

`--asAdmin`
MartinM85 marked this conversation as resolved.
Show resolved Hide resolved
: Run the command as admin for apps you don't have access to.

`-e, --environmentName [environmentName]`
: The name of the environment. Required when using `asAdmin`.
```

<Global />
Expand All @@ -26,6 +32,8 @@ m365 pa app remove [options]

By default, the command will try to remove a Power App. As maker, you are able to delete the Power Apps you own. As administrator, you are also able to delete Power Apps from other users.

To remove the app from another user, use the `asAdmin` option and make sure to specify the `environmentName` option.
MartinM85 marked this conversation as resolved.
Show resolved Hide resolved

To remove a model-driven Power App you need administrator permissions.

If the Power App with the name you specified doesn't exist, you will get the `Error: App 'abc' does not exist` error.
Expand All @@ -44,6 +52,12 @@ Removes the specified Power App without confirmation
m365 pa app remove --name 3989cb59-ce1a-4a5c-bb78-257c5c39381d --force
```

Removes the specified Power App you don't have access to
MartinM85 marked this conversation as resolved.
Show resolved Hide resolved

```sh
m365 pa app remove --name 3989cb59-ce1a-4a5c-bb78-257c5c39381d --environmentName Default-d87a7535-dd31-4437-bfe1-95340acd55c5 --asAdmin
```

## Response

The command won't return a response on success.
45 changes: 20 additions & 25 deletions src/m365/pa/commands/app/app-remove.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,9 @@ describe(commands.APP_REMOVE, () => {
assert(loggerLogToStderrSpy.called);
});

it('removes the specified Microsoft Power App from other user when prompt confirmed (debug)', async () => {
it('removes the specified Microsoft Power App from other user as admin when prompt confirmed (debug)', async () => {
sinon.stub(request, 'delete').callsFake(async (opts) => {
if (opts.url === `https://api.powerapps.com/providers/Microsoft.PowerApps/apps/e0c89645-7f00-4877-a290-cbaf6e060da1?api-version=2017-08-01`) {
if (opts.url === `https://api.powerapps.com/providers/Microsoft.PowerApps/scopes/admin/environments/4ce50206-9576-4237-8b17-38d8aadfaa35/apps/e0c89645-7f00-4877-a290-cbaf6e060da1?api-version=2017-08-01`) {
return { statusCode: 200 };
}

Expand All @@ -148,7 +148,9 @@ describe(commands.APP_REMOVE, () => {
await command.action(logger, {
options: {
debug: true,
name: 'e0c89645-7f00-4877-a290-cbaf6e060da1'
name: 'e0c89645-7f00-4877-a290-cbaf6e060da1',
environmentName: '4ce50206-9576-4237-8b17-38d8aadfaa35',
asAdmin: true
}
});
assert(loggerLogToStderrSpy.called);
Expand Down Expand Up @@ -244,28 +246,6 @@ describe(commands.APP_REMOVE, () => {
} as any);
});

it('supports specifying name', () => {
const options = command.options;
let containsOption = false;
options.forEach(o => {
if (o.option.indexOf('--name') > -1) {
containsOption = true;
}
});
assert(containsOption);
});

it('supports specifying confirm', () => {
const options = command.options;
let containsOption = false;
options.forEach(o => {
if (o.option.indexOf('--force') > -1) {
containsOption = true;
}
});
assert(containsOption);
});

it('correctly handles random api error', async () => {
sinon.stub(request, 'delete').rejects(new Error("Something went wrong"));

Expand All @@ -279,4 +259,19 @@ describe(commands.APP_REMOVE, () => {
}
} as any), new CommandError("Something went wrong"));
});

it('fails validation if asAdmin specified without environment', async () => {
const actual = await command.validate({ options: { name: "5369f386-e380-46cb-82a4-4e18f9e4f3a7", asAdmin: true } }, commandInfo);
assert.notStrictEqual(actual, true);
});

it('fails validation if environment specified without admin', async () => {
const actual = await command.validate({ options: { name: "5369f386-e380-46cb-82a4-4e18f9e4f3a7", environmentName: 'Default-d87a7535-dd31-4437-bfe1-95340acd55c6' } }, commandInfo);
assert.notStrictEqual(actual, true);
});

it('passes validation if asAdmin specified with environment', async () => {
const actual = await command.validate({ options: { name: "5369f386-e380-46cb-82a4-4e18f9e4f3a7", asAdmin: true, environmentName: 'Default-d87a7535-dd31-4437-bfe1-95340acd55c6' } }, commandInfo);
assert.strictEqual(actual, true);
});
});
30 changes: 28 additions & 2 deletions src/m365/pa/commands/app/app-remove.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ interface CommandArgs {
interface Options extends GlobalOptions {
name: string;
force?: boolean;
environmentName?: string;
asAdmin?: boolean;
}

class PaAppRemoveCommand extends PowerAppsCommand {
Expand All @@ -37,7 +39,9 @@ class PaAppRemoveCommand extends PowerAppsCommand {
#initTelemetry(): void {
this.telemetry.push((args: CommandArgs) => {
Object.assign(this.telemetryProperties, {
force: typeof args.options.force !== 'undefined'
force: typeof args.options.force !== 'undefined',
asAdmin: !!args.options.asAdmin,
environmentName: typeof args.options.environmentName !== 'undefined'
});
});
}
Expand All @@ -49,6 +53,12 @@ class PaAppRemoveCommand extends PowerAppsCommand {
},
{
option: '-f, --force'
},
{
option: '--asAdmin'
},
{
option: '-e, --environmentName [environmentName]'
}
);
}
Expand All @@ -60,6 +70,14 @@ class PaAppRemoveCommand extends PowerAppsCommand {
return `${args.options.name} is not a valid GUID`;
}

if (args.options.asAdmin && !args.options.environmentName) {
return 'When specifying the asAdmin option, the environment option is required as well.';
}

if (args.options.environmentName && !args.options.asAdmin) {
return 'When specifying the environment option, the asAdmin option is required as well.';
}

return true;
}
);
Expand All @@ -71,8 +89,16 @@ class PaAppRemoveCommand extends PowerAppsCommand {
}

const removePaApp = async (): Promise<void> => {
let endpoint;
if (args.options.asAdmin) {
endpoint = `${this.resource}/providers/Microsoft.PowerApps/scopes/admin/environments/${formatting.encodeQueryParameter(args.options.environmentName!)}/apps/${formatting.encodeQueryParameter(args.options.name)}?api-version=2017-08-01`;
}
else {
endpoint = `${this.resource}/providers/Microsoft.PowerApps/apps/${formatting.encodeQueryParameter(args.options.name)}?api-version=2017-08-01`;
}
Comment on lines +93 to +98
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These endpoint URLs have a lot of common parts. Let's just use a single inline condition to add the admin scope and environment when needed (just like we do with pa app get).

Copy link
Contributor Author

@MartinM85 MartinM85 Dec 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that single inline condition was there and it was changed according to the previous review. In pa app get I've removed the single inline condition

Comment on lines +92 to +98
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's isolate the common parts.


const requestOptions: CliRequestOptions = {
url: `${this.resource}/providers/Microsoft.PowerApps/apps/${formatting.encodeQueryParameter(args.options.name)}?api-version=2017-08-01`,
url: endpoint,
fullResponse: true,
headers: {
accept: 'application/json'
Expand Down