Skip to content

Add tracking for the project types that users are creating and working with #2492

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 2 commits into from
Feb 7, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
39 changes: 20 additions & 19 deletions lib/services/project-templates-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,31 +6,32 @@ temp.track();
export class ProjectTemplatesService implements IProjectTemplatesService {

public constructor(private $errors: IErrors,
private $fs: IFileSystem,
private $logger: ILogger,
private $npm: INodePackageManager,
private $npmInstallationManager: INpmInstallationManager) { }
private $fs: IFileSystem,
private $logger: ILogger,
private $npm: INodePackageManager,
private $npmInstallationManager: INpmInstallationManager,
private $analyticsService: IAnalyticsService) { }

public prepareTemplate(originalTemplateName: string, projectDir: string): IFuture<string> {
return ((): string => {
let realTemplatePath: string;
if(originalTemplateName) {
let templateName = originalTemplateName || constants.RESERVED_TEMPLATE_NAMES["default"],
version: string = null;

if (originalTemplateName) {
// support <reserved_name>@<version> syntax
let data = originalTemplateName.split("@"),
name = data[0],
version = data[1];

if(constants.RESERVED_TEMPLATE_NAMES[name.toLowerCase()]) {
realTemplatePath = this.prepareNativeScriptTemplate(constants.RESERVED_TEMPLATE_NAMES[name.toLowerCase()], version, projectDir).wait();
} else {
// Use the original template name, specified by user as it may be case-sensitive.
realTemplatePath = this.prepareNativeScriptTemplate(name, version, projectDir).wait();
}
} else {
realTemplatePath = this.prepareNativeScriptTemplate(constants.RESERVED_TEMPLATE_NAMES["default"], null/*version*/, projectDir).wait();
name = data[0];

version = data[1];

templateName = constants.RESERVED_TEMPLATE_NAMES[name.toLowerCase()] || name;
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing case when template is case sensitive folder.
If the developer uses App as originalTemplateName and then uses app these can be two different apps. I'm OK with us not supporting this scenario, and either way there aren't such tests, but thought you should know.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fact the case is handled - when originalTemplateName is app, the name variable will be set to app.
At this point templateName will be set to app.

At another point, when user passes --template App, originalTemplateName will be set to App, the name variable will be set to App and the templateName will be set to App again.

}

if(realTemplatePath) {
this.$analyticsService.track("Template used for project creation", templateName).wait();

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

if (realTemplatePath) {
//this removes dependencies from templates so they are not copied to app folder
this.$fs.deleteDirectory(path.join(realTemplatePath, constants.NODE_MODULES_FOLDER_NAME));
return realTemplatePath;
Expand All @@ -51,7 +52,7 @@ export class ProjectTemplatesService implements IProjectTemplatesService {
*/
private prepareNativeScriptTemplate(templateName: string, version?: string, projectDir?: string): IFuture<string> {
this.$logger.trace(`Using NativeScript verified template: ${templateName} with version ${version}.`);
return this.$npmInstallationManager.install(templateName, projectDir, {version: version, dependencyType: "save"});
return this.$npmInstallationManager.install(templateName, projectDir, { version: version, dependencyType: "save" });
}
}
$injector.register("projectTemplatesService", ProjectTemplatesService);
20 changes: 11 additions & 9 deletions test/project-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ class ProjectIntegrationTest {
this.testInjector.register("fs", FileSystem);
this.testInjector.register("projectDataService", ProjectDataServiceLib.ProjectDataService);
this.testInjector.register("staticConfig", StaticConfig);
this.testInjector.register("analyticsService", { track: () => Future.fromResult() });

this.testInjector.register("npmInstallationManager", NpmInstallationManager);
this.testInjector.register("npm", NpmLib.NodePackageManager);
Expand All @@ -134,12 +135,12 @@ class ProjectIntegrationTest {

describe("Project Service Tests", () => {
describe("project service integration tests", () => {
let defaultTemplatePath:string;
let defaultSpecificVersionTemplatePath:string;
let angularTemplatePath:string;
let defaultTemplatePath: string;
let defaultSpecificVersionTemplatePath: string;
let angularTemplatePath: string;
let typescriptTemplatePath: string;

before(function() {
before(function () {
let projectIntegrationTest = new ProjectIntegrationTest();
let fs: IFileSystem = projectIntegrationTest.testInjector.resolve("fs");
let npmInstallationManager: INpmInstallationManager = projectIntegrationTest.testInjector.resolve("npmInstallationManager");
Expand All @@ -153,7 +154,7 @@ describe("Project Service Tests", () => {
"readme": "dummy",
"repository": "dummy"
});
npmInstallationManager.install("tns-template-hello-world", defaultTemplateDir, {dependencyType: "save"}).wait();
npmInstallationManager.install("tns-template-hello-world", defaultTemplateDir, { dependencyType: "save" }).wait();
Copy link
Contributor

Choose a reason for hiding this comment

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

can use constants.RESERVED_TEMPLATE_NAMES["default"] instead of "tns-template-hello-world"

defaultTemplatePath = path.join(defaultTemplateDir, "node_modules", "tns-template-hello-world");
fs.deleteDirectory(path.join(defaultTemplatePath, "node_modules"));

Expand All @@ -166,7 +167,7 @@ describe("Project Service Tests", () => {
"readme": "dummy",
"repository": "dummy"
});
npmInstallationManager.install("tns-template-hello-world", defaultSpecificVersionTemplateDir, {version: "1.4.0", dependencyType: "save"}).wait();
npmInstallationManager.install("tns-template-hello-world", defaultSpecificVersionTemplateDir, { version: "1.4.0", dependencyType: "save" }).wait();
Copy link
Contributor

Choose a reason for hiding this comment

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

can use constants.RESERVED_TEMPLATE_NAMES["default"] instead of "tns-template-hello-world"

defaultSpecificVersionTemplatePath = path.join(defaultSpecificVersionTemplateDir, "node_modules", "tns-template-hello-world");
fs.deleteDirectory(path.join(defaultSpecificVersionTemplatePath, "node_modules"));

Expand All @@ -179,7 +180,7 @@ describe("Project Service Tests", () => {
"readme": "dummy",
"repository": "dummy"
});
npmInstallationManager.install("tns-template-hello-world-ng", angularTemplateDir, {dependencyType: "save"}).wait();
npmInstallationManager.install("tns-template-hello-world-ng", angularTemplateDir, { dependencyType: "save" }).wait();
angularTemplatePath = path.join(angularTemplateDir, "node_modules", "tns-template-hello-world-ng");
fs.deleteDirectory(path.join(angularTemplatePath, "node_modules"));

Expand All @@ -192,7 +193,7 @@ describe("Project Service Tests", () => {
"readme": "dummy",
"repository": "dummy"
});
npmInstallationManager.install("tns-template-hello-world-ts", typescriptTemplateDir, {dependencyType: "save"}).wait();
npmInstallationManager.install("tns-template-hello-world-ts", typescriptTemplateDir, { dependencyType: "save" }).wait();
typescriptTemplatePath = path.join(typescriptTemplateDir, "node_modules", "tns-template-hello-world-ts");
fs.deleteDirectory(path.join(typescriptTemplatePath, "node_modules"));
});
Expand Down Expand Up @@ -447,7 +448,7 @@ describe("Project Service Tests", () => {
projectIntegrationTest.createProject(projectName).wait();
options.copyFrom = defaultTemplatePath;

projectIntegrationTest.assertProject(tempFolder, projectName, `org.nativescript.${projectName}`,null).wait();
projectIntegrationTest.assertProject(tempFolder, projectName, `org.nativescript.${projectName}`, null).wait();
});
});

Expand All @@ -468,6 +469,7 @@ function createTestInjector() {
testInjector.register("projectDataService", ProjectDataServiceLib.ProjectDataService);

testInjector.register("staticConfig", StaticConfig);
testInjector.register("analyticsService", { track: () => Future.fromResult() });

testInjector.register("npmInstallationManager", NpmInstallationManager);
testInjector.register("httpClient", HttpClientLib.HttpClient);
Expand Down
32 changes: 17 additions & 15 deletions test/project-templates-service.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {Yok} from "../lib/common/yok";
import { Yok } from "../lib/common/yok";
import * as stubs from "./stubs";
import {ProjectTemplatesService} from "../lib/services/project-templates-service";
import { ProjectTemplatesService } from "../lib/services/project-templates-service";
import * as assert from "assert";
import Future = require("fibers/future");
import * as path from "path";
Expand All @@ -9,28 +9,28 @@ import temp = require("temp");
let isDeleteDirectoryCalledForNodeModulesDir = false;
let nativeScriptValidatedTemplatePath = "nsValidatedTemplatePath";

function createTestInjector(configuration?: {shouldNpmInstallThrow: boolean, npmInstallationDirContents: string[], npmInstallationDirNodeModulesContents: string[]}): IInjector {
function createTestInjector(configuration?: { shouldNpmInstallThrow: boolean, npmInstallationDirContents: string[], npmInstallationDirNodeModulesContents: string[] }): IInjector {
let injector = new Yok();
injector.register("errors", stubs.ErrorsStub);
injector.register("logger", stubs.LoggerStub);
injector.register("fs", {
readDirectory: (dirPath: string): string[] => {
if(dirPath.toLowerCase().indexOf("node_modules") !== -1) {
if (dirPath.toLowerCase().indexOf("node_modules") !== -1) {
return configuration.npmInstallationDirNodeModulesContents;
}
return configuration.npmInstallationDirContents;
},

deleteDirectory: (directory: string) => {
if(directory.indexOf("node_modules") !== -1) {
if (directory.indexOf("node_modules") !== -1) {
isDeleteDirectoryCalledForNodeModulesDir = true;
}
}

});
injector.register("npm", {
install: (packageName: string, pathToSave: string, config?: any) => {
if(configuration.shouldNpmInstallThrow) {
if (configuration.shouldNpmInstallThrow) {
throw new Error("NPM install throws error.");
}

Expand All @@ -40,7 +40,7 @@ function createTestInjector(configuration?: {shouldNpmInstallThrow: boolean, npm

injector.register("npmInstallationManager", {
install: (packageName: string, options?: INpmInstallOptions) => {
if(configuration.shouldNpmInstallThrow) {
if (configuration.shouldNpmInstallThrow) {
throw new Error("NPM install throws error.");
}

Expand All @@ -50,6 +50,8 @@ function createTestInjector(configuration?: {shouldNpmInstallThrow: boolean, npm

injector.register("projectTemplatesService", ProjectTemplatesService);

injector.register("analyticsService", { track: () => Future.fromResult() });

return injector;
}

Expand All @@ -61,36 +63,36 @@ describe("project-templates-service", () => {
});

describe("prepareTemplate", () => {
describe("throws error", () =>{
describe("throws error", () => {
it("when npm install fails", () => {
testInjector = createTestInjector({shouldNpmInstallThrow: true, npmInstallationDirContents: [], npmInstallationDirNodeModulesContents: null});
testInjector = createTestInjector({ shouldNpmInstallThrow: true, npmInstallationDirContents: [], npmInstallationDirNodeModulesContents: null });
projectTemplatesService = testInjector.resolve("projectTemplatesService");
let tempFolder = temp.mkdirSync("preparetemplate");
assert.throws(() => projectTemplatesService.prepareTemplate("invalidName", tempFolder).wait());
});
});

describe("returns correct path to template", () => {
it("when reserved template name is used", () =>{
testInjector = createTestInjector({shouldNpmInstallThrow: false, npmInstallationDirContents: [], npmInstallationDirNodeModulesContents: []});
it("when reserved template name is used", () => {
testInjector = createTestInjector({ shouldNpmInstallThrow: false, npmInstallationDirContents: [], npmInstallationDirNodeModulesContents: [] });
projectTemplatesService = testInjector.resolve("projectTemplatesService");
let tempFolder = temp.mkdirSync("preparetemplate");
let actualPathToTemplate = projectTemplatesService.prepareTemplate("typescript", tempFolder).wait();
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)", () =>{
testInjector = createTestInjector({shouldNpmInstallThrow: false, npmInstallationDirContents: [], npmInstallationDirNodeModulesContents: []});
it("when reserved template name is used (case-insensitive test)", () => {
testInjector = createTestInjector({ shouldNpmInstallThrow: false, npmInstallationDirContents: [], npmInstallationDirNodeModulesContents: [] });
projectTemplatesService = testInjector.resolve("projectTemplatesService");
let tempFolder = temp.mkdirSync("preparetemplate");
let actualPathToTemplate = projectTemplatesService.prepareTemplate("tYpEsCriPT", tempFolder).wait();
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", () =>{
testInjector = createTestInjector({shouldNpmInstallThrow: false, npmInstallationDirContents: [], npmInstallationDirNodeModulesContents: []});
it("uses defaultTemplate when undefined is passed as parameter", () => {
testInjector = createTestInjector({ shouldNpmInstallThrow: false, npmInstallationDirContents: [], npmInstallationDirNodeModulesContents: [] });
projectTemplatesService = testInjector.resolve("projectTemplatesService");
let tempFolder = temp.mkdirSync("preparetemplate");
let actualPathToTemplate = projectTemplatesService.prepareTemplate(undefined, tempFolder).wait();
Expand Down