fix(compass-web): Rename connections list to clusters list in compass-web COMPASS-8698

[djechlin-mongodb]

Description

• Rename "Connections" and "Search connections" to "Clusters" and "Search clusters"
• Do not impact other uses of connections since in many contexts they still are "connections", for instance you filter by active/closed connection

Checklist

• New tests and/or benchmarks are included
• Documentation is changed or added
• I have signed the MongoDB Contributor License Agreement (https://www.mongodb.com/legal/contributor-agreement)

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)

[gribnoysup]

We're lacking a bit of documentation around this, I'm sorry for that, but so far we were going a bit back and forth on this and decided not to introduce catch-all contexts that would allow to identify that you're in compass-web like that: it makes it too easy to introduce behavior and UI that's different per runtime and this goes against the goal of unification, so we usually either have very focused preferences to toggle some feature or provide interfaces for something like this without tying them to the runtime identification specifically, this forces us to develop this without necessarily tying the behavior to the runtime unless absolutely necessary.

In this case I would suggest to go with the latter option and make these labels configurable with a property on the sidebar component:

type SidebarPluginProps = {
  // ... existing props
  navigationItemsTitle?: string;
  navigationItemsFilterLabel?: string;
}

Then in compass-web you can pass those directly to the place where we render the sidebar:

renderSidebar={() => {
  return (
    <CompassSidebarPlugin
        navigationItemsTitle="Clusters"
        showSidebarHeader={false}
        
// ... rest of the code

This requires a bit more wiring, but is in line with the existing showSidebarHeader interface, in line with just normal React component composition patterns, and allows us to avoid introducing this catch-all "is mms" flag that we're trying to avoid.

What do you think, does this makes sense? Happy to discuss more!

[djechlin-mongodb]

I'd push back here. A general unification goal is minimize web/desktop diffs. One necessary diff is loading atlas clusters v connections. This JIRA bug is not asking for a new diff, it is a bug following from how we implemented the atlas v connections diff. So implementation should not introduce any new "are we in compass" logic, but make the text decision using whatever component decided to load clusters instead of connections.

Context v prop drilling is mostly just a code organization choice, with context being more decoupled/less boilerplate for where the context/property gets set. The important thing is the context/property gets set by an object with direct knowledge of what it should be. I decided AtlasCloudConnectionsProvider is a sensible object to own that knowledge. (The store classes are too; they just got messy and don't really have fixed methods like that.) I would try not to use compass-web, as compass-web doesn't have direct knowledge that we are fetching atlas cloud connections, it actually first delegates that decision to AtlasCloudConnectionsProvider. Prop-drilling from compass-web would be less preferred because it's introducing a second, incorrectly decoupled way of saying if we're in atlas or not. It would be like if compass web said something like

<Atlas>welcome to atlas!</Atlas><Desktop>Here's your local connections!</Desktop> -- you'd rather write compass-web in a way that it's impossible to have code like that.

Also note this complexity is irreducible. Sidebar is now a component flexible enough to load Connections or Clusters. It will therefore need context/prop/some other way to tell and display either Connections or Clusters. (not that you're suggesting it) but I've avoid making it super duper general like taking in 'Connections' or 'Clusters' text since it sees to dilute its responsibility as a few to display a bunch of ConnectionInfo[] (that may be clusters) sensibly; instead it starts delegating how to display them to a bunch of other components, instead of asking which to display and doing that.

LMK what you think or discuss. I'm not committed to one of a few different implementation choices, but I do think I minimized maintenance cost by not introducing any "are we in MMS" decision points, provided we are all aligned on the initial bug.

[gribnoysup]

One necessary diff is loading atlas clusters v connections.

That's only temporarily true while Atlas login is not fully available in Compass, so I'd avoid making design decisions assuming this. This is specifically the reason I'm hesitant to tie this check to anything related to the storage like you're doing in the current implementation.

Context v prop drilling is mostly just a code organization choice

That's totally fair, I don't have a strong preference here whether to use one or another. I do have a strong preference for this context to be more tied to the sidebar code though and not explicitly connect a label override to the environment code is running in / specific store implementation being used.

Also note this complexity is irreducible. Sidebar is now a component flexible enough to load Connections or Clusters. It will therefore need context/prop/some other way to tell and display either Connections or Clusters.

Sidebar is not responsible for loading connections, and most importantly all connections are abstracted into a single ConnectionInfo type by design, you shouldn't be able to do this sort of catch all distinction for all connections sidebar is rendering based on the service used as we are planning for eventually being able to render a mix of those.

If you want to delegate the job of detecting what label to render to the sidebar component, then the most correct way right now would be to check that every ConnectionInfo item that is available has the atlasMetadata property available, but I'm pretty sure we'll have to change this UI at least once more when adding support for Atlas connections in Compass desktop, so I'd really consider opting for something that is just specifically made to allow you to override the labels in the sidebar.

[djechlin-mongodb]

So the situation is

• compass-web knows it should say Clusters
• you can't derive that it should say Clusters from connection loading strategy because Compass will at some point be able to load atlas clusters by this mechanic
• you could drive it from if all(connectioninfos have atlas metadata) say clusters, I actually thought of that but didn't like the idea that a mix of atlas/nonatlas in compass, as when a nonatlas connection appears/disappears for whatever reason, will wind up toggling the "Clusters" heading in Compass desktop. And you're saying it's not that future-proofed anyway.

I think what I'll do is rename the Context object, and move it outside the "AtlasCloudConnectionsProvider", but have it so that compass-web picks the connection load strategy and the header name next to each other, so the reader hopefully understands that you need to pick a header name that makes sense for your connection load strategy and maintains them together.

Commenting now... code forthcoming.

[gribnoysup]

Sounds like a plan 👍 I mentioned this already, I do think that sidebar package itself is probably a good place for this contex

[gribnoysup]

Let's only export provider from the package as it's the only thing that's used outside of sidebar, this context is still not something I would want us to allow checking against otherwise in other parts of the application

[djechlin-mongodb]

Done.

[gribnoysup]

Code looks good, seems like there might be some linting changes, so just double-check it's green before merging

[gribnoysup]

Unused, will fail in CI

[djechlin-mongodb]

yup got it