Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX (Extension) @W-16733527@ Additional telemetry for apex guru #139

Merged
merged 2 commits into from
Sep 30, 2024
Merged

FIX (Extension) @W-16733527@ Additional telemetry for apex guru #139

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
101a2c2
FIX (Extension) @W-16733527@ Additional telemetry for apex guru
jag-j Sep 27, 2024
2c630e6
FIX (Extension) @W-16733527@ Additional telemetry for apex guru - add…
jag-j Sep 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/apexguru/apex-guru-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ export async function runApexGuruOnFile(selection: vscode.Uri, runInfo: RunInfo)
new DiagnosticManager().displayDiagnostics([selection.fsPath], [ruleResult], diagnosticCollection);
TelemetryService.sendCommandEvent(Constants.TELEM_SUCCESSFUL_APEX_GURU_FILE_ANALYSIS, {
executedCommand: commandName,
duration: (Date.now() - startTime).toString()
duration: (Date.now() - startTime).toString(),
violationsCount: ruleResult.violations.length.toString(),
violationsWithSuggestedCode: getViolationsWithSuggestions(ruleResult).toString()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be violationsWithSuggestedCodeCount to match violationsCount?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! that sounds better. I'll rename it.

});
void vscode.window.showInformationMessage(messages.apexGuru.finishedScan(ruleResult.violations.length));
});
Expand All @@ -68,6 +70,11 @@ export async function runApexGuruOnFile(selection: vscode.Uri, runInfo: RunInfo)
}
}

export function getViolationsWithSuggestions(ruleResult: RuleResult): number {
// Filter violations that have a non-empty suggestedCode and get count
return ruleResult.violations.filter(violation => (violation as ApexGuruViolation).suggestedCode?.trim() !== '').length;
}

export async function pollAndGetApexGuruResponse(connection: Connection, requestId: string, maxWaitTimeInSeconds: number, retryIntervalInMillis: number): Promise<ApexGuruQueryResponse> {
let queryResponse: ApexGuruQueryResponse;
let lastErrorMessage = '';
Expand Down
11 changes: 10 additions & 1 deletion src/extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,16 @@ export async function activate(context: vscode.ExtensionContext): Promise<vscode
outputChannel: outputChannel
});
});
context.subscriptions.push(runApexGuruOnSelectedFile, runApexGuruOnCurrentFile);
const insertApexGuruSuggestions = vscode.commands.registerCommand(Constants.COMMAND_INCLUDE_APEX_GURU_SUGGESTIONS, async (document: vscode.TextDocument, position: vscode.Position, suggestedCode: string) => {
const edit = new vscode.WorkspaceEdit();
edit.insert(document.uri, position, suggestedCode);
await vscode.workspace.applyEdit(edit);
TelemetryService.sendCommandEvent(Constants.TELEM_SUCCESSFUL_APEX_GURU_FILE_ANALYSIS, {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New telemetry to get number of times users used the suggestions ApexGuru provided, along with the number of lines that were included.

executedCommand: Constants.COMMAND_INCLUDE_APEX_GURU_SUGGESTIONS,
lines: suggestedCode.split('\n').length.toString()
});
})
context.subscriptions.push(runApexGuruOnSelectedFile, runApexGuruOnCurrentFile, insertApexGuruSuggestions);
}

