Skip to content

Commit 51fc9c5

Browse files
rosen-vladimirovrosen-vladimirov
authored
Merge pull request #4194 from NativeScript/vladimirov/analytics-project-dir
fix: project properties are not tracked on every analytics hit
2 parents e8e126a + 8ab8ed3 commit 51fc9c5

File tree

4 files changed

+107
-28
lines changed

4 files changed

+107
-28
lines changed

‎lib/common/test/unit-tests/analytics-service.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,9 @@ function createTestInjector(testScenario: ITestScenario): IInjector {
9696
testInjector.register("childProcess", {});
9797
testInjector.register("projectDataService", {});
9898
testInjector.register("mobileHelper", {});
99+
testInjector.register("projectHelper", {
100+
projectDir: ""
101+
});
99102

100103
return testInjector;
101104
}

‎lib/services/analytics/analytics-service.ts

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ export class AnalyticsService implements IAnalyticsService, IDisposable {
1919
private $analyticsSettingsService: IAnalyticsSettingsService,
2020
private $childProcess: IChildProcess,
2121
private $projectDataService: IProjectDataService,
22-
private $mobileHelper: Mobile.IMobileHelper) {
22+
private $mobileHelper: Mobile.IMobileHelper,
23+
private $projectHelper: IProjectHelper) {
2324
}
2425

