Open application only if new update is available when using tray to check for updates (#5948)

* Prevent opening of application if no new updates were downloaded when checking for updates using tray when application window is closed

Signed-off-by: Janne Savolainen <janne.savolainen@live.fi>

* Fix getting initial value of sync box

Signed-off-by: Janne Savolainen <janne.savolainen@live.fi>

* Tweak naming of variable

Signed-off-by: Janne Savolainen <janne.savolainen@live.fi>

src/behaviours/application-update/installing-update-using-topbar-button.test.tsx

@@ -73,15 +73,14 @@ describe("encourage user to update when sufficient time passed since update was
     });
 
     describe("given the update check", () => {
-      let processCheckingForUpdates: (source: string) => Promise<void>;
+      let processCheckingForUpdates: (source: string) => Promise<{ updateIsReadyToBeInstalled: boolean }>;
-      let processCheckingForUpdatesPromise: Promise<void>;
 
       beforeEach(async () => {
         processCheckingForUpdates = applicationBuilder.dis.mainDi.inject(
           processCheckingForUpdatesInjectable,
         );
 
-        processCheckingForUpdatesPromise = processCheckingForUpdates("irrelevant");
+        processCheckingForUpdates("irrelevant");
       });
 
       describe("when update downloaded", () => {
@@ -94,7 +93,6 @@ describe("encourage user to update when sufficient time passed since update was
           });
 
           await downloadPlatformUpdateMock.resolve({ downloadWasSuccessful: true });
-          await processCheckingForUpdatesPromise;
 
           button = rendered.getByTestId("update-button");
         });
@@ -107,6 +105,11 @@ describe("encourage user to update when sufficient time passed since update was
           expect(button).toHaveAttribute("data-warning-level", "light");
         });
 