if (SettingsManager.getSfgeDeltaRunsEnabled()) {
Expand Down
1 change: 1 addition & 0 deletions src/lib/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export const COMMAND_REMOVE_DIAGNOSTICS_ON_SELECTED_FILE = 'sfca.removeDiagnosti
export const COMMAND_DIAGNOSTICS_IN_RANGE = 'sfca.removeDiagnosticsInRange';
export const COMMAND_RUN_APEX_GURU_ON_FILE = 'sfca.runApexGuruAnalysisOnSelectedFile';
export const COMMAND_RUN_APEX_GURU_ON_ACTIVE_FILE = 'sfca.runApexGuruAnalysisOnCurrentFile';
export const COMMAND_INCLUDE_APEX_GURU_SUGGESTIONS = 'sfca.includeApexGuruSuggestions';

// telemetry event keys
export const TELEM_SUCCESSFUL_STATIC_ANALYSIS = 'sfdx__codeanalyzer_static_run_complete';
Expand Down
12 changes: 6 additions & 6 deletions src/lib/fixer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,14 @@ export class _ApexGuruFixGenerator extends FixGenerator {

const action = new vscode.CodeAction(messages.fixer.fixWithApexGuruSuggestions, vscode.CodeActionKind.QuickFix);
action.diagnostics = [this.diagnostic];

const edit = new vscode.WorkspaceEdit();
const range = this.diagnostic.range; // Assuming the range is the location of the existing code in the document
const diagnosticStartLine = new vscode.Position(range.start.line, range.start.character);
edit.insert(document.uri, diagnosticStartLine, suggestedCode + '\n');
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I do this, there is no way to get the telemetry on how many times users clicked on the "include apex guru suggestions" quick-fix. I had to to make this a command and add it to the action and I can report telemetry on that command.


// Assign the edit to the action
action.edit = edit;

action.command = {
title: 'Apply ApexGuru Fix',
command: Constants.COMMAND_INCLUDE_APEX_GURU_SUGGESTIONS,
arguments: [document, diagnosticStartLine, suggestedCode + '\n']
}

return action;
}
Expand Down
136 changes: 136 additions & 0 deletions src/test/suite/apexguru/apex-guru-service.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,4 +371,140 @@ suite('Apex Guru Test Suite', () => {
expect(connectionStub.request.callCount).to.be.greaterThan(0);
});
});
suite('#getViolationsWithSuggestions', () => {
test('Returns 0 when there are no violations', () => {
// ===== SETUP =====
const ruleResult: RuleResult = {
engine: 'fake_engine',
fileName: 'test.cls',
violations: []
};
// ===== TEST =====
const result = ApexGuruFunctions.getViolationsWithSuggestions(ruleResult);
// ===== ASSERTIONS =====
expect(result).to.equal(0);
});

test('Returns 0 when there are violations but no suggestions', () => {
// ===== SETUP =====
const ruleResult: RuleResult = {
engine: 'fake_engine',
fileName: 'test.cls',
violations: [
{
ruleName: 'BestPractices',
message: 'Avoid using System.debug',
severity: 1,
category: 'BestPractices',
line: 10,
column: 1,
currentCode: 'System.debug();',
suggestedCode: '', // No suggested code
url: 'TestFile.cls'
} as ApexGuruViolation
]
};
// ===== TEST =====
const result = ApexGuruFunctions.getViolationsWithSuggestions(ruleResult);
// ===== ASSERTIONS =====
expect(result).to.equal(0);
});

test('Returns correct count when there are violations with suggestions', () => {
// ===== SETUP =====
const ruleResult: RuleResult = {
engine: 'fake_engine',
fileName: 'test.cls',
violations: [
{
ruleName: 'BestPractices',
message: 'Avoid using System.debug',
severity: 1,
category: 'BestPractices',
line: 10,
column: 1,
currentCode: 'System.debug();',
suggestedCode: 'System.out.println("Hello World");',
url: 'TestFile.cls'
} as ApexGuruViolation
]
};
// ===== TEST =====
const result = ApexGuruFunctions.getViolationsWithSuggestions(ruleResult);
// ===== ASSERTIONS =====
expect(result).to.equal(1);
});

test('Returns correct count when multiple violations have suggestions', () => {
// ===== SETUP =====
const ruleResult: RuleResult = {
engine: 'fake_engine',
fileName: 'test.cls',
violations: [
{
ruleName: 'BestPractices',
message: 'Avoid using System.debug',
severity: 1,
category: 'BestPractices',
line: 10,
column: 1,
currentCode: 'System.debug();',
suggestedCode: 'System.out.println("Hello World");',
url: 'TestFile.cls'
} as ApexGuruViolation,
{
ruleName: 'CodeQuality',
message: 'Improve variable naming',
severity: 1,
category: 'CodeQuality',
line: 12,
column: 2,
currentCode: 'int x;',
suggestedCode: 'int userCount;',
url: 'TestFile.cls'
} as ApexGuruViolation
]
};
// ===== TEST =====
const result = ApexGuruFunctions.getViolationsWithSuggestions(ruleResult);
// ===== ASSERTIONS =====
expect(result).to.equal(2);
});

test('Ignores violations without suggestedCode', () => {
// ===== SETUP =====
const ruleResult: RuleResult = {
engine: 'fake_engine',
fileName: 'test.cls',
violations: [
{
ruleName: 'BestPractices',
message: 'Avoid using System.debug',
severity: 1,
category: 'BestPractices',
line: 10,
column: 1,
currentCode: 'System.debug();',
suggestedCode: 'System.out.println("Hello World");',
url: 'TestFile.cls'
} as ApexGuruViolation,
{
ruleName: 'CodeQuality',
message: 'Improve variable naming',
severity: 1,
category: 'CodeQuality',
line: 12,
column: 2,
currentCode: 'int x;',
suggestedCode: '', // No suggestion for this violation
url: 'TestFile.cls'
} as ApexGuruViolation
]
};
// ===== TEST =====
const result = ApexGuruFunctions.getViolationsWithSuggestions(ruleResult);
// ===== ASSERTIONS =====
expect(result).to.equal(1);
});
});
});
8 changes: 3 additions & 5 deletions src/test/suite/fixer.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import * as vscode from 'vscode';
import {expect} from 'chai';
import path = require('path');
import * as Constants from '../../lib/constants';
import {_NoOpFixGenerator, _PmdFixGenerator, _ApexGuruFixGenerator} from '../../lib/fixer';

suite('fixer.ts', () => {
Expand Down Expand Up @@ -529,9 +530,7 @@ suite('fixer.ts', () => {

// Validate results.
expect(fixes).to.have.lengthOf(1, 'One fix should be offered');
const fix = fixes[0].edit.get(fileUri)[0];
expect(fix.newText).to.equal('apex guru suggested code\n', 'The suppression code should match the suggestion');
expect(fix.range.start.line).to.equal(7, 'The suppression should be added at the diagnostic line');
expect(fixes[0].command.command).to.equal(Constants.COMMAND_INCLUDE_APEX_GURU_SUGGESTIONS);
});

test('Should not generate a suppression fix if line is already processed', async () => {
Expand Down Expand Up @@ -604,8 +603,7 @@ suite('fixer.ts', () => {
const fix = fixGenerator.generateApexGuruSuppresssion(doc);

// Validate results.
expect(fix.edit.get(fileUri)[0].newText).to.equal('apex guru suggested code\n', 'The suppression code should match the suggestion');
expect(fix.edit.get(fileUri)[0].range.start.line).to.equal(7, 'The suppression should be added at the diagnostic line');
expect(fix.command.command).to.equal(Constants.COMMAND_INCLUDE_APEX_GURU_SUGGESTIONS);
});
});
});
Expand Down