Skip to content

fix(GDPR): Do not track local project paths #3592

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.

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

Merged
merged 3 commits into from
May 17, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
12 changes: 12 additions & 0 deletions lib/constants.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
require("colors");

export const APP_FOLDER_NAME = "app";
export const APP_RESOURCES_FOLDER_NAME = "App_Resources";
export const PROJECT_FRAMEWORK_FOLDER_NAME = "framework";
Expand Down Expand Up @@ -78,6 +80,8 @@ export const RESERVED_TEMPLATE_NAMES: IStringDictionary = {
"angular": "tns-template-hello-world-ng"
};

export const ANALYTICS_LOCAL_TEMPLATE_PREFIX = "localTemplate_";

export class ITMSConstants {
static ApplicationMetadataFile = "metadata.xml";
static VerboseLoggingLevels = {
Expand Down Expand Up @@ -170,3 +174,11 @@ export class AssetConstants {
public static defaultScale = 1;
public static defaultOverlayImageScale = 0.8;
}

export const PROGRESS_PRIVACY_POLICY_URL = "https://www.progress.com/legal/privacy-policy";
export class SubscribeForNewsletterMessages {
public static AgreeToReceiveEmailMsg = "I agree to receive email communications from Progress Software or its Partners (`https://www.progress.com/partners/partner-directory`)," +
"containing information about Progress Software's products. Consent may be withdrawn at any time.";
public static ReviewPrivacyPolicyMsg = `You can review the Progress Software Privacy Policy at \`${PROGRESS_PRIVACY_POLICY_URL}\``;
public static PromptMsg = "Input your e-mail address to agree".green + " or " + "leave empty to decline".red.bold + ":";
}
22 changes: 19 additions & 3 deletions lib/services/project-templates-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,16 @@ export class ProjectTemplatesService implements IProjectTemplatesService {

const templateName = constants.RESERVED_TEMPLATE_NAMES[name.toLowerCase()] || name;

const realTemplatePath = await this.prepareNativeScriptTemplate(templateName, version, projectDir);

await this.$analyticsService.track("Template used for project creation", templateName);

await this.$analyticsService.trackEventActionInGoogleAnalytics({
action: constants.TrackActionNames.CreateProject,
isForDevice: null,
additionalData: templateName
additionalData: this.getTemplateNameToBeTracked(templateName, realTemplatePath)
});

const realTemplatePath = await this.prepareNativeScriptTemplate(templateName, version, projectDir);

// this removes dependencies from templates so they are not copied to app folder
this.$fs.deleteDirectory(path.join(realTemplatePath, constants.NODE_MODULES_FOLDER_NAME));

Expand All @@ -46,5 +46,21 @@ export class ProjectTemplatesService implements IProjectTemplatesService {
this.$logger.trace(`Using NativeScript verified template: ${templateName} with version ${version}.`);
return this.$npmInstallationManager.install(templateName, projectDir, { version: version, dependencyType: "save" });
}

private getTemplateNameToBeTracked(templateName: string, realTemplatePath: string): string {
Copy link
Contributor

@Fatme Fatme May 17, 2018

Choose a reason for hiding this comment

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

Not sure, but maybe the whole code should be in try { } catch { }. In case something fails here we don't want to stop the command execution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice idea, I've added the try-catch and test for this case.

if (this.$fs.exists(templateName)) {
// local template is used
const pathToPackageJson = path.join(realTemplatePath, constants.PACKAGE_JSON_FILE_NAME);
let templateNameToTrack = path.basename(templateName);
if (this.$fs.exists(pathToPackageJson)) {
const templatePackageJsonContent = this.$fs.readJson(pathToPackageJson);
templateNameToTrack = templatePackageJsonContent.name;
}

return `${constants.ANALYTICS_LOCAL_TEMPLATE_PREFIX}${templateNameToTrack}`;
}

return templateName;
}
}
$injector.register("projectTemplatesService", ProjectTemplatesService);
7 changes: 5 additions & 2 deletions lib/services/subscription-service.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as emailValidator from "email-validator";
import * as queryString from "querystring";
import * as helpers from "../common/helpers";
import { SubscribeForNewsletterMessages } from "../constants";

export class SubscriptionService implements ISubscriptionService {
constructor(private $httpClient: Server.IHttpClient,
Expand All @@ -11,8 +12,10 @@ export class SubscriptionService implements ISubscriptionService {

public async subscribeForNewsletter(): Promise<void> {
if (await this.shouldAskForEmail()) {
this.$logger.out("Enter your e-mail address to subscribe to the NativeScript Newsletter and hear about product updates, tips & tricks, and community happenings:");
const email = await this.getEmail("(press Enter for blank)");
this.$logger.printMarkdown(SubscribeForNewsletterMessages.AgreeToReceiveEmailMsg);
this.$logger.printMarkdown(SubscribeForNewsletterMessages.ReviewPrivacyPolicyMsg);

const email = await this.getEmail(SubscribeForNewsletterMessages.PromptMsg);
await this.$userSettingsService.saveSetting("EMAIL_REGISTERED", true);
await this.sendEmail(email);
}
Expand Down
2 changes: 1 addition & 1 deletion npm-shrinkwrap.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "nativescript",
"preferGlobal": true,
"version": "4.0.1",
"version": "4.0.2",
"author": "Telerik <[email protected]>",
"description": "Command-line interface for building NativeScript projects",
"bin": {
Expand Down
69 changes: 59 additions & 10 deletions test/project-templates-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import * as stubs from "./stubs";
import { ProjectTemplatesService } from "../lib/services/project-templates-service";
import { assert } from "chai";
import * as path from "path";
import temp = require("temp");
import * as constants from "../lib/constants";

let isDeleteDirectoryCalledForNodeModulesDir = false;
Expand All @@ -25,9 +24,12 @@ function createTestInjector(configuration?: { shouldNpmInstallThrow: boolean, np
if (directory.indexOf("node_modules") !== -1) {
isDeleteDirectoryCalledForNodeModulesDir = true;
}
}
},

exists: (filePath: string): boolean => false

});

injector.register("npm", {
install: (packageName: string, pathToSave: string, config?: any) => {
if (configuration.shouldNpmInstallThrow) {
Expand Down Expand Up @@ -70,38 +72,85 @@ describe("project-templates-service", () => {
it("when npm install fails", async () => {
testInjector = createTestInjector({ shouldNpmInstallThrow: true, npmInstallationDirContents: [], npmInstallationDirNodeModulesContents: null });
projectTemplatesService = testInjector.resolve("projectTemplatesService");
const tempFolder = temp.mkdirSync("preparetemplate");
await assert.isRejected(projectTemplatesService.prepareTemplate("invalidName", tempFolder));
await assert.isRejected(projectTemplatesService.prepareTemplate("invalidName", "tempFolder"));
});
});

describe("returns correct path to template", () => {
it("when reserved template name is used", async () => {
testInjector = createTestInjector({ shouldNpmInstallThrow: false, npmInstallationDirContents: [], npmInstallationDirNodeModulesContents: [] });
projectTemplatesService = testInjector.resolve("projectTemplatesService");
const tempFolder = temp.mkdirSync("preparetemplate");
const actualPathToTemplate = await projectTemplatesService.prepareTemplate("typescript", tempFolder);
const actualPathToTemplate = await projectTemplatesService.prepareTemplate("typescript", "tempFolder");
assert.strictEqual(path.basename(actualPathToTemplate), nativeScriptValidatedTemplatePath);
assert.strictEqual(isDeleteDirectoryCalledForNodeModulesDir, true, "When correct path is returned, template's node_modules directory should be deleted.");
});

it("when reserved template name is used (case-insensitive test)", async () => {
testInjector = createTestInjector({ shouldNpmInstallThrow: false, npmInstallationDirContents: [], npmInstallationDirNodeModulesContents: [] });
projectTemplatesService = testInjector.resolve("projectTemplatesService");
const tempFolder = temp.mkdirSync("preparetemplate");
const actualPathToTemplate = await projectTemplatesService.prepareTemplate("tYpEsCriPT", tempFolder);
const actualPathToTemplate = await projectTemplatesService.prepareTemplate("tYpEsCriPT", "tempFolder");
assert.strictEqual(path.basename(actualPathToTemplate), nativeScriptValidatedTemplatePath);
assert.strictEqual(isDeleteDirectoryCalledForNodeModulesDir, true, "When correct path is returned, template's node_modules directory should be deleted.");
});

it("uses defaultTemplate when undefined is passed as parameter", async () => {
testInjector = createTestInjector({ shouldNpmInstallThrow: false, npmInstallationDirContents: [], npmInstallationDirNodeModulesContents: [] });
projectTemplatesService = testInjector.resolve("projectTemplatesService");
const tempFolder = temp.mkdirSync("preparetemplate");
const actualPathToTemplate = await projectTemplatesService.prepareTemplate(constants.RESERVED_TEMPLATE_NAMES["default"], tempFolder);
const actualPathToTemplate = await projectTemplatesService.prepareTemplate(constants.RESERVED_TEMPLATE_NAMES["default"], "tempFolder");
assert.strictEqual(path.basename(actualPathToTemplate), nativeScriptValidatedTemplatePath);
assert.strictEqual(isDeleteDirectoryCalledForNodeModulesDir, true, "When correct path is returned, template's node_modules directory should be deleted.");
});
});

describe("sends correct information to Google Analytics", () => {
let analyticsService: IAnalyticsService;
let dataSentToGoogleAnalytics: IEventActionData;
beforeEach(() => {
testInjector = createTestInjector({ shouldNpmInstallThrow: false, npmInstallationDirContents: [], npmInstallationDirNodeModulesContents: [] });
analyticsService = testInjector.resolve<IAnalyticsService>("analyticsService");
dataSentToGoogleAnalytics = null;
analyticsService.trackEventActionInGoogleAnalytics = async (data: IEventActionData): Promise<void> => {
dataSentToGoogleAnalytics = data;
};
projectTemplatesService = testInjector.resolve("projectTemplatesService");
});

it("sends template name when the template is used from npm", async () => {
const templateName = "template-from-npm";
await projectTemplatesService.prepareTemplate(templateName, "tempFolder");
assert.deepEqual(dataSentToGoogleAnalytics, {
action: constants.TrackActionNames.CreateProject,
isForDevice: null,
additionalData: templateName
});
});

it("sends template name (from template's package.json) when the template is used from local path", async () => {
const templateName = "my-own-local-template";
const localTemplatePath = "/Users/username/localtemplate";
const fs = testInjector.resolve<IFileSystem>("fs");
fs.exists = (path: string): boolean => true;
fs.readJson = (filename: string, encoding?: string): any => ({ name: templateName });
await projectTemplatesService.prepareTemplate(localTemplatePath, "tempFolder");
assert.deepEqual(dataSentToGoogleAnalytics, {
action: constants.TrackActionNames.CreateProject,
isForDevice: null,
additionalData: `${constants.ANALYTICS_LOCAL_TEMPLATE_PREFIX}${templateName}`
});
});

it("sends the template name (path to dirname) when the template is used from local path but there's no package.json at the root", async () => {
const templateName = "localtemplate";
const localTemplatePath = `/Users/username/${templateName}`;
const fs = testInjector.resolve<IFileSystem>("fs");
fs.exists = (localPath: string): boolean => path.basename(localPath) !== constants.PACKAGE_JSON_FILE_NAME;
await projectTemplatesService.prepareTemplate(localTemplatePath, "tempFolder");
assert.deepEqual(dataSentToGoogleAnalytics, {
action: constants.TrackActionNames.CreateProject,
isForDevice: null,
additionalData: `${constants.ANALYTICS_LOCAL_TEMPLATE_PREFIX}${templateName}`
});
});
});
});
});
9 changes: 7 additions & 2 deletions test/services/subscription-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { assert } from "chai";
import { SubscriptionService } from "../../lib/services/subscription-service";
import { LoggerStub } from "../stubs";
import { stringify } from "querystring";
import { SubscribeForNewsletterMessages } from "../../lib/constants";
const helpers = require("../../lib/common/helpers");

interface IValidateTestData {
Expand Down Expand Up @@ -153,12 +154,16 @@ describe("subscriptionService", () => {
loggerOutput += args.join(" ");
};

logger.printMarkdown = (message: string): void => {
loggerOutput += message;
};

await subscriptionService.subscribeForNewsletter();

assert.equal(loggerOutput, "Enter your e-mail address to subscribe to the NativeScript Newsletter and hear about product updates, tips & tricks, and community happenings:");
assert.equal(loggerOutput, `${SubscribeForNewsletterMessages.AgreeToReceiveEmailMsg}${SubscribeForNewsletterMessages.ReviewPrivacyPolicyMsg}`);
});

const expectedMessageInPrompter = "(press Enter for blank)";
const expectedMessageInPrompter = SubscribeForNewsletterMessages.PromptMsg;
it(`calls prompter with specific message - ${expectedMessageInPrompter}`, async () => {
const testInjector = createTestInjector();
const subscriptionService = testInjector.resolve<SubscriptionServiceTester>(SubscriptionServiceTester);
Expand Down