From 4e91c57764b7686df8670e819dea91a2a817eb18 Mon Sep 17 00:00:00 2001 From: Jackson Kearl Date: Mon, 9 Jul 2018 15:38:53 -0700 Subject: [PATCH] Fix bug causing recommendations to sometimes appear in two places (#53906) * Fix bug causing recommendations to sometimes appear in two places * Naming * Group fetch stage calls --- .../electron-browser/extensionTipsService.ts | 25 ++++++++----------- .../extensionsTipsService.test.ts | 20 +++++++-------- 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/src/vs/workbench/parts/extensions/electron-browser/extensionTipsService.ts b/src/vs/workbench/parts/extensions/electron-browser/extensionTipsService.ts index 784c508d624..fbaf724d4ad 100644 --- a/src/vs/workbench/parts/extensions/electron-browser/extensionTipsService.ts +++ b/src/vs/workbench/parts/extensions/electron-browser/extensionTipsService.ts @@ -81,7 +81,7 @@ export class ExtensionTipsService extends Disposable implements IExtensionTipsSe private _workspaceIgnoredRecommendations: string[] = []; private _extensionsRecommendationsUrl: string; private _disposables: IDisposable[] = []; - public loadRecommendationsPromise: TPromise; + public loadWorkspaceConfigPromise: TPromise; private proactiveRecommendationsFetched: boolean = false; private readonly _onRecommendationChange: Emitter = new Emitter(); @@ -120,22 +120,19 @@ export class ExtensionTipsService extends Disposable implements IExtensionTipsSe let globallyIgnored = JSON.parse(this.storageService.get('extensionsAssistant/ignored_recommendations', StorageScope.GLOBAL, '[]')); this._globallyIgnoredRecommendations = globallyIgnored.map(id => id.toLowerCase()); - this.loadRecommendationsPromise = this.getWorkspaceRecommendations() - .then(() => { - // these must be called after workspace configs have been refreshed. - this.fetchCachedDynamicWorkspaceRecommendations(); - this.fetchFileBasedRecommendations(); - this.fetchExperimentalRecommendations(); - return this.promptWorkspaceRecommendations(); - }).then(() => { - this._modelService.onModelAdded(this.promptFiletypeBasedRecommendations, this, this._disposables); - this._modelService.getModels().forEach(model => this.promptFiletypeBasedRecommendations(model)); - }); - + this.fetchCachedDynamicWorkspaceRecommendations(); + this.fetchFileBasedRecommendations(); + this.fetchExperimentalRecommendations(); if (!this.configurationService.getValue(ShowRecommendationsOnlyOnDemandKey)) { this.fetchProactiveRecommendations(true); } + this.loadWorkspaceConfigPromise = this.getWorkspaceRecommendations().then(() => { + this.promptWorkspaceRecommendations(); + this._modelService.onModelAdded(this.promptFiletypeBasedRecommendations, this, this._disposables); + this._modelService.getModels().forEach(model => this.promptFiletypeBasedRecommendations(model)); + }); + this._register(this.contextService.onDidChangeWorkspaceFolders(e => this.onWorkspaceFoldersChanged(e))); this._register(this.configurationService.onDidChangeConfiguration(e => { if (!this.proactiveRecommendationsFetched && !this.configurationService.getValue(ShowRecommendationsOnlyOnDemandKey)) { @@ -411,7 +408,7 @@ export class ExtensionTipsService extends Disposable implements IExtensionTipsSe if (this._exeBasedRecommendations[extensionId]) { sources.push('executable'); } - if (this._dynamicWorkspaceRecommendations[extensionId]) { + if (this._dynamicWorkspaceRecommendations.indexOf(extensionId) !== -1) { sources.push('dynamic'); } return ({ extensionId, sources }); diff --git a/src/vs/workbench/parts/extensions/test/electron-browser/extensionsTipsService.test.ts b/src/vs/workbench/parts/extensions/test/electron-browser/extensionsTipsService.test.ts index faa0bf3f29f..89cbd28c3d6 100644 --- a/src/vs/workbench/parts/extensions/test/electron-browser/extensionsTipsService.test.ts +++ b/src/vs/workbench/parts/extensions/test/electron-browser/extensionsTipsService.test.ts @@ -287,7 +287,7 @@ suite('ExtensionsTipsService Test', () => { function testNoPromptForValidRecommendations(recommendations: string[]) { return setUpFolderWorkspace('myFolder', recommendations).then(() => { testObject = instantiationService.createInstance(ExtensionTipsService); - return testObject.loadRecommendationsPromise.then(() => { + return testObject.loadWorkspaceConfigPromise.then(() => { assert.equal(Object.keys(testObject.getAllRecommendationsWithReason()).length, recommendations.length); assert.ok(!prompted); }); @@ -297,7 +297,7 @@ suite('ExtensionsTipsService Test', () => { function testNoPromptOrRecommendationsForValidRecommendations(recommendations: string[]) { return setUpFolderWorkspace('myFolder', mockTestData.validRecommendedExtensions).then(() => { testObject = instantiationService.createInstance(ExtensionTipsService); - assert.equal(!testObject.loadRecommendationsPromise, true); + assert.equal(!testObject.loadWorkspaceConfigPromise, true); assert.ok(!prompted); return testObject.getWorkspaceRecommendations().then(() => { @@ -327,7 +327,7 @@ suite('ExtensionsTipsService Test', () => { test('ExtensionTipsService: Prompt for valid workspace recommendations', () => { return setUpFolderWorkspace('myFolder', mockTestData.recommendedExtensions).then(() => { testObject = instantiationService.createInstance(ExtensionTipsService); - return testObject.loadRecommendationsPromise.then(() => { + return testObject.loadWorkspaceConfigPromise.then(() => { const recommendations = Object.keys(testObject.getAllRecommendationsWithReason()); assert.equal(recommendations.length, mockTestData.validRecommendedExtensions.length); @@ -359,7 +359,7 @@ suite('ExtensionsTipsService Test', () => { testConfigurationService.setUserConfiguration(ConfigurationKey, { showRecommendationsOnlyOnDemand: true }); return setUpFolderWorkspace('myFolder', mockTestData.validRecommendedExtensions).then(() => { testObject = instantiationService.createInstance(ExtensionTipsService); - return testObject.loadRecommendationsPromise.then(() => { + return testObject.loadWorkspaceConfigPromise.then(() => { assert.equal(Object.keys(testObject.getAllRecommendationsWithReason()).length, 0); assert.ok(!prompted); }); @@ -387,7 +387,7 @@ suite('ExtensionsTipsService Test', () => { return setUpFolderWorkspace('myFolder', mockTestData.validRecommendedExtensions).then(() => { testObject = instantiationService.createInstance(ExtensionTipsService); - return testObject.loadRecommendationsPromise.then(() => { + return testObject.loadWorkspaceConfigPromise.then(() => { const recommendations = testObject.getAllRecommendationsWithReason(); assert.ok(!recommendations['ms-vscode.csharp']); // stored recommendation that has been globally ignored assert.ok(recommendations['ms-python.python']); // stored recommendation @@ -407,7 +407,7 @@ suite('ExtensionsTipsService Test', () => { return setUpFolderWorkspace('myFolder', mockTestData.validRecommendedExtensions, ignoredRecommendations).then(() => { testObject = instantiationService.createInstance(ExtensionTipsService); - return testObject.loadRecommendationsPromise.then(() => { + return testObject.loadWorkspaceConfigPromise.then(() => { const recommendations = testObject.getAllRecommendationsWithReason(); assert.ok(!recommendations['ms-vscode.csharp']); // stored recommendation that has been workspace ignored assert.ok(recommendations['ms-python.python']); // stored recommendation @@ -435,7 +435,7 @@ suite('ExtensionsTipsService Test', () => { return setUpFolderWorkspace('myFolder', mockTestData.validRecommendedExtensions, workspaceIgnoredRecommendations).then(() => { testObject = instantiationService.createInstance(ExtensionTipsService); - return testObject.loadRecommendationsPromise.then(() => { + return testObject.loadWorkspaceConfigPromise.then(() => { const recommendations = testObject.getAllRecommendationsWithReason(); assert.ok(recommendations['ms-python.python']); @@ -462,7 +462,7 @@ suite('ExtensionsTipsService Test', () => { return setUpFolderWorkspace('myFolder', mockTestData.validRecommendedExtensions).then(() => { testObject = instantiationService.createInstance(ExtensionTipsService); - return testObject.loadRecommendationsPromise.then(() => { + return testObject.loadWorkspaceConfigPromise.then(() => { const recommendations = testObject.getAllRecommendationsWithReason(); assert.ok(recommendations['ms-python.python']); assert.ok(recommendations['mockpublisher1.mockextension1']); @@ -516,7 +516,7 @@ suite('ExtensionsTipsService Test', () => { return setUpFolderWorkspace('myFolder', []).then(() => { testObject = instantiationService.createInstance(ExtensionTipsService); - return testObject.loadRecommendationsPromise.then(() => { + return testObject.loadWorkspaceConfigPromise.then(() => { const recommendations = testObject.getFileBasedRecommendations(); assert.equal(recommendations.length, 2); assert.ok(recommendations.some(({ extensionId }) => extensionId === 'ms-vscode.csharp')); // stored recommendation that exists in product.extensionTips @@ -535,7 +535,7 @@ suite('ExtensionsTipsService Test', () => { return setUpFolderWorkspace('myFolder', []).then(() => { testObject = instantiationService.createInstance(ExtensionTipsService); - return testObject.loadRecommendationsPromise.then(() => { + return testObject.loadWorkspaceConfigPromise.then(() => { const recommendations = testObject.getFileBasedRecommendations(); assert.equal(recommendations.length, 2); assert.ok(recommendations.some(({ extensionId }) => extensionId === 'ms-vscode.csharp')); // stored recommendation that exists in product.extensionTips