Skip to content

Conversation

gagik
Copy link
Contributor

@gagik gagik commented Mar 3, 2025

It is actually possible to end up "removing" a preset connection when using the command palette...

It does "remove" the connection for the session and doesn't error but this removal doesn't persist on refresh (since the preset connection just gets reloaded from the config).

This changes it so we instead filter that.

@gagik gagik force-pushed the gagik/filter-vscode-presets branch from c1811dd to 5be9bd3 Compare March 3, 2025 14:42
@@ -743,7 +743,11 @@ export default class ConnectionController {
async onRemoveMongoDBConnection(): Promise<boolean> {
log.info('mdb.removeConnection command called');

const connectionIds = Object.keys(this._connections);
const connectionIds = Object.entries(this._connections).filter(
([, connection]) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

[nit] consider mapping this tuple to an object with id and connection fields - that way it'll be easier to know what you're destructuring into without having to remember what value is at what index.

Copy link
Contributor

Choose a reason for hiding this comment

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

Saw that you're out today, so I went ahead and applied this suggestion - happy to revert it if you disagree with the change, but wanted to merge the PR to get into the next release

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no sounds good, sorry wasn't out for the day just for an hour 👍

@nirinchev nirinchev merged commit 9960602 into main Mar 7, 2025
9 checks passed
@nirinchev nirinchev deleted the gagik/filter-vscode-presets branch March 7, 2025 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants