-
Notifications
You must be signed in to change notification settings - Fork 214
feat: disable db and coll stats via preferences COMPASS-5387 #6852
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
Conversation
efcd98c
to
a2461a5
Compare
packages/compass-settings/src/components/settings/settings-list.tsx
Outdated
Show resolved
Hide resolved
packages/compass-saved-aggregations-queries/src/stores/open-item.ts
Outdated
Show resolved
Hide resolved
* @returns | ||
*/ | ||
async fetch({ dataService, fetchInfo = true, force = false }) { | ||
if (!shouldFetch(this.status, force)) { | ||
return; | ||
} | ||
|
||
const instanceModel = getParentByType(this, 'Instance'); | ||
const { enableDbAndCollStats } = instanceModel.preferences.getPreferences(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of exposing all the preferences directly from the instance model, can we limit the exposed interface to only what we need here? Something like:
const { enableDbAndCollStats } = instanceModel.preferences.getPreferences(); | |
const enableDbAndCollStats = instanceModel.shouldAllowStatsFetching(); |
As a rule of thumb I'd suggest to try to stay minimal when introducing these sort of cross boudary interfaces so that the intent of the exposed interface is clearer from just the name and the actual implementation can be changed easier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's a good idea, I'll do that!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so I ended up using a property instead of a function, as I wanted to make sure we listen to updates on the preferences.
9133126
to
cf4086f
Compare
b1de544
to
856f210
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good, one question on a possible improvement
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! Nice work on this, big preference add.
https://jira.mongodb.org/browse/COMPASS-5387
Description
The stats are enabled by default and can be turned off in the configuration.
This affects views:
Menu
Databases list & grid
Collections list & grid
Collection -> Documents tab
Checklist
Motivation and Context
Open Questions
Dependents
Types of changes