diff --git a/src/js/containers/router/RouterRoutes.js b/src/js/containers/router/RouterRoutes.js index f299b4f85f..b041d1a432 100644 --- a/src/js/containers/router/RouterRoutes.js +++ b/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 }, { diff --git a/src/js/containers/search/SearchContainer.jsx b/src/js/containers/search/SearchContainer.jsx index 8ac3a85967..4d616a412c 100644 --- a/src/js/containers/search/SearchContainer.jsx +++ b/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 ( { SearchContainer.propTypes = propTypes; export default SearchContainer; + +export const SearchContainerRedirect = () => { + const { urlHash: pathHash } = useParams(); + return ( + + ); +}; + +SearchContainer.propTypes = { + history: PropTypes.object.isRequired +}; diff --git a/src/js/containers/search/table/ResultsTableContainer.jsx b/src/js/containers/search/table/ResultsTableContainer.jsx index 069924040e..36d4f31096 100644 --- a/src/js/containers/search/table/ResultsTableContainer.jsx +++ b/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(); } } diff --git a/src/js/dataMapping/shared/checkboxTree/checkboxTree.jsx b/src/js/dataMapping/shared/checkboxTree/checkboxTree.jsx index dce17d6d75..6e93c12d0c 100644 --- a/src/js/dataMapping/shared/checkboxTree/checkboxTree.jsx +++ b/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' +]; diff --git a/src/js/helpers/searchHelper.js b/src/js/helpers/searchHelper.js index ce27cfb66e..0c2685d5e3 100644 --- a/src/js/helpers/searchHelper.js +++ b/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; +}; diff --git a/tests/containers/search/SearchContainer-test.jsx b/tests/containers/search/SearchContainer-test.jsx index d74074c310..d9805e0f9e 100644 --- a/tests/containers/search/SearchContainer-test.jsx +++ b/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(, {}); 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(, {}); 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(, {}); + jest.spyOn(redux, 'useSelector').mockReturnValue({ ...mockRedux, appliedFilters: { filters: { - ...mockRedux.appliedFilters, + ...mockRedux.appliedFilters.filters, timePeriodFY: Set(['2020']) } } }); - render(, {}); + rerender(, {}); 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(); }); diff --git a/tests/containers/search/table/ResultsTableContainer-test.jsx b/tests/containers/search/table/ResultsTableContainer-test.jsx index fe29e57182..26f61505f2 100644 --- a/tests/containers/search/table/ResultsTableContainer-test.jsx +++ b/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(); 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(); diff --git a/tests/helpers/searchHelper-test.js b/tests/helpers/searchHelper-test.js index f218c77c74..d76c2cab65 100644 --- a/tests/helpers/searchHelper-test.js +++ b/tests/helpers/searchHelper-test.js @@ -1,26 +1,71 @@ -import { initialState } from 'redux/reducers/search/searchFiltersReducer'; +import { initialState, CheckboxTreeSelections } from 'redux/reducers/search/searchFiltersReducer'; import { areFiltersEqual, isSearchHashReady } from 'helpers/searchHelper'; -test('areFiltersEqual should return true when selected filters are effectively blank', () => { - expect(areFiltersEqual(initialState, initialState)).toBeTruthy(); -}); - -test('areFiltersEqual should return false when filters are selected', () => { - expect(areFiltersEqual(initialState, { +test.each([ + ['obj1 & obj2 are both initial state', true, initialState, initialState], + ['obj1 is initial state and obj2 has timePeriodFY changed', false, initialState, { ...initialState, timePeriodFY: new Set(['2020']) }], + ['empty vs defined def codes', false, initialState, { ...initialState, - timePeriodFY: new Set(['2020']) - })).toBeFalsy(); -}); - -test('areFiltersEqual should return false when filters are selected', () => { - expect(areFiltersEqual(initialState, { + defCodes: CheckboxTreeSelections({ require: ['12'], counts: [], exclude: [] }) + }], + ['same def codes', true, { + ...initialState, + defCodes: CheckboxTreeSelections({ require: ['21', '12'], counts: [], exclude: [] }) + }, + { + ...initialState, + defCodes: CheckboxTreeSelections({ require: ['12', '21'], counts: [], exclude: [] }) + }], + ['same def codes (w/ 2d array)', true, { + ...initialState, + defCodes: CheckboxTreeSelections({ require: [['21', '2121'], ['64', '6464']], counts: [], exclude: [] }) + }, + { + ...initialState, + defCodes: CheckboxTreeSelections({ require: [['64', '6464'], ['21', '2121']], counts: [], exclude: [] }) + }], + ['different def codes on require', false, { ...initialState, - timePeriodFY: new Set(['2020']) - })).toBeFalsy(); + defCodes: CheckboxTreeSelections({ require: ['21'], counts: [], exclude: [] }) + }, + { + ...initialState, + defCodes: CheckboxTreeSelections({ require: ['22'], counts: [], exclude: [] }) + }], + ['different def codes on require (w/ 2d array)', false, { + ...initialState, + defCodes: CheckboxTreeSelections({ require: [['21', '2121']], counts: [], exclude: [] }) + }, + { + ...initialState, + defCodes: CheckboxTreeSelections({ require: [['22', '2222']], counts: [], exclude: [] }) + }], + ['different def codes on exclude', false, { + ...initialState, + defCodes: CheckboxTreeSelections({ require: ['21'], counts: [], exclude: ['1'] }) + }, + { + ...initialState, + defCodes: CheckboxTreeSelections({ require: ['21'], counts: [], exclude: ['2'] }) + }], + ['different def codes on exclude (w/ 2d array)', false, { + ...initialState, + defCodes: CheckboxTreeSelections({ require: [['21', '2121']], counts: [], exclude: [['20', '2020']] }) + }, + { + ...initialState, + defCodes: CheckboxTreeSelections({ require: [['21', '2121']], counts: [], exclude: [['19', '1919']] }) + }] +])('%s, returns %s', (str, rtrn, obj1, obj2) => { + expect(areFiltersEqual(obj1, obj2)).toEqual(rtrn); }); -test('isSearchHashReady knows when theres a hash', () => { - expect(isSearchHashReady('/search')).toEqual(false); - expect(isSearchHashReady('/search/')).toEqual(false); - expect(isSearchHashReady('/search/test')).toEqual(true); +test.each([ + ['', false], + ['/?', false], + ['/?fy=2020&period=12&hash=', false], + ['/?fy=2020&period=12&hash=t', true] +])('when input (a query param) is %s return value is %s', (input, rtrn) => { + expect(isSearchHashReady({ search: input })).toEqual(rtrn); }); +