Merge pull request #2365 from fedspendingtransparency/hotfix/dev-6748…

…/advanced-search-download-bug

[DEV-6748] Refactor Adv Search to use Query Params

src/js/containers/router/RouterRoutes.js

@@ -9,6 +9,7 @@ import kGlobalConstants from 'GlobalConstants';
 
 const Homepage = React.lazy(() => import('components/homepage/Homepage').then((comp) => comp));
 const SearchContainer = React.lazy(() => import('containers/search/SearchContainer').then((comp) => comp));
+const SearchContainerRedirect = React.lazy(() => import('containers/search/SearchContainer').then((module) => ({ default: module.SearchContainerRedirect })));
 const ExplorerLanding = React.lazy(() => import('components/explorer/landing/ExplorerLanding').then((comp) => comp));
 const ExplorerDetailPageContainer = React.lazy(() => import('containers/explorer/detail/ExplorerDetailPageContainer').then((comp) => comp));
 const AwardContainer = React.lazy(() => import('containers/award/AwardContainer').then((comp) => comp));
@@ -49,7 +50,7 @@ export const routes = [
     },
     {
         path: '/search/:urlHash',
-        component: SearchContainer,
+        component: SearchContainerRedirect,
         exact: true
     },
     {

src/js/containers/search/SearchContainer.jsx

@@ -7,7 +7,7 @@ import React, { useState, useEffect, useCallback, useRef } from 'react';
 import PropTypes from 'prop-types';
 import { useDispatch, useSelector } from 'react-redux';
 import { isCancel } from 'axios';
-import { useParams } from 'react-router-dom';
+import { Redirect, useLocation, useParams } from 'react-router-dom';
 
 import { filterStoreVersion, requiredTypes, initialState } from 'redux/reducers/search/searchFiltersReducer';
 import { restoreHashedFilters } from 'redux/actions/search/searchHashActions';
@@ -71,7 +71,8 @@ export const parseRemoteFilters = (data) => {
 };
 
 const SearchContainer = ({ history }) => {
-    const { urlHash } = useParams();
+    const { hash: urlHash } = SearchHelper.getObjFromQueryParams(useLocation().search);
+
     const dispatch = useDispatch();
     const {
         filters: stagedFilters,
@@ -138,11 +139,11 @@ const SearchContainer = ({ history }) => {
             });
             request.current.promise
                 .then((res) => {
-                    dispatch(setAppliedFilterEmptiness(false));
                     const filtersInImmutableStructure = parseRemoteFilters(res.data.filter);
                     if (filtersInImmutableStructure) {
                         // apply the filters to both the staged and applied stores
                         dispatch(restoreHashedFilters(filtersInImmutableStructure));
+                        dispatch(setAppliedFilterEmptiness(false));
                         // set download availability
                         setDownloadAvailability(filtersInImmutableStructure);
                     }
@@ -158,6 +159,9 @@ const SearchContainer = ({ history }) => {
                     }
                 });
         }
+        else if (SearchHelper.areFiltersSelected(appliedFilters) && SearchHelper.areFiltersEmpty(stagedFilters)) {
+            dispatch(restoreHashedFilters(appliedFilters));
+        }
         else if (!urlHash) {
             dispatch(resetAppliedFilters());
             dispatch(clearAllFilters());
@@ -167,15 +171,23 @@ const SearchContainer = ({ history }) => {
             if (request.current) {
                 request.current.cancel();
             }
+            // clear selected filters so we don't fetch previous search
+            // only when query hash is defined b/c if its a urlHash, we cant know if we're remounting w/ the query hash or going somewhere else
+            dispatch(resetAppliedFilters());
+            dispatch(clearAllFilters());
         };
     }, []);
 
     useEffect(() => {
         if (areAppliedFiltersEmpty && prevAreAppliedFiltersEmpty === false) {
             // all the filters were cleared, reset to a blank hash
-            history.replace('/search');
+            history.replace({
+                pathname: '/search',
+                search: ''
+            });
+            setDownloadAvailable(false);
         }
-    }, [areAppliedFiltersEmpty]);
+    }, [areAppliedFiltersEmpty, urlHash]);
 
     const generateHash = useCallback(() => {
         // POST an API request to retrieve the Redux state
@@ -193,8 +205,10 @@ const SearchContainer = ({ history }) => {
         request.current.promise
             .then((res) => {
                 // update the URL with the received hash
-                const newHash = res.data.hash;
-                history.replace(`/search/${newHash}`);
+                history.replace({
+                    pathname: `/search/`,
+                    search: `?${new URLSearchParams({ hash: res.data.hash }).toString()}`
+                });
                 setGenerateHashInFlight(false);
             })
             .catch((err) => {
@@ -224,6 +238,12 @@ const SearchContainer = ({ history }) => {
         }
     }, [appliedFilters, urlHash]);
 
+    useEffect(() => {
+        if (SearchHelper.areFiltersDifferent(appliedFilters, stagedFilters) && SearchHelper.areFiltersDifferent(prevAppliedFilters, appliedFilters)) {
+            dispatch(restoreHashedFilters(appliedFilters));
+        }
+    }, [appliedFilters, stagedFilters]);
+
     return (
         <SearchPage
             hash={urlHash}
@@ -238,3 +258,18 @@ const SearchContainer = ({ history }) => {
 
 SearchContainer.propTypes = propTypes;
 export default SearchContainer;
+
+export const SearchContainerRedirect = () => {
+    const { urlHash: pathHash } = useParams();
+    return (
+        <Redirect
+            to={{
+                pathname: '/search/',
+                search: `?${new URLSearchParams({ hash: pathHash }).toString()}`
+            }} />
+    );
+};
+
+SearchContainer.propTypes = {
+    history: PropTypes.object.isRequired
+};

src/js/containers/search/table/ResultsTableContainer.jsx

@@ -122,19 +122,18 @@ export class ResultsTableContainer extends React.Component {
         // we can't hide the table entirely because the viewport is required to calculate the
         // row rendering
         this.loadColumns();
-        if (SearchHelper.isSearchHashReady(this.props.location.pathname)) {
+        if (SearchHelper.isSearchHashReady(this.props.location)) {
             this.pickDefaultTab();
         }
     }
 
     componentDidUpdate(prevProps) {
-        const filtersChanged = !SearchHelper.areFiltersEqual(prevProps.filters, this.props.filters);
-        if (filtersChanged && !this.props.noApplied) {
-            // filters changed, update the search object
+        if (prevProps.subaward !== this.props.subaward && !this.props.noApplied) {
+            // subaward toggle changed, update the search object
             this.pickDefaultTab();
         }
-        else if (prevProps.subaward !== this.props.subaward && !this.props.noApplied) {
-            // subaward toggle changed, update the search object
+        else if (SearchHelper.isSearchHashReady(this.props.location) && this.props.location.search !== prevProps.location.search) {
+            // hash is (a) defined and (b) new
             this.pickDefaultTab();
         }
     }

src/js/dataMapping/shared/checkboxTree/checkboxTree.jsx

@@ -35,3 +35,10 @@ export const treeIcons = {
     parentOpen: null,
     leaf: null
 };
+
+export const checkboxTreeFilters = [
+    'defCodes',
+    'pscCodes',
+    'naicsCodes',
+    'tasCodes'
+];

src/js/helpers/searchHelper.js

@@ -4,7 +4,10 @@
   **/
 
 import { is } from 'immutable';
+import { isEqual, sortBy } from 'lodash';
 import { initialState } from 'redux/reducers/search/searchFiltersReducer';
+import { checkboxTreeFilters } from 'dataMapping/shared/checkboxTree/checkboxTree';
+
 import { apiRequest } from './apiRequest';
 
 // Agency search for autocomplete
@@ -134,13 +137,19 @@ export const fetchLastUpdate = () => apiRequest({
     url: 'v2/awards/last_updated/'
 });
 
+const areCheckboxSelectionsEqual = ({ exclude: exclude1, require: require1 }, { exclude: exclude2, require: require2 }) => {
+    if (!isEqual(sortBy(require1), sortBy(require2))) return false;
+    if (!isEqual(sortBy(exclude1), sortBy(exclude2))) return false;
+    return true;
+};
+
 /**
  * Equality Comparison of two objects:
  * @param {Object} filters object to be measured for equality
  * @param {Object} filterReference object by which equality is measured  against
  * @returns {boolean}
  */
-export const areFiltersEqual = (filters, filterReference = initialState) => {
+export const areFiltersEqual = (filters = initialState, filterReference = initialState) => {
     if (!filterReference && filters) return false;
     const referenceObject = Object.assign({}, filterReference);
     const comparisonObject = Object.assign({}, filters);
@@ -161,14 +170,24 @@ export const areFiltersEqual = (filters, filterReference = initialState) => {
 
     // we need to iterate through each of the filter Redux keys in order to perform equality
     // comparisons on Immutable children (via the Immutable is() function)
-    const filterKeys = Object.keys(comparisonObject);
+    const immutableFilterKeys = Object
+        .keys(comparisonObject)
+        .filter((k) => !checkboxTreeFilters.includes(k));
 
-    for (let i = 0; i < filterKeys.length; i++) {
-        const key = filterKeys[i];
+    for (let i = 0; i < immutableFilterKeys.length; i++) {
+        const key = immutableFilterKeys[i];
         const unfilteredValue = comparisonObject[key];
         const currentValue = referenceObject[key];
         if (!is(unfilteredValue, currentValue)) return false;
     }
+
+    for (let i = 0; i < checkboxTreeFilters.length; i++) {
+        const key = checkboxTreeFilters[i];
+        const unfilteredValue = comparisonObject[key].toObject();
+        const currentValue = referenceObject[key].toObject();
+        if (!areCheckboxSelectionsEqual(unfilteredValue, currentValue)) return false;
+    }
+
     return true;
 };
 
@@ -177,7 +196,24 @@ export const areFiltersSelected = (filters) => !areFiltersEqual(filters);
 
 export const areFiltersDifferent = (a, b) => !areFiltersEqual(a, b);
 
-export const isSearchHashReady = (str) => str
-    .split('/search/')
-    .filter((s) => s && s !== "/search")
-    .length > 0;
+export const isSearchHashReady = ({ search }) => {
+    if (search) {
+        const params = new URLSearchParams(search);
+        for (const [key, value] of params.entries()) {
+            if (key === 'hash' && value) {
+                return true;
+            }
+        }
+        return false;
+    }
+    return false;
+};
+
+export const getObjFromQueryParams = (str) => {
+    const params = new URLSearchParams(str);
+    const obj = {};
+    for (const [key, value] of params.entries()) {
+        obj[key] = value;
+    }
+    return obj;
+};

tests/containers/search/SearchContainer-test.jsx

@@ -5,7 +5,7 @@
 import React from 'react';
 import { render, waitFor } from '@testing-library/react';
 import { Set } from 'immutable';
-import { useParams } from 'react-router-dom';
+import { useLocation } from 'react-router-dom';
 import * as redux from 'react-redux';
 
 import SearchContainer, { parseRemoteFilters } from 'containers/search/SearchContainer';
@@ -23,6 +23,8 @@ jest.mock('components/search/SearchPage', () => (
     jest.fn(() => null)
 ));
 
+jest.spyOn(URLSearchParams.prototype, 'toString').mockImplementation(() => 'str');
+
 jest.mock('helpers/searchHelper', () => ({
     ...jest.requireActual('helpers/searchHelper'),
     ...require('./filters/searchHelper')
@@ -41,7 +43,7 @@ jest.mock('react-redux', () => {
 
 jest.mock('react-router-dom', () => ({
     ...jest.requireActual('react-router-dom'),
-    useParams: jest.fn().mockReturnValue({ urlHash: 'abc' })
+    useLocation: jest.fn().mockReturnValue({ search: '' })
 }));
 
 jasmine.DEFAULT_TIMEOUT_INTERVAL = 10000;
@@ -71,8 +73,9 @@ test('parseRemoteFilters should return an immutable data structure when versions
     expect(parseRemoteFilters(mock.filter).timePeriodFY).toEqual(expectedFilter);
 });
 
-test('a non-hashed does not make a request to the api', async () => {
-    useParams.mockReturnValueOnce({ urlHash: null });
+test('a non-hashed url does not make a request to the api', async () => {
+    restoreUrlHash.mockClear();
+    generateUrlHash.mockClear();
     render(<SearchContainer />, {});
     await waitFor(() => {
         expect(restoreUrlHash).not.toHaveBeenCalled();
@@ -81,6 +84,8 @@ test('a non-hashed does not make a request to the api', async () => {
 });
 
 test('a hashed url makes a request to the api & sets loading state', async () => {
+    restoreUrlHash.mockClear();
+    useLocation.mockReturnValueOnce({ search: '?hash=abc' });
     const setLoadingStateFn = jest.spyOn(appliedFilterActions, 'setAppliedFilterEmptiness');
     render(<SearchContainer />, {});
     await waitFor(() => {
@@ -90,26 +95,33 @@ test('a hashed url makes a request to the api & sets loading state', async () =>
     });
 });
 
+
 test('when filters change (a) hash is generated, (b) loading is set & (c) url is updated', async () => {
     restoreUrlHash.mockClear();
-    useParams.mockReturnValueOnce({ urlHash: null });
     const setLoading = jest.spyOn(appliedFilterActions, 'setAppliedFilterEmptiness');
     const mockReplace = jest.fn();
+    const { rerender } = render(<SearchContainer history={{ replace: mockReplace }} />, {});
+
     jest.spyOn(redux, 'useSelector').mockReturnValue({
         ...mockRedux,
         appliedFilters: {
             filters: {
-                ...mockRedux.appliedFilters,
+                ...mockRedux.appliedFilters.filters,
                 timePeriodFY: Set(['2020'])
             }
         }
     });
 
-    render(<SearchContainer history={{ replace: mockReplace }} />, {});
+    rerender(<SearchContainer history={{ replace: mockReplace }} />, {});
 
     await waitFor(() => {
         expect(generateUrlHash).toHaveBeenCalledTimes(1);
         expect(mockReplace).toHaveBeenCalledTimes(1);
+        expect(mockReplace).toHaveBeenLastCalledWith({
+            pathname: '/search/',
+            // not ?hash=str because we aren't mocking out new URLSearchParams
+            search: '?str'
+        });
         expect(setLoading).toHaveBeenCalledWith(false);
         expect(restoreUrlHash).not.toHaveBeenCalled();
     });

tests/containers/search/table/ResultsTableContainer-test.jsx

@@ -39,19 +39,15 @@ jest.mock('helpers/textMeasurement', () => (
 ));
 
 describe('ResultsTableContainer', () => {
-    it('should pick a default tab whenever the Redux filters change & hash is present', async () => {
+    it('should pick a default tab whenever the hash changes', async () => {
         const container = mount(<ResultsTableContainer
-            location={{ pathname: '/search/123 ' }}
+            location={{ pathname: '/search/ ', search: '?hash=123' }}
             {...mockActions}
             {...mockRedux} />);
 
         container.instance().parseTabCounts = jest.fn();
 
-        // update the filters
-        const newFilters = Object.assign({}, mockRedux.filters, {
-            timePeriodFY: new Set(['1987'])
-        });
-        container.setProps({ filters: newFilters });
+        container.setProps({ location: { search: '?hash=456' } });
 
         await container.instance().tabCountRequest.promise;
 
@@ -60,7 +56,7 @@ describe('ResultsTableContainer', () => {
 
     it('should pick a default tab whenever the subaward toggle changes & hash is present', async () => {
         const container = mount(<ResultsTableContainer
-            location={{ pathname: '/search/123' }}
+            location={{ pathname: '/search', search: '?hash=123' }}
             {...mockActions}
             {...mockRedux} />);