Expand account picker support to multiple providers (#23952)

* fix dropdown behavior

* expand account picker support to multiple providers

* fix tests

* fixed unresolved promise issue
This commit is contained in:
Christopher Suh
2023-07-21 08:43:18 -07:00
committed by GitHub
parent 9a891605c8
commit f5bed289af
10 changed files with 57 additions and 20 deletions

View File

@@ -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}`);

View File

@@ -21,7 +21,6 @@ export class AccountPickerViewModel {
public selectedTenantId: string | undefined;
constructor(
_providerId: string,
@IAccountManagementService private _accountManagementService: IAccountManagementService
) {
// Create our event emitters

View File

@@ -21,6 +21,7 @@ export interface IAccountManagementService {
getAccountProviderMetadata(): Promise<azdata.AccountProviderMetadata[]>;
getAccountsForProvider(providerId: string): Promise<azdata.Account[]>;
getAccounts(): Promise<azdata.Account[]>;
promptProvider(): Promise<string | undefined>;
/**
* @deprecated
*/

View File

@@ -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<UpdateAccountListEventParams>
): AccountPickerViewModel {
let vm = new AccountPickerViewModel('azure', ams);
let vm = new AccountPickerViewModel(ams);
vm.updateAccountListEvent(evUpdate.eventHandler);
return vm;

View File

@@ -59,6 +59,10 @@ export class TestAccountManagementService implements IAccountManagementService {
return Promise.resolve(undefined!);
}
promptProvider() {
return Promise.resolve('');
}
removeAccount(accountKey: azdata.AccountKey): Promise<boolean> {
throw new Error('Method not implemented');
}

View File

@@ -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<string, azdata.AccountProviderMetadata>();
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<string | undefined> {
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<void> {
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

View File

@@ -62,7 +62,6 @@ export class AccountPicker extends Disposable {
public get onTenantSelectionChangeEvent(): Event<string | undefined> { 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<string | undefined>();
// 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());

View File

@@ -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();
}

View File

@@ -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<UpdateAccountListEventParams>();

View File

@@ -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<any>();
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;