Disable Uninstall and Enable/Disable buttons while uninstalling. Add Notification for uninstall. (#1539)

Signed-off-by: Panu Horsmalahti <phorsmalahti@mirantis.com>

package.json

@@ -232,7 +232,7 @@
     "mac-ca": "^1.0.4",
     "marked": "^1.1.0",
     "md5-file": "^5.0.0",
-    "mobx": "^5.15.5",
+    "mobx": "^5.15.7",
     "mobx-observable-history": "^1.0.3",
     "mock-fs": "^4.12.0",
     "node-pty": "^0.9.0",

src/extensions/__tests__/extension-loader.test.ts

@@ -18,6 +18,7 @@ jest.mock(
                   name: "TestExtension",
                   version: "1.0.0",
                 },
+                id: manifestPath,
                 absolutePath: "/test/1",
                 manifestPath,
                 isBundled: false,
@@ -31,6 +32,7 @@ jest.mock(
                   name: "TestExtension2",
                   version: "2.0.0",
                 },
+                id: manifestPath2,
                 absolutePath: "/test/2",
                 manifestPath: manifestPath2,
                 isBundled: false,
@@ -54,6 +56,7 @@ jest.mock(
                       name: "TestExtension",
                       version: "1.0.0",
                     },
+                    id: manifestPath,
                     absolutePath: "/test/1",
                     manifestPath,
                     isBundled: false,
@@ -67,6 +70,7 @@ jest.mock(
                       name: "TestExtension3",
                       version: "3.0.0",
                     },
+                    id: manifestPath3,
                     absolutePath: "/test/3",
                     manifestPath: manifestPath3,
                     isBundled: false,
@@ -99,6 +103,7 @@ describe("ExtensionLoader", () => {
         Map {
           "manifest/path" => Object {
             "absolutePath": "/test/1",
+            "id": "manifest/path",
             "isBundled": false,
             "isEnabled": true,
             "manifest": Object {
@@ -109,6 +114,7 @@ describe("ExtensionLoader", () => {
           },
           "manifest/path3" => Object {
             "absolutePath": "/test/3",
+            "id": "manifest/path3",
             "isBundled": false,
             "isEnabled": true,
             "manifest": Object {

src/extensions/__tests__/lens-extension.test.ts

@@ -9,6 +9,7 @@ describe("lens extension", () => {
         name: "foo-bar",
         version: "0.1.1"
       },
+      id: "/this/is/fake/package.json",
       absolutePath: "/absolute/fake/",
       manifestPath: "/this/is/fake/package.json",
       isBundled: false,

src/extensions/extension-discovery.ts

@@ -10,6 +10,8 @@ import { extensionsStore } from "./extensions-store";
 import type { LensExtensionId, LensExtensionManifest } from "./lens-extension";
 
 export interface InstalledExtension {
+    id: LensExtensionId;
+
     readonly manifest: LensExtensionManifest;
 
     // Absolute path to the non-symlinked source folder,
@@ -254,6 +256,7 @@ export class ExtensionDiscovery {
       const isEnabled = isBundled || extensionsStore.isEnabled(installedManifestPath);
 
       return {
+        id: installedManifestPath,
         absolutePath: path.dirname(manifestPath),
         manifestPath: installedManifestPath,
         manifest: manifestJson,
@@ -273,7 +276,7 @@ export class ExtensionDiscovery {
     await this.installPackages();
     const extensions = bundledExtensions.concat(localExtensions);
 
-    return new Map(extensions.map(ext => [ext.manifestPath, ext]));
+    return new Map(extensions.map(extension => [extension.id, extension]));
   }
 
   /**

src/extensions/extension-loader.ts

@@ -61,7 +61,7 @@ export class ExtensionLoader {
   }
 
   addExtension(extension: InstalledExtension) {
-    this.extensions.set(extension.manifestPath as LensExtensionId, extension);
+    this.extensions.set(extension.id, extension);
   }
 
   removeInstance(lensExtensionId: LensExtensionId) {
@@ -139,8 +139,7 @@ export class ExtensionLoader {
       ];
 
       this.events.on("remove", (removedExtension: LensRendererExtension) => {
-        // manifestPath is considered the id
+        if (removedExtension.id === extension.id) {
-        if (removedExtension.manifestPath === extension.manifestPath) {
           removeItems.forEach(remove => {
             remove();
           });
@@ -163,7 +162,7 @@ export class ExtensionLoader {
       ];
 
       this.events.on("remove", (removedExtension: LensRendererExtension) => {
-        if (removedExtension.manifestPath === extension.manifestPath) {
+        if (removedExtension.id === extension.id) {
           removeItems.forEach(remove => {
             remove();
           });
@@ -191,7 +190,7 @@ export class ExtensionLoader {
       ];
 
       this.events.on("remove", (removedExtension: LensRendererExtension) => {
-        if (removedExtension.manifestPath === extension.manifestPath) {
+        if (removedExtension.id === extension.id) {
           removeItems.forEach(remove => {
             remove();
           });

src/extensions/lens-extension.ts

@@ -16,23 +16,20 @@ export interface LensExtensionManifest {
 }
 
 export class LensExtension {
+  readonly id: LensExtensionId;
   readonly manifest: LensExtensionManifest;
   readonly manifestPath: string;
   readonly isBundled: boolean;
 
   @observable private isEnabled = false;
 
-  constructor({ manifest, manifestPath, isBundled }: InstalledExtension) {
+  constructor({ id, manifest, manifestPath, isBundled }: InstalledExtension) {
+    this.id = id;
     this.manifest = manifest;
     this.manifestPath = manifestPath;
     this.isBundled = !!isBundled;
   }
 
-  get id(): LensExtensionId {
-    // This is the symlinked path under node_modules
-    return this.manifestPath;
-  }
-
   get name() {
     return this.manifest.name;
   }

src/extensions/registries/__tests__/page-registry.test.ts

@@ -11,6 +11,7 @@ describe("getPageUrl", () => {
         name: "foo-bar",
         version: "0.1.1"
       },
+      id: "/this/is/fake/package.json",
       absolutePath: "/absolute/fake/",
       manifestPath: "/this/is/fake/package.json",
       isBundled: false,
@@ -42,6 +43,7 @@ describe("globalPageRegistry", () => {
         name: "@acme/foo-bar",
         version: "0.1.1"
       },
+      id: "/this/is/fake/package.json",
       absolutePath: "/absolute/fake/",
       manifestPath: "/this/is/fake/package.json",
       isBundled: false,

src/jest.setup.ts

@@ -1,4 +1,3 @@
-
 import fetchMock from "jest-fetch-mock";
 // rewire global.fetch to call 'fetchMock'
 fetchMock.enableMocks();

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

@@ -0,0 +1,44 @@
+import '@testing-library/jest-dom/extend-expect';
+import { fireEvent, render, screen } from '@testing-library/react';
+import React from 'react';
+import { extensionDiscovery } from "../../../../extensions/extension-discovery";
+import { Extensions } from "../extensions";
+
+jest.mock("../../../../extensions/extension-discovery", () => ({
+  ...jest.requireActual("../../../../extensions/extension-discovery"),
+  extensionDiscovery: {
+    localFolderPath: "/fake/path",
+    uninstallExtension: jest.fn()
+  }
+}));
+
+jest.mock("../../../../extensions/extension-loader", () => ({
+  ...jest.requireActual("../../../../extensions/extension-loader"),
+  extensionLoader: {
+    userExtensions: new Map([
+      ["extensionId", {
+        id: "extensionId",
+        manifest: {
+          name: "test",
+          version: "1.2.3"
+        },
+        absolutePath: "/absolute/path",
+        manifestPath: "/symlinked/path/package.json",
+        isBundled: false,
+        isEnabled: true
+      }]
+    ])
+  }
+}));
+
+describe("Extensions", () => {
+  it("disables uninstall and disable buttons while uninstalling", () => {
+    render(<Extensions />);
+
+    fireEvent.click(screen.getByText("Uninstall"));
+    
+    expect(extensionDiscovery.uninstallExtension).toHaveBeenCalledWith("/absolute/path");
+    expect(screen.getByText("Disable").closest("button")).toBeDisabled();
+    expect(screen.getByText("Uninstall").closest("button")).toBeDisabled();
+  });
+});

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

@@ -1,8 +1,9 @@
 import { t, Trans } from "@lingui/macro";
 import { remote, shell } from "electron";
 import fse from "fs-extra";
-import { computed, observable } from "mobx";
+import { map, omit } from "lodash";
-import { observer } from "mobx-react";
+import { computed, observable, ObservableMap, reaction } from "mobx";
+import { disposeOnUnmount, observer } from "mobx-react";
 import os from "os";
 import path from "path";
 import React from "react";
@@ -38,6 +39,12 @@ interface InstallRequestValidated extends InstallRequestPreloaded {
   tempFile: string; // temp system path to packed extension for unpacking
 }
 
+interface ExtensionState {
+  displayName: string;
+  // Possible states the extension can be
+  state: "uninstalling";
+}
+
 @observer
 export class Extensions extends React.Component {
   private supportedFormats = [".tar", ".tgz"];
@@ -49,17 +56,47 @@ export class Extensions extends React.Component {
     }
   };
 
+  @observable
+  extensionState = observable.map<string, ExtensionState>();
+
   @observable search = "";
   @observable installPath = "";
 
+  /**
+   * Extensions that were removed from extensions but are still in "uninstalling" state
+   */
+  @computed get removedUninstalling() {
+    return Array.from(this.extensionState.entries()).filter(([id, extension]) => 
+      extension.state === "uninstalling" && !this.extensions.find(extension => extension.id === id)
+    ).map(([id, extension]) => ({ ...extension, id }));
+  }
+  
+  componentDidMount() {
+    disposeOnUnmount(this,
+      reaction(() => this.extensions, (extensions) => {
+        const removedUninstalling = this.removedUninstalling;
+
+        removedUninstalling.forEach(({ displayName }) => {
+          Notifications.ok(
+            <p>Extension <b>{displayName}</b> successfully uninstalled!</p>
+          );
+        });
+
+        removedUninstalling.forEach(({ id }) => {
+          this.extensionState.delete(id);
+        });
+      })
+    );
+  }
+
   @computed get extensions() {
     const searchText = this.search.toLowerCase();
     return Array.from(extensionLoader.userExtensions.values()).filter(ext => {
       const { name, description } = ext.manifest;
       return [
         name.toLowerCase().includes(searchText),
-        description.toLowerCase().includes(searchText),
+        description?.toLowerCase().includes(searchText),
-      ].some(v => v);
+      ].some(value => value);
     });
   }
 
@@ -278,14 +315,21 @@ export class Extensions extends React.Component {
   }
 
   async uninstallExtension(extension: InstalledExtension) {
-    const extensionName = extensionDisplayName(extension.manifest.name, extension.manifest.version);
+    const displayName = extensionDisplayName(extension.manifest.name, extension.manifest.version);
 
     try {
+      this.extensionState.set(extension.id, {
+        state: "uninstalling",
+        displayName
+      });
+
       await extensionDiscovery.uninstallExtension(extension.absolutePath);
     } catch (error) {
       Notifications.error(
-        <p>Uninstalling extension <b>{extensionName}</b> has failed: <em>{error?.message ?? ""}</em></p>
+        <p>Uninstalling extension <b>{displayName}</b> has failed: <em>{error?.message ?? ""}</em></p>
       );
+      // Remove uninstall state on uninstall failure
+      this.extensionState.delete(extension.id);
     }
   }
 
@@ -305,11 +349,12 @@ export class Extensions extends React.Component {
     }
 
     return extensions.map(ext => {
-      const { manifestPath: extId, isEnabled, manifest } = ext;
+      const { id, isEnabled, manifest } = ext;
       const { name, description } = manifest;
+      const isUninstalling = this.extensionState.get(id)?.state === "uninstalling";
 
       return (
-        <div key={extId} className="extension flex gaps align-center">
+        <div key={id} className="extension flex gaps align-center">
           <div className="box grow">
             <div className="name">
               Name: <code className="name">{name}</code>
@@ -320,12 +365,12 @@ export class Extensions extends React.Component {
           </div>
           <div className="actions">
             {!isEnabled && (
-              <Button plain active onClick={() => ext.isEnabled = true}>Enable</Button>
+              <Button plain active disabled={isUninstalling} onClick={() => ext.isEnabled = true}>Enable</Button>
             )}
             {isEnabled && (
-              <Button accent onClick={() => ext.isEnabled = false}>Disable</Button>
+              <Button accent disabled={isUninstalling} onClick={() => ext.isEnabled = false}>Disable</Button>
             )}
-            <Button plain active onClick={() => {
+            <Button plain active disabled={isUninstalling} waiting={isUninstalling}onClick={() => {
               this.uninstallExtension(ext);
             }}>Uninstall</Button>
           </div>

src/renderer/components/button/button.scss

@@ -12,6 +12,7 @@
   flex-shrink: 0;
   line-height: 1;
   font-size: $font-size;
+  user-select: none;
 
   &[href] {
     display: inline-block;

yarn.lock

@@ -9879,10 +9879,10 @@ mobx@^5.15.4:
   resolved "https://registry.yarnpkg.com/mobx/-/mobx-5.15.4.tgz#9da1a84e97ba624622f4e55a0bf3300fb931c2ab"
   integrity sha512-xRFJxSU2Im3nrGCdjSuOTFmxVDGeqOHL+TyADCGbT0k4HHqGmx5u2yaHNryvoORpI4DfbzjJ5jPmuv+d7sioFw==
 
-mobx@^5.15.5:
+mobx@^5.15.7:
-  version "5.15.5"
+  version "5.15.7"
-  resolved "https://registry.yarnpkg.com/mobx/-/mobx-5.15.5.tgz#69715dc8662f64d153309bfe95169b8df4b4be4b"
+  resolved "https://registry.yarnpkg.com/mobx/-/mobx-5.15.7.tgz#b9a5f2b6251f5d96980d13c78e9b5d8d4ce22665"
-  integrity sha512-hzk17T+/IIYLPWClRcfoA6Q5aZhFpDCr1oh8RZzu+esWP77IX/lS0V/Ee1Np+aOPKFfbSInF0reHH0L/aFfSrw==
+  integrity sha512-wyM3FghTkhmC+hQjyPGGFdpehrcX1KOXsDuERhfK2YbJemkUhEB+6wzEN639T21onxlfYBmriA1PFnvxTUhcKw==
 
 mock-fs@^4.12.0:
   version "4.12.0"