Skip to content

Commit

Permalink
Merge pull request #2481 from zowe/fix/uss/tree-ux
Browse files Browse the repository at this point in the history
fix(tree,jobs): Various fixes to improve tree UX; add checks for "Delete Job"
  • Loading branch information
JillieBeanSim authored Oct 2, 2023
2 parents 094f904 + e38ff9e commit 8b2e848
Show file tree
Hide file tree
Showing 33 changed files with 138 additions and 61 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ function createGlobalMocks() {
showWarningMessage: jest.fn(),
createOutputChannel: jest.fn(),
createQuickPick: jest.fn(),
createTreeView: jest.fn(),
createTreeView: jest.fn().mockReturnValue({ onDidCollapseElement: jest.fn() }),
createWebviewPanel: jest.fn(),
withProgress: jest.fn(),
showTextDocument: jest.fn(),
Expand Down
4 changes: 4 additions & 0 deletions packages/zowe-explorer/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ All notable changes to the "vscode-extension-for-zowe" extension will be documen
### Bug fixes

- Fixed submitting local JCL using command pallet option `Zowe Explorer: Submit JCL` by adding a check for chosen profile returned to continue the action. [#1625](https://github.com/zowe/vscode-extension-for-zowe/issues/1625)
- Fixed issue where USS nodes were not removed from tree during deletion. [#2479](https://github.com/zowe/vscode-extension-for-zowe/issues/2479)
- Fixed issue where new USS nodes from a paste operation were not shown in tree until refreshed. [#2479](https://github.com/zowe/vscode-extension-for-zowe/issues/2479)
- Fixed issue where the "Delete Job" action showed a successful deletion message, even if the API returned an error.
- USS directories, PDS nodes, job nodes and session nodes now update with their respective "collapsed icon" when collapsed.
- Fixed bug where the list of datasets from a filter search was not re-sorted after a new data set was created in Zowe Explorer. [#2473](https://github.com/zowe/vscode-extension-for-zowe/issues/2473)

## `2.11.0`
Expand Down
9 changes: 8 additions & 1 deletion packages/zowe-explorer/__mocks__/vscode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,12 @@ export namespace extensions {
};
}
}

export interface TreeViewExpansionEvent<T> {
/**
* Element that is expanded or collapsed.
*/
readonly element: T;
}
export interface TreeView<T> {
/**
* An optional human-readable message that will be rendered in the view.
Expand Down Expand Up @@ -177,6 +182,8 @@ export interface TreeView<T> {
* **NOTE:** The {@link TreeDataProvider} that the `TreeView` {@link window.createTreeView is registered with} with must implement {@link TreeDataProvider.getParent getParent} method to access this API.
*/
reveal(element: T, options?: { select?: boolean; focus?: boolean; expand?: boolean | number }): Thenable<void>;

onDidCollapseElement: Event<TreeViewExpansionEvent<T>>;
}

export class FileDecoration {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,10 @@ async function createGlobalMocks() {
configurable: true,
});
Object.defineProperty(globals, "ISTHEIA", { get: () => false, configurable: true });
Object.defineProperty(vscode.window, "createTreeView", { value: jest.fn(), configurable: true });
Object.defineProperty(vscode.window, "createTreeView", {
value: jest.fn().mockReturnValue({ onDidCollapseElement: jest.fn() }),
configurable: true,
});
Object.defineProperty(vscode.workspace, "getConfiguration", {
value: newMocks.mockGetConfiguration,
configurable: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,10 @@ describe("ZoweExplorerExtender unit tests", () => {
})
.mockReturnValue(newMocks.profiles),
});
Object.defineProperty(vscode.window, "createTreeView", { value: jest.fn(), configurable: true });
Object.defineProperty(vscode.window, "createTreeView", {
value: jest.fn().mockReturnValue({ onDidCollapseElement: jest.fn() }),
configurable: true,
});
Object.defineProperty(vscode.window, "showErrorMessage", {
value: newMocks.mockErrorMessage,
configurable: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,6 @@ describe("Unit Tests (Jest)", () => {
undefined,
profileOne
);
infoChild.id = "root.Use the search button to display data sets";
rootNode.contextValue = globals.DS_SESSION_CONTEXT;
rootNode.dirty = false;
await expect(await rootNode.getChildren()).toEqual([infoChild]);
Expand All @@ -259,7 +258,6 @@ describe("Unit Tests (Jest)", () => {
undefined,
profileOne
);
infoChild.id = "root.Use the search button to display data sets";
rootNode.contextValue = globals.DS_SESSION_CONTEXT;
await expect(await rootNode.getChildren()).toEqual([infoChild]);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ async function createGlobalMocks() {
mockLoadNamedProfile: jest.fn(),
mockDefaultProfile: jest.fn(),
withProgress: jest.fn(),
createTreeView: jest.fn(),
createTreeView: jest.fn().mockReturnValue({ onDidCollapseElement: jest.fn() }),
mockAffects: jest.fn(),
mockEditSession: jest.fn(),
mockCheckCurrentProfile: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { ZoweLogger } from "../../../src/utils/LoggerUtils";

describe("ZoweSaveQueue - unit tests", () => {
const createGlobalMocks = () => {
jest.spyOn(Gui, "createTreeView").mockReturnValue({ onDidCollapseElement: jest.fn() } as any);
const globalMocks = {
errorMessageSpy: jest.spyOn(Gui, "errorMessage"),
markDocumentUnsavedSpy: jest.spyOn(workspaceUtils, "markDocumentUnsaved"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,10 @@ function createGlobalMocks() {

globalMocks.mockProfileInstance = createInstanceOfProfile(globalMocks.testProfileLoaded);

Object.defineProperty(vscode.window, "createTreeView", { value: jest.fn(), configurable: true });
Object.defineProperty(vscode.window, "createTreeView", {
value: jest.fn().mockReturnValue({ onDidCollapseElement: jest.fn() }),
configurable: true,
});
Object.defineProperty(Gui, "showMessage", { value: jest.fn(), configurable: true });
Object.defineProperty(Gui, "setStatusBarMessage", { value: jest.fn().mockReturnValue({ dispose: jest.fn() }), configurable: true });
Object.defineProperty(vscode.window, "showTextDocument", { value: jest.fn(), configurable: true });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ async function createGlobalMocks() {
mockMoveSync: jest.fn(),
mockGetAllProfileNames: jest.fn(),
mockReveal: jest.fn(),
mockCreateTreeView: jest.fn(),
mockCreateTreeView: jest.fn().mockReturnValue({ onDidCollapseElement: jest.fn() }),
mockExecuteCommand: jest.fn(),
mockRegisterCommand: jest.fn(),
mockOnDidSaveTextDocument: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import * as vscode from "vscode";

describe("Checking icon generator's basics", () => {
const setGlobalMocks = () => {
const createTreeView = jest.fn();
const createTreeView = jest.fn().mockReturnValue({ onDidCollapseElement: jest.fn() });
const getConfiguration = jest.fn();

Object.defineProperty(vscode.window, "createTreeView", { value: createTreeView });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jest.mock("vscode");

describe("Checking message generator's basics", () => {
const setGlobalMocks = () => {
const createTreeView = jest.fn();
const createTreeView = jest.fn().mockReturnValue({ onDidCollapseElement: jest.fn() });
const getConfiguration = jest.fn();

Object.defineProperty(vscode.window, "createTreeView", { value: createTreeView });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ async function createGlobalMocks() {
mockGetJob: jest.fn(),
mockRefresh: jest.fn(),
mockAffectsConfig: jest.fn(),
createTreeView: jest.fn(),
createTreeView: jest.fn().mockReturnValue({ onDidCollapseElement: jest.fn() }),
mockGetSpoolFiles: jest.fn(),
mockDeleteJobs: jest.fn(),
mockShowInputBox: jest.fn(),
Expand Down Expand Up @@ -85,7 +85,7 @@ async function createGlobalMocks() {
};
}),
};

jest.spyOn(Gui, "createTreeView").mockImplementation(globalMocks.createTreeView);
Object.defineProperty(ProfilesCache, "getConfigInstance", {
value: jest.fn(() => {
return {
Expand Down Expand Up @@ -173,7 +173,6 @@ async function createGlobalMocks() {
Object.defineProperty(ZoweLogger, "warn", { value: jest.fn(), configurable: true });
Object.defineProperty(ZoweLogger, "info", { value: jest.fn(), configurable: true });
Object.defineProperty(ZoweLogger, "trace", { value: jest.fn(), configurable: true });
globalMocks.createTreeView.mockReturnValue("testTreeView");
globalMocks.testSessionNode = createJobSessionNode(globalMocks.testSession, globalMocks.testProfile);
globalMocks.mockGetJob.mockReturnValue(globalMocks.testIJob);
globalMocks.mockGetJobsByOwnerAndPrefix.mockReturnValue([globalMocks.testIJob, globalMocks.testIJobComplete]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ async function createGlobalMocks() {
mockAffectsConfig: jest.fn(),
createTreeView: jest.fn(() => ({
reveal: jest.fn(),
onDidCollapseElement: jest.fn(),
})),
mockCreateSessCfgFromArgs: jest.fn(),
mockGetSpoolFiles: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import { SpoolFile } from "../../../src/SpoolProvider";
const activeTextEditorDocument = jest.fn();

function createGlobalMocks() {
jest.spyOn(Gui, "createTreeView").mockReturnValue({ onDidCollapseElement: jest.fn() } as any);
Object.defineProperty(vscode.workspace, "getConfiguration", {
value: jest.fn().mockImplementation(() => new Map([["zowe.jobs.confirmSubmission", false]])),
configurable: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,10 @@ async function createGlobalMocks() {
}),
configurable: true,
});
Object.defineProperty(vscode.window, "createTreeView", { value: jest.fn(), configurable: true });
Object.defineProperty(vscode.window, "createTreeView", {
value: jest.fn().mockReturnValue({ onDidCollapseElement: jest.fn() }),
configurable: true,
});
Object.defineProperty(vscode.workspace, "getConfiguration", { value: jest.fn(), configurable: true });
Object.defineProperty(vscode.window, "showInformationMessage", { value: jest.fn(), configurable: true });
Object.defineProperty(vscode.window, "showInputBox", { value: jest.fn(), configurable: true });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import { ZoweLogger } from "../../../src/utils/LoggerUtils";
function createGlobalMocks() {
const globalMocks = {
session: createISessionWithoutCredentials(),
createTreeView: jest.fn(),
createTreeView: jest.fn().mockReturnValue({ onDidCollapseElement: jest.fn() }),
mockLog: jest.fn(),
mockDebug: jest.fn(),
mockError: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ async function createGlobalMocks() {
showInputBox: jest.fn(),
filters: jest.fn(),
getFilters: jest.fn(),
createTreeView: jest.fn(),
createTreeView: jest.fn().mockReturnValue({ onDidCollapseElement: jest.fn() }),
createQuickPick: jest.fn(),
getConfiguration: jest.fn(),
ZosmfSession: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,10 @@ async function createGlobalMocks() {
configurable: true,
});
Object.defineProperty(vscode.window, "showInputBox", { value: globalMocks.showInputBox, configurable: true });
Object.defineProperty(vscode.window, "createTreeView", { value: jest.fn(), configurable: true });
Object.defineProperty(vscode.window, "createTreeView", {
value: jest.fn().mockReturnValue({ onDidCollapseElement: jest.fn() }),
configurable: true,
});
Object.defineProperty(zowe, "ZosmfSession", { value: globalMocks.ZosmfSession, configurable: true });
Object.defineProperty(globalMocks.ZosmfSession, "createSessCfgFromArgs", {
value: globalMocks.createSessCfgFromArgs,
Expand Down
16 changes: 12 additions & 4 deletions packages/zowe-explorer/__tests__/__unit__/uss/actions.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ function createGlobalMocks() {
setStatusBarMessage: jest.fn().mockReturnValue({ dispose: jest.fn() }),
showWarningMessage: jest.fn(),
showErrorMessage: jest.fn(),
createTreeView: jest.fn(),
createTreeView: jest.fn().mockReturnValue({ onDidCollapseElement: jest.fn() }),
fileToUSSFile: jest.fn(),
Upload: jest.fn(),
isBinaryFileSync: jest.fn(),
Expand Down Expand Up @@ -891,9 +891,7 @@ describe("USS Action Unit Tests - copy file / directory", () => {
it("tests pasteUssFile executed successfully with selected nodes", async () => {
const globalMocks = createGlobalMocks();
const blockMocks = await createBlockMocks(globalMocks);
const parent = blockMocks.treeNodes.testUSSTree.getTreeView();
parent.selection = blockMocks.nodes[0];
await ussNodeActions.pasteUssFile(blockMocks.treeNodes.testUSSTree, undefined);
await ussNodeActions.pasteUssFile(blockMocks.treeNodes.testUSSTree, blockMocks.nodes[0]);
expect(sharedUtils.getSelectedNodeList(blockMocks.treeNodes.ussNode, blockMocks.treeNodes.ussNodes)).toEqual([blockMocks.treeNodes.ussNode]);
});
it("tests pasteUssFile executed successfully with one node", async () => {
Expand All @@ -905,6 +903,16 @@ describe("USS Action Unit Tests - copy file / directory", () => {
await ussNodeActions.pasteUssFile(blockMocks.treeNodes.testUSSTree, blockMocks.nodes[0]);
expect(sharedUtils.getSelectedNodeList(blockMocks.treeNodes.ussNode, blockMocks.treeNodes.ussNodes)).toEqual([blockMocks.treeNodes.ussNode]);
});
it("tests pasteUss returns early if APIs are not supported", async () => {
const globalMocks = createGlobalMocks();
const blockMocks = await createBlockMocks(globalMocks);
const testNode = blockMocks.nodes[0];
testNode.copyUssFile = testNode.pasteUssTree = null;
const infoMessageSpy = jest.spyOn(Gui, "infoMessage");
await ussNodeActions.pasteUss(blockMocks.treeNodes.testUSSTree, testNode);
expect(infoMessageSpy).toHaveBeenCalledWith("The paste operation is not supported for this node.");
infoMessageSpy.mockRestore();
});
});

describe("USS Action Unit Tests - function deleteUSSFilesPrompt", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ describe("SessionUtils removeSession Unit Tests", () => {
newMocks.datasetSessionNode = createDatasetSessionNode(newMocks.session, newMocks.imperativeProfile);
newMocks.testDatasetTree = createDatasetTree(newMocks.datasetSessionNode, newMocks.treeView);
newMocks.testDatasetTree.addFileHistory("[profile1]: TEST.NODE");
Object.defineProperty(vscode.window, "createTreeView", { value: jest.fn(), configurable: true });
Object.defineProperty(vscode.window, "createTreeView", {
value: jest.fn().mockReturnValue({ onDidCollapseElement: jest.fn() }),
configurable: true,
});
Object.defineProperty(vscode, "ConfigurationTarget", { value: jest.fn(), configurable: true });
newMocks.mockGetConfiguration.mockReturnValue(createPersistentConfig());
Object.defineProperty(vscode.workspace, "getConfiguration", {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* This program and the accompanying materials are made available under the terms of the
* Eclipse Public License v2.0 which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-v20.html
*
* SPDX-License-Identifier: EPL-2.0
*
* Copyright Contributors to the Zowe Project.
*
*/

import { TreeViewUtils } from "../../../src/utils/TreeViewUtils";
import * as globals from "../../../src/globals";

describe("TreeViewUtils Unit Tests", () => {
it("refreshIconOnCollapse - generated listener function works as intended", () => {
const testTreeProvider = { mOnDidChangeTreeData: { fire: jest.fn() } } as any;
const listenerFn = TreeViewUtils.refreshIconOnCollapse(
[(node): boolean => (node.contextValue as any).includes(globals.DS_PDS_CONTEXT) as boolean],
testTreeProvider
);
const element = { label: "somenode", contextValue: globals.DS_PDS_CONTEXT } as any;
listenerFn({ element });
expect(testTreeProvider.mOnDidChangeTreeData.fire).toHaveBeenCalledWith(element);
});
});
3 changes: 2 additions & 1 deletion packages/zowe-explorer/i18n/sample/src/uss/actions.i18n.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@
"deleteUssPrompt.confirmation.message": "Are you sure you want to delete the following item?\nThis will permanently remove the following file or folder from your system.\n\n{0}",
"deleteUssPrompt.confirmation.cancel.log.debug": "Delete action was canceled.",
"ZoweUssNode.copyDownload.progress": "Copying file structure...",
"ZoweUssNode.copyUpload.progress": "Pasting files..."
"ZoweUssNode.copyUpload.progress": "Pasting files...",
"uss.paste.apiNotAvailable": "The paste operation is not supported for this node."
}
1 change: 1 addition & 0 deletions packages/zowe-explorer/src/dataset/DatasetTree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ export class DatasetTree extends ZoweTreeProvider implements IZoweTree<IZoweData
treeDataProvider: this,
canSelectMany: true,
});
this.treeView.onDidCollapseElement(TreeViewUtils.refreshIconOnCollapse([contextually.isPds, contextually.isDsSession], this));
}

/**
Expand Down
14 changes: 9 additions & 5 deletions packages/zowe-explorer/src/dataset/ZoweDatasetNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export class ZoweDatasetNode extends ZoweTreeNode implements IZoweDatasetTreeNod
if (icon) {
this.iconPath = icon.path;
}
if (!globals.ISTHEIA && this.getParent() && contextually.isSession(this.getParent())) {
if (!globals.ISTHEIA && contextually.isSession(this)) {
this.id = `${mParent?.id ?? mParent?.label?.toString() ?? "<root>"}.${this.label as string}`;
}
}
Expand Down Expand Up @@ -256,10 +256,14 @@ export class ZoweDatasetNode extends ZoweTreeNode implements IZoweDatasetTreeNod
.filter((label) => this.children.find((c) => (c.label as string) === label) == null)
.map((label) => elementChildren[label]);

this.children = this.children
.concat(newChildren)
.filter((c) => (c.label as string) in elementChildren)
.sort((a, b) => ((a.label as string) < (b.label as string) ? -1 : 1));
const removedChildren = this.children.filter((c) => !((c.label as string) in elementChildren));

if (newChildren.length > 0 || removedChildren.length > 0) {
this.children = this.children
.concat(newChildren)
.filter((c) => !removedChildren.includes(c))
.sort((a, b) => ((a.label as string) < (b.label as string) ? -1 : 1));
}
}

return this.children;
Expand Down
1 change: 1 addition & 0 deletions packages/zowe-explorer/src/job/ZosJobsProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ export class ZosJobsProvider extends ZoweTreeProvider implements IZoweTree<IZowe
treeDataProvider: this,
canSelectMany: true,
});
this.treeView.onDidCollapseElement(TreeViewUtils.refreshIconOnCollapse([contextually.isJob, contextually.isJobsSession], this));
}

public rename(_node: IZoweJobTreeNode): void {
Expand Down
2 changes: 1 addition & 1 deletion packages/zowe-explorer/src/job/ZoweJobNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export class Job extends ZoweTreeNode implements IZoweJobTreeNode {
this.iconPath = icon.path;
}

if (!globals.ISTHEIA && !(this instanceof Spool)) {
if (!globals.ISTHEIA && contextually.isSession(this)) {
this.id = `${mParent?.id ?? mParent?.label?.toString() ?? "<root>"}.${this.label as string}`;
}
}
Expand Down
2 changes: 0 additions & 2 deletions packages/zowe-explorer/src/job/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -387,8 +387,6 @@ async function deleteSingleJob(job: IZoweJobTreeNode, jobsProvider: IZoweTree<IZ
} catch (error) {
await errorHandling(error, job.getProfile().name);
}

Gui.showMessage(localize("deleteCommand.job", "Job {0} was deleted.", jobName));
}

async function deleteMultipleJobs(jobs: ReadonlyArray<IZoweJobTreeNode>, jobsProvider: IZoweTree<IZoweJobTreeNode>): Promise<void> {
Expand Down
2 changes: 1 addition & 1 deletion packages/zowe-explorer/src/shared/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ function appendSuffix(label: string): string {
const bracket = label.indexOf("(");
const split = bracket > -1 ? label.substr(0, bracket).split(".", limit) : label.split(".", limit);
for (let i = split.length - 1; i > 0; i--) {
if (["JCL", "CNTL"].includes(split[i])) {
if (["JCL", "JCLLIB", "CNTL"].includes(split[i])) {
return label.concat(".jcl");
}
if (["COBOL", "CBL", "COB", "SCBL"].includes(split[i])) {
Expand Down
1 change: 1 addition & 0 deletions packages/zowe-explorer/src/uss/USSTree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export class USSTree extends ZoweTreeProvider implements IZoweTree<IZoweUSSTreeN
treeDataProvider: this,
canSelectMany: true,
});
this.treeView.onDidCollapseElement(TreeViewUtils.refreshIconOnCollapse([contextually.isUssDirectory, contextually.isUssSession], this));
}

/**
Expand Down
Loading

0 comments on commit 8b2e848

Please sign in to comment.