Refactor the Extensions settings page (#2221)

src/renderer/bootstrap.tsx

@@ -19,6 +19,7 @@ import { filesystemProvisionerStore } from "../main/extension-filesystem";
 import { App } from "./components/app";
 import { LensApp } from "./lens-app";
 import { themeStore } from "./theme.store";
+import { ExtensionInstallationStateStore } from "./components/+extensions/extension-install.store";
 
 /**
  * If this is a development buid, wait a second to attach
@@ -50,6 +51,7 @@ export async function bootstrap(App: AppComponent) {
   await attachChromeDebugger();
   rootElem.classList.toggle("is-mac", isMac);
 
+  ExtensionInstallationStateStore.bindIpcListeners();
   extensionLoader.init();
   extensionDiscovery.init();
 

src/renderer/components/+extensions/__tests__/extensions.test.tsx

@@ -1,14 +1,15 @@
 import "@testing-library/jest-dom/extend-expect";
-import { fireEvent, render, screen, waitFor } from "@testing-library/react";
+import { fireEvent, render, waitFor } from "@testing-library/react";
 import fse from "fs-extra";
 import React from "react";
 import { extensionDiscovery } from "../../../../extensions/extension-discovery";
 import { ConfirmDialog } from "../../confirm-dialog";
-import { Notifications } from "../../notifications";
-import { ExtensionStateStore } from "../extension-install.store";
+import { ExtensionInstallationStateStore } from "../extension-install.store";
 import { Extensions } from "../extensions";
 
+jest.setTimeout(30000);
 jest.mock("fs-extra");
+jest.mock("../../notifications");
 
 jest.mock("../../../../common/utils", () => ({
   ...jest.requireActual("../../../../common/utils"),
@@ -42,62 +43,42 @@ jest.mock("../../../../extensions/extension-loader", () => ({
         isBundled: false,
         isEnabled: true
       }]
-    ])
+    ]),
+    getExtension: jest.fn(() => ({ manifest: {} })),
   }
 }));
 
-jest.mock("../../notifications", () => ({
-  ok: jest.fn(),
-  error: jest.fn(),
-  info: jest.fn()
-}));
-
 describe("Extensions", () => {
   beforeEach(() => {
-    ExtensionStateStore.resetInstance();
+    ExtensionInstallationStateStore.reset();
   });
 
   it("disables uninstall and disable buttons while uninstalling", async () => {
-    render(<><Extensions /><ConfirmDialog/></>);
+    const res = render(<><Extensions /><ConfirmDialog /></>);
 
-    expect(screen.getByText("Disable").closest("button")).not.toBeDisabled();
-    expect(screen.getByText("Uninstall").closest("button")).not.toBeDisabled();
+    expect(res.getByText("Disable").closest("button")).not.toBeDisabled();
+    expect(res.getByText("Uninstall").closest("button")).not.toBeDisabled();
 
-    fireEvent.click(screen.getByText("Uninstall"));
+    fireEvent.click(res.getByText("Uninstall"));
 
     // Approve confirm dialog
-    fireEvent.click(screen.getByText("Yes"));
+    fireEvent.click(res.getByText("Yes"));
 
-    expect(extensionDiscovery.uninstallExtension).toHaveBeenCalled();
-    expect(screen.getByText("Disable").closest("button")).toBeDisabled();
-    expect(screen.getByText("Uninstall").closest("button")).toBeDisabled();
-  });
-
-  it("displays error notification on uninstall error", () => {
-    (extensionDiscovery.uninstallExtension as any).mockImplementationOnce(() =>
-      Promise.reject()
-    );
-    render(<><Extensions /><ConfirmDialog/></>);
-
-    expect(screen.getByText("Disable").closest("button")).not.toBeDisabled();
-    expect(screen.getByText("Uninstall").closest("button")).not.toBeDisabled();
-
-    fireEvent.click(screen.getByText("Uninstall"));
-
-    // Approve confirm dialog
-    fireEvent.click(screen.getByText("Yes"));
-
-    waitFor(() => {
-      expect(screen.getByText("Disable").closest("button")).not.toBeDisabled();
-      expect(screen.getByText("Uninstall").closest("button")).not.toBeDisabled();
-      expect(Notifications.error).toHaveBeenCalledTimes(1);
+    await waitFor(() => {
+      expect(extensionDiscovery.uninstallExtension).toHaveBeenCalled();
+      expect(res.getByText("Disable").closest("button")).toBeDisabled();
+      expect(res.getByText("Uninstall").closest("button")).toBeDisabled();
+    }, {
+      timeout: 30000,
     });
   });
 
-  it("disables install button while installing", () => {
-    render(<Extensions />);
+  it("disables install button while installing", async () => {
+    const res = render(<Extensions />);
 
-    fireEvent.change(screen.getByPlaceholderText("Path or URL to an extension package", {
+    (fse.unlink as jest.MockedFunction<typeof fse.unlink>).mockReturnValue(Promise.resolve() as any);
+
+    fireEvent.change(res.getByPlaceholderText("Path or URL to an extension package", {
       exact: false
     }), {
       target: {
@@ -105,25 +86,21 @@ describe("Extensions", () => {
       }
     });
 
-    fireEvent.click(screen.getByText("Install"));
-
-    waitFor(() => {
-      expect(screen.getByText("Install").closest("button")).toBeDisabled();
-      expect(fse.move).toHaveBeenCalledWith("");
-      expect(Notifications.error).not.toHaveBeenCalled();
-    });
+    fireEvent.click(res.getByText("Install"));
+    expect(res.getByText("Install").closest("button")).toBeDisabled();
   });
 
-  it("displays spinner while extensions are loading", () => {
+  it("displays spinner while extensions are loading", async () => {
     extensionDiscovery.isLoaded = false;
-    const { container } = render(<Extensions />);
+    const res = render(<Extensions />);
 
-    expect(container.querySelector(".Spinner")).toBeInTheDocument();
+    expect(res.container.querySelector(".Spinner")).toBeInTheDocument();
+  });
 
+  it("does not display the spinner while extensions are not loading", async () => {
     extensionDiscovery.isLoaded = true;
+    const res = render(<Extensions />);
 
-    waitFor(() => 
-      expect(container.querySelector(".Spinner")).not.toBeInTheDocument()
-    );
+    expect(res.container.querySelector(".Spinner")).not.toBeInTheDocument();
   });
 });

src/renderer/components/+extensions/extension-install.store.ts

@@ -1,13 +1,218 @@
-import { observable } from "mobx";
-import { autobind, Singleton } from "../../utils";
+import { action, computed, observable } from "mobx";
+import logger from "../../../main/logger";
+import { disposer, ExtendableDisposer } from "../../utils";
+import * as uuid from "uuid";
+import { broadcastMessage } from "../../../common/ipc";
+import { ipcRenderer } from "electron";
 
-interface ExtensionState {
-  displayName: string;
-  // Possible states the extension can be
-  state: "installing" | "uninstalling";
+export enum ExtensionInstallationState {
+  INSTALLING = "installing",
+  UNINSTALLING = "uninstalling",
+  IDLE = "idle",
 }
 
-@autobind()
-export class ExtensionStateStore extends Singleton {
-  extensionState = observable.map<string, ExtensionState>();
+const Prefix = "[ExtensionInstallationStore]";
+
+export class ExtensionInstallationStateStore {
+  private static InstallingFromMainChannel = "extension-installation-state-store:install";
+  private static ClearInstallingFromMainChannel = "extension-installation-state-store:clear-install";
+  private static PreInstallIds = observable.set<string>();
+  private static UninstallingExtensions = observable.set<string>();
+  private static InstallingExtensions = observable.set<string>();
+
+  static bindIpcListeners() {
+    ipcRenderer
+      .on(ExtensionInstallationStateStore.InstallingFromMainChannel, (event, extId) => {
+        ExtensionInstallationStateStore.setInstalling(extId);
+      })
+      .on(ExtensionInstallationStateStore.ClearInstallingFromMainChannel, (event, extId) => {
+        ExtensionInstallationStateStore.clearInstalling(extId);
+      });
+  }
+
+  @action static reset() {
+    logger.warn(`${Prefix}: resetting, may throw errors`);
+    ExtensionInstallationStateStore.InstallingExtensions.clear();
+    ExtensionInstallationStateStore.UninstallingExtensions.clear();
+    ExtensionInstallationStateStore.PreInstallIds.clear();
+  }
+
+  /**
+   * Strictly transitions an extension from not installing to installing
+   * @param extId the ID of the extension
+   * @throws if state is not IDLE
+   */
+  @action static setInstalling(extId: string): void {
+    logger.debug(`${Prefix}: trying to set ${extId} as installing`);
+
+    const curState = ExtensionInstallationStateStore.getInstallationState(extId);
+
+    if (curState !== ExtensionInstallationState.IDLE) {
+      throw new Error(`${Prefix}: cannot set ${extId} as installing. Is currently ${curState}.`);
+    }
+
+    ExtensionInstallationStateStore.InstallingExtensions.add(extId);
+  }
+
+  /**
+   * Broadcasts that an extension is being installed by the main process
+   * @param extId the ID of the extension
+   */
+  static setInstallingFromMain(extId: string): void {
+    broadcastMessage(ExtensionInstallationStateStore.InstallingFromMainChannel, extId);
+  }
+
+  /**
+   * Broadcasts that an extension is no longer being installed by the main process
+   * @param extId the ID of the extension
+   */
+  static clearInstallingFromMain(extId: string): void {
+    broadcastMessage(ExtensionInstallationStateStore.ClearInstallingFromMainChannel, extId);
+  }
+
+  /**
+   * Marks the start of a pre-install phase of an extension installation. The
+   * part of the installation before the tarball has been unpacked and the ID
+   * determined.
+   * @returns a disposer which should be called to mark the end of the install phase
+   */
+  @action static startPreInstall(): ExtendableDisposer {
+    const preInstallStepId = uuid.v4();
+
+    logger.debug(`${Prefix}: starting a new preinstall phase: ${preInstallStepId}`);
+    ExtensionInstallationStateStore.PreInstallIds.add(preInstallStepId);
+
+    return disposer(() => {
+      ExtensionInstallationStateStore.PreInstallIds.delete(preInstallStepId);
+      logger.debug(`${Prefix}: ending a preinstall phase: ${preInstallStepId}`);
+    });
+  }
+
+  /**
+   * Strictly transitions an extension from not uninstalling to uninstalling
+   * @param extId the ID of the extension
+   * @throws if state is not IDLE
+   */
+  @action static setUninstalling(extId: string): void {
+    logger.debug(`${Prefix}: trying to set ${extId} as uninstalling`);
+
+    const curState = ExtensionInstallationStateStore.getInstallationState(extId);
+
+    if (curState !== ExtensionInstallationState.IDLE) {
+      throw new Error(`${Prefix}: cannot set ${extId} as uninstalling. Is currently ${curState}.`);
+    }
+
+    ExtensionInstallationStateStore.UninstallingExtensions.add(extId);
+  }
+
+  /**
+   * Strictly clears the INSTALLING state of an extension
+   * @param extId The ID of the extension
+   * @throws if state is not INSTALLING
+   */
+  @action static clearInstalling(extId: string): void {
+    logger.debug(`${Prefix}: trying to clear ${extId} as installing`);
+
+    const curState = ExtensionInstallationStateStore.getInstallationState(extId);
+
+    switch (curState) {
+      case ExtensionInstallationState.INSTALLING:
+        return void ExtensionInstallationStateStore.InstallingExtensions.delete(extId);
+      default:
+        throw new Error(`${Prefix}: cannot clear INSTALLING state for ${extId}, it is currently ${curState}`);
+    }
+  }
+
+  /**
+   * Strictly clears the UNINSTALLING state of an extension
+   * @param extId The ID of the extension
+   * @throws if state is not UNINSTALLING
+   */
+  @action static clearUninstalling(extId: string): void {
+    logger.debug(`${Prefix}: trying to clear ${extId} as uninstalling`);
+
+    const curState = ExtensionInstallationStateStore.getInstallationState(extId);
+
+    switch (curState) {
+      case ExtensionInstallationState.UNINSTALLING:
+        return void ExtensionInstallationStateStore.UninstallingExtensions.delete(extId);
+      default:
+        throw new Error(`${Prefix}: cannot clear UNINSTALLING state for ${extId}, it is currently ${curState}`);
+    }
+  }
+
+  /**
+   * Returns the current state of the extension. IDLE is default value.
+   * @param extId The ID of the extension
+   */
+  static getInstallationState(extId: string): ExtensionInstallationState {
+    if (ExtensionInstallationStateStore.InstallingExtensions.has(extId)) {
+      return ExtensionInstallationState.INSTALLING;
+    }
+
+    if (ExtensionInstallationStateStore.UninstallingExtensions.has(extId)) {
+      return ExtensionInstallationState.UNINSTALLING;
+    }
+
+    return ExtensionInstallationState.IDLE;
+  }
+
+  /**
+   * Returns true if the extension is currently INSTALLING
+   * @param extId The ID of the extension
+   */
+  static isExtensionInstalling(extId: string): boolean {
+    return ExtensionInstallationStateStore.getInstallationState(extId) === ExtensionInstallationState.INSTALLING;
+  }
+
+  /**
+   * Returns true if the extension is currently UNINSTALLING
+   * @param extId The ID of the extension
+   */
+  static isExtensionUninstalling(extId: string): boolean {
+    return ExtensionInstallationStateStore.getInstallationState(extId) === ExtensionInstallationState.UNINSTALLING;
+  }
+
+  /**
+   * Returns true if the extension is currently IDLE
+   * @param extId The ID of the extension
+   */
+  static isExtensionIdle(extId: string): boolean {
+    return ExtensionInstallationStateStore.getInstallationState(extId) === ExtensionInstallationState.IDLE;
+  }
+
+  /**
+   * The current number of extensions installing
+   */
+  @computed static get installing(): number {
+    return ExtensionInstallationStateStore.InstallingExtensions.size;
+  }
+
+  /**
+   * If there is at least one extension currently installing
+   */
+  @computed static get anyInstalling(): boolean {
+    return ExtensionInstallationStateStore.installing > 0;
+  }
+
+  /**
+   * The current number of extensions preinstalling
+   */
+  @computed static get preinstalling(): number {
+    return ExtensionInstallationStateStore.PreInstallIds.size;
+  }
+
+  /**
+   * If there is at least one extension currently downloading
+   */
+  @computed static get anyPreinstalling(): boolean {
+    return ExtensionInstallationStateStore.preinstalling > 0;
+  }
+
+  /**
+   * If there is at least one installing or preinstalling step taking place
+   */
+  @computed static get anyPreInstallingOrInstalling(): boolean {
+    return ExtensionInstallationStateStore.anyInstalling || ExtensionInstallationStateStore.anyPreinstalling;
+  }
 }

