Skip to content

Commit

Permalink
Allow preference to be honored at the Workspace level
Browse files Browse the repository at this point in the history
This will allow you to have workspace level preferences of accounts. The ideal flow is that an extension can ask for the preferred session (silently) first, to not annoy the user... and if the extension/user wants to use a different account, the extension can ask to clear the session preference and the user will select the account they want to use.

Also serializes the adding of accounts in the menu which allows every account shows up instead of just the last account (a bug I noticed doing this work)

ref #152399
  • Loading branch information
TylerLeonhardt committed Apr 17, 2023
1 parent f63c43c commit 31fd0ce
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 29 deletions.
48 changes: 22 additions & 26 deletions src/vs/workbench/browser/parts/activitybar/activitybarActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,17 +272,11 @@ export class AccountsActivityActionViewItem extends MenuActivityActionViewItem {
}));

this._register(this.authenticationService.onDidChangeSessions(async e => {
const promises = [];
for (const changed of e.event.changed) {
promises.push(this.addOrUpdateAccount(e.providerId, changed.account));
}
for (const added of e.event.added) {
promises.push(this.addOrUpdateAccount(e.providerId, added.account));
}
const result = await Promise.allSettled(promises);
for (const r of result) {
if (r.status === 'rejected') {
this.logService.error(r.reason);
for (const changed of [...e.event.changed, ...e.event.added]) {
try {
await this.addOrUpdateAccount(e.providerId, changed.account);
} catch (e) {
this.logService.error(e);
}
}
for (const removed of e.event.removed) {
Expand Down Expand Up @@ -395,13 +389,19 @@ export class AccountsActivityActionViewItem extends MenuActivityActionViewItem {
//#region groupedAccounts helpers

private async addOrUpdateAccount(providerId: string, account: AuthenticationSessionAccount): Promise<void> {
const accounts = this.groupedAccounts.get(providerId);
const existingAccount = accounts?.find(a => a.id === account.id);
if (existingAccount) {
if (existingAccount.label !== account.label) {
existingAccount.label = account.label;
let accounts = this.groupedAccounts.get(providerId);
if (accounts) {
const existingAccount = accounts.find(a => a.id === account.id);
if (existingAccount) {
// Update the label if it has changed
if (existingAccount.label !== account.label) {
existingAccount.label = account.label;
}
return;
}
return;
} else {
accounts = [];
this.groupedAccounts.set(providerId, accounts);
}

const sessionFromEmbedder = await this.sessionFromEmbedder;
Expand All @@ -417,11 +417,6 @@ export class AccountsActivityActionViewItem extends MenuActivityActionViewItem {
}
}

if (!accounts) {
this.groupedAccounts.set(providerId, [{ ...account, canSignOut }]);
return;
}

accounts.push({ ...account, canSignOut });
}

Expand All @@ -447,10 +442,11 @@ export class AccountsActivityActionViewItem extends MenuActivityActionViewItem {
const sessions = await this.authenticationService.getSessions(providerId);
this.problematicProviders.delete(providerId);

const result = await Promise.allSettled(sessions.map(s => this.addOrUpdateAccount(providerId, s.account)));
for (const r of result) {
if (r.status === 'rejected') {
this.logService.error(r.reason);
for (const session of sessions) {
try {
await this.addOrUpdateAccount(providerId, session.account);
} catch (e) {
this.logService.error(e);
}
}
} catch (e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -413,15 +413,39 @@ export class AuthenticationService extends Disposable implements IAuthentication
// * Extension id: The extension that has a preference
// * Provider id: The provider that the preference is for
// * The scopes: The subset of sessions that the preference applies to
this.storageService.store(`${extensionId}-${providerId}-${session.scopes.join(' ')}`, session.id, StorageScope.APPLICATION, StorageTarget.MACHINE);
const key = `${extensionId}-${providerId}-${session.scopes.join(' ')}`;

// Store the preference in the workspace and application storage. This allows new workspaces to
// have a preference set already to limit the number of prompts that are shown... but also allows
// a specific workspace to override the global preference.
this.storageService.store(key, session.id, StorageScope.WORKSPACE, StorageTarget.MACHINE);
this.storageService.store(key, session.id, StorageScope.APPLICATION, StorageTarget.MACHINE);
}

getSessionPreference(providerId: string, extensionId: string, scopes: string[]): string | undefined {
return this.storageService.get(`${extensionId}-${providerId}-${scopes.join(' ')}`, StorageScope.APPLICATION, undefined);
// The 3 parts of this key are important:
// * Extension id: The extension that has a preference
// * Provider id: The provider that the preference is for
// * The scopes: The subset of sessions that the preference applies to
const key = `${extensionId}-${providerId}-${scopes.join(' ')}`;

// If a preference is set in the workspace, use that. Otherwise, use the global preference.
return this.storageService.get(key, StorageScope.WORKSPACE) ?? this.storageService.get(key, StorageScope.APPLICATION);
}

removeSessionPreference(providerId: string, extensionId: string, scopes: string[]): void {
this.storageService.remove(`${extensionId}-${providerId}-${scopes.join(' ')}`, StorageScope.APPLICATION);
// The 3 parts of this key are important:
// * Extension id: The extension that has a preference
// * Provider id: The provider that the preference is for
// * The scopes: The subset of sessions that the preference applies to
const key = `${extensionId}-${providerId}-${scopes.join(' ')}`;

// This won't affect any other workspaces that have a preference set, but it will remove the preference
// for this workspace and the global preference. This is only paired with a call to updateSessionPreference...
// so we really don't _need_ to remove them as they are about to be overridden anyway... but it's more correct
// to remove them first... and in case this gets called from somewhere else in the future.
this.storageService.remove(key, StorageScope.WORKSPACE);
this.storageService.remove(key, StorageScope.APPLICATION);
}

//#endregion
Expand Down

0 comments on commit 31fd0ce

Please sign in to comment.