-
Notifications
You must be signed in to change notification settings - Fork 244
feat(compass-connections-navigation): add links to cluster level atlas pages COMPASS-9967 #7516
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
6a3cb7e to
3618d86
Compare
| const url = new URL(`/v2/${projectId}`, window.location.origin); | ||
| const fragmentPath = [ | ||
| 'host', | ||
| clusterType === 'REPLICASET' ? 'replicaSet' : 'cluster', |
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.
We already have derived metrics type in the atlasMetadata under metricsType field, it's a delibirate decision not to encode this logic on the client and keep it strictly on the backend
You also only need to set the hash part of the url, the first part linking to project would be already correctly set in the url (you can look at getAtlasPerformanceAdvisorLink method for an example of how we do this elsewhere)
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.
Yea found that property later oops, I was like there's no way there's just two values for this 😅
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.
Since I want to open in a new tab and I'm using window.open I think I need the whole url unless there's something I'm missing
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.
It's the same logic as for anchor tags, you can just pass the hash part to the open method, it will concat it with the current path for you and only replace the hash
| gap: spacing[200], | ||
| }); | ||
|
|
||
| function buildMonitoringUrl(atlasMetadata?: AtlasClusterMetadata) { |
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.
Starting to think that maybe it's worth moving all those in a single place. Creating a new package seems like an overkill, maybe close to atlas service?
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.
Pull Request Overview
This pull request adds navigation links to Atlas cluster-level pages from various locations in the Compass UI. The changes enable users to access Atlas monitoring, cluster overview, and query insights directly from the connections navigation sidebar and collection/database headers.
- Adds three new action types for Atlas cluster navigation: cluster overview, monitoring, and query insights
- Implements URL builder functions for constructing Atlas page links
- Integrates monitoring links into collection and database header action bars
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/compass-connections-navigation/src/constants.tsx | Defines new action types for Atlas cluster navigation |
| packages/compass-connections-navigation/src/item-actions.ts | Adds new Atlas navigation actions to connection item menus |
| packages/compass-sidebar/src/components/multiple-connections/connections-navigation.tsx | Implements action handlers and URL builders for Atlas navigation |
| packages/databases-collections-list/src/items-table.tsx | Adds monitoring link button to database/collection table headers |
| packages/compass-collection/src/components/collection-header-actions/collection-header-actions.tsx | Adds monitoring link button to collection header actions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/compass-sidebar/src/components/multiple-connections/connections-navigation.tsx
Outdated
Show resolved
Hide resolved
...es/compass-collection/src/components/collection-header-actions/collection-header-actions.tsx
Outdated
Show resolved
Hide resolved
packages/compass-sidebar/src/components/multiple-connections/connections-navigation.tsx
Outdated
Show resolved
Hide resolved
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.
Pull Request Overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...es/compass-collection/src/components/collection-header-actions/collection-header-actions.tsx
Show resolved
Hide resolved
| isDisabled: false, | ||
| disabledDescription: 'Not supported', |
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.
Nit: I don't think you need to pass this if disabled is always set to false, but not that ot matters too much
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.
isDisabled will default to false right? :)
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.
fixed!
20a0322 to
c7cdcea
Compare
Description
WIP.
Need to figure out how to navigate from an action properly.
Checklist
Motivation and Context
Open Questions
Dependents
Types of changes