fix(v2): Toggle off Auto Save and warn user after save error (#3359)

* fix: save loop in DS/USS after save error Signed-off-by: Trae Yelovich <[email protected]> * tests: ZoweSaveQueue.markAllUnsaved Signed-off-by: Trae Yelovich <[email protected]> * deps: update lockfile Signed-off-by: Trae Yelovich <[email protected]> * tests: workspace.checkAutoSaveForError Signed-off-by: Trae Yelovich <[email protected]> * tests: fix saveUSSFile tests; add auto save case Signed-off-by: Trae Yelovich <[email protected]> * refactor: rename function; update tests Signed-off-by: Trae Yelovich <[email protected]> * tests: cases for data set saveFile function Signed-off-by: Trae Yelovich <[email protected]> * chore: update changelog Signed-off-by: Trae Yelovich <[email protected]> * add edge case for Enable Auto Save button Signed-off-by: Trae Yelovich <[email protected]> --------- Signed-off-by: Trae Yelovich <[email protected]> Signed-off-by: Billie Simmons <[email protected]> Co-authored-by: Billie Simmons <[email protected]>
zowe · Dec 16, 2024 · cf55928 · cf55928
1 parent d7f1903
commit cf55928
Show file tree

Hide file tree

Showing 12 changed files with 220 additions and 5 deletions.
diff --git a/packages/zowe-explorer/CHANGELOG.md b/packages/zowe-explorer/CHANGELOG.md
@@ -19,6 +19,7 @@ All notable changes to the "vscode-extension-for-zowe" extension will be documen
 - Fixed an issue where data set nodes did not update if migrated or recalled outside of Zowe Explorer. [#3294](https://github.com/zowe/zowe-explorer-vscode/issues/3294)
 - Fixed an issue where clicking on a file in the Unix System Services tree caused the tree to abruptly change focus to the selected item. [#2486](https://github.com/zowe/zowe-explorer-vscode/issues/2486)
 - Fixed an issue where binary USS files were not fetched using the "Pull from Mainframe" context menu option. [#3355](https://github.com/zowe/zowe-explorer-vscode/issues/3355)
+- Fixed an issue with Auto Save where a failed UNIX file or data set save operation caused an infinite loop of save requests. [#2406](https://github.com/zowe/zowe-explorer-vscode/issues/2406), [#2627](https://github.com/zowe/zowe-explorer-vscode/issues/2627)
 
 ## `2.18.0`
 

diff --git a/packages/zowe-explorer/__tests__/__unit__/abstract/ZoweSaveQueue.unit.test.ts b/packages/zowe-explorer/__tests__/__unit__/abstract/ZoweSaveQueue.unit.test.ts
@@ -14,7 +14,6 @@ import { createUSSTree } from "../../../__mocks__/mockCreators/uss";
 import { saveUSSFile } from "../../../src/uss/actions";
 import * as workspaceUtils from "../../../src/utils/workspace";
 import { Gui } from "@zowe/zowe-explorer-api";
-import * as globals from "../../../src/globals";
 import * as vscode from "vscode";
 import { ZoweLogger } from "../../../src/utils/LoggerUtils";
 
@@ -24,6 +23,7 @@ describe("ZoweSaveQueue - unit tests", () => {
         const globalMocks = {
             errorMessageSpy: jest.spyOn(Gui, "errorMessage"),
             markDocumentUnsavedSpy: jest.spyOn(workspaceUtils, "markDocumentUnsaved"),
+            handleAutoSaveOnErrorMock: jest.spyOn(workspaceUtils, "handleAutoSaveOnError").mockImplementation(),
             processNextSpy: jest.spyOn(ZoweSaveQueue as any, "processNext"),
             allSpy: jest.spyOn(ZoweSaveQueue, "all"),
             trees: {
@@ -118,4 +118,46 @@ describe("ZoweSaveQueue - unit tests", () => {
             );
         }
     });
+
+    describe("markAllUnsaved", () => {
+        function getBlockMocks(): Record<string, jest.SpyInstance> {
+            return {
+                markDocumentUnsaved: jest.spyOn(workspaceUtils, "markDocumentUnsaved").mockClear().mockImplementation(),
+            };
+        }
+
+        it("marks all documents as unsaved in the queue", async () => {
+            const blockMocks = getBlockMocks();
+            (ZoweSaveQueue as any).savingQueue = [
+                {
+                    savedFile: { fileName: "some.jcl" },
+                } as any,
+                {
+                    savedFile: { fileName: "SOME.C.DS(MEM).c" } as any,
+                } as any,
+            ];
+            await ZoweSaveQueue.markAllUnsaved();
+            expect(blockMocks.markDocumentUnsaved).toHaveBeenCalledTimes(2);
+        });
+
+        it("clears the queue if clearQueue is true", async () => {
+            (ZoweSaveQueue as any).savingQueue = [
+                {
+                    savedFile: { fileName: "some.jcl" },
+                } as any,
+            ];
+            await ZoweSaveQueue.markAllUnsaved();
+            expect((ZoweSaveQueue as any).savingQueue.length).toBe(0);
+        });
+
+        it("does not clear the queue if clearQueue is false", async () => {
+            (ZoweSaveQueue as any).savingQueue = [
+                {
+                    savedFile: { fileName: "some.jcl" },
+                } as any,
+            ];
+            await ZoweSaveQueue.markAllUnsaved(false);
+            expect((ZoweSaveQueue as any).savingQueue.length).toBe(1);
+        });
+    });
 });
diff --git a/packages/zowe-explorer/__tests__/__unit__/dataset/actions.unit.test.ts b/packages/zowe-explorer/__tests__/__unit__/dataset/actions.unit.test.ts
@@ -43,6 +43,7 @@ import { getNodeLabels } from "../../../src/dataset/utils";
 import { ZoweLogger } from "../../../src/utils/LoggerUtils";
 import { mocked } from "../../../__mocks__/mockUtils";
 import { LocalFileManagement } from "../../../src/utils/LocalFileManagement";
+import * as workspaceUtils from "../../../src/utils/workspace";
 
 // Missing the definition of path module, because I need the original logic for tests
 jest.mock("fs");
@@ -1073,6 +1074,7 @@ describe("Dataset Actions Unit Tests - Function saveFile", () => {
             profileInstance,
             testDatasetTree,
             uploadContentSpy,
+            handleAutoSaveOnError: jest.spyOn(workspaceUtils, "handleAutoSaveOnError").mockImplementation(),
         };
     }
 
@@ -1238,6 +1240,7 @@ describe("Dataset Actions Unit Tests - Function saveFile", () => {
         (testDocument as any).fileName = path.join(globals.DS_DIR, testDocument.fileName);
 
         await dsActions.saveFile(testDocument, blockMocks.testDatasetTree);
+        expect(blockMocks.handleAutoSaveOnError).toHaveBeenCalled();
 
         expect(globalMocks.concatChildNodes).toBeCalled();
         expect(mocked(Gui.errorMessage)).toBeCalledWith("failed");

diff --git a/packages/zowe-explorer/__tests__/__unit__/uss/actions.unit.test.ts b/packages/zowe-explorer/__tests__/__unit__/uss/actions.unit.test.ts
@@ -32,6 +32,7 @@ import * as vscode from "vscode";
 import * as path from "path";
 import * as globals from "../../../src/globals";
 import * as sharedUtils from "../../../src/shared/utils";
+import * as workspaceUtils from "../../../src/utils/workspace";
 import { ZoweUSSNode } from "../../../src/uss/ZoweUSSNode";
 import * as isbinaryfile from "isbinaryfile";
 import * as fs from "fs";
@@ -41,6 +42,7 @@ import { ZoweLogger } from "../../../src/utils/LoggerUtils";
 import { AttributeView } from "../../../src/uss/AttributeView";
 import { mocked } from "../../../__mocks__/mockUtils";
 import { LocalFileManagement } from "../../../src/utils/LocalFileManagement";
+import { SettingsConfig } from "../../../src/utils/SettingsConfig";
 
 function createGlobalMocks() {
     const globalMocks = {
@@ -461,6 +463,9 @@ describe("USS Action Unit Tests - Function saveUSSFile", () => {
             ussNode: createUSSNode(globalMocks.testSession, globalMocks.testProfile),
             ussFavoriteNode: createFavoriteUSSNode(globalMocks.testSession, globalMocks.testProfile),
             putUSSPayload: jest.fn().mockResolvedValue(`{"stdout":[""]}`),
+            handleAutoSaveOnError: jest.spyOn(workspaceUtils, "handleAutoSaveOnError"),
+            // Disable auto save for most of the test cases
+            getDirectValue: jest.spyOn(SettingsConfig, "getDirectValue").mockReturnValueOnce("off"),
         };
 
         newMocks.node = new ZoweUSSNode({
@@ -544,6 +549,21 @@ describe("USS Action Unit Tests - Function saveUSSFile", () => {
         );
     });
 
+    it("calls handleAutoSaveOnError in the event of an error", async () => {
+        const globalMocks = createGlobalMocks();
+        const blockMocks = await createBlockMocks(globalMocks);
+
+        globalMocks.withProgress.mockImplementation((progLocation, callback) => callback());
+        globalMocks.fileToUSSFile.mockResolvedValue(blockMocks.testResponse);
+        blockMocks.testResponse.success = false;
+        blockMocks.testResponse.commandResponse = "Save failed";
+
+        globalMocks.withProgress.mockReturnValueOnce(blockMocks.testResponse);
+
+        await ussNodeActions.saveUSSFile(blockMocks.testDoc, blockMocks.testUSSTree);
+        expect(blockMocks.handleAutoSaveOnError).toHaveBeenCalled();
+    });
+
     it("Tests that saveUSSFile fails when save fails", async () => {
         const globalMocks = createGlobalMocks();
         const blockMocks = await createBlockMocks(globalMocks);
@@ -570,6 +590,7 @@ describe("USS Action Unit Tests - Function saveUSSFile", () => {
         globalMocks.withProgress.mockRejectedValueOnce(Error("Test Error"));
 
         await ussNodeActions.saveUSSFile(blockMocks.testDoc, blockMocks.testUSSTree);
+        expect(blockMocks.handleAutoSaveOnError).toHaveBeenCalled();
         expect(globalMocks.showErrorMessage.mock.calls.length).toBe(1);
         expect(globalMocks.showErrorMessage.mock.calls[0][0]).toBe("Error: Test Error");
         expect(mocked(vscode.workspace.applyEdit)).toHaveBeenCalledTimes(2);

diff --git a/packages/zowe-explorer/__tests__/__unit__/utils/workspace.unit.test.ts b/packages/zowe-explorer/__tests__/__unit__/utils/workspace.unit.test.ts
@@ -12,6 +12,9 @@
 import * as vscode from "vscode";
 import * as workspaceUtils from "../../../src/utils/workspace";
 import { workspaceUtilMaxEmptyWindowsInTheRow } from "../../../src/config/constants";
+import { SettingsConfig } from "../../../src/utils/SettingsConfig";
+import { ZoweSaveQueue } from "../../../src/abstract/ZoweSaveQueue";
+import { Gui } from "@zowe/zowe-explorer-api";
 
 function createGlobalMocks() {
     const activeTextEditor = jest.fn();
@@ -216,3 +219,70 @@ describe("Workspace Utils Unit Tests - function awaitForDocumentBeingSaved", ()
         await expect(workspaceUtils.awaitForDocumentBeingSaved()).resolves.not.toThrow();
     });
 });
+
+describe("Workspace Utils Unit Tests - function handleAutoSaveOnError", () => {
+    function getBlockMocks(autoSaveEnabled: boolean = true, userResponse?: string): Record<string, jest.SpyInstance> {
+        const executeCommand = jest.spyOn(vscode.commands, "executeCommand").mockClear();
+        const getDirectValue = jest.spyOn(SettingsConfig, "getDirectValue");
+        if (autoSaveEnabled) {
+            executeCommand.mockResolvedValueOnce(undefined);
+            getDirectValue.mockReturnValueOnce("afterDelay");
+        } else {
+            getDirectValue.mockReturnValueOnce("off");
+        }
+
+        if (userResponse === "Enable Auto Save") {
+            executeCommand.mockResolvedValueOnce(undefined);
+        }
+
+        return {
+            errorMessage: jest.spyOn(Gui, "errorMessage").mockResolvedValueOnce(userResponse),
+            executeCommand,
+            getDirectValue,
+            markAllUnsaved: jest.spyOn(ZoweSaveQueue, "markAllUnsaved"),
+        };
+    }
+
+    it("returns early if Auto Save is disabled", async () => {
+        const blockMocks = getBlockMocks(false);
+        await workspaceUtils.handleAutoSaveOnError();
+        expect(blockMocks.getDirectValue).toHaveBeenCalledWith("files.autoSave");
+        expect(blockMocks.executeCommand).not.toHaveBeenCalled();
+    });
+
+    it("toggles off auto save if enabled", async () => {
+        const blockMocks = getBlockMocks(true);
+        await workspaceUtils.handleAutoSaveOnError();
+        expect(blockMocks.executeCommand).toHaveBeenCalledWith("workbench.action.toggleAutoSave");
+    });
+
+    it("calls ZoweSaveQueue.markAllUnsaved to mark documents unsaved and clear queue", async () => {
+        const blockMocks = getBlockMocks(true);
+        await workspaceUtils.handleAutoSaveOnError();
+        expect(blockMocks.markAllUnsaved).toHaveBeenCalled();
+    });
+
+    it("prompts the user to reactivate Auto Save in event of save error", async () => {
+        const blockMocks = getBlockMocks(true);
+        await workspaceUtils.handleAutoSaveOnError();
+        expect(blockMocks.errorMessage).toHaveBeenCalledWith(
+            "Zowe Explorer encountered a save error and has disabled Auto Save. Once the issue is addressed, enable Auto Save and try again.",
+            { items: ["Enable Auto Save"] }
+        );
+    });
+
+    it("does nothing if 'Enable Auto Save' clicked and Auto Save is already on", async () => {
+        const blockMocks = getBlockMocks(true, "Enable Auto Save");
+        await workspaceUtils.handleAutoSaveOnError();
+        expect(blockMocks.executeCommand).toHaveBeenCalledWith("workbench.action.toggleAutoSave");
+        expect(blockMocks.executeCommand).toHaveBeenCalledTimes(1);
+    });
+
+    it("reactivates Auto Save if 'Enable Auto Save' clicked", async () => {
+        const blockMocks = getBlockMocks(true, "Enable Auto Save");
+        blockMocks.getDirectValue.mockReturnValueOnce("off");
+        await workspaceUtils.handleAutoSaveOnError();
+        expect(blockMocks.executeCommand).toHaveBeenCalledTimes(2);
+        expect(blockMocks.executeCommand).toHaveBeenCalledWith("workbench.action.toggleAutoSave");
+    });
+});
diff --git a/packages/zowe-explorer/i18n/sample/src/uss/USSTree.i18n.json b/packages/zowe-explorer/i18n/sample/src/uss/USSTree.i18n.json
@@ -1,6 +1,5 @@
 {
   "Favorites": "Favorites",
-  "renameUSS.unsavedWork": "Unable to rename {0} because you have unsaved changes in this {1}. Please save your work before renaming the {1}.",
   "renameUSS.enterName": "Enter a new name for the {0}",
   "renameUSS.error": "Unable to rename node:",
   "renameUSS.duplicateName": "A {0} already exists with this name. Please choose a different name.",

diff --git a/packages/zowe-explorer/i18n/sample/src/utils/TreeViewUtils.i18n.json b/packages/zowe-explorer/i18n/sample/src/utils/TreeViewUtils.i18n.json
@@ -0,0 +1,4 @@
+{
+  "uss.renameNode": "Rename",
+  "unsavedChanges.errorMsg": "Unable to {0} {1} because you have unsaved changes in this {2}. Please save your work and try again."
+}
diff --git a/packages/zowe-explorer/i18n/sample/src/utils/workspace.i18n.json b/packages/zowe-explorer/i18n/sample/src/utils/workspace.i18n.json
@@ -0,0 +1,4 @@
+{
+  "zowe.autoSaveToggled": "Zowe Explorer encountered a save error and has disabled Auto Save. Once the issue is addressed, enable Auto Save and try again.",
+  "zowe.enableAutoSave": "Enable Auto Save"
+}
diff --git a/packages/zowe-explorer/src/abstract/ZoweSaveQueue.ts b/packages/zowe-explorer/src/abstract/ZoweSaveQueue.ts
@@ -12,7 +12,7 @@
 import * as path from "path";
 import * as vscode from "vscode";
 import { Gui, IZoweTree, IZoweTreeNode } from "@zowe/zowe-explorer-api";
-import { markDocumentUnsaved } from "../utils/workspace";
+import { handleAutoSaveOnError, markDocumentUnsaved } from "../utils/workspace";
 import { ZoweLogger } from "../utils/LoggerUtils";
 
 // Set up localization
@@ -56,6 +56,20 @@ export class ZoweSaveQueue {
     private static ongoingSave = Promise.resolve();
     private static savingQueue: SaveRequest[] = [];
 
+    /**
+     * Marks all documents in the save queue as unsaved.
+     * @param clearQueue Whether to clear the queue after marking the documents as unsaved (default: `true`)
+     */
+    public static async markAllUnsaved(clearQueue: boolean = true): Promise<void> {
+        for (const { savedFile } of this.savingQueue) {
+            await markDocumentUnsaved(savedFile);
+        }
+
+        if (clearQueue) {
+            this.savingQueue = [];
+        }
+    }
+
     /**
      * Iterate over the queue and process next item until it is empty.
      */
@@ -71,6 +85,7 @@ export class ZoweSaveQueue {
         } catch (err) {
             ZoweLogger.error(err);
             await markDocumentUnsaved(nextRequest.savedFile);
+            await handleAutoSaveOnError();
             await Gui.errorMessage(
                 localize(
                     "processNext.error.uploadFailed",

diff --git a/packages/zowe-explorer/src/dataset/actions.ts b/packages/zowe-explorer/src/dataset/actions.ts
@@ -33,7 +33,7 @@ import { Profiles } from "../Profiles";
 import { getIconByNode } from "../generators/icons";
 import { ZoweDatasetNode } from "./ZoweDatasetNode";
 import * as contextually from "../shared/context";
-import { markDocumentUnsaved, setFileSaved } from "../utils/workspace";
+import { handleAutoSaveOnError, markDocumentUnsaved, setFileSaved } from "../utils/workspace";
 import { IUploadOptions } from "@zowe/zos-files-for-zowe-sdk";
 import { ZoweLogger } from "../utils/LoggerUtils";
 import { ProfileManagement } from "../utils/ProfileManagement";
@@ -1566,6 +1566,7 @@ export async function saveFile(doc: vscode.TextDocument, datasetProvider: api.IZ
             return;
         }
     } catch (err) {
+        await handleAutoSaveOnError();
         await markDocumentUnsaved(doc);
         await errorHandling(err, sesName);
         return;
@@ -1610,10 +1611,12 @@ export async function saveFile(doc: vscode.TextDocument, datasetProvider: api.IZ
         } else if (!uploadResponse.success && uploadResponse.commandResponse.includes("Rest API failure with HTTP(S) status 412")) {
             resolveFileConflict(node, prof, doc, label);
         } else {
+            await handleAutoSaveOnError();
             await markDocumentUnsaved(doc);
             api.Gui.errorMessage(uploadResponse.commandResponse);
         }
     } catch (err) {
+        await handleAutoSaveOnError();
         await markDocumentUnsaved(doc);
         await errorHandling(err, sesName);
     }

diff --git a/packages/zowe-explorer/src/uss/actions.ts b/packages/zowe-explorer/src/uss/actions.ts
@@ -21,7 +21,7 @@ import { Profiles } from "../Profiles";
 import { ZoweExplorerApiRegister } from "../ZoweExplorerApiRegister";
 import { isBinaryFileSync } from "isbinaryfile";
 import * as contextually from "../shared/context";
-import { markDocumentUnsaved, setFileSaved } from "../utils/workspace";
+import { handleAutoSaveOnError, markDocumentUnsaved, setFileSaved } from "../utils/workspace";
 import * as nls from "vscode-nls";
 import { refreshAll } from "../shared/refresh";
 import { autoDetectEncoding, fileExistsCaseSensitiveSync } from "./utils";
@@ -319,6 +319,7 @@ export async function saveUSSFile(doc: vscode.TextDocument, ussFileProvider: IZo
             LocalFileManagement.removeRecoveredFile(doc);
             // this part never runs! zowe.Upload.fileToUSSFile doesn't return success: false, it just throws the error which is caught below!!!!!
         } else {
+            await handleAutoSaveOnError();
             await markDocumentUnsaved(doc);
             Gui.errorMessage(uploadResponse.commandResponse);
         }
@@ -328,6 +329,7 @@ export async function saveUSSFile(doc: vscode.TextDocument, ussFileProvider: IZo
         if (errorMessage.includes("Rest API failure with HTTP(S) status 412")) {
             resolveFileConflict(node, prof, doc, remote);
         } else {
+            await handleAutoSaveOnError();
             await markDocumentUnsaved(doc);
             await errorHandling(err, sesName);
         }