From c4f018faba6b57fb552ab663c1b369f06ac85743 Mon Sep 17 00:00:00 2001 From: Jacob Samuel Lu Date: Tue, 4 Nov 2025 14:00:27 -0500 Subject: [PATCH 01/12] WIP --- .../src/stores/collection-tab.ts | 76 ++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/packages/compass-collection/src/stores/collection-tab.ts b/packages/compass-collection/src/stores/collection-tab.ts index 10f60a834e2..b89068e9d76 100644 --- a/packages/compass-collection/src/stores/collection-tab.ts +++ b/packages/compass-collection/src/stores/collection-tab.ts @@ -27,7 +27,27 @@ import { ExperimentTestName, ExperimentTestGroup, } from '@mongodb-js/compass-telemetry/provider'; -import { SCHEMA_ANALYSIS_STATE_INITIAL } from '../schema-analysis-types'; +import { + SCHEMA_ANALYSIS_STATE_INITIAL, + SCHEMA_ANALYSIS_STATE_ERROR, + SCHEMA_ANALYSIS_STATE_COMPLETE, +} from '../schema-analysis-types'; +import type { CollectionState } from '../modules/collection-tab'; + +/** + * Determines if schema analysis should be re-triggered after document insertion. + * Re-triggers when: + * 1. Previous analysis failed (error state) + * 2. Analysis completed but no schema data (empty collection) + */ +function shouldRetriggerSchemaAnalysis(state: CollectionState): boolean { + return ( + state.schemaAnalysis.status === SCHEMA_ANALYSIS_STATE_ERROR || + (state.schemaAnalysis.status === SCHEMA_ANALYSIS_STATE_COMPLETE && + (!state.schemaAnalysis.processedSchema || + Object.keys(state.schemaAnalysis.processedSchema).length === 0)) + ); +} export type CollectionTabOptions = { /** @@ -141,6 +161,60 @@ export function activatePlugin( store.dispatch(selectTab('Schema')); }); + // Listen for document insertions to re-trigger schema analysis for previously empty collections + on( + localAppRegistry, + 'document-inserted', + (payload: { + ns: string; + view?: string; + mode: string; + multiple: boolean; + docs: unknown[]; + }) => { + // Ensure event is for the current namespace + if (payload.ns === namespace) { + const currentState = store.getState(); + if (shouldRetriggerSchemaAnalysis(currentState)) { + // Check if user is in Mock Data Generator experiment variant before re-triggering + const shouldRunSchemaAnalysis = async () => { + try { + const assignment = await experimentationServices.getAssignment( + ExperimentTestName.mockDataGenerator, + false // Don't track "Experiment Viewed" event here + ); + return ( + assignment?.assignmentData?.variant === + ExperimentTestGroup.mockDataGeneratorVariant + ); + } catch (error) { + // On error, default to not running schema analysis + logger.debug( + 'Failed to get Mock Data Generator experiment assignment for document insertion re-trigger', + { + experiment: ExperimentTestName.mockDataGenerator, + namespace: namespace, + error: error instanceof Error ? error.message : String(error), + } + ); + return false; + } + }; + + void shouldRunSchemaAnalysis().then((shouldRun) => { + if (shouldRun) { + logger.debug( + 'Re-triggering schema analysis after document insertion', + { namespace } + ); + void store.dispatch(analyzeCollectionSchema()); + } + }); + } + } + } + ); + void collectionModel.fetchMetadata({ dataService }).then((metadata) => { store.dispatch(collectionMetadataFetched(metadata)); From 1be1a7728d63a6c74020f6c6dbeaeefb80e2e34e Mon Sep 17 00:00:00 2001 From: Jacob Samuel Lu Date: Tue, 4 Nov 2025 14:09:12 -0500 Subject: [PATCH 02/12] WIP --- .../src/stores/collection-tab.ts | 89 ++++++++----------- 1 file changed, 39 insertions(+), 50 deletions(-) diff --git a/packages/compass-collection/src/stores/collection-tab.ts b/packages/compass-collection/src/stores/collection-tab.ts index b89068e9d76..8e3be2fd3c4 100644 --- a/packages/compass-collection/src/stores/collection-tab.ts +++ b/packages/compass-collection/src/stores/collection-tab.ts @@ -49,6 +49,35 @@ function shouldRetriggerSchemaAnalysis(state: CollectionState): boolean { ); } +/** + * Checks if user is in Mock Data Generator experiment variant. + * Returns false on error to default to not running schema analysis. + */ +async function shouldRunSchemaAnalysis( + experimentationServices: ExperimentationServices, + logger: Logger, + namespace: string +): Promise { + try { + const assignment = await experimentationServices.getAssignment( + ExperimentTestName.mockDataGenerator, + false // Don't track "Experiment Viewed" event here + ); + return ( + assignment?.assignmentData?.variant === + ExperimentTestGroup.mockDataGeneratorVariant + ); + } catch (error) { + // On error, default to not running schema analysis + logger.debug('Failed to get Mock Data Generator experiment assignment', { + experiment: ExperimentTestName.mockDataGenerator, + namespace: namespace, + error: error instanceof Error ? error.message : String(error), + }); + return false; + } +} + export type CollectionTabOptions = { /** * Workspace Tab ID @@ -177,31 +206,11 @@ export function activatePlugin( const currentState = store.getState(); if (shouldRetriggerSchemaAnalysis(currentState)) { // Check if user is in Mock Data Generator experiment variant before re-triggering - const shouldRunSchemaAnalysis = async () => { - try { - const assignment = await experimentationServices.getAssignment( - ExperimentTestName.mockDataGenerator, - false // Don't track "Experiment Viewed" event here - ); - return ( - assignment?.assignmentData?.variant === - ExperimentTestGroup.mockDataGeneratorVariant - ); - } catch (error) { - // On error, default to not running schema analysis - logger.debug( - 'Failed to get Mock Data Generator experiment assignment for document insertion re-trigger', - { - experiment: ExperimentTestName.mockDataGenerator, - namespace: namespace, - error: error instanceof Error ? error.message : String(error), - } - ); - return false; - } - }; - - void shouldRunSchemaAnalysis().then((shouldRun) => { + void shouldRunSchemaAnalysis( + experimentationServices, + logger, + namespace + ).then((shouldRun) => { if (shouldRun) { logger.debug( 'Re-triggering schema analysis after document insertion', @@ -243,31 +252,11 @@ export function activatePlugin( if (!metadata.isReadonly && !metadata.isTimeSeries) { // Check experiment variant before running schema analysis // Only run schema analysis if user is in treatment variant - const shouldRunSchemaAnalysis = async () => { - try { - const assignment = await experimentationServices.getAssignment( - ExperimentTestName.mockDataGenerator, - false // Don't track "Experiment Viewed" event here - ); - return ( - assignment?.assignmentData?.variant === - ExperimentTestGroup.mockDataGeneratorVariant - ); - } catch (error) { - // On error, default to not running schema analysis - logger.debug( - 'Failed to get Mock Data Generator experiment assignment', - { - experiment: ExperimentTestName.mockDataGenerator, - namespace: namespace, - error: error instanceof Error ? error.message : String(error), - } - ); - return false; - } - }; - - void shouldRunSchemaAnalysis().then((shouldRun) => { + void shouldRunSchemaAnalysis( + experimentationServices, + logger, + namespace + ).then((shouldRun) => { if (shouldRun) { void store.dispatch(analyzeCollectionSchema()); } From c973dc466359c69d069a4c2a5f80446df30b346f Mon Sep 17 00:00:00 2001 From: Jacob Samuel Lu Date: Tue, 4 Nov 2025 14:50:56 -0500 Subject: [PATCH 03/12] WIP --- .../src/stores/collection-tab.ts | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/packages/compass-collection/src/stores/collection-tab.ts b/packages/compass-collection/src/stores/collection-tab.ts index 8e3be2fd3c4..8f38cb359fd 100644 --- a/packages/compass-collection/src/stores/collection-tab.ts +++ b/packages/compass-collection/src/stores/collection-tab.ts @@ -108,7 +108,7 @@ export type CollectionTabServices = { export function activatePlugin( { namespace, editViewName, tabId }: CollectionTabOptions, - services: CollectionTabServices, + services: CollectionTabServices & { globalAppRegistry: AppRegistry }, { on, cleanup, addCleanup }: ActivateHelpers ): { store: ReturnType; @@ -118,6 +118,7 @@ export function activatePlugin( dataService, collection: collectionModel, localAppRegistry, + globalAppRegistry, atlasAiService, workspaces, experimentationServices, @@ -192,17 +193,23 @@ export function activatePlugin( // Listen for document insertions to re-trigger schema analysis for previously empty collections on( - localAppRegistry, + globalAppRegistry, 'document-inserted', - (payload: { - ns: string; - view?: string; - mode: string; - multiple: boolean; - docs: unknown[]; - }) => { - // Ensure event is for the current namespace - if (payload.ns === namespace) { + ( + payload: { + ns: string; + view?: string; + mode: string; + multiple: boolean; + docs: unknown[]; + }, + { connectionId }: { connectionId?: string } = {} + ) => { + // Ensure event is for the current connection and namespace + if ( + connectionId === connectionInfoRef.current.id && + payload.ns === namespace + ) { const currentState = store.getState(); if (shouldRetriggerSchemaAnalysis(currentState)) { // Check if user is in Mock Data Generator experiment variant before re-triggering From 62240a09a7df34674f2200c689d68558fd40e361 Mon Sep 17 00:00:00 2001 From: Jacob Samuel Lu Date: Tue, 4 Nov 2025 15:04:47 -0500 Subject: [PATCH 04/12] Tests --- .../src/stores/collection-tab.spec.ts | 244 +++++++++++++++++- 1 file changed, 231 insertions(+), 13 deletions(-) diff --git a/packages/compass-collection/src/stores/collection-tab.spec.ts b/packages/compass-collection/src/stores/collection-tab.spec.ts index b773949675a..ee3ec5dfea2 100644 --- a/packages/compass-collection/src/stores/collection-tab.spec.ts +++ b/packages/compass-collection/src/stores/collection-tab.spec.ts @@ -86,12 +86,36 @@ describe('Collection Tab Content store', function () { const sandbox = Sinon.createSandbox(); const localAppRegistry = sandbox.spy(new AppRegistry()); + const globalAppRegistry = sandbox.spy(new AppRegistry()); const analyzeCollectionSchemaStub = sandbox .stub(collectionTabModule, 'analyzeCollectionSchema') .returns(async () => {}); - const dataService = {} as any; - const atlasAiService = {} as any; + // Mock the cleanup helper to track event listeners + const eventListeners = new Map void)[]>(); + const mockCleanupHelper = { + on: sandbox.spy( + ( + _registry: unknown, + eventName: string, + listener: (...args: unknown[]) => void + ) => { + if (!eventListeners.has(eventName)) { + eventListeners.set(eventName, []); + } + eventListeners.get(eventName)!.push(listener); + } + ), + cleanup: sandbox.spy(), + addCleanup: sandbox.spy(), + }; + + beforeEach(function () { + eventListeners.clear(); + }); + + const dataService = {} as unknown; + const atlasAiService = {} as unknown; let store: ReturnType['store']; let deactivate: ReturnType['deactivate']; @@ -125,17 +149,18 @@ describe('Collection Tab Content store', function () { ...options, }, { - dataService, - atlasAiService, + dataService: dataService as never, + atlasAiService: atlasAiService as never, localAppRegistry, - collection: mockCollection as any, - workspaces: workspaces as any, - experimentationServices: experimentationServices as any, - connectionInfoRef: connectionInfoRef as any, + globalAppRegistry, + collection: mockCollection as never, + workspaces: workspaces as never, + experimentationServices: experimentationServices as never, + connectionInfoRef: connectionInfoRef as never, logger, preferences, }, - { on() {}, cleanup() {}, addCleanup() {} } as any + mockCleanupHelper as never )); await waitFor(() => { expect(store.getState()) @@ -156,7 +181,7 @@ describe('Collection Tab Content store', function () { const store = await configureStore(undefined, { openCollectionWorkspaceSubtab, }); - store.dispatch(selectTab('Documents') as any); + store.dispatch(selectTab('Documents') as never); expect(openCollectionWorkspaceSubtab).to.have.been.calledWith( 'workspace-tab-id', 'Documents' @@ -470,12 +495,205 @@ describe('Collection Tab Content store', function () { }); // Dispatch cancel action - store.dispatch(collectionTabModule.cancelSchemaAnalysis() as any); + store.dispatch(collectionTabModule.cancelSchemaAnalysis() as never); // Verify the state is reset to initial - expect((store.getState() as any).schemaAnalysis.status).to.equal( - 'initial' + expect( + (store.getState() as { schemaAnalysis: { status: string } }) + .schemaAnalysis.status + ).to.equal('initial'); + }); + }); + + describe('document-inserted event listener', function () { + it('should re-trigger schema analysis when document is inserted into current collection', async function () { + const getAssignment = sandbox.spy(() => + Promise.resolve( + createMockAssignment(ExperimentTestGroup.mockDataGeneratorVariant) + ) + ); + const assignExperiment = sandbox.spy(() => Promise.resolve(null)); + + const store = await configureStore( + undefined, + undefined, + { getAssignment, assignExperiment }, + mockAtlasConnectionInfo, + undefined, + undefined, + { ...defaultMetadata, isReadonly: false, isTimeSeries: false } + ); + + // Wait for initial schema analysis to complete + await waitFor(() => { + expect(analyzeCollectionSchemaStub).to.have.been.calledOnce; + }); + + // Reset the stub to track new calls + analyzeCollectionSchemaStub.resetHistory(); + + // Simulate the empty collection + store.dispatch({ + type: 'compass-collection/SchemaAnalysisFailed', + error: new Error('No documents found'), + } as never); + + // Trigger the document-inserted event listener + const documentInsertedListeners = + eventListeners.get('document-inserted') || []; + expect(documentInsertedListeners).to.have.length(1); + + // Call the event listener with the event payload + documentInsertedListeners[0]( + { + ns: defaultMetadata.namespace, + view: 'default', + mode: 'default', + multiple: false, + docs: [{ _id: 'test-doc-id', name: 'test' }], + }, + { connectionId: mockAtlasConnectionInfo.current.id } ); + + // Wait for schema analysis to be re-triggered + await waitFor(() => { + expect(analyzeCollectionSchemaStub).to.have.been.calledOnce; + }); + }); + + it('should not re-trigger schema analysis for different collection', async function () { + const getAssignment = sandbox.spy(() => + Promise.resolve( + createMockAssignment(ExperimentTestGroup.mockDataGeneratorVariant) + ) + ); + const assignExperiment = sandbox.spy(() => Promise.resolve(null)); + + await configureStore( + undefined, + undefined, + { getAssignment, assignExperiment }, + mockAtlasConnectionInfo + ); + + // Wait for initial schema analysis to complete + await waitFor(() => { + expect(analyzeCollectionSchemaStub).to.have.been.calledOnce; + }); + + // Reset the stub to track new calls + analyzeCollectionSchemaStub.resetHistory(); + + // Trigger the document-inserted event listener with different collection + const documentInsertedListeners = + eventListeners.get('document-inserted') || []; + expect(documentInsertedListeners).to.have.length(1); + + // Call the event listener with different collection namespace + documentInsertedListeners[0]( + { + ns: 'different.collection', + view: 'default', + mode: 'default', + multiple: false, + docs: [{ _id: 'test-doc-id', name: 'test' }], + }, + { connectionId: mockAtlasConnectionInfo.current.id } + ); + + // Wait a bit to ensure schema analysis is not called + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(analyzeCollectionSchemaStub).to.not.have.been.called; + }); + + it('should not re-trigger schema analysis for different connection', async function () { + const getAssignment = sandbox.spy(() => + Promise.resolve( + createMockAssignment(ExperimentTestGroup.mockDataGeneratorVariant) + ) + ); + const assignExperiment = sandbox.spy(() => Promise.resolve(null)); + + await configureStore( + undefined, + undefined, + { getAssignment, assignExperiment }, + mockAtlasConnectionInfo + ); + + // Wait for initial schema analysis to complete + await waitFor(() => { + expect(analyzeCollectionSchemaStub).to.have.been.calledOnce; + }); + + // Reset the stub to track new calls + analyzeCollectionSchemaStub.resetHistory(); + + // Trigger the document-inserted event listener with different connection + const documentInsertedListeners = + eventListeners.get('document-inserted') || []; + expect(documentInsertedListeners).to.have.length(1); + + // Call the event listener with different connection ID + documentInsertedListeners[0]( + { + ns: defaultMetadata.namespace, + view: 'default', + mode: 'default', + multiple: false, + docs: [{ _id: 'test-doc-id', name: 'test' }], + }, + { connectionId: 'different-connection-id' } + ); + + // Wait a bit to ensure schema analysis is not called + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(analyzeCollectionSchemaStub).to.not.have.been.called; + }); + + it('should not re-trigger schema analysis when user is not in experiment variant', async function () { + const getAssignment = sandbox.spy(() => + Promise.resolve( + createMockAssignment(ExperimentTestGroup.mockDataGeneratorControl) + ) + ); + const assignExperiment = sandbox.spy(() => Promise.resolve(null)); + + await configureStore( + undefined, + undefined, + { getAssignment, assignExperiment }, + mockAtlasConnectionInfo + ); + + // Wait for initial assignment check + await waitFor(() => { + expect(getAssignment).to.have.been.calledOnce; + }); + + // Schema analysis should not have been called initially + expect(analyzeCollectionSchemaStub).to.not.have.been.called; + + // Trigger the document-inserted event listener + const documentInsertedListeners = + eventListeners.get('document-inserted') || []; + expect(documentInsertedListeners).to.have.length(1); + + // Call the event listener + documentInsertedListeners[0]( + { + ns: defaultMetadata.namespace, + view: 'default', + mode: 'default', + multiple: false, + docs: [{ _id: 'test-doc-id', name: 'test' }], + }, + { connectionId: mockAtlasConnectionInfo.current.id } + ); + + // Wait a bit to ensure schema analysis is not called + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(analyzeCollectionSchemaStub).to.not.have.been.called; }); }); }); From 65016f32aa2805fb245e7c7b475a7511d0c88759 Mon Sep 17 00:00:00 2001 From: Jacob Samuel Lu Date: Tue, 4 Nov 2025 16:16:51 -0500 Subject: [PATCH 05/12] Github comments --- .../src/stores/collection-tab.spec.ts | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/packages/compass-collection/src/stores/collection-tab.spec.ts b/packages/compass-collection/src/stores/collection-tab.spec.ts index ee3ec5dfea2..d98b03f3d68 100644 --- a/packages/compass-collection/src/stores/collection-tab.spec.ts +++ b/packages/compass-collection/src/stores/collection-tab.spec.ts @@ -18,6 +18,9 @@ import { import { type CollectionMetadata } from 'mongodb-collection-model'; import type { types } from '@mongodb-js/mdb-experiment-js'; +// Wait time in ms for async operations to complete +const WAIT_TIME = 50; + // Helper function to create proper mock assignment objects for testing const createMockAssignment = ( variant: ExperimentTestGroup @@ -114,8 +117,8 @@ describe('Collection Tab Content store', function () { eventListeners.clear(); }); - const dataService = {} as unknown; - const atlasAiService = {} as unknown; + const dataService = {} as never; + const atlasAiService = {} as never; let store: ReturnType['store']; let deactivate: ReturnType['deactivate']; @@ -149,8 +152,8 @@ describe('Collection Tab Content store', function () { ...options, }, { - dataService: dataService as never, - atlasAiService: atlasAiService as never, + dataService, + atlasAiService, localAppRegistry, globalAppRegistry, collection: mockCollection as never, @@ -231,7 +234,7 @@ describe('Collection Tab Content store', function () { ); // Wait a bit to ensure assignment would have happened if it was going to - await new Promise((resolve) => setTimeout(resolve, 50)); + await new Promise((resolve) => setTimeout(resolve, WAIT_TIME)); expect(assignExperiment).to.not.have.been.called; }); @@ -254,7 +257,7 @@ describe('Collection Tab Content store', function () { ); // Wait a bit to ensure assignment would have happened if it was going to - await new Promise((resolve) => setTimeout(resolve, 50)); + await new Promise((resolve) => setTimeout(resolve, WAIT_TIME)); expect(assignExperiment).to.not.have.been.called; // Store should still be functional @@ -303,7 +306,7 @@ describe('Collection Tab Content store', function () { ); // Wait a bit to ensure assignment would have happened if it was going to - await new Promise((resolve) => setTimeout(resolve, 50)); + await new Promise((resolve) => setTimeout(resolve, WAIT_TIME)); expect(assignExperiment).to.not.have.been.called; }); @@ -321,7 +324,7 @@ describe('Collection Tab Content store', function () { ); // Wait a bit to ensure assignment would have happened if it was going to - await new Promise((resolve) => setTimeout(resolve, 50)); + await new Promise((resolve) => setTimeout(resolve, WAIT_TIME)); expect(assignExperiment).to.not.have.been.called; }); }); @@ -402,7 +405,7 @@ describe('Collection Tab Content store', function () { }); // Wait a bit to ensure schema analysis would not have been called - await new Promise((resolve) => setTimeout(resolve, 50)); + await new Promise((resolve) => setTimeout(resolve, WAIT_TIME)); expect(analyzeCollectionSchemaStub).to.not.have.been.called; }); @@ -453,7 +456,7 @@ describe('Collection Tab Content store', function () { }); // Wait a bit to ensure schema analysis would not have been called - await new Promise((resolve) => setTimeout(resolve, 50)); + await new Promise((resolve) => setTimeout(resolve, WAIT_TIME)); expect(analyzeCollectionSchemaStub).to.not.have.been.called; }); @@ -475,7 +478,7 @@ describe('Collection Tab Content store', function () { }); // Wait a bit to ensure schema analysis would not have been called - await new Promise((resolve) => setTimeout(resolve, 50)); + await new Promise((resolve) => setTimeout(resolve, WAIT_TIME)); expect(analyzeCollectionSchemaStub).to.not.have.been.called; }); }); @@ -602,7 +605,7 @@ describe('Collection Tab Content store', function () { ); // Wait a bit to ensure schema analysis is not called - await new Promise((resolve) => setTimeout(resolve, 50)); + await new Promise((resolve) => setTimeout(resolve, WAIT_TIME)); expect(analyzeCollectionSchemaStub).to.not.have.been.called; }); @@ -647,7 +650,7 @@ describe('Collection Tab Content store', function () { ); // Wait a bit to ensure schema analysis is not called - await new Promise((resolve) => setTimeout(resolve, 50)); + await new Promise((resolve) => setTimeout(resolve, WAIT_TIME)); expect(analyzeCollectionSchemaStub).to.not.have.been.called; }); @@ -692,7 +695,7 @@ describe('Collection Tab Content store', function () { ); // Wait a bit to ensure schema analysis is not called - await new Promise((resolve) => setTimeout(resolve, 50)); + await new Promise((resolve) => setTimeout(resolve, WAIT_TIME)); expect(analyzeCollectionSchemaStub).to.not.have.been.called; }); }); From 3014ccfc63476db18f33f33923c9aee4d5100f1b Mon Sep 17 00:00:00 2001 From: Jacob Samuel Lu Date: Wed, 5 Nov 2025 12:20:26 -0500 Subject: [PATCH 06/12] Test --- .../src/stores/collection-tab.spec.ts | 90 +++++++++---------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/packages/compass-collection/src/stores/collection-tab.spec.ts b/packages/compass-collection/src/stores/collection-tab.spec.ts index d98b03f3d68..1d11548755f 100644 --- a/packages/compass-collection/src/stores/collection-tab.spec.ts +++ b/packages/compass-collection/src/stores/collection-tab.spec.ts @@ -94,29 +94,34 @@ describe('Collection Tab Content store', function () { .stub(collectionTabModule, 'analyzeCollectionSchema') .returns(async () => {}); - // Mock the cleanup helper to track event listeners - const eventListeners = new Map void)[]>(); - const mockCleanupHelper = { + // Track event listeners for proper cleanup + const eventListeners: Array<{ + registry: AppRegistry; + eventName: string; + listener: (...args: unknown[]) => void; + }> = []; + + const mockActivateHelpers = { on: sandbox.spy( ( - _registry: unknown, + registry: AppRegistry, eventName: string, listener: (...args: unknown[]) => void ) => { - if (!eventListeners.has(eventName)) { - eventListeners.set(eventName, []); - } - eventListeners.get(eventName)!.push(listener); + registry.on(eventName, listener); + eventListeners.push({ registry, eventName, listener }); } ), - cleanup: sandbox.spy(), + cleanup: sandbox.spy(() => { + // Remove all tracked event listeners + eventListeners.forEach(({ registry, eventName, listener }) => { + registry.removeListener(eventName, listener); + }); + eventListeners.length = 0; + }), addCleanup: sandbox.spy(), }; - beforeEach(function () { - eventListeners.clear(); - }); - const dataService = {} as never; const atlasAiService = {} as never; let store: ReturnType['store']; @@ -128,7 +133,7 @@ describe('Collection Tab Content store', function () { experimentationServices: Partial = {}, connectionInfoRef: Partial< ReturnType - > = {}, + > = mockAtlasConnectionInfo, logger = createNoopLogger('COMPASS-COLLECTION-TEST'), preferences = new ReadOnlyPreferenceAccess({ enableGenAIFeatures: true, @@ -163,7 +168,7 @@ describe('Collection Tab Content store', function () { logger, preferences, }, - mockCleanupHelper as never + mockActivateHelpers as never )); await waitFor(() => { expect(store.getState()) @@ -174,6 +179,7 @@ describe('Collection Tab Content store', function () { }; afterEach(function () { + mockActivateHelpers.cleanup(); sandbox.resetHistory(); deactivate(); }); @@ -181,9 +187,13 @@ describe('Collection Tab Content store', function () { describe('selectTab', function () { it('should set active tab', async function () { const openCollectionWorkspaceSubtab = sandbox.spy(); - const store = await configureStore(undefined, { - openCollectionWorkspaceSubtab, - }); + const assignExperiment = sandbox.spy(() => Promise.resolve(null)); + + const store = await configureStore( + undefined, + { openCollectionWorkspaceSubtab }, + { assignExperiment } + ); store.dispatch(selectTab('Documents') as never); expect(openCollectionWorkspaceSubtab).to.have.been.calledWith( 'workspace-tab-id', @@ -541,13 +551,9 @@ describe('Collection Tab Content store', function () { error: new Error('No documents found'), } as never); - // Trigger the document-inserted event listener - const documentInsertedListeners = - eventListeners.get('document-inserted') || []; - expect(documentInsertedListeners).to.have.length(1); - - // Call the event listener with the event payload - documentInsertedListeners[0]( + // Trigger the document-inserted event + globalAppRegistry.emit( + 'document-inserted', { ns: defaultMetadata.namespace, view: 'default', @@ -587,13 +593,9 @@ describe('Collection Tab Content store', function () { // Reset the stub to track new calls analyzeCollectionSchemaStub.resetHistory(); - // Trigger the document-inserted event listener with different collection - const documentInsertedListeners = - eventListeners.get('document-inserted') || []; - expect(documentInsertedListeners).to.have.length(1); - - // Call the event listener with different collection namespace - documentInsertedListeners[0]( + // Trigger the document-inserted event with different collection + globalAppRegistry.emit( + 'document-inserted', { ns: 'different.collection', view: 'default', @@ -632,13 +634,9 @@ describe('Collection Tab Content store', function () { // Reset the stub to track new calls analyzeCollectionSchemaStub.resetHistory(); - // Trigger the document-inserted event listener with different connection - const documentInsertedListeners = - eventListeners.get('document-inserted') || []; - expect(documentInsertedListeners).to.have.length(1); - - // Call the event listener with different connection ID - documentInsertedListeners[0]( + // Trigger the document-inserted event with different connection + globalAppRegistry.emit( + 'document-inserted', { ns: defaultMetadata.namespace, view: 'default', @@ -677,13 +675,15 @@ describe('Collection Tab Content store', function () { // Schema analysis should not have been called initially expect(analyzeCollectionSchemaStub).to.not.have.been.called; - // Trigger the document-inserted event listener - const documentInsertedListeners = - eventListeners.get('document-inserted') || []; - expect(documentInsertedListeners).to.have.length(1); + // Verify the schema analysis state is INITIAL (as expected for control variant) + const initialState = store.getState() as { + schemaAnalysis: { status: string }; + }; + expect(initialState.schemaAnalysis.status).to.equal('initial'); - // Call the event listener - documentInsertedListeners[0]( + // Trigger the document-inserted event + globalAppRegistry.emit( + 'document-inserted', { ns: defaultMetadata.namespace, view: 'default', From 0d52f752e9d25d8d735141c41e14254d937da539 Mon Sep 17 00:00:00 2001 From: Jacob Samuel Lu Date: Fri, 7 Nov 2025 15:19:09 -0500 Subject: [PATCH 07/12] Test --- .../src/stores/collection-tab.spec.ts | 57 +++++++------------ 1 file changed, 19 insertions(+), 38 deletions(-) diff --git a/packages/compass-collection/src/stores/collection-tab.spec.ts b/packages/compass-collection/src/stores/collection-tab.spec.ts index 1d11548755f..00a47e6e004 100644 --- a/packages/compass-collection/src/stores/collection-tab.spec.ts +++ b/packages/compass-collection/src/stores/collection-tab.spec.ts @@ -4,7 +4,10 @@ import { selectTab } from '../modules/collection-tab'; import * as collectionTabModule from '../modules/collection-tab'; import { waitFor } from '@mongodb-js/testing-library-compass'; import Sinon from 'sinon'; -import AppRegistry from '@mongodb-js/compass-app-registry'; +import type { ActivateHelpers } from '@mongodb-js/compass-app-registry'; +import AppRegistry, { + createActivateHelpers, +} from '@mongodb-js/compass-app-registry'; import { expect } from 'chai'; import type { workspacesServiceLocator } from '@mongodb-js/compass-workspaces/provider'; import type { ExperimentationServices } from '@mongodb-js/compass-telemetry/provider'; @@ -94,36 +97,10 @@ describe('Collection Tab Content store', function () { .stub(collectionTabModule, 'analyzeCollectionSchema') .returns(async () => {}); - // Track event listeners for proper cleanup - const eventListeners: Array<{ - registry: AppRegistry; - eventName: string; - listener: (...args: unknown[]) => void; - }> = []; - - const mockActivateHelpers = { - on: sandbox.spy( - ( - registry: AppRegistry, - eventName: string, - listener: (...args: unknown[]) => void - ) => { - registry.on(eventName, listener); - eventListeners.push({ registry, eventName, listener }); - } - ), - cleanup: sandbox.spy(() => { - // Remove all tracked event listeners - eventListeners.forEach(({ registry, eventName, listener }) => { - registry.removeListener(eventName, listener); - }); - eventListeners.length = 0; - }), - addCleanup: sandbox.spy(), - }; + let mockActivateHelpers: ActivateHelpers; - const dataService = {} as never; - const atlasAiService = {} as never; + const dataService = {} as any; + const atlasAiService = {} as any; let store: ReturnType['store']; let deactivate: ReturnType['deactivate']; @@ -161,14 +138,14 @@ describe('Collection Tab Content store', function () { atlasAiService, localAppRegistry, globalAppRegistry, - collection: mockCollection as never, - workspaces: workspaces as never, - experimentationServices: experimentationServices as never, - connectionInfoRef: connectionInfoRef as never, + collection: mockCollection as any, + workspaces: workspaces as any, + experimentationServices: experimentationServices as any, + connectionInfoRef: connectionInfoRef as any, logger, preferences, }, - mockActivateHelpers as never + mockActivateHelpers )); await waitFor(() => { expect(store.getState()) @@ -178,6 +155,10 @@ describe('Collection Tab Content store', function () { return store; }; + beforeEach(function () { + mockActivateHelpers = createActivateHelpers(); + }); + afterEach(function () { mockActivateHelpers.cleanup(); sandbox.resetHistory(); @@ -194,7 +175,7 @@ describe('Collection Tab Content store', function () { { openCollectionWorkspaceSubtab }, { assignExperiment } ); - store.dispatch(selectTab('Documents') as never); + store.dispatch(selectTab('Documents') as any); expect(openCollectionWorkspaceSubtab).to.have.been.calledWith( 'workspace-tab-id', 'Documents' @@ -508,7 +489,7 @@ describe('Collection Tab Content store', function () { }); // Dispatch cancel action - store.dispatch(collectionTabModule.cancelSchemaAnalysis() as never); + store.dispatch(collectionTabModule.cancelSchemaAnalysis() as any); // Verify the state is reset to initial expect( @@ -549,7 +530,7 @@ describe('Collection Tab Content store', function () { store.dispatch({ type: 'compass-collection/SchemaAnalysisFailed', error: new Error('No documents found'), - } as never); + } as any); // Trigger the document-inserted event globalAppRegistry.emit( From b5767e218a250dfcd703f93b9e97c4aa21975985 Mon Sep 17 00:00:00 2001 From: Jacob Samuel Lu Date: Fri, 7 Nov 2025 15:21:57 -0500 Subject: [PATCH 08/12] comment --- .../src/stores/collection-tab.ts | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/packages/compass-collection/src/stores/collection-tab.ts b/packages/compass-collection/src/stores/collection-tab.ts index 8f38cb359fd..53bdd7a4110 100644 --- a/packages/compass-collection/src/stores/collection-tab.ts +++ b/packages/compass-collection/src/stores/collection-tab.ts @@ -213,19 +213,22 @@ export function activatePlugin( const currentState = store.getState(); if (shouldRetriggerSchemaAnalysis(currentState)) { // Check if user is in Mock Data Generator experiment variant before re-triggering - void shouldRunSchemaAnalysis( - experimentationServices, - logger, - namespace - ).then((shouldRun) => { - if (shouldRun) { - logger.debug( - 'Re-triggering schema analysis after document insertion', - { namespace } - ); - void store.dispatch(analyzeCollectionSchema()); - } - }); + shouldRunSchemaAnalysis(experimentationServices, logger, namespace) + .then((shouldRun) => { + if (shouldRun) { + logger.debug( + 'Re-triggering schema analysis after document insertion', + { namespace } + ); + void store.dispatch(analyzeCollectionSchema()); + } + }) + .catch((error) => { + logger.debug('Error checking schema analysis experiment', { + namespace: namespace, + error: error instanceof Error ? error.message : String(error), + }); + }); } } } From 4b0290419e3ab865b49524a56fd7f95c5499e13f Mon Sep 17 00:00:00 2001 From: Jacob Samuel Lu Date: Fri, 7 Nov 2025 15:52:57 -0500 Subject: [PATCH 09/12] Selector refactor --- .../collection-header-actions.tsx | 26 +++------ .../collection-header/collection-header.tsx | 22 +++++--- .../src/stores/collection-tab.ts | 54 ++++++++++++++++--- 3 files changed, 71 insertions(+), 31 deletions(-) diff --git a/packages/compass-collection/src/components/collection-header-actions/collection-header-actions.tsx b/packages/compass-collection/src/components/collection-header-actions/collection-header-actions.tsx index a1ef5f33167..76e8db85df6 100644 --- a/packages/compass-collection/src/components/collection-header-actions/collection-header-actions.tsx +++ b/packages/compass-collection/src/components/collection-header-actions/collection-header-actions.tsx @@ -24,11 +24,7 @@ import { useTrackOnChange, type TrackFunction, } from '@mongodb-js/compass-telemetry/provider'; -import { - SCHEMA_ANALYSIS_STATE_ANALYZING, - type SchemaAnalysisStatus, - type SchemaAnalysisError, -} from '../../schema-analysis-types'; +import { type SchemaAnalysisError } from '../../schema-analysis-types'; import { MAX_COLLECTION_NESTING_DEPTH } from '../mock-data-generator-modal/utils'; import { buildChartsUrl, @@ -61,7 +57,8 @@ type CollectionHeaderActionsProps = { hasSchemaAnalysisData: boolean; schemaAnalysisError: SchemaAnalysisError | null; analyzedSchemaDepth: number; - schemaAnalysisStatus: SchemaAnalysisStatus | null; + isCollectionEmpty: boolean; + hasUnsupportedStateError: boolean; }; const CollectionHeaderActions: React.FunctionComponent< @@ -76,8 +73,9 @@ const CollectionHeaderActions: React.FunctionComponent< onOpenMockDataModal, hasSchemaAnalysisData, analyzedSchemaDepth, - schemaAnalysisStatus, schemaAnalysisError, + isCollectionEmpty, + hasUnsupportedStateError, }: CollectionHeaderActionsProps) => { const connectionInfo = useConnectionInfo(); const { id: connectionId, atlasMetadata } = connectionInfo; @@ -117,21 +115,13 @@ const CollectionHeaderActions: React.FunctionComponent< const exceedsMaxNestingDepth = analyzedSchemaDepth > MAX_COLLECTION_NESTING_DEPTH; - const isCollectionEmpty = - !hasSchemaAnalysisData && - schemaAnalysisStatus !== SCHEMA_ANALYSIS_STATE_ANALYZING; - - const hasSchemaAnalysisUnsupportedStateError = Boolean( - schemaAnalysisError && schemaAnalysisError.errorType === 'unsupportedState' - ); - const isView = isReadonly && sourceName && !editViewName; const showViewEdit = isView && !preferencesReadWrite; const shouldDisableMockDataButton = !hasSchemaAnalysisData || exceedsMaxNestingDepth || - hasSchemaAnalysisUnsupportedStateError; + hasUnsupportedStateError; const onMockDataGeneratorCtaButtonClicked = useCallback(() => { track('Mock Data Generator Opened', { @@ -189,7 +179,7 @@ const CollectionHeaderActions: React.FunctionComponent< enabled={ exceedsMaxNestingDepth || isCollectionEmpty || - hasSchemaAnalysisUnsupportedStateError + hasUnsupportedStateError } trigger={
@@ -206,7 +196,7 @@ const CollectionHeaderActions: React.FunctionComponent< } > <> - {hasSchemaAnalysisUnsupportedStateError ? ( + {hasUnsupportedStateError ? ( {schemaAnalysisError?.errorMessage} diff --git a/packages/compass-collection/src/components/collection-header/collection-header.tsx b/packages/compass-collection/src/components/collection-header/collection-header.tsx index bbca8d22b48..0321189da2b 100644 --- a/packages/compass-collection/src/components/collection-header/collection-header.tsx +++ b/packages/compass-collection/src/components/collection-header/collection-header.tsx @@ -23,11 +23,16 @@ import { connect } from 'react-redux'; import { openMockDataGeneratorModal } from '../../modules/collection-tab'; import type { CollectionState } from '../../modules/collection-tab'; import { - SCHEMA_ANALYSIS_STATE_COMPLETE, SCHEMA_ANALYSIS_STATE_ERROR, + SCHEMA_ANALYSIS_STATE_COMPLETE, type SchemaAnalysisStatus, type SchemaAnalysisError, } from '../../schema-analysis-types'; +import { + selectHasSchemaAnalysisData, + selectIsCollectionEmpty, + selectHasUnsupportedStateError, +} from '../../stores/collection-tab'; const collectionHeaderStyles = css({ padding: spacing[400], @@ -73,6 +78,8 @@ type CollectionHeaderProps = { analyzedSchemaDepth: number; schemaAnalysisStatus: SchemaAnalysisStatus | null; schemaAnalysisError: SchemaAnalysisError | null; + isCollectionEmpty: boolean; + hasUnsupportedStateError: boolean; }; const getInsightsForPipeline = (pipeline: any[], isAtlas: boolean) => { @@ -110,8 +117,9 @@ const CollectionHeader: React.FunctionComponent = ({ onOpenMockDataModal, hasSchemaAnalysisData, analyzedSchemaDepth, - schemaAnalysisStatus, schemaAnalysisError, + isCollectionEmpty, + hasUnsupportedStateError, }) => { const darkMode = useDarkMode(); const showInsights = usePreference('showInsights'); @@ -192,8 +200,9 @@ const CollectionHeader: React.FunctionComponent = ({ onOpenMockDataModal={onOpenMockDataModal} hasSchemaAnalysisData={hasSchemaAnalysisData} analyzedSchemaDepth={analyzedSchemaDepth} - schemaAnalysisStatus={schemaAnalysisStatus} schemaAnalysisError={schemaAnalysisError} + isCollectionEmpty={isCollectionEmpty} + hasUnsupportedStateError={hasUnsupportedStateError} />
@@ -209,15 +218,14 @@ const mapStateToProps = (state: CollectionState) => { schemaAnalysis && schemaAnalysis.status === SCHEMA_ANALYSIS_STATE_ERROR ? schemaAnalysis.error : null, - hasSchemaAnalysisData: - schemaAnalysis && - schemaAnalysis.status === SCHEMA_ANALYSIS_STATE_COMPLETE && - Object.keys(schemaAnalysis.processedSchema).length > 0, + hasSchemaAnalysisData: selectHasSchemaAnalysisData(state), analyzedSchemaDepth: schemaAnalysis && schemaAnalysis.status === SCHEMA_ANALYSIS_STATE_COMPLETE ? schemaAnalysis.schemaMetadata.maxNestingDepth : 0, schemaAnalysisStatus: schemaAnalysis?.status || null, + isCollectionEmpty: selectIsCollectionEmpty(state), + hasUnsupportedStateError: selectHasUnsupportedStateError(state), }; }; diff --git a/packages/compass-collection/src/stores/collection-tab.ts b/packages/compass-collection/src/stores/collection-tab.ts index 53bdd7a4110..a8d10bcfedd 100644 --- a/packages/compass-collection/src/stores/collection-tab.ts +++ b/packages/compass-collection/src/stores/collection-tab.ts @@ -31,21 +31,63 @@ import { SCHEMA_ANALYSIS_STATE_INITIAL, SCHEMA_ANALYSIS_STATE_ERROR, SCHEMA_ANALYSIS_STATE_COMPLETE, + SCHEMA_ANALYSIS_STATE_ANALYZING, } from '../schema-analysis-types'; import type { CollectionState } from '../modules/collection-tab'; +/** + * Determines if collection has valid schema analysis data. + * Returns true when analysis is complete and has processed schema data. + */ +export function selectHasSchemaAnalysisData(state: CollectionState): boolean { + return !!( + state.schemaAnalysis && + state.schemaAnalysis.status === SCHEMA_ANALYSIS_STATE_COMPLETE && + Object.keys(state.schemaAnalysis.processedSchema).length > 0 + ); +} + +/** + * Determines if schema analysis error is of 'unsupportedState' type. + * Used for showing specific error messages and disabling certain features. + */ +export function selectHasUnsupportedStateError( + state: CollectionState +): boolean { + return ( + state.schemaAnalysis?.status === SCHEMA_ANALYSIS_STATE_ERROR && + state.schemaAnalysis?.error?.errorType === 'unsupportedState' + ); +} + +/** + * Determines if collection appears empty (no schema data and not analyzing). + * Used for UI states and button enabling/disabling. + */ +export function selectIsCollectionEmpty(state: CollectionState): boolean { + return ( + !selectHasSchemaAnalysisData(state) && + state.schemaAnalysis?.status !== SCHEMA_ANALYSIS_STATE_ANALYZING + ); +} + /** * Determines if schema analysis should be re-triggered after document insertion. * Re-triggers when: * 1. Previous analysis failed (error state) * 2. Analysis completed but no schema data (empty collection) */ -function shouldRetriggerSchemaAnalysis(state: CollectionState): boolean { +export function selectShouldRetriggerSchemaAnalysis( + state: CollectionState +): boolean { + // Don't retrigger if already analyzing + if (state.schemaAnalysis?.status === SCHEMA_ANALYSIS_STATE_ANALYZING) { + return false; + } + return ( - state.schemaAnalysis.status === SCHEMA_ANALYSIS_STATE_ERROR || - (state.schemaAnalysis.status === SCHEMA_ANALYSIS_STATE_COMPLETE && - (!state.schemaAnalysis.processedSchema || - Object.keys(state.schemaAnalysis.processedSchema).length === 0)) + state.schemaAnalysis?.status === SCHEMA_ANALYSIS_STATE_ERROR || + !selectHasSchemaAnalysisData(state) ); } @@ -211,7 +253,7 @@ export function activatePlugin( payload.ns === namespace ) { const currentState = store.getState(); - if (shouldRetriggerSchemaAnalysis(currentState)) { + if (selectShouldRetriggerSchemaAnalysis(currentState)) { // Check if user is in Mock Data Generator experiment variant before re-triggering shouldRunSchemaAnalysis(experimentationServices, logger, namespace) .then((shouldRun) => { From 941f92094288c693f9c4715ce524d5d3c019de25 Mon Sep 17 00:00:00 2001 From: Jacob Samuel Lu Date: Fri, 7 Nov 2025 16:06:58 -0500 Subject: [PATCH 10/12] Import-finished --- .../src/stores/collection-tab.spec.ts | 188 ++++++++++++++++++ .../src/stores/collection-tab.ts | 64 ++++-- 2 files changed, 232 insertions(+), 20 deletions(-) diff --git a/packages/compass-collection/src/stores/collection-tab.spec.ts b/packages/compass-collection/src/stores/collection-tab.spec.ts index 00a47e6e004..48d5469d497 100644 --- a/packages/compass-collection/src/stores/collection-tab.spec.ts +++ b/packages/compass-collection/src/stores/collection-tab.spec.ts @@ -680,4 +680,192 @@ describe('Collection Tab Content store', function () { expect(analyzeCollectionSchemaStub).to.not.have.been.called; }); }); + + describe('import-finished event listener', function () { + it('should re-trigger schema analysis when import is completed for current collection', async function () { + const getAssignment = sandbox.spy(() => + Promise.resolve( + createMockAssignment(ExperimentTestGroup.mockDataGeneratorVariant) + ) + ); + const assignExperiment = sandbox.spy(() => Promise.resolve(null)); + + const store = await configureStore( + undefined, + undefined, + { getAssignment, assignExperiment }, + mockAtlasConnectionInfo, + undefined, + undefined, + { ...defaultMetadata, isReadonly: false, isTimeSeries: false } + ); + + // Wait for initial schema analysis to complete + await waitFor(() => { + expect(analyzeCollectionSchemaStub).to.have.been.calledOnce; + }); + + // Reset the stub to track new calls + analyzeCollectionSchemaStub.resetHistory(); + + // Simulate the empty collection + store.dispatch({ + type: 'compass-collection/SchemaAnalysisFailed', + error: new Error('No documents found'), + }); + + // Emit import-finished event + globalAppRegistry.emit( + 'import-finished', + { + ns: defaultMetadata.namespace, + connectionId: mockAtlasConnectionInfo.current.id, + }, + { connectionId: mockAtlasConnectionInfo.current.id } + ); + + // Wait for schema analysis to be re-triggered + await waitFor(() => { + expect(analyzeCollectionSchemaStub).to.have.been.calledOnce; + }); + }); + + it('should not re-trigger schema analysis for different collection', async function () { + const getAssignment = sandbox.spy(() => + Promise.resolve( + createMockAssignment(ExperimentTestGroup.mockDataGeneratorVariant) + ) + ); + const assignExperiment = sandbox.spy(() => Promise.resolve(null)); + + const store = await configureStore( + undefined, + undefined, + { getAssignment, assignExperiment }, + mockAtlasConnectionInfo, + undefined, + undefined, + { ...defaultMetadata, isReadonly: false, isTimeSeries: false } + ); + + // Wait for initial schema analysis to complete + await waitFor(() => { + expect(analyzeCollectionSchemaStub).to.have.been.calledOnce; + }); + + // Reset the stub to track new calls + analyzeCollectionSchemaStub.resetHistory(); + + // Simulate the empty collection + store.dispatch({ + type: 'compass-collection/SchemaAnalysisFailed', + error: new Error('No documents found'), + }); + + // Emit import-finished event for different collection + globalAppRegistry.emit( + 'import-finished', + { + ns: 'different.collection', + connectionId: mockAtlasConnectionInfo.current.id, + }, + { connectionId: mockAtlasConnectionInfo.current.id } + ); + + // Wait a bit to ensure no action is taken + await new Promise((resolve) => setTimeout(resolve, WAIT_TIME)); + expect(analyzeCollectionSchemaStub).to.not.have.been.called; + }); + + it('should not re-trigger schema analysis for different connection', async function () { + const getAssignment = sandbox.spy(() => + Promise.resolve( + createMockAssignment(ExperimentTestGroup.mockDataGeneratorVariant) + ) + ); + const assignExperiment = sandbox.spy(() => Promise.resolve(null)); + + const store = await configureStore( + undefined, + undefined, + { getAssignment, assignExperiment }, + mockAtlasConnectionInfo, + undefined, + undefined, + { ...defaultMetadata, isReadonly: false, isTimeSeries: false } + ); + + // Wait for initial schema analysis to complete + await waitFor(() => { + expect(analyzeCollectionSchemaStub).to.have.been.calledOnce; + }); + + // Reset the stub to track new calls + analyzeCollectionSchemaStub.resetHistory(); + + // Simulate the empty collection + store.dispatch({ + type: 'compass-collection/SchemaAnalysisFailed', + error: new Error('No documents found'), + }); + + // Emit import-finished event for different connection + globalAppRegistry.emit( + 'import-finished', + { + ns: defaultMetadata.namespace, + connectionId: 'different-connection-id', + }, + { connectionId: 'different-connection-id' } + ); + + // Wait a bit to ensure no action is taken + await new Promise((resolve) => setTimeout(resolve, WAIT_TIME)); + expect(analyzeCollectionSchemaStub).to.not.have.been.called; + }); + + it('should not re-trigger schema analysis when user is not in experiment variant', async function () { + const getAssignment = sandbox.spy(() => + Promise.resolve( + createMockAssignment(ExperimentTestGroup.mockDataGeneratorControl) + ) + ); + const assignExperiment = sandbox.spy(() => Promise.resolve(null)); + + await configureStore( + undefined, + undefined, + { getAssignment, assignExperiment }, + mockAtlasConnectionInfo + ); + + // Wait for initial assignment check + await waitFor(() => { + expect(getAssignment).to.have.been.calledOnce; + }); + + // Schema analysis should not have been called initially + expect(analyzeCollectionSchemaStub).to.not.have.been.called; + + // Verify the schema analysis state is INITIAL (as expected for control variant) + const initialState = store.getState() as { + schemaAnalysis: { status: string }; + }; + expect(initialState.schemaAnalysis.status).to.equal('initial'); + + // Emit import-finished event + globalAppRegistry.emit( + 'import-finished', + { + ns: defaultMetadata.namespace, + connectionId: mockAtlasConnectionInfo.current.id, + }, + { connectionId: mockAtlasConnectionInfo.current.id } + ); + + // Wait a bit to ensure schema analysis is not called + await new Promise((resolve) => setTimeout(resolve, WAIT_TIME)); + expect(analyzeCollectionSchemaStub).to.not.have.been.called; + }); + }); }); diff --git a/packages/compass-collection/src/stores/collection-tab.ts b/packages/compass-collection/src/stores/collection-tab.ts index a8d10bcfedd..e54a989b8ab 100644 --- a/packages/compass-collection/src/stores/collection-tab.ts +++ b/packages/compass-collection/src/stores/collection-tab.ts @@ -233,6 +233,28 @@ export function activatePlugin( store.dispatch(selectTab('Schema')); }); + const handleSchemaAnalysisRetrigger = (eventType: string) => { + const currentState = store.getState(); + if (selectShouldRetriggerSchemaAnalysis(currentState)) { + // Check if user is in Mock Data Generator experiment variant before re-triggering + shouldRunSchemaAnalysis(experimentationServices, logger, namespace) + .then((shouldRun) => { + if (shouldRun) { + logger.debug(`Re-triggering schema analysis after ${eventType}`, { + namespace, + }); + void store.dispatch(analyzeCollectionSchema()); + } + }) + .catch((error) => { + logger.debug('Error checking schema analysis experiment', { + namespace: namespace, + error: error instanceof Error ? error.message : String(error), + }); + }); + } + }; + // Listen for document insertions to re-trigger schema analysis for previously empty collections on( globalAppRegistry, @@ -252,26 +274,28 @@ export function activatePlugin( connectionId === connectionInfoRef.current.id && payload.ns === namespace ) { - const currentState = store.getState(); - if (selectShouldRetriggerSchemaAnalysis(currentState)) { - // Check if user is in Mock Data Generator experiment variant before re-triggering - shouldRunSchemaAnalysis(experimentationServices, logger, namespace) - .then((shouldRun) => { - if (shouldRun) { - logger.debug( - 'Re-triggering schema analysis after document insertion', - { namespace } - ); - void store.dispatch(analyzeCollectionSchema()); - } - }) - .catch((error) => { - logger.debug('Error checking schema analysis experiment', { - namespace: namespace, - error: error instanceof Error ? error.message : String(error), - }); - }); - } + handleSchemaAnalysisRetrigger('document insertion'); + } + } + ); + + // Listen for import completion to re-trigger schema analysis for previously empty collections + on( + globalAppRegistry, + 'import-finished', + ( + payload: { + ns: string; + connectionId?: string; + }, + { connectionId }: { connectionId?: string } = {} + ) => { + // Ensure event is for the current connection and namespace + if ( + connectionId === connectionInfoRef.current.id && + payload.ns === namespace + ) { + handleSchemaAnalysisRetrigger('import finished'); } } ); From 457d4d84fef681963c3468baf062262cd2156a03 Mon Sep 17 00:00:00 2001 From: Jacob Samuel Lu Date: Mon, 10 Nov 2025 13:42:42 -0500 Subject: [PATCH 11/12] Comment --- .../collection-header-actions.tsx | 4 +--- .../compass-collection/src/stores/collection-tab.ts | 11 ++++------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/packages/compass-collection/src/components/collection-header-actions/collection-header-actions.tsx b/packages/compass-collection/src/components/collection-header-actions/collection-header-actions.tsx index 76e8db85df6..2253ddc03a7 100644 --- a/packages/compass-collection/src/components/collection-header-actions/collection-header-actions.tsx +++ b/packages/compass-collection/src/components/collection-header-actions/collection-header-actions.tsx @@ -119,9 +119,7 @@ const CollectionHeaderActions: React.FunctionComponent< const showViewEdit = isView && !preferencesReadWrite; const shouldDisableMockDataButton = - !hasSchemaAnalysisData || - exceedsMaxNestingDepth || - hasUnsupportedStateError; + !hasSchemaAnalysisData || exceedsMaxNestingDepth; const onMockDataGeneratorCtaButtonClicked = useCallback(() => { track('Mock Data Generator Opened', { diff --git a/packages/compass-collection/src/stores/collection-tab.ts b/packages/compass-collection/src/stores/collection-tab.ts index e54a989b8ab..16251ec13d2 100644 --- a/packages/compass-collection/src/stores/collection-tab.ts +++ b/packages/compass-collection/src/stores/collection-tab.ts @@ -73,9 +73,8 @@ export function selectIsCollectionEmpty(state: CollectionState): boolean { /** * Determines if schema analysis should be re-triggered after document insertion. - * Re-triggers when: - * 1. Previous analysis failed (error state) - * 2. Analysis completed but no schema data (empty collection) + * Re-triggers when collection has no valid schema analysis data (error states, + * initial state, and completed analysis with empty schema). */ export function selectShouldRetriggerSchemaAnalysis( state: CollectionState @@ -85,10 +84,8 @@ export function selectShouldRetriggerSchemaAnalysis( return false; } - return ( - state.schemaAnalysis?.status === SCHEMA_ANALYSIS_STATE_ERROR || - !selectHasSchemaAnalysisData(state) - ); + // Re-trigger if no valid schema data + return !selectHasSchemaAnalysisData(state); } /** From a2078ad6240ffc4e75f719f01a2deebc9921ad5b Mon Sep 17 00:00:00 2001 From: Jacob Samuel Lu Date: Tue, 11 Nov 2025 13:29:26 -0500 Subject: [PATCH 12/12] Empty collection refactor --- .../collection-header-actions.spec.tsx | 35 +++++++++++++++++-- .../src/modules/collection-tab.ts | 16 ++++++++- .../src/schema-analysis-types.ts | 7 +++- .../src/stores/collection-tab.spec.ts | 10 +++--- .../src/stores/collection-tab.ts | 4 +-- 5 files changed, 60 insertions(+), 12 deletions(-) diff --git a/packages/compass-collection/src/components/collection-header-actions/collection-header-actions.spec.tsx b/packages/compass-collection/src/components/collection-header-actions/collection-header-actions.spec.tsx index 763e81b165b..f279e9097f6 100644 --- a/packages/compass-collection/src/components/collection-header-actions/collection-header-actions.spec.tsx +++ b/packages/compass-collection/src/components/collection-header-actions/collection-header-actions.spec.tsx @@ -58,8 +58,9 @@ describe('CollectionHeaderActions [Component]', function () { onOpenMockDataModal={sinon.stub()} hasSchemaAnalysisData={true} analyzedSchemaDepth={2} - schemaAnalysisStatus="complete" schemaAnalysisError={null} + isCollectionEmpty={false} + hasUnsupportedStateError={false} {...props} /> @@ -354,7 +355,9 @@ describe('CollectionHeaderActions [Component]', function () { isReadonly: false, hasSchemaAnalysisData: true, analyzedSchemaDepth: MAX_COLLECTION_NESTING_DEPTH + 1, - schemaAnalysisStatus: 'complete', + schemaAnalysisError: null, + isCollectionEmpty: false, + hasUnsupportedStateError: false, onOpenMockDataModal: sinon.stub(), }, {}, @@ -374,11 +377,37 @@ describe('CollectionHeaderActions [Component]', function () { namespace: 'test.collection', isReadonly: false, hasSchemaAnalysisData: false, - schemaAnalysisStatus: 'error', schemaAnalysisError: { errorType: 'unsupportedState', errorMessage: 'Unsupported state', }, + isCollectionEmpty: false, + hasUnsupportedStateError: true, + onOpenMockDataModal: sinon.stub(), + }, + {}, + atlasConnectionInfo + ); + + const button = screen.getByTestId( + 'collection-header-generate-mock-data-button' + ); + expect(button).to.exist; + expect(button).to.have.attribute('aria-disabled', 'true'); + }); + + it('should disable button when collection is empty', async function () { + await renderCollectionHeaderActions( + { + namespace: 'test.collection', + isReadonly: false, + hasSchemaAnalysisData: false, + schemaAnalysisError: { + errorType: 'empty', + errorMessage: 'No documents found in the collection to analyze.', + }, + isCollectionEmpty: true, + hasUnsupportedStateError: false, onOpenMockDataModal: sinon.stub(), }, {}, diff --git a/packages/compass-collection/src/modules/collection-tab.ts b/packages/compass-collection/src/modules/collection-tab.ts index 09c243f18d1..741bed1e674 100644 --- a/packages/compass-collection/src/modules/collection-tab.ts +++ b/packages/compass-collection/src/modules/collection-tab.ts @@ -50,6 +50,13 @@ const DEFAULT_SAMPLE_SIZE = 100; const NO_DOCUMENTS_ERROR = 'No documents found in the collection to analyze.'; +export class EmptyCollectionError extends Error { + constructor() { + super(NO_DOCUMENTS_ERROR); + this.name = 'EmptyCollectionError'; + } +} + function isAction( action: AnyAction, type: A['type'] @@ -68,6 +75,13 @@ function getErrorDetails(error: Error): SchemaAnalysisError { }; } + if (error instanceof EmptyCollectionError) { + return { + errorType: 'empty', + errorMessage: error.message, + }; + } + const errorCode = (error as MongoError).code; const errorMessage = error.message || 'Unknown error'; let errorType: SchemaAnalysisError['errorType'] = 'general'; @@ -756,7 +770,7 @@ export const analyzeCollectionSchema = (): CollectionThunkAction< logger.debug(NO_DOCUMENTS_ERROR); dispatch({ type: CollectionActions.SchemaAnalysisFailed, - error: new Error(NO_DOCUMENTS_ERROR), + error: new EmptyCollectionError(), }); return; } diff --git a/packages/compass-collection/src/schema-analysis-types.ts b/packages/compass-collection/src/schema-analysis-types.ts index b1705b41cc6..da98c0fbc3a 100644 --- a/packages/compass-collection/src/schema-analysis-types.ts +++ b/packages/compass-collection/src/schema-analysis-types.ts @@ -22,7 +22,12 @@ export type SchemaAnalysisStartedState = { export type SchemaAnalysisError = { errorMessage: string; - errorType: 'timeout' | 'highComplexity' | 'general' | 'unsupportedState'; + errorType: + | 'timeout' + | 'highComplexity' + | 'general' + | 'unsupportedState' + | 'empty'; }; export type SchemaAnalysisErrorState = { diff --git a/packages/compass-collection/src/stores/collection-tab.spec.ts b/packages/compass-collection/src/stores/collection-tab.spec.ts index 48d5469d497..398bbac6809 100644 --- a/packages/compass-collection/src/stores/collection-tab.spec.ts +++ b/packages/compass-collection/src/stores/collection-tab.spec.ts @@ -1,6 +1,6 @@ import type { CollectionTabOptions } from './collection-tab'; import { activatePlugin } from './collection-tab'; -import { selectTab } from '../modules/collection-tab'; +import { selectTab, EmptyCollectionError } from '../modules/collection-tab'; import * as collectionTabModule from '../modules/collection-tab'; import { waitFor } from '@mongodb-js/testing-library-compass'; import Sinon from 'sinon'; @@ -529,7 +529,7 @@ describe('Collection Tab Content store', function () { // Simulate the empty collection store.dispatch({ type: 'compass-collection/SchemaAnalysisFailed', - error: new Error('No documents found'), + error: new EmptyCollectionError(), } as any); // Trigger the document-inserted event @@ -711,7 +711,7 @@ describe('Collection Tab Content store', function () { // Simulate the empty collection store.dispatch({ type: 'compass-collection/SchemaAnalysisFailed', - error: new Error('No documents found'), + error: new EmptyCollectionError(), }); // Emit import-finished event @@ -759,7 +759,7 @@ describe('Collection Tab Content store', function () { // Simulate the empty collection store.dispatch({ type: 'compass-collection/SchemaAnalysisFailed', - error: new Error('No documents found'), + error: new EmptyCollectionError(), }); // Emit import-finished event for different collection @@ -806,7 +806,7 @@ describe('Collection Tab Content store', function () { // Simulate the empty collection store.dispatch({ type: 'compass-collection/SchemaAnalysisFailed', - error: new Error('No documents found'), + error: new EmptyCollectionError(), }); // Emit import-finished event for different connection diff --git a/packages/compass-collection/src/stores/collection-tab.ts b/packages/compass-collection/src/stores/collection-tab.ts index 16251ec13d2..7b558c41d73 100644 --- a/packages/compass-collection/src/stores/collection-tab.ts +++ b/packages/compass-collection/src/stores/collection-tab.ts @@ -66,8 +66,8 @@ export function selectHasUnsupportedStateError( */ export function selectIsCollectionEmpty(state: CollectionState): boolean { return ( - !selectHasSchemaAnalysisData(state) && - state.schemaAnalysis?.status !== SCHEMA_ANALYSIS_STATE_ANALYZING + state.schemaAnalysis?.status === SCHEMA_ANALYSIS_STATE_ERROR && + state.schemaAnalysis?.error?.errorType === 'empty' ); }