Merge pull request #6598 from kuangp/fix/searchBar

fix(search): Fix search page to respond to searches made from sidebar

plugins/search/src/components/SearchBar/SearchBar.tsx

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import React, { ChangeEvent, useState } from 'react';
+import React, { ChangeEvent, useEffect, useState } from 'react';
 import { useDebounce } from 'react-use';
 import { InputBase, InputAdornment, IconButton } from '@material-ui/core';
 import SearchIcon from '@material-ui/icons/Search';
@@ -31,6 +31,10 @@ export const SearchBar = ({ className, debounceTime = 0 }: Props) => {
   const { term, setTerm } = useSearch();
   const [value, setValue] = useState<string>(term);
 
+  useEffect(() => {
+    setValue(prevValue => (prevValue !== term ? term : prevValue));
+  }, [term]);
+
   useDebounce(() => setTerm(value), debounceTime, [value]);
 
   const handleQuery = (e: ChangeEvent<HTMLInputElement>) => {

plugins/search/src/components/SearchContext/SearchContext.tsx

@@ -54,7 +54,7 @@ export const SearchContextProvider = ({
     term: '',
     pageCursor: '',
     filters: {},
-    types: ['*'],
+    types: [],
   },
   children,
 }: PropsWithChildren<{ initialState?: SettableSearchContext }>) => {

plugins/search/src/components/SearchPage/SearchPage.test.tsx

@@ -18,7 +18,7 @@ import React from 'react';
 import { renderInTestApp } from '@backstage/test-utils';
 import { useLocation, useOutlet } from 'react-router';
 
-import { useSearch, SearchContextProvider } from '../SearchContext';
+import { useSearch } from '../SearchContext';
 import { SearchPage } from './';
 
 jest.mock('react-router', () => ({
@@ -29,6 +29,11 @@ jest.mock('react-router', () => ({
   useOutlet: jest.fn().mockReturnValue('Route Children'),
 }));
 
+const setTermMock = jest.fn();
+const setTypesMock = jest.fn();
+const setFiltersMock = jest.fn();
+const setPageCursorMock = jest.fn();
+
 jest.mock('../SearchContext', () => ({
   ...jest.requireActual('../SearchContext'),
   SearchContextProvider: jest
@@ -36,9 +41,13 @@ jest.mock('../SearchContext', () => ({
     .mockImplementation(({ children }) => children),
   useSearch: jest.fn().mockReturnValue({
     term: '',
+    setTerm: (term: any) => setTermMock(term),
     types: [],
+    setTypes: (types: any) => setTypesMock(types),
     filters: {},
+    setFilters: (filters: any) => setFiltersMock(filters),
     pageCursor: '',
+    setPageCursor: (pageCursor: any) => setPageCursorMock(pageCursor),
   }),
 }));
 
@@ -58,7 +67,7 @@ describe('SearchPage', () => {
     window.history.replaceState = origReplaceState;
   });
 
-  it('uses initial term state from location', async () => {
+  it('sets term state from location', async () => {
     // Given this initial location.search value...
     const expectedFilterField = 'anyKey';
     const expectedFilterValue = 'anyValue';
@@ -75,13 +84,11 @@ describe('SearchPage', () => {
     // When we render the page...
     await renderInTestApp(<SearchPage />);
 
-    // Then search context should be initialized with these values...
-    const calls = (SearchContextProvider as jest.Mock).mock.calls[0];
-    const actualInitialState = calls[0].initialState;
-    expect(actualInitialState.term).toEqual(expectedTerm);
-    expect(actualInitialState.types).toEqual(expectedTypes);
-    expect(actualInitialState.pageCursor).toEqual(expectedPageCursor);
-    expect(actualInitialState.filters).toStrictEqual(expectedFilters);
+    // Then search context should be set with these values...
+    expect(setTermMock).toHaveBeenCalledWith(expectedTerm);
+    expect(setTypesMock).toHaveBeenCalledWith(expectedTypes);
+    expect(setPageCursorMock).toHaveBeenCalledWith(expectedPageCursor);
+    expect(setFiltersMock).toHaveBeenCalledWith(expectedFilters);
   });
 
   it('renders provided router element', async () => {
@@ -103,6 +110,10 @@ describe('SearchPage', () => {
       types: ['software-catalog'],
       pageCursor: 'page2-or-something',
       filters: { anyKey: 'anyValue' },
+      setTerm: setTermMock,
+      setTypes: setTypesMock,
+      setFilters: setFiltersMock,
+      setPageCursor: setPageCursorMock,
     });
     const expectedLocation = encodeURI(
       '?query=bieber&types[]=software-catalog&pageCursor=page2-or-something&filters[anyKey]=anyValue',

plugins/search/src/components/SearchPage/SearchPage.tsx

@@ -14,7 +14,8 @@
  * limitations under the License.
  */
 
-import React from 'react';
+import React, { useEffect } from 'react';
+import { usePrevious } from 'react-use';
 import qs from 'qs';
 import { useLocation, useOutlet } from 'react-router';
 import { SearchContextProvider, useSearch } from '../SearchContext';
@@ -22,46 +23,72 @@ import { JsonObject } from '@backstage/config';
 import { LegacySearchPage } from '../LegacySearchPage';
 
 export const UrlUpdater = () => {
-  const { term, types, pageCursor, filters } = useSearch();
+  const location = useLocation();
+  const {
+    term,
+    setTerm,
+    types,
+    setTypes,
+    pageCursor,
+    setPageCursor,
+    filters,
+    setFilters,
+  } = useSearch();
 
-  const newParams = qs.stringify(
-    {
-      query: term,
-      types,
-      pageCursor,
-      filters,
-    },
-    { arrayFormat: 'brackets' },
-  );
-  const newUrl = `${window.location.pathname}?${newParams}`;
+  const prevQueryParams = usePrevious(location.search);
+  useEffect(() => {
+    // Only respond to changes to url query params
+    if (location.search === prevQueryParams) {
+      return;
+    }
 
-  // We directly manipulate window history here in order to not re-render
-  // infinitely (state => location => state => etc). The intention of this
-  // code is just to ensure the right query/filters are loaded when a user
-  // clicks the "back" button after clicking a result.
-  window.history.replaceState(null, document.title, newUrl);
+    const query =
+      qs.parse(location.search.substring(1), { arrayLimit: 0 }) || {};
+
+    if (query.filters) {
+      setFilters(query.filters as JsonObject);
+    }
+
+    if (query.query) {
+      setTerm(query.query as string);
+    }
+
+    if (query.pageCursor) {
+      setPageCursor(query.pageCursor as string);
+    }
+
+    if (query.types) {
+      setTypes(query.types as string[]);
+    }
+  }, [prevQueryParams, location, setTerm, setTypes, setPageCursor, setFilters]);
+
+  useEffect(() => {
+    const newParams = qs.stringify(
+      {
+        query: term,
+        types,
+        pageCursor,
+        filters,
+      },
+      { arrayFormat: 'brackets' },
+    );
+    const newUrl = `${window.location.pathname}?${newParams}`;
+
+    // We directly manipulate window history here in order to not re-render
+    // infinitely (state => location => state => etc). The intention of this
+    // code is just to ensure the right query/filters are loaded when a user
+    // clicks the "back" button after clicking a result.
+    window.history.replaceState(null, document.title, newUrl);
+  }, [term, types, pageCursor, filters]);
 
   return null;
 };
 
 export const SearchPage = () => {
-  const location = useLocation();
   const outlet = useOutlet();
-  const query = qs.parse(location.search.substring(1), { arrayLimit: 0 }) || {};
-  const filters = (query.filters as JsonObject) || {};
-  const queryString = (query.query as string) || '';
-  const pageCursor = (query.pageCursor as string) || '';
-  const types = (query.types as string[]) || [];
-
-  const initialState = {
-    term: queryString || '',
-    types,
-    pageCursor,
-    filters,
-  };
 
   return (
-    <SearchContextProvider initialState={initialState}>
+    <SearchContextProvider>
       <UrlUpdater />
       {outlet || <LegacySearchPage />}
     </SearchContextProvider>