From f5bed289affca2fce04fee6dd2db3ce02a8f5c83 Mon Sep 17 00:00:00 2001 From: Christopher Suh Date: Fri, 21 Jul 2023 08:43:18 -0700 Subject: [PATCH] Expand account picker support to multiple providers (#23952) * fix dropdown behavior * expand account picker support to multiple providers * fix tests * fixed unresolved promise issue --- .../accounts/common/accountActions.ts | 7 +++- .../accounts/common/accountPickerViewModel.ts | 1 - .../platform/accounts/common/interfaces.ts | 1 + .../common/accountPickerViewModel.test.ts | 4 +-- .../common/testAccountManagementService.ts | 4 +++ .../browser/accountManagementService.ts | 36 ++++++++++++++++++- .../browser/accountPickerImpl.ts | 9 ++--- .../browser/accountPickerService.ts | 4 +-- .../browser/accountManagementService.test.ts | 2 +- .../test/browser/accountPickerService.test.ts | 9 +++-- 10 files changed, 57 insertions(+), 20 deletions(-) diff --git a/src/sql/platform/accounts/common/accountActions.ts b/src/sql/platform/accounts/common/accountActions.ts index 1be58d5735..35150c2499 100644 --- a/src/sql/platform/accounts/common/accountActions.ts +++ b/src/sql/platform/accounts/common/accountActions.ts @@ -50,7 +50,12 @@ export class AddAccountAction extends Action { // Fire the event that we've started adding accounts this._addAccountStartEmitter.fire(); try { - await this._accountManagementService.addAccount(this._providerId); + if (!this._providerId) { + let providerId = await this._accountManagementService.promptProvider(); + await this._accountManagementService.addAccount(providerId); + } else { + await this._accountManagementService.addAccount(this._providerId); + } this._addAccountCompleteEmitter.fire(); } catch (err) { this.logService.error(`Error while adding account: ${err}`); diff --git a/src/sql/platform/accounts/common/accountPickerViewModel.ts b/src/sql/platform/accounts/common/accountPickerViewModel.ts index 7a51f08750..2fa0657a98 100644 --- a/src/sql/platform/accounts/common/accountPickerViewModel.ts +++ b/src/sql/platform/accounts/common/accountPickerViewModel.ts @@ -21,7 +21,6 @@ export class AccountPickerViewModel { public selectedTenantId: string | undefined; constructor( - _providerId: string, @IAccountManagementService private _accountManagementService: IAccountManagementService ) { // Create our event emitters diff --git a/src/sql/platform/accounts/common/interfaces.ts b/src/sql/platform/accounts/common/interfaces.ts index 0d69e03b5b..8044a58d35 100644 --- a/src/sql/platform/accounts/common/interfaces.ts +++ b/src/sql/platform/accounts/common/interfaces.ts @@ -21,6 +21,7 @@ export interface IAccountManagementService { getAccountProviderMetadata(): Promise; getAccountsForProvider(providerId: string): Promise; getAccounts(): Promise; + promptProvider(): Promise; /** * @deprecated */ diff --git a/src/sql/platform/accounts/test/common/accountPickerViewModel.test.ts b/src/sql/platform/accounts/test/common/accountPickerViewModel.test.ts index 9a7929194c..f5d1802e6b 100644 --- a/src/sql/platform/accounts/test/common/accountPickerViewModel.test.ts +++ b/src/sql/platform/accounts/test/common/accountPickerViewModel.test.ts @@ -57,7 +57,7 @@ suite('Account picker view model tests', () => { test('Construction - Events are properly defined', () => { // If: I create an account picker viewmodel let mockAccountManagementService = getMockAccountManagementService(false, false); - let vm = new AccountPickerViewModel('azure', mockAccountManagementService.object); + let vm = new AccountPickerViewModel(mockAccountManagementService.object); // Then: // ... The event for the view models should be properly initialized @@ -137,7 +137,7 @@ function getViewModel( ams: TestAccountManagementService, evUpdate: EventVerifierSingle ): AccountPickerViewModel { - let vm = new AccountPickerViewModel('azure', ams); + let vm = new AccountPickerViewModel(ams); vm.updateAccountListEvent(evUpdate.eventHandler); return vm; diff --git a/src/sql/platform/accounts/test/common/testAccountManagementService.ts b/src/sql/platform/accounts/test/common/testAccountManagementService.ts index ed7a7c9fcc..35a0cf2a12 100644 --- a/src/sql/platform/accounts/test/common/testAccountManagementService.ts +++ b/src/sql/platform/accounts/test/common/testAccountManagementService.ts @@ -59,6 +59,10 @@ export class TestAccountManagementService implements IAccountManagementService { return Promise.resolve(undefined!); } + promptProvider() { + return Promise.resolve(''); + } + removeAccount(accountKey: azdata.AccountKey): Promise { throw new Error('Method not implemented'); } diff --git a/src/sql/workbench/services/accountManagement/browser/accountManagementService.ts b/src/sql/workbench/services/accountManagement/browser/accountManagementService.ts index 41f538a22f..5d7a2bedc3 100644 --- a/src/sql/workbench/services/accountManagement/browser/accountManagementService.ts +++ b/src/sql/workbench/services/accountManagement/browser/accountManagementService.ts @@ -27,6 +27,8 @@ import { Action } from 'vs/base/common/actions'; import { DisposableStore } from 'vs/base/common/lifecycle'; import { IAdsTelemetryService } from 'sql/platform/telemetry/common/telemetry'; import { TelemetryAction, TelemetryError, TelemetryView } from 'sql/platform/telemetry/common/telemetryKeys'; +import { Iterable } from 'vs/base/common/iterator'; +import { IQuickInputService, IQuickPickItem } from 'vs/platform/quickinput/common/quickInput'; export class AccountManagementService implements IAccountManagementService { // CONSTANTS /////////////////////////////////////////////////////////// @@ -39,6 +41,7 @@ export class AccountManagementService implements IAccountManagementService { private _accountDialogController?: AccountDialogController; private _autoOAuthDialogController?: AutoOAuthDialogController; private _mementoContext?: Memento; + public providerMap = new Map(); protected readonly disposables = new DisposableStore(); // EVENT EMITTERS ////////////////////////////////////////////////////// @@ -59,7 +62,8 @@ export class AccountManagementService implements IAccountManagementService { @IOpenerService private _openerService: IOpenerService, @ILogService private readonly _logService: ILogService, @INotificationService private readonly _notificationService: INotificationService, - @IAdsTelemetryService private _telemetryService: IAdsTelemetryService + @IAdsTelemetryService private _telemetryService: IAdsTelemetryService, + @IQuickInputService private _quickInputService: IQuickInputService ) { this._mementoContext = new Memento(AccountManagementService.ACCOUNT_MEMENTO, this._storageService); const mementoObj = this._mementoContext.getMemento(StorageScope.APPLICATION, StorageTarget.MACHINE); @@ -113,6 +117,34 @@ export class AccountManagementService implements IAccountManagementService { } + public async promptProvider(): Promise { + const vals = Iterable.consume(this.providerMap.values())[0]; + + let pickedValue: string | undefined; + if (vals.length === 0) { + this._notificationService.error(localize('accountDialog.noCloudsRegistered', "You have no clouds enabled. Go to Settings -> Search Azure Account Configuration -> Enable at least one cloud")); + } + if (vals.length > 1) { + const buttons: IQuickPickItem[] = vals.map(v => { + return { label: v.displayName } as IQuickPickItem; + }); + + const picked = await this._quickInputService.pick(buttons, { canPickMany: false }); + + pickedValue = picked?.label; + } else { + pickedValue = vals[0].displayName; + } + + const v = vals.filter(v => v.displayName === pickedValue)?.[0]; + + if (!v) { + this._notificationService.error(localize('accountDialog.didNotPickAuthProvider', "You didn't select any authentication provider. Please try again.")); + return undefined; + } + return v.id; + } + /** * Asks the requested provider to prompt for an account * @param providerId ID of the provider to ask to prompt for an account @@ -453,6 +485,7 @@ export class AccountManagementService implements IAccountManagementService { } private async _registerProvider(providerMetadata: azdata.AccountProviderMetadata, provider: azdata.AccountProvider): Promise { + this.providerMap.set(providerMetadata.id, providerMetadata); this._providers[providerMetadata.id] = { metadata: providerMetadata, provider: provider, @@ -502,6 +535,7 @@ export class AccountManagementService implements IAccountManagementService { } public unregisterProvider(providerMetadata: azdata.AccountProviderMetadata): void { + this.providerMap.delete(providerMetadata.id); const p = this._providers[providerMetadata.id]; this.fireAccountListUpdate(p, false); // Delete this account provider diff --git a/src/sql/workbench/services/accountManagement/browser/accountPickerImpl.ts b/src/sql/workbench/services/accountManagement/browser/accountPickerImpl.ts index a0331e91db..92209bfa27 100644 --- a/src/sql/workbench/services/accountManagement/browser/accountPickerImpl.ts +++ b/src/sql/workbench/services/accountManagement/browser/accountPickerImpl.ts @@ -62,7 +62,6 @@ export class AccountPicker extends Disposable { public get onTenantSelectionChangeEvent(): Event { return this._onTenantSelectionChangeEvent.event; } constructor( - private _providerId: string, @IThemeService private _themeService: IThemeService, @IInstantiationService private _instantiationService: IInstantiationService, @IContextViewService private _contextViewService: IContextViewService @@ -77,11 +76,9 @@ export class AccountPicker extends Disposable { this._onTenantSelectionChangeEvent = new Emitter(); // Create the view model, wire up the events, and initialize with baseline data - this.viewModel = this._instantiationService.createInstance(AccountPickerViewModel, this._providerId); + this.viewModel = this._instantiationService.createInstance(AccountPickerViewModel); this.viewModel.updateAccountListEvent(arg => { - if (arg.providerId === this._providerId) { - this.updateAccountList(arg.accountList); - } + this.updateAccountList(arg.accountList); }); } @@ -149,7 +146,7 @@ export class AccountPicker extends Disposable { }; // Create the add account action - const addAccountAction = this._instantiationService.createInstance(AddAccountAction, this._providerId); + const addAccountAction = this._instantiationService.createInstance(AddAccountAction, undefined); addAccountAction.addAccountCompleteEvent(() => this._addAccountCompleteEmitter.fire()); addAccountAction.addAccountErrorEvent((msg) => this._addAccountErrorEmitter.fire(msg)); addAccountAction.addAccountStartEvent(() => this._addAccountStartEmitter.fire()); diff --git a/src/sql/workbench/services/accountManagement/browser/accountPickerService.ts b/src/sql/workbench/services/accountManagement/browser/accountPickerService.ts index faa7af1cef..d2db020b92 100644 --- a/src/sql/workbench/services/accountManagement/browser/accountPickerService.ts +++ b/src/sql/workbench/services/accountManagement/browser/accountPickerService.ts @@ -71,9 +71,7 @@ export class AccountPickerService implements IAccountPickerService { */ public renderAccountPicker(rootContainer: HTMLElement): void { if (!this._accountPicker) { - // TODO: expand support to multiple providers - const providerId: string = 'azure_publicCloud'; - this._accountPicker = this._instantiationService.createInstance(AccountPicker, providerId); + this._accountPicker = this._instantiationService.createInstance(AccountPicker); this._accountPicker.createAccountPickerComponent(); } diff --git a/src/sql/workbench/services/accountManagement/test/browser/accountManagementService.test.ts b/src/sql/workbench/services/accountManagement/test/browser/accountManagementService.test.ts index b054a7e63d..738a62f8e7 100644 --- a/src/sql/workbench/services/accountManagement/test/browser/accountManagementService.test.ts +++ b/src/sql/workbench/services/accountManagement/test/browser/accountManagementService.test.ts @@ -540,7 +540,7 @@ function getTestState(): AccountManagementState { // Create the account management service let ams = new AccountManagementService(mockInstantiationService.object, new TestStorageService(), - undefined, undefined, undefined, testNotificationService, mockTelemetryService); + undefined, undefined, undefined, testNotificationService, mockTelemetryService, undefined); // Wire up event handlers let evUpdate = new EventVerifierSingle(); diff --git a/src/sql/workbench/services/accountManagement/test/browser/accountPickerService.test.ts b/src/sql/workbench/services/accountManagement/test/browser/accountPickerService.test.ts index d525be842e..d46247b27b 100644 --- a/src/sql/workbench/services/accountManagement/test/browser/accountPickerService.test.ts +++ b/src/sql/workbench/services/accountManagement/test/browser/accountPickerService.test.ts @@ -91,20 +91,19 @@ suite('Account picker service tests', () => { function createInstantiationService(): InstantiationService { // Create a mock account picker view model - let providerId = 'azure'; - let accountPickerViewModel = new AccountPickerViewModel(providerId, new TestAccountManagementService()); + let accountPickerViewModel = new AccountPickerViewModel(new TestAccountManagementService()); let mockAccountViewModel = TypeMoq.Mock.ofInstance(accountPickerViewModel); let mockEvent = new Emitter(); mockAccountViewModel.setup(x => x.updateAccountListEvent).returns(() => mockEvent.event); // Create a mocked out instantiation service let instantiationService = TypeMoq.Mock.ofType(InstantiationService, TypeMoq.MockBehavior.Strict); - instantiationService.setup(x => x.createInstance(TypeMoq.It.isValue(AccountPickerViewModel), TypeMoq.It.isAny())) + instantiationService.setup(x => x.createInstance(TypeMoq.It.isValue(AccountPickerViewModel))) .returns(() => mockAccountViewModel.object); // Create a mock account picker - let accountPicker = new AccountPicker('provider', new TestThemeService(), instantiationService.object, undefined!); + let accountPicker = new AccountPicker(new TestThemeService(), instantiationService.object, undefined!); let mockAccountDialog = TypeMoq.Mock.ofInstance(accountPicker); mockAccountDialog.setup(x => x.addAccountCompleteEvent) @@ -121,7 +120,7 @@ function createInstantiationService(): InstantiationService { .returns((container) => undefined); mockAccountDialog.setup(x => x.createAccountPickerComponent()); - instantiationService.setup(x => x.createInstance(TypeMoq.It.isValue(AccountPicker), TypeMoq.It.isAny())) + instantiationService.setup(x => x.createInstance(TypeMoq.It.isValue(AccountPicker))) .returns(() => mockAccountDialog.object); return instantiationService.object;