-
Notifications
You must be signed in to change notification settings - Fork 340
Enhancement: Run "pa app remove" as admin. Closes #5519 #5610
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
Changes from all commits
4b464ed
6a361ed
d7b5dbf
2e1521c
af55609
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,8 @@ interface CommandArgs { | |
interface Options extends GlobalOptions { | ||
name: string; | ||
force?: boolean; | ||
environmentName?: string; | ||
asAdmin?: boolean; | ||
} | ||
|
||
class PaAppRemoveCommand extends PowerAppsCommand { | ||
|
@@ -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' | ||
}); | ||
}); | ||
} | ||
|
@@ -49,6 +53,12 @@ class PaAppRemoveCommand extends PowerAppsCommand { | |
}, | ||
{ | ||
option: '-f, --force' | ||
}, | ||
{ | ||
option: '--asAdmin' | ||
}, | ||
{ | ||
option: '-e, --environmentName [environmentName]' | ||
} | ||
); | ||
} | ||
|
@@ -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; | ||
} | ||
); | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Comment on lines
+92
to
+98
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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' | ||
|
Uh oh!
There was an error while loading. Please reload this page.