Skip to content

Conversation

@djechlin-mongodb
Copy link
Contributor

Description

Checklist

Motivation and Context

  • Bugfix
  • New feature
  • Dependency update
  • Misc

Open Questions

Dependents

Types of changes

  • Backport Needed
  • Patch (non-breaking change which fixes an issue)
  • Minor (non-breaking change which adds functionality)
  • Major (fix or feature that would cause existing functionality to change)

@djechlin-mongodb djechlin-mongodb added fix no release notes Fix or feature not for release notes labels Nov 11, 2024
const isGlobalWritesSupported =
useConnectionSupports(connectionInfoRef.current.id, 'globalWrites') &&
!props.isReadonly &&
!['config', 'local', 'admin'].includes(props.namespace.split('.')[0]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

We use mongodb-ns package for this. You'd want to check the specialish prop on the parsed namespace here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Copy link
Collaborator

@gribnoysup gribnoysup left a comment

Choose a reason for hiding this comment

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

I'd probably suggest adding either a plugin level unit test or an e2e test for this, but otherwise code looks good!

@djechlin-mongodb
Copy link
Contributor Author

Member

Looks like there's no tests set up on this already so there's a bit of work to scaffold them. I discussed with Paula, we think it's just a lot to set up right now so going with merging.

@djechlin-mongodb djechlin-mongodb merged commit e07f229 into main Nov 13, 2024
23 of 29 checks passed
@djechlin-mongodb djechlin-mongodb deleted the COMPASS-8447-views-internal-no-global-writes branch November 13, 2024 14:48
@gribnoysup
Copy link
Collaborator

we think it's just a lot to set up right now so going with merging.

Fair enough, but FWIW the e2e one is then probably the easiest to add, you would only need to open collection by name in any existing e2e test and check that the tab is not there, not extra setup needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix no release notes Fix or feature not for release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants