Skip to content

fix: global writes feature flag COMPAS-8524 #6539

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.

Sign up for GitHub

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 1 commit into from
Nov 29, 2024
+14 −11
Merged

fix: global writes feature flag COMPAS-8524 #6539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion packages/compass-collection/src/components/collection-tab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
useConnectionInfoRef,
useConnectionSupports,
} from '@mongodb-js/compass-connections/provider';
import { usePreference } from 'compass-preferences-model/provider';

type CollectionSubtabTrackingId = Lowercase<CollectionSubtab> extends infer U
? U extends string
Expand Down Expand Up @@ -119,13 +120,17 @@ function WithErrorBoundary({
function useCollectionTabs(props: CollectionMetadata) {
const pluginTabs = useCollectionSubTabs();
const connectionInfoRef = useConnectionInfoRef();
const isGlobalWritesEnabled = usePreference('enableGlobalWrites');
const isGlobalWritesSupported =
useConnectionSupports(connectionInfoRef.current.id, 'globalWrites') &&
!props.isReadonly &&
!toNS(props.namespace).specialish;
return pluginTabs
.filter((x) => {
if (x.name === 'GlobalWrites' && !isGlobalWritesSupported) {
if (
x.name === 'GlobalWrites' &&
(!isGlobalWritesEnabled || !isGlobalWritesSupported)
) {
return false;
}
return true;
Expand Down
8 changes: 8 additions & 0 deletions packages/compass-preferences-model/src/feature-flags.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export type FeatureFlags = {
enableQueryHistoryAutocomplete: boolean;
enableProxySupport: boolean;
enableRollingIndexes: boolean;
enableGlobalWrites: boolean;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are we adding it as a feature flag on compass side? should it not be part of preferences and FF as part of MMS only?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good question. Tbh I am confused about the different types of preferences in compass and wanted to check this with @gribnoysup. I think Global Writes should be in the same group as Rolling Indexes, as both features are web only for now, but we might be adding them to desktop later.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, so I think it's understandable that you can see these in either of those preferences groups, we don't really have a very clean separation there for case like this.

Saying that I think these map more to what we use the feature flag group of preferences for: they are temporary, will only be used for the initial rollout and then removed, we don't really need a dedicated preference for these features as their availability in UI has a stricter check against a cluster metadata that is not available in compass desktop (and at the point where it will be, they should start working without the feature flag required to toggle them)

This is different from something like saved queries feature where bringing it to the web will require more effort in making the system work universally across environments and so it's not something that is just temporarily disabled in the same sense as these that will be used for the rollout.

And to be clear this is a very loose definition that can be understandably argued against, we might consider introducing some new preferences group to remove the ambiguity, maybe something to think through a bit more, but probably doesn't need to block this particular change.

Copy link
Collaborator

Choose a reason for hiding this comment

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

my only concern was if this is a ff on compass side, it will show up in settings-modal. but not a blocker.

};

export const featureFlags: Required<{
Expand Down Expand Up @@ -92,4 +93,11 @@ export const featureFlags: Required<{
short: 'Enable creating indexes with the rolling build in Atlas Cloud',
},
},

enableGlobalWrites: {
stage: 'development',
description: {
short: 'Enable Global Writes tab in Atlas Cloud',
},
},
};
10 changes: 0 additions & 10 deletions packages/compass-preferences-model/src/preferences-schema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ export type InternalUserPreferences = {
telemetryAnonymousId?: string;
telemetryAtlasUserId?: string;
userCreatedAt: number;
enableGlobalWrites: boolean;
};

// UserPreferences contains all preferences stored to disk.
Expand Down Expand Up @@ -877,15 +876,6 @@ export const storedUserPreferencesProps: Required<{
validator: z.boolean().default(true),
type: 'boolean',
},

enableGlobalWrites: {
ui: false,
cli: false,
global: false,
description: null,
validator: z.boolean().default(false),
type: 'boolean',
},
enableGenAIFeaturesAtlasProject: {
ui: false,
cli: true,
Expand Down
Loading