Skip to content
Draft
Show file tree
Hide file tree
Changes from 6 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
95 changes: 78 additions & 17 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 1 addition & 4 deletions packages/collection-model/lib/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -422,10 +422,7 @@ const CollectionCollection = AmpersandCollection.extend(
// filtering, but for now this preserves the current behavior
// and changing it right away will expand the scope of the
// refactor significantly. We can address this in COMPASS-5211
return (
getNamespaceInfo(coll._id).system === false ||
getNamespaceInfo(coll._id).collection === 'system.profile'
);
return getNamespaceInfo(coll._id).system === false;
})
.map(({ _id, ...rest }) => {
return {
Expand Down
2 changes: 1 addition & 1 deletion packages/collection-model/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
"ampersand-collection": "^2.0.2",
"ampersand-model": "^8.0.1",
"mongodb-data-service": "^22.33.0",
"mongodb-ns": "^2.4.2"
"mongodb-ns": "^2.4.3"
},
"devDependencies": {
"@mongodb-js/eslint-config-compass": "^1.4.10",
Expand Down
2 changes: 1 addition & 1 deletion packages/compass-aggregations/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@
"mongodb-data-service": "^22.33.0",
"mongodb-database-model": "^2.34.0",
"mongodb-instance-model": "^12.46.0",
"mongodb-ns": "^2.4.2",
"mongodb-ns": "^2.4.3",
"mongodb-query-parser": "^4.3.0",
"mongodb-schema": "^12.6.2",
"re-resizable": "^6.9.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/compass-app-stores/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@
"mongodb-database-model": "^2.34.0",
"mongodb-instance-model": "^12.46.0",
"compass-preferences-model": "^2.54.0",
"mongodb-ns": "^2.4.2",
"mongodb-ns": "^2.4.3",
"react": "^17.0.2"
},
"is_compass_plugin": true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
createNumbersCollection,
} from '../helpers/insert-data';

const INITIAL_DATABASE_NAMES = ['admin', 'config', 'local', 'test'];
const INITIAL_DATABASE_NAMES = ['admin', 'local', 'test'];

describe('Instance databases tab', function () {
let compass: Compass;
Expand Down
15 changes: 10 additions & 5 deletions packages/database-model/lib/model.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';
const AmpersandModel = require('ampersand-model');
const AmpersandCollection = require('ampersand-collection');
const toNs = require('mongodb-ns');
const {
Collection: MongoDbCollectionCollection,
} = require('mongodb-collection-model');
Expand Down Expand Up @@ -262,11 +263,15 @@ const DatabaseCollection = AmpersandCollection.extend(
});

this.set(
dbs.map(({ _id, name, inferred_from_privileges }) => ({
_id,
name,
inferred_from_privileges,
}))
dbs
.filter((db) => {
return toNs(db.name).special === false;
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

From the top of my head I think that should be an okay change, but I don't know enough about these special collections to say with confidence that we are not breaking some power user use-cases in Compass. Would it make sense to gate this with some new preference setting and allow people to opt-in?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The relevant DBs can be found here. Aside from _mdb_internal* I think it's literally just config

There do seem to be valid reasons to edit config, but only under exceptional circumstances (docs). I think you're right that this is a good use an opt-in preference

.map(({ _id, name, inferred_from_privileges }) => ({
_id,
name,
inferred_from_privileges,
}))
);
},

Expand Down
3 changes: 2 additions & 1 deletion packages/database-model/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@
"ampersand-collection": "^2.0.2",
"ampersand-model": "^8.0.1",
"mongodb-collection-model": "^5.34.0",
"mongodb-data-service": "^22.33.0"
"mongodb-data-service": "^22.33.0",
"mongodb-ns": "^2.4.3"
},
"devDependencies": {
"@mongodb-js/eslint-config-compass": "^1.4.10",
Expand Down
2 changes: 1 addition & 1 deletion packages/databases-collections/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
"mongodb-collection-model": "^5.34.0",
"mongodb-database-model": "^2.34.0",
"mongodb-instance-model": "^12.46.0",
"mongodb-ns": "^2.4.2",
"mongodb-ns": "^2.4.3",
"mongodb-query-parser": "^4.3.0",
"prop-types": "^15.7.2",
"react": "^17.0.2",
Expand Down
18 changes: 13 additions & 5 deletions packages/databases-collections/src/modules/create-namespace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ import toNS from 'mongodb-ns';
* No dots in DB name error message.
*/
export const NO_DOT = 'Database names may not contain a "."';
export const INTERNAL_COLLECTION =
'The collection provided is reserved for use by MongoDB. Please choose a different name.';
export const INTERNAL_DATABASE =
'The database provided is reserved for use by MongoDB. Please choose a different name.';

type CreateNamespaceState = {
isRunning: boolean;
Expand Down Expand Up @@ -379,6 +383,14 @@ export const createNamespace = (
dispatch(handleError(new Error(NO_DOT)));
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I broke this flow ~2 years ago and we never noticed. This should return, otherwise we are continuing to create the namespace even if validation failed.

Suggested change
dispatch(handleError(new Error(NO_DOT)));
return dispatch(handleError(new Error(NO_DOT)));

}

if (dbName && toNS(dbName).special) {
dispatch(handleError(new Error(INTERNAL_DATABASE)));
}

if (toNS(namespace).special) {
dispatch(handleError(new Error(INTERNAL_COLLECTION)));
}
Comment on lines 386 to 392
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same wrt returns

Suggested change
if (dbName && toNS(dbName).special) {
dispatch(handleError(new Error(INTERNAL_DATABASE)));
}
if (toNS(namespace).special) {
dispatch(handleError(new Error(INTERNAL_COLLECTION)));
}
if (dbName && toNS(dbName).special) {
return dispatch(handleError(new Error(INTERNAL_DATABASE)));
}
if (toNS(namespace).special) {
return dispatch(handleError(new Error(INTERNAL_COLLECTION)));
}


try {
dispatch(toggleIsRunning(true));
const ds = connections.getDataServiceForConnection(connectionId);
Expand All @@ -403,15 +415,11 @@ export const createNamespace = (
connectionId,
});

// For special namespaces (admin, local, config), we do not want
// to navigate user to the global-writes tab if it's supported.
const isSpecialNS = toNS(namespace).isSpecial;
const isGlobalWritesSupported =
connectionInfo && connectionSupports(connectionInfo, 'globalWrites');
workspaces.openCollectionWorkspace(connectionId, namespace, {
newTab: true,
initialSubtab:
!isSpecialNS && isGlobalWritesSupported ? 'GlobalWrites' : undefined,
initialSubtab: isGlobalWritesSupported ? 'GlobalWrites' : undefined,
});
dispatch(reset());
} catch (e) {
Expand Down
Loading