+        // TODO: Implement after starting main and renderer is separated in ApplicationBuilder
+        xit("given closing the application window, when starting the application window again, still shows the button", () => {
+          expect(button).toBeInTheDocument();
+        });
+
         describe("given some time passes, when checking for updates again", () => {
           beforeEach(() => {
             advanceFakeTime(daysToMilliseconds(2));

src/behaviours/application-update/installing-update-using-tray.test.ts

@@ -13,13 +13,14 @@ import type { AsyncFnMock } from "@async-fn/jest";
 import asyncFn from "@async-fn/jest";
 import type { DownloadPlatformUpdate } from "../../main/application-update/download-platform-update/download-platform-update.injectable";
 import downloadPlatformUpdateInjectable from "../../main/application-update/download-platform-update/download-platform-update.injectable";
-import showApplicationWindowInjectable from "../../main/start-main-application/lens-window/show-application-window.injectable";
+import closeAllWindowsInjectable from "../../main/start-main-application/lens-window/hide-all-windows/close-all-windows.injectable";
+import applicationWindowInjectable from "../../main/start-main-application/lens-window/application-window/application-window.injectable";
+import type { LensWindow } from "../../main/start-main-application/lens-window/application-window/lens-window-injection-token";
 
 describe("installing update using tray", () => {
   let applicationBuilder: ApplicationBuilder;
   let checkForPlatformUpdatesMock: AsyncFnMock<CheckForPlatformUpdates>;
   let downloadPlatformUpdateMock: AsyncFnMock<DownloadPlatformUpdate>;
-  let showApplicationWindowMock: jest.Mock;
 
   beforeEach(() => {
     applicationBuilder = getApplicationBuilder();
@@ -27,9 +28,6 @@ describe("installing update using tray", () => {
     applicationBuilder.beforeApplicationStart(({ mainDi }) => {
       checkForPlatformUpdatesMock = asyncFn();
       downloadPlatformUpdateMock = asyncFn();
-      showApplicationWindowMock = jest.fn();
-
-      mainDi.override(showApplicationWindowInjectable, () => showApplicationWindowMock);
 
       mainDi.override(
         checkForPlatformUpdatesInjectable,
@@ -61,15 +59,77 @@ describe("installing update using tray", () => {
       expect(applicationBuilder.tray.get("install-update")).toBeNull();
     });
 
-    describe("when user checks for updates using tray", () => {
+    describe("given all application windows are closed, when checking for updates", () => {
-      let processCheckingForUpdatesPromise: Promise<void>;
+      let applicationWindow: LensWindow;
+      let closeAllWindows: () => void;
 
+      beforeEach(() => {
+        const mainDi = applicationBuilder.dis.mainDi;
+
+        closeAllWindows = mainDi.inject(closeAllWindowsInjectable);
+
+        applicationWindow = mainDi.inject(applicationWindowInjectable);
+
+        closeAllWindows();
+
+        applicationBuilder.tray.click("check-for-updates");
+      });
+
+      describe("when check for update resolves with new update", () => {
         beforeEach(async () => {
-        processCheckingForUpdatesPromise = applicationBuilder.tray.click("check-for-updates");
+          await checkForPlatformUpdatesMock.resolve({
+            updateWasDiscovered: true,
+            version: "some-version",
+          });
         });
 
         it("does not show application window yet", () => {
-        expect(showApplicationWindowMock).not.toHaveBeenCalled();
+          expect(applicationWindow.isVisible).toBe(false);
+        });
+
+        describe("when download of update resolves with success", () => {
+          beforeEach(async () => {
+
+            await downloadPlatformUpdateMock.resolve({ downloadWasSuccessful: true });
+          });
+
+          it("shows the application window", () => {
+            expect(applicationWindow.isVisible).toBe(true);
+          });
+
+          it("given closing application window again and checking for updates again using tray, when check resolves with same version that was earlier downloaded, shows the application window", async () => {
+            closeAllWindows();
+
+            applicationBuilder.tray.click("check-for-updates");
+
+            await checkForPlatformUpdatesMock.resolve({
+              updateWasDiscovered: true,
+              version: "some-version",
+            });
+
+            expect(applicationWindow.isVisible).toBe(true);
+          });
+        });
+
+        it("when download of update resolves with failure, does not show the application window", async () => {
+          await downloadPlatformUpdateMock.resolve({ downloadWasSuccessful: false });
+
+          expect(applicationWindow.isVisible).toBe(false);
+        });
+      });
+
+      it("when process resolves without new update, does not show the application window", async () => {
+        await checkForPlatformUpdatesMock.resolve({
+          updateWasDiscovered: false,
+        });
+
+        expect(applicationWindow.isVisible).toBe(false);
+      });
+    });
+
+    describe("when user checks for updates using tray", () => {
+      beforeEach(() => {
+        applicationBuilder.tray.click("check-for-updates");
       });
 
       it("user cannot check for updates again", () => {
@@ -97,12 +157,6 @@ describe("installing update using tray", () => {
           await checkForPlatformUpdatesMock.resolve({
             updateWasDiscovered: false,
           });
-
-          await processCheckingForUpdatesPromise;
-        });
-
-        it("shows application window", () => {
-          expect(showApplicationWindowMock).toHaveBeenCalled();
         });
 
         it("user cannot install update", () => {
@@ -132,12 +186,6 @@ describe("installing update using tray", () => {
             updateWasDiscovered: true,
             version: "some-version",
           });
-
-          await processCheckingForUpdatesPromise;
-        });
-
-        it("shows application window", () => {
-          expect(showApplicationWindowMock).toHaveBeenCalled();
         });
 
         it("user cannot check for updates again yet", () => {

src/behaviours/application-update/installing-update.test.ts

@@ -63,7 +63,7 @@ describe("installing update", () => {
 
   describe("when started", () => {
     let rendered: RenderResult;
-    let processCheckingForUpdates: (source: string) => Promise<void>;
+    let processCheckingForUpdates: (source: string) => Promise<{ updateIsReadyToBeInstalled: boolean }>;
 
     beforeEach(async () => {
       rendered = await applicationBuilder.render();
@@ -84,10 +84,8 @@ describe("installing update", () => {
     });
 
     describe("when user checks for updates", () => {
-      let processCheckingForUpdatesPromise: Promise<void>;
+      beforeEach(() => {
-
+        processCheckingForUpdates("irrelevant");
-      beforeEach(async () => {
-        processCheckingForUpdatesPromise = processCheckingForUpdates("irrelevant");
       });
 
       it("checks for updates", () => {
@@ -112,8 +110,6 @@ describe("installing update", () => {
           await checkForPlatformUpdatesMock.resolve({
             updateWasDiscovered: false,
           });
-
-          await processCheckingForUpdatesPromise;
         });
 
         it("shows tray icon for normal", () => {
@@ -137,8 +133,6 @@ describe("installing update", () => {
             updateWasDiscovered: true,
             version: "some-version",
           });
-
-          await processCheckingForUpdatesPromise;
         });
 
         it("starts downloading the update", () => {
@@ -243,7 +237,6 @@ describe("installing update", () => {
                   "/some-static-files-directory/icons/trayIconCheckingForUpdatesTemplate.png",
                 );
               });
-
             });
           });
         });

src/behaviours/application-update/periodical-checking-of-updates.test.ts

@@ -7,8 +7,6 @@ import { getApplicationBuilder } from "../../renderer/components/test-utils/get-
 import type { RenderResult } from "@testing-library/react";
 import electronUpdaterIsActiveInjectable from "../../main/electron-app/features/electron-updater-is-active.injectable";
 import publishIsConfiguredInjectable from "../../main/application-update/publish-is-configured.injectable";
-import type { AsyncFnMock } from "@async-fn/jest";
-import asyncFn from "@async-fn/jest";
 import processCheckingForUpdatesInjectable from "../../main/application-update/check-for-updates/process-checking-for-updates.injectable";
 import periodicalCheckForUpdatesInjectable from "../../main/application-update/periodical-check-for-updates/periodical-check-for-updates.injectable";
 import { advanceFakeTime, useFakeTime } from "../../common/test-utils/use-fake-time";
@@ -17,7 +15,7 @@ const ENOUGH_TIME = 1000 * 60 * 60 * 2;
 
 describe("periodical checking of updates", () => {
   let applicationBuilder: ApplicationBuilder;
-  let processCheckingForUpdatesMock: AsyncFnMock<() => Promise<void>>;
+  let processCheckingForUpdatesMock: jest.Mock;
 
   beforeEach(() => {
     useFakeTime("2015-10-21T07:28:00Z");
@@ -28,7 +26,7 @@ describe("periodical checking of updates", () => {
       mainDi.unoverride(periodicalCheckForUpdatesInjectable);
       mainDi.permitSideEffects(periodicalCheckForUpdatesInjectable);
 
-      processCheckingForUpdatesMock = asyncFn();
+      processCheckingForUpdatesMock = jest.fn();
 
       mainDi.override(
         processCheckingForUpdatesInjectable,

src/behaviours/application-update/selection-of-update-stability.test.ts

@@ -67,7 +67,7 @@ describe("selection of update stability", () => {
 
   describe("when started", () => {
     let rendered: RenderResult;
-    let processCheckingForUpdates: (source: string) => Promise<void>;
+    let processCheckingForUpdates: (source: string) => Promise<{ updateIsReadyToBeInstalled: boolean }>;
 
     beforeEach(async () => {
       rendered = await applicationBuilder.render();

src/main/application-update/check-for-updates-tray-item.injectable.ts

@@ -59,9 +59,11 @@ const checkForUpdatesTrayItemInjectable = getInjectable({
 
       click: pipeline(
         async () => {
-          await processCheckingForUpdates("tray");
+          const { updateIsReadyToBeInstalled } = await processCheckingForUpdates("tray");
 
+          if (updateIsReadyToBeInstalled) {
             await showApplicationWindow();
+          }
         },
 
         withErrorLoggingFor(() => "[TRAY]: Checking for updates failed."),

src/main/application-update/check-for-updates/process-checking-for-updates.injectable.ts

@@ -9,7 +9,6 @@ import discoveredUpdateVersionInjectable from "../../../common/application-updat
 import { runInAction } from "mobx";
 import downloadUpdateInjectable from "../download-update/download-update.injectable";
 import checkForUpdatesStartingFromChannelInjectable from "./check-for-updates-starting-from-channel.injectable";
-import withOrphanPromiseInjectable from "../../../common/utils/with-orphan-promise/with-orphan-promise.injectable";
 import emitEventInjectable from "../../../common/app-event-bus/emit-event.injectable";
 import { getCurrentDateTime } from "../../../common/utils/date/get-current-date-time";
 
@@ -22,7 +21,6 @@ const processCheckingForUpdatesInjectable = getInjectable({
     const checkingForUpdatesState = di.inject(updatesAreBeingDiscoveredInjectable);
     const discoveredVersionState = di.inject(discoveredUpdateVersionInjectable);
     const checkForUpdatesStartingFromChannel = di.inject(checkForUpdatesStartingFromChannelInjectable);
-    const withOrphanPromise = di.inject(withOrphanPromiseInjectable);
     const emitEvent = di.inject(emitEventInjectable);
 
     return async (source: string) => {
@@ -44,7 +42,7 @@ const processCheckingForUpdatesInjectable = getInjectable({
           checkingForUpdatesState.set(false);
         });
 
-        return;
+        return { updateIsReadyToBeInstalled: false };
       }
 
       const { version, actualUpdateChannel } = result;
@@ -56,7 +54,7 @@ const processCheckingForUpdatesInjectable = getInjectable({
           checkingForUpdatesState.set(false);
         });
 
-        return;
+        return { updateIsReadyToBeInstalled: true };
       }
 
       emitEvent({
@@ -74,7 +72,9 @@ const processCheckingForUpdatesInjectable = getInjectable({
         checkingForUpdatesState.set(false);
       });
 
-      withOrphanPromise(async () => await downloadUpdate())();
+      const { downloadWasSuccessful } = await downloadUpdate();
+
+      return { updateIsReadyToBeInstalled: downloadWasSuccessful };
     };
   },
 });

src/renderer/utils/sync-box/provide-initial-values-for-sync-boxes.injectable.ts

@@ -7,6 +7,10 @@ import { beforeFrameStartsInjectionToken } from "../../before-frame-starts/befor
 import syncBoxInitialValueChannelInjectable from "../../../common/utils/sync-box/sync-box-initial-value-channel.injectable";
 import createSyncBoxStateInjectable from "../../../common/utils/sync-box/sync-box-state.injectable";
 import { requestFromChannelInjectionToken } from "../../../common/utils/channel/request-from-channel-injection-token";
+import { runInAction } from "mobx";
+import type { SyncBox } from "../../../common/utils/sync-box/sync-box-injection-token";
+import { syncBoxInjectionToken } from "../../../common/utils/sync-box/sync-box-injection-token";
+import assert from "assert";
 
 const provideInitialValuesForSyncBoxesInjectable = getInjectable({
   id: "provide-initial-values-for-sync-boxes",
@@ -14,14 +18,24 @@ const provideInitialValuesForSyncBoxesInjectable = getInjectable({
   instantiate: (di) => {
     const requestFromChannel = di.inject(requestFromChannelInjectionToken);
     const syncBoxInitialValueChannel = di.inject(syncBoxInitialValueChannelInjectable);
-    const setSyncBoxState = (id: string, state: any) => di.inject(createSyncBoxStateInjectable, id).set(state);
+
+    const syncBoxes = di.injectMany(syncBoxInjectionToken);
+
+    const setSyncBoxState = (syncBox: SyncBox<any>, state: any) =>
+      di.inject(createSyncBoxStateInjectable, syncBox.id).set(state);
 
     return {
       run: async () => {
         const initialValues = await requestFromChannel(syncBoxInitialValueChannel);
 
+        runInAction(() => {
           initialValues.forEach(({ id, value }) => {
-          setSyncBoxState(id, value);
+            const syncBox = syncBoxes.find((box) => box.id === id);
+
+            assert(syncBox);
+
+            setSyncBoxState(syncBox, value);
+          });
         });
       },
     };