Merge pull request #164 from microsoft/cli-update-check-refactor

Check for version updates no more than 1x per day, based on timestamp in config

Author: munozemilio

packages/cli/scripts/postinstall.js

@@ -36,6 +36,7 @@ const getUserConfig = async () => {
  const promptTelemetry = async () => {
   try {
     const userConfig = await getUserConfig()
+    userConfig.lastVersionCheck = new Date()
     if (userConfig.telemetry === null) {
       const disableTelemetry = await cli.prompt(chalk.red('Telemetry is disabled. Would you like to opt in? Only command and flags usage will be sent. (Y/N)'))
       if (disableTelemetry === 'Y' || disableTelemetry === 'y') {
@@ -53,7 +54,6 @@ const getUserConfig = async () => {
       }
 
       await fs.mkdirp(pathToConfigJson)
-      
       await fs.writeFile(path.join(pathToConfigJson, 'config.json'), JSON.stringify(userConfig, null, 2))
     }
   /* tslint:disable:no-unused */

packages/cli/src/hooks/init/inithook.ts

@@ -7,31 +7,64 @@ const latestVersion = require('latest-version')
 const semver = require('semver')
 
 const hook: Hook<'init'> = async function (opts) {
-  // Look for updates
-  try {
-    const latest = await latestVersion(opts.config.name, {version: `>${opts.config.version}`})
-    if (semver.gt(latest, opts.config.version)) {
-      this.log('Update available ')
-      this.log('     Run ')
-      this.log(`npm i -g ${opts.config.name} `)
-    }
-  /* tslint:disable:no-unused */
-  } catch (err) {
-      // swallow the exception; we don't want to crash the app
-      // on a failed attempt to check version
+  // get config settings
+  let userConfig: any
+  const curDateTime = new Date()
+  const configFileExists = fs.existsSync(path.join(this.config.configDir, 'config.json'))
+
+  const writeUserConfig = async (userconfig: any) => {
+    await fs.mkdirp(this.config.configDir)
+    await fs.writeFile(path.join(this.config.configDir, 'config.json'), JSON.stringify(userconfig, null, 2))
   }
 
-  // Ensure telemetry is set
   try {
-    let userConfig: any = ''
-    if (fs.existsSync(path.join(this.config.configDir, 'config.json'))) {
+    // if config file exists, load settings
+    if (configFileExists) {
       userConfig = await fs.readJSON(path.join(this.config.configDir, 'config.json'))
     } else {
+      // otherwise create in-memory config
       userConfig = {
         telemetry: null,
+        lastVersionCheck: null
+      }
+    }
+
+    const checkForUpdate = async () => {
+      const latest = await latestVersion(opts.config.name, {version: `>${opts.config.version}`})
+      if (semver.gt(latest, opts.config.version)) {
+        this.log('Update available ')
+        this.log('     Run ')
+        this.log(`npm i -g ${opts.config.name} `)
       }
     }
 
+    const updateUserConfig = async (curVersionCheck: Date) => {
+      userConfig.lastVersionCheck = curVersionCheck
+      await writeUserConfig(userConfig)
+    }
+
+    const isToday = (dateObj: Date | null, today: Date) => {
+      return dateObj && dateObj.getDate() === today.getDate() &&
+        dateObj.getMonth() === today.getMonth() &&
+        dateObj.getFullYear() === today.getFullYear()
+    }
+
+    // if there's no timestamp in config, create one and check for updates
+    // if there is a timestamp in config and it's not from today, check for updates
+    const lastCheck = userConfig.lastVersionCheck ? new Date(userConfig.lastVersionCheck) : null
+    if (!isToday(lastCheck, curDateTime)) {
+      await checkForUpdate()
+      await updateUserConfig(curDateTime)
+    }
+
+  /* tslint:disable:no-unused */
+  } catch (err) {
+      // swallow the exception; we don't want to crash the app
+      // on a failed attempt to check version
+  }
+
+  // Ensure telemetry is set
+  try {
     if (userConfig.telemetry === null) {
       const disableTelemetry = await cli.prompt(chalk.red('Telemetry is disabled. Would you like to opt in?. Only command and flags usage will be sent. (Y/N)'))
       if (disableTelemetry === 'Y' || disableTelemetry === 'y') {
@@ -48,9 +81,7 @@ const hook: Hook<'init'> = async function (opts) {
         this.log(chalk.blue('bf config:telemetry:enable'))
       }
 
-      await fs.mkdirp(this.config.configDir)
-
-      await fs.writeFileSync(path.join(this.config.configDir, 'config.json'), JSON.stringify(userConfig, null, 2))
+      await writeUserConfig(userConfig)
     }
 
     this.config.pjson.telemetry = userConfig.telemetry

packages/cli/test/commands/inithook.test.ts

@@ -5,6 +5,7 @@ const path = require('path')
 const semver = require('semver')
 const os = require('os')
 
+const lastversioncheck = path.join(__dirname, '../fixtures/lastversioncheck')
 const upgradeavailable = path.join(__dirname, '../fixtures/upgradeavailable')
 const rootTelemetryNull = path.join(__dirname, '../fixtures/telemetrynull')
 
@@ -89,13 +90,13 @@ describe('Check if telemetry is not changed if config is not null', () => {
 })
 
 describe('Update available to stdout', () => {
-  before(function() {
+  beforeEach(function() {
     // runs before all tests in this block
     fs.mkdirSync(pathToConfigJsonUpdate)
-    fs.writeFileSync(path.join(pathToConfigJsonUpdate, 'config.json'), JSON.stringify({telemetry: true,}, null, 2))
+    fs.writeFileSync(path.join(pathToConfigJsonUpdate, 'config.json'), JSON.stringify({telemetry: true}, null, 2))
   });
 
-  after(function() {
+  afterEach(function() {
     // runs after all tests in this block
     fs.removeSync(pathToConfigJsonUpdate)
   });
@@ -110,11 +111,50 @@ describe('Update available to stdout', () => {
     })
     .it('it should output to stdout if update is available')
 
-    test
+  test
     .loadConfig({root: upgradeavailable})
     .stub(semver, 'gt', () => false)
     .stdout()
     .hook('init', {argv: ['arg']}, {root: upgradeavailable})
     .do(output => expect(output.stdout).to.equal(''))
     .it('it should not output anything if no update is available')
+
+  test
+    .loadConfig({root: lastversioncheck})
+    .stub(semver, 'gt', () => true)
+    .stdout()
+    .hook('init', {argv: ['arg']}, {root: lastversioncheck})
+    .do(output => {
+      expect(output.stdout).to.contain('Update available')
+    })
+    .it('it should write to message stdout if update is available and last checked timestamp is expired')
+
 })
+
+describe('bypass version update if it\'s already been checked today', async () => {
+  beforeEach(async () => {
+    // runs before all tests in this block
+    const today = new Date()
+    let userConfig = await fs.readJSON(path.join(lastversioncheck, 'package.json'))
+    userConfig.lastversioncheck = today
+    fs.writeFileSync(path.join(lastversioncheck, 'package.json'), JSON.stringify(userConfig, null, 2))
+    fs.mkdirSync(pathToConfigJsonUpdate)
+    fs.writeFileSync(path.join(pathToConfigJsonUpdate, 'config.json'), JSON.stringify({telemetry: true}, null, 2))
+  });
+
+  afterEach(function() {
+    // runs after all tests in this block
+    fs.removeSync(pathToConfigJsonUpdate)
+  });
+
+  test
+    .loadConfig({root: lastversioncheck})
+    .stub(semver, 'gt', () => true)
+    .stdout()
+    .hook('init', {argv: ['arg']}, {root: lastversioncheck})
+    .do(output => {
+      expect(output.stdout).to.contain('')
+    })
+    .it('it should not output anything if last upgrade check was today')
+
+});

packages/cli/test/fixtures/lastversioncheck/package.json

@@ -0,0 +1,18 @@
+{
+  "name": "botbuilder",
+  "version": "1.0.0",
+  "lastVersionCheck": "2019-09-23T19:27:24.987Z",
+  "files": [],
+  "oclif": {
+    "commands": "../../../src/commands",
+    "bin": "bf",
+    "plugins": [
+      "@oclif/plugin-help",
+      "@microsoft/bf-chatdown"
+    ],
+    "hooks": {
+      "init": "../../../src/hooks/init/inithook"
+    }
+  },
+  "lastversioncheck": "2019-09-27T23:36:07.377Z"
+}