Added `selectedTab` to global search state (#214)

Breadcrumbs will return to the correct search tab and page index

- Added selectedTab to global search state, removed selectedTab from SearchPage state.
- Simplified SearchPage logic. Every action first updates the page URL, which is then read by componentDidUpdate and compared to the current state to determine if an action needs to be taken.
- Split SearchResponsePayload into SearchResponsePayload and searchAllResponsepayload
- Added jest mock functions to mock router history functions.
- Clicking breadcrumb does not re-execute a search
- Fixes redirect issue creating multiple window.history entries.

amundsen_application/static/js/components/SearchPage/index.tsx

@@ -4,6 +4,7 @@ import { bindActionCreators } from 'redux';
 import * as DocumentTitle from 'react-document-title';
 import * as qs from 'simple-query-string';
 import { RouteComponentProps } from 'react-router';
+import { Search } from 'history';
 
 import AppConfig from 'config/config';
 import LoadingSpinner from 'components/common/LoadingSpinner';
@@ -13,17 +14,18 @@ import TabsComponent from 'components/common/Tabs';
 import SearchBar from './SearchBar';
 
 import { GlobalState } from 'ducks/rootReducer';
-import { searchAll, searchResource } from 'ducks/search/reducer';
+import { searchAll, searchResource, updateSearchTab } from 'ducks/search/reducer';
 import {
   DashboardSearchResults,
   SearchAllRequest,
   SearchResourceRequest,
   SearchResults,
   TableSearchResults,
+  UpdateSearchTabRequest,
   UserSearchResults,
 } from 'ducks/search/types';
 
-import { Resource, ResourceType, SearchAllOptions } from 'interfaces';
+import { Resource, ResourceType } from 'interfaces';
 
 // TODO: Use css-modules instead of 'import'
 import './styles.scss';
@@ -44,6 +46,7 @@ import {
 
 export interface StateFromProps {
   searchTerm: string;
+  selectedTab: ResourceType;
   isLoading: boolean;
   tables: TableSearchResults;
   dashboards: DashboardSearchResults;
@@ -51,51 +54,78 @@ export interface StateFromProps {
 }
 
 export interface DispatchFromProps {
-  searchAll: (term: string, options?: SearchAllOptions) => SearchAllRequest;
-  searchResource: (resource: ResourceType, term: string, pageIndex: number) => SearchResourceRequest;
+  searchAll: (term: string, selectedTab: ResourceType, pageIndex: number) => SearchAllRequest;
+  searchResource: (term: string, resource: ResourceType, pageIndex: number) => SearchResourceRequest;
+  updateSearchTab: (selectedTab: ResourceType) => UpdateSearchTabRequest;
 }
 
 export type SearchPageProps = StateFromProps & DispatchFromProps & RouteComponentProps<any>;
 
-interface SearchPageState {
-  selectedTab: ResourceType;
-}
-
-export class SearchPage extends React.Component<SearchPageProps, SearchPageState> {
+export class SearchPage extends React.Component<SearchPageProps> {
   public static defaultProps: Partial<SearchPageProps> = {};
 
   constructor(props) {
     super(props);
-
-    this.state = {
-      selectedTab: ResourceType.table,
-    };
   }
 
   componentDidMount() {
-    const params = qs.parse(this.props.location.search);
-    const { searchTerm, pageIndex, selectedTab } = params;
-    const { term, index, currentTab } = this.getSanitizedUrlParams(searchTerm, pageIndex, selectedTab);
-    this.setState({ selectedTab: currentTab });
-    if (term !== '') {
-      this.props.searchAll(term, this.createSearchOptions(index, currentTab));
-      if (currentTab !== selectedTab || pageIndex !== index) {
-        this.updatePageUrl(term, currentTab, index);
+    const urlSearchParams = this.getUrlParams(this.props.location.search);
+    const globalStateParams = this.getGlobalStateParams();
+
+    if (this.shouldUpdateFromGlobalState(urlSearchParams, globalStateParams)) {
+       this.updatePageUrl(globalStateParams.term, globalStateParams.tab, globalStateParams.index, true);
+
+    } else if (this.shouldUpdateFromUrlParams(urlSearchParams, globalStateParams)) {
+      this.props.searchAll(urlSearchParams.term, urlSearchParams.tab, urlSearchParams.index);
+      this.updatePageUrl(urlSearchParams.term, urlSearchParams.tab, urlSearchParams.index, true);
+    }
+  }
+
+  shouldUpdateFromGlobalState(urlParams, globalStateParams): boolean {
+    return urlParams.term === '' && globalStateParams.term !== '';
+  }
+
+  shouldUpdateFromUrlParams(urlParams, globalStateParams): boolean {
+    return urlParams.term !== '' && urlParams.term !== globalStateParams.term;
+  }
+
+  componentDidUpdate(prevProps: SearchPageProps) {
+    const nextUrlParams = this.getUrlParams(this.props.location.search);
+    const prevUrlParams = this.getUrlParams(prevProps.location.search);
+
+    // If urlParams and globalState are synced, no need to update
+    if (this.isUrlStateSynced(nextUrlParams)) return;
+
+    // Capture any updates in URL
+    if (this.shouldUpdateSearchTerm(nextUrlParams, prevUrlParams)) {
+      this.props.searchAll(nextUrlParams.term, nextUrlParams.tab, nextUrlParams.index);
+
+    } else if (this.shouldUpdateTab(nextUrlParams, prevUrlParams)) {
+      this.props.updateSearchTab(nextUrlParams.tab)
+
+    } else if (this.shouldUpdatePageIndex(nextUrlParams, prevUrlParams)) {
+      this.props.searchResource(nextUrlParams.term, nextUrlParams.tab, nextUrlParams.index);
     }
   }
+
+  isUrlStateSynced(urlParams): boolean {
+    const globalStateParams = this.getGlobalStateParams();
+
+    return urlParams.term === globalStateParams.term &&
+      urlParams.tab === globalStateParams.tab &&
+      urlParams.index === globalStateParams.index;
   }
 
-  componentDidUpdate(prevProps) {
-    if (this.props.location.search !== prevProps.location.search) {
-      const params = qs.parse(this.props.location.search);
-      const { searchTerm, pageIndex, selectedTab } = params;
-      const { term, index, currentTab } = this.getSanitizedUrlParams(searchTerm, pageIndex, selectedTab);
-      this.setState({ selectedTab: currentTab });
-      const prevTerm = prevProps.searchTerm;
-      if (term !== prevTerm) {
-        this.props.searchAll(term, this.createSearchOptions(index, currentTab));
+  shouldUpdateSearchTerm(nextUrlParams, prevUrlParams): boolean {
+    return nextUrlParams.term !== prevUrlParams.term;
   }
+
+  shouldUpdateTab(nextUrlParams, prevUrlParams): boolean {
+    return nextUrlParams.tab !== prevUrlParams.tab;
   }
+
+  shouldUpdatePageIndex(nextUrlParams, prevUrlParams): boolean {
+    return nextUrlParams.index !== prevUrlParams.index;
   }
 
   getSelectedTabByResourceType = (newTab: ResourceType): ResourceType => {
@@ -105,24 +135,29 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState
         return newTab;
       case ResourceType.dashboard:
       default:
-        return this.state.selectedTab;
+        return this.props.selectedTab;
     }
   };
 
-  createSearchOptions = (pageIndex: number, selectedTab: ResourceType) => {
+  getUrlParams(search: Search) {
+    const urlParams = qs.parse(search);
+    const { searchTerm, pageIndex, selectedTab } = urlParams;
+    const index = parseInt(pageIndex, 10);
     return {
-      dashboardIndex: (selectedTab === ResourceType.dashboard) ? pageIndex : 0,
-      userIndex: (selectedTab === ResourceType.user) ? pageIndex : 0,
-      tableIndex: (selectedTab === ResourceType.table) ? pageIndex : 0,
+      term: (searchTerm || '').trim(),
+      tab: this.getSelectedTabByResourceType(selectedTab),
+      index: isNaN(index) ? 0 : index,
     };
   };
 
-  getSanitizedUrlParams = (searchTerm: string, pageIndex: number, selectedTab: ResourceType) => {
-    const currentTab = this.getSelectedTabByResourceType(selectedTab);
-    const index = pageIndex || 0;
-    const term = searchTerm ? searchTerm : "";
-    return {term, index, currentTab};
+  getGlobalStateParams() {
+    return {
+      term: this.props.searchTerm,
+      tab: this.props.selectedTab,
+      index: this.getPageIndexByResourceType(this.props.selectedTab),
     };
+  }
+
 
   getPageIndexByResourceType = (tab: ResourceType): number => {
     switch(tab) {
@@ -137,19 +172,22 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState
   };
 
   onPaginationChange = (index: number): void => {
-    this.props.searchResource(this.state.selectedTab, this.props.searchTerm, index);
-    this.updatePageUrl(this.props.searchTerm, this.state.selectedTab, index);
+    this.updatePageUrl(this.props.searchTerm, this.props.selectedTab, index);
   };
 
   onTabChange = (tab: ResourceType): void => {
-    const currentTab = this.getSelectedTabByResourceType(tab);
-    this.setState({ selectedTab: currentTab });
-    this.updatePageUrl(this.props.searchTerm, currentTab, this.getPageIndexByResourceType(currentTab));
+    const newTab = this.getSelectedTabByResourceType(tab);
+    this.updatePageUrl(this.props.searchTerm, newTab, this.getPageIndexByResourceType(newTab));
   };
 
-  updatePageUrl = (searchTerm: string, tab: ResourceType, pageIndex: number): void => {
+  updatePageUrl = (searchTerm: string, tab: ResourceType, pageIndex: number, replace: boolean = false): void => {
     const pathName = `/search?searchTerm=${searchTerm}&selectedTab=${tab}&pageIndex=${pageIndex}`;
+
+    if (replace) {
+      this.props.history.replace(pathName);
+    } else {
       this.props.history.push(pathName);
+    }
   };
 
   renderSearchResults = () => {
@@ -173,7 +211,7 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState
         <TabsComponent
           tabs={ tabConfig }
           defaultTab={ ResourceType.table }
-          activeKey={ this.state.selectedTab }
+          activeKey={ this.props.selectedTab }
           onSelect={ this.onTabChange }
         />
       </div>
@@ -283,6 +321,7 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState
 export const mapStateToProps = (state: GlobalState) => {
   return {
     searchTerm: state.search.search_term,
+    selectedTab: state.search.selectedTab,
     isLoading: state.search.isLoading,
     tables: state.search.tables,
     users: state.search.users,
@@ -291,7 +330,7 @@ export const mapStateToProps = (state: GlobalState) => {
 };
 
 export const mapDispatchToProps = (dispatch: any) => {
-  return bindActionCreators({ searchAll, searchResource } , dispatch);
+  return bindActionCreators({ searchAll, searchResource, updateSearchTab }, dispatch);
 };
 
 export default connect<StateFromProps, DispatchFromProps>(mapStateToProps, mapDispatchToProps)(SearchPage);

amundsen_application/static/js/components/SearchPage/tests/index.spec.tsx

@@ -6,10 +6,9 @@ import { shallow } from 'enzyme';
 
 import AppConfig from 'config/config';
 import { ResourceType } from 'interfaces';
-import { SearchPage, SearchPageProps, mapDispatchToProps, mapStateToProps } from '../';
+import { mapDispatchToProps, mapStateToProps, SearchPage, SearchPageProps } from '../';
 import {
   DOCUMENT_TITLE_SUFFIX,
-  PAGINATION_PAGE_RANGE,
   PAGE_INDEX_ERROR_MESSAGE,
   RESULTS_PER_PAGE,
   SEARCH_ERROR_MESSAGE_INFIX,
@@ -31,6 +30,7 @@ import LoadingSpinner from 'components/common/LoadingSpinner';
 
 import ResourceList from 'components/common/ResourceList';
 import globalState from 'fixtures/globalState';
+import { searchAll, updateSearchTab } from 'ducks/search/reducer';
 import { getMockRouterProps } from 'fixtures/mockRouter';
 
 describe('SearchPage', () => {
@@ -39,12 +39,14 @@ describe('SearchPage', () => {
     const routerProps = getMockRouterProps<any>(null, location);
     const props: SearchPageProps = {
       searchTerm: globalState.search.search_term,
+      selectedTab: ResourceType.table,
       isLoading: false,
       dashboards: globalState.search.dashboards,
       tables: globalState.search.tables,
       users: globalState.search.users,
       searchAll: jest.fn(),
       searchResource: jest.fn(),
+      updateSearchTab: jest.fn(),
       ...routerProps,
       ...propOverrides,
     };
@@ -52,23 +54,18 @@ describe('SearchPage', () => {
     return { props, wrapper };
   };
 
-  describe('constructor', () => {
-    it('sets the default selectedTab', () => {
-      const { props, wrapper } = setup();
-      expect(wrapper.state().selectedTab).toEqual(ResourceType.table);
-    });
-  });
-
   describe('componentDidMount', () => {
     let props;
     let wrapper;
-    let mockSearchOptions;
-    let mockSanitizedUrlParams;
 
-    let createSearchOptionsSpy;
-    let getSanitizedUrlParamsSpy;
-    let searchAllSpy;
+    let getGlobalStateParamsSpy;
+    let shouldUpdateFromGlobalStateSpy;
+    let shouldUpdateFromUrlParamsSpy;
+    let getUrlParamsSpy;
     let updatePageUrlSpy;
+    let searchAllSpy;
+    let mockUrlParams;
+    let mockGlobalStateParams;
 
     beforeAll(() => {
       const setupResult = setup(null, {
@@ -77,141 +74,158 @@ describe('SearchPage', () => {
       props = setupResult.props;
       wrapper = setupResult.wrapper;
 
-      mockSanitizedUrlParams = { 'term': 'testName', ' index': 1, 'currentTab': 'table' };
-      getSanitizedUrlParamsSpy = jest.spyOn(wrapper.instance(), 'getSanitizedUrlParams').mockImplementation(() => {
-        return mockSanitizedUrlParams;
+      getUrlParamsSpy = jest.spyOn(wrapper.instance(), 'getUrlParams');
+      getGlobalStateParamsSpy = jest.spyOn(wrapper.instance(), 'getGlobalStateParams');
+
+      shouldUpdateFromUrlParamsSpy = jest.spyOn(wrapper.instance(), 'shouldUpdateFromUrlParams');
+      shouldUpdateFromGlobalStateSpy = jest.spyOn(wrapper.instance(), 'shouldUpdateFromGlobalState');
+      updatePageUrlSpy = jest.spyOn(wrapper.instance(), 'updatePageUrl');
+      searchAllSpy = jest.spyOn(props, 'searchAll');
     });
-      mockSearchOptions = { 'dashboardIndex': 0, 'tableIndex': 0, 'userIndex': 1 };
-      createSearchOptionsSpy = jest.spyOn(wrapper.instance(), 'createSearchOptions').mockImplementation(() => {
-        return mockSearchOptions;
+
+    it('calls getUrlParams and getGlobalStateParams', () => {
+      wrapper.instance().componentDidMount();
+      expect(getUrlParamsSpy).toHaveBeenCalledWith(props.location.search);
+      expect(getGlobalStateParamsSpy).toHaveBeenCalled();
     });
-      searchAllSpy = jest.spyOn(props, 'searchAll');
-      updatePageUrlSpy = jest.spyOn(wrapper.instance(), 'updatePageUrl');
-      setStateSpy.mockClear();
 
+    describe('when rendering from GlobalState', () => {
+      beforeAll(() => {
+        mockUrlParams = { term: '', tab: ResourceType.table, index: 0 };
+        mockGlobalStateParams = { term: 'test', tab: ResourceType.table, index: 2 };
+        getUrlParamsSpy.mockReset().mockImplementationOnce(() => mockUrlParams);
+        getGlobalStateParamsSpy.mockReset().mockImplementationOnce(() => mockGlobalStateParams);
+
+        shouldUpdateFromGlobalStateSpy.mockReset().mockImplementationOnce(() => true);
+        shouldUpdateFromUrlParamsSpy.mockReset().mockImplementationOnce(() => false);
         wrapper.instance().componentDidMount();
       });
 
-    it('calls setState', () => {
-      expect(setStateSpy).toHaveBeenCalledWith({ selectedTab: mockSanitizedUrlParams.currentTab });
+      it('calls updatePageUrl with correct parameters', () => {
+        expect(updatePageUrlSpy).toHaveBeenCalledWith(mockGlobalStateParams.term, mockGlobalStateParams.tab, mockGlobalStateParams.index, true)
+      });
+
+      it('does not call shouldUpdateFromUrlParams', () => {
+        expect(shouldUpdateFromUrlParamsSpy).not.toHaveBeenCalled()
+      });
     });
 
-    describe('when searchTerm in params is valid', () => {
+    describe('when rendering from URL state', () => {
       beforeAll(() => {
+        mockUrlParams = { term: '', tab: ResourceType.table, index: 0 };
+        mockGlobalStateParams = { term: 'test', tab: ResourceType.table, index: 2 };
+        getUrlParamsSpy.mockReset().mockImplementationOnce(() => mockUrlParams);
+        getGlobalStateParamsSpy.mockReset().mockImplementationOnce(() => mockGlobalStateParams);
+
+        searchAllSpy.mockClear();
         updatePageUrlSpy.mockClear();
-        const {props, wrapper} = setup(null, {
-          search: '/search?searchTerm=testName&selectedTab=table&pageIndex=1',
-        });
-        updatePageUrlSpy = jest.spyOn(wrapper.instance(), 'updatePageUrl');
+        shouldUpdateFromGlobalStateSpy.mockReset().mockImplementationOnce(() => false);
+        shouldUpdateFromUrlParamsSpy.mockReset().mockImplementationOnce(() => true);
         wrapper.instance().componentDidMount();
       });
-      it('calls searchAll', () => {
-        expect(searchAllSpy).toHaveBeenCalledWith(mockSanitizedUrlParams.term, mockSearchOptions);
+
+      it('calls shouldUpdateFromGlobalState with correct params', () => {
+        expect(searchAllSpy).toHaveBeenCalledWith(mockUrlParams.term, mockUrlParams.tab, mockUrlParams.index);
       });
 
-      it('does not call updateURL', () => {
-        expect(updatePageUrlSpy).not.toHaveBeenCalled();
+      it('calls updatePageUrl with correct params', () => {
+        expect(updatePageUrlSpy).toHaveBeenCalledWith(mockUrlParams.term, mockUrlParams.tab, mockUrlParams.index, true);
+      });
     });
   });
 
-    describe('when pageIndex in params is undefined', () => {
-      let mockSanitizedUrlParams;
-      let getSanitizedUrlParamsSpy;
-
-      let updatePageUrlSpy;
+  describe('shouldUpdateFromGlobalState', () => {
+    let wrapper;
 
     beforeAll(() => {
-        const {props, wrapper} = setup(null, {
-          search: '/search?searchTerm=testName',
+      const setupResult = setup();
+      wrapper = setupResult.wrapper;
     });
-        mockSanitizedUrlParams = { 'term': 'testName', ' index': 0, 'currentTab': 'table' };
-        getSanitizedUrlParamsSpy = jest.spyOn(wrapper.instance(), 'getSanitizedUrlParams').mockImplementation(() => {
-          return mockSanitizedUrlParams;
+
+    describe('when `urlParams.term` is empty and `globalState.term` is initialized', () => {
+      it('returns a value of true', () => {
+        const mockUrlParams = { term: '', tab: ResourceType.table, index: 0 };
+        const mockGlobalStateParams = { term: 'test', tab: ResourceType.table, index: 2 };
+        expect(wrapper.instance().shouldUpdateFromGlobalState(mockUrlParams, mockGlobalStateParams)).toBe(true);
       });
-        updatePageUrlSpy = jest.spyOn(wrapper.instance(), 'updatePageUrl');
-        wrapper.instance().componentDidMount();
     });
-      it('uses 0 as pageIndex', () => {
-        expect(updatePageUrlSpy).toHaveBeenCalledWith(mockSanitizedUrlParams.term, mockSanitizedUrlParams.currentTab, mockSanitizedUrlParams.index);
+
+    describe('when `urlParams.term` is initialized', () => {
+      it('returns a value of false', () => {
+        const mockUrlParams = { term: 'testTerm', tab: ResourceType.table, index: 0 };
+        const mockGlobalStateParams = { term: '', tab: ResourceType.table, index: 2 };
+        expect(wrapper.instance().shouldUpdateFromGlobalState(mockUrlParams, mockGlobalStateParams)).toBe(false);
       });
     });
 
-    describe('when searchTerm in params is undefined', () => {
-      beforeAll(() => {
-        searchAllSpy.mockClear();
-        updatePageUrlSpy.mockClear();
-        const {props, wrapper} = setup(null, {
-          search: '/search?selectedTab=table&pageIndex=1',
+    describe('when `globalState.term` is empty', () => {
+      it('returns a value of false', () => {
+        const mockUrlParams = { term: '', tab: ResourceType.table, index: 0 };
+        const mockGlobalStateParams = { term: '', tab: ResourceType.table, index: 0 };
+        expect(wrapper.instance().shouldUpdateFromGlobalState(mockUrlParams, mockGlobalStateParams)).toBe(false);
       });
-        updatePageUrlSpy = jest.spyOn(wrapper.instance(), 'updatePageUrl');
-        wrapper.instance().componentDidMount();
     });
-      it('does not call searchAll', () => {
-        expect(searchAllSpy).not.toHaveBeenCalled();
   });
 
-      it('does not call updatePageUrl', () => {
-        expect(updatePageUrlSpy).not.toHaveBeenCalled();
-      });
-    });
+  describe('shouldUpdateFromUrlParams', () => {
+    let wrapper;
 
-    describe('when searchTerm in params is empty string', () => {
     beforeAll(() => {
-        searchAllSpy.mockClear();
-        updatePageUrlSpy.mockClear();
-        const {props, wrapper} = setup(null, {
-          search: '/search?searchTerm=&selectedTab=table&pageIndex=1',
+      const setupResult = setup();
+      wrapper = setupResult.wrapper;
     });
-        updatePageUrlSpy = jest.spyOn(wrapper.instance(), 'updatePageUrl');
-        wrapper.instance().componentDidMount();
+
+    describe('when urlParams.term is empty', () => {
+      it('returns a value of false', () => {
+
+        const mockUrlParams = { term: '', tab: ResourceType.table, index: 0 };
+        const mockGlobalStateParams = { term: '', tab: ResourceType.table, index: 0 };
+        expect(wrapper.instance().shouldUpdateFromUrlParams(mockUrlParams, mockGlobalStateParams)).toBe(false);
       });
-      it('does not call searchAll', () => {
-        expect(searchAllSpy).not.toHaveBeenCalled();
     });
 
-      it('does not call updatePageUrl', () => {
-        expect(updatePageUrlSpy).not.toHaveBeenCalled();
+    describe('when urlParams.term is initialized and equals globalState.term', () => {
+      it('returns a value of false', () => {
+        const mockUrlParams = { term: 'test', tab: ResourceType.table, index: 0 };
+        const mockGlobalStateParams = { term: 'test', tab: ResourceType.table, index: 0 };
+        expect(wrapper.instance().shouldUpdateFromUrlParams(mockUrlParams, mockGlobalStateParams)).toBe(false);
       });
     });
 
-    afterAll(() => {
-      createSearchOptionsSpy.mockRestore();
+    describe('when urlParams are initialized and not equal to global state', () => {
+      it('returns a value of true', () => {
+        const mockUrlParams = { term: 'test', tab: ResourceType.table, index: 0 };
+        const mockGlobalStateParams = { term: '', tab: ResourceType.table, index: 0 };
+        expect(wrapper.instance().shouldUpdateFromUrlParams(mockUrlParams, mockGlobalStateParams)).toBe(true);
+      });
     });
   });
   
   describe('componentDidUpdate', () => {
-    let searchAllSpy;
+    let props;
+    let wrapper;
 
-    let mockSearchOptions;
-    let mockSanitizedUrlParams;
+    let mockNextUrlParams;
+    let mockPrevUrlParams;
+    let mockPrevProps;
 
-    let createSearchOptionsSpy;
-    let getSanitizedUrlParamsSpy;
+    let getUrlParamsSpy;
+    let isUrlStateSyncedSpy;
+    let shouldUpdateSearchTermSpy;
+    let shouldUpdateTabSpy;
+    let shouldUpdatePageIndexSpy;
+    let searchAllSpy;
+    let updateSearchTabSpy;
+    let searchResourceSpy;
 
-    let props;
-    let wrapper;
     beforeAll(() => {
       const setupResult = setup(null, {
-        search: '/search?searchTerm=current&selectedTab=table&pageIndex=0',
+        search: '/search?searchTerm=testName&selectedTab=table&pageIndex=1',
       });
       props = setupResult.props;
       wrapper = setupResult.wrapper;
 
-      mockSanitizedUrlParams = { 'term': 'current', ' index': 0, 'currentTab': 'table' };
-      getSanitizedUrlParamsSpy = jest.spyOn(wrapper.instance(), 'getSanitizedUrlParams').mockImplementation(() => {
-        return mockSanitizedUrlParams;
-      });
-
-      mockSearchOptions = { 'dashboardIndex': 0, 'tableIndex': 1, 'userIndex': 0 };
-      createSearchOptionsSpy = jest.spyOn(wrapper.instance(), 'createSearchOptions').mockImplementation(() => {
-        return mockSearchOptions;
-      });
-
-      searchAllSpy = jest.spyOn(props, 'searchAll');
-
-      setStateSpy.mockClear();
-
-      const mockPrevProps = {
+      mockPrevProps = {
         searchTerm: 'previous',
         location: {
           search: '/search?searchTerm=previous&selectedTab=table&pageIndex=0',
@@ -220,132 +234,252 @@ describe('SearchPage', () => {
           hash: 'mockstr',
         }
       };
+
+      getUrlParamsSpy = jest.spyOn(wrapper.instance(), 'getUrlParams');
+      isUrlStateSyncedSpy = jest.spyOn(wrapper.instance(), 'isUrlStateSynced');
+      shouldUpdateSearchTermSpy = jest.spyOn(wrapper.instance(), 'shouldUpdateSearchTerm');
+      shouldUpdateTabSpy = jest.spyOn(wrapper.instance(), 'shouldUpdateTab');
+      shouldUpdatePageIndexSpy = jest.spyOn(wrapper.instance(), 'shouldUpdatePageIndex');
+      searchAllSpy = jest.spyOn(props, 'searchAll');
+      updateSearchTabSpy = jest.spyOn(props, 'updateSearchTab');
+      searchResourceSpy = jest.spyOn(props, 'searchResource');
+    });
+
+    it('calls getUrlParams for both current and prev props', () => {
+      wrapper.instance().componentDidUpdate(mockPrevProps);
+      expect(getUrlParamsSpy).toHaveBeenCalledTimes(2);
+      expect(getUrlParamsSpy).toHaveBeenCalledWith(mockPrevProps.location.search);
+      expect(getUrlParamsSpy).toHaveBeenCalledWith(props.location.search);
+    });
+
+
+    it('exits the function when isUrlStateSynced returns true', () => {
+      isUrlStateSyncedSpy.mockImplementationOnce(() => true);
+
+      shouldUpdateSearchTermSpy.mockClear();
+      shouldUpdateTabSpy.mockClear();
+      wrapper.instance().componentDidUpdate(mockPrevProps);
+
+      expect(shouldUpdateSearchTermSpy).not.toHaveBeenCalled();
+      expect(shouldUpdateTabSpy).not.toHaveBeenCalled();
+    });
+
+
+    describe('when the search term has changed', () => {
+      beforeAll(() => {
+        mockNextUrlParams = { term: 'new term', tab: ResourceType.table, index: 0 };
+        mockPrevUrlParams = { term: 'old term', tab: ResourceType.table, index: 0 };
+
+        getUrlParamsSpy.mockReset()
+          .mockImplementationOnce(() => mockNextUrlParams)
+          .mockImplementationOnce(() => mockPrevUrlParams);
+        isUrlStateSyncedSpy.mockClear().mockImplementationOnce(() => false);
+
+        shouldUpdateSearchTermSpy.mockReset().mockImplementationOnce(() => true);
+        shouldUpdateTabSpy.mockReset().mockImplementationOnce(() => false);
+        shouldUpdatePageIndexSpy.mockReset().mockImplementationOnce(() => false);
+
+        searchAllSpy.mockClear();
+        updateSearchTabSpy.mockClear();
+        searchResourceSpy.mockClear();
+
         wrapper.instance().componentDidUpdate(mockPrevProps);
       });
 
-    it('calls setState', () => {
-      expect(setStateSpy).toHaveBeenCalledWith({ selectedTab: ResourceType.table });
+      it('calls searchAll', () => {
+        expect(searchAllSpy).toHaveBeenCalledWith(mockNextUrlParams.term, mockNextUrlParams.tab, mockNextUrlParams.index);
       });
 
-    it('calls searchAll if called with a new search term', () => {
-      expect(searchAllSpy).toHaveBeenCalledWith(mockSanitizedUrlParams.term, mockSearchOptions);
+      it('does not call updateSearchTab nor searchResource', () => {
+        expect(updateSearchTabSpy).not.toHaveBeenCalled();
+        expect(searchResourceSpy).not.toHaveBeenCalled();
+      });
     });
 
-    it('does not call searchAll if called with the same search term with a new page', () => {
+    describe('when the search tab has changed', () => {
+      beforeAll(() => {
+        mockNextUrlParams = { term: 'old term', tab: ResourceType.user, index: 0 };
+        mockPrevUrlParams = { term: 'old term', tab: ResourceType.table, index: 0 };
+
+        getUrlParamsSpy.mockReset()
+          .mockImplementationOnce(() => mockNextUrlParams)
+          .mockImplementationOnce(() => mockPrevUrlParams);
+        isUrlStateSyncedSpy.mockClear().mockImplementationOnce(() => false);
+
+        shouldUpdateSearchTermSpy.mockReset().mockImplementationOnce(() => false);
+        shouldUpdateTabSpy.mockReset().mockImplementationOnce(() => true);
+        shouldUpdatePageIndexSpy.mockReset().mockImplementationOnce(() => false);
+
         searchAllSpy.mockClear();
-      const mockPrevProps = {
-        searchTerm: 'current',
-        location: {
-          search: '/search?searchTerm=current&current=table&pageIndex=1',
-          pathname: 'mockstr',
-          state: jest.fn(),
-          hash: 'mockstr',
-        }
-      };
+        updateSearchTabSpy.mockClear();
+        searchResourceSpy.mockClear();
         wrapper.instance().componentDidUpdate(mockPrevProps);
+      });
+
+      it('calls updateSearchTab', () => {
+        expect(updateSearchTabSpy).toHaveBeenCalledWith(mockNextUrlParams.tab)
+      });
+
+      it('does not call searchAll nor searchResource', () => {
         expect(searchAllSpy).not.toHaveBeenCalled();
+        expect(searchResourceSpy).not.toHaveBeenCalled();
       });
     });
 
-  describe('getSanitizedUrlParams', () => {
-    let props;
-    let wrapper;
-    let getSelectedTabByResourceTypeSpy;
+    describe('when the page index has changed', () => {
+      beforeAll(() => {
+        mockNextUrlParams = { term: 'old term', tab: ResourceType.table, index: 1 };
+        mockPrevUrlParams = { term: 'old term', tab: ResourceType.table, index: 0 };
 
-    let mockSelectedTab;
+        getUrlParamsSpy.mockReset()
+          .mockImplementationOnce(() => mockNextUrlParams)
+          .mockImplementationOnce(() => mockPrevUrlParams);
+        isUrlStateSyncedSpy.mockClear().mockImplementationOnce(() => false);
 
-    beforeAll(() => {
-      const setupResult = setup(null, {
-        search: '/search?searchTerm=current&selectedTab=table&pageIndex=0',
+        shouldUpdateSearchTermSpy.mockReset().mockImplementationOnce(() => false);
+        shouldUpdateTabSpy.mockReset().mockImplementationOnce(() => false);
+        shouldUpdatePageIndexSpy.mockReset().mockImplementationOnce(() => true);
+
+        searchAllSpy.mockClear();
+        updateSearchTabSpy.mockClear();
+        searchResourceSpy.mockClear();
+        wrapper.instance().componentDidUpdate(mockPrevProps);
+      });
+
+      it('calls searchResource', () => {
+        expect(searchResourceSpy).toHaveBeenCalledWith(mockNextUrlParams.term, mockNextUrlParams.tab, mockNextUrlParams.index)
+      });
+
+      it('does not call searchAll nor updateSearchTab', () => {
+        expect(searchAllSpy).not.toHaveBeenCalled();
+        expect(updateSearchTabSpy).not.toHaveBeenCalled();
+      });
     });
-      props = setupResult.props;
-      wrapper = setupResult.wrapper;
-      const mockResourceType = ResourceType.table;
-      getSelectedTabByResourceTypeSpy = jest.spyOn(wrapper.instance(), 'getSelectedTabByResourceType').mockImplementation(() => {
-        return mockResourceType;
   });
 
-      mockSelectedTab = ResourceType.table;
+  describe('shouldUpdateSearchTerm', () => {
+    const wrapper = setup().wrapper;
 
-      wrapper.instance().getSanitizedUrlParams('current', 0, mockSelectedTab)
+    it('returns true when the search term is different', () => {
+      const nextUrlParams = { term: 'new term', tab: ResourceType.table, index: 0 };
+      const prevUrlParams = { term: 'old term', tab: ResourceType.table, index: 0 };
+      expect(wrapper.instance().shouldUpdateSearchTerm(nextUrlParams, prevUrlParams)).toBe(true)
     });
 
-    it('calls getSelectedTabByResourceType with correct value', () => {
-      expect(getSelectedTabByResourceTypeSpy).toHaveBeenCalledWith(mockSelectedTab);
+    it('returns false when the search term is the same', () => {
+      const nextUrlParams = { term: 'old term', tab: ResourceType.table, index: 0 };
+      const prevUrlParams = { term: 'old term', tab: ResourceType.table, index: 0 };
+      expect(wrapper.instance().shouldUpdateSearchTerm(nextUrlParams, prevUrlParams)).toBe(false)
+    });
   });
 
-    it('output of getSanitizedUrlParams is expected', () => {
-      const expected = {'term': 'current', 'index': 0, 'currentTab': ResourceType.table};
-      expect(wrapper.instance().getSanitizedUrlParams('current', 0, ResourceType.table)).toEqual(expected);
+  describe('shouldUpdateTab', () => {
+    const wrapper = setup().wrapper;
+
+    it('returns true when the tab is different', () => {
+      const nextUrlParams = { term: 'old term', tab: ResourceType.user, index: 0 };
+      const prevUrlParams = { term: 'old term', tab: ResourceType.table, index: 0 };
+      expect(wrapper.instance().shouldUpdateTab(nextUrlParams, prevUrlParams)).toBe(true)
     });
 
-    it('output of getSanitizedUrlParams is expected for undefined vars', () => {
-      const expected = {'term': '', 'index': 0, 'currentTab': ResourceType.table};
-      expect(wrapper.instance().getSanitizedUrlParams(undefined, undefined, ResourceType.table)).toEqual(expected);
+    it('returns false when the tab is the same', () => {
+      const nextUrlParams = { term: 'old term', tab: ResourceType.table, index: 0 };
+      const prevUrlParams = { term: 'old term', tab: ResourceType.table, index: 0 };
+      expect(wrapper.instance().shouldUpdateTab(nextUrlParams, prevUrlParams)).toBe(false)
     });
   });
 
-  describe('getSelectedTabByResourceType', () => {
+  describe('shouldUpdatePageIndex', () => {
+    const wrapper = setup().wrapper;
+
+    it('returns true when the pageIndex is different', () => {
+      const nextUrlParams = { term: 'old term', tab: ResourceType.table, index: 1 };
+      const prevUrlParams = { term: 'old term', tab: ResourceType.table, index: 0 };
+      expect(wrapper.instance().shouldUpdatePageIndex(nextUrlParams, prevUrlParams)).toBe(true)
+    });
+
+    it('returns false when the pageIndex is the same', () => {
+      const nextUrlParams = { term: 'old term', tab: ResourceType.table, index: 0 };
+      const prevUrlParams = { term: 'old term', tab: ResourceType.table, index: 0 };
+      expect(wrapper.instance().shouldUpdatePageIndex(nextUrlParams, prevUrlParams)).toBe(false)
+    });
+  });
+
+  describe('getUrlParams', () => {
     let props;
     let wrapper;
 
+    let urlString;
+    let urlParams;
+
+    let getSelectedTabByResourceTypeSpy;
+
     beforeAll(() => {
-      const setupResult = setup();
+      const setupResult = setup(null, {
+        search: '/search?searchTerm=current&selectedTab=table&pageIndex=0',
+      });
       props = setupResult.props;
       wrapper = setupResult.wrapper;
+
+      getSelectedTabByResourceTypeSpy = jest.spyOn(wrapper.instance(), 'getSelectedTabByResourceType')
+        .mockImplementation((selectedTab) => selectedTab);
     });
 
-    it('returns given tab if equal to ResourceType.table', () => {
-      expect(wrapper.instance().getSelectedTabByResourceType(ResourceType.table)).toEqual(ResourceType.table);
+    it('parses url params correctly', () => {
+      urlString = '/search?searchTerm=tableSearch&selectedTab=table&pageIndex=3';
+      urlParams = wrapper.instance().getUrlParams(urlString);
+
+      expect(getSelectedTabByResourceTypeSpy).toHaveBeenLastCalledWith('table');
+      expect(urlParams.term).toEqual('tableSearch');
+      expect(urlParams.tab).toEqual('table');
+      expect(urlParams.index).toEqual(3);
     });
 
-    it('returns given tab if equal to ResourceType.user', () => {
-      expect(wrapper.instance().getSelectedTabByResourceType(ResourceType.user)).toEqual(ResourceType.user);
+    it('trims empty spaces from searchTerm', () => {
+      urlString = '/search?searchTerm= term%20&selectedTab=user&pageIndex=0';
+      urlParams = wrapper.instance().getUrlParams(urlString);
+
+      expect(urlParams.term).toEqual('term');
     });
 
-    it('returns state.selectedTab if given equal to ResourceType.dashboard', () => {
-      wrapper.setState({ selectedTab: 'user' })
-      expect(wrapper.instance().getSelectedTabByResourceType(ResourceType.dashboard)).toEqual('user');
+    it('defaults NaN pageIndex as 0', () => {
+      urlString = '/search?searchTerm=current&selectedTab=table&pageIndex=NotANumber';
+      urlParams = wrapper.instance().getUrlParams(urlString);
+
+      expect(urlParams.index).toEqual(0);
     });
 
-    it('returns state.selectedTab in default case', () => {
-      wrapper.setState({ selectedTab: 'table' })
-      // @ts-ignore: cover default case
-      expect(wrapper.instance().getSelectedTabByResourceType('not valid')).toEqual('table');
+    it('defaults invalid tabs to the current tab', () => {
+      getSelectedTabByResourceTypeSpy.mockRestore();
+      getSelectedTabByResourceTypeSpy = jest.spyOn(wrapper.instance(), 'getSelectedTabByResourceType');
+      urlString = '/search?searchTerm=current&selectedTab=invalidTabType&pageIndex=0';
+      urlParams = wrapper.instance().getUrlParams(urlString);
+
+      expect(getSelectedTabByResourceTypeSpy).toHaveBeenLastCalledWith('invalidTabType');
+      expect(urlParams.tab).toEqual(props.selectedTab);
     });
   });
 
-  describe('createSearchOptions', () => {
-    let props;
+  describe('getSelectedTabByResourceType', () => {
     let wrapper;
+
     beforeAll(() => {
       const setupResult = setup();
-      props = setupResult.props;
       wrapper = setupResult.wrapper;
     });
 
-    it('generates correct options if selectedTab === ResourceType.dashboard', () => {
-      expect(wrapper.instance().createSearchOptions(5, ResourceType.dashboard)).toMatchObject({
-        dashboardIndex: 5,
-        userIndex: 0,
-        tableIndex: 0,
-      });
+    it('returns given tab if equal to ResourceType.table', () => {
+      expect(wrapper.instance().getSelectedTabByResourceType(ResourceType.table)).toEqual(ResourceType.table);
     });
 
-    it('generates correct options if selectedTab === ResourceType.user', () => {
-      expect(wrapper.instance().createSearchOptions(5, ResourceType.user)).toMatchObject({
-        dashboardIndex: 0,
-        userIndex: 5,
-        tableIndex: 0,
-      });
+    it('returns given tab if equal to ResourceType.user', () => {
+      expect(wrapper.instance().getSelectedTabByResourceType(ResourceType.user)).toEqual(ResourceType.user);
     });
 
-    it('generates correct options if selectedTab === ResourceType.table', () => {
-      expect(wrapper.instance().createSearchOptions(5, ResourceType.table)).toMatchObject({
-        dashboardIndex: 0,
-        userIndex: 0,
-        tableIndex: 5,
-      });
+    it('returns state.selectedTab in default case', () => {
+      wrapper.setState({ selectedTab: 'table' })
+      // @ts-ignore: cover default case
+      expect(wrapper.instance().getSelectedTabByResourceType('not valid')).toEqual('table');
     });
   });
 
@@ -398,12 +532,8 @@ describe('SearchPage', () => {
       wrapper.instance().onPaginationChange(testIndex);
     });
 
-    it('calls props.searchResource with correct parameters', () => {
-      expect(searchResourceSpy).toHaveBeenCalledWith(wrapper.state().selectedTab, props.searchTerm, testIndex);
-    });
-
     it('calls updatePageUrl with correct parameters', () => {
-      expect(updatePageUrlSpy).toHaveBeenCalledWith(props.searchTerm, wrapper.state().selectedTab, testIndex);
+      expect(updatePageUrlSpy).toHaveBeenCalledWith(props.searchTerm, props.selectedTab, testIndex);
     });
   });
 
@@ -437,10 +567,6 @@ describe('SearchPage', () => {
       expect(getSelectedTabByResourceTypeSpy).toHaveBeenCalledWith(givenTab);
     });
 
-    it('calls setState with correct parameters', () => {
-      expect(setStateSpy).toHaveBeenCalledWith({ selectedTab: givenTab });
-    });
-
     it('calls updatePageUrl with correct parameters', () => {
       expect(updatePageUrlSpy).toHaveBeenCalledWith(props.searchTerm, givenTab, mockPageIndex);
     });
@@ -452,15 +578,39 @@ describe('SearchPage', () => {
   });
 
   describe('updatePageUrl', () => {
-    it('pushes correct update to the window state', () => {
-      const { props, wrapper } = setup();
+    let props;
+    let wrapper;
+    let historyPushSpy;
+    let historyReplaceSpy;
     const pageIndex = 2;
     const searchTerm = 'testing';
     const tab = ResourceType.user;
     const expectedPath = `/search?searchTerm=${searchTerm}&selectedTab=${tab}&pageIndex=${pageIndex}`;
-      const historyPushSpy = jest.spyOn(props.history, 'push');
+
+    beforeAll(() => {
+      const setupResult = setup();
+      props = setupResult.props;
+      wrapper = setupResult.wrapper;
+      historyPushSpy = jest.spyOn(props.history, 'push');
+      historyReplaceSpy = jest.spyOn(props.history, 'replace');
+    });
+
+    it('pushes correct update to the window state', () => {
+      historyPushSpy.mockClear();
+      historyReplaceSpy.mockClear();
+
       wrapper.instance().updatePageUrl(searchTerm, tab, pageIndex);
       expect(historyPushSpy).toHaveBeenCalledWith(expectedPath);
+      expect(historyReplaceSpy).not.toHaveBeenCalled();
+    });
+
+    it('calls `history.replace` when replace is set to true', () => {
+      historyPushSpy.mockClear();
+      historyReplaceSpy.mockClear();
+
+      wrapper.instance().updatePageUrl(searchTerm, tab, pageIndex, true);
+      expect(historyPushSpy).not.toHaveBeenCalled();
+      expect(historyReplaceSpy).toHaveBeenCalledWith(expectedPath);
     });
   });
 
@@ -603,7 +753,7 @@ describe('SearchPage', () => {
       AppConfig.indexUsers.enabled = false;
       const content = shallow(wrapper.instance().renderSearchResults());
       const tabProps = content.find(TabsComponent).props();
-      expect(tabProps.activeKey).toEqual(wrapper.state().selectedTab);
+      expect(tabProps.activeKey).toEqual(props.selectedTab);
       expect(tabProps.defaultTab).toEqual(ResourceType.table);
       expect(tabProps.onSelect).toEqual(wrapper.instance().onTabChange);
 

amundsen_application/static/js/components/common/Breadcrumb/index.tsx

@@ -23,7 +23,7 @@ export const Breadcrumb: React.SFC<BreadcrumbProps> = (props) => {
     path = '/';
     text = 'Home';
     if (props.searchTerm) {
-      path = `/search?searchTerm=${props.searchTerm}&selectedTab=table&pageIndex=0`;
+      path = `/search`;
       text = 'Search Results';
     }
   }

amundsen_application/static/js/components/common/Breadcrumb/tests/index.spec.tsx

@@ -40,7 +40,7 @@ describe('Breadcrumb', () => {
 
     it('renders Link with correct path', () => {
       expect(subject.find(Link).props()).toMatchObject({
-        to: '/search?searchTerm=testTerm&selectedTab=table&pageIndex=0',
+        to: '/search',
       });
     });
 

amundsen_application/static/js/ducks/search/api/tests/index.spec.ts

@@ -6,7 +6,7 @@ import { DashboardSearchResults, TableSearchResults, UserSearchResults } from 'd
 
 import globalState from 'fixtures/globalState';
 
-import { ResourceType, SearchAllOptions } from 'interfaces';
+import { ResourceType } from 'interfaces';
 
 import * as API from '../v0';
 

amundsen_application/static/js/ducks/search/reducer.ts

-import { ResourceType, SearchAllOptions } from 'interfaces';
+import { ResourceType } from 'interfaces';
 
 import {
-  SearchResponsePayload,
+  DashboardSearchResults,
   SearchAll,
   SearchAllRequest,
   SearchAllReset,
   SearchAllResponse,
+  SearchAllResponsePayload,
   SearchResource,
+  SearchResponsePayload,
   SearchResourceRequest,
   SearchResourceResponse,
-  DashboardSearchResults,
   TableSearchResults,
+  UpdateSearchTab,
+  UpdateSearchTabRequest,
   UserSearchResults,
 } from './types';
 
 export interface SearchReducerState {
   search_term: string;
+  selectedTab: ResourceType;
   isLoading: boolean;
   dashboards: DashboardSearchResults;
   tables: TableSearchResults;
@@ -23,23 +27,24 @@ export interface SearchReducerState {
 };
 
 /* ACTIONS */
-export function searchAll(term: string, options: SearchAllOptions): SearchAllRequest {
+export function searchAll(term: string, resource: ResourceType, pageIndex: number): SearchAllRequest {
   return {
     payload: {
-      options,
+      resource,
+      pageIndex,
       term,
     },
     type: SearchAll.REQUEST,
   };
 };
-export function searchAllSuccess(searchResults: SearchResponsePayload): SearchAllResponse {
+export function searchAllSuccess(searchResults: SearchAllResponsePayload): SearchAllResponse {
   return { type: SearchAll.SUCCESS, payload: searchResults };
 };
 export function searchAllFailure(): SearchAllResponse {
   return { type: SearchAll.FAILURE };
 };
 
-export function searchResource(resource: ResourceType, term: string, pageIndex: number): SearchResourceRequest {
+export function searchResource(term: string, resource: ResourceType, pageIndex: number): SearchResourceRequest {
   return {
     payload: {
       pageIndex,
@@ -62,10 +67,19 @@ export function searchReset(): SearchAllReset {
   };
 };
 
+export function updateSearchTab(selectedTab: ResourceType): UpdateSearchTabRequest {
+  return {
+    payload: { selectedTab },
+    type: UpdateSearchTab.REQUEST,
+  };
+}
+
+
 /* REDUCER */
 export const initialState: SearchReducerState = {
   search_term: '',
   isLoading: false,
+  selectedTab: ResourceType.table,
   dashboards: {
     page_index: 0,
     results: [],
@@ -121,6 +135,11 @@ export default function reducer(state: SearchReducerState = initialState, action
         ...initialState,  
         search_term: state.search_term,
       };
+    case UpdateSearchTab.REQUEST:
+      return {
+        ...state,
+        selectedTab: (<UpdateSearchTabRequest>action).payload.selectedTab
+      };
     default:
       return state;
   };

amundsen_application/static/js/ducks/search/sagas.ts

@@ -13,25 +13,30 @@ import {
 } from './types';
 
 import {
-  searchAllSuccess, searchAllFailure,
+  initialState, searchAllSuccess, searchAllFailure,
   searchResourceSuccess, searchResourceFailure,
 } from './reducer';
 
 export function* searchAllWorker(action: SearchAllRequest): SagaIterator {
-  const { options, term } = action.payload;
+  const { resource, pageIndex, term } = action.payload;
+  const tableIndex = resource === ResourceType.table ? pageIndex : 0;
+  const userIndex = resource === ResourceType.user ? pageIndex : 0;
+  const dashboardIndex = resource === ResourceType.dashboard ? pageIndex : 0;
+
   try {
     const [tableResponse, userResponse, dashboardResponse] = yield all([
-      call(API.searchResource, options.tableIndex, ResourceType.table, term),
-      call(API.searchResource, options.userIndex, ResourceType.user, term),
-      call(API.searchResource, options.dashboardIndex, ResourceType.dashboard, term),
+      call(API.searchResource, tableIndex, ResourceType.table, term),
+      call(API.searchResource, userIndex, ResourceType.user, term),
+      call(API.searchResource, dashboardIndex, ResourceType.dashboard, term),
     ]);
     const searchAllResponse = {
       search_term: term,
-      tables: tableResponse.tables,
-      users: userResponse.users,
-      dashboards: dashboardResponse.dashboards,
+      selectedTab: resource,
+      tables: tableResponse.tables || initialState.tables,
+      users: userResponse.users || initialState.users,
+      dashboards: dashboardResponse.dashboards || initialState.dashboards,
     };
-    yield put({ type: SearchAll.SUCCESS, payload: searchAllResponse });
+    yield put(searchAllSuccess(searchAllResponse));
   } catch (e) {
     yield put(searchAllFailure());
   }

amundsen_application/static/js/ducks/search/tests/index.spec.ts

@@ -2,48 +2,92 @@ import { testSaga } from 'redux-saga-test-plan';
 
 import { ResourceType } from 'interfaces';
 
-import globalState from 'fixtures/globalState';
-
 import * as API from '../api/v0';
 
 import reducer, {
-  searchAll, searchAllSuccess, searchAllFailure,
-  searchResource, searchResourceSuccess, searchResourceFailure,
+  initialState,
+  searchAll,
+  searchAllFailure,
+  searchAllSuccess,
+  SearchReducerState,
   searchReset,
-  initialState, SearchReducerState,
+  searchResource,
+  searchResourceFailure,
+  searchResourceSuccess,
+  updateSearchTab,
 } from '../reducer';
-import {
-  searchAllWatcher, searchAllWorker,
-  searchResourceWatcher, searchResourceWorker
-} from '../sagas';
-import {
-  SearchAll, SearchAllRequest, SearchAllResponse,
-  SearchResource, SearchResourceRequest, SearchResourceResponse,
-  SearchResponsePayload,
-} from '../types';
+import { searchAllWatcher, searchAllWorker, searchResourceWatcher, searchResourceWorker } from '../sagas';
+import { SearchAll, SearchAllResponsePayload, SearchResource, SearchResponsePayload, UpdateSearchTab, } from '../types';
 
 describe('search ducks', () => {
-  let expectedSearchResults: SearchResponsePayload;
-  beforeAll(() => {
-    expectedSearchResults = globalState.search;
-  });
+  const expectedSearchResults: SearchResponsePayload = {
+    search_term: 'testName',
+    tables: {
+      page_index: 0,
+      results: [
+        {
+          cluster: 'testCluster',
+          database: 'testDatabase',
+          description: 'I have a lot of users',
+          key: 'testDatabase://testCluster.testSchema/testName',
+          last_updated_epoch: 946684799,
+          name: 'testName',
+          schema_name: 'testSchema',
+          type: ResourceType.table,
+        },
+      ],
+      total_results: 1,
+    },
+  };
+  const expectedSearchAllResults: SearchAllResponsePayload = {
+    search_term: 'testName',
+    selectedTab: ResourceType.table,
+    dashboards: {
+      page_index: 0,
+      results: [],
+      total_results: 0,
+    },
+    tables: {
+      page_index: 0,
+      results: [
+        {
+          cluster: 'testCluster',
+          database: 'testDatabase',
+          description: 'I have a lot of users',
+          key: 'testDatabase://testCluster.testSchema/testName',
+          last_updated_epoch: 946684799,
+          name: 'testName',
+          schema_name: 'testSchema',
+          type: ResourceType.table,
+        },
+      ],
+      total_results: 1,
+    },
+    users: {
+      page_index: 0,
+      results: [],
+      total_results: 0,
+    },
+  };
 
   describe('actions', () => {
     it('searchAll - returns the action to search all resources', () => {
       const term = 'test';
-      const options = {};
-      const action = searchAll(term, options);
+      const resource = ResourceType.table;
+      const pageIndex = 0;
+      const action = searchAll(term, resource, pageIndex);
       const { payload } = action;
       expect(action.type).toBe(SearchAll.REQUEST);
-      expect(payload.options).toBe(options);
+      expect(payload.resource).toBe(resource);
       expect(payload.term).toBe(term);
+      expect(payload.pageIndex).toBe(pageIndex);
     });
 
     it('searchAllSuccess - returns the action to process the success', () => {
-      const action = searchAllSuccess(expectedSearchResults);
+      const action = searchAllSuccess(expectedSearchAllResults);
       const { payload } = action;
       expect(action.type).toBe(SearchAll.SUCCESS);
-      expect(payload).toBe(expectedSearchResults);
+      expect(payload).toBe(expectedSearchAllResults);
     });
 
     it('searchAllFailure - returns the action to process the failure', () => {
@@ -52,10 +96,10 @@ describe('search ducks', () => {
     });
 
     it('searchResource - returns the action to search all resources', () => {
-      const pageIndex = 0;
-      const resource = ResourceType.table;
       const term = 'test';
-      const action = searchResource(resource, term, pageIndex);
+      const resource = ResourceType.table;
+      const pageIndex = 0;
+      const action = searchResource(term, resource, pageIndex);
       const { payload } = action;
       expect(action.type).toBe(SearchResource.REQUEST);
       expect(payload.resource).toBe(resource);
@@ -79,6 +123,14 @@ describe('search ducks', () => {
       const action = searchReset();
       expect(action.type).toBe(SearchAll.RESET);
     });
+
+    it('updateSearchTab - returns the action to update the search tab', () => {
+      const selectedTab = ResourceType.user;
+      const action = updateSearchTab(selectedTab);
+      const payload = action.payload;
+      expect(action.type).toBe(UpdateSearchTab.REQUEST);
+      expect(payload.selectedTab).toBe(selectedTab);
+    });
   });
 
   describe('reducer', () => {
@@ -92,8 +144,9 @@ describe('search ducks', () => {
 
     it('should handle SearchAll.REQUEST', () => {
       const term = 'testSearch';
-      const options = {};
-      expect(reducer(testState, searchAll(term, options))).toEqual({
+      const resource = ResourceType.table;
+      const pageIndex = 0;
+      expect(reducer(testState, searchAll(term, resource, pageIndex))).toEqual({
         ...testState,
         search_term: term,
         isLoading: true,
@@ -101,7 +154,7 @@ describe('search ducks', () => {
     });
 
     it('should handle SearchAll.SUCCESS', () => {
-      expect(reducer(testState, searchAllSuccess(expectedSearchResults))).toEqual({
+      expect(reducer(testState, searchAllSuccess(expectedSearchAllResults))).toEqual({
         ...initialState,
         ...expectedSearchResults,
         isLoading: false,
@@ -120,7 +173,7 @@ describe('search ducks', () => {
     });
 
     it('should handle SearchResource.REQUEST', () => {
-      expect(reducer(testState, searchResource(ResourceType.table, 'test', 0))).toEqual({
+      expect(reducer(testState, searchResource('test', ResourceType.table, 0))).toEqual({
         ...initialState,
         isLoading: true,
       });
@@ -140,6 +193,14 @@ describe('search ducks', () => {
         search_term: testState.search_term,
       });
     });
+
+    it('should handle UpdateSearchTab.REQUEST', () => {
+      const selectedTab = ResourceType.user;
+      expect(reducer(testState, updateSearchTab(selectedTab))).toEqual({
+        ...testState,
+        selectedTab,
+      });
+    });
   });
 
   describe('sagas', () => {
@@ -166,7 +227,7 @@ describe('search ducks', () => {
       });*/
 
       it('handles request error', () => {
-        testSaga(searchAllWorker, searchAll('test', {}))
+        testSaga(searchAllWorker, searchAll('test', ResourceType.table, 0))
           .next().throw(new Error()).put(searchAllFailure())
           .next().isDone();
       });
@@ -185,14 +246,14 @@ describe('search ducks', () => {
         const pageIndex = 0;
         const resource = ResourceType.table;
         const term = 'test';
-        testSaga(searchResourceWorker, searchResource(resource, term, pageIndex))
+        testSaga(searchResourceWorker, searchResource(term, resource, pageIndex))
           .next().call(API.searchResource, pageIndex, resource, term)
           .next(expectedSearchResults).put(searchResourceSuccess(expectedSearchResults))
           .next().isDone();
       });
 
       it('handles request error', () => {
-        testSaga(searchResourceWorker, searchResource(ResourceType.table, 'test', 0))
+        testSaga(searchResourceWorker, searchResource('test', ResourceType.table, 0))
           .next().throw(new Error()).put(searchResourceFailure())
           .next().isDone();
       });

amundsen_application/static/js/ducks/search/types.ts

@@ -2,7 +2,6 @@ import {
   DashboardResource,
   Resource,
   ResourceType,
-  SearchAllOptions,
   TableResource,
   UserResource,
 } from 'interfaces';
@@ -18,7 +17,12 @@ export type UserSearchResults = SearchResults<UserResource>;
 
 export interface SearchResponsePayload {
   search_term: string;
-  isLoading: boolean;
+  dashboards?: DashboardSearchResults;
+  tables?: TableSearchResults;
+  users?: UserSearchResults;
+};
+export interface SearchAllResponsePayload extends SearchResponsePayload {
+  selectedTab: ResourceType;
   dashboards: DashboardSearchResults;
   tables: TableSearchResults;
   users: UserSearchResults;
@@ -32,14 +36,15 @@ export enum SearchAll {
 };
 export interface SearchAllRequest {
   payload: {
-    options: SearchAllOptions;
+    resource: ResourceType;
+    pageIndex: number;
     term: string;
   };
   type: SearchAll.REQUEST;
 };
 export interface SearchAllResponse {
   type: SearchAll.SUCCESS | SearchAll.FAILURE;
-  payload?: SearchResponsePayload;
+  payload?: SearchAllResponsePayload;
 };
 export interface SearchAllReset {
   type: SearchAll.RESET;
@@ -62,3 +67,13 @@ export interface SearchResourceResponse {
   type: SearchResource.SUCCESS | SearchResource.FAILURE;
   payload?: SearchResponsePayload;
 };
+
+export enum UpdateSearchTab {
+  REQUEST = 'amundsen/search/UPDATE_SEARCH_TAB_REQUEST',
+}
+export interface UpdateSearchTabRequest {
+  type: UpdateSearchTab.REQUEST;
+  payload: {
+    selectedTab: ResourceType;
+  }
+}

amundsen_application/static/js/fixtures/globalState.ts

@@ -54,6 +54,7 @@ const globalState: GlobalState = {
   ],
   search: {
     search_term: 'testName',
+    selectedTab: ResourceType.table,
     isLoading: false,
     dashboards: {
       page_index: 0,

amundsen_application/static/js/fixtures/mockRouter.ts

@@ -26,7 +26,7 @@ export function getMockRouterProps<P>(data: P, location: Partial<History.Locatio
       action: 'POP',
       location: mockLocation,
       push: () => {},
-      replace: null,
+      replace: () => {},
       go: null,
       goBack: null,
       goForward: null,

amundsen_application/static/js/interfaces/Search.ts

-export interface SearchAllOptions {
-  dashboardIndex?: number;
-  tableIndex?: number;
-  userIndex?: number;
-}

amundsen_application/static/js/interfaces/index.ts

@@ -2,7 +2,6 @@ export * from './Announcements';
 export * from './Enums';
 export * from './Feedback';
 export * from './Resources';
-export * from './Search';
 export * from './TableMetadata';
 export * from './Tags';
 export * from './User';

amundsen_application/static/package-lock.json

@@ -6517,7 +6517,7 @@
         },
         "tar": {
           "version": "4.4.1",
-          "resolved": "https://registry.npmjs.org/tar/-/tar-4.4.1.tgz",
+          "resolved": false,
           "integrity": "sha512-O+v1r9yN4tOsvl90p5HAP4AEqbYhx4036AGMm075fH9F8Qwi3oJ+v4u50FkT/KkvywNGtwkk0zRI+8eYm1X/xg==",
           "optional": true,
           "requires": {
@@ -13306,7 +13306,7 @@
     "react-avatar": {
       "version": "2.5.1",
       "resolved": "https://registry.npmjs.org/react-avatar/-/react-avatar-2.5.1.tgz",
-      "integrity": "sha512-bwH5pWY6uxaKZt+IZBfD+SU3Dpy3FaKbmAzrOI4N8SATUPLXOdGaJHWUl6Vl8hHSwWSsoLh/m7xYHdnn0lofZw==",
+      "integrity": "sha1-W76MHQpSWT1GCPs9hinamV7rcJU=",
       "requires": {
         "babel-runtime": ">=5.0.0",
         "is-retina": "^1.0.3",