From b1101cca1f15dbc7384985dc9f9e54463ec2c5d8 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Tue, 3 Apr 2018 12:21:22 +0300 Subject: [PATCH 1/4] feat(analytics): Remove tracking in cross-client project We are no longer using the cross clients analytics project, so delete the code that sends information to it. --- .../analytics/google-analytics-provider.ts | 33 +++---------------- 1 file changed, 5 insertions(+), 28 deletions(-) diff --git a/lib/services/analytics/google-analytics-provider.ts b/lib/services/analytics/google-analytics-provider.ts index 2ce3a8284e..785e6d1c10 100644 --- a/lib/services/analytics/google-analytics-provider.ts +++ b/lib/services/analytics/google-analytics-provider.ts @@ -4,7 +4,6 @@ import { AnalyticsClients } from "../../common/constants"; export class GoogleAnalyticsProvider implements IGoogleAnalyticsProvider { private static GA_TRACKING_ID = "UA-111455-44"; - private static GA_CROSS_CLIENT_TRACKING_ID = "UA-111455-51"; private currentPage: string; constructor(private clientId: string, @@ -15,15 +14,12 @@ export class GoogleAnalyticsProvider implements IGoogleAnalyticsProvider { } public async trackHit(trackInfo: IGoogleAnalyticsData): Promise { - const trackingIds = [GoogleAnalyticsProvider.GA_TRACKING_ID, GoogleAnalyticsProvider.GA_CROSS_CLIENT_TRACKING_ID]; const sessionId = uuid.v4(); - for (const gaTrackingId of trackingIds) { - try { - await this.track(gaTrackingId, trackInfo, sessionId); - } catch (e) { - this.$logger.trace("Analytics exception: ", e); - } + try { + await this.track(GoogleAnalyticsProvider.GA_TRACKING_ID, trackInfo, sessionId); + } catch (e) { + this.$logger.trace("Analytics exception: ", e); } } @@ -41,14 +37,7 @@ export class GoogleAnalyticsProvider implements IGoogleAnalyticsProvider { } }); - switch (gaTrackingId) { - case GoogleAnalyticsProvider.GA_CROSS_CLIENT_TRACKING_ID: - this.setCrossClientCustomDimensions(visitor, sessionId); - break; - default: - await this.setCustomDimensions(visitor, trackInfo.customDimensions, sessionId); - break; - } + await this.setCustomDimensions(visitor, trackInfo.customDimensions, sessionId); switch (trackInfo.googleAnalyticsDataType) { case GoogleAnalyticsDataType.Page: @@ -83,18 +72,6 @@ export class GoogleAnalyticsProvider implements IGoogleAnalyticsProvider { }); } - private async setCrossClientCustomDimensions(visitor: ua.Visitor, sessionId: string): Promise { - const customDimensions: IStringDictionary = { - [GoogleAnalyticsCrossClientCustomDimensions.sessionId]: sessionId, - [GoogleAnalyticsCrossClientCustomDimensions.clientId]: this.clientId, - [GoogleAnalyticsCrossClientCustomDimensions.crossClientId]: this.clientId, - }; - - _.each(customDimensions, (value, key) => { - visitor.set(key, value); - }); - } - private trackEvent(visitor: ua.Visitor, trackInfo: IGoogleAnalyticsEventData): Promise { return new Promise((resolve, reject) => { visitor.event(trackInfo.category, trackInfo.action, trackInfo.label, trackInfo.value, { p: this.currentPage }, (err: Error) => { From 676b77426de00f5142e3663b43e17e524024e885 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Wed, 4 Apr 2018 01:26:44 +0300 Subject: [PATCH 2/4] fix: Getting started prompter fails The prompter for getting started changes fails in some cases, as we do not await the validation of the livesyncCommandHelper.validate call. So we try to execute some operations as we think the environment is setup correctly, but we are unable to do so and the code fails. Also add additional option to prompter in case cloud extension is installed. This gives better visibility of the feature --- lib/commands/run.ts | 2 +- lib/definitions/platform.d.ts | 2 +- lib/services/analytics/analytics-service.ts | 2 +- lib/services/android-project-service.ts | 2 +- lib/services/doctor-service.ts | 42 +++++++++++++++--- lib/services/ios-project-service.ts | 2 +- .../platform-environment-requirements.ts | 44 ++++++++++++++----- 7 files changed, 76 insertions(+), 20 deletions(-) diff --git a/lib/commands/run.ts b/lib/commands/run.ts index 38365c2a59..f2d361d609 100644 --- a/lib/commands/run.ts +++ b/lib/commands/run.ts @@ -28,7 +28,7 @@ export class RunCommandBase implements ICommand { this.platform = this.$devicePlatformsConstants.Android; } - this.$liveSyncCommandHelper.validatePlatform(this.platform); + await this.$liveSyncCommandHelper.validatePlatform(this.platform); return true; } diff --git a/lib/definitions/platform.d.ts b/lib/definitions/platform.d.ts index 190467ddd0..59d6977667 100644 --- a/lib/definitions/platform.d.ts +++ b/lib/definitions/platform.d.ts @@ -381,5 +381,5 @@ interface IUpdateAppOptions extends IOptionalFilesToSync, IOptionalFilesToRemove } interface IPlatformEnvironmentRequirements { - checkEnvironmentRequirements(platform: string): Promise; + checkEnvironmentRequirements(platform: string, projectDir: string): Promise; } \ No newline at end of file diff --git a/lib/services/analytics/analytics-service.ts b/lib/services/analytics/analytics-service.ts index 3c28cd138f..5e24e81871 100644 --- a/lib/services/analytics/analytics-service.ts +++ b/lib/services/analytics/analytics-service.ts @@ -73,7 +73,7 @@ export class AnalyticsService extends AnalyticsServiceBase { // In some cases (like in case action is Build and platform is Android), we do not know if the deviceType is emulator or device. // Just exclude the device_type in this case. - if (isForDevice !== null) { + if (isForDevice !== null && isForDevice !== undefined) { const deviceType = isForDevice ? DeviceTypes.Device : (this.$mobileHelper.isAndroidPlatform(platform) ? DeviceTypes.Emulator : DeviceTypes.Simulator); label = this.addDataToLabel(label, deviceType); } diff --git a/lib/services/android-project-service.ts b/lib/services/android-project-service.ts index d47626112f..66ce5ec28a 100644 --- a/lib/services/android-project-service.ts +++ b/lib/services/android-project-service.ts @@ -133,7 +133,7 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject this.validatePackageName(projectData.projectId); this.validateProjectName(projectData.projectName); - await this.$platformEnvironmentRequirements.checkEnvironmentRequirements(this.getPlatformData(projectData).normalizedPlatformName); + await this.$platformEnvironmentRequirements.checkEnvironmentRequirements(this.getPlatformData(projectData).normalizedPlatformName, projectData.projectDir); this.$androidToolsInfo.validateTargetSdk({ showWarningsAsErrors: true }); } diff --git a/lib/services/doctor-service.ts b/lib/services/doctor-service.ts index 4b0b95580b..2da86d2f25 100644 --- a/lib/services/doctor-service.ts +++ b/lib/services/doctor-service.ts @@ -53,32 +53,64 @@ class DoctorService implements IDoctorService { } } - public runSetupScript(): Promise { + public async runSetupScript(): Promise { + await this.$analyticsService.trackEventActionInGoogleAnalytics({ + action: "Run setup script", + additionalData: "Start", + }); + if (this.$hostInfo.isLinux) { + await this.$analyticsService.trackEventActionInGoogleAnalytics({ + action: "Run setup script", + additionalData: "Skipped as OS is Linux", + }); return; } this.$logger.out("Running the setup script to try and automatically configure your environment."); if (this.$hostInfo.isDarwin) { - return this.runSetupScriptCore(DoctorService.DarwinSetupScriptLocation, []); + await this.runSetupScriptCore(DoctorService.DarwinSetupScriptLocation, []); } if (this.$hostInfo.isWindows) { - return this.runSetupScriptCore(DoctorService.WindowsSetupScriptExecutable, DoctorService.WindowsSetupScriptArguments); + await this.runSetupScriptCore(DoctorService.WindowsSetupScriptExecutable, DoctorService.WindowsSetupScriptArguments); } + + await this.$analyticsService.trackEventActionInGoogleAnalytics({ + action: "Run setup script", + additionalData: "Finished", + }); } public async canExecuteLocalBuild(platform?: string): Promise { + await this.$analyticsService.trackEventActionInGoogleAnalytics({ + action: "Check Local Build Setup", + additionalData: "Started", + }); const infos = await doctor.getInfos({ platform }); const warnings = this.filterInfosByType(infos, constants.WARNING_TYPE_NAME); - if (warnings.length > 0) { + const hasWarnings = warnings.length > 0; + if (hasWarnings) { + // TODO: Separate the track per platform: + // Could be in two separate trackings or in the same, but with additional information, for example: + // Errors:__.join(--)$$__.join(--)... + await this.$analyticsService.trackEventActionInGoogleAnalytics({ + action: "Check Local Build Setup", + additionalData: `Warnings:${warnings.map(w => w.message).join("__")}`, + }); this.printInfosCore(infos); } else { infos.map(info => this.$logger.trace(info.message)); } - return warnings.length === 0; + + await this.$analyticsService.trackEventActionInGoogleAnalytics({ + action: "Check Local Build Setup", + additionalData: `Finished: ${hasWarnings}`, + }); + + return !hasWarnings; } private async promptForDocs(link: string): Promise { diff --git a/lib/services/ios-project-service.ts b/lib/services/ios-project-service.ts index 4191bee724..b8054ab86e 100644 --- a/lib/services/ios-project-service.ts +++ b/lib/services/ios-project-service.ts @@ -139,7 +139,7 @@ export class IOSProjectService extends projectServiceBaseLib.PlatformProjectServ return; } - await this.$platformEnvironmentRequirements.checkEnvironmentRequirements(this.getPlatformData(projectData).normalizedPlatformName); + await this.$platformEnvironmentRequirements.checkEnvironmentRequirements(this.getPlatformData(projectData).normalizedPlatformName, projectData.projectDir); const xcodeBuildVersion = await this.getXcodeVersion(); if (helpers.versionCompare(xcodeBuildVersion, IOSProjectService.XCODEBUILD_MIN_VERSION) < 0) { diff --git a/lib/services/platform-environment-requirements.ts b/lib/services/platform-environment-requirements.ts index 0eba53eb58..faf33d9192 100644 --- a/lib/services/platform-environment-requirements.ts +++ b/lib/services/platform-environment-requirements.ts @@ -9,10 +9,12 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ private $logger: ILogger, private $nativeScriptCloudExtensionService: INativeScriptCloudExtensionService, private $prompter: IPrompter, - private $staticConfig: IStaticConfig) { } + private $staticConfig: IStaticConfig, + private $analyticsService: IAnalyticsService) { } public static CLOUD_SETUP_OPTION_NAME = "Configure for Cloud Builds"; public static LOCAL_SETUP_OPTION_NAME = "Configure for Local Builds"; + public static TRY_CLOUD_OPERATION_OPTION_NAME = "Try Cloud Operation"; public static MANUALLY_SETUP_OPTION_NAME = "Skip Step and Configure Manually"; private static BOTH_CLOUD_SETUP_AND_LOCAL_SETUP_OPTION_NAME = "Configure for Both Local and Cloud Builds"; private static CHOOSE_OPTIONS_MESSAGE = "To continue, choose one of the following options: "; @@ -27,14 +29,24 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ "deploy": "tns cloud deploy" }; - public async checkEnvironmentRequirements(platform: string): Promise { + public async checkEnvironmentRequirements(platform: string, projectDir: string): Promise { if (process.env.NS_SKIP_ENV_CHECK) { + await this.$analyticsService.trackEventActionInGoogleAnalytics({ + action: "Check Environment Requirements", + additionalData: "Skipped:NS_SKIP_ENV_CHECK is set", + projectDir + }); return true; } const canExecute = await this.$doctorService.canExecuteLocalBuild(platform); if (!canExecute) { if (!isInteractive()) { + await this.$analyticsService.trackEventActionInGoogleAnalytics({ + action: "Check Environment Requirements", + additionalData: "Non-interactive terminal, unable to execute local builds.", + projectDir + }); this.fail(this.getNonInteractiveConsoleMessage(platform)); } @@ -74,6 +86,11 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ this.processManuallySetup(platform); } + + if (selectedOption === PlatformEnvironmentRequirements.TRY_CLOUD_OPERATION_OPTION_NAME) { + const message = `You can use ${_.lowerFirst(this.getCloudBuildsMessage(platform))}`; + this.fail(message); + } } return true; @@ -166,18 +183,25 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ ]); } - private promptForChoice(): Promise { + private async promptForChoice(): Promise { const choices = this.$nativeScriptCloudExtensionService.isInstalled() ? [ + PlatformEnvironmentRequirements.TRY_CLOUD_OPERATION_OPTION_NAME, PlatformEnvironmentRequirements.LOCAL_SETUP_OPTION_NAME, PlatformEnvironmentRequirements.MANUALLY_SETUP_OPTION_NAME, ] : [ - PlatformEnvironmentRequirements.CLOUD_SETUP_OPTION_NAME, - PlatformEnvironmentRequirements.LOCAL_SETUP_OPTION_NAME, - PlatformEnvironmentRequirements.BOTH_CLOUD_SETUP_AND_LOCAL_SETUP_OPTION_NAME, - PlatformEnvironmentRequirements.MANUALLY_SETUP_OPTION_NAME, - ]; - - return this.$prompter.promptForChoice(PlatformEnvironmentRequirements.CHOOSE_OPTIONS_MESSAGE, choices); + PlatformEnvironmentRequirements.CLOUD_SETUP_OPTION_NAME, + PlatformEnvironmentRequirements.LOCAL_SETUP_OPTION_NAME, + PlatformEnvironmentRequirements.BOTH_CLOUD_SETUP_AND_LOCAL_SETUP_OPTION_NAME, + PlatformEnvironmentRequirements.MANUALLY_SETUP_OPTION_NAME, + ]; + + const selection = await this.$prompter.promptForChoice(PlatformEnvironmentRequirements.CHOOSE_OPTIONS_MESSAGE, choices); + await this.$analyticsService.trackEventActionInGoogleAnalytics({ + action: "Check Environment Requirements", + additionalData: `User selected: ${selection}` + // consider passing projectDir here + }); + return selection; } private getEnvVerificationMessage() { From 94978183e1f329d29c4345d21fb57f8e6dceceae Mon Sep 17 00:00:00 2001 From: fatme Date: Wed, 4 Apr 2018 14:16:14 +0300 Subject: [PATCH 3/4] * Show getting started prompts on doctor command * Fix unit test * Show message that user can use forum or slack when manually setup is selected * Fix message when nativescript-cloud extension is installed --- lib/definitions/platform.d.ts | 2 +- lib/services/android-project-service.ts | 2 +- lib/services/doctor-service.ts | 36 ++------------ lib/services/ios-project-service.ts | 2 +- .../platform-environment-requirements.ts | 47 ++++++++----------- .../platform-environment-requirements.ts | 5 +- 6 files changed, 30 insertions(+), 64 deletions(-) diff --git a/lib/definitions/platform.d.ts b/lib/definitions/platform.d.ts index 59d6977667..4f098575eb 100644 --- a/lib/definitions/platform.d.ts +++ b/lib/definitions/platform.d.ts @@ -381,5 +381,5 @@ interface IUpdateAppOptions extends IOptionalFilesToSync, IOptionalFilesToRemove } interface IPlatformEnvironmentRequirements { - checkEnvironmentRequirements(platform: string, projectDir: string): Promise; + checkEnvironmentRequirements(platform?: string): Promise; } \ No newline at end of file diff --git a/lib/services/android-project-service.ts b/lib/services/android-project-service.ts index 66ce5ec28a..d47626112f 100644 --- a/lib/services/android-project-service.ts +++ b/lib/services/android-project-service.ts @@ -133,7 +133,7 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject this.validatePackageName(projectData.projectId); this.validateProjectName(projectData.projectName); - await this.$platformEnvironmentRequirements.checkEnvironmentRequirements(this.getPlatformData(projectData).normalizedPlatformName, projectData.projectDir); + await this.$platformEnvironmentRequirements.checkEnvironmentRequirements(this.getPlatformData(projectData).normalizedPlatformName); this.$androidToolsInfo.validateTargetSdk({ showWarningsAsErrors: true }); } diff --git a/lib/services/doctor-service.ts b/lib/services/doctor-service.ts index 2da86d2f25..bf59077b4e 100644 --- a/lib/services/doctor-service.ts +++ b/lib/services/doctor-service.ts @@ -5,18 +5,14 @@ import { doctor, constants } from "nativescript-doctor"; class DoctorService implements IDoctorService { private static DarwinSetupScriptLocation = path.join(__dirname, "..", "..", "setup", "mac-startup-shell-script.sh"); - private static DarwinSetupDocsLink = "https://docs.nativescript.org/start/ns-setup-os-x"; private static WindowsSetupScriptExecutable = "powershell.exe"; private static WindowsSetupScriptArguments = ["start-process", "-FilePath", "PowerShell.exe", "-NoNewWindow", "-Wait", "-ArgumentList", '"-NoProfile -ExecutionPolicy Bypass -Command iex ((new-object net.webclient).DownloadString(\'https://www.nativescript.org/setup/win\'))"']; - private static WindowsSetupDocsLink = "https://docs.nativescript.org/start/ns-setup-win"; - private static LinuxSetupDocsLink = "https://docs.nativescript.org/start/ns-setup-linux"; constructor(private $analyticsService: IAnalyticsService, private $hostInfo: IHostInfo, private $logger: ILogger, private $childProcess: IChildProcess, - private $opener: IOpener, - private $prompter: IPrompter, + private $injector: IInjector, private $terminalSpinnerService: ITerminalSpinnerService, private $versionsService: IVersionsService) { } @@ -41,7 +37,6 @@ class DoctorService implements IDoctorService { if (hasWarnings) { this.$logger.info("There seem to be issues with your configuration."); - await this.promptForHelp(); } else { this.$logger.out("No issues were detected.".bold); } @@ -51,6 +46,8 @@ class DoctorService implements IDoctorService { } catch (err) { this.$logger.error("Cannot get the latest versions information from npm. Please try again later."); } + + await this.$injector.resolve("platformEnvironmentRequirements").checkEnvironmentRequirements(null); } public async runSetupScript(): Promise { @@ -113,33 +110,6 @@ class DoctorService implements IDoctorService { return !hasWarnings; } - private async promptForDocs(link: string): Promise { - if (await this.$prompter.confirm("Do you want to visit the official documentation?", () => helpers.isInteractive())) { - this.$opener.open(link); - } - } - - private async promptForSetupScript(executablePath: string, setupScriptArgs: string[]): Promise { - if (await this.$prompter.confirm("Do you want to run the setup script?", () => helpers.isInteractive())) { - await this.runSetupScriptCore(executablePath, setupScriptArgs); - } - } - - private async promptForHelp(): Promise { - if (this.$hostInfo.isDarwin) { - await this.promptForHelpCore(DoctorService.DarwinSetupDocsLink, DoctorService.DarwinSetupScriptLocation, []); - } else if (this.$hostInfo.isWindows) { - await this.promptForHelpCore(DoctorService.WindowsSetupDocsLink, DoctorService.WindowsSetupScriptExecutable, DoctorService.WindowsSetupScriptArguments); - } else { - await this.promptForDocs(DoctorService.LinuxSetupDocsLink); - } - } - - private async promptForHelpCore(link: string, setupScriptExecutablePath: string, setupScriptArgs: string[]): Promise { - await this.promptForDocs(link); - await this.promptForSetupScript(setupScriptExecutablePath, setupScriptArgs); - } - private async runSetupScriptCore(executablePath: string, setupScriptArgs: string[]): Promise { return this.$childProcess.spawnFromEvent(executablePath, setupScriptArgs, "close", { stdio: "inherit" }); } diff --git a/lib/services/ios-project-service.ts b/lib/services/ios-project-service.ts index b8054ab86e..4191bee724 100644 --- a/lib/services/ios-project-service.ts +++ b/lib/services/ios-project-service.ts @@ -139,7 +139,7 @@ export class IOSProjectService extends projectServiceBaseLib.PlatformProjectServ return; } - await this.$platformEnvironmentRequirements.checkEnvironmentRequirements(this.getPlatformData(projectData).normalizedPlatformName, projectData.projectDir); + await this.$platformEnvironmentRequirements.checkEnvironmentRequirements(this.getPlatformData(projectData).normalizedPlatformName); const xcodeBuildVersion = await this.getXcodeVersion(); if (helpers.versionCompare(xcodeBuildVersion, IOSProjectService.XCODEBUILD_MIN_VERSION) < 0) { diff --git a/lib/services/platform-environment-requirements.ts b/lib/services/platform-environment-requirements.ts index faf33d9192..81b200733a 100644 --- a/lib/services/platform-environment-requirements.ts +++ b/lib/services/platform-environment-requirements.ts @@ -21,6 +21,7 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ private static NOT_CONFIGURED_ENV_AFTER_SETUP_SCRIPT_MESSAGE = `The setup script was not able to configure your environment for local builds. To execute local builds, you have to set up your environment manually. In case you have any questions, you can check our forum: 'http://forum.nativescript.org' and our public Slack channel: 'https://nativescriptcommunity.slack.com/'. ${PlatformEnvironmentRequirements.CHOOSE_OPTIONS_MESSAGE}`; private static MISSING_LOCAL_SETUP_MESSAGE = "Your environment is not configured properly and you will not be able to execute local builds."; private static MISSING_LOCAL_AND_CLOUD_SETUP_MESSAGE = `You are missing the ${constants.NATIVESCRIPT_CLOUD_EXTENSION_NAME} extension and you will not be able to execute cloud builds. ${PlatformEnvironmentRequirements.MISSING_LOCAL_SETUP_MESSAGE} ${PlatformEnvironmentRequirements.CHOOSE_OPTIONS_MESSAGE} `; + private static MISSING_LOCAL_BUT_CLOUD_SETUP_MESSAGE = `You have ${constants.NATIVESCRIPT_CLOUD_EXTENSION_NAME} extension installed but ${_.lowerFirst(PlatformEnvironmentRequirements.MISSING_LOCAL_SETUP_MESSAGE)}`; private static RUN_TNS_SETUP_MESSAGE = 'Run $ tns setup command to run the setup script to try to automatically configure your environment for local builds.'; private cliCommandToCloudCommandName: IStringDictionary = { @@ -29,12 +30,11 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ "deploy": "tns cloud deploy" }; - public async checkEnvironmentRequirements(platform: string, projectDir: string): Promise { + public async checkEnvironmentRequirements(platform?: string): Promise { if (process.env.NS_SKIP_ENV_CHECK) { await this.$analyticsService.trackEventActionInGoogleAnalytics({ action: "Check Environment Requirements", - additionalData: "Skipped:NS_SKIP_ENV_CHECK is set", - projectDir + additionalData: "Skipped:NS_SKIP_ENV_CHECK is set" }); return true; } @@ -44,8 +44,7 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ if (!isInteractive()) { await this.$analyticsService.trackEventActionInGoogleAnalytics({ action: "Check Environment Requirements", - additionalData: "Non-interactive terminal, unable to execute local builds.", - projectDir + additionalData: "Non-interactive terminal, unable to execute local builds." }); this.fail(this.getNonInteractiveConsoleMessage(platform)); } @@ -54,8 +53,8 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ const selectedOption = await this.promptForChoice(); - await this.processCloudBuildsIfNeeded(platform, selectedOption); - this.processManuallySetupIfNeeded(platform, selectedOption); + await this.processCloudBuildsIfNeeded(selectedOption, platform); + this.processManuallySetupIfNeeded(selectedOption, platform); if (selectedOption === PlatformEnvironmentRequirements.LOCAL_SETUP_OPTION_NAME) { await this.$doctorService.runSetupScript(); @@ -72,14 +71,13 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ PlatformEnvironmentRequirements.MANUALLY_SETUP_OPTION_NAME ]); - await this.processCloudBuildsIfNeeded(platform, option); - - this.processManuallySetupIfNeeded(platform, option); + await this.processCloudBuildsIfNeeded(option, platform); + this.processManuallySetupIfNeeded(option, platform); } } if (selectedOption === PlatformEnvironmentRequirements.BOTH_CLOUD_SETUP_AND_LOCAL_SETUP_OPTION_NAME) { - await this.processBothCloudBuildsAndSetupScript(platform); + await this.processBothCloudBuildsAndSetupScript(); if (await this.$doctorService.canExecuteLocalBuild(platform)) { return true; } @@ -88,30 +86,29 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ } if (selectedOption === PlatformEnvironmentRequirements.TRY_CLOUD_OPERATION_OPTION_NAME) { - const message = `You can use ${_.lowerFirst(this.getCloudBuildsMessage(platform))}`; - this.fail(message); + this.fail(this.getCloudBuildsMessage(platform)); } } return true; } - private async processCloudBuildsIfNeeded(platform: string, selectedOption: string): Promise { + private async processCloudBuildsIfNeeded(selectedOption: string, platform?: string): Promise { if (selectedOption === PlatformEnvironmentRequirements.CLOUD_SETUP_OPTION_NAME) { await this.processCloudBuilds(platform); } } private async processCloudBuilds(platform: string): Promise { - await this.processCloudBuildsCore(platform); + await this.processCloudBuildsCore(); this.fail(this.getCloudBuildsMessage(platform)); } - private processCloudBuildsCore(platform: string): Promise { + private processCloudBuildsCore(): Promise { return this.$nativeScriptCloudExtensionService.install(); } - private getCloudBuildsMessage(platform: string): string { + private getCloudBuildsMessage(platform?: string): string { const cloudCommandName = this.cliCommandToCloudCommandName[this.$commandsService.currentCommandData.commandName]; if (!cloudCommandName) { return `In order to test your application use the $ tns login command to log in with your account and then $ tns cloud build command to build your app in the cloud.`; @@ -124,19 +121,19 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ return `Use the $ tns login command to log in with your account and then $ ${cloudCommandName.toLowerCase()} ${platform.toLowerCase()} command.`; } - private processManuallySetupIfNeeded(platform: string, selectedOption: string) { + private processManuallySetupIfNeeded(selectedOption: string, platform?: string) { if (selectedOption === PlatformEnvironmentRequirements.MANUALLY_SETUP_OPTION_NAME) { this.processManuallySetup(platform); } } - private processManuallySetup(platform: string): void { - this.fail(`To be able to build for ${platform}, verify that your environment is configured according to the system requirements described at ${this.$staticConfig.SYS_REQUIREMENTS_LINK}`); + private processManuallySetup(platform?: string): void { + this.fail(`To be able to ${platform ? `build for ${platform}` : 'build'}, verify that your environment is configured according to the system requirements described at ${this.$staticConfig.SYS_REQUIREMENTS_LINK}. In case you have any questions, you can check our forum: 'http://forum.nativescript.org' and our public Slack channel: 'https://nativescriptcommunity.slack.com/'.`); } - private async processBothCloudBuildsAndSetupScript(platform: string): Promise { + private async processBothCloudBuildsAndSetupScript(): Promise { try { - await this.processCloudBuildsCore(platform); + await this.processCloudBuildsCore(); } catch (e) { this.$logger.trace(`Error while installing ${constants.NATIVESCRIPT_CLOUD_EXTENSION_NAME} extension. ${e.message}.`); } @@ -165,12 +162,9 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ } private getInteractiveConsoleMessage(platform: string) { - const message = `The ${constants.NATIVESCRIPT_CLOUD_EXTENSION_NAME} extension is installed and you can ${_.lowerFirst(this.getCloudBuildsMessage(platform))}`; - return this.$nativeScriptCloudExtensionService.isInstalled() ? this.buildMultilineMessage([ - `${message.bold}`, - `${PlatformEnvironmentRequirements.MISSING_LOCAL_SETUP_MESSAGE} ${PlatformEnvironmentRequirements.CHOOSE_OPTIONS_MESSAGE}`, + `${PlatformEnvironmentRequirements.MISSING_LOCAL_BUT_CLOUD_SETUP_MESSAGE} ${PlatformEnvironmentRequirements.CHOOSE_OPTIONS_MESSAGE}`, `Select "Configure for Local Builds" to run the setup script and automatically configure your environment for local builds.`, `Select "Skip Step and Configure Manually" to disregard this option and install any required components manually.` ]) : @@ -199,7 +193,6 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ await this.$analyticsService.trackEventActionInGoogleAnalytics({ action: "Check Environment Requirements", additionalData: `User selected: ${selection}` - // consider passing projectDir here }); return selection; } diff --git a/test/services/platform-environment-requirements.ts b/test/services/platform-environment-requirements.ts index abb8bb76ee..28d577b120 100644 --- a/test/services/platform-environment-requirements.ts +++ b/test/services/platform-environment-requirements.ts @@ -13,6 +13,9 @@ const nonInteractiveConsoleMessageWhenExtensionIsInstalled = `Your environment i function createTestInjector() { const testInjector = new Yok(); + testInjector.register("analyticsService", { + trackEventActionInGoogleAnalytics: () => ({}) + }); testInjector.register("commandsService", {currentCommandData: {commandName: "test", commandArguments: [""]}}); testInjector.register("doctorService", {}); testInjector.register("errors", { @@ -103,7 +106,7 @@ describe("platformEnvironmentRequirements ", () => { await assert.isRejected(platformEnvironmentRequirements.checkEnvironmentRequirements(platform)); assert.isTrue(promptForChoiceData.length === 1); assert.deepEqual("To continue, choose one of the following options: ", promptForChoiceData[0].message); - assert.deepEqual(['Configure for Local Builds', 'Skip Step and Configure Manually'], promptForChoiceData[0].choices); + assert.deepEqual(['Try Cloud Operation', 'Configure for Local Builds', 'Skip Step and Configure Manually'], promptForChoiceData[0].choices); }); it("should skip env chech when NS_SKIP_ENV_CHECK environment variable is passed", async() => { process.env.NS_SKIP_ENV_CHECK = true; From 32115dadc67e6ce2822f8bb921b3156f9f57634b Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Wed, 4 Apr 2018 15:42:53 +0300 Subject: [PATCH 4/4] fix: Use correct messages and tracking for getting started prompters --- lib/constants.ts | 5 +- lib/services/doctor-service.ts | 22 ++--- .../platform-environment-requirements.ts | 92 ++++++++++++------- lib/services/platform-service.ts | 2 +- 4 files changed, 74 insertions(+), 47 deletions(-) diff --git a/lib/constants.ts b/lib/constants.ts index 31fa4d306f..7983e71a1a 100644 --- a/lib/constants.ts +++ b/lib/constants.ts @@ -135,7 +135,10 @@ export const enum TrackActionNames { CreateProject = "Create project", Debug = "Debug", Deploy = "Deploy", - LiveSync = "LiveSync" + LiveSync = "LiveSync", + RunSetupScript = "Run Setup Script", + CheckLocalBuildSetup = "Check Local Build Setup", + CheckEnvironmentRequirements = "Check Environment Requirements" } export const enum BuildStates { diff --git a/lib/services/doctor-service.ts b/lib/services/doctor-service.ts index bf59077b4e..514614833c 100644 --- a/lib/services/doctor-service.ts +++ b/lib/services/doctor-service.ts @@ -1,6 +1,7 @@ import { EOL } from "os"; import * as path from "path"; import * as helpers from "../common/helpers"; +import { TrackActionNames } from "../constants"; import { doctor, constants } from "nativescript-doctor"; class DoctorService implements IDoctorService { @@ -52,13 +53,13 @@ class DoctorService implements IDoctorService { public async runSetupScript(): Promise { await this.$analyticsService.trackEventActionInGoogleAnalytics({ - action: "Run setup script", - additionalData: "Start", + action: TrackActionNames.RunSetupScript, + additionalData: "Starting", }); if (this.$hostInfo.isLinux) { await this.$analyticsService.trackEventActionInGoogleAnalytics({ - action: "Run setup script", + action: TrackActionNames.RunSetupScript, additionalData: "Skipped as OS is Linux", }); return; @@ -75,26 +76,23 @@ class DoctorService implements IDoctorService { } await this.$analyticsService.trackEventActionInGoogleAnalytics({ - action: "Run setup script", + action: TrackActionNames.RunSetupScript, additionalData: "Finished", }); } public async canExecuteLocalBuild(platform?: string): Promise { await this.$analyticsService.trackEventActionInGoogleAnalytics({ - action: "Check Local Build Setup", - additionalData: "Started", + action: TrackActionNames.CheckLocalBuildSetup, + additionalData: "Starting", }); const infos = await doctor.getInfos({ platform }); const warnings = this.filterInfosByType(infos, constants.WARNING_TYPE_NAME); const hasWarnings = warnings.length > 0; if (hasWarnings) { - // TODO: Separate the track per platform: - // Could be in two separate trackings or in the same, but with additional information, for example: - // Errors:__.join(--)$$__.join(--)... await this.$analyticsService.trackEventActionInGoogleAnalytics({ - action: "Check Local Build Setup", + action: TrackActionNames.CheckLocalBuildSetup, additionalData: `Warnings:${warnings.map(w => w.message).join("__")}`, }); this.printInfosCore(infos); @@ -103,8 +101,8 @@ class DoctorService implements IDoctorService { } await this.$analyticsService.trackEventActionInGoogleAnalytics({ - action: "Check Local Build Setup", - additionalData: `Finished: ${hasWarnings}`, + action: TrackActionNames.CheckLocalBuildSetup, + additionalData: `Finished: Is setup correct: ${!hasWarnings}`, }); return !hasWarnings; diff --git a/lib/services/platform-environment-requirements.ts b/lib/services/platform-environment-requirements.ts index 81b200733a..cfc1213c2f 100644 --- a/lib/services/platform-environment-requirements.ts +++ b/lib/services/platform-environment-requirements.ts @@ -1,4 +1,4 @@ -import * as constants from "../constants"; +import { NATIVESCRIPT_CLOUD_EXTENSION_NAME, TrackActionNames } from "../constants"; import { isInteractive } from "../common/helpers"; import { EOL } from "os"; @@ -18,10 +18,10 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ public static MANUALLY_SETUP_OPTION_NAME = "Skip Step and Configure Manually"; private static BOTH_CLOUD_SETUP_AND_LOCAL_SETUP_OPTION_NAME = "Configure for Both Local and Cloud Builds"; private static CHOOSE_OPTIONS_MESSAGE = "To continue, choose one of the following options: "; - private static NOT_CONFIGURED_ENV_AFTER_SETUP_SCRIPT_MESSAGE = `The setup script was not able to configure your environment for local builds. To execute local builds, you have to set up your environment manually. In case you have any questions, you can check our forum: 'http://forum.nativescript.org' and our public Slack channel: 'https://nativescriptcommunity.slack.com/'. ${PlatformEnvironmentRequirements.CHOOSE_OPTIONS_MESSAGE}`; + private static NOT_CONFIGURED_ENV_AFTER_SETUP_SCRIPT_MESSAGE = `The setup script was not able to configure your environment for local builds. To execute local builds, you have to set up your environment manually. In case you have any questions, you can check our forum: 'http://forum.nativescript.org' and our public Slack channel: 'https://nativescriptcommunity.slack.com/'.`; private static MISSING_LOCAL_SETUP_MESSAGE = "Your environment is not configured properly and you will not be able to execute local builds."; - private static MISSING_LOCAL_AND_CLOUD_SETUP_MESSAGE = `You are missing the ${constants.NATIVESCRIPT_CLOUD_EXTENSION_NAME} extension and you will not be able to execute cloud builds. ${PlatformEnvironmentRequirements.MISSING_LOCAL_SETUP_MESSAGE} ${PlatformEnvironmentRequirements.CHOOSE_OPTIONS_MESSAGE} `; - private static MISSING_LOCAL_BUT_CLOUD_SETUP_MESSAGE = `You have ${constants.NATIVESCRIPT_CLOUD_EXTENSION_NAME} extension installed but ${_.lowerFirst(PlatformEnvironmentRequirements.MISSING_LOCAL_SETUP_MESSAGE)}`; + private static MISSING_LOCAL_AND_CLOUD_SETUP_MESSAGE = `You are missing the ${NATIVESCRIPT_CLOUD_EXTENSION_NAME} extension and you will not be able to execute cloud builds. ${PlatformEnvironmentRequirements.MISSING_LOCAL_SETUP_MESSAGE} ${PlatformEnvironmentRequirements.CHOOSE_OPTIONS_MESSAGE} `; + private static MISSING_LOCAL_BUT_CLOUD_SETUP_MESSAGE = `You have ${NATIVESCRIPT_CLOUD_EXTENSION_NAME} extension installed, so you can execute cloud builds, but ${_.lowerFirst(PlatformEnvironmentRequirements.MISSING_LOCAL_SETUP_MESSAGE)}`; private static RUN_TNS_SETUP_MESSAGE = 'Run $ tns setup command to run the setup script to try to automatically configure your environment for local builds.'; private cliCommandToCloudCommandName: IStringDictionary = { @@ -33,8 +33,8 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ public async checkEnvironmentRequirements(platform?: string): Promise { if (process.env.NS_SKIP_ENV_CHECK) { await this.$analyticsService.trackEventActionInGoogleAnalytics({ - action: "Check Environment Requirements", - additionalData: "Skipped:NS_SKIP_ENV_CHECK is set" + action: TrackActionNames.CheckEnvironmentRequirements, + additionalData: "Skipped: NS_SKIP_ENV_CHECK is set" }); return true; } @@ -43,15 +43,27 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ if (!canExecute) { if (!isInteractive()) { await this.$analyticsService.trackEventActionInGoogleAnalytics({ - action: "Check Environment Requirements", + action: TrackActionNames.CheckEnvironmentRequirements, additionalData: "Non-interactive terminal, unable to execute local builds." }); this.fail(this.getNonInteractiveConsoleMessage(platform)); } - this.$logger.info(this.getInteractiveConsoleMessage(platform)); + const infoMessage = this.getInteractiveConsoleMessage(platform); + this.$logger.info(infoMessage); - const selectedOption = await this.promptForChoice(); + const choices = this.$nativeScriptCloudExtensionService.isInstalled() ? [ + PlatformEnvironmentRequirements.TRY_CLOUD_OPERATION_OPTION_NAME, + PlatformEnvironmentRequirements.LOCAL_SETUP_OPTION_NAME, + PlatformEnvironmentRequirements.MANUALLY_SETUP_OPTION_NAME, + ] : [ + PlatformEnvironmentRequirements.CLOUD_SETUP_OPTION_NAME, + PlatformEnvironmentRequirements.LOCAL_SETUP_OPTION_NAME, + PlatformEnvironmentRequirements.BOTH_CLOUD_SETUP_AND_LOCAL_SETUP_OPTION_NAME, + PlatformEnvironmentRequirements.MANUALLY_SETUP_OPTION_NAME, + ]; + + const selectedOption = await this.promptForChoice({ infoMessage, choices }); await this.processCloudBuildsIfNeeded(selectedOption, platform); this.processManuallySetupIfNeeded(selectedOption, platform); @@ -64,12 +76,24 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ } if (this.$nativeScriptCloudExtensionService.isInstalled()) { - this.processManuallySetup(platform); + const option = await this.promptForChoice({ + infoMessage: PlatformEnvironmentRequirements.NOT_CONFIGURED_ENV_AFTER_SETUP_SCRIPT_MESSAGE, + choices: [ + PlatformEnvironmentRequirements.TRY_CLOUD_OPERATION_OPTION_NAME, + PlatformEnvironmentRequirements.MANUALLY_SETUP_OPTION_NAME + ] + }); + + this.processTryCloudSetupIfNeeded(option, platform); + this.processManuallySetupIfNeeded(option, platform); } else { - const option = await this.$prompter.promptForChoice(PlatformEnvironmentRequirements.NOT_CONFIGURED_ENV_AFTER_SETUP_SCRIPT_MESSAGE, [ - PlatformEnvironmentRequirements.CLOUD_SETUP_OPTION_NAME, - PlatformEnvironmentRequirements.MANUALLY_SETUP_OPTION_NAME - ]); + const option = await this.promptForChoice({ + infoMessage: PlatformEnvironmentRequirements.NOT_CONFIGURED_ENV_AFTER_SETUP_SCRIPT_MESSAGE, + choices: [ + PlatformEnvironmentRequirements.CLOUD_SETUP_OPTION_NAME, + PlatformEnvironmentRequirements.MANUALLY_SETUP_OPTION_NAME + ] + }); await this.processCloudBuildsIfNeeded(option, platform); this.processManuallySetupIfNeeded(option, platform); @@ -85,9 +109,7 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ this.processManuallySetup(platform); } - if (selectedOption === PlatformEnvironmentRequirements.TRY_CLOUD_OPERATION_OPTION_NAME) { - this.fail(this.getCloudBuildsMessage(platform)); - } + this.processTryCloudSetupIfNeeded(selectedOption, platform); } return true; @@ -121,6 +143,12 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ return `Use the $ tns login command to log in with your account and then $ ${cloudCommandName.toLowerCase()} ${platform.toLowerCase()} command.`; } + private processTryCloudSetupIfNeeded(selectedOption: string, platform?: string) { + if (selectedOption === PlatformEnvironmentRequirements.TRY_CLOUD_OPERATION_OPTION_NAME) { + this.fail(this.getCloudBuildsMessage(platform)); + } + } + private processManuallySetupIfNeeded(selectedOption: string, platform?: string) { if (selectedOption === PlatformEnvironmentRequirements.MANUALLY_SETUP_OPTION_NAME) { this.processManuallySetup(platform); @@ -135,7 +163,7 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ try { await this.processCloudBuildsCore(); } catch (e) { - this.$logger.trace(`Error while installing ${constants.NATIVESCRIPT_CLOUD_EXTENSION_NAME} extension. ${e.message}.`); + this.$logger.trace(`Error while installing ${NATIVESCRIPT_CLOUD_EXTENSION_NAME} extension. ${e.message}.`); } await this.$doctorService.runSetupScript(); @@ -156,7 +184,7 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ this.buildMultilineMessage([ PlatformEnvironmentRequirements.MISSING_LOCAL_AND_CLOUD_SETUP_MESSAGE, PlatformEnvironmentRequirements.RUN_TNS_SETUP_MESSAGE, - `Run $ tns cloud setup command to install the ${constants.NATIVESCRIPT_CLOUD_EXTENSION_NAME} extension to configure your environment for cloud builds.`, + `Run $ tns cloud setup command to install the ${NATIVESCRIPT_CLOUD_EXTENSION_NAME} extension to configure your environment for cloud builds.`, this.getEnvVerificationMessage() ]); } @@ -170,30 +198,28 @@ export class PlatformEnvironmentRequirements implements IPlatformEnvironmentRequ ]) : this.buildMultilineMessage([ PlatformEnvironmentRequirements.MISSING_LOCAL_AND_CLOUD_SETUP_MESSAGE, - `Select "Configure for Cloud Builds" to install the ${constants.NATIVESCRIPT_CLOUD_EXTENSION_NAME} extension and automatically configure your environment for cloud builds.`, + `Select "Configure for Cloud Builds" to install the ${NATIVESCRIPT_CLOUD_EXTENSION_NAME} extension and automatically configure your environment for cloud builds.`, `Select "Configure for Local Builds" to run the setup script and automatically configure your environment for local builds.`, `Select "Configure for Both Local and Cloud Builds" to automatically configure your environment for both options.`, `Select "Configure for Both Local and Cloud Builds" to automatically configure your environment for both options.` ]); } - private async promptForChoice(): Promise { - const choices = this.$nativeScriptCloudExtensionService.isInstalled() ? [ - PlatformEnvironmentRequirements.TRY_CLOUD_OPERATION_OPTION_NAME, - PlatformEnvironmentRequirements.LOCAL_SETUP_OPTION_NAME, - PlatformEnvironmentRequirements.MANUALLY_SETUP_OPTION_NAME, - ] : [ - PlatformEnvironmentRequirements.CLOUD_SETUP_OPTION_NAME, - PlatformEnvironmentRequirements.LOCAL_SETUP_OPTION_NAME, - PlatformEnvironmentRequirements.BOTH_CLOUD_SETUP_AND_LOCAL_SETUP_OPTION_NAME, - PlatformEnvironmentRequirements.MANUALLY_SETUP_OPTION_NAME, - ]; + private async promptForChoice(opts: { infoMessage: string, choices: string[], }): Promise { + this.$logger.info(opts.infoMessage); - const selection = await this.$prompter.promptForChoice(PlatformEnvironmentRequirements.CHOOSE_OPTIONS_MESSAGE, choices); await this.$analyticsService.trackEventActionInGoogleAnalytics({ - action: "Check Environment Requirements", + action: TrackActionNames.CheckEnvironmentRequirements, + additionalData: `User should select: ${opts.infoMessage}` + }); + + const selection = await this.$prompter.promptForChoice(PlatformEnvironmentRequirements.CHOOSE_OPTIONS_MESSAGE, opts.choices); + + await this.$analyticsService.trackEventActionInGoogleAnalytics({ + action: TrackActionNames.CheckEnvironmentRequirements, additionalData: `User selected: ${selection}` }); + return selection; } diff --git a/lib/services/platform-service.ts b/lib/services/platform-service.ts index c817cbf714..94fff61155 100644 --- a/lib/services/platform-service.ts +++ b/lib/services/platform-service.ts @@ -341,7 +341,7 @@ export class PlatformService extends EventEmitter implements IPlatformService { } const platformData = this.$platformsData.getPlatformData(platform, projectData); - const forDevice = !buildConfig || buildConfig.buildForDevice; + const forDevice = !buildConfig || !!buildConfig.buildForDevice; outputPath = outputPath || (forDevice ? platformData.deviceBuildOutputPath : platformData.emulatorBuildOutputPath || platformData.deviceBuildOutputPath); if (!this.$fs.exists(outputPath)) { return true;