Skip to content

feat(compass-collection): gate experiment feature behind redux state check instead of hook call - CLOUDP-344022 #7298

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

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

feat(compass-collection): gate experiment feature behind redux state check instead of hook call - CLOUDP-344022 #7298

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
eda194e
WIP
jcobis Sep 9, 2025
c23b7d7
Consolidate helper functions in test file
jcobis Sep 9, 2025
f7a0f35
Remove useAssignment hook
jcobis Sep 9, 2025
6548018
typo
jcobis Sep 9, 2025
b1daccc
Change action to not dispatch error for empty collection
jcobis Sep 9, 2025
ebe4412
Make test method constant
jcobis Sep 9, 2025
47f2247
Restore useAssignment hook
jcobis Sep 9, 2025
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
75 changes: 15 additions & 60 deletions ...ss-collection/src/components/collection-header-actions/collection-header-actions.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,16 @@ import {
import type { PreferencesAccess } from 'compass-preferences-model';
import { createSandboxFromDefaultPreferences } from 'compass-preferences-model';
import { PreferencesProvider } from 'compass-preferences-model/provider';
import { ExperimentTestName } from '@mongodb-js/compass-telemetry/provider';
import { CompassExperimentationProvider } from '@mongodb-js/compass-telemetry';

import type { ConnectionInfo } from '@mongodb-js/compass-connections/provider';

import CollectionHeaderActions from '../collection-header-actions';

describe('CollectionHeaderActions [Component]', function () {
let preferences: PreferencesAccess;
let mockUseAssignment: sinon.SinonStub;

beforeEach(async function () {
preferences = await createSandboxFromDefaultPreferences();
mockUseAssignment = sinon.stub();
mockUseAssignment.returns({
assignment: {
assignmentData: {
variant: 'mockDataGeneratorControl',
},
},
});
});

afterEach(function () {
Expand All @@ -45,27 +35,19 @@ describe('CollectionHeaderActions [Component]', function () {
connectionInfo?: ConnectionInfo
) => {
return renderWithActiveConnection(
<CompassExperimentationProvider
useAssignment={mockUseAssignment}
assignExperiment={sinon.stub()}
getAssignment={sinon.stub().resolves(null)}
>
<WorkspacesServiceProvider
value={workspaceService as WorkspacesService}
>
<PreferencesProvider value={preferences}>
<CollectionHeaderActions
namespace="test.test"
isReadonly={false}
onOpenMockDataModal={sinon.stub()}
hasSchemaAnalysisData={true}
analyzedSchemaDepth={2}
schemaAnalysisStatus="complete"
{...props}
/>
</PreferencesProvider>
</WorkspacesServiceProvider>
</CompassExperimentationProvider>,
<WorkspacesServiceProvider value={workspaceService as WorkspacesService}>
<PreferencesProvider value={preferences}>
<CollectionHeaderActions
namespace="test.test"
isReadonly={false}
onOpenMockDataModal={sinon.stub()}
hasSchemaAnalysisData={true}
analyzedSchemaDepth={2}
schemaAnalysisStatus="complete"
{...props}
/>
</PreferencesProvider>
</WorkspacesServiceProvider>,
connectionInfo
);
};
Expand Down Expand Up @@ -225,34 +207,15 @@ describe('CollectionHeaderActions [Component]', function () {
},
};

it('should call useAssignment with correct parameters', async function () {
await renderCollectionHeaderActions({
namespace: 'test.collection',
isReadonly: false,
});

expect(mockUseAssignment).to.have.been.calledWith(
ExperimentTestName.mockDataGenerator,
true // trackIsInSample - Experiment viewed analytics event
);
});

it('should call onOpenMockDataModal when CTA button is clicked', async function () {
const onOpenMockDataModal = sinon.stub();

mockUseAssignment.returns({
assignment: {
assignmentData: {
variant: 'mockDataGeneratorVariant',
},
},
});

await renderCollectionHeaderActions(
{
namespace: 'test.collection',
isReadonly: false,
onOpenMockDataModal,
schemaAnalysisStatus: 'complete',
},
{},
atlasConnectionInfo
Expand All @@ -267,14 +230,6 @@ describe('CollectionHeaderActions [Component]', function () {
});

it('should disable button for deeply nested collections', async function () {
mockUseAssignment.returns({
assignment: {
assignmentData: {
variant: 'mockDataGeneratorVariant', // Treatment variant
},
},
});

await renderCollectionHeaderActions(
{
namespace: 'test.collection',
Expand Down
24 changes: 6 additions & 18 deletions ...compass-collection/src/components/collection-header-actions/collection-header-actions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,10 @@ import React from 'react';
import { usePreferences } from 'compass-preferences-model/provider';
import toNS from 'mongodb-ns';
import { wrapField } from '@mongodb-js/mongodb-constants';
import {
useTelemetry,
useAssignment,
ExperimentTestName,
ExperimentTestGroup,
} from '@mongodb-js/compass-telemetry/provider';
import { useTelemetry } from '@mongodb-js/compass-telemetry/provider';
import {
SCHEMA_ANALYSIS_STATE_ANALYZING,
SCHEMA_ANALYSIS_STATE_COMPLETE,
type SchemaAnalysisStatus,
} from '../../schema-analysis-types';

Expand Down Expand Up @@ -82,21 +78,13 @@ const CollectionHeaderActions: React.FunctionComponent<
usePreferences(['readOnly', 'enableShell']);
const track = useTelemetry();

// Get experiment assignment for Mock Data Generator
const mockDataGeneratorAssignment = useAssignment(
ExperimentTestName.mockDataGenerator,
true // trackIsInSample - this will fire the "Experiment Viewed" event
);

const { database, collection } = toNS(namespace);

// Check if user is in treatment group for Mock Data Generator experiment
const isInMockDataTreatmentVariant =
mockDataGeneratorAssignment?.assignment?.assignmentData?.variant ===
ExperimentTestGroup.mockDataGeneratorVariant;

// Show Mock Data Generator button if schema analysis has bee initiated (indicating user is in treatment variant)
// Schema analysis only runs for users in the treatment variant
const shouldShowMockDataButton =
isInMockDataTreatmentVariant &&
(schemaAnalysisStatus === SCHEMA_ANALYSIS_STATE_ANALYZING ||
schemaAnalysisStatus === SCHEMA_ANALYSIS_STATE_COMPLETE) &&
Comment on lines 85 to +87
Copy link
Collaborator

@gribnoysup gribnoysup Sep 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This change means that even when the experiment related code is removed, the button now will always jump in the view because the analysis doesn't start immediately: first render happens before that. This also makes the button pop out if the analysis has failed. Not sure if this is an intended change in the behavior here. Feels like checking the assignment status is a clearer way of dealing with this UI: when experiment code is removed, this button will always be visible if other conditions are met, right?

Copy link
Collaborator Author

@jcobis jcobis Sep 9, 2025

Choose a reason for hiding this comment

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

My thinking was that this will actually reduce the latency for which the button pops in, since before this change, it'd have to wait for the useAssignment call to resolve before showing the button.

True that currently this PR has the button pop out on failed analysis, but I think that's okay, as that's an unexpected error state. Alternatively, we could keep the button displayed but disabled.

Yes, when we remove experiment code, we can remove this. I can add a note to do so here if in the future we do remove the experiment gate.

If you feel this PR is better left unmerged, happy to skip it. Maybe we can keep it unmerged for now.

Copy link
Collaborator

@gribnoysup gribnoysup Sep 9, 2025

Choose a reason for hiding this comment

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

Hmmm, If the plan is to remove the button if we failed to analyze instead of surfacing the error somehow, then indeed it's probably better to not show the button at all and limit the state checks even more than you have here to only showing it when analysis is completed successfully, otherwise in the current version the button shows up hen analysis starts, it's disabled because analysis is in progress, then it just disappears with no notice on error, this doesn't make much sense as a behavior 🙂

Copy link
Collaborator Author

@jcobis jcobis Sep 9, 2025

Choose a reason for hiding this comment

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

Ok, sounds good. For now, I'll leave this PR and we can revisit/reconsider it once higher priority work is done.

Copy link
Collaborator

@gribnoysup gribnoysup Sep 10, 2025

Choose a reason for hiding this comment

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

Sure! Do you mind switching it to draft so that it doesn't look like ready for review?

atlasMetadata && // Only show in Atlas
!isReadonly && // Don't show for readonly collections (views)
!sourceName; // sourceName indicates it's a view
Expand Down
Loading
Loading