NEW: @W-18538308@: Make ApexGuru integration GA by removing pilot flag... (#283)

Author: stephen-carter-at-sf

package-lock.json

@@ -60,6 +60,7 @@
         "jest": "^30.0.5",
         "jest-mock-vscode": "^4.6.0",
         "mocha": "^10.8.2",
+        "npm-run-all": "^4.1.5",
         "ovsx": "^0.10.5",
         "proxyquire": "^2.1.3",
         "rimraf": "*",
@@ -140,11 +141,6 @@
                         "type": "boolean",
                         "default": false,
                         "description": "Scan files on open automatically."
-                    },
-                    "codeAnalyzer.apexGuru.enabled": {
-                        "type": "boolean",
-                        "default": false,
-                        "description": "(Pilot) Discover critical problems and performance issues in your Apex code with ApexGuru, which analyzes your Apex files for you. This feature is in a closed pilot; contact your account representative to learn more."
                     }
                 }
             },
@@ -188,7 +184,7 @@
                 },
                 {
                     "command": "sfca.runApexGuruAnalysisOnCurrentFile",
-                    "when": "sfca.extensionActivated && sfca.apexGuruEnabled && resourceExtname =~ /\\.cls|\\.trigger|\\.apex/"
+                    "when": "sfca.extensionActivated && sfca.shouldShowApexGuruButtons && resourceExtname =~ /\\.cls|\\.trigger|\\.apex/"
                 }
             ],
             "editor/context": [
@@ -202,7 +198,7 @@
                 },
                 {
                     "command": "sfca.runApexGuruAnalysisOnCurrentFile",
-                    "when": "sfca.extensionActivated && sfca.apexGuruEnabled && resourceExtname =~ /\\.cls|\\.trigger|\\.apex/"
+                    "when": "sfca.extensionActivated && sfca.shouldShowApexGuruButtons && resourceExtname =~ /\\.cls|\\.trigger|\\.apex/"
                 }
             ],
             "explorer/context": [
@@ -216,7 +212,7 @@
                 },
                 {
                     "command": "sfca.runApexGuruAnalysisOnSelectedFile",
-                    "when": "sfca.extensionActivated && sfca.apexGuruEnabled && explorerResourceIsFolder == false && resourceExtname =~ /\\.cls|\\.trigger|\\.apex/"
+                    "when": "sfca.extensionActivated && sfca.shouldShowApexGuruButtons && explorerResourceIsFolder == false && resourceExtname =~ /\\.cls|\\.trigger|\\.apex/"
                 }
             ]
         },

package.json

@@ -33,7 +33,7 @@ import {PMDSupressionsCodeActionProvider} from './lib/pmd/pmd-suppressions-code-
 import {ApplyViolationFixesActionProvider} from './lib/apply-violation-fixes-action-provider';
 import {ApplyViolationFixesAction} from './lib/apply-violation-fixes-action';
 import {ViolationSuggestionsHoverProvider} from './lib/violation-suggestions-hover-provider';
-import {ApexGuruAccess, ApexGuruAvailability, ApexGuruService, LiveApexGuruService} from './lib/apexguru/apex-guru-service';
+import {ApexGuruAccess, ApexGuruService, LiveApexGuruService} from './lib/apexguru/apex-guru-service';
 import {ApexGuruRunAction} from './lib/apexguru/apex-guru-run-action';
 import {OrgConnectionService} from './lib/external-services/org-connection-service';
 
@@ -246,18 +246,12 @@ export async function activate(context: vscode.ExtensionContext): Promise<SFCAEx
     // =================================================================================================================
     const apexGuruService: ApexGuruService = new LiveApexGuruService(orgConnectionService, fileHandler, logger);
     const apexGuruRunAction: ApexGuruRunAction = new ApexGuruRunAction(taskWithProgressRunner, apexGuruService, diagnosticManager, telemetryService, display);
