Skip to content

feat(sidebar): filter both databases and collections with dot notation #6912

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

Merged
merged 3 commits into from
Jun 12, 2025

Conversation

ooorayray
Copy link
Contributor

@ooorayray ooorayray commented May 11, 2025

Description

Updates the sidebar filter to allow filtering both databases and collections by dot notation: "."

Screen.Recording.2025-05-11.at.3.13.17.AM.mov

Checklist

  • New tests and/or benchmarks are included
  • Documentation is changed or added
  • If this change updates the UI, screenshots/videos are added and a design review is requested
  • I have signed the MongoDB Contributor License Agreement (https://www.mongodb.com/legal/contributor-agreement)

Motivation and Context

When there are many databases with the same set of collections (think multi-tenancy), filtering by either databases or collections is insufficient. Filtering on both allows one to access a specific database and collection quickly.

  • 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)

@ooorayray ooorayray force-pushed the filter-by-dot-notation branch from 93ed66a to e18c41f Compare May 11, 2025 10:05
@harris
Copy link

harris commented May 12, 2025

This is amazing! Thanks @ooorayray !!!

initialProps: {
connections: mockSidebarConnections,
filter: {
regex: new RegExp('ready_1_1.coll_ready_shared_name', 'i'), // this matches only coll_ready_shared_name collection in ready_1_1 database
Copy link
Contributor

@kraenhansen kraenhansen May 13, 2025

Choose a reason for hiding this comment

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

While this works, the . has special meaning in the regular expression and I believe it should be escaped here to capture the intent of the change, to mean a literal "." (dot).

Suggested change
regex: new RegExp('ready_1_1.coll_ready_shared_name', 'i'), // this matches only coll_ready_shared_name collection in ready_1_1 database
regex: new RegExp('ready_1_1\.coll_ready_shared_name', 'i'), // this matches only coll_ready_shared_name collection in ready_1_1 database

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed!

Copy link
Contributor

@kraenhansen kraenhansen left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution - it looks good to me 👍 I wonder if it would make sense (in this PR or a follow-up) to do the same for filterDatabases by passing in the connectionName too and have that propagate into filterDatabases as well.

@ooorayray
Copy link
Contributor Author

Thanks for your contribution - it looks good to me 👍 I wonder if it would make sense (in this PR or a follow-up) to do the same for filterDatabases by passing in the connectionName too and have that propagate into filterDatabases as well.

Perhaps this would make more sense in the follow-up. I took a stab at this but it seems more involved: for example, would we allow . as a valid filter?

@ooorayray
Copy link
Contributor Author

Hi @kraenhansen, please let me know if there are any other blockers for this PR that I can help resolve. Thanks!

@gribnoysup gribnoysup self-assigned this Jun 12, 2025
@gribnoysup
Copy link
Collaborator

Hey @ooorayray! Thanks for the contribution, sorry for the delay on getting back to you on this. Change looks good to me and I just kicked off our CI to make sure that all the existing tests are passing and don't need to be adjusted.

@gribnoysup gribnoysup added release notes feat no-title-validation Skips validation of PR titles (conventional commit adherence + JIRA ticket inclusion) labels Jun 12, 2025
@gribnoysup
Copy link
Collaborator

Seems like we're missing some permissions for smoketests to run for external contribs, but otherwise CI is green, so I'll merge. @ooorayray thanks again for the contribution, very nice change to the search logic! It should get into the next release

@gribnoysup gribnoysup merged commit 41461e3 into mongodb-js:main Jun 12, 2025
29 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat no-title-validation Skips validation of PR titles (conventional commit adherence + JIRA ticket inclusion) release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants