chore(compass-indexes): use dataService from thunk extra args, remove from store (#6192)

* use dataService from thunk extra args, remove from store

* dataService always exists now

* more flakyness

* skip flaky test

* remove unused debug

* remove another unused debug

Author: lerouxb

packages/compass-indexes/src/components/indexes/indexes.spec.tsx

@@ -32,8 +32,11 @@ const DEFAULT_PROPS: Partial<RootState> = {
   },
 } as any;
 
-const renderIndexes = (props: Partial<RootState> = {}) => {
-  const store = setupStore();
+const renderIndexes = (
+  props: Partial<RootState> = {},
+  dataProvider: Partial<IndexesDataService> = {}
+) => {
+  const store = setupStore(undefined, dataProvider);
 
   const allProps: Partial<RootState> = {
     ...DEFAULT_PROPS,
@@ -78,12 +81,14 @@ describe('Indexes Component', function () {
   });
 
   it('renders indexes toolbar when there is a search indexes error', async function () {
-    const store = renderIndexes();
-
     // the component will load the search indexes the moment we switch to them
-    store.getState()!.dataService!.getSearchIndexes = function () {
-      return Promise.reject(new Error('This is an error.'));
+    const getSearchIndexesStub = sinon
+      .stub()
+      .rejects(new Error('This is an error.'));
+    const dataProvider = {
+      getSearchIndexes: getSearchIndexesStub,
     };
+    renderIndexes({}, dataProvider);
 
     const toolbar = screen.getByTestId('indexes-toolbar');
     expect(toolbar).to.exist;
@@ -267,11 +272,11 @@ describe('Indexes Component', function () {
 
   context('search indexes', function () {
     it('renders the search indexes table if the current view changes to search indexes', async function () {
-      const store = renderIndexes();
-
-      store.getState()!.dataService!.getSearchIndexes = function () {
-        return Promise.resolve(searchIndexes);
+      const getSearchIndexesStub = sinon.stub().resolves(searchIndexes);
+      const dataProvider = {
+        getSearchIndexes: getSearchIndexesStub,
       };
+      renderIndexes({}, dataProvider);
 
       // switch to the Search Indexes tab
       const toolbar = screen.getByTestId('indexes-toolbar');
@@ -284,18 +289,17 @@ describe('Indexes Component', function () {
     });
 
     it('refreshes the search indexes if the search indexes view is active', async function () {
-      const store = renderIndexes();
-
-      const spy = sinon.spy(
-        store.getState()?.dataService as IndexesDataService,
-        'getSearchIndexes'
-      );
+      const getSearchIndexesStub = sinon.stub().resolves(searchIndexes);
+      const dataProvider = {
+        getSearchIndexes: getSearchIndexesStub,
+      };
+      renderIndexes({}, dataProvider);
 
       // switch to the Search Indexes tab
       const toolbar = screen.getByTestId('indexes-toolbar');
       fireEvent.click(within(toolbar).getByText('Search Indexes'));
 
-      expect(spy.callCount).to.equal(1);
+      expect(getSearchIndexesStub.callCount).to.equal(1);
 
       // click the refresh button
       const refreshButton = within(toolbar).getByText('Refresh');
@@ -304,7 +308,7 @@ describe('Indexes Component', function () {
       );
       fireEvent.click(refreshButton);
 
-      expect(spy.callCount).to.equal(2);
+      expect(getSearchIndexesStub.callCount).to.equal(2);
     });
 
     it('switches to the search indexes table when a search index is created', async function () {

packages/compass-indexes/src/components/search-indexes-modals/base-search-index-modal.spec.tsx

@@ -164,7 +164,8 @@ describe('Base Search Index Modal', function () {
         });
       });
 
-      it('resets the template on type switch', async function () {
+      // TODO(COMPASS-7557): super flaky even on mac
+      it.skip('resets the template on type switch', async function () {
         userEvent.click(
           screen.getByTestId('search-index-type-vectorSearch-button'),
           undefined,

packages/compass-indexes/src/modules/data-service.spec.ts

@@ -1,7 +1,6 @@
 import { combineReducers } from 'redux';
 import type { Action, AnyAction } from 'redux';
 import type AppRegistry from 'hadron-app-registry';
-import dataService from './data-service';
 import isWritable from './is-writable';
 import indexView from './index-view';
 import isReadonlyView from './is-readonly-view';
@@ -11,16 +10,17 @@ import searchIndexes from './search-indexes';
 import serverVersion from './server-version';
 import namespace from './namespace';
 import type { ThunkAction, ThunkDispatch } from 'redux-thunk';
+import type { DataService } from 'mongodb-data-service';
 import type { Logger } from '@mongodb-js/compass-logging';
 import type { TrackFunction } from '@mongodb-js/compass-telemetry';
 import type { ConnectionInfoAccess } from '@mongodb-js/compass-connections/provider';
+import type { IndexesDataServiceProps } from '../stores/store';
 
 const reducer = combineReducers({
   isWritable,
   isReadonlyView,
   indexView,
   description,
-  dataService,
   serverVersion,
   namespace,
   regularIndexes,
@@ -35,6 +35,7 @@ export type IndexesExtraArgs = {
   localAppRegistry: AppRegistry;
   logger: Logger;
   track: TrackFunction;
+  dataService: Pick<DataService, IndexesDataServiceProps>;
   connectionInfoAccess: ConnectionInfoAccess;
 };
 export type IndexesThunkDispatch<A extends Action = AnyAction> = ThunkDispatch<

packages/compass-indexes/src/modules/data-service.ts

@@ -69,23 +69,6 @@ describe('regular-indexes module', function () {
       expect(indexesSpy.callCount).to.equal(0);
     });
 
-    it('when dataService is not connected, sets refreshing to false', async function () {
-      const store = setupStore({}, {
-        isConnected() {
-          return false;
-        },
-      } as any);
-      store.dispatch({
-        type: ActionTypes.IndexesAdded,
-        indexes: defaultSortedIndexes,
-      });
-      await store.dispatch(fetchIndexes());
-
-      const state = store.getState().regularIndexes;
-      expect(state.indexes).to.deep.equal(defaultSortedIndexes);
-      expect(state.isRefreshing).to.equal(false);
-    });
-
     it('sets indexes to empty array when there is an error', async function () {
       const error = new Error('failed to connect to server');
       const store = setupStore({}, {

packages/compass-indexes/src/modules/index.ts

@@ -209,10 +209,9 @@ export const fetchIndexes = (): IndexesThunkAction<
   Promise<void>,
   RegularIndexesActions
 > => {
-  return async (dispatch, getState, { logger: { debug } }) => {
+  return async (dispatch, getState, { dataService }) => {
     const {
       isReadonlyView,
-      dataService,
       namespace,
       regularIndexes: { inProgressIndexes },
     } = getState();
@@ -222,12 +221,6 @@ export const fetchIndexes = (): IndexesThunkAction<
       return;
     }
 
-    if (!dataService || !dataService.isConnected()) {
-      dispatch(setIsRefreshing(false));
-      debug('warning: trying to load indexes but dataService is disconnected');
-      return;
-    }
-
     try {
       dispatch(setError(null));
       const indexes = await dataService.indexes(namespace);
@@ -304,8 +297,8 @@ export const showCreateModal = (): IndexesThunkAction<void> => {
 export const hideIndex = (
   indexName: string
 ): IndexesThunkAction<Promise<void>> => {
-  return async (dispatch, getState) => {
-    const { dataService, namespace } = getState();
+  return async (dispatch, getState, { dataService }) => {
+    const { namespace } = getState();
     const confirmed = await showConfirmation({
       title: `Hiding \`${indexName}\``,
       description: hideModalDescription(indexName),
@@ -338,8 +331,8 @@ export const hideIndex = (
 export const unhideIndex = (
   indexName: string
 ): IndexesThunkAction<Promise<void>> => {
-  return async (dispatch, getState) => {
-    const { namespace, dataService } = getState();
+  return async (dispatch, getState, { dataService }) => {
+    const { namespace } = getState();
     const confirmed = await showConfirmation({
       title: `Unhiding \`${indexName}\``,
       description: unhideModalDescription(indexName),

packages/compass-indexes/src/modules/regular-indexes.spec.ts

@@ -23,12 +23,21 @@ import { writeStateChanged } from './is-writable';
 describe('search-indexes module', function () {
   let store: ReturnType<typeof setupStore>;
   let dataProvider: Partial<IndexesDataService>;
-  let getSearchIndexesStub: any;
+  let createSearchIndexStub: sinon.SinonStub;
+  let updateSearchIndexStub: sinon.SinonStub;
+  let getSearchIndexesStub: sinon.SinonStub;
+  let dropSearchIndexStub: sinon.SinonStub;
 
   beforeEach(function () {
+    createSearchIndexStub = sinon.stub().resolves('foo');
+    updateSearchIndexStub = sinon.stub().resolves();
+    getSearchIndexesStub = sinon.stub().resolves(searchIndexes);
+    dropSearchIndexStub = sinon.stub().resolves();
     dataProvider = {
-      createSearchIndex: sinon.spy(),
-      updateSearchIndex: sinon.spy(),
+      createSearchIndex: createSearchIndexStub,
+      updateSearchIndex: updateSearchIndexStub,
+      getSearchIndexes: getSearchIndexesStub,
+      dropSearchIndex: dropSearchIndexStub,
     };
 
     store = setupStore(
@@ -38,13 +47,6 @@ describe('search-indexes module', function () {
       },
       dataProvider
     );
-
-    getSearchIndexesStub = sinon
-      .stub(
-        store.getState().dataService as IndexesDataService,
-        'getSearchIndexes'
-      )
-      .resolves(searchIndexes);
   });
 
   it('has not available search indexes state by default', function () {
@@ -79,12 +81,6 @@ describe('search-indexes module', function () {
       expect(store.getState().searchIndexes.status).to.equal('NOT_READY');
     });
 
-    it('does nothing if there is no dataService', function () {
-      store.getState().dataService = null;
-      store.dispatch(fetchSearchIndexes);
-      // would throw if it tried to use it
-    });
-
     it('fetches the indexes', async function () {
       expect(getSearchIndexesStub.callCount).to.equal(0);
       expect(store.getState().searchIndexes.status).to.equal('NOT_READY');
@@ -105,17 +101,11 @@ describe('search-indexes module', function () {
       expect(store.getState().searchIndexes.status).to.equal('READY');
 
       // replace the stub
-      getSearchIndexesStub.restore();
-      getSearchIndexesStub = sinon
-        .stub(
-          store.getState().dataService as IndexesDataService,
-          'getSearchIndexes'
-        )
-        .callsFake(() => {
-          return new Promise(() => {
-            // never resolves
-          });
+      getSearchIndexesStub.callsFake(() => {
+        return new Promise(() => {
+          // never resolves
         });
+      });
 
       // not awaiting because REFRESHING happens during the action
       void store.dispatch(fetchSearchIndexes());
@@ -150,13 +140,7 @@ describe('search-indexes module', function () {
 
     it('sets the status to ERROR if loading the indexes fails', async function () {
       // replace the stub
-      getSearchIndexesStub.restore();
-      getSearchIndexesStub = sinon
-        .stub(
-          store.getState().dataService as IndexesDataService,
-          'getSearchIndexes'
-        )
-        .rejects(new Error('this is an error'));
+      getSearchIndexesStub.rejects(new Error('this is an error'));
 
       await store.dispatch(fetchSearchIndexes());
 
@@ -257,29 +241,25 @@ describe('search-indexes module', function () {
   });
 
   context('drop search index', function () {
-    let dropSearchIndexStub: sinon.SinonStub;
     let showConfirmationStub: sinon.SinonStub;
     beforeEach(function () {
-      dropSearchIndexStub = sinon.stub(
-        store.getState().dataService as IndexesDataService,
-        'dropSearchIndex'
-      );
       showConfirmationStub = sinon.stub(searchIndexesSlice, 'showConfirmation');
     });
 
     afterEach(function () {
       showConfirmationStub.restore();
-      dropSearchIndexStub.restore();
     });
 
     it('does not drop index when user does not confirm', async function () {
-      showConfirmationStub.resolves(false);
+      dropSearchIndexStub.resolves(false);
       await store.dispatch(dropSearchIndex('index_name'));
       expect(dropSearchIndexStub.callCount).to.equal(0);
     });
 
     it('drops index successfully', async function () {
       showConfirmationStub.resolves(true);
+      dropSearchIndexStub.resolves(true);
+
       dropSearchIndexStub.resolves(true);
       await store.dispatch(dropSearchIndex('index_name'));
       expect(dropSearchIndexStub.firstCall.args).to.deep.equal([