-
-    // TODO: This is temporary and will change soon when we remove pilot flag and instead add a watch to org auth changes
-    const isApexGuruEnabled: () => Promise<boolean> = async () => {
-        if (!settingsManager.getApexGuruEnabled()) {
-            return false;
-        }
-        const availability: ApexGuruAvailability = await apexGuruService.getAvailability();
-        if (availability.access === ApexGuruAccess.ENABLED || availability.access === ApexGuruAccess.ELIGIBLE) {
-            return true;
-        }
-    };
-    await establishVariableInContext(Constants.CONTEXT_VAR_APEX_GURU_ENABLED, isApexGuruEnabled);
+    apexGuruService.onAccessChange((access: ApexGuruAccess) => {
+        logger.debug(`Access to ApexGuru has been set '${access}'.`);
+        void vscode.commands.executeCommand('setContext', Constants.CONTEXT_VAR_SHOULD_SHOW_APEX_GURU_BUTTONS, 
+            access === ApexGuruAccess.ENABLED || access === ApexGuruAccess.ELIGIBLE);
+    });
+    void apexGuruService.updateAvailability(); // This asyncronously triggers the first AccessChanged Event to establish the context variable
 
     // COMMAND_RUN_APEX_GURU_ON_FILE: Invokable by 'explorer/context' menu only when: "sfca.apexGuruEnabled && explorerResourceIsFolder == false && resourceExtname =~ /\\.cls|\\.trigger|\\.apex/"
     registerCommand(Constants.COMMAND_RUN_APEX_GURU_ON_FILE, async (selection: vscode.Uri, multiSelect?: vscode.Uri[]) => {
@@ -322,19 +316,6 @@ export function _isValidFileForAnalysis(documentUri: vscode.Uri): boolean {
     return allowedFileTypes.includes(path.extname(documentUri.fsPath));
 }
 
-// TODO: This is only used by apex guru right now and is tied to the pilot setting. Soon we will be removing the pilot
-// setting and instead we should be adding a watch to the onOrgChange event of the OrgConnectionService instead.
-// Inside our package.json you'll see things like:
-//     "when": "sfca.apexGuruEnabled"
-// which helps determine when certain commands and menus are available.
-// To make these "context variables" set and stay updated when settings change, use this helper function:
-async function establishVariableInContext(varUsedInPackageJson: string, getValueFcn: () => Promise<boolean>): Promise<void> {
-    await vscode.commands.executeCommand('setContext', varUsedInPackageJson, await getValueFcn());
-    vscode.workspace.onDidChangeConfiguration(async () => {
-        await vscode.commands.executeCommand('setContext', varUsedInPackageJson, await getValueFcn());
-    });
-}
-
 async function getActiveDocument(): Promise<vscode.TextDocument | null> {
     // Note that the active editor window could be the output window instead of the actual file editor, so we
     // force focus it first to ensure we are getting the correct editor

src/extension.ts

@@ -33,7 +33,7 @@ export class ApexGuruRunAction {
             const startTime: number = Date.now();
 
             try {
-                const availability: ApexGuruAvailability = await this.apexGuruService.getAvailability();
+                const availability: ApexGuruAvailability = this.apexGuruService.getAvailability();
                 if (availability.access !== ApexGuruAccess.ENABLED) {
                     this.display.displayError(availability.message);
                     this.telemetryService.sendCommandEvent(Constants.TELEM_APEX_GURU_FILE_ANALYSIS_NOT_ENABLED, {

src/lib/apexguru/apex-guru-run-action.ts

@@ -8,9 +8,10 @@
 import {CodeLocation, Fix, Suggestion, Violation} from '../diagnostics';
 import {Logger} from "../logger";
 import {getErrorMessage, indent} from '../utils';
-import {HttpMethods, HttpRequest, OrgConnectionService} from '../external-services/org-connection-service';
+import {HttpMethods, HttpRequest, OrgConnectionService, OrgUserInfo} from '../external-services/org-connection-service';
 import {FileHandler} from '../fs-utils';
 import { messages } from '../messages';
+import { EventEmitter } from 'node:stream';
 
 export const APEX_GURU_ENGINE_NAME: string = 'apexguru';
 const APEX_GURU_MAX_TIMEOUT_SECONDS = 60;
@@ -24,7 +25,9 @@ const RESPONSE_STATUS = {
 }
 
 export interface ApexGuruService {
-    getAvailability(): Promise<ApexGuruAvailability>;
+    getAvailability(): ApexGuruAvailability;
+    updateAvailability(): Promise<void>;
+    onAccessChange(callback: (access: ApexGuruAccess) => void): void;
     scan(absFileToScan: string): Promise<Violation[]>;
 }
 
@@ -48,12 +51,15 @@ export enum ApexGuruAccess {
     NOT_AUTHED = "not-authed"
 }
 
+const ACCESS_CHANGED_EVENT = "apexGuruAccessChanged";
+
 export class LiveApexGuruService implements ApexGuruService {
     private readonly orgConnectionService: OrgConnectionService;
     private readonly fileHandler: FileHandler;
     private readonly logger: Logger;
     private readonly maxTimeoutSeconds: number;
     private readonly retryIntervalMillis: number;
+    private readonly eventEmitter: EventEmitter = new EventEmitter();
     private availability?: ApexGuruAvailability;
 
     constructor(
@@ -67,42 +73,60 @@ export class LiveApexGuruService implements ApexGuruService {
         this.logger = logger;
         this.maxTimeoutSeconds = maxTimeoutSeconds;
         this.retryIntervalMillis = retryIntervalMillis;
+
+        // Every time an org is changed (authed or unauthed) then we recalculate the availability asyncronously
+        orgConnectionService.onOrgChange((_orgUserInfo: OrgUserInfo) => {
+            void this.updateAvailability();
+        });
     }
 
-    async getAvailability(): Promise<ApexGuruAvailability> {
+    getAvailability(): ApexGuruAvailability {
         if (this.availability === undefined) {
-            await this.updateAvailability();
+            // This should never happen in production because updateAvailability must be called prior to enabling
+            // the user to even have access to any of the ApexGuru scan buttons. If it does, we should investigate.
+            throw new Error('The getAvailability method should not be called until updateAvailability is first called');
         }
         return this.availability;
     }
 
-    // TODO: Soon with W-18538308 we will be using the connection.onOrgChange to wire up to this
-    private async updateAvailability(): Promise<void> {
+    onAccessChange(callback: (access: ApexGuruAccess) => void): void {
+        this.eventEmitter.addListener(ACCESS_CHANGED_EVENT, callback);
+    }
+
+    async updateAvailability(): Promise<void> {
         if (!this.orgConnectionService.isAuthed()) {
-            this.availability = {
+            this.setAvailability({
                 access: ApexGuruAccess.NOT_AUTHED,
                 message: messages.apexGuru.noOrgAuthed
-            };
+            });
             return;
         }
 
         const response: ApexGuruResponse = await this.request('GET', await this.getValidateEndpoint());
 
         if (response.status === RESPONSE_STATUS.SUCCESS) {
-            this.availability = { 
+            this.setAvailability({ 
                 access: ApexGuruAccess.ENABLED,
 
                 // This message isn't used anywhere except for debugging purposes and it allows us to make message field
                 // a string instead of a string | undefined.
                 message: "ApexGuru access is enabled."
-            };
+            });
         } else {
-            this.availability = {
+            this.setAvailability({
                 access:  response.status === RESPONSE_STATUS.FAILED ? ApexGuruAccess.ELIGIBLE : ApexGuruAccess.INELIGIBLE,
 
                 // There should always be a message on failed and error responses, but adding this here just in case
                 message: response.message ?? `ApexGuru access is not enabled. Response:  ${JSON.stringify(response)}`
-            };
+            });
+        }
+    }
+
+    private setAvailability(availability: ApexGuruAvailability) {
+        const oldAccess: ApexGuruAccess | undefined = this.availability?.access;
+        this.availability = availability;
+        if (availability.access !== oldAccess) {
+            this.eventEmitter.emit(ACCESS_CHANGED_EVENT, availability.access);
         }
     }
 

src/lib/apexguru/apex-guru-service.ts

@@ -58,7 +58,7 @@ export const ABSOLUTE_MINIMUM_REQUIRED_CODE_ANALYZER_CLI_PLUGIN_VERSION = '5.0.0
 
 // Context variables (dynamically set but consumed by the "when" conditions in the package.json "contributes" sections)
 export const CONTEXT_VAR_EXTENSION_ACTIVATED = 'sfca.extensionActivated';
-export const CONTEXT_VAR_APEX_GURU_ENABLED = 'sfca.apexGuruEnabled';
+export const CONTEXT_VAR_SHOULD_SHOW_APEX_GURU_BUTTONS = 'sfca.shouldShowApexGuruButtons';
 
 // Documentation URLs
 export const DOCS_SETUP_LINK = 'https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/guide/analyze-vscode.html#install-and-configure-code-analyzer-vs-code-extension';

src/lib/constants.ts

@@ -3,7 +3,7 @@ import * as vscode from "vscode";
 export interface OrgConnectionService {
     isAuthed(): boolean;
     getApiVersion(): Promise<string>;
-    onOrgChange(callback: () => void): void;
+    onOrgChange(callback: (orgUserInfo: OrgUserInfo) => void): void;
     request<T>(requestOptions: HttpRequest): Promise<T>;
 }
 
@@ -21,7 +21,7 @@ export class NoOpOrgConnectionService implements OrgConnectionService {
         throw new Error(`Cannot get the api verison because no org is authed.`);
     }
 
-    onOrgChange(_callback: () => void): void {
+    onOrgChange(_callback: (orgUserInfo: OrgUserInfo) => void): void {
         // No-op
     }
 
@@ -50,7 +50,7 @@ export class LiveOrgConnectionService implements OrgConnectionService {
     }
 
 
-    onOrgChange(callback: (event: OrgUserInfo) => void): void {
+    onOrgChange(callback: (orgUserInfo: OrgUserInfo) => void): void {
         this.workpaceContext.onOrgChange(callback);
     }
 

src/lib/external-services/org-connection-service.ts

@@ -10,7 +10,6 @@ export interface SettingsManager {
     // General Settings
     getAnalyzeOnOpen(): boolean;
     getAnalyzeOnSave(): boolean;
-    getApexGuruEnabled(): boolean;
 
     // Configuration Settings
     getCodeAnalyzerConfigFile(): string;
@@ -32,10 +31,6 @@ export class SettingsManagerImpl implements SettingsManager {
         return vscode.workspace.getConfiguration('codeAnalyzer.analyzeOnSave').get('enabled');
     }
 
-    public getApexGuruEnabled(): boolean {
-        return vscode.workspace.getConfiguration('codeAnalyzer.apexGuru').get('enabled');
-    }
-
     // =================================================================================================================
     // ==== Configuration Settings
     // =================================================================================================================

src/lib/settings.ts

@@ -50,19 +50,4 @@ suite('SettingsManager Test Suite', () => {
         expect(result).to.equal(mockAnalyzeOnOpenEnabled);
         expect(getConfigurationStub.calledOnceWith('codeAnalyzer.analyzeOnOpen')).to.equal(true);
     });
-
-    test('getApexGuruEnabled should return the apexGuru enabled setting', () => {
-        // ===== SETUP =====
-        const mockAnalyzeOnSaveEnabled = true;
-        getConfigurationStub.withArgs('codeAnalyzer.apexGuru').returns({
-            get: Sinon.stub().returns(mockAnalyzeOnSaveEnabled)
-        });
-
-        // ===== TEST =====
-        const result = new SettingsManagerImpl().getApexGuruEnabled();
-
-        // ===== ASSERTIONS =====
-        expect(result).to.equal(mockAnalyzeOnSaveEnabled);
-        expect(getConfigurationStub.calledOnceWith('codeAnalyzer.apexGuru')).to.equal(true);
-    });
 });