From 84018db7a70a44042ee0cb70a6fefcf7ed1c47e8 Mon Sep 17 00:00:00 2001 From: Vered Constantin1 Date: Sun, 10 Mar 2024 11:21:29 +0200 Subject: [PATCH 1/7] fix: add report closing wizard --- .../src/panels/AbstractWebviewPanel.ts | 12 +++++++++- .../usage-report/usage-analytics-wrapper.ts | 22 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/backend/src/panels/AbstractWebviewPanel.ts b/packages/backend/src/panels/AbstractWebviewPanel.ts index 5508dd7f5..6a027ccbf 100644 --- a/packages/backend/src/panels/AbstractWebviewPanel.ts +++ b/packages/backend/src/panels/AbstractWebviewPanel.ts @@ -6,6 +6,7 @@ import { getClassLogger } from "../logger/logger-wrapper"; import { RpcExtension } from "@sap-devx/webview-rpc/out.ext/rpc-extension"; import { createFlowPromise, FlowPromise } from "../utils/promise"; import * as cheerio from "cheerio"; +import { AnalyticsWrapper } from "../usage-report/usage-analytics-wrapper"; export abstract class AbstractWebviewPanel { public viewType: string; @@ -105,7 +106,16 @@ export abstract class AbstractWebviewPanel { protected dispose() { this.setFocused(false); - + const yeomanui = (this as any).yeomanui; + const wizardStepName = yeomanui.gen.prompts.items[yeomanui.promptCount - 1].name; + // generatorName: string, wizardStepName: string, currentStep: number, totalNumOfSteps: number + AnalyticsWrapper.updateGeneratorClosedManually( + yeomanui.generatorName, + wizardStepName, + yeomanui.promptCount, + yeomanui.gen.prompts.items.length, + this.logger, + ); // Clean up our resources this.webViewPanel.dispose(); this.webViewPanel = null; diff --git a/packages/backend/src/usage-report/usage-analytics-wrapper.ts b/packages/backend/src/usage-report/usage-analytics-wrapper.ts index a8d1e4ebe..067916cbe 100644 --- a/packages/backend/src/usage-report/usage-analytics-wrapper.ts +++ b/packages/backend/src/usage-report/usage-analytics-wrapper.ts @@ -10,6 +10,7 @@ export class AnalyticsWrapper { private static readonly EVENT_TYPES = { PROJECT_GENERATION_STARTED: "Project generation started", PROJECT_GENERATOR_SELECTED: "Project generator selected", + PROJECT_GENERATOR_CLOSED: "Project generator was closed manually", PROJECT_GENERATED_SUCCESSFULLY: "Project generated successfully", }; @@ -87,4 +88,25 @@ export class AnalyticsWrapper { logger?.error(error); } } + + public static updateGeneratorClosedManually( + generatorName: string, + wizardStepName: string, + currentStep: number, + totalNumOfSteps: number, + logger?: IChildLogger, + ): void { + try { + const eventName = AnalyticsWrapper.EVENT_TYPES.PROJECT_GENERATOR_CLOSED; + const properties: any = { + generatorName, + wizardStepName, + currentStep, + totalNumOfSteps, + }; + AnalyticsWrapper.report({ eventName, properties, logger }); + } catch (error) { + logger?.error(error); + } + } } From ec31b16ce34e9973e6255366ee27fbdf13efbcf3 Mon Sep 17 00:00:00 2001 From: Vered Constantin1 Date: Sun, 10 Mar 2024 14:37:59 +0200 Subject: [PATCH 2/7] fix: add test --- .../src/panels/AbstractWebviewPanel.ts | 25 +++++++------ .../backend/test/panels/YeomanUIPanel.spec.ts | 36 +++++++++++++++++++ 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/packages/backend/src/panels/AbstractWebviewPanel.ts b/packages/backend/src/panels/AbstractWebviewPanel.ts index 6a027ccbf..4c71e47d1 100644 --- a/packages/backend/src/panels/AbstractWebviewPanel.ts +++ b/packages/backend/src/panels/AbstractWebviewPanel.ts @@ -7,6 +7,7 @@ import { RpcExtension } from "@sap-devx/webview-rpc/out.ext/rpc-extension"; import { createFlowPromise, FlowPromise } from "../utils/promise"; import * as cheerio from "cheerio"; import { AnalyticsWrapper } from "../usage-report/usage-analytics-wrapper"; +import { get } from "lodash"; export abstract class AbstractWebviewPanel { public viewType: string; @@ -106,16 +107,20 @@ export abstract class AbstractWebviewPanel { protected dispose() { this.setFocused(false); - const yeomanui = (this as any).yeomanui; - const wizardStepName = yeomanui.gen.prompts.items[yeomanui.promptCount - 1].name; - // generatorName: string, wizardStepName: string, currentStep: number, totalNumOfSteps: number - AnalyticsWrapper.updateGeneratorClosedManually( - yeomanui.generatorName, - wizardStepName, - yeomanui.promptCount, - yeomanui.gen.prompts.items.length, - this.logger, - ); + const yeomanui: any = get(this, "yeomanui"); + if (yeomanui) { + const promptItems: any = get(yeomanui, "gen.prompts.items") ?? []; + const promptCount = yeomanui.promptCount; + const wizardStepName = promptItems[promptCount - 1].name; + AnalyticsWrapper.updateGeneratorClosedManually( + yeomanui.generatorName, + wizardStepName, + promptCount, + promptItems.length, + this.logger, + ); + } + // Clean up our resources this.webViewPanel.dispose(); this.webViewPanel = null; diff --git a/packages/backend/test/panels/YeomanUIPanel.spec.ts b/packages/backend/test/panels/YeomanUIPanel.spec.ts index ca953c040..e9a7cea66 100644 --- a/packages/backend/test/panels/YeomanUIPanel.spec.ts +++ b/packages/backend/test/panels/YeomanUIPanel.spec.ts @@ -11,6 +11,7 @@ import { expect } from "chai"; import { join } from "path"; import { homedir } from "os"; import messages from "../../src/messages"; +import { AnalyticsWrapper } from "../../src/usage-report/usage-analytics-wrapper"; describe("YeomanUIPanel unit test", () => { let sandbox: SinonSandbox; @@ -22,6 +23,7 @@ describe("YeomanUIPanel unit test", () => { let panel: YeomanUIPanel.YeomanUIPanel; let setWebviewPanelStub: SinonStub; let createWebviewPanelStub: SinonStub; + let trackerWrapperMock: SinonMock; before(() => { sandbox = createSandbox(); @@ -41,6 +43,7 @@ describe("YeomanUIPanel unit test", () => { panel = new YeomanUIPanel.YeomanUIPanel(vscode.context); setWebviewPanelStub = sandbox.stub(panel, "setWebviewPanel"); createWebviewPanelStub = sandbox.stub(panel, "createWebviewPanel"); + trackerWrapperMock = sandbox.mock(AnalyticsWrapper); }); afterEach(() => { @@ -49,6 +52,7 @@ describe("YeomanUIPanel unit test", () => { npmUtilsMock.verify(); windowMock.verify(); commandsMock.verify(); + trackerWrapperMock.verify(); setWebviewPanelStub.restore(); createWebviewPanelStub.restore(); }); @@ -269,4 +273,36 @@ describe("YeomanUIPanel unit test", () => { expect(await panel["showOpenDialog"](required, canSelectFiles)).to.equal(required); }); }); + + describe("dispose", () => { + it("dispose - calling usage analytics when panel is manually closed.", () => { + const objYeomanui: any = { + generatorName: "generator-name", + promptCount: 1, + gen: { + prompts: { + items: [ + { + name: "step1", + }, + { + name: "step2", + }, + ], + }, + }, + }; + set(panel, "yeomanui", objYeomanui); + const webviewPanel = { + dispose: () => {}, + }; + set(panel, "webViewPanel", webviewPanel); + set(panel, "disposables", []); + set(panel, "cleanFlowPromise", () => {}); + + commandsMock.expects("executeCommand").withExactArgs("setContext", "yeomanUI.Focused", false).resolves(); + trackerWrapperMock.expects("updateGeneratorClosedManually").withArgs("generator-name", "step1", 1, 2).resolves(); + panel["dispose"](); + }); + }); }); From 03f73fc05120746ae8a7ac096115902f9590c135 Mon Sep 17 00:00:00 2001 From: Vered Constantin1 Date: Sun, 10 Mar 2024 14:48:33 +0200 Subject: [PATCH 3/7] fix: test --- packages/backend/test/panels/YeomanUIPanel.spec.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/backend/test/panels/YeomanUIPanel.spec.ts b/packages/backend/test/panels/YeomanUIPanel.spec.ts index e9a7cea66..2596d15b7 100644 --- a/packages/backend/test/panels/YeomanUIPanel.spec.ts +++ b/packages/backend/test/panels/YeomanUIPanel.spec.ts @@ -301,7 +301,11 @@ describe("YeomanUIPanel unit test", () => { set(panel, "cleanFlowPromise", () => {}); commandsMock.expects("executeCommand").withExactArgs("setContext", "yeomanUI.Focused", false).resolves(); - trackerWrapperMock.expects("updateGeneratorClosedManually").withArgs("generator-name", "step1", 1, 2).resolves(); + trackerWrapperMock + .expects("updateGeneratorClosedManually") + .withArgs("generator-name", "step1", 1, 2) + .once() + .resolves(); panel["dispose"](); }); }); From 5c7caeee14266849b7037e40228f2a4efb10b142 Mon Sep 17 00:00:00 2001 From: Vered Constantin1 Date: Sun, 17 Mar 2024 14:33:13 +0200 Subject: [PATCH 4/7] fix: verify closed manually --- .../src/panels/AbstractWebviewPanel.ts | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/packages/backend/src/panels/AbstractWebviewPanel.ts b/packages/backend/src/panels/AbstractWebviewPanel.ts index 4c71e47d1..262d68d3b 100644 --- a/packages/backend/src/panels/AbstractWebviewPanel.ts +++ b/packages/backend/src/panels/AbstractWebviewPanel.ts @@ -110,15 +110,19 @@ export abstract class AbstractWebviewPanel { const yeomanui: any = get(this, "yeomanui"); if (yeomanui) { const promptItems: any = get(yeomanui, "gen.prompts.items") ?? []; - const promptCount = yeomanui.promptCount; - const wizardStepName = promptItems[promptCount - 1].name; - AnalyticsWrapper.updateGeneratorClosedManually( - yeomanui.generatorName, - wizardStepName, - promptCount, - promptItems.length, - this.logger, - ); + const currentPromptCount = yeomanui.promptCount; + const numOfPromopts = promptItems.length; + // Verify the dispose happened before the user has finished the wizard and not by clicking "Finish". + if (currentPromptCount <= numOfPromopts) { + const wizardStepName = promptItems[currentPromptCount - 1].name; + AnalyticsWrapper.updateGeneratorClosedManually( + yeomanui.generatorName, + wizardStepName, + currentPromptCount, + numOfPromopts, + this.logger, + ); + } } // Clean up our resources From cb593c2ddac1fe8725f337f331392975d7628c7e Mon Sep 17 00:00:00 2001 From: Vered Constantin1 Date: Mon, 18 Mar 2024 12:29:31 +0200 Subject: [PATCH 5/7] fix: report close manually --- .../src/panels/AbstractWebviewPanel.ts | 26 +++++++------- packages/backend/src/utils/constants.ts | 1 + packages/backend/src/vscode-youi-events.ts | 4 ++- .../backend/test/panels/YeomanUIPanel.spec.ts | 34 +++++++++++++++++++ 4 files changed, 52 insertions(+), 13 deletions(-) diff --git a/packages/backend/src/panels/AbstractWebviewPanel.ts b/packages/backend/src/panels/AbstractWebviewPanel.ts index 262d68d3b..9feb66766 100644 --- a/packages/backend/src/panels/AbstractWebviewPanel.ts +++ b/packages/backend/src/panels/AbstractWebviewPanel.ts @@ -8,6 +8,7 @@ import { createFlowPromise, FlowPromise } from "../utils/promise"; import * as cheerio from "cheerio"; import { AnalyticsWrapper } from "../usage-report/usage-analytics-wrapper"; import { get } from "lodash"; +import { Constants } from "../utils/constants"; export abstract class AbstractWebviewPanel { public viewType: string; @@ -107,22 +108,23 @@ export abstract class AbstractWebviewPanel { protected dispose() { this.setFocused(false); + const isGeneratorCompleted = get(this.webViewPanel, Constants.GENERATOR_COMPLETED); const yeomanui: any = get(this, "yeomanui"); - if (yeomanui) { + // Verify the dispose happened before the user has finished the wizard and not by clicking "Finish". + // When user clicks "Finish" the generated has ended and the success/failure result will be set in "GENERATOR_COMPLETED". + // If it has not been ended, "GENERATOR_COMPLETED" will be undefined. + if (yeomanui && isGeneratorCompleted === undefined) { const promptItems: any = get(yeomanui, "gen.prompts.items") ?? []; const currentPromptCount = yeomanui.promptCount; const numOfPromopts = promptItems.length; - // Verify the dispose happened before the user has finished the wizard and not by clicking "Finish". - if (currentPromptCount <= numOfPromopts) { - const wizardStepName = promptItems[currentPromptCount - 1].name; - AnalyticsWrapper.updateGeneratorClosedManually( - yeomanui.generatorName, - wizardStepName, - currentPromptCount, - numOfPromopts, - this.logger, - ); - } + const wizardStepName = promptItems[currentPromptCount - 1].name; + AnalyticsWrapper.updateGeneratorClosedManually( + yeomanui.generatorName, + wizardStepName, + currentPromptCount, + numOfPromopts, + this.logger, + ); } // Clean up our resources diff --git a/packages/backend/src/utils/constants.ts b/packages/backend/src/utils/constants.ts index ca856d20f..f91ae509d 100644 --- a/packages/backend/src/utils/constants.ts +++ b/packages/backend/src/utils/constants.ts @@ -6,6 +6,7 @@ import { devspace } from "@sap/bas-sdk"; class ConstantsUtil { public IS_IN_BAS = !isEmpty(get(process, "env.WS_BASE_URL")) || devspace.getBasMode() === "personal-edition"; public HOMEDIR_PROJECTS: string = join(homedir(), "projects"); + public GENERATOR_COMPLETED: string = "generatorCompleted"; } export const Constants = new ConstantsUtil(); diff --git a/packages/backend/src/vscode-youi-events.ts b/packages/backend/src/vscode-youi-events.ts index e9a4ad549..3f10ad71a 100644 --- a/packages/backend/src/vscode-youi-events.ts +++ b/packages/backend/src/vscode-youi-events.ts @@ -1,5 +1,5 @@ import * as vscode from "vscode"; -import { isEmpty, size, isNil } from "lodash"; +import { isEmpty, size, isNil, set } from "lodash"; import { YouiEvents } from "./youi-events"; import { IRpc } from "@sap-devx/webview-rpc/out.ext/rpc-common"; import { GeneratorOutput } from "./vscode-output"; @@ -8,6 +8,7 @@ import { getClassLogger } from "./logger/logger-wrapper"; import { getImage } from "./images/messageImages"; import { AppWizard, MessageType, Severity } from "@sap-devx/yeoman-ui-types"; import { WorkspaceFile } from "./utils/workspaceFile"; +import { Constants } from "./utils/constants"; class YoUiAppWizard extends AppWizard { constructor(private readonly events: VSCodeYouiEvents) { @@ -65,6 +66,7 @@ export class VSCodeYouiEvents implements YouiEvents { type: string, targetFolderPath?: string, ): void { + set(this.webviewPanel, Constants.GENERATOR_COMPLETED, success); this.doClose(); void this.showDoneMessage(success, message, selectedWorkspace, type, targetFolderPath); } diff --git a/packages/backend/test/panels/YeomanUIPanel.spec.ts b/packages/backend/test/panels/YeomanUIPanel.spec.ts index 2596d15b7..ffdad265b 100644 --- a/packages/backend/test/panels/YeomanUIPanel.spec.ts +++ b/packages/backend/test/panels/YeomanUIPanel.spec.ts @@ -308,5 +308,39 @@ describe("YeomanUIPanel unit test", () => { .resolves(); panel["dispose"](); }); + + it("dispose - not calling usage analytics when generation ended and user clicked finish.", () => { + const objYeomanui: any = { + generatorName: "generator-name", + promptCount: 1, + gen: { + prompts: { + items: [ + { + name: "step1", + }, + { + name: "step2", + }, + ], + }, + }, + }; + set(panel, "yeomanui", objYeomanui); + const webviewPanel = { + dispose: () => {}, + }; + set(webviewPanel, Constants.GENERATOR_COMPLETED, true); + set(panel, "webViewPanel", webviewPanel); + set(panel, "disposables", []); + set(panel, "cleanFlowPromise", () => {}); + + commandsMock.expects("executeCommand").withExactArgs("setContext", "yeomanUI.Focused", false).resolves(); + trackerWrapperMock + .expects("updateGeneratorClosedManually") + .never() + .resolves(); + panel["dispose"](); + }); }); }); From db8a0ec84af0fc1769213c2030e29c6633fe2064 Mon Sep 17 00:00:00 2001 From: Vered Constantin1 Date: Mon, 18 Mar 2024 12:41:47 +0200 Subject: [PATCH 6/7] fix: refactor test --- .../backend/test/panels/YeomanUIPanel.spec.ts | 73 +++++++------------ 1 file changed, 25 insertions(+), 48 deletions(-) diff --git a/packages/backend/test/panels/YeomanUIPanel.spec.ts b/packages/backend/test/panels/YeomanUIPanel.spec.ts index ffdad265b..0c3f06037 100644 --- a/packages/backend/test/panels/YeomanUIPanel.spec.ts +++ b/packages/backend/test/panels/YeomanUIPanel.spec.ts @@ -275,32 +275,37 @@ describe("YeomanUIPanel unit test", () => { }); describe("dispose", () => { - it("dispose - calling usage analytics when panel is manually closed.", () => { - const objYeomanui: any = { - generatorName: "generator-name", - promptCount: 1, - gen: { - prompts: { - items: [ - { - name: "step1", - }, - { - name: "step2", - }, - ], - }, + const objYeomanui: any = { + generatorName: "generator-name", + promptCount: 1, + gen: { + prompts: { + items: [ + { + name: "step1", + }, + { + name: "step2", + }, + ], }, - }; + }, + }; + + const webviewPanel = { + dispose: () => {}, + }; + + beforeEach(() => { set(panel, "yeomanui", objYeomanui); - const webviewPanel = { - dispose: () => {}, - }; set(panel, "webViewPanel", webviewPanel); set(panel, "disposables", []); set(panel, "cleanFlowPromise", () => {}); commandsMock.expects("executeCommand").withExactArgs("setContext", "yeomanUI.Focused", false).resolves(); + }); + + it("dispose - calling usage analytics when panel is manually closed.", () => { trackerWrapperMock .expects("updateGeneratorClosedManually") .withArgs("generator-name", "step1", 1, 2) @@ -310,36 +315,8 @@ describe("YeomanUIPanel unit test", () => { }); it("dispose - not calling usage analytics when generation ended and user clicked finish.", () => { - const objYeomanui: any = { - generatorName: "generator-name", - promptCount: 1, - gen: { - prompts: { - items: [ - { - name: "step1", - }, - { - name: "step2", - }, - ], - }, - }, - }; - set(panel, "yeomanui", objYeomanui); - const webviewPanel = { - dispose: () => {}, - }; set(webviewPanel, Constants.GENERATOR_COMPLETED, true); - set(panel, "webViewPanel", webviewPanel); - set(panel, "disposables", []); - set(panel, "cleanFlowPromise", () => {}); - - commandsMock.expects("executeCommand").withExactArgs("setContext", "yeomanUI.Focused", false).resolves(); - trackerWrapperMock - .expects("updateGeneratorClosedManually") - .never() - .resolves(); + trackerWrapperMock.expects("updateGeneratorClosedManually").never().resolves(); panel["dispose"](); }); }); From 7b86e8ad3c31fcc4d1d1acc77ee950225e80f72b Mon Sep 17 00:00:00 2001 From: Vered Constantin1 Date: Mon, 18 Mar 2024 14:07:34 +0200 Subject: [PATCH 7/7] fix: cr fix --- packages/backend/src/panels/AbstractWebviewPanel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/panels/AbstractWebviewPanel.ts b/packages/backend/src/panels/AbstractWebviewPanel.ts index 9feb66766..48768ab90 100644 --- a/packages/backend/src/panels/AbstractWebviewPanel.ts +++ b/packages/backend/src/panels/AbstractWebviewPanel.ts @@ -119,7 +119,7 @@ export abstract class AbstractWebviewPanel { const numOfPromopts = promptItems.length; const wizardStepName = promptItems[currentPromptCount - 1].name; AnalyticsWrapper.updateGeneratorClosedManually( - yeomanui.generatorName, + yeomanui.generatorName ?? "", wizardStepName, currentPromptCount, numOfPromopts,