Add confirmation dialog to extension uninstall (#1547)

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

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

@@ -2,13 +2,15 @@ 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 { ConfirmDialog } from "../../confirm-dialog";
+import { Notifications } from "../../notifications";
 import { Extensions } from "../extensions";
 
 jest.mock("../../../../extensions/extension-discovery", () => ({
   ...jest.requireActual("../../../../extensions/extension-discovery"),
   extensionDiscovery: {
     localFolderPath: "/fake/path",
-    uninstallExtension: jest.fn()
+    uninstallExtension: jest.fn(() => Promise.resolve())
   }
 }));
 
@@ -31,14 +33,47 @@ jest.mock("../../../../extensions/extension-loader", () => ({
   }
 }));
 
+jest.mock("../../notifications", () => ({
+  ok: jest.fn(),
+  error: jest.fn(),
+  info: jest.fn()
+}));
+
 describe("Extensions", () => {
-  it("disables uninstall and disable buttons while uninstalling", () => {
-    render(<Extensions />);
+  it("disables uninstall and disable buttons while uninstalling", async () => {
+    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"));
     
     expect(extensionDiscovery.uninstallExtension).toHaveBeenCalledWith("/absolute/path");
     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"));
+
+    setTimeout(() => {
+      expect(screen.getByText("Disable").closest("button")).not.toBeDisabled();
+      expect(screen.getByText("Uninstall").closest("button")).not.toBeDisabled();
+      expect(Notifications.error).toHaveBeenCalledTimes(1);
+    }, 100);
+  });
 });

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

@@ -1,8 +1,7 @@
 import { t, Trans } from "@lingui/macro";
 import { remote, shell } from "electron";
 import fse from "fs-extra";
-import { map, omit } from "lodash";
-import { computed, observable, ObservableMap, reaction } from "mobx";
+import { computed, observable, reaction } from "mobx";
 import { disposeOnUnmount, observer } from "mobx-react";
 import os from "os";
 import path from "path";
@@ -16,6 +15,7 @@ import logger from "../../../main/logger";
 import { _i18n } from "../../i18n";
 import { prevDefault } from "../../utils";
 import { Button } from "../button";
+import { ConfirmDialog } from "../confirm-dialog";
 import { Icon } from "../icon";
 import { DropFileInput, Input, InputValidator, InputValidators, SearchInput } from "../input";
 import { PageLayout } from "../layout/page-layout";
@@ -314,6 +314,17 @@ export class Extensions extends React.Component {
     }
   }
 
+  confirmUninstallExtension = (extension: InstalledExtension) => {
+    const displayName = extensionDisplayName(extension.manifest.name, extension.manifest.version);
+
+    ConfirmDialog.open({
+      message: <p>Are you sure you want to uninstall extension <b>{displayName}</b>?</p>,
+      labelOk: <Trans>Yes</Trans>,
+      labelCancel: <Trans>No</Trans>,
+      ok: () => this.uninstallExtension(extension)
+    });
+  };
+
   async uninstallExtension(extension: InstalledExtension) {
     const displayName = extensionDisplayName(extension.manifest.name, extension.manifest.version);
 
@@ -348,8 +359,8 @@ export class Extensions extends React.Component {
       );
     }
 
-    return extensions.map(ext => {
-      const { id, isEnabled, manifest } = ext;
+    return extensions.map(extension => {
+      const { id, isEnabled, manifest } = extension;
       const { name, description } = manifest;
       const isUninstalling = this.extensionState.get(id)?.state === "uninstalling";
 
@@ -365,13 +376,17 @@ export class Extensions extends React.Component {
           </div>
           <div className="actions">
             {!isEnabled && (
-              <Button plain active disabled={isUninstalling} onClick={() => ext.isEnabled = true}>Enable</Button>
+              <Button plain active disabled={isUninstalling} onClick={() => {
+                extension.isEnabled = true;
+              }}>Enable</Button>
             )}
             {isEnabled && (
-              <Button accent disabled={isUninstalling} onClick={() => ext.isEnabled = false}>Disable</Button>
+              <Button accent disabled={isUninstalling} onClick={() => {
+                extension.isEnabled = false;
+              }}>Disable</Button>
             )}
-            <Button plain active disabled={isUninstalling} waiting={isUninstalling}onClick={() => {
-              this.uninstallExtension(ext);
+            <Button plain active disabled={isUninstalling} waiting={isUninstalling} onClick={() => {
+              this.confirmUninstallExtension(extension);
             }}>Uninstall</Button>
           </div>
         </div>