2526
public setShouldDispose(shouldDispose: boolean): void {
@@ -81,12 +82,7 @@ export class AnalyticsService implements IAnalyticsService, IDisposable {
8182
await this.initAnalyticsStatuses();
8283

8384
if (!this.$staticConfig.disableAnalytics && this.analyticsStatuses[this.$staticConfig.TRACK_FEATURE_USAGE_SETTING_NAME] === AnalyticsStatus.enabled) {
84-
gaSettings.customDimensions = gaSettings.customDimensions || {};
85-
gaSettings.customDimensions[GoogleAnalyticsCustomDimensions.client] = this.$options.analyticsClient || (isInteractive() ? AnalyticsClients.Cli : AnalyticsClients.Unknown);
86-
87-
const googleAnalyticsData: IGoogleAnalyticsTrackingInformation = _.merge({ type: TrackingTypes.GoogleAnalyticsData, category: AnalyticsClients.Cli }, gaSettings);
88-
this.$logger.trace("Will send the following information to Google Analytics:", googleAnalyticsData);
89-
return this.sendMessageToBroker(googleAnalyticsData);
85+
return this.forcefullyTrackInGoogleAnalytics(gaSettings);
9086
}
9187
}
9288

@@ -115,12 +111,7 @@ export class AnalyticsService implements IAnalyticsService, IDisposable {
115111
}
116112

117113
const customDimensions: IStringDictionary = {};
118-
if (data.projectDir) {
119-
const projectData = this.$projectDataService.getProjectData(data.projectDir);
120-
customDimensions[GoogleAnalyticsCustomDimensions.projectType] = projectData.projectType;
121-
const isShared = projectData.nsConfig ? (projectData.nsConfig.shared || false) : false;
122-
customDimensions[GoogleAnalyticsCustomDimensions.isShared] = isShared.toString();
123-
}
114+
this.setProjectRelatedCustomDimensions(customDimensions, data.projectDir);
124115

125116
const googleAnalyticsEventData: IGoogleAnalyticsEventData = {
126117
googleAnalyticsDataType: GoogleAnalyticsDataType.Event,
@@ -132,6 +123,36 @@ export class AnalyticsService implements IAnalyticsService, IDisposable {
132123
await this.trackInGoogleAnalytics(googleAnalyticsEventData);
133124
}
134125

126+
private forcefullyTrackInGoogleAnalytics(gaSettings: IGoogleAnalyticsData): Promise<void> {
127+
gaSettings.customDimensions = gaSettings.customDimensions || {};
128+
gaSettings.customDimensions[GoogleAnalyticsCustomDimensions.client] = this.$options.analyticsClient || (isInteractive() ? AnalyticsClients.Cli : AnalyticsClients.Unknown);
129+
this.setProjectRelatedCustomDimensions(gaSettings.customDimensions);
130+
131+
const googleAnalyticsData: IGoogleAnalyticsTrackingInformation = _.merge({ type: TrackingTypes.GoogleAnalyticsData, category: AnalyticsClients.Cli }, gaSettings);
132+
this.$logger.trace("Will send the following information to Google Analytics:", googleAnalyticsData);
133+
return this.sendMessageToBroker(googleAnalyticsData);
134+
}
135+
136+
private setProjectRelatedCustomDimensions(customDimensions: IStringDictionary, projectDir?: string): IStringDictionary {
137+
if (!projectDir) {
138+
try {
139+
projectDir = this.$projectHelper.projectDir;
140+
} catch (err) {
141+
// In case there's no project dir here, the above getter will fail.
142+
this.$logger.trace("Unable to get the projectDir from projectHelper", err);
143+
}
144+
}
145+
146+
if (projectDir) {
147+
const projectData = this.$projectDataService.getProjectData(projectDir);
148+
customDimensions[GoogleAnalyticsCustomDimensions.projectType] = projectData.projectType;
149+
const isShared = !!(projectData.nsConfig && projectData.nsConfig.shared);
150+
customDimensions[GoogleAnalyticsCustomDimensions.isShared] = isShared.toString();
151+
}
152+
153+
return customDimensions;
154+
}
155+
135156
public dispose(): void {
136157
if (this.brokerProcess && this.shouldDisposeInstance) {
137158
this.brokerProcess.disconnect();

‎test/services/analytics/analytics-service.ts

Lines changed: 61 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ const helpers = require("../../../lib/common/helpers");
99
const originalIsInteractive = helpers.isInteractive;
1010

1111
const trackFeatureUsage = "TrackFeatureUsage";
12-
const createTestInjector = (): IInjector => {
12+
const sampleProjectType = "SampleProjectType";
13+
14+
const createTestInjector = (opts?: { projectHelperErrorMsg?: string, projectDir?: string }): IInjector => {
1315
const testInjector = new Yok();
1416
testInjector.register("options", {});
1517
testInjector.register("logger", stubs.LoggerStub);
@@ -37,8 +39,15 @@ const createTestInjector = (): IInjector => {
3739
testInjector.register("processService", {
3840
attachToProcessExitSignals: (context: any, callback: () => void): void => undefined
3941
});
40-
testInjector.register("projectDataService", {});
42+
testInjector.register("projectDataService", {
43+
getProjectData: (projectDir?: string): IProjectData => {
44+
return <any>{
45+
projectType: sampleProjectType
46+
};
47+
}
48+
});
4149
testInjector.register("mobileHelper", {});
50+
testInjector.register("projectHelper", new stubs.ProjectHelperStub(opts && opts.projectHelperErrorMsg, opts && opts.projectDir));
4251

4352
return testInjector;
4453
};
@@ -133,11 +142,11 @@ describe("analyticsService", () => {
133142

134143
assert.isTrue(opts.isChildProcessSpawned);
135144
const logger = testInjector.resolve<stubs.LoggerStub>("logger");
136-
assert.isTrue(logger.traceOutput.indexOf(opts.expectedErrorMessage) !== -1);
145+
assert.isTrue(logger.traceOutput.indexOf(opts.expectedErrorMessage) !== -1, `Tried to find error '${opts.expectedErrorMessage}', but couldn't. logger's trace output is: ${logger.traceOutput}`);
137146
};
138147

139-
const setupTest = (expectedErrorMessage: string): any => {
140-
const testInjector = createTestInjector();
148+
const setupTest = (expectedErrorMessage: string, projectHelperErrorMsg?: string): any => {
149+
const testInjector = createTestInjector({ projectHelperErrorMsg });
141150
const opts = {
142151
isChildProcessSpawned: false,
143152
expectedErrorMessage
@@ -209,13 +218,33 @@ describe("analyticsService", () => {
209218

210219
await assertExpectedError(testInjector, opts);
211220
});
221+
222+
it("when trying to get projectDir from projectHelper fails", async () => {
223+
const projectHelperErrorMsg = "Failed to find project directory.";
224+
const { testInjector, childProcess, opts } = setupTest(`Unable to get the projectDir from projectHelper Error: ${projectHelperErrorMsg}`, projectHelperErrorMsg);
225+
childProcess.spawn = (command: string, args?: string[], options?: any): any => {
226+
opts.isChildProcessSpawned = true;
227+
const spawnedProcess: any = getSpawnedProcess();
228+
229+
spawnedProcess.connected = true;
230+
spawnedProcess.send = (msg: any, action: () => void): void => {
231+
opts.messageSent = msg;
232+
action();
233+
};
234+
235+
setTimeout(() => spawnedProcess.emit("message", AnalyticsMessages.BrokerReadyToReceive), 1);
236+
return spawnedProcess;
237+
};
238+
239+
await assertExpectedError(testInjector, opts);
240+
});
212241
});
213242

214243
describe("sends correct message to broker", () => {
215-
const setupTest = (expectedResult: any, dataToSend: any, terminalOpts?: { isInteractive: boolean }): { testInjector: IInjector, opts: any } => {
244+
const setupTest = (expectedResult: any, dataToSend: any, terminalOpts?: { isInteractive: boolean }, projectHelperOpts?: { projectDir: string }): { testInjector: IInjector, opts: any } => {
216245
helpers.isInteractive = () => terminalOpts ? terminalOpts.isInteractive : true;
217246

218-
const testInjector = createTestInjector();
247+
const testInjector = createTestInjector(projectHelperOpts);
219248
const opts = {
220249
isChildProcessSpawned: false,
221250
expectedResult,
@@ -260,19 +289,38 @@ describe("analyticsService", () => {
260289
}
261290
});
262291

263-
const getExpectedResult = (gaDataType: string, analyticsClient?: string): any => ({
264-
type: "googleAnalyticsData",
265-
category: "CLI",
266-
googleAnalyticsDataType: gaDataType,
267-
customDimensions: { customDimension1: "value1", cd5: analyticsClient || "CLI" }
268-
});
292+
const getExpectedResult = (gaDataType: string, analyticsClient?: string, projectType?: string): any => {
293+
const expectedResult: any = {
294+
type: "googleAnalyticsData",
295+
category: "CLI",
296+
googleAnalyticsDataType: gaDataType,
297+
customDimensions: { customDimension1: "value1", cd5: analyticsClient || "CLI" }
298+
};
299+
300+
if (projectType) {
301+
expectedResult.customDimensions[GoogleAnalyticsCustomDimensions.projectType] = projectType;
302+
expectedResult.customDimensions[GoogleAnalyticsCustomDimensions.isShared] = false.toString();
303+
}
304+
305+
return expectedResult;
306+
};
269307

270308
_.each([GoogleAnalyticsDataType.Page, GoogleAnalyticsDataType.Event], (googleAnalyticsDataType: string) => {
271309
it(`when data is ${googleAnalyticsDataType}`, async () => {
272310
const { testInjector, opts } = setupTest(getExpectedResult(googleAnalyticsDataType), getDataToSend(googleAnalyticsDataType));
273311
await assertExpectedResult(testInjector, opts);
274312
});
275313

314+
it(`when data is ${googleAnalyticsDataType} and command is executed from projectDir`, async () => {
315+
const { testInjector, opts } = setupTest(
316+
getExpectedResult(googleAnalyticsDataType, null, sampleProjectType),
317+
getDataToSend(googleAnalyticsDataType),
318+
null,
319+
{ projectDir: "/some-dir" }
320+
);
321+
await assertExpectedResult(testInjector, opts);
322+
});
323+
276324
it(`when data is ${googleAnalyticsDataType} and terminal is not interactive`, async () => {
277325
const { testInjector, opts } = setupTest(getExpectedResult(googleAnalyticsDataType, AnalyticsClients.Unknown), getDataToSend(googleAnalyticsDataType), { isInteractive: false });
278326
await assertExpectedResult(testInjector, opts);

‎test/stubs.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -519,8 +519,15 @@ export class ProjectDataService implements IProjectDataService {
519519
}
520520

521521
export class ProjectHelperStub implements IProjectHelper {
522-
get projectDir(): string {
523-
return "";
522+
constructor(public projectHelperErrorMsg?: string, public customProjectDir?: string) {
523+
}
524+
525+
public get projectDir(): string {
526+
if (this.projectHelperErrorMsg) {
527+
throw new Error(this.projectHelperErrorMsg);
528+
}
529+
530+
return this.customProjectDir || "";
524531
}
525532

526533
generateDefaultAppId(appName: string, baseAppId: string): string {

0 commit comments

Comments
 (0)