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.

Author: rosen-vladimirov

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;
 }

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<void> {
+		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();

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 <any>{
+				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<stubs.LoggerStub>("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,19 +289,38 @@ 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 () => {
 					const { testInjector, opts } = setupTest(getExpectedResult(googleAnalyticsDataType), getDataToSend(googleAnalyticsDataType));
 					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);

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 {