Added loading spinner for search requests (#182)

* added loading spinner for search requests

* fixed multiple requests issue, SearchResourceAction now sets loading state

* added tests, cleaned up reducer

* fixed typos

* fixed previous search term call

* made tests cleaner

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

@@ -8,6 +8,7 @@ import { RouteComponentProps } from 'react-router';
 
 import SearchBar from './SearchBar';
 import SearchList from './SearchList';
+import LoadingSpinner from 'components/common/LoadingSpinner';
 
 import BookmarkList from 'components/common/Bookmark/BookmarkList'
 import InfoButton from 'components/common/InfoButton';
@@ -48,6 +49,7 @@ import {
 
 export interface StateFromProps {
   searchTerm: string;
+  isLoading: boolean;
   popularTables: TableResource[];
   tables: TableSearchResults;
   dashboards: DashboardSearchResults;
@@ -97,7 +99,10 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState
       const { searchTerm, pageIndex, selectedTab } = params;
       const { term, index, currentTab } = this.getSanitizedUrlParams(searchTerm, pageIndex, selectedTab);
       this.setState({ selectedTab: currentTab });
-      this.props.searchAll(term, this.createSearchOptions(index, currentTab));
+      const prevTerm = prevProps.searchTerm;
+      if (term !== prevTerm) {
+        this.props.searchAll(term, this.createSearchOptions(index, currentTab));
+      }
     }
   }
 
@@ -252,6 +257,16 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState
       );
   };
 
