From 8ab8ed31078885da8c303930c70bdf1be97ec65e Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Mon, 3 Dec 2018 17:09:48 +0200 Subject: [PATCH] fix: project properties are not tracked on every analytics hit Currently projectType and isShared properties are tracked only when we send events to Google Analytics and when the caller sends the projectDir to the method. In fact we need to track them(based on the projectDir) for every hit in Google Analytics. To achieve this, use the projectHelper, which tries to determine the projectDir. Track the projectType on every page and event send to Google Analytics. --- .../test/unit-tests/analytics-service.ts | 3 + lib/services/analytics/analytics-service.ts | 47 ++++++++---- test/services/analytics/analytics-service.ts | 74 +++++++++++++++---- test/stubs.ts | 11 ++- 4 files changed, 107 insertions(+), 28 deletions(-) diff --git a/lib/common/test/unit-tests/analytics-service.ts b/lib/common/test/unit-tests/analytics-service.ts index 75b544aacc..356c6d3e53 100644 --- a/lib/common/test/unit-tests/analytics-service.ts +++ b/lib/common/test/unit-tests/analytics-service.ts @@ -96,6 +96,9 @@ function createTestInjector(testScenario: ITestScenario): IInjector { testInjector.register("childProcess", {}); testInjector.register("projectDataService", {}); testInjector.register("mobileHelper", {}); + testInjector.register("projectHelper", { + projectDir: "" + }); return testInjector; } diff --git a/lib/services/analytics/analytics-service.ts b/lib/services/analytics/analytics-service.ts index 153546de0d..82e4fb1da6 100644 --- a/lib/services/analytics/analytics-service.ts +++ b/lib/services/analytics/analytics-service.ts @@ -19,7 +19,8 @@ export class AnalyticsService implements IAnalyticsService, IDisposable { private $analyticsSettingsService: IAnalyticsSettingsService, private $childProcess: IChildProcess, private $projectDataService: IProjectDataService, - private $mobileHelper: Mobile.IMobileHelper) { + private $mobileHelper: Mobile.IMobileHelper, + private $projectHelper: IProjectHelper) { } public setShouldDispose(shouldDispose: boolean): void { @@ -81,12 +82,7 @@ export class AnalyticsService implements IAnalyticsService, IDisposable { await this.initAnalyticsStatuses(); if (!this.$staticConfig.disableAnalytics && this.analyticsStatuses[this.$staticConfig.TRACK_FEATURE_USAGE_SETTING_NAME] === AnalyticsStatus.enabled) { - gaSettings.customDimensions = gaSettings.customDimensions || {}; - gaSettings.customDimensions[GoogleAnalyticsCustomDimensions.client] = this.$options.analyticsClient || (isInteractive() ? AnalyticsClients.Cli : AnalyticsClients.Unknown); - - const googleAnalyticsData: IGoogleAnalyticsTrackingInformation = _.merge({ type: TrackingTypes.GoogleAnalyticsData, category: AnalyticsClients.Cli }, gaSettings); - this.$logger.trace("Will send the following information to Google Analytics:", googleAnalyticsData); - return this.sendMessageToBroker(googleAnalyticsData); + return this.forcefullyTrackInGoogleAnalytics(gaSettings); } } @@ -115,12 +111,7 @@ export class AnalyticsService implements IAnalyticsService, IDisposable { } const customDimensions: IStringDictionary = {}; - if (data.projectDir) { - const projectData = this.$projectDataService.getProjectData(data.projectDir); - customDimensions[GoogleAnalyticsCustomDimensions.projectType] = projectData.projectType; - const isShared = projectData.nsConfig ? (projectData.nsConfig.shared || false) : false; - customDimensions[GoogleAnalyticsCustomDimensions.isShared] = isShared.toString(); - } + this.setProjectRelatedCustomDimensions(customDimensions, data.projectDir); const googleAnalyticsEventData: IGoogleAnalyticsEventData = { googleAnalyticsDataType: GoogleAnalyticsDataType.Event, @@ -132,6 +123,36 @@ export class AnalyticsService implements IAnalyticsService, IDisposable { await this.trackInGoogleAnalytics(googleAnalyticsEventData); } + private forcefullyTrackInGoogleAnalytics(gaSettings: IGoogleAnalyticsData): Promise { + gaSettings.customDimensions = gaSettings.customDimensions || {}; + gaSettings.customDimensions[GoogleAnalyticsCustomDimensions.client] = this.$options.analyticsClient || (isInteractive() ? AnalyticsClients.Cli : AnalyticsClients.Unknown); + this.setProjectRelatedCustomDimensions(gaSettings.customDimensions); + + const googleAnalyticsData: IGoogleAnalyticsTrackingInformation = _.merge({ type: TrackingTypes.GoogleAnalyticsData, category: AnalyticsClients.Cli }, gaSettings); + this.$logger.trace("Will send the following information to Google Analytics:", googleAnalyticsData); + return this.sendMessageToBroker(googleAnalyticsData); + } + + private setProjectRelatedCustomDimensions(customDimensions: IStringDictionary, projectDir?: string): IStringDictionary { + if (!projectDir) { + try { + projectDir = this.$projectHelper.projectDir; + } catch (err) { + // In case there's no project dir here, the above getter will fail. + this.$logger.trace("Unable to get the projectDir from projectHelper", err); + } + } + + if (projectDir) { + const projectData = this.$projectDataService.getProjectData(projectDir); + customDimensions[GoogleAnalyticsCustomDimensions.projectType] = projectData.projectType; + const isShared = !!(projectData.nsConfig && projectData.nsConfig.shared); + customDimensions[GoogleAnalyticsCustomDimensions.isShared] = isShared.toString(); + } + + return customDimensions; + } + public dispose(): void { if (this.brokerProcess && this.shouldDisposeInstance) { this.brokerProcess.disconnect(); diff --git a/test/services/analytics/analytics-service.ts b/test/services/analytics/analytics-service.ts index eb3ae50f08..fccf54535e 100644 --- a/test/services/analytics/analytics-service.ts +++ b/test/services/analytics/analytics-service.ts @@ -9,7 +9,9 @@ const helpers = require("../../../lib/common/helpers"); const originalIsInteractive = helpers.isInteractive; const trackFeatureUsage = "TrackFeatureUsage"; -const createTestInjector = (): IInjector => { +const sampleProjectType = "SampleProjectType"; + +const createTestInjector = (opts?: { projectHelperErrorMsg?: string, projectDir?: string }): IInjector => { const testInjector = new Yok(); testInjector.register("options", {}); testInjector.register("logger", stubs.LoggerStub); @@ -37,8 +39,15 @@ const createTestInjector = (): IInjector => { testInjector.register("processService", { attachToProcessExitSignals: (context: any, callback: () => void): void => undefined }); - testInjector.register("projectDataService", {}); + testInjector.register("projectDataService", { + getProjectData: (projectDir?: string): IProjectData => { + return { + projectType: sampleProjectType + }; + } + }); testInjector.register("mobileHelper", {}); + testInjector.register("projectHelper", new stubs.ProjectHelperStub(opts && opts.projectHelperErrorMsg, opts && opts.projectDir)); return testInjector; }; @@ -133,11 +142,11 @@ describe("analyticsService", () => { assert.isTrue(opts.isChildProcessSpawned); const logger = testInjector.resolve("logger"); - assert.isTrue(logger.traceOutput.indexOf(opts.expectedErrorMessage) !== -1); + assert.isTrue(logger.traceOutput.indexOf(opts.expectedErrorMessage) !== -1, `Tried to find error '${opts.expectedErrorMessage}', but couldn't. logger's trace output is: ${logger.traceOutput}`); }; - const setupTest = (expectedErrorMessage: string): any => { - const testInjector = createTestInjector(); + const setupTest = (expectedErrorMessage: string, projectHelperErrorMsg?: string): any => { + const testInjector = createTestInjector({ projectHelperErrorMsg }); const opts = { isChildProcessSpawned: false, expectedErrorMessage @@ -209,13 +218,33 @@ describe("analyticsService", () => { await assertExpectedError(testInjector, opts); }); + + it("when trying to get projectDir from projectHelper fails", async () => { + const projectHelperErrorMsg = "Failed to find project directory."; + const { testInjector, childProcess, opts } = setupTest(`Unable to get the projectDir from projectHelper Error: ${projectHelperErrorMsg}`, projectHelperErrorMsg); + childProcess.spawn = (command: string, args?: string[], options?: any): any => { + opts.isChildProcessSpawned = true; + const spawnedProcess: any = getSpawnedProcess(); + + spawnedProcess.connected = true; + spawnedProcess.send = (msg: any, action: () => void): void => { + opts.messageSent = msg; + action(); + }; + + setTimeout(() => spawnedProcess.emit("message", AnalyticsMessages.BrokerReadyToReceive), 1); + return spawnedProcess; + }; + + await assertExpectedError(testInjector, opts); + }); }); describe("sends correct message to broker", () => { - const setupTest = (expectedResult: any, dataToSend: any, terminalOpts?: { isInteractive: boolean }): { testInjector: IInjector, opts: any } => { + const setupTest = (expectedResult: any, dataToSend: any, terminalOpts?: { isInteractive: boolean }, projectHelperOpts?: { projectDir: string }): { testInjector: IInjector, opts: any } => { helpers.isInteractive = () => terminalOpts ? terminalOpts.isInteractive : true; - const testInjector = createTestInjector(); + const testInjector = createTestInjector(projectHelperOpts); const opts = { isChildProcessSpawned: false, expectedResult, @@ -260,12 +289,21 @@ describe("analyticsService", () => { } }); - const getExpectedResult = (gaDataType: string, analyticsClient?: string): any => ({ - type: "googleAnalyticsData", - category: "CLI", - googleAnalyticsDataType: gaDataType, - customDimensions: { customDimension1: "value1", cd5: analyticsClient || "CLI" } - }); + const getExpectedResult = (gaDataType: string, analyticsClient?: string, projectType?: string): any => { + const expectedResult: any = { + type: "googleAnalyticsData", + category: "CLI", + googleAnalyticsDataType: gaDataType, + customDimensions: { customDimension1: "value1", cd5: analyticsClient || "CLI" } + }; + + if (projectType) { + expectedResult.customDimensions[GoogleAnalyticsCustomDimensions.projectType] = projectType; + expectedResult.customDimensions[GoogleAnalyticsCustomDimensions.isShared] = false.toString(); + } + + return expectedResult; + }; _.each([GoogleAnalyticsDataType.Page, GoogleAnalyticsDataType.Event], (googleAnalyticsDataType: string) => { it(`when data is ${googleAnalyticsDataType}`, async () => { @@ -273,6 +311,16 @@ describe("analyticsService", () => { await assertExpectedResult(testInjector, opts); }); + it(`when data is ${googleAnalyticsDataType} and command is executed from projectDir`, async () => { + const { testInjector, opts } = setupTest( + getExpectedResult(googleAnalyticsDataType, null, sampleProjectType), + getDataToSend(googleAnalyticsDataType), + null, + { projectDir: "/some-dir" } + ); + await assertExpectedResult(testInjector, opts); + }); + it(`when data is ${googleAnalyticsDataType} and terminal is not interactive`, async () => { const { testInjector, opts } = setupTest(getExpectedResult(googleAnalyticsDataType, AnalyticsClients.Unknown), getDataToSend(googleAnalyticsDataType), { isInteractive: false }); await assertExpectedResult(testInjector, opts); diff --git a/test/stubs.ts b/test/stubs.ts index be32216147..ba4753506c 100644 --- a/test/stubs.ts +++ b/test/stubs.ts @@ -519,8 +519,15 @@ export class ProjectDataService implements IProjectDataService { } export class ProjectHelperStub implements IProjectHelper { - get projectDir(): string { - return ""; + constructor(public projectHelperErrorMsg?: string, public customProjectDir?: string) { + } + + public get projectDir(): string { + if (this.projectHelperErrorMsg) { + throw new Error(this.projectHelperErrorMsg); + } + + return this.customProjectDir || ""; } generateDefaultAppId(appName: string, baseAppId: string): string {