Skip to content

Commit f19b977

Browse files
misha-dbmisha-db
committed
Split uploadFile into toolbar and context-menu commands
When the toolbar button is clicked, VS Code passes the currently-selected tree node as `element`, making it impossible to distinguish toolbar vs context-menu invocations from a single handler. Changes: - Add `databricks.wsfs.uploadFile.toolbar` command bound to `view/title`; the existing `databricks.wsfs.uploadFile` stays on `view/item/context` only. - Both handlers resolve the target folder directly from `element` (toolbar variant via `resolveTargetElementForToolbar`, which gates on `treeView.selection` to detect whether anything is truly selected). - Extract shared upload logic into private `doUploadFile`. - Remove the now-unused `resolveTargetElement` helper. - Update unit tests: rename mislabelled "title bar" cases, add dedicated suites for `createFolderFromToolbar` and `uploadFileFromToolbar` covering the full selection-state matrix.
1 parent 3e153fe commit f19b977

4 files changed

Lines changed: 147 additions & 66 deletions

File tree

‎packages/databricks-vscode/package.json‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,13 @@
189189
"enablement": "databricks.context.activated && databricks.context.loggedIn && !databricks.context.remoteMode",
190190
"category": "Databricks"
191191
},
192+
{
193+
"command": "databricks.wsfs.createFolder.toolbar",
194+
"title": "Create Folder",
195+
"icon": "$(new-folder)",
196+
"enablement": "databricks.context.activated && databricks.context.loggedIn && !databricks.context.remoteMode",
197+
"category": "Databricks"
198+
},
192199
{
193200
"command": "databricks.wsfs.openInBrowser",
194201
"title": "Open in Browser",
@@ -213,6 +220,12 @@
213220
"icon": "$(cloud-upload)",
214221
"category": "Databricks"
215222
},
223+
{
224+
"command": "databricks.wsfs.uploadFile.toolbar",
225+
"title": "Upload File",
226+
"icon": "$(cloud-upload)",
227+
"category": "Databricks"
228+
},
216229
{
217230
"command": "databricks.wsfs.downloadFile",
218231
"title": "Download",
@@ -709,7 +722,7 @@
709722
"group": "navigation@1"
710723
},
711724
{
712-
"command": "databricks.wsfs.createFolder",
725+
"command": "databricks.wsfs.createFolder.toolbar",
713726
"when": "view == workspaceFsView",
714727
"group": "navigation@1"
715728
},
@@ -724,7 +737,7 @@
724737
"group": "navigation@1"
725738
},
726739
{
727-
"command": "databricks.wsfs.uploadFile",
740+
"command": "databricks.wsfs.uploadFile.toolbar",
728741
"when": "view == workspaceFsView",
729742
"group": "navigation@1"
730743
},
@@ -1113,6 +1126,10 @@
11131126
"command": "databricks.wsfs.createFolder",
11141127
"when": "!databricks.context.remoteMode"
11151128
},
1129+
{
1130+
"command": "databricks.wsfs.createFolder.toolbar",
1131+
"when": "false"
1132+
},
11161133
{
11171134
"command": "databricks.wsfs.refresh",
11181135
"when": "!databricks.context.remoteMode"
@@ -1137,6 +1154,10 @@
11371154
"command": "databricks.wsfs.uploadFile",
11381155
"when": "false"
11391156
},
1157+
{
1158+
"command": "databricks.wsfs.uploadFile.toolbar",
1159+
"when": "false"
1160+
},
11401161
{
11411162
"command": "databricks.utils.openExternal",
11421163
"when": "false"

‎packages/databricks-vscode/src/extension.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,11 @@ export async function activate(
424424
workspaceFsCommands.createFolder,
425425
workspaceFsCommands
426426
),
427+
telemetry.registerCommand(
428+
"databricks.wsfs.createFolder.toolbar",
429+
workspaceFsCommands.createFolderFromToolbar,
430+
workspaceFsCommands
431+
),
427432
telemetry.registerCommand(
428433
"databricks.wsfs.openInBrowser",
429434
workspaceFsCommands.openInBrowser,
@@ -444,6 +449,11 @@ export async function activate(
444449
workspaceFsCommands.uploadFile,
445450
workspaceFsCommands
446451
),
452+
telemetry.registerCommand(
453+
"databricks.wsfs.uploadFile.toolbar",
454+
workspaceFsCommands.uploadFileFromToolbar,
455+
workspaceFsCommands
456+
),
447457
telemetry.registerCommand(
448458
"databricks.wsfs.downloadFile",
449459
workspaceFsCommands.downloadFile,

‎packages/databricks-vscode/src/workspace-fs/WorkspaceFsCommands.test.ts‎

Lines changed: 70 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ describe("WorkspaceFsCommands – target folder resolution", () => {
3838

3939
const entityA = makeEntity("/Users/me/A");
4040
const entityB = makeEntity("/Users/me/B");
41+
const fileEntity = {
42+
path: "/Users/me/A/note.py",
43+
type: "FILE",
44+
} as unknown as WorkspaceFsEntity;
4145

4246
beforeEach(() => {
4347
fakeTreeView = new FakeTreeView();
@@ -71,95 +75,117 @@ describe("WorkspaceFsCommands – target folder resolution", () => {
7175
};
7276
});
7377

74-
describe("createFolder", () => {
75-
it("title bar, nothing selected → targets root", async () => {
78+
// Context-menu command: element is the right-clicked node; treeView
79+
// selection is irrelevant.
80+
describe("createFolder (context menu)", () => {
81+
it("no element → targets root", async () => {
7682
await commands.createFolder(undefined);
7783
assert.strictEqual(capturedRootPath, ROOT_PATH);
7884
});
7985

80-
it("title bar, A selected → targets A", async () => {
81-
fakeTreeView.simulateSelect(entityA);
86+
it("element=A → targets A", async () => {
8287
await commands.createFolder(entityA);
8388
assert.strictEqual(capturedRootPath, entityA.path);
8489
});
8590

86-
it("title bar, A selected then deselected → targets root", async () => {
87-
fakeTreeView.simulateSelect(entityA);
88-
fakeTreeView.simulateSelect(undefined);
89-
await commands.createFolder(undefined);
90-
assert.strictEqual(capturedRootPath, ROOT_PATH);
91-
});
92-
93-
it("context menu on B while A is selected → targets B", async () => {
91+
it("element=B while A is selected → targets B", async () => {
9492
fakeTreeView.simulateSelect(entityA);
95-
// Right-click on B does NOT update selection; selection[0] is still A
9693
await commands.createFolder(entityB);
9794
assert.strictEqual(capturedRootPath, entityB.path);
9895
});
96+
});
9997

100-
it("context menu on B with nothing selected → targets B", async () => {
101-
await commands.createFolder(entityB);
102-
assert.strictEqual(capturedRootPath, entityB.path);
98+
// Toolbar command: VS Code passes the currently-selected node as element.
99+
// resolveTargetElementForToolbar uses treeView.selection to detect whether
100+
// something is truly selected; if not, it falls back to root.
101+
describe("createFolderFromToolbar (toolbar)", () => {
102+
it("nothing selected → targets root", async () => {
103+
await commands.createFolderFromToolbar(undefined);
104+
assert.strictEqual(capturedRootPath, ROOT_PATH);
103105
});
104106

105-
it("context menu on A while A is selected → targets A", async () => {
107+
it("A selected, toolbar clicked → targets A", async () => {
106108
fakeTreeView.simulateSelect(entityA);
107-
// Right-click on the already-selected item: element === selection[0]
108-
await commands.createFolder(entityA);
109+
// VS Code passes the selected node as element on toolbar click
110+
await commands.createFolderFromToolbar(entityA);
109111
assert.strictEqual(capturedRootPath, entityA.path);
110112
});
113+
114+
it("selection cleared before toolbar click → targets root", async () => {
115+
fakeTreeView.simulateSelect(entityA);
116+
fakeTreeView.simulateSelect(undefined);
117+
await commands.createFolderFromToolbar(undefined);
118+
assert.strictEqual(capturedRootPath, ROOT_PATH);
119+
});
120+
121+
it("element passed but nothing selected (edge case) → targets root", async () => {
122+
// treeView.selection is empty; resolveTargetElementForToolbar
123+
// ignores the element and returns undefined → root
124+
await commands.createFolderFromToolbar(entityA);
125+
assert.strictEqual(capturedRootPath, ROOT_PATH);
126+
});
111127
});
112128

113-
describe("uploadFile", () => {
114-
// uploadFile bails early when workspaceClient is undefined, before
115-
// reaching getValidRoot. Provide a non-null client so the activeElement
116-
// logic is exercised.
129+
describe("uploadFile (context menu)", () => {
130+
// doUploadFile checks workspaceClient before reaching getValidRoot;
131+
// provide a non-null client so root-path resolution is exercised.
117132
beforeEach(() => {
118133
when(mockConnectionManager.workspaceClient).thenReturn({} as any);
119134
});
120135

121-
it("title bar, nothing selected → targets root", async () => {
136+
it("no element → targets root", async () => {
122137
await commands.uploadFile(undefined);
123138
assert.strictEqual(capturedRootPath, ROOT_PATH);
124139
});
125140

126-
it("title bar, A selected → targets A", async () => {
127-
fakeTreeView.simulateSelect(entityA);
141+
it("element=A (directory) → targets A", async () => {
128142
await commands.uploadFile(entityA);
129143
assert.strictEqual(capturedRootPath, entityA.path);
130144
});
131145

132-
it("title bar, A selected then deselected → targets root", async () => {
146+
it("element=B while A is selected → targets B", async () => {
133147
fakeTreeView.simulateSelect(entityA);
134-
fakeTreeView.simulateSelect(undefined);
135-
await commands.uploadFile(undefined);
148+
await commands.uploadFile(entityB);
149+
assert.strictEqual(capturedRootPath, entityB.path);
150+
});
151+
152+
it("element=file (non-directory) → targets root", async () => {
153+
await commands.uploadFile(fileEntity);
136154
assert.strictEqual(capturedRootPath, ROOT_PATH);
137155
});
156+
});
138157

139-
it("context menu on B while A is selected → targets B", async () => {
140-
fakeTreeView.simulateSelect(entityA);
141-
await commands.uploadFile(entityB);
142-
assert.strictEqual(capturedRootPath, entityB.path);
158+
describe("uploadFileFromToolbar (toolbar)", () => {
159+
beforeEach(() => {
160+
when(mockConnectionManager.workspaceClient).thenReturn({} as any);
143161
});
144162

145-
it("context menu on B with nothing selected → targets B", async () => {
146-
await commands.uploadFile(entityB);
147-
assert.strictEqual(capturedRootPath, entityB.path);
163+
it("nothing selected → targets root", async () => {
164+
await commands.uploadFileFromToolbar(undefined);
165+
assert.strictEqual(capturedRootPath, ROOT_PATH);
148166
});
149167

150-
it("context menu on A while A is selected → targets A", async () => {
168+
it("A selected, toolbar clicked → targets A", async () => {
151169
fakeTreeView.simulateSelect(entityA);
152-
await commands.uploadFile(entityA);
170+
await commands.uploadFileFromToolbar(entityA);
153171
assert.strictEqual(capturedRootPath, entityA.path);
154172
});
155173

156-
it("title bar, file (non-directory) selected → targets root", async () => {
157-
const fileEntity = {
158-
path: "/Users/me/A/note.py",
159-
type: "FILE",
160-
} as unknown as WorkspaceFsEntity;
174+
it("selection cleared before toolbar click → targets root", async () => {
175+
fakeTreeView.simulateSelect(entityA);
176+
fakeTreeView.simulateSelect(undefined);
177+
await commands.uploadFileFromToolbar(undefined);
178+
assert.strictEqual(capturedRootPath, ROOT_PATH);
179+
});
180+
181+
it("element passed but nothing selected (edge case) → targets root", async () => {
182+
await commands.uploadFileFromToolbar(entityA);
183+
assert.strictEqual(capturedRootPath, ROOT_PATH);
184+
});
185+
186+
it("file selected, toolbar clicked → targets root", async () => {
161187
fakeTreeView.simulateSelect(fileEntity);
162-
await commands.uploadFile(fileEntity);
188+
await commands.uploadFileFromToolbar(fileEntity);
163189
assert.strictEqual(capturedRootPath, ROOT_PATH);
164190
});
165191
});

‎packages/databricks-vscode/src/workspace-fs/WorkspaceFsCommands.ts‎

Lines changed: 44 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -18,21 +18,14 @@ const withLogContext = logging.withLogContext;
1818

1919
export class WorkspaceFsCommands implements Disposable {
2020
private disposables: Disposable[] = [];
21-
private selectedElement: WorkspaceFsEntity | undefined;
2221

2322
constructor(
2423
private workspaceFolderManager: WorkspaceFolderManager,
2524
private connectionManager: ConnectionManager,
2625
private workspaceFsDataProvider: WorkspaceFsDataProvider,
2726
private fsp: WorkspaceFsFileSystemProvider,
2827
private readonly treeView: TreeView<WorkspaceFsEntity>
29-
) {
30-
this.disposables.push(
31-
treeView.onDidChangeSelection((e) => {
32-
this.selectedElement = e.selection[0];
33-
})
34-
);
35-
}
28+
) {}
3629

3730
@withLogContext(Loggers.Extension)
3831
async getValidRoot(
@@ -78,21 +71,40 @@ export class WorkspaceFsCommands implements Disposable {
7871
return root;
7972
}
8073

81-
private resolveTargetElement(
74+
@withLogContext(Loggers.Extension)
75+
async createFolder(element?: WorkspaceFsEntity, @context ctx?: Context) {
76+
const rootPath =
77+
element?.path ??
78+
this.connectionManager.databricksWorkspace?.currentFsRoot.path;
79+
return this.doCreateFolder(rootPath, ctx);
80+
}
81+
82+
private resolveTargetElementForToolbar(
8283
element?: WorkspaceFsEntity
8384
): WorkspaceFsEntity | undefined {
84-
return element !== this.treeView.selection[0]
85-
? element
86-
: this.selectedElement;
85+
if (this.treeView.selection[0] === undefined) {
86+
return undefined;
87+
}
88+
return element;
8789
}
8890

8991
@withLogContext(Loggers.Extension)
90-
async createFolder(element?: WorkspaceFsEntity, @context ctx?: Context) {
91-
const activeElement = this.resolveTargetElement(element);
92+
async createFolderFromToolbar(
93+
element?: WorkspaceFsEntity,
94+
@context ctx?: Context
95+
) {
96+
const activeElement = this.resolveTargetElementForToolbar(element);
9297
const rootPath =
9398
activeElement?.path ??
9499
this.connectionManager.databricksWorkspace?.currentFsRoot.path;
100+
return this.doCreateFolder(rootPath, ctx);
101+
}
95102

103+
@withLogContext(Loggers.Extension)
104+
private async doCreateFolder(
105+
rootPath: string | undefined,
106+
@context ctx?: Context
107+
) {
96108
const root = await this.getValidRoot(rootPath, ctx);
97109
if (!root) {
98110
return;
@@ -190,19 +202,31 @@ export class WorkspaceFsCommands implements Disposable {
190202
}
191203

192204
async uploadFile(element?: WorkspaceFsEntity) {
193-
const client = this.connectionManager.workspaceClient;
194-
if (!client) {
195-
window.showErrorMessage("Please login first to upload a file");
196-
return;
197-
}
205+
const rootPath =
206+
(element?.type === "DIRECTORY" || element?.type === "REPO"
207+
? element?.path
208+
: undefined) ??
209+
this.connectionManager.databricksWorkspace?.currentFsRoot.path;
210+
return this.doUploadFile(rootPath);
211+
}
198212

199-
const activeElement = this.resolveTargetElement(element);
213+
async uploadFileFromToolbar(element?: WorkspaceFsEntity) {
214+
const activeElement = this.resolveTargetElementForToolbar(element);
200215
const rootPath =
201216
(activeElement?.type === "DIRECTORY" ||
202217
activeElement?.type === "REPO"
203218
? activeElement?.path
204219
: undefined) ??
205220
this.connectionManager.databricksWorkspace?.currentFsRoot.path;
221+
return this.doUploadFile(rootPath);
222+
}
223+
224+
private async doUploadFile(rootPath: string | undefined) {
225+
const client = this.connectionManager.workspaceClient;
226+
if (!client) {
227+
window.showErrorMessage("Please login first to upload a file");
228+
return;
229+
}
206230

207231
const root = await this.getValidRoot(rootPath);
208232
if (!root) {

0 commit comments

Comments
 (0)