+  renderContent = () => {
+    if (this.props.isLoading) {
+      return (<LoadingSpinner/>);
+    }
+    if (this.props.searchTerm.length > 0) {
+      return this.renderSearchResults();
+    }
+    return this.renderPopularTables();
+  };
+
   render() {
     const { searchTerm } = this.props;
     const innerContent = (
@@ -259,8 +274,7 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState
         <div className="row">
           <div className="col-xs-12 col-md-offset-1 col-md-10">
             <SearchBar handleValueSubmit={ this.onSearchBarSubmit } searchTerm={ searchTerm }/>
-            { searchTerm.length > 0 && this.renderSearchResults() }
-            { searchTerm.length === 0 && this.renderPopularTables()  }
+            { this.renderContent() }
           </div>
         </div>
       </div>
@@ -279,6 +293,7 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState
 export const mapStateToProps = (state: GlobalState) => {
   return {
     searchTerm: state.search.search_term,
+    isLoading: state.search.isLoading,
     popularTables: state.popularTables,
     tables: state.search.tables,
     users: state.search.users,

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

@@ -29,12 +29,14 @@ import SearchBar from '../SearchBar';
 import SearchList from '../SearchList';
 
 import globalState from 'fixtures/globalState';
+import LoadingSpinner from 'components/common/LoadingSpinner';
 
 describe('SearchPage', () => {
   const setStateSpy = jest.spyOn(SearchPage.prototype, 'setState');
   const setup = (propOverrides?: Partial<SearchPageProps>) => {
     const props: SearchPageProps = {
       searchTerm: globalState.search.search_term,
+      isLoading: false,
       popularTables: globalState.popularTables,
       dashboards: globalState.search.dashboards,
       tables: globalState.search.tables,
@@ -226,16 +228,16 @@ describe('SearchPage', () => {
   });
 
   describe('componentDidUpdate', () => {
-    let props;
-    let wrapper;
     let searchAllSpy;
-    
+      
     let mockSearchOptions;
     let mockSanitizedUrlParams;
 
     let createSearchOptionsSpy;
     let getSanitizedUrlParamsSpy;
 
+    let props;
+    let wrapper;
     beforeAll(() => {
       const setupResult = setup({
         location: {
@@ -263,6 +265,7 @@ describe('SearchPage', () => {
       setStateSpy.mockClear();
 
       const mockPrevProps = {
+        searchTerm: 'previous',
         location: {
           search: '/search?searchTerm=previous&selectedTab=table&pageIndex=0', 
           pathname: 'mockstr',
@@ -277,9 +280,24 @@ describe('SearchPage', () => {
       expect(setStateSpy).toHaveBeenCalledWith({ selectedTab: ResourceType.table });
     });
 
-    it('calls searchAll', () => {
+    it('calls searchAll if called with a new search term', () => {
       expect(searchAllSpy).toHaveBeenCalledWith(mockSanitizedUrlParams.term, mockSearchOptions);
     });
+
+    it('does not call searchAll if called with the same search term with a new page', () => {
+      searchAllSpy.mockClear();
+      const mockPrevProps = {
+        searchTerm: 'current',
+        location: {
+          search: '/search?searchTerm=current&current=table&pageIndex=1', 
+          pathname: 'mockstr',
+          state: jest.fn(),
+          hash: 'mockstr',
+        }
+      };
+      wrapper.instance().componentDidUpdate(mockPrevProps);
+      expect(searchAllSpy).not.toHaveBeenCalled();
+    });
   });
 
   describe('getSanitizedUrlParams', () => {
@@ -606,6 +624,23 @@ describe('SearchPage', () => {
     });
   });
 
+  describe('renderContent', () => {
+    it('renders popular tables if searchTerm is empty', () => {
+      const {props, wrapper} = setup({ searchTerm: '' });
+      expect(wrapper.instance().renderContent()).toEqual(wrapper.instance().renderPopularTables());
+    });
+
+    it('renders search results when given search term', () => {
+      const {props, wrapper} = setup({ searchTerm: 'test' });
+      expect(wrapper.instance().renderContent()).toEqual(wrapper.instance().renderSearchResults());
+    });
+
+    it('renders loading spinner when in loading state', () => {
+      const {props, wrapper} = setup({ isLoading: true });
+      expect(wrapper.instance().renderContent()).toEqual(<LoadingSpinner/>);
+    });
+  });
+
   describe('renderPopularTables', () => {
     let content;
     let props;
@@ -727,6 +762,10 @@ describe('mapStateToProps', () => {
     expect(result.searchTerm).toEqual(globalState.search.search_term);
   });
 
+  it('sets isLoading on the props', () => {
+    expect(result.isLoading).toEqual(globalState.search.isLoading);
+  });
+
   it('sets popularTables on the props', () => {
     expect(result.popularTables).toEqual(globalState.popularTables);
   });

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

@@ -12,10 +12,11 @@ import {
 } from './types';
 import { ResourceType } from 'components/common/ResourceListItem/types';
 
-export type SearchReducerAction = SearchAllResponse | SearchResourceResponse | SearchAllRequest;
+export type SearchReducerAction = SearchAllResponse | SearchResourceResponse | SearchAllRequest | SearchResourceRequest;
 
 export interface SearchReducerState {
   search_term: string;
+  isLoading: boolean;
   dashboards: DashboardSearchResults;
   tables: TableSearchResults;
   users: UserSearchResults;
@@ -40,6 +41,7 @@ export function searchResource(resource: ResourceType, term: string, pageIndex:
 
 const initialState: SearchReducerState = {
   search_term: '',
+  isLoading: false,
   dashboards: {
     page_index: 0,
     results: [],
@@ -64,6 +66,12 @@ export default function reducer(state: SearchReducerState = initialState, action
       return {
         ...state,
         search_term: action.term,
+        isLoading: true,
+      };
+    case SearchResource.ACTION:
+      return {
+        ...state,
+        isLoading: true,
       };
     // SearchAll will reset all resources with search results or the initial state
     case SearchAll.SUCCESS:
@@ -71,6 +79,7 @@ export default function reducer(state: SearchReducerState = initialState, action
       return {
         ...initialState,
         ...newState,
+        isLoading: false,
       };
     // SearchResource will set only a single resource and preserves search state for other resources
     case SearchResource.SUCCESS:
@@ -78,10 +87,14 @@ export default function reducer(state: SearchReducerState = initialState, action
       return {
         ...state,
         ...resourceNewState,
+        isLoading: false,
       };
     case SearchAll.FAILURE:
     case SearchResource.FAILURE:
-      return initialState;
+      return {
+        ...initialState,
+        isLoading: false,
+      };      
     default:
       return state;
   }

amundsen_application/static/js/fixtures/globalState.ts

@@ -56,6 +56,7 @@ const globalState: GlobalState = {
   ],
   search: {
     search_term: 'testName',
+    isLoading: false,
     dashboards: {
       page_index: 0,
       results: [],