Skip to content

feat(tree-explorer): add ability to set preset connections in settings.json VSCODE-665 #909

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.

Sign up for GitHub

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

Merged
gagik merged 10 commits into from
Jan 20, 2025
Merged

feat(tree-explorer): add ability to set preset connections in settings.json VSCODE-665 #909

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
db87e39
add preset settings
gagik Jan 12, 2025
06b1e90
add tests
gagik Jan 13, 2025
d2d58e6
feat(tree-explorer): add reactivity, sample value and remove configur…
gagik Jan 16, 2025
bf9cd64
feat(tree-explorer): automatically derive workspace or global settings
gagik Jan 16, 2025
0128835
feat(tree-explorer): allow running edit preset connections from the h…
gagik Jan 16, 2025
0850375
fix(tree-explorer): update tests to use inspect and check for global …
gagik Jan 16, 2025
bf39230
refactor: rename to presetConnections, update test strict equality, u…
gagik Jan 20, 2025
a72bad8
test: add clear connection tests and use test strict equality
gagik Jan 20, 2025
6cf0f1c
refactor: changes from review
gagik Jan 20, 2025
63c719d
fix(tree-explorer): fix issues with collapsing active connection
gagik Jan 20, 2025
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
11 changes: 10 additions & 1 deletion .vscode/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,14 @@
"out": true
},
// Turn off tsc task auto detection since we have the necessary tasks as npm scripts
"typescript.tsc.autoDetect": "off"
"typescript.tsc.autoDetect": "off",
"mdb.presetSavedConnections":[
{
"name": "Local Test Connection",
"connectionString": "mongodb://localhost:27017"
}
],
"conventionalCommits.scopes": [
"tree-explorer"
]
}
70 changes: 60 additions & 10 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,10 @@
"command": "mdb.copyConnectionString",
"title": "Copy Connection String"
},
{
"command": "mdb.openWorkspaceSettingsFile",
"title": "Open Workspace Settings"
},
{
"command": "mdb.renameConnection",
"title": "Rename Connection..."
Expand Down Expand Up @@ -499,32 +503,32 @@
"view/item/context": [
{
"command": "mdb.addDatabase",
"when": "view == mongoDBConnectionExplorer && viewItem == connectedConnectionTreeItem && mdb.isAtlasStreams == false",
"when": "view == mongoDBConnectionExplorer && (viewItem == connectedConnectionTreeItem || viewItem == connectedImmutableConnectionTreeItem) && mdb.isAtlasStreams == false",
"group": "inline"
},
{
"command": "mdb.addDatabase",
"when": "view == mongoDBConnectionExplorer && viewItem == connectedConnectionTreeItem && mdb.isAtlasStreams == false",
"when": "view == mongoDBConnectionExplorer && (viewItem == connectedConnectionTreeItem || viewItem == connectedImmutableConnectionTreeItem) && mdb.isAtlasStreams == false",
"group": "1@1"
},
{
"command": "mdb.addStreamProcessor",
"when": "view == mongoDBConnectionExplorer && viewItem == connectedConnectionTreeItem && mdb.isAtlasStreams == true",
"when": "view == mongoDBConnectionExplorer && (viewItem == connectedConnectionTreeItem || viewItem == connectedImmutableConnectionTreeItem) && mdb.isAtlasStreams == true",
"group": "inline"
},
{
"command": "mdb.addStreamProcessor",
"when": "view == mongoDBConnectionExplorer && viewItem == connectedConnectionTreeItem && mdb.isAtlasStreams == true",
"when": "view == mongoDBConnectionExplorer && (viewItem == connectedConnectionTreeItem || viewItem == connectedImmutableConnectionTreeItem) && mdb.isAtlasStreams == true",
"group": "1@1"
},
{
"command": "mdb.refreshConnection",
"when": "view == mongoDBConnectionExplorer && viewItem == connectedConnectionTreeItem",
"when": "view == mongoDBConnectionExplorer && (viewItem == connectedConnectionTreeItem || viewItem == connectedImmutableConnectionTreeItem)",
"group": "1@2"
},
{
"command": "mdb.treeViewOpenMongoDBShell",
"when": "view == mongoDBConnectionExplorer && viewItem == connectedConnectionTreeItem",
"when": "view == mongoDBConnectionExplorer && (viewItem == connectedConnectionTreeItem || viewItem == connectedImmutableConnectionTreeItem)",
"group": "2@1"
},
{
Expand All @@ -537,14 +541,19 @@
"when": "view == mongoDBConnectionExplorer && viewItem == connectedConnectionTreeItem",
"group": "3@2"
},
{
"command": "mdb.openWorkspaceSettingsFile",
Copy link
Contributor Author

@gagik gagik Jan 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think it'd be nice to include ability to quickly open workspace settings here when the item is preset (piggybacking on an existing VSCode action).

We can't easily detect where this preset comes from (i.e. it could also come from user settings) so we could also include a shortcut to open user settings, but maybe we don't want to encourage that and assume they know what they're doing by that point?

If someone that defined this in their user settings (as opposed to workspace) and clicks the Open Workspace Settings, it'd open or create a .vscode/settings.json file in the workspace, and setting things there would override user settings, which seems reasonable.

"when": "view == mongoDBConnectionExplorer && (viewItem == connectedImmutableConnectionTreeItem)",
"group": "3@2"
},
{
"command": "mdb.copyConnectionString",
"when": "view == mongoDBConnectionExplorer && viewItem == connectedConnectionTreeItem",
"when": "view == mongoDBConnectionExplorer && (viewItem == connectedConnectionTreeItem || viewItem == connectedImmutableConnectionTreeItem)",
"group": "4@1"
},
{
"command": "mdb.disconnectFromConnectionTreeItem",
"when": "view == mongoDBConnectionExplorer && viewItem == connectedConnectionTreeItem",
"when": "view == mongoDBConnectionExplorer && (viewItem == connectedConnectionTreeItem || viewItem == connectedImmutableConnectionTreeItem)",
"group": "5@1"
},
{
Expand All @@ -559,7 +568,7 @@
},
{
"command": "mdb.connectToConnectionTreeItem",
"when": "view == mongoDBConnectionExplorer && viewItem == disconnectedConnectionTreeItem",
"when": "view == mongoDBConnectionExplorer && (viewItem == disconnectedConnectionTreeItem || viewItem == disconnectedImmutableConnectionTreeItem)",
"group": "1@1"
},
{
Expand All @@ -572,9 +581,14 @@
"when": "view == mongoDBConnectionExplorer && viewItem == disconnectedConnectionTreeItem",
"group": "2@2"
},
{
"command": "mdb.openWorkspaceSettingsFile",
"when": "view == mongoDBConnectionExplorer && (viewItem == disconnectedImmutableConnectionTreeItem)",
"group": "2@2"
},
{
"command": "mdb.copyConnectionString",
"when": "view == mongoDBConnectionExplorer && viewItem == disconnectedConnectionTreeItem",
"when": "view == mongoDBConnectionExplorer && (viewItem == disconnectedConnectionTreeItem || viewItem == disconnectedImmutableConnectionTreeItem)",
"group": "3@1"
},
{
Expand Down Expand Up @@ -1171,6 +1185,42 @@
"type": "string",
"default": "",
"description": "Specify a shell command that is run to start the browser for authenticating with the OIDC identity provider for the server connection. Leave this empty for default browser."
},
"mdb.presetSavedConnections": {
"scope": "window",
"type": "array",
"description": "Defines preset saved connections.",
"examples": [
[
{
"name": "Local Test Connection",
"connectionString": "mongodb://localhost:27017"
}
]
],
"items": {
"type": "object",
"examples": [
{
"name": "Local Test Connection",
"connectionString": "mongodb://localhost:27017"
}
],
"properties": {
"name": {
"type": "string",
"description": "Name of the connection."
},
"connectionString": {
"type": "string",
"description": "Connection string. Do not store sensitive credentials here."
}
},
"required": [
"name",
"connectionString"
]
}
}
}
},
Expand Down
1 change: 1 addition & 0 deletions src/commands/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ enum EXTENSION_COMMANDS {
MDB_EDIT_CONNECTION = 'mdb.editConnection',
MDB_REFRESH_CONNECTION = 'mdb.refreshConnection',
MDB_COPY_CONNECTION_STRING = 'mdb.copyConnectionString',
MDB_OPEN_WORKSPACE_SETTINGS_FILE = 'mdb.openWorkspaceSettingsFile',
MDB_REMOVE_CONNECTION_TREE_VIEW = 'mdb.treeItemRemoveConnection',
MDB_RENAME_CONNECTION = 'mdb.renameConnection',
MDB_ADD_DATABASE = 'mdb.addDatabase',
Expand Down
2 changes: 2 additions & 0 deletions src/connectionController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ export default class ConnectionController {
}

async loadSavedConnections(): Promise<void> {
this._connections = Object.create(null);
Copy link
Contributor

@nirinchev nirinchev Jan 17, 2025

Choose a reason for hiding this comment

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

Is this fixing a bug where deleted saved connections would show up because they're not overridden in the for loop? Couldn't spot a test that verifies the behavior before was incorrect - am I just blind or should we add one?

Copy link
Contributor Author

@gagik gagik Jan 20, 2025

Choose a reason for hiding this comment

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

Yeah, this hasn't been an issue because we'd only run loadSavedConnections at startup / when this._connections would be not yet defined. Now with the reloading on save this becomes more obvious. I'll add a test.


const loadedConnections = await this._connectionStorage.loadConnections();

for (const connection of loadedConnections) {
Expand Down
24 changes: 14 additions & 10 deletions src/explorer/connectionTreeItem.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@
import type TreeItemParent from './treeItemParentInterface';
import StreamProcessorTreeItem from './streamProcessorTreeItem';

export enum ConnectionItemContextValues {
disconnected = 'disconnectedConnectionTreeItem',
connected = 'connectedConnectionTreeItem',
}
export type ConnectionItemContextValue = `${'disconnected' | 'connected'}${
| ''
Copy link
Contributor

@nirinchev nirinchev Jan 17, 2025

Choose a reason for hiding this comment

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

[extreme nit] The leading | makes the formatting really awkard - is it even worse without it?

Copy link
Contributor Author

@gagik gagik Jan 20, 2025

Choose a reason for hiding this comment

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

This gets automatically added by the linter 🤷

| 'Immutable'}ConnectionTreeItem`;

function getIconPath(isActiveConnection: boolean): {
light: string;
Expand All @@ -39,7 +38,7 @@
extends vscode.TreeItem
implements TreeItemParent, vscode.TreeDataProvider<ConnectionTreeItem>
{
contextValue = ConnectionItemContextValues.disconnected;
contextValue: ConnectionItemContextValue = 'disconnectedConnectionTreeItem';

private _childrenCache: {
[key: string]: DatabaseTreeItem | StreamProcessorTreeItem;
Expand All @@ -50,6 +49,7 @@
connectionId: string;

isExpanded: boolean;
isMutable: boolean;

constructor({
connectionId,
Expand All @@ -58,6 +58,7 @@
connectionController,
cacheIsUpToDate,
childrenCache,
isMutable,
}: {
connectionId: string;
collapsibleState: vscode.TreeItemCollapsibleState;
Expand All @@ -67,21 +68,24 @@
childrenCache: {
[key: string]: DatabaseTreeItem | StreamProcessorTreeItem;
}; // Existing cache.
isMutable: boolean;
}) {
super(
connectionController.getSavedConnectionName(connectionId),
collapsibleState
);

if (
const isConnected =
connectionController.getActiveConnectionId() === connectionId &&
!connectionController.isDisconnecting() &&
!connectionController.isConnecting()
) {
this.contextValue = ConnectionItemContextValues.connected;
}
!connectionController.isConnecting();

this.contextValue = `${isConnected ? 'connected' : 'disconnected'}${
isMutable ? '' : 'Immutable'
Copy link
Contributor Author

@gagik gagik Jan 13, 2025

Choose a reason for hiding this comment

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

naming here I'm not so sure about. Immutable could also be "Preset", I just thought of making this more generic. The idea is that it's not a connection that can be removed/edited/etc.

}ConnectionTreeItem`;

this.connectionId = connectionId;
this.isMutable = isMutable;
this._connectionController = connectionController;
this.isExpanded = isExpanded;
this._childrenCache = childrenCache;
Expand Down Expand Up @@ -204,7 +208,7 @@
return Object.values(this._childrenCache);
}

private async _buildChildrenCacheForDatabases(dataService: DataService) {

Check warning on line 211 in src/explorer/connectionTreeItem.ts

View workflow job for this annotation

GitHub Actions / Test and Build (ubuntu-latest) 

Missing return type on function

Check warning on line 211 in src/explorer/connectionTreeItem.ts

View workflow job for this annotation

GitHub Actions / Test and Build (macos-latest) 

Missing return type on function
const databases = await this.listDatabases();
databases.sort((a: string, b: string) => a.localeCompare(b));

Expand All @@ -226,7 +230,7 @@
return newChildrenCache;
}

private async _buildChildrenCacheForStreams(dataService: DataService) {

Check warning on line 233 in src/explorer/connectionTreeItem.ts

View workflow job for this annotation

GitHub Actions / Test and Build (ubuntu-latest) 

Missing return type on function

Check warning on line 233 in src/explorer/connectionTreeItem.ts

View workflow job for this annotation

GitHub Actions / Test and Build (macos-latest) 

Missing return type on function
const processors = await this.listStreamProcessors();
processors.sort((a, b) => a.name.localeCompare(b.name));

Expand Down
82 changes: 46 additions & 36 deletions src/explorer/explorerTreeController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { DOCUMENT_ITEM } from './documentTreeItem';
import { DOCUMENT_LIST_ITEM, CollectionTypes } from './documentListTreeItem';
import EXTENSION_COMMANDS from '../commands';
import { sortTreeItemsByLabel } from './treeItemUtils';
import type { LoadedConnection } from '../storage/connectionStorage';

const log = createLogger('explorer tree controller');

Expand Down Expand Up @@ -130,6 +131,46 @@ export default class ExplorerTreeController
return element;
}

private _getConnectionExpandedState(connection: LoadedConnection): {
collapsibleState: vscode.TreeItemCollapsibleState;
isExpanded: boolean;
} {
const pastConnectionTreeItems = this._connectionTreeItems;
const isActiveConnection =
connection.id === this._connectionController.getActiveConnectionId();
const isBeingConnectedTo =
this._connectionController.isConnecting() &&
connection.id === this._connectionController.getConnectingConnectionId();

let collapsibleState = isActiveConnection
? vscode.TreeItemCollapsibleState.Expanded
: vscode.TreeItemCollapsibleState.Collapsed;

if (
pastConnectionTreeItems[connection.id] &&
!pastConnectionTreeItems[connection.id].isExpanded
) {
// Connection was manually collapsed while being active.
collapsibleState = vscode.TreeItemCollapsibleState.Collapsed;
}
if (isActiveConnection && this._connectionController.isDisconnecting()) {
// Don't show a collapsable state when the connection is being disconnected from.
collapsibleState = vscode.TreeItemCollapsibleState.None;
}
if (isBeingConnectedTo) {
// Don't show a collapsable state when the connection is being connected to.
collapsibleState = vscode.TreeItemCollapsibleState.None;
}
return {
collapsibleState,
// Set expanded when we're connecting to a connection so that it
// expands when it's connected.
isExpanded:
isBeingConnectedTo ||
collapsibleState === vscode.TreeItemCollapsibleState.Expanded,
};
}

getChildren(element?: any): Thenable<any[]> {
// When no element is present we are at the root.
if (!element) {
Expand All @@ -139,45 +180,14 @@ export default class ExplorerTreeController

// Create new connection tree items, using cached children wherever possible.
connections.forEach((connection) => {
const isActiveConnection =
connection.id === this._connectionController.getActiveConnectionId();
const isBeingConnectedTo =
this._connectionController.isConnecting() &&
connection.id ===
this._connectionController.getConnectingConnectionId();

let connectionExpandedState = isActiveConnection
? vscode.TreeItemCollapsibleState.Expanded
: vscode.TreeItemCollapsibleState.Collapsed;

if (
pastConnectionTreeItems[connection.id] &&
!pastConnectionTreeItems[connection.id].isExpanded
) {
// Connection was manually collapsed while being active.
connectionExpandedState = vscode.TreeItemCollapsibleState.Collapsed;
}
if (
isActiveConnection &&
this._connectionController.isDisconnecting()
) {
// Don't show a collapsable state when the connection is being disconnected from.
connectionExpandedState = vscode.TreeItemCollapsibleState.None;
}
if (isBeingConnectedTo) {
// Don't show a collapsable state when the connection is being connected to.
connectionExpandedState = vscode.TreeItemCollapsibleState.None;
}
const { collapsibleState, isExpanded } =
this._getConnectionExpandedState(connection);

this._connectionTreeItems[connection.id] = new ConnectionTreeItem({
connectionId: connection.id,
collapsibleState: connectionExpandedState,
// Set expanded when we're connecting to a connection so that it
// expands when it's connected.
isExpanded:
isBeingConnectedTo ||
connectionExpandedState ===
vscode.TreeItemCollapsibleState.Expanded,
collapsibleState,
isExpanded,
isMutable: connection.isMutable ?? true,
connectionController: this._connectionController,
cacheIsUpToDate: pastConnectionTreeItems[connection.id]
? pastConnectionTreeItems[connection.id].cacheIsUpToDate
Expand Down
20 changes: 20 additions & 0 deletions src/mdbExtensionController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,15 @@ export default class MDBExtensionController implements vscode.Disposable {
this._editorsController.registerProviders();
}

subscribeToConfigurationChanges(): void {
const subscription = vscode.workspace.onDidChangeConfiguration((event) => {
if (event.affectsConfiguration('mdb.presetSavedConnections')) {
void this._connectionController.loadSavedConnections();
}
});
this._context.subscriptions.push(subscription);
Copy link
Contributor Author

@gagik gagik Jan 16, 2025

Choose a reason for hiding this comment

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

If I understand correctly, if I add this subscription to the context, it'll auto-dispose right so no further manual handling needed? @alenakhineika

Copy link
Contributor

@alenakhineika alenakhineika Jan 20, 2025

Choose a reason for hiding this comment

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

This is how I understand this as well.

}

async activate(): Promise<void> {
this._explorerController.activateConnectionsTreeView();
this._helpExplorer.activateHelpTreeView(this._telemetryService);
Expand All @@ -172,6 +181,7 @@ export default class MDBExtensionController implements vscode.Disposable {

this.registerCommands();
this.showOverviewPageIfRecentlyInstalled();
this.subscribeToConfigurationChanges();

const copilot = vscode.extensions.getExtension(COPILOT_EXTENSION_ID);
void vscode.commands.executeCommand(
Expand Down Expand Up @@ -480,6 +490,16 @@ export default class MDBExtensionController implements vscode.Disposable {
return true;
}
);
this.registerCommand(
EXTENSION_COMMANDS.MDB_OPEN_WORKSPACE_SETTINGS_FILE,
async () => {
await vscode.commands.executeCommand(
'workbench.action.openWorkspaceSettingsFile'
);

return true;
}
);
this.registerCommand(
EXTENSION_COMMANDS.MDB_COPY_CONNECTION_STRING,
async (element: ConnectionTreeItem): Promise<boolean> => {
Expand Down
Loading
Loading