Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 11 commits into from
Dec 16, 2024
1 change: 1 addition & 0 deletions packages/zowe-explorer/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ All notable changes to the "vscode-extension-for-zowe" extension will be documen
- Fixed an issue where a migrated data set is unusable after it is recalled through Zowe Explorer. [#3294](https://github.com/zowe/zowe-explorer-vscode/issues/3294)
- Fixed an issue where a recalled PDS is expandable after it is migrated through Zowe Explorer. [#3294](https://github.com/zowe/zowe-explorer-vscode/issues/3294)
- 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 when Auto Save is enabled where a failed UNIX file or data set save operation resulted in an indefinite loop of additional save requests. [#2406](https://github.com/zowe/zowe-explorer-vscode/issues/2406), [#2627](https://github.com/zowe/zowe-explorer-vscode/issues/2627)
- 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)

## `2.18.0`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -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: {
Expand Down Expand Up @@ -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);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -1073,6 +1074,7 @@ describe("Dataset Actions Unit Tests - Function saveFile", () => {
profileInstance,
testDatasetTree,
uploadContentSpy,
handleAutoSaveOnError: jest.spyOn(workspaceUtils, "handleAutoSaveOnError").mockImplementation(),
};
}

Expand Down Expand Up @@ -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");
Expand Down
21 changes: 21 additions & 0 deletions packages/zowe-explorer/__tests__/__unit__/uss/actions.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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 = {
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -216,3 +219,61 @@ 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("reactivates Auto Save if 'Enable Auto Save' clicked", async () => {
const blockMocks = getBlockMocks(true, "Enable Auto Save");
await workspaceUtils.handleAutoSaveOnError();
expect(blockMocks.executeCommand).toHaveBeenCalledWith("workbench.action.toggleAutoSave");
});
});
Original file line number Diff line number Diff line change
@@ -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.",
Expand Down
Original file line number Diff line number Diff line change
@@ -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."
}
Original file line number Diff line number Diff line change
@@ -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"
}
17 changes: 16 additions & 1 deletion packages/zowe-explorer/src/abstract/ZoweSaveQueue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
*/
Expand All @@ -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",
Expand Down
5 changes: 4 additions & 1 deletion packages/zowe-explorer/src/dataset/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
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";
Expand Down Expand Up @@ -246,7 +246,7 @@
* @param {IZoweDatasetTreeNode} node - The node selected for deletion
* @param {DatasetTree} datasetProvider - the tree which contains the nodes
*/
export async function deleteDatasetPrompt(datasetProvider: api.IZoweTree<api.IZoweDatasetTreeNode>, node?: api.IZoweDatasetTreeNode): Promise<void> {

Check warning on line 249 in packages/zowe-explorer/src/dataset/actions.ts

View workflow job for this annotation

GitHub Actions / lint

Async function 'deleteDatasetPrompt' has a complexity of 25. Maximum allowed is 15
ZoweLogger.trace("dataset.actions.deleteDatasetPrompt called.");
let nodes: api.IZoweDatasetTreeNode[];
const treeView = datasetProvider.getTreeView();
Expand Down Expand Up @@ -643,7 +643,7 @@
const templates = datasetProvider.getDsTemplates();
const templateNames: string[] = [];
templates?.forEach((template) => {
Object.entries(template).forEach(([key, value]) => {

Check warning on line 646 in packages/zowe-explorer/src/dataset/actions.ts

View workflow job for this annotation

GitHub Actions / lint

'value' is defined but never used. Allowed unused args must match /^_/u
templateNames.push(key);
});
});
Expand Down Expand Up @@ -688,7 +688,7 @@
if (template[key].dsorg === "PS") {
typeEnum = 4;
} else {
typeEnum = 3;

Check warning on line 691 in packages/zowe-explorer/src/dataset/actions.ts

View workflow job for this annotation

GitHub Actions / lint

No magic number: 3
}
propertiesFromDsType = value;
}
Expand Down Expand Up @@ -1510,7 +1510,7 @@
* @export
* @param {vscode.TextDocument} doc - TextDocument that is being saved
*/
export async function saveFile(doc: vscode.TextDocument, datasetProvider: api.IZoweTree<api.IZoweDatasetTreeNode>): Promise<void> {

Check warning on line 1513 in packages/zowe-explorer/src/dataset/actions.ts

View workflow job for this annotation

GitHub Actions / lint

Async function 'saveFile' has a complexity of 17. Maximum allowed is 15
ZoweLogger.trace("dataset.actions.saveFile called.");
// Check if file is a data set, instead of some other file
const docPath = path.join(doc.fileName, "..");
Expand Down Expand Up @@ -1566,6 +1566,7 @@
return;
}
} catch (err) {
await handleAutoSaveOnError();
await markDocumentUnsaved(doc);
await errorHandling(err, sesName);
return;
Expand Down Expand Up @@ -1610,10 +1611,12 @@
} 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);
}
Expand Down
4 changes: 3 additions & 1 deletion packages/zowe-explorer/src/uss/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down
51 changes: 51 additions & 0 deletions packages/zowe-explorer/src/utils/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,17 @@
workspaceUtilFileSaveInterval,
workspaceUtilFileSaveMaxIterationCount,
} from "../config/constants";
import { SettingsConfig } from "./SettingsConfig";
import { Gui } from "@zowe/zowe-explorer-api";

// Set up localization
import * as nls from "vscode-nls";
import { ZoweSaveQueue } from "../abstract/ZoweSaveQueue";
nls.config({
messageFormat: nls.MessageFormat.bundle,
bundleFormat: nls.BundleFormat.standalone,
})();
const localize: nls.LocalizeFunc = nls.loadMessageBundle();

interface IExtTextEditor extends vscode.TextEditor {
id: string;
Expand Down Expand Up @@ -143,3 +154,43 @@
edits2.delete(document.uri, new vscode.Range(new vscode.Position(0, 0), new vscode.Position(0, 1)));
await vscode.workspace.applyEdit(edits2);
}

/**
* Call in the event of a save error. This function checks if `Auto Save` is enabled:
* - If enabled, it is toggled off to prevent an error loop and an error message is shown to inform the user.
* - If disabled, the function returns.
*
* The user can either dismiss the message or click the "Enable Auto Save" button to reactivate the `Auto Save` feature.
*/
export async function handleAutoSaveOnError(): Promise<void> {
// If auto save is disabled, return. Otherwise, toggle it off
if (SettingsConfig.getDirectValue("files.autoSave") === "off") {
return;
}
await vscode.commands.executeCommand("workbench.action.toggleAutoSave");

// Mark remaining documents in queue as unsaved and clear the queue
await ZoweSaveQueue.markAllUnsaved();

// Inform the user that `Auto Save` was disabled and offer a button to re-enable it
Gui.errorMessage(
localize(
"zowe.autoSaveToggled",
"Zowe Explorer encountered a save error and has disabled Auto Save. Once the issue is addressed, enable Auto Save and try again."
),
{
items: [localize("zowe.enableAutoSave", "Enable Auto Save")],
}
).then(async (value) => {
if (!value) {
// User dismissed the message
return;
}

// Toggle auto save back on IFF it wasn't already reactivated
// (the prompt can be answered or dismissed at any point after its shown)
if (SettingsConfig.getDirectValue("files.autoSave") === "off") {
await vscode.commands.executeCommand("workbench.action.toggleAutoSave");

Check warning on line 193 in packages/zowe-explorer/src/utils/workspace.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/utils/workspace.ts#L193

Added line #L193 was not covered by tests
}
});
}
Loading
Loading