src/renderer/components/button/button.tsx

@@ -1,5 +1,5 @@
 import "./button.scss";
-import React, { ButtonHTMLAttributes, ReactNode } from "react";
+import React, { ButtonHTMLAttributes } from "react";
 import { cssNames } from "../../utils";
 import { TooltipDecoratorProps, withTooltip } from "../tooltip";
 
@@ -26,29 +26,22 @@ export class Button extends React.PureComponent<ButtonProps, {}> {
 
   render() {
     const {
-      className, waiting, label, primary, accent, plain, hidden, active, big,
-      round, outlined, tooltip, light, children, ...props
+      waiting, label, primary, accent, plain, hidden, active, big,
+      round, outlined, tooltip, light, children, ...btnProps
     } = this.props;
-    const btnProps: Partial<ButtonProps> = props;
 
     if (hidden) return null;
 
-    btnProps.className = cssNames("Button", className, {
+    btnProps.className = cssNames("Button", btnProps.className, {
       waiting, primary, accent, plain, active, big, round, outlined, light,
     });
 
-    const btnContent: ReactNode = (
-      <>
-        {label}
-        {children}
-      </>
-    );
-
     // render as link
     if (this.props.href) {
       return (
         <a {...btnProps} ref={e => this.link = e}>
-          {btnContent}
+          {label}
+          {children}
         </a>
       );
     }
@@ -56,7 +49,8 @@ export class Button extends React.PureComponent<ButtonProps, {}> {
     // render as button
     return (
       <button type="button" {...btnProps} ref={e => this.button = e}>
-        {btnContent}
+        {label}
+        {children}
       </button>
     );
   }

src/renderer/components/confirm-dialog/confirm-dialog.tsx

@@ -11,14 +11,18 @@ import { Icon } from "../icon";
 export interface ConfirmDialogProps extends Partial<DialogProps> {
 }
 
-export interface ConfirmDialogParams {
-  ok?: () => void;
+export interface ConfirmDialogParams extends ConfirmDialogBooleanParams {
+  ok?: () => any | Promise<any>;
+  cancel?: () => any | Promise<any>;
+}
+
+export interface ConfirmDialogBooleanParams {
   labelOk?: ReactNode;
   labelCancel?: ReactNode;
-  message?: ReactNode;
+  message: ReactNode;
   icon?: ReactNode;
-  okButtonProps?: Partial<ButtonProps>
-  cancelButtonProps?: Partial<ButtonProps>
+  okButtonProps?: Partial<ButtonProps>;
+  cancelButtonProps?: Partial<ButtonProps>;
 }
 
 @observer
@@ -33,19 +37,26 @@ export class ConfirmDialog extends React.Component<ConfirmDialogProps> {
     ConfirmDialog.params = params;
   }
 
-  static close() {
-    ConfirmDialog.isOpen = false;
+  static confirm(params: ConfirmDialogBooleanParams): Promise<boolean> {
+    return new Promise(resolve => {
+      ConfirmDialog.open({
+        ok: () => resolve(true),
+        cancel: () => resolve(false),
+        ...params,
+      });
+    });
   }
 
-  public defaultParams: ConfirmDialogParams = {
+  static defaultParams: Partial<ConfirmDialogParams> = {
     ok: noop,
+    cancel: noop,
     labelOk: "Ok",
     labelCancel: "Cancel",
     icon: <Icon big material="warning"/>,
   };
 
   get params(): ConfirmDialogParams {
-    return Object.assign({}, this.defaultParams, ConfirmDialog.params);
+    return Object.assign({}, ConfirmDialog.defaultParams, ConfirmDialog.params);
   }
 
   ok = async () => {
@@ -54,16 +65,21 @@ export class ConfirmDialog extends React.Component<ConfirmDialogProps> {
       await Promise.resolve(this.params.ok()).catch(noop);
     } finally {
       this.isSaving = false;
+      ConfirmDialog.isOpen = false;
     }
-    this.close();
   };
 
   onClose = () => {
     this.isSaving = false;
   };
 
-  close = () => {
-    ConfirmDialog.close();
+  close = async () => {
+    try {
+      await Promise.resolve(this.params.cancel()).catch(noop);
+    } finally {
+      this.isSaving = false;
+      ConfirmDialog.isOpen = false;
+    }
   };
 
   render() {

src/renderer/components/input/input.tsx

@@ -315,6 +315,7 @@ export class Input extends React.Component<InputProps, State> {
       rows: multiLine ? (rows || 1) : null,
       ref: this.bindRef,
       spellCheck: "false",
+      disabled,
     });
     const showErrors = errors.length > 0 && !valid && dirty;
     const errorsInfo = (

src/renderer/components/input/input_validators.ts

@@ -47,6 +47,14 @@ export const isUrl: InputValidator = {
   },
 };
 
+export const isExtensionNameInstallRegex = /^(?<name>(@[-\w]+\/)?[-\w]+)(@(?<version>\d\.\d\.\d(-\w+)?))?$/gi;
+
+export const isExtensionNameInstall: InputValidator = {
+  condition: ({ type }) => type === "text",
+  message: () => "Not an extension name with optional version",
+  validate: value => value.match(isExtensionNameInstallRegex) !== null,
+};
+
 export const isPath: InputValidator = {
   condition: ({ type }) => type === "text",
   message: () => `This field must be a valid path`,

src/renderer/protocol-handler/app-handlers.ts

@@ -1,6 +1,6 @@
 import { addClusterURL } from "../components/+add-cluster";
 import { clusterSettingsURL } from "../components/+cluster-settings";
-import { extensionsURL } from "../components/+extensions";
+import { attemptInstallByInfo, extensionsURL } from "../components/+extensions";
 import { landingURL } from "../components/+landing-page";
 import { preferencesURL } from "../components/+preferences";
 import { clusterViewURL } from "../components/cluster-manager/cluster-view.route";
@@ -8,6 +8,7 @@ import { LensProtocolRouterRenderer } from "./router";
 import { navigate } from "../navigation/helpers";
 import { clusterStore } from "../../common/cluster-store";
 import { workspaceStore } from "../../common/workspace-store";
+import { EXTENSION_NAME_MATCH, EXTENSION_PUBLISHER_MATCH, LensProtocolRouter } from "../../common/protocol-handler";
 
 export function bindProtocolAddRouteHandlers() {
   LensProtocolRouterRenderer
@@ -54,5 +55,15 @@ export function bindProtocolAddRouteHandlers() {
     })
     .addInternalHandler("/extensions", () => {
       navigate(extensionsURL());
+    })
+    .addInternalHandler(`/extensions/install${LensProtocolRouter.ExtensionUrlSchema}`, ({ pathname, search: { version } }) => {
+      const name = [
+        pathname[EXTENSION_PUBLISHER_MATCH],
+        pathname[EXTENSION_NAME_MATCH],
+      ].filter(Boolean)
+        .join("/");
+
+      navigate(extensionsURL());
+      attemptInstallByInfo({ name, version, requireConfirmation: true });
     });
 }