fix: Improved Electron auto update UX (#2925)

* More auto update polish.

* Added more tests.

Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>

Composer/packages/client/src/components/AppUpdater/index.tsx

@@ -80,11 +80,16 @@ export const AppUpdater: React.FC<{}> = _props => {
           break;
         }
 
-        case 'update-not-available':
-          // TODO: re-enable once we have implemented explicit "check for updates"
-          // setAppUpdateStatus({ status: AppUpdaterStatus.UPDATE_UNAVAILABLE });
-          // setAppUpdateShowing(true);
+        case 'update-not-available': {
+          const explicit = payload;
+          if (explicit) {
+            // the user has explicitly checked for an update via the Help menu;
+            // we should display some UI feedback if there are no updates available
+            setAppUpdateStatus({ status: AppUpdaterStatus.UPDATE_UNAVAILABLE });
+            setAppUpdateShowing(true);
+          }
           break;
+        }
 
         case 'update-downloaded':
           setAppUpdateStatus({ status: AppUpdaterStatus.UPDATE_SUCCEEDED });
@@ -188,7 +193,7 @@ export const AppUpdater: React.FC<{}> = _props => {
       default:
         return undefined;
     }
-  }, [status, progressPercent]);
+  }, [status, progressPercent, error]);
 
   const footer = useMemo(() => {
     switch (status) {

Composer/packages/electron-server/__tests__/appMenu.test.ts

@@ -52,7 +52,7 @@ describe('App menu', () => {
 
     // Help
     expect(menuTemplate[4].label).toBe('Help');
-    expect(menuTemplate[4].submenu.length).toBe(10);
+    expect(menuTemplate[4].submenu.length).toBe(12);
 
     expect(mockSetApplicationMenu).toHaveBeenCalledTimes(1);
   });
@@ -87,7 +87,7 @@ describe('App menu', () => {
 
     // Help
     expect(menuTemplate[5].label).toBe('Help');
-    expect(menuTemplate[5].submenu.length).toBe(10);
+    expect(menuTemplate[5].submenu.length).toBe(12);
 
     expect(mockSetApplicationMenu).toHaveBeenCalledTimes(1);
   });

Composer/packages/electron-server/__tests__/appUpdater.test.ts

@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 const mockAutoUpdater = {
+  allowDowngrade: false,
   checkForUpdates: jest.fn(),
   downloadUpdate: jest.fn(),
   on: jest.fn(),
@@ -14,24 +15,59 @@ jest.mock('electron-updater', () => ({
   },
 }));
 
+const mockGetVersion = jest.fn(() => '0.0.1');
+jest.mock('electron', () => ({
+  app: {
+    getVersion: () => mockGetVersion(),
+  },
+}));
+
 import { AppUpdater } from '../src/appUpdater';
 
 describe('App updater', () => {
   let appUpdater: AppUpdater;
   beforeEach(() => {
-    appUpdater = new AppUpdater();
+    mockAutoUpdater.allowDowngrade = false;
+    appUpdater = AppUpdater.getInstance();
+    (appUpdater as any).checkingForUpdate = false;
+    (appUpdater as any).downloadingUpdate = false;
     mockAutoUpdater.checkForUpdates.mockClear();
     mockAutoUpdater.checkForUpdates.mockClear();
     mockAutoUpdater.downloadUpdate.mockClear();
     mockAutoUpdater.on.mockClear();
     mockAutoUpdater.quitAndInstall.mockClear();
     mockAutoUpdater.setFeedURL.mockClear();
+    mockGetVersion.mockClear();
   });
 
-  it('should check for updates', () => {
-    appUpdater.checkForUpdates();
+  it('should check for updates from the nightly repo', () => {
+    const getSettingsSpy = jest.spyOn(appUpdater as any, 'getSettings').mockReturnValue({ useNightly: true });
+    appUpdater.checkForUpdates(true);
+
+    expect(mockAutoUpdater.checkForUpdates).toHaveBeenCalled();
+    expect((appUpdater as any).explicitCheck).toBe(true);
+    expect(mockAutoUpdater.setFeedURL).toHaveBeenCalledWith({
+      provider: 'github',
+      repo: 'BotFramework-Composer-Nightlies',
+      owner: 'microsoft',
+      vPrefixedTagName: true,
+    });
+    getSettingsSpy.mockRestore();
+  });
+
+  it('should check for updates from the stable repo', () => {
+    const getSettingsSpy = jest.spyOn(appUpdater as any, 'getSettings').mockReturnValue({ useNightly: false });
+    appUpdater.checkForUpdates(true);
 
     expect(mockAutoUpdater.checkForUpdates).toHaveBeenCalled();
+    expect((appUpdater as any).explicitCheck).toBe(true);
+    expect(mockAutoUpdater.setFeedURL).toHaveBeenCalledWith({
+      provider: 'github',
+      repo: 'BotFramework-Composer',
+      owner: 'microsoft',
+      vPrefixedTagName: true,
+    });
+    getSettingsSpy.mockRestore();
   });
 
   it('should not check for updates if it is already checking for an update', () => {
@@ -48,6 +84,34 @@ describe('App updater', () => {
     expect(mockAutoUpdater.checkForUpdates).not.toHaveBeenCalled();
   });
 
+  it('should not allow a downgrade when checking for updates from nightly to (stable or nightly)', () => {
+    mockGetVersion.mockReturnValueOnce('0.0.1-nightly.12345.abcdef');
+    mockAutoUpdater.allowDowngrade = true;
+    appUpdater.checkForUpdates();
+
+    expect(mockAutoUpdater.allowDowngrade).toBe(false);
+  });
+
+  it('should not allow a downgrade when checking for updates from stable to stable', () => {
+    mockGetVersion.mockReturnValueOnce('0.0.1');
+    const getSettingsSpy = jest.spyOn(appUpdater as any, 'getSettings').mockReturnValue({ useNightly: false });
+    mockAutoUpdater.allowDowngrade = true;
+    appUpdater.checkForUpdates();
+
+    expect(mockAutoUpdater.allowDowngrade).toBe(false);
+    getSettingsSpy.mockRestore();
+  });
+
+  it('should allow a downgrade when checking for updates from stable to nightly', () => {
+    mockGetVersion.mockReturnValueOnce('0.0.1');
+    const getSettingsSpy = jest.spyOn(appUpdater as any, 'getSettings').mockReturnValue({ useNightly: true });
+    mockAutoUpdater.allowDowngrade = false;
+    appUpdater.checkForUpdates();
+
+    expect(mockAutoUpdater.allowDowngrade).toBe(true);
+    getSettingsSpy.mockRestore();
+  });
+
   it('should download an update', () => {
     appUpdater.downloadUpdate();
 
@@ -86,11 +150,14 @@ describe('App updater', () => {
 
   it('should handle no available update', () => {
     const emitSpy = jest.spyOn(appUpdater, 'emit');
+    const explicit = true;
     (appUpdater as any).checkingForUpdate = true;
+    (appUpdater as any).explicitCheck = explicit;
     (appUpdater as any).onUpdateNotAvailable('update info');
 
     expect((appUpdater as any).checkingForUpdate).toBe(false);
-    expect(emitSpy).toHaveBeenCalledWith('update-not-available', 'update info');
+    expect((appUpdater as any).explicitCheck).toBe(false);
+    expect(emitSpy).toHaveBeenCalledWith('update-not-available', explicit);
   });
 
   it('should handle download progress', () => {

Composer/packages/electron-server/package.json

@@ -59,6 +59,7 @@
     "debug": "4.1.1",
     "electron-updater": "4.2.5",
     "fix-path": "^3.0.0",
-    "lodash": "^4.17.15"
+    "lodash": "^4.17.15",
+    "semver": "7.3.2"
   }
 }

Composer/packages/electron-server/src/appMenu.ts

@@ -4,6 +4,7 @@
 import { app, dialog, Menu, MenuItemConstructorOptions, shell } from 'electron';
 
 import { isMac } from './utility/platform';
+import { AppUpdater } from './appUpdater';
 
 function getAppMenu(): MenuItemConstructorOptions[] {
   if (isMac()) {
@@ -128,6 +129,13 @@ export function initAppMenu() {
           },
         },
         { type: 'separator' },
+        {
+          label: 'Check for Updates',
+          click: () => {
+            AppUpdater.getInstance().checkForUpdates(true);
+          },
+        },
+        { type: 'separator' },
         {
           label: 'About',
           click: async () => {

Composer/packages/electron-server/src/appUpdater.ts

@@ -4,28 +4,32 @@
 import { EventEmitter } from 'events';
 
 import { autoUpdater, UpdateInfo } from 'electron-updater';
+import { app } from 'electron';
+import { prerelease as isNightly } from 'semver';
 
 import logger from './utility/logger';
 const log = logger.extend('app-updater');
 
+interface AppUpdaterSettings {
+  autoDownload: boolean;
+  useNightly: boolean;
+}
+
+let appUpdater: AppUpdater | undefined;
 export class AppUpdater extends EventEmitter {
   private checkingForUpdate = false;
   private downloadingUpdate = false;
+  private explicitCheck = false;
 
   constructor() {
     super();
 
-    const settings = { autoDownload: false, useNightly: false }; // TODO: implement and load these settings from disk / memory
+    const settings = this.getSettings();
     autoUpdater.allowDowngrade = false;
     autoUpdater.allowPrerelease = true;
     autoUpdater.autoDownload = settings.autoDownload;
-    autoUpdater.setFeedURL({
-      provider: 'github',
-      repo: 'BotFramework-Composer',
-      owner: 'microsoft',
-    });
 
-    autoUpdater.on('error', this.onError);
+    autoUpdater.on('error', this.onError.bind(this));
     autoUpdater.on('checking-for-update', this.onCheckingForUpdate.bind(this));
     autoUpdater.on('update-available', this.onUpdateAvailable.bind(this));
     autoUpdater.on('update-not-available', this.onUpdateNotAvailable.bind(this));
@@ -34,8 +38,23 @@ export class AppUpdater extends EventEmitter {
     logger('Initialized');
   }
 
-  public checkForUpdates() {
+  public static getInstance(): AppUpdater {
+    if (!appUpdater) {
+      appUpdater = new AppUpdater();
+    }
+    return appUpdater;
+  }
+
+  /**
+   * Checks GitHub for Composer updates
+   * @param explicit If true, the user explicitly checked for an update via the Help menu,
+   * and we will show UI if there are no available updates.
+   */
+  public checkForUpdates(explicit = false) {
     if (!(this.checkingForUpdate || this.downloadingUpdate)) {
+      this.explicitCheck = explicit;
+      this.setFeedURL();
+      this.determineUpdatePath();
       autoUpdater.checkForUpdates();
     }
   }
@@ -72,8 +91,8 @@ export class AppUpdater extends EventEmitter {
 
   private onUpdateNotAvailable(updateInfo: UpdateInfo) {
     log('Update not available: %O', updateInfo);
-    this.checkingForUpdate = false;
-    this.emit('update-not-available', updateInfo);
+    this.emit('update-not-available', this.explicitCheck);
+    this.resetToIdle();
   }
 
   private onDownloadProgress(progress: any) {
@@ -91,5 +110,59 @@ export class AppUpdater extends EventEmitter {
     log('Resetting to idle...');
     this.checkingForUpdate = false;
     this.downloadingUpdate = false;
+    this.explicitCheck = false;
+  }
+
+  private setFeedURL() {
+    const settings = this.getSettings();
+    if (settings.useNightly) {
+      log('Updates set to be retrieved from nightly repo.');
+      autoUpdater.setFeedURL({
+        provider: 'github',
+        repo: 'BotFramework-Composer-Nightlies',
+        owner: 'microsoft',
+        vPrefixedTagName: true, // whether our release tags are prefixed with v or not
+      });
+    } else {
+      log('Updates set to be retrieved from stable repo.');
+      autoUpdater.setFeedURL({
+        provider: 'github',
+        repo: 'BotFramework-Composer',
+        owner: 'microsoft',
+        vPrefixedTagName: true,
+      });
+    }
+  }
+
+  private determineUpdatePath() {
+    const currentVersion = app.getVersion();
+
+    // The following paths don't need to allow downgrade:
+    //    nightly -> stable     (1.0.1-nightly.x.x -> 1.0.2)
+    //    nightly -> nightly    (1.0.1-nightly.x.x -> 1.0.1-nightly.y.x)
+    if (isNightly(currentVersion)) {
+      log(`Updating from nightly to (stable | nightly). Not allowing downgrade.`);
+      autoUpdater.allowDowngrade = false;
+      return;
+    }
+
+    // https://github.com/npm/node-semver/blob/v7.3.2/classes/semver.js#L127
+    // The following path needs to allow downgrade to work:
+    //    stable -> nightly     (1.0.1 -> 1.0.1-nightly.x.x)
+    const settings = this.getSettings();
+    if (!isNightly(currentVersion) && settings.useNightly) {
+      log(`Updating from stable to nightly. Allowing downgrade.`);
+      autoUpdater.allowDowngrade = true;
+      return;
+    }
+
+    // stable -> stable
+    log('Updating from stable to stable. Not allowing downgrade.');
+    autoUpdater.allowDowngrade = false;
+  }
+
+  private getSettings(): AppUpdaterSettings {
+    // TODO: replace with actual implementation that fetches settings from disk
+    return { autoDownload: false, useNightly: true };
   }
 }

Composer/packages/electron-server/src/main.ts

@@ -63,7 +63,7 @@ function initializeAppUpdater() {
   log('Initializing app updater...');
   const mainWindow = ElectronWindow.getInstance().browserWindow;
   if (mainWindow) {
-    const appUpdater = new AppUpdater();
+    const appUpdater = AppUpdater.getInstance();
     appUpdater.on('update-available', (updateInfo: UpdateInfo) => {
       // TODO: if auto/silent download is enabled in settings, don't send this event.
       // instead, just download silently
@@ -72,8 +72,8 @@ function initializeAppUpdater() {
     appUpdater.on('progress', progress => {
       mainWindow.webContents.send('app-update', 'progress', progress);
     });
-    appUpdater.on('update-not-available', () => {
-      mainWindow.webContents.send('app-update', 'update-not-available');
+    appUpdater.on('update-not-available', (explicitCheck: boolean) => {
+      mainWindow.webContents.send('app-update', 'update-not-available', explicitCheck);
     });
     appUpdater.on('update-downloaded', () => {
       mainWindow.webContents.send('app-update', 'update-downloaded');

Composer/yarn.lock

@@ -16167,6 +16167,11 @@ semver@7.0.0:
   resolved "https://registry.yarnpkg.com/semver/-/semver-7.0.0.tgz#5f3ca35761e47e05b206c6daff2cf814f0316b8e"
   integrity sha512-+GB6zVA9LWh6zovYQLALHwv5rb2PHGlJi3lfiqIHxR0uuwCgefcOJc59v9fv1w8GbStwxuuqqAjI9NMAOOgq1A==
 
+semver@7.3.2:
+  version "7.3.2"
+  resolved "https://registry.yarnpkg.com/semver/-/semver-7.3.2.tgz#604962b052b81ed0786aae84389ffba70ffd3938"
+  integrity sha512-OrOb32TeeambH6UrhtShmF7CRDqhL6/5XpPNp2DuRH6+9QLw/orhp72j87v8Qa1ScDkvrrBNpZcDejAirJmfXQ==
+
 semver@^5.0.1:
   version "5.7.0"
   resolved "https://registry.yarnpkg.com/semver/-/semver-5.7.0.tgz#790a7cf6fea5459bac96110b29b60412dc8ff96b"