diff --git a/ADVANCED.md b/ADVANCED.md index 8d0907ec..5ec48e28 100644 --- a/ADVANCED.md +++ b/ADVANCED.md @@ -144,6 +144,20 @@ For example: ``` +### Combining Actions + +There are certain cases where you may need to apply one or more actions at a time. Search UI intelligently +batches actions into a single API call. + +For example, if you need to apply two filters at once, it is perfectly acceptable to write the following code: + +``` +addFilter("states", "Alaska", "any"); +addFilter("world_heritage_site", "true"); +``` + +This will only result in a single API call. + ## Headless Core Reference ### SearchProvider @@ -182,19 +196,19 @@ ex. ### Actions -| method | params | return | description | -| ------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | ----------------------------------------------------------------------------------------------------------------------------------------- | -| `addFilter` | `name` String - field name to filter on
`value` String - field value to filter on
`filterType` String - type of filter to apply: "all", "any", or "none" | | Add a filter in addition to current filters values. | -| `setFilter` | `name` String - field name to filter on
`value` String - field value to filter on
`filterType` String - type of filter to apply: "all", "any", or "none" | | Set a filter value, replacing current filter values. | -| `removeFilter` | `name` String - field to remove filters from
`value` String - (Optional) Specify which filter value to remove
`filterType` String - (Optional) Specify which filter type to remove: "all", "any", or "none" | | Removes filters or filter values. | -| `reset` | | | Reset state to initial search state. | -| `clearFilters` | `except` Array[String] - List of field names that should NOT be cleared | | Clear all filters. | -| `setCurrent` | Integer | | Update the current page number. Used for paging. | -| `setResultsPerPage` | Integer | | | -| `setSearchTerm` | `searchTerm` String
`options` Object
`options.refresh` Boolean - Refresh search results on update. Default: `true`.
`options.debounce` Number - Length to debounce any resulting queries
`options.autocompleteSuggestions` Boolean - Fetch query suggestions for autocomplete on update, stored in `autocompletedSuggestions` state
`options.autocompleteResults` Boolean - Fetch results on update, stored in `autocompletedResults` state | | | -| `setSort` | `sortField` String - field to sort on
`sortDirection` String - "asc" or "desc" | | | -| `trackClickThrough` | `documentId` String - The document ID associated with the result that was clicked
`tag` - Array[String] Optional tags which can be used to categorize this click event | | Report a clickthrough event, which is when a user clicks on a result link. | -| `a11yNotify` | `messageFunc` String - object key to run as function
`messageArgs` Object - Arguments to pass to form your screen reader message string | | Reads out a screen reader accessible notification. See `a11yNotificationMessages` under [Advanced Configuration](#advanced-configuration) | +| method | params | return | description | +| ------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | ----------------------------------------------------------------------------------------------------------------------------------------- | +| `addFilter` | `name` String - field name to filter on
`value` String - field value to filter on
`filterType` String - type of filter to apply: "all", "any", or "none" | | Add a filter in addition to current filters values. | +| `setFilter` | `name` String - field name to filter on
`value` String - field value to filter on
`filterType` String - type of filter to apply: "all", "any", or "none" | | Set a filter value, replacing current filter values. | +| `removeFilter` | `name` String - field to remove filters from
`value` String - (Optional) Specify which filter value to remove
`filterType` String - (Optional) Specify which filter type to remove: "all", "any", or "none" | | Removes filters or filter values. | +| `reset` | | | Reset state to initial search state. | +| `clearFilters` | `except` Array[String] - List of field names that should NOT be cleared | | Clear all filters. | +| `setCurrent` | Integer | | Update the current page number. Used for paging. | +| `setResultsPerPage` | Integer | | | +| `setSearchTerm` | `searchTerm` String
`options` Object
`options.refresh` Boolean - Refresh search results on update. Default: `true`.
`options.debounce` Number - Length to debounce any resulting queries.
`options.shouldClearFilters` Boolean - Should existing filters be cleared? Default: `true`.
`options.autocompleteSuggestions` Boolean - Fetch query suggestions for autocomplete on update, stored in `autocompletedSuggestions` state
`options.autocompleteResults` Boolean - Fetch results on update, stored in `autocompletedResults` state | | | +| `setSort` | `sortField` String - field to sort on
`sortDirection` String - "asc" or "desc" | | | +| `trackClickThrough` | `documentId` String - The document ID associated with the result that was clicked
`tag` - Array[String] Optional tags which can be used to categorize this click event | | Report a clickthrough event, which is when a user clicks on a result link. | +| `a11yNotify` | `messageFunc` String - object key to run as function
`messageArgs` Object - Arguments to pass to form your screen reader message string | | Reads out a screen reader accessible notification. See `a11yNotificationMessages` under [Advanced Configuration](#advanced-configuration) | ### State @@ -529,6 +543,7 @@ page for suggestions, and maintaining the default behavior when selecting a resu | Name | type | Required? | Default | Options | Description | | ----------------------------- | ---------------------------------------------------------------------------- | --------- | ------------------------------------------------------------------ | ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | className | String | no | | | | +| shouldClearFilters | Boolean | no | true | | Should existing filters be cleared when a new search is performed? | | inputProps | Object | no | | | Props for underlying 'input' element. I.e., `{ placeholder: "Enter Text"}`. | | searchAsYouType | Boolean | no | false | | Executes a new search query with every key stroke. You can fine tune the number of queries made by adjusting the `debounceLength` parameter. | | debounceLength | Number | no | 200 | | When using `searchAsYouType`, it can be useful to "debounce" search requests to avoid creating an excessive number of requests. This controls the length to debounce / wait. | @@ -750,15 +765,15 @@ import { MultiCheckboxFacet } from "@elastic/react-search-ui-views"; ### Properties -| Name | type | Required? | Default | Options | Description | -| ------------ | --------------- | --------- | ------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| className | String | no | | | | -| field | String | yes | | | Field name corresponding to this filter. This requires that the corresponding field has been configured in `facets` on the top level Provider. | -| filterType | String | no | "all" | "all", "any", "none" | The type of filter to apply with the selected values. I.e., should "all" of the values match, or just "any" of the values, or "none" of the values. Note: See the example above which describes using "disjunctive" facets in conjunction with filterType. | -| label | String | yes | | | A static label to show in the facet filter. | -| show | Number | no | 5 | | The number of facet filter options to show before concatenating with a "more" link. | +| Name | type | Required? | Default | Options | Description | +| ------------ | --------------- | --------- | ------------------------------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| className | String | no | | | | +| field | String | yes | | | Field name corresponding to this filter. This requires that the corresponding field has been configured in `facets` on the top level Provider. | +| filterType | String | no | "all" | "all", "any", "none" | The type of filter to apply with the selected values. I.e., should "all" of the values match, or just "any" of the values, or "none" of the values. Note: See the example above which describes using "disjunctive" facets in conjunction with filterType. | +| label | String | yes | | | A static label to show in the facet filter. | +| show | Number | no | 5 | | The number of facet filter options to show before concatenating with a "more" link. | | view | Render Function | no | [MultiCheckboxFacet](packages/react-search-ui-views/src/MultiCheckboxFacet.js) | [SingleLinksFacet](packages/react-search-ui-views/src/SingleLinksFacet.js)
[SingleSelectFacet](packages/react-search-ui-views/src/SingleSelectFacet.js) [BooleanFacet](packages/react-search-ui-views/src/BooleanFacet.js) | Used to override the default view for this Component. See [Customization: Component views and HTML](#component-views-and-html) for more information. | -| isFilterable | Boolean | no | false | | Whether or not to show Facet quick filter. | +| isFilterable | Boolean | no | false | | Whether or not to show Facet quick filter. | --- @@ -1119,8 +1134,7 @@ return ( | `urlPushDebounceLength` | Integer | optional | 500 | The amount of time in milliseconds to debounce/delay updating the browser url after the UI update. This, for example, prevents excessive history entries while a user is still typing in a live search box. | | `hasA11yNotifications` | Boolean | optional | false | Search UI will create a visually hidden live region to announce search results & other actions to screen reader users. This accessibility feature will be turned on by default in our 2.0 release. | | `a11yNotificationMessages` | Object | optional | {} | You can override our default screen reader [messages](packages/search-ui/src/A11yNotifications.js#L49) (e.g. for localization), or create your own custom notification, by passing in your own key and message function(s). | -| `alwaysSearchOnInitialLoad` | Boolean | optional | false | If true, Search UI will always do an initial search, even when no inital Request State is set. - +| `alwaysSearchOnInitialLoad` | Boolean | optional | false | If true, Search UI will always do an initial search, even when no inital Request State is set. | ## Query Config diff --git a/packages/react-search-ui-views/src/SearchBox.js b/packages/react-search-ui-views/src/SearchBox.js index 875e5274..0b02d585 100644 --- a/packages/react-search-ui-views/src/SearchBox.js +++ b/packages/react-search-ui-views/src/SearchBox.js @@ -22,6 +22,8 @@ function SearchBox(props) { onSubmit, useAutocomplete, value, + // NOTE: These are explicitly de-structured but not used so that they are + // not passed through to the input with the 'rest' parameter // eslint-disable-next-line no-unused-vars autocompletedResults, // eslint-disable-next-line no-unused-vars diff --git a/packages/react-search-ui/src/containers/SearchBox.js b/packages/react-search-ui/src/containers/SearchBox.js index 18e359d9..8f3fb758 100644 --- a/packages/react-search-ui/src/containers/SearchBox.js +++ b/packages/react-search-ui/src/containers/SearchBox.js @@ -33,6 +33,7 @@ export class SearchBoxContainer extends Component { ]), autocompleteView: PropTypes.func, className: PropTypes.string, + shouldClearFilters: PropTypes.bool, debounceLength: PropTypes.number, inputProps: PropTypes.object, inputView: PropTypes.func, @@ -51,7 +52,8 @@ export class SearchBoxContainer extends Component { }; static defaultProps = { - autocompleteMinimumCharacters: 0 + autocompleteMinimumCharacters: 0, + shouldClearFilters: true }; state = { @@ -71,15 +73,19 @@ export class SearchBoxContainer extends Component { }; completeSuggestion = searchTerm => { - const { setSearchTerm } = this.props; - setSearchTerm(searchTerm); + const { shouldClearFilters, setSearchTerm } = this.props; + setSearchTerm(searchTerm, { + shouldClearFilters + }); }; handleSubmit = e => { - const { searchTerm, setSearchTerm } = this.props; + const { shouldClearFilters, searchTerm, setSearchTerm } = this.props; e.preventDefault(); - setSearchTerm(searchTerm); + setSearchTerm(searchTerm, { + shouldClearFilters + }); }; handleChange = value => { @@ -87,6 +93,7 @@ export class SearchBoxContainer extends Component { autocompleteMinimumCharacters, autocompleteResults, autocompleteSuggestions, + shouldClearFilters, searchAsYouType, setSearchTerm, debounceLength @@ -99,6 +106,7 @@ export class SearchBoxContainer extends Component { searchAsYouType) && { debounce: debounceLength || 200 }), + shouldClearFilters, refresh: !!searchAsYouType, autocompleteResults: !!autocompleteResults, autocompleteSuggestions: !!autocompleteSuggestions diff --git a/packages/react-search-ui/src/containers/__tests__/SearchBox.test.js b/packages/react-search-ui/src/containers/__tests__/SearchBox.test.js index f51214d6..36fb61e2 100644 --- a/packages/react-search-ui/src/containers/__tests__/SearchBox.test.js +++ b/packages/react-search-ui/src/containers/__tests__/SearchBox.test.js @@ -203,6 +203,65 @@ describe("useAutocomplete", () => { }); }); +describe("shouldClearFilters prop", () => { + it("will be passed through to setSearchTerm on submit", () => { + let viewProps; + + shallow( + (viewProps = props)} + /> + ); + + const { onSubmit } = viewProps; + onSubmit({ + preventDefault: () => {} + }); + const call = params.setSearchTerm.mock.calls[0]; + expect(call[1].shouldClearFilters).toEqual(false); + }); + + it("will be passed through to setSearchTerm on change", () => { + let viewProps; + + shallow( + (viewProps = props)} + /> + ); + + const { onChange } = viewProps; + onChange("new term"); + const call = params.setSearchTerm.mock.calls[0]; + expect(call[1].shouldClearFilters).toEqual(false); + }); + + it("will call setSearchTerm if no onSelectAutocomplete is specified and a suggestion is selected", () => { + let viewProps; + + shallow( + (viewProps = props)} + /> + ); + + const { onSelectAutocomplete } = viewProps; + onSelectAutocomplete({ + suggestion: "bird" + }); + + const call = params.setSearchTerm.mock.calls[0]; + expect(call[1].shouldClearFilters).toEqual(false); + }); +}); + it("will call back to setSearchTerm with refresh: false when input is changed", () => { let viewProps; shallow( @@ -220,6 +279,7 @@ it("will call back to setSearchTerm with refresh: false when input is changed", refresh: false, autocompleteResults: false, autocompleteSuggestions: false, + shouldClearFilters: true, autocompleteMinimumCharacters: 0 } ]); @@ -243,6 +303,7 @@ it("will call back to setSearchTerm with autocompleteMinimumCharacters setting", refresh: false, autocompleteResults: false, autocompleteSuggestions: false, + shouldClearFilters: true, autocompleteMinimumCharacters: 3 } ]); @@ -270,6 +331,7 @@ it("will call back to setSearchTerm with refresh: true when input is changed and debounce: 200, autocompleteResults: false, autocompleteMinimumCharacters: 0, + shouldClearFilters: true, autocompleteSuggestions: false } ]); @@ -298,6 +360,7 @@ it("will call back to setSearchTerm with a specific debounce when input is chang debounce: 500, autocompleteResults: false, autocompleteMinimumCharacters: 0, + shouldClearFilters: true, autocompleteSuggestions: false } ]); @@ -329,6 +392,7 @@ it("will call back to setSearchTerm with a specific debounce when input is chang debounce: 500, autocompleteResults: true, autocompleteMinimumCharacters: 0, + shouldClearFilters: true, autocompleteSuggestions: false } ]); @@ -357,6 +421,7 @@ it("will call back to setSearchTerm with a specific debounce when input is chang debounce: 500, autocompleteSuggestions: true, autocompleteMinimumCharacters: 0, + shouldClearFilters: true, autocompleteResults: false } ]); @@ -378,7 +443,7 @@ it("will call back setSearchTerm with refresh: true when form is submitted", () }); const call = params.setSearchTerm.mock.calls[0]; - expect(call).toEqual(["a term"]); + expect(call).toEqual(["a term", { shouldClearFilters: true }]); }); describe("onSelectAutocomplete", () => { @@ -420,6 +485,25 @@ describe("onSelectAutocomplete", () => { expect(passedAutocompleteResults).toBeDefined(); expect(passedDefaultOnSelectAutocomplete).toBeDefined(); }); + + it("will call setSearchTerm if no onSelectAutocomplete is specified and a suggestion is selected", () => { + let viewProps; + + shallow( + (viewProps = props)} + /> + ); + const { onSelectAutocomplete } = viewProps; + onSelectAutocomplete({ + suggestion: "bird" + }); + + const call = params.setSearchTerm.mock.calls[0]; + expect(call[0]).toEqual("bird"); + }); }); describe("autocomplete clickthroughs", () => { diff --git a/packages/search-ui/package.json b/packages/search-ui/package.json index 7a3d65d9..0d951f2e 100644 --- a/packages/search-ui/package.json +++ b/packages/search-ui/package.json @@ -45,5 +45,10 @@ "deep-equal": "^1.0.1", "history": "^4.9.0", "qs": "^6.7.0" + }, + "jest": { + "setupFilesAfterEnv": [ + "/src/setupTests.js" + ] } } diff --git a/packages/search-ui/src/DebounceManager.js b/packages/search-ui/src/DebounceManager.js index dd57f551..c4412aca 100644 --- a/packages/search-ui/src/DebounceManager.js +++ b/packages/search-ui/src/DebounceManager.js @@ -1,25 +1,34 @@ import debounceFn from "debounce-fn"; -export default class DebounceManager { +class DebounceManager { debounceCache = {}; - /* - The purpose of this is to: - Dynamically debounce and cache a debounced version of a function at the time of calling that function. This avoids - managing debounced version of functions locally. - - Assumption: - Functions are debounced on a combination of unique function and wait times. So debouncing won't work on - subsequent calls with different wait times or different functions. That also means that the debounce manager - can be used for different functions in parallel, and keep the two functions debounced separately. - - */ - runWithDebounce(wait, fn, ...parameters) { + /** + * Dynamically debounce and cache a debounced version of a function at the time of calling that function. This avoids + * managing debounced version of functions locally. + * + * In other words, debounce usually works by debouncing based on + * referential identity of a function. This works by comparing provided function names. + * + * This also has the ability to short-circuit a debounce all-together, if no wait + * time is provided. + * + * Assumption: + * Functions are debounced on a combination of unique function name and wait times. So debouncing won't work on + * subsequent calls with different wait times or different functions. That also means that the debounce manager + * can be used for different functions in parallel, and keep the two functions debounced separately. + * + * @param {number} wait Milliseconds to debounce. Executes immediately if falsey. + * @param {function} fn Function to debounce + * @param {function} functionName Name of function to debounce, used to create a unique key + * @param {...any} parameters Parameters to pass to function + */ + runWithDebounce(wait, functionName, fn, ...parameters) { if (!wait) { return fn(...parameters); } - const key = fn.toString() + wait.toString(); + const key = `${functionName}|${wait.toString()}`; let debounced = this.debounceCache[key]; if (!debounced) { this.debounceCache[key] = debounceFn(fn, { wait }); @@ -27,4 +36,41 @@ export default class DebounceManager { } debounced(...parameters); } + + /** + * Cancels existing debounced function calls. + * + * This will cancel any debounced function call, regardless of the debounce length that was provided. + * + * For example, making the following series of calls will create multiple debounced functions, because + * they are cached by a combination of unique name and debounce length. + * + * runWithDebounce(1000, "_updateSearchResults", this._updateSearchResults) + * runWithDebounce(500, "_updateSearchResults", this._updateSearchResults) + * runWithDebounce(1000, "_updateSearchResults", this._updateSearchResults) + * + * Calling the following will cancel all of those, if they have not yet executed: + * + * cancelByName("_updateSearchResults") + * + * @param {string} functionName The name of the function that was debounced. This needs to match exactly what was provided + * when runWithDebounce was called originally. + */ + cancelByName(functionName) { + Object.entries(this.debounceCache) + .filter(([cachedKey]) => cachedKey.startsWith(`${functionName}|`)) + // eslint-disable-next-line no-unused-vars + .forEach(([_, cachedValue]) => cachedValue.cancel()); + } } +/** + * Perform a standard debounce + * + * @param {number} wait Milliseconds to debounce. Executes immediately if falsey. + * @param {function} fn Function to debounce + */ +DebounceManager.debounce = (wait, fn) => { + return debounceFn(fn, { wait }); +}; + +export default DebounceManager; diff --git a/packages/search-ui/src/SearchDriver.js b/packages/search-ui/src/SearchDriver.js index 2c53937d..b53687b0 100644 --- a/packages/search-ui/src/SearchDriver.js +++ b/packages/search-ui/src/SearchDriver.js @@ -208,14 +208,13 @@ export default class SearchDriver { }); }; - _updateSearchResults = ( - searchParameters, - { skipPushToUrl = false, ignoreIsLoadingCheck = false } = {} - ) => { + /** + * This method is used to update state and trigger a new search. + */ + _updateSearchResults = (searchParameters, { skipPushToUrl = false } = {}) => { const { current, filters, - isLoading, resultsPerPage, searchTerm, sortDirection, @@ -225,19 +224,64 @@ export default class SearchDriver { ...searchParameters }; - if (isLoading && !ignoreIsLoadingCheck) return; + // State updates should always be applied in the order that they are made. This function, _updateSearchResults, + // makes state updates. + // In the case where a call to "_updateSearchResults" was made and delayed for X amount of time using + // `debounceManager.runWithDebounce`, and a subsequent call is made _updateSearchResults before that delay ends, we + // want to make sure that outstanding call to "_updateSearchResults" is cancelled, as it would apply state updates + // out of order. + this.debounceManager.cancelByName("_updateSearchResults"); this._setState({ current, error: "", filters, - isLoading: true, resultsPerPage, searchTerm, sortDirection, sortField }); + this._makeSearchRequest({ + skipPushToUrl + }); + }; + + /** + * This method is separated out from _updateSearchResults so that it + * can be debounced. + * + * By debouncing our API calls, we can effectively allow action chaining. + * + * For Ex: + * + * If a user needs to make multiple filter updates at once, they could + * do so by calling an action 3 times in a row: + * + * addFilter("states", "California"); + * addFilter("states", "Nebraska"); + * addFilter("states", "Pennsylvania"); + * + * We don't want to make 3 separate API calls like that in quick succession, + * so we debounce the API calls. + * + * Application state updates are performed in _updateSearchResults, but we + * wait to make the actual API calls until all actions have been called. + */ + _makeSearchRequest = DebounceManager.debounce(0, ({ skipPushToUrl }) => { + const { + current, + filters, + resultsPerPage, + searchTerm, + sortDirection, + sortField + } = this.state; + + this._setState({ + isLoading: true + }); + const requestId = this.requestSequencer.next(); const queryConfig = { @@ -285,6 +329,7 @@ export default class SearchDriver { // in a live search box for instance. this.debounceManager.runWithDebounce( this.urlPushDebounceLength, + "pushStateToURL", this.URLManager.pushStateToURL.bind(this.URLManager), { current, @@ -303,7 +348,7 @@ export default class SearchDriver { }); } ); - }; + }); _setState(newState) { const state = { ...this.state, ...newState }; diff --git a/packages/search-ui/src/__tests__/DebounceManager.test.js b/packages/search-ui/src/__tests__/DebounceManager.test.js new file mode 100644 index 00000000..266ab79b --- /dev/null +++ b/packages/search-ui/src/__tests__/DebounceManager.test.js @@ -0,0 +1,91 @@ +import DebounceManager from "../DebounceManager"; + +describe("#runWithDebounce", () => { + it("will run the provided function after the provided number of milliseconds", () => { + const manager = new DebounceManager(); + const debounced = jest.fn(); + + manager.runWithDebounce(1000, "debounced", debounced); + + jest.advanceTimersByTime(500); + expect(debounced.mock.calls.length).toBe(0); + jest.advanceTimersByTime(1001); + expect(debounced.mock.calls.length).toBe(1); + }); + + it("will debounce the function", () => { + const manager = new DebounceManager(); + const debounced = jest.fn(); + + manager.runWithDebounce(1000, "debounced", debounced); + + jest.advanceTimersByTime(100); + + manager.runWithDebounce(1000, "debounced", debounced); + + expect(debounced).not.toHaveBeenCalled(); + jest.advanceTimersByTime(1001); + expect(debounced.mock.calls.length).toBe(1); + }); + + it("will will debounce functions with different wait times separately", () => { + const manager = new DebounceManager(); + const debounced = jest.fn(); + + manager.runWithDebounce(1000, "debounced", debounced); + + manager.runWithDebounce(1000, "debounced", debounced); + + manager.runWithDebounce(1000, "debounced", debounced); + + manager.runWithDebounce(999, "debounced", debounced); + + expect(debounced).not.toHaveBeenCalled(); + jest.advanceTimersByTime(1001); + expect(debounced.mock.calls.length).toBe(2); + }); + + it("will will debounce functions with different names separately", () => { + const manager = new DebounceManager(); + const debounced = jest.fn(); + + manager.runWithDebounce(1000, "debounced", debounced); + + manager.runWithDebounce(1000, "debounced", debounced); + + manager.runWithDebounce(1000, "different", debounced); + + manager.runWithDebounce(999, "debounced", debounced); + + expect(debounced).not.toHaveBeenCalled(); + jest.advanceTimersByTime(1001); + expect(debounced.mock.calls.length).toBe(3); + }); +}); + +describe("#cancelByName", () => { + it("cancels debouncing", () => { + const manager = new DebounceManager(); + const debounced = jest.fn(); + + manager.runWithDebounce(1000, "debounced", debounced); + + manager.cancelByName("debounced"); + jest.advanceTimersByTime(1001); + expect(debounced.mock.calls.length).toBe(0); + }); + + it("cancels functions with different times, but not different names", () => { + const manager = new DebounceManager(); + const debounced = jest.fn(); + + manager.runWithDebounce(1000, "debounced", debounced); + manager.runWithDebounce(500, "debounced", debounced); + manager.runWithDebounce(1000, "different", debounced); + manager.runWithDebounce(999, "much different", debounced); + + manager.cancelByName("debounced"); + jest.advanceTimersByTime(1001); + expect(debounced.mock.calls.length).toBe(2); + }); +}); diff --git a/packages/search-ui/src/__tests__/SearchDriver.test.js b/packages/search-ui/src/__tests__/SearchDriver.test.js index 35878503..b1f51e75 100644 --- a/packages/search-ui/src/__tests__/SearchDriver.test.js +++ b/packages/search-ui/src/__tests__/SearchDriver.test.js @@ -171,7 +171,9 @@ describe("searchQuery config", () => { } }); + jest.runAllTimers(); driver.setSearchTerm("test"); + jest.runAllTimers(); } it("will fetch a conditional facet that passes its check", () => { @@ -215,7 +217,9 @@ describe("searchQuery config", () => { } }); + jest.runAllTimers(); driver.setSearchTerm("test"); + jest.runAllTimers(); } it("will pass through facet configuration", () => { @@ -456,3 +460,69 @@ describe("_updateSearchResults", () => { }); }); }); + +describe("When multiple actions are called", () => { + it("Will batch them into a single API call", () => { + const { driver, mockApiConnector } = setupDriver(); + driver.setSearchTerm("term"); + driver.addFilter("field1", "value1"); + driver.addFilter("field2", "value2"); + driver.addFilter("field3", "value3"); + jest.runAllTimers(); + expect(getSearchCalls(mockApiConnector)).toHaveLength(1); + expect(getSearchCalls(mockApiConnector)[0][0].filters).toEqual([ + { + field: "field1", + values: ["value1"], + type: "all" + }, + { + field: "field2", + values: ["value2"], + type: "all" + }, + { + field: "field3", + values: ["value3"], + type: "all" + } + ]); + }); + + describe("setSearchTerm is called with a debounce", () => { + it("The original call which should have been triggered by setSearchTerm should be cancelled.", () => { + const { driver, mockApiConnector } = setupDriver(); + driver.setSearchTerm("park", { refresh: true, debounce: 1000 }); + driver.addFilter("field1", "value1"); + driver.addFilter("field2", "value2"); + driver.addFilter("field3", "value3"); + jest.advanceTimersByTime(500); + + expect(getSearchCalls(mockApiConnector)).toHaveLength(1); + expect(getSearchCalls(mockApiConnector)[0][0].searchTerm).toEqual("park"); + expect(getSearchCalls(mockApiConnector)[0][0].filters).toEqual([ + { + field: "field1", + values: ["value1"], + type: "all" + }, + { + field: "field2", + values: ["value2"], + type: "all" + }, + { + field: "field3", + values: ["value3"], + type: "all" + } + ]); + + jest.runAllTimers(); + // If this case were not working correctly, there would have been a second call here + // which would have cleared out the existing filters, since that is the behavior of a "setSearchTerm" + // action that is debounced by 1000 milliseconds. + expect(getSearchCalls(mockApiConnector)).toHaveLength(1); + }); + }); +}); diff --git a/packages/search-ui/src/__tests__/actions/addFilter.test.js b/packages/search-ui/src/__tests__/actions/addFilter.test.js index 065bb2d5..da3beea7 100644 --- a/packages/search-ui/src/__tests__/actions/addFilter.test.js +++ b/packages/search-ui/src/__tests__/actions/addFilter.test.js @@ -30,6 +30,7 @@ describe("#addFilter", () => { }); driver.addFilter(name, value, type); + jest.runAllTimers(); return updatedStateAfterAction.state; } diff --git a/packages/search-ui/src/__tests__/actions/clearFilters.test.js b/packages/search-ui/src/__tests__/actions/clearFilters.test.js index 3835660f..a2c1e750 100644 --- a/packages/search-ui/src/__tests__/actions/clearFilters.test.js +++ b/packages/search-ui/src/__tests__/actions/clearFilters.test.js @@ -24,6 +24,7 @@ describe("#clearFilters", () => { }); driver.clearFilters(except); + jest.runAllTimers(); return updatedStateAfterAction.state; } diff --git a/packages/search-ui/src/__tests__/actions/removeFilter.test.js b/packages/search-ui/src/__tests__/actions/removeFilter.test.js index 14e98ff9..e1f23d6c 100644 --- a/packages/search-ui/src/__tests__/actions/removeFilter.test.js +++ b/packages/search-ui/src/__tests__/actions/removeFilter.test.js @@ -30,6 +30,7 @@ describe("#removeFilter", () => { }); driver.removeFilter(name, value, type); + jest.runAllTimers(); return updatedStateAfterAction.state; } diff --git a/packages/search-ui/src/__tests__/actions/setCurrent.test.js b/packages/search-ui/src/__tests__/actions/setCurrent.test.js index a3144d35..99b9d77e 100644 --- a/packages/search-ui/src/__tests__/actions/setCurrent.test.js +++ b/packages/search-ui/src/__tests__/actions/setCurrent.test.js @@ -15,6 +15,7 @@ describe("#setCurrent", () => { { initialState } ); driver.setCurrent(current); + jest.runAllTimers(); return { state: updatedStateAfterAction.state, stateAfterCreation: stateAfterCreation diff --git a/packages/search-ui/src/__tests__/actions/setFilter.test.js b/packages/search-ui/src/__tests__/actions/setFilter.test.js index 0aedb600..39c8bd0d 100644 --- a/packages/search-ui/src/__tests__/actions/setFilter.test.js +++ b/packages/search-ui/src/__tests__/actions/setFilter.test.js @@ -30,6 +30,7 @@ describe("#setFilter", () => { }); driver.setFilter(name, value, type); + jest.runAllTimers(); return updatedStateAfterAction.state; } diff --git a/packages/search-ui/src/__tests__/actions/setResultsPerPage.test.js b/packages/search-ui/src/__tests__/actions/setResultsPerPage.test.js index e83d276f..510fd5f3 100644 --- a/packages/search-ui/src/__tests__/actions/setResultsPerPage.test.js +++ b/packages/search-ui/src/__tests__/actions/setResultsPerPage.test.js @@ -15,6 +15,7 @@ describe("#setResultsPerPage", () => { { initialState } ); driver.setResultsPerPage(resultsPerPage); + jest.runAllTimers(); return { state: updatedStateAfterAction.state, stateAfterCreation: stateAfterCreation diff --git a/packages/search-ui/src/__tests__/actions/setSearchTerm.test.js b/packages/search-ui/src/__tests__/actions/setSearchTerm.test.js index 00eaa936..8db88d7d 100644 --- a/packages/search-ui/src/__tests__/actions/setSearchTerm.test.js +++ b/packages/search-ui/src/__tests__/actions/setSearchTerm.test.js @@ -1,9 +1,4 @@ -import { - getAutocompleteCalls, - getSearchCalls, - setupDriver, - waitABit -} from "../../test/helpers"; +import { getAutocompleteCalls, setupDriver } from "../../test/helpers"; import { itResetsCurrent, itResetsFilters, @@ -26,7 +21,8 @@ describe("#setSearchTerm", () => { autocompleteResults, autocompleteSuggestions, refresh, - initialState = {} + initialState = {}, + shouldClearFilters } = {} ) { const { driver, stateAfterCreation, updatedStateAfterAction } = setupDriver( @@ -35,8 +31,12 @@ describe("#setSearchTerm", () => { driver.setSearchTerm(term, { autocompleteResults, autocompleteSuggestions, - refresh + refresh, + shouldClearFilters }); + + jest.runAllTimers(); + return { state: updatedStateAfterAction.state, stateAfterCreation: stateAfterCreation @@ -70,6 +70,28 @@ describe("#setSearchTerm", () => { }).state ); + it("Does not update filters when 'shouldClearFilters' is set to false", () => { + const state = subject("test", { + initialState: { + filters: [ + { + field: "filter1", + values: ["value1"], + type: "all" + } + ] + }, + shouldClearFilters: false + }).state; + expect(state.filters).toEqual([ + { + field: "filter1", + values: ["value1"], + type: "all" + } + ]); + }); + it("Does not update other Search Parameter values", () => { const initialState = { resultsPerPage: 60, @@ -100,21 +122,33 @@ describe("#setSearchTerm", () => { expect(subject("term", { refresh: false }).state.wasSearched).toBe(false); }); - it("Will not debounce requests if there is no debounce specified", async () => { - const { driver, mockApiConnector } = setupDriver(); - driver.setSearchTerm("term", { refresh: true }); - driver.setSearchTerm("term", { refresh: true }); - driver.setSearchTerm("term", { refresh: true }); - expect(getSearchCalls(mockApiConnector)).toHaveLength(3); + it("Will debounce the request state update if a debounce is specified", () => { + const { driver, updatedStateAfterAction } = setupDriver({ + initialState: { + searchTerm: "park", + current: 2 + } + }); + jest.runAllTimers(); + driver.setSearchTerm("term", { debounce: 1000 }); + expect(updatedStateAfterAction.state.current).toBe(2); + jest.advanceTimersByTime(500); + expect(updatedStateAfterAction.state.current).toBe(2); + jest.advanceTimersByTime(500); + expect(updatedStateAfterAction.state.current).toBe(1); }); - it("Will debounce requests", async () => { - const { driver, mockApiConnector } = setupDriver(); - driver.setSearchTerm("term", { refresh: true, debounce: 10 }); - driver.setSearchTerm("term", { refresh: true, debounce: 10 }); - driver.setSearchTerm("term", { refresh: true, debounce: 10 }); - await waitABit(100); - expect(getSearchCalls(mockApiConnector)).toHaveLength(1); + it("Will not debounce the request state update if no debounce is specified", () => { + const { driver, updatedStateAfterAction } = setupDriver({ + initialState: { + searchTerm: "park", + current: 2 + } + }); + jest.runAllTimers(); + expect(updatedStateAfterAction.state.current).toBe(2); + driver.setSearchTerm("term"); + expect(updatedStateAfterAction.state.current).toBe(1); }); describe("when autocompleteResults is true", () => { @@ -125,7 +159,7 @@ describe("#setSearchTerm", () => { ).toEqual([{}, {}]); }); - it("Will not debounce requests if there is no debounce specified", async () => { + it("Will not debounce requests if there is no debounce specified", () => { const { driver, mockApiConnector } = setupDriver(); driver.setSearchTerm("term", { autocompleteResults: true, @@ -142,7 +176,7 @@ describe("#setSearchTerm", () => { expect(getAutocompleteCalls(mockApiConnector)).toHaveLength(3); }); - it("Will debounce requests", async () => { + it("Will debounce requests", () => { const { driver, mockApiConnector } = setupDriver(); driver.setSearchTerm("term", { autocompleteResults: true, @@ -159,7 +193,7 @@ describe("#setSearchTerm", () => { debounce: 10, refresh: false }); - await waitABit(100); + jest.runAllTimers(); expect(getAutocompleteCalls(mockApiConnector)).toHaveLength(1); }); @@ -201,7 +235,8 @@ describe("#setSearchTerm", () => { ] }); }); - it("Will not debounce requests if there is no debounce specified", async () => { + + it("Will not debounce requests if there is no debounce specified", () => { const { driver, mockApiConnector } = setupDriver(); driver.setSearchTerm("term", { autocompleteSuggestions: true, @@ -215,9 +250,11 @@ describe("#setSearchTerm", () => { autocompleteSuggestions: true, refresh: false }); + jest.runAllTimers(); expect(getAutocompleteCalls(mockApiConnector)).toHaveLength(3); }); - it("Will debounce requests", async () => { + + it("Will debounce requests", () => { const { driver, mockApiConnector } = setupDriver(); driver.setSearchTerm("term", { autocompleteSuggestions: true, @@ -234,7 +271,7 @@ describe("#setSearchTerm", () => { debounce: 10, refresh: false }); - await waitABit(100); + jest.runAllTimers(); expect(getAutocompleteCalls(mockApiConnector)).toHaveLength(1); }); diff --git a/packages/search-ui/src/__tests__/actions/setSort.test.js b/packages/search-ui/src/__tests__/actions/setSort.test.js index 63496b8b..b33e9732 100644 --- a/packages/search-ui/src/__tests__/actions/setSort.test.js +++ b/packages/search-ui/src/__tests__/actions/setSort.test.js @@ -15,6 +15,7 @@ describe("#setSort", () => { { initialState } ); driver.setSort(sortField, sortDirection); + jest.runAllTimers(); return { state: updatedStateAfterAction.state, stateAfterCreation: stateAfterCreation diff --git a/packages/search-ui/src/__tests__/actions/trackClickThrough.test.js b/packages/search-ui/src/__tests__/actions/trackClickThrough.test.js index 5911b619..a9c539da 100644 --- a/packages/search-ui/src/__tests__/actions/trackClickThrough.test.js +++ b/packages/search-ui/src/__tests__/actions/trackClickThrough.test.js @@ -4,6 +4,7 @@ describe("#trackClickThrough", () => { function subject({ initialState } = {}, documentId, tags) { const { driver, mockApiConnector } = setupDriver({ initialState }); driver.trackClickThrough(documentId, tags); + jest.runAllTimers(); return { driver, mockApiConnector }; } diff --git a/packages/search-ui/src/actions/setSearchTerm.js b/packages/search-ui/src/actions/setSearchTerm.js index 1661dd1e..0003160c 100644 --- a/packages/search-ui/src/actions/setSearchTerm.js +++ b/packages/search-ui/src/actions/setSearchTerm.js @@ -18,6 +18,7 @@ export default function setSearchTerm( autocompleteMinimumCharacters = 0, autocompleteResults = false, autocompleteSuggestions = false, + shouldClearFilters = true, refresh = true, debounce = 0 } = {} @@ -30,12 +31,12 @@ export default function setSearchTerm( if (refresh) { this.debounceManager.runWithDebounce( debounce, + "_updateSearchResults", this._updateSearchResults, { current: 1, - filters: [] - }, - { ignoreIsLoadingCheck: true } + ...(shouldClearFilters && { filters: [] }) + } ); } @@ -45,6 +46,7 @@ export default function setSearchTerm( ) { this.debounceManager.runWithDebounce( debounce, + "_updateAutocomplete", this._updateAutocomplete, searchTerm, { diff --git a/packages/search-ui/src/setupTests.js b/packages/search-ui/src/setupTests.js new file mode 100644 index 00000000..ac7fb769 --- /dev/null +++ b/packages/search-ui/src/setupTests.js @@ -0,0 +1,3 @@ +// We use fake timers to avoid having to use async programming +// in our tests. +jest.useFakeTimers(); diff --git a/packages/search-ui/src/test/helpers.js b/packages/search-ui/src/test/helpers.js index 4870f027..17c7603d 100644 --- a/packages/search-ui/src/test/helpers.js +++ b/packages/search-ui/src/test/helpers.js @@ -66,6 +66,8 @@ export function setupDriver({ mockSearchResponse, ...rest } = {}) { updatedStateAfterAction.state = newState; }); + jest.runAllTimers(); + return { stateAfterCreation: driver.getState(), driver, @@ -92,12 +94,6 @@ export function doesStateHaveResponseData(response) { ); } -export function waitABit(length) { - return new Promise(function(resolve) { - setTimeout(() => resolve(), length); - }); -} - export function getSearchCalls(mockApiConnector) { return mockApiConnector.onSearch.mock.calls; }