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 Refresh button in Unix Attributes View #3354

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/zowe-explorer/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ All notable changes to the "vscode-extension-for-zowe" extension will be documen
- 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 where listing data sets resulted in an error after opening a data set with an encoding. [#3347](https://github.com/zowe/zowe-explorer-vscode/issues/3347)
- Fixed an issue with edit UNIX file attributes refresh button. [#3238](https://github.com/zowe/zowe-explorer-vscode/issues/3238)
- 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)

## `3.0.3`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,15 @@ describe("AttributeView unit tests", () => {
const updateAttributesMock = jest.spyOn(node, "setAttributes").mockImplementation();
const onUpdateMock = jest.fn();
const onUpdateMocked = new MockedProperty(ZoweUSSNode.prototype, "onUpdate", undefined, onUpdateMock);

beforeAll(() => {
const getAttributesMock = jest.spyOn(node, "getAttributes").mockImplementation();
const attrError = new Error("Failed to update attributes");
const attributes = {
owner: "owner",
group: "group",
perms: "-rwxrwxrwx",
};

beforeEach(() => {
jest.spyOn(ZoweExplorerApiRegister, "getUssApi").mockReturnValue({
updateAttributes: jest.fn(),
getTag: () => Promise.resolve("UTF-8"),
Expand All @@ -48,28 +55,26 @@ describe("AttributeView unit tests", () => {

afterAll(() => {
createDirMock.mockRestore();
onUpdateMocked[Symbol.dispose]();
});

it("refreshes properly when webview sends 'refresh' command", async () => {
// case 1: node is a root node
onUpdateMock.mockReturnValue(true as any);
await (view as any).onDidReceiveMessage({ command: "refresh" });
expect(treeProvider.refresh).toHaveBeenCalled();

// case 2: node is a child node
node.getParent = jest.fn().mockReturnValueOnce({ label: "parent node" } as IZoweUSSTreeNode);
await (view as any).onDidReceiveMessage({ command: "refresh" });
expect(treeProvider.refreshElement).toHaveBeenCalled();

expect(node.onUpdate).toHaveBeenCalledTimes(2);
});

it("dispatches node data to webview when 'ready' command is received", async () => {
const attrs = {
group: "group",
perms: "-rwxrwxrwx",
};
const getAttributesMock = jest.spyOn(ZoweUSSNode.prototype, "getAttributes").mockResolvedValue(attrs as any);
getAttributesMock.mockResolvedValue(attrs as any);
await (view as any).onDidReceiveMessage({ command: "ready" });
expect(view.panel.webview.postMessage).toHaveBeenCalledWith({
attributes: attrs,
Expand All @@ -79,21 +84,31 @@ describe("AttributeView unit tests", () => {
getAttributesMock.mockRestore();
});

it("updates attributes when 'update-attributes' command is received", async () => {
const getAttributesMock = jest.spyOn(ZoweUSSNode.prototype, "getAttributes");
it("updates attributes when 'update-attributes' command is received: case 1", async () => {
// case 1: no attributes provided from webview (sanity check)
updateAttrsApiMock.mockClear();
await (view as any).onDidReceiveMessage({ command: "update-attributes" });
expect(updateAttrsApiMock).not.toHaveBeenCalled();

const attributes = {
owner: "owner",
group: "group",
perms: "-rwxrwxrwx",
};
getAttributesMock.mockResolvedValue(attributes as any);

// case 3: attributes provided from webview, pass owner/group as IDs
await (view as any).onDidReceiveMessage({
command: "update-attributes",
attrs: {
owner: "1",
group: "9001",
perms: attributes.perms,
},
});
expect(updateAttributesMock).toHaveBeenCalled();
expect(view.panel.webview.postMessage).toHaveBeenCalled();
});

it("updates attributes when 'update-attributes' command is received: case 2", async () => {
// case 2: attributes provided from webview, pass owner/group as name
getAttributesMock.mockResolvedValue(attributes as any);

await (view as any).onDidReceiveMessage({
command: "update-attributes",
attrs: attributes,
Expand All @@ -102,8 +117,12 @@ describe("AttributeView unit tests", () => {
expect(view.panel.webview.postMessage).toHaveBeenCalledWith({
updated: true,
});
});

it("updates attributes when 'update-attributes' command is received: case 3", async () => {
// case 3: attributes provided from webview, pass owner/group as IDs
getAttributesMock.mockResolvedValue(attributes as any);

// case 2: attributes provided from webview, pass owner/group as IDs
await (view as any).onDidReceiveMessage({
command: "update-attributes",
attrs: {
Expand All @@ -117,12 +136,12 @@ describe("AttributeView unit tests", () => {
});

it("handles any errors while updating attributes", async () => {
const getAttributesMock = jest.spyOn(ZoweUSSNode.prototype, "getAttributes").mockRejectedValue(new Error("Failed to update attributes"));
updateAttributesMock.mockRejectedValue(attrError as any);
await (view as any).onDidReceiveMessage({
command: "update-attributes",
attrs: { owner: "someowner" },
});
expect(getAttributesMock).toHaveBeenCalled();

expect(view.panel.webview.postMessage).toHaveBeenCalledWith({
updated: false,
});
Expand Down
85 changes: 48 additions & 37 deletions packages/zowe-explorer/l10n/bundle.l10n.json
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,42 @@
"Profile auth error": "Profile auth error",
"Profile is not authenticated, please log in to continue": "Profile is not authenticated, please log in to continue",
"Retrieving response from USS list API": "Retrieving response from USS list API",
"The 'move' function is not implemented for this USS API.": "The 'move' function is not implemented for this USS API.",
"Failed to move {0}/File path": {
"message": "Failed to move {0}",
"comment": [
"File path"
]
},
"Profile does not exist for this file.": "Profile does not exist for this file.",
"Saving USS file...": "Saving USS file...",
"Failed to rename {0}/File path": {
"message": "Failed to rename {0}",
"comment": [
"File path"
]
},
"Failed to delete {0}/File name": {
"message": "Failed to delete {0}",
"comment": [
"File name"
]
},
"No error details given": "No error details given",
"Error fetching destination {0} for paste action: {1}/USS pathError message": {
"message": "Error fetching destination {0} for paste action: {1}",
"comment": [
"USS path",
"Error message"
]
},
"Failed to copy {0} to {1}/Source pathDestination path": {
"message": "Failed to copy {0} to {1}",
"comment": [
"Source path",
"Destination path"
]
},
"Downloaded: {0}/Download time": {
"message": "Downloaded: {0}",
"comment": [
Expand Down Expand Up @@ -262,42 +298,6 @@
"initializeUSSFavorites.error.buttonRemove": "initializeUSSFavorites.error.buttonRemove",
"File does not exist. It may have been deleted.": "File does not exist. It may have been deleted.",
"Pulling from Mainframe...": "Pulling from Mainframe...",
"The 'move' function is not implemented for this USS API.": "The 'move' function is not implemented for this USS API.",
"Failed to move {0}/File path": {
"message": "Failed to move {0}",
"comment": [
"File path"
]
},
"Profile does not exist for this file.": "Profile does not exist for this file.",
"Saving USS file...": "Saving USS file...",
"Failed to rename {0}/File path": {
"message": "Failed to rename {0}",
"comment": [
"File path"
]
},
"Failed to delete {0}/File name": {
"message": "Failed to delete {0}",
"comment": [
"File name"
]
},
"No error details given": "No error details given",
"Error fetching destination {0} for paste action: {1}/USS pathError message": {
"message": "Error fetching destination {0} for paste action: {1}",
"comment": [
"USS path",
"Error message"
]
},
"Failed to copy {0} to {1}/Source pathDestination path": {
"message": "Failed to copy {0} to {1}",
"comment": [
"Source path",
"Destination path"
]
},
"{0} location/Node type": {
"message": "{0} location",
"comment": [
Expand Down Expand Up @@ -898,7 +898,6 @@
"The number of data sets that are about to be searched"
]
},
"Percent Complete": "Percent Complete",
"Failed to upload changes for {0}: {1}/Build file hyperlinkError message": {
"message": "Failed to upload changes for {0}: {1}",
"comment": [
Expand All @@ -907,6 +906,18 @@
]
},
"Error encountered while activating and initializing logger": "Error encountered while activating and initializing logger",
"Insufficient read permissions for {0} in local storage./Local storage key": {
"message": "Insufficient read permissions for {0} in local storage.",
"comment": [
"Local storage key"
]
},
"Insufficient write permissions for {0} in local storage./Local storage key": {
"message": "Insufficient write permissions for {0} in local storage.",
"comment": [
"Local storage key"
]
},
"Add Credentials": "Add Credentials",
"Add username and password for basic authentication": "Add username and password for basic authentication",
"Update stored username and password": "Update stored username and password",
Expand Down
21 changes: 11 additions & 10 deletions packages/zowe-explorer/l10n/poeditor.json
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,15 @@
"Profile auth error": "",
"Profile is not authenticated, please log in to continue": "",
"Retrieving response from USS list API": "",
"The 'move' function is not implemented for this USS API.": "",
"Failed to move {0}": "",
"Profile does not exist for this file.": "",
"Saving USS file...": "",
"Failed to rename {0}": "",
"Failed to delete {0}": "",
"No error details given": "",
"Error fetching destination {0} for paste action: {1}": "",
"Failed to copy {0} to {1}": "",
"Downloaded: {0}": "",
"Encoding: {0}": "",
"Binary": "",
Expand Down Expand Up @@ -568,15 +577,6 @@
"initializeUSSFavorites.error.buttonRemove": "",
"File does not exist. It may have been deleted.": "",
"Pulling from Mainframe...": "",
"The 'move' function is not implemented for this USS API.": "",
"Failed to move {0}": "",
"Profile does not exist for this file.": "",
"Saving USS file...": "",
"Failed to rename {0}": "",
"Failed to delete {0}": "",
"No error details given": "",
"Error fetching destination {0} for paste action: {1}": "",
"Failed to copy {0} to {1}": "",
"{0} location": "",
"Choose a location to create the {0}": "",
"Name of file or directory": "",
Expand Down Expand Up @@ -813,9 +813,10 @@
"Contents": "",
"Open": "",
"Are you sure you want to search {0} data sets and members?": "",
"Percent Complete": "",
"Failed to upload changes for {0}: {1}": "",
"Error encountered while activating and initializing logger": "",
"Insufficient read permissions for {0} in local storage.": "",
"Insufficient write permissions for {0} in local storage.": "",
"Add Credentials": "",
"Add username and password for basic authentication": "",
"Update stored username and password": "",
Expand Down
35 changes: 14 additions & 21 deletions packages/zowe-explorer/src/trees/uss/USSAttributeView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
*/

import { Types, Gui, MainframeInteraction, IZoweUSSTreeNode, WebView } from "@zowe/zowe-explorer-api";
import { Disposable, ExtensionContext } from "vscode";
import { ExtensionContext } from "vscode";
import { ZoweExplorerApiRegister } from "../../extending/ZoweExplorerApiRegister";
import { SharedContext } from "../shared/SharedContext";
import * as vscode from "vscode";
Expand All @@ -22,8 +22,6 @@ export class USSAttributeView extends WebView {
private readonly ussApi: MainframeInteraction.IUss;
private readonly canUpdate: boolean;

private onUpdateDisposable: Disposable;

public constructor(context: ExtensionContext, treeProvider: Types.IZoweUSSTreeType, node: IZoweUSSTreeNode) {
const label = node.label ? `Edit Attributes: ${node.label as string}` : "Edit Attributes";
super(label, "edit-attributes", context, {
Expand All @@ -45,14 +43,11 @@ export class USSAttributeView extends WebView {
switch (message.command) {
case "refresh":
if (this.canUpdate) {
this.onUpdateDisposable = this.ussNode.onUpdate(async (node) => {
await this.attachTag(node);
await this.panel.webview.postMessage({
attributes: await this.ussNode.getAttributes(),
name: node.fullPath,
readonly: this.ussApi.updateAttributes == null,
});
this.onUpdateDisposable.dispose();
Comment on lines -48 to -55
Copy link
Member

@traeok traeok Dec 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'll still need the onUpdate event listener. Once the node has been updated in the tree, the attributes in the webview are updated because this callback will be fired in getChildren (see line 233 in ZoweUSSNode.ts). Then, on lines 53-57 in this file, the refresh is called on the tree node to get the new attributes.

It seems like removing this causes every other refresh call to work - for example, if I change the permissions in my terminal and then refresh in ZE, the first refresh does not show any changes, but the second one pulls the right permissions:
ZE-EditAttributesRefresh

Also, the onUpdate callback is working as expected in v2. It seems like the onUpdateEmitter might not be firing in v3.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm seeing the same behavior as Trae.
Also, the GIF shows that the Apply changes button is enabled the second time the refresh button is clicked, even though nothing the user (I) didn't make changes 😅

await this.attachTag(this.ussNode);
await this.panel.webview.postMessage({
attributes: await this.ussNode.getAttributes(),
name: this.ussNode.fullPath,
readonly: this.ussApi.updateAttributes == null,
});

if (this.ussNode.getParent()) {
Expand Down Expand Up @@ -103,38 +98,36 @@ export class USSAttributeView extends WebView {

try {
if (Object.keys(message?.attrs).length > 0) {
const oldAttrs = await this.ussNode.getAttributes();
const oldAttrs: Partial<Types.FileAttributes> = (await this.ussNode.getAttributes()) ?? {};
const attrs = message.attrs;
const newAttrs: Partial<Types.FileAttributes> = {};
if (!isNaN(parseInt(attrs.owner))) {
if (attrs.owner && !isNaN(parseInt(attrs.owner))) {
const uid = parseInt(attrs.owner);
newAttrs.uid = uid;

// set owner to the UID to prevent mismatched UIDs/owners
newAttrs.owner = attrs.owner;
} else if (oldAttrs.owner !== attrs.owner) {
} else if (attrs.owner && (!oldAttrs.owner || oldAttrs.owner !== attrs.owner)) {
newAttrs.owner = attrs.owner;
}

if (!isNaN(parseInt(attrs.group))) {
// must provide owner when changing group
if (attrs.group && attrs.owner && !isNaN(parseInt(attrs.group))) {
const gid = parseInt(attrs.group);
// must provide owner when changing group
newAttrs.owner = attrs.owner;
newAttrs.gid = gid;

// set group to the GID to prevent mismatched GIDs/groups
newAttrs.group = attrs.group;
} else if (oldAttrs.group !== attrs.group) {
// must provide owner when changing group
} else if (attrs.owner && attrs.group && (!oldAttrs.group || oldAttrs.group !== attrs.group)) {
newAttrs.owner = attrs.owner;
newAttrs.group = attrs.group;
}

if (oldAttrs.perms !== attrs.perms) {
if (attrs.perms && (!oldAttrs.perms || oldAttrs.perms !== attrs.perms)) {
newAttrs.perms = attrs.perms;
}

if (oldAttrs.tag !== attrs.tag && this.ussApi.getTag) {
if (attrs.tag && (!oldAttrs.tag || oldAttrs.tag !== attrs.tag) && this.ussApi.getTag) {
newAttrs.tag = attrs.tag;
}

Expand Down
Loading