From e782ab85c348c7c3954eaf113245abebc3885029 Mon Sep 17 00:00:00 2001 From: Alex S Date: Mon, 28 May 2018 10:28:41 +0200 Subject: [PATCH] fix(clear-all): apply excludeAttribute correctly with clearsQuery (#2935) * fix(clear-all): apply excludeAttribute correctly This fix actually contains a rewrite of the underlying functions that will compute the list of attributes to clear. The complete required behaviour was spread both in the connectors and the utility functions. In order to simplify the code and make it easier to test, this refactoring was needed. A next step for this is to use a single connnector for current refined values and clear all widgets. * refactor(connectClearAll-test): move to jest * refactor(connectCurrentRefinedValues-test): move to jest * refactor(utils): use [].forEach --- .../__tests__/connectClearAll-test.js | 261 +++++++-- src/connectors/clear-all/connectClearAll.js | 96 ++- .../connectCurrentRefinedValues-test.js | 34 +- .../connectCurrentRefinedValues.js | 39 +- src/lib/__tests__/utils-test.js | 546 +++++++----------- src/lib/utils.js | 85 +-- .../clear-all/__tests__/clear-all-test.js | 2 +- .../__tests__/current-refined-values-test.js | 6 +- 8 files changed, 579 insertions(+), 490 deletions(-) diff --git a/src/connectors/clear-all/__tests__/connectClearAll-test.js b/src/connectors/clear-all/__tests__/connectClearAll-test.js index 4e8500b68b..9819601e9b 100644 --- a/src/connectors/clear-all/__tests__/connectClearAll-test.js +++ b/src/connectors/clear-all/__tests__/connectClearAll-test.js @@ -1,5 +1,3 @@ -import sinon from 'sinon'; - import jsHelper from 'algoliasearch-helper'; const SearchResults = jsHelper.SearchResults; @@ -8,10 +6,10 @@ import connectClearAll from '../connectClearAll.js'; describe('connectClearAll', () => { it('Renders during init and render', () => { const helper = jsHelper({}); - helper.search = sinon.stub(); + helper.search = () => {}; // test that the dummyRendering is called with the isFirstRendering // flag set accordingly - const rendering = sinon.stub(); + const rendering = jest.fn(); const makeWidget = connectClearAll(rendering); const widget = makeWidget({ foo: 'bar', // dummy param to test `widgetParams` @@ -19,7 +17,7 @@ describe('connectClearAll', () => { expect(widget.getConfiguration).toBe(undefined); // test if widget is not rendered yet at this point - expect(rendering.callCount).toBe(0); + expect(rendering).toHaveBeenCalledTimes(0); widget.init({ helper, @@ -29,11 +27,11 @@ describe('connectClearAll', () => { }); // test that rendering has been called during init with isFirstRendering = true - expect(rendering.callCount).toBe(1); + expect(rendering).toHaveBeenCalledTimes(1); // test if isFirstRendering is true during init - expect(rendering.lastCall.args[1]).toBe(true); + expect(rendering.mock.calls[0][1]).toBe(true); - const firstRenderingOptions = rendering.lastCall.args[0]; + const firstRenderingOptions = rendering.mock.calls[0][0]; expect(firstRenderingOptions.hasRefinements).toBe(false); expect(firstRenderingOptions.widgetParams).toEqual({ foo: 'bar', // dummy param to test `widgetParams` @@ -47,10 +45,10 @@ describe('connectClearAll', () => { }); // test that rendering has been called during init with isFirstRendering = false - expect(rendering.callCount).toBe(2); - expect(rendering.lastCall.args[1]).toBe(false); + expect(rendering).toHaveBeenCalledTimes(2); + expect(rendering.mock.calls[1][1]).toBe(false); - const secondRenderingOptions = rendering.lastCall.args[0]; + const secondRenderingOptions = rendering.mock.calls[1][0]; expect(secondRenderingOptions.hasRefinements).toBe(false); }); @@ -61,11 +59,11 @@ describe('connectClearAll', () => { const helper = jsHelper({}, '', { facets: ['myFacet'], }); - helper.search = sinon.stub(); + helper.search = () => {}; helper.setQuery('not empty'); helper.toggleRefinement('myFacet', 'myValue'); - const rendering = sinon.stub(); + const rendering = jest.fn(); const makeWidget = connectClearAll(rendering); const widget = makeWidget({ clearsQuery: false }); @@ -78,7 +76,7 @@ describe('connectClearAll', () => { expect(helper.hasRefinements('myFacet')).toBe(true); expect(helper.state.query).toBe('not empty'); - const initClearMethod = rendering.lastCall.args[0].refine; + const initClearMethod = rendering.mock.calls[0][0].refine; initClearMethod(); expect(helper.hasRefinements('myFacet')).toBe(false); @@ -95,7 +93,7 @@ describe('connectClearAll', () => { expect(helper.hasRefinements('myFacet')).toBe(true); expect(helper.state.query).toBe('not empty'); - const renderClearMethod = rendering.lastCall.args[0].refine; + const renderClearMethod = rendering.mock.calls[1][0].refine; renderClearMethod(); expect(helper.hasRefinements('myFacet')).toBe(false); expect(helper.state.query).toBe('not empty'); @@ -108,11 +106,11 @@ describe('connectClearAll', () => { const helper = jsHelper({}, '', { facets: ['myFacet'], }); - helper.search = sinon.stub(); + helper.search = () => {}; helper.setQuery('a query'); helper.toggleRefinement('myFacet', 'myValue'); - const rendering = sinon.stub(); + const rendering = jest.fn(); const makeWidget = connectClearAll(rendering); const widget = makeWidget({ clearsQuery: true }); @@ -125,7 +123,7 @@ describe('connectClearAll', () => { expect(helper.hasRefinements('myFacet')).toBe(true); expect(helper.state.query).toBe('a query'); - const initClearMethod = rendering.lastCall.args[0].refine; + const initClearMethod = rendering.mock.calls[0][0].refine; initClearMethod(); expect(helper.hasRefinements('myFacet')).toBe(false); @@ -143,22 +141,22 @@ describe('connectClearAll', () => { expect(helper.hasRefinements('myFacet')).toBe(true); expect(helper.state.query).toBe('another query'); - const renderClearMethod = rendering.lastCall.args[0].refine; + const renderClearMethod = rendering.mock.calls[1][0].refine; renderClearMethod(); expect(helper.hasRefinements('myFacet')).toBe(false); expect(helper.state.query).toBe(''); }); - it('some refinements from results <=> hasRefinements = true', () => { + it('some refinements from results <-> hasRefinements = true', () => { // test if the values sent to the rendering function // are consistent with the search state const helper = jsHelper({}, undefined, { facets: ['aFacet'], }); helper.toggleRefinement('aFacet', 'some value'); - helper.search = sinon.stub(); + helper.search = () => {}; - const rendering = sinon.stub(); + const rendering = jest.fn(); const makeWidget = connectClearAll(rendering); const widget = makeWidget(); @@ -169,7 +167,7 @@ describe('connectClearAll', () => { onHistoryChange: () => {}, }); - expect(rendering.lastCall.args[0].hasRefinements).toBe(true); + expect(rendering.mock.calls[0][0].hasRefinements).toBe(true); widget.render({ results: new SearchResults(helper.state, [{}]), @@ -178,19 +176,19 @@ describe('connectClearAll', () => { createURL: () => '#', }); - expect(rendering.lastCall.args[0].hasRefinements).toBe(true); + expect(rendering.mock.calls[1][0].hasRefinements).toBe(true); }); - it('(clearsQuery: true) query not empty <=> hasRefinements = true', () => { + it('(clearsQuery: true) query not empty <-> hasRefinements = true', () => { // test if the values sent to the rendering function // are consistent with the search state const helper = jsHelper({}, undefined, { facets: ['aFacet'], }); helper.setQuery('no empty'); - helper.search = sinon.stub(); + helper.search = () => {}; - const rendering = sinon.stub(); + const rendering = jest.fn(); const makeWidget = connectClearAll(rendering); const widget = makeWidget({ clearsQuery: true, @@ -203,7 +201,7 @@ describe('connectClearAll', () => { onHistoryChange: () => {}, }); - expect(rendering.lastCall.args[0].hasRefinements).toBe(true); + expect(rendering.mock.calls[0][0].hasRefinements).toBe(true); widget.render({ results: new SearchResults(helper.state, [{}]), @@ -212,7 +210,40 @@ describe('connectClearAll', () => { createURL: () => '#', }); - expect(rendering.lastCall.args[0].hasRefinements).toBe(true); + expect(rendering.mock.calls[1][0].hasRefinements).toBe(true); + }); + + it('(clearsQuery: true) no refinements <-> hasRefinements = false', () => { + // test if the values sent to the rendering function + // are consistent with the search state + const helper = jsHelper({}, undefined, { + facets: ['aFacet'], + }); + helper.search = () => {}; + + const rendering = jest.fn(); + const makeWidget = connectClearAll(rendering); + const widget = makeWidget({ + clearsQuery: true, + }); + + widget.init({ + helper, + state: helper.state, + createURL: () => '#', + onHistoryChange: () => {}, + }); + + expect(rendering.mock.calls[0][0].hasRefinements).toBe(false); + + widget.render({ + results: new SearchResults(helper.state, [{}]), + state: helper.state, + helper, + createURL: () => '#', + }); + + expect(rendering.mock.calls[1][0].hasRefinements).toBe(false); }); it('(clearsQuery: false) no refinements <=> hasRefinements = false', () => { @@ -221,9 +252,9 @@ describe('connectClearAll', () => { const helper = jsHelper({}); helper.setQuery('not empty'); - helper.search = sinon.stub(); + helper.search = () => {}; - const rendering = sinon.stub(); + const rendering = jest.fn(); const makeWidget = connectClearAll(rendering); const widget = makeWidget({ clearsQuery: false }); @@ -234,7 +265,7 @@ describe('connectClearAll', () => { onHistoryChange: () => {}, }); - expect(rendering.lastCall.args[0].hasRefinements).toBe(false); + expect(rendering.mock.calls[0][0].hasRefinements).toBe(false); widget.render({ results: new SearchResults(helper.state, [{}]), @@ -243,6 +274,170 @@ describe('connectClearAll', () => { createURL: () => '#', }); - expect(rendering.lastCall.args[0].hasRefinements).toBe(false); + expect(rendering.mock.calls[1][0].hasRefinements).toBe(false); + }); + + it('can exclude some attributes', () => { + const helper = jsHelper({ addAlgoliaAgent: () => {} }, '', { + facets: ['facet'], + }); + helper.search = () => {}; + + const rendering = jest.fn(); + const makeWidget = connectClearAll(rendering); + const widget = makeWidget({ + excludeAttributes: ['facet'], + }); + + helper.toggleRefinement('facet', 'value'); + + { + helper.setQuery('not empty'); + + widget.init({ + helper, + state: helper.state, + createURL: () => '#', + onHistoryChange: () => {}, + }); + + expect(helper.hasRefinements('facet')).toBe(true); + + const refine = rendering.mock.calls[0][0].refine; + refine(); + + expect(helper.hasRefinements('facet')).toBe(true); + } + + { + // facet has not been cleared and it is still refined with value + helper.setQuery('not empty'); + + widget.render({ + helper, + state: helper.state, + results: new SearchResults(helper.state, [{}]), + createURL: () => '#', + onHistoryChange: () => {}, + }); + + expect(helper.hasRefinements('facet')).toBe(true); + + const refine = rendering.mock.calls[1][0].refine; + refine(); + + expect(helper.hasRefinements('facet')).toBe(true); + } + }); + + it('can exclude some attributes when clearsQuery is active', () => { + const helper = jsHelper({ addAlgoliaAgent: () => {} }, '', { + facets: ['facet'], + }); + helper.search = () => {}; + + const rendering = jest.fn(); + const makeWidget = connectClearAll(rendering); + const widget = makeWidget({ + excludeAttributes: ['facet'], + clearsQuery: true, + }); + + helper.toggleRefinement('facet', 'value'); + + { + helper.setQuery('not empty'); + + widget.init({ + helper, + state: helper.state, + createURL: () => '#', + onHistoryChange: () => {}, + }); + + expect(helper.hasRefinements('facet')).toBe(true); + + const refine = rendering.mock.calls[0][0].refine; + refine(); + + expect(helper.hasRefinements('facet')).toBe(true); + } + + { + helper.setQuery('not empty'); + + widget.render({ + helper, + state: helper.state, + results: new SearchResults(helper.state, [{}]), + createURL: () => '#', + onHistoryChange: () => {}, + }); + + expect(helper.hasRefinements('facet')).toBe(true); + + const refine = rendering.mock.calls[1][0].refine; + refine(); + + expect(helper.hasRefinements('facet')).toBe(true); + } + }); + + describe('createURL', () => { + it('consistent with the list of excludedAttributes', () => { + const helper = jsHelper({ addAlgoliaAgent: () => {} }, '', { + facets: ['facet', 'otherFacet'], + }); + helper.search = () => {}; + + const rendering = jest.fn(); + const makeWidget = connectClearAll(rendering); + const widget = makeWidget({ + excludeAttributes: ['facet'], + clearsQuery: true, + }); + + helper.toggleRefinement('facet', 'value'); + helper.toggleRefinement('otherFacet', 'value'); + + { + helper.setQuery('not empty'); + + widget.init({ + helper, + state: helper.state, + createURL: opts => opts, + onHistoryChange: () => {}, + }); + + const { createURL, refine } = rendering.mock.calls[0][0]; + + // The state represented by the URL should be equal to a state + // after refining. + const createURLState = createURL(); + refine(); + const stateAfterRefine = helper.state; + + expect(createURLState).toEqual(stateAfterRefine); + } + + { + widget.render({ + helper, + state: helper.state, + results: new SearchResults(helper.state, [{}]), + createURL: () => '#', + onHistoryChange: () => {}, + }); + + const { createURL, refine } = rendering.mock.calls[1][0]; + + const createURLState = createURL(); + refine(); + const stateAfterRefine = helper.state; + + expect(createURLState).toEqual(stateAfterRefine); + } + }); }); }); diff --git a/src/connectors/clear-all/connectClearAll.js b/src/connectors/clear-all/connectClearAll.js index a16dc49914..994a6265b9 100644 --- a/src/connectors/clear-all/connectClearAll.js +++ b/src/connectors/clear-all/connectClearAll.js @@ -1,8 +1,7 @@ import { checkRendering, - getRefinements, - clearRefinementsFromState, - clearRefinementsAndSearch, + clearRefinements, + getAttributesToClear, } from '../../lib/utils.js'; const usage = `Usage: @@ -24,17 +23,6 @@ search.addWidget( Full documentation available at https://community.algolia.com/instantsearch.js/v2/connectors/connectClearAll.html `; -const refine = ({ - helper, - clearAttributes, - hasRefinements, - clearsQuery, -}) => () => { - if (hasRefinements) { - clearRefinementsAndSearch(helper, clearAttributes, clearsQuery); - } -}; - /** * @typedef {Object} CustomClearAllWidgetOptions * @property {string[]} [excludeAttributes = []] Every attributes that should not be removed when calling `refine()`. @@ -98,38 +86,42 @@ export default function connectClearAll(renderFn, unmountFn) { const { excludeAttributes = [], clearsQuery = false } = widgetParams; return { - // Provide the same function to the `renderFn` so that way the user - // has to only bind it once when `isFirstRendering` for instance - _refine() {}, - _cachedRefine() { - this._refine(); - }, - init({ helper, instantSearchInstance, createURL }) { - this._cachedRefine = this._cachedRefine.bind(this); - - const clearAttributes = getRefinements({}, helper.state) - .map(one => one.attributeName) - .filter(one => excludeAttributes.indexOf(one) === -1); - - const hasRefinements = clearsQuery - ? clearAttributes.length !== 0 || helper.state.query !== '' - : clearAttributes.length !== 0; - const preparedCreateURL = () => - createURL(clearRefinementsFromState(helper.state, [], clearsQuery)); - - this._refine = refine({ + const attributesToClear = getAttributesToClear({ helper, - clearAttributes, - hasRefinements, - clearsQuery, + blackList: excludeAttributes, }); + const hasRefinements = clearsQuery + ? attributesToClear.length !== 0 || helper.state.query !== '' + : attributesToClear.length !== 0; + + this._refine = () => { + helper + .setState( + clearRefinements({ + helper, + blackList: excludeAttributes, + clearsQuery, + }) + ) + .search(); + }; + + this._createURL = () => + createURL( + clearRefinements({ + helper, + blackList: excludeAttributes, + clearsQuery, + }) + ); + renderFn( { - refine: this._cachedRefine, + refine: this._refine, hasRefinements, - createURL: preparedCreateURL, + createURL: this._createURL, instantSearchInstance, widgetParams, }, @@ -137,29 +129,21 @@ export default function connectClearAll(renderFn, unmountFn) { ); }, - render({ results, state, createURL, helper, instantSearchInstance }) { - const clearAttributes = getRefinements(results, state) - .map(one => one.attributeName) - .filter(one => excludeAttributes.indexOf(one) === -1); - - const hasRefinements = clearsQuery - ? clearAttributes.length !== 0 || helper.state.query !== '' - : clearAttributes.length !== 0; - const preparedCreateURL = () => - createURL(clearRefinementsFromState(state, [], clearsQuery)); - - this._refine = refine({ + render({ helper, instantSearchInstance }) { + const attributesToClear = getAttributesToClear({ helper, - clearAttributes, - hasRefinements, - clearsQuery, + blackList: excludeAttributes, }); + const hasRefinements = clearsQuery + ? attributesToClear.length !== 0 || helper.state.query !== '' + : attributesToClear.length !== 0; + renderFn( { - refine: this._cachedRefine, + refine: this._refine, hasRefinements, - createURL: preparedCreateURL, + createURL: this._createURL, instantSearchInstance, widgetParams, }, diff --git a/src/connectors/current-refined-values/__tests__/connectCurrentRefinedValues-test.js b/src/connectors/current-refined-values/__tests__/connectCurrentRefinedValues-test.js index 3510acabef..2384896ebc 100644 --- a/src/connectors/current-refined-values/__tests__/connectCurrentRefinedValues-test.js +++ b/src/connectors/current-refined-values/__tests__/connectCurrentRefinedValues-test.js @@ -1,4 +1,3 @@ -import sinon from 'sinon'; import jsHelper from 'algoliasearch-helper'; const SearchResults = jsHelper.SearchResults; import connectCurrentRefinedValues from '../connectCurrentRefinedValues.js'; @@ -6,10 +5,10 @@ import connectCurrentRefinedValues from '../connectCurrentRefinedValues.js'; describe('connectCurrentRefinedValues', () => { it('Renders during init and render', () => { const helper = jsHelper({}); - helper.search = sinon.stub(); + helper.search = () => {}; // test that the dummyRendering is called with the isFirstRendering // flag set accordingly - const rendering = sinon.stub(); + const rendering = jest.fn(); const makeWidget = connectCurrentRefinedValues(rendering); const widget = makeWidget({ foo: 'bar', // dummy param to test `widgetParams` @@ -17,7 +16,7 @@ describe('connectCurrentRefinedValues', () => { expect(widget.getConfiguration).toBe(undefined); // test if widget is not rendered yet at this point - expect(rendering.callCount).toBe(0); + expect(rendering).toHaveBeenCalledTimes(0); widget.init({ helper, @@ -27,11 +26,11 @@ describe('connectCurrentRefinedValues', () => { }); // test that rendering has been called during init with isFirstRendering = true - expect(rendering.callCount).toBe(1); + expect(rendering).toHaveBeenCalledTimes(1); // test if isFirstRendering is true during init - expect(rendering.lastCall.args[1]).toBe(true); + expect(rendering.mock.calls[0][1]).toBe(true); - const firstRenderingOptions = rendering.lastCall.args[0]; + const firstRenderingOptions = rendering.mock.calls[0][0]; expect(firstRenderingOptions.refinements).toEqual([]); expect(firstRenderingOptions.widgetParams).toEqual({ foo: 'bar', @@ -45,10 +44,10 @@ describe('connectCurrentRefinedValues', () => { }); // test that rendering has been called during init with isFirstRendering = false - expect(rendering.callCount).toBe(2); - expect(rendering.lastCall.args[1]).toBe(false); + expect(rendering).toHaveBeenCalledTimes(2); + expect(rendering.mock.calls[1][1]).toBe(false); - const secondRenderingOptions = rendering.lastCall.args[0]; + const secondRenderingOptions = rendering.mock.calls[0][0]; expect(secondRenderingOptions.refinements).toEqual([]); expect(secondRenderingOptions.widgetParams).toEqual({ foo: 'bar', @@ -61,8 +60,8 @@ describe('connectCurrentRefinedValues', () => { const helper = jsHelper({}, '', { facets: ['myFacet'], }); - helper.search = sinon.stub(); - const rendering = sinon.stub(); + helper.search = () => {}; + const rendering = jest.fn(); const makeWidget = connectCurrentRefinedValues(rendering); const widget = makeWidget(); @@ -75,7 +74,7 @@ describe('connectCurrentRefinedValues', () => { onHistoryChange: () => {}, }); - const firstRenderingOptions = rendering.lastCall.args[0]; + const firstRenderingOptions = rendering.mock.calls[0][0]; const refinements = firstRenderingOptions.refinements; expect(typeof firstRenderingOptions.refine).toBe('function'); expect(refinements).toHaveLength(1); @@ -91,7 +90,7 @@ describe('connectCurrentRefinedValues', () => { createURL: () => '#', }); - const secondRenderingOptions = rendering.lastCall.args[0]; + const secondRenderingOptions = rendering.mock.calls[1][0]; const otherRefinements = secondRenderingOptions.refinements; expect(typeof secondRenderingOptions.refine).toBe('function'); expect(otherRefinements).toHaveLength(1); @@ -100,7 +99,9 @@ describe('connectCurrentRefinedValues', () => { }); it('should clear also the search query', () => { - const helper = jsHelper({}, '', {}); + const helper = jsHelper({}, '', { + facets: ['myFacet'], + }); helper.search = jest.fn(); const rendering = jest.fn(); @@ -108,6 +109,7 @@ describe('connectCurrentRefinedValues', () => { const widget = makeWidget({ clearsQuery: true }); helper.setQuery('foobar'); + helper.toggleRefinement('myFacet', 'value'); expect(helper.state.query).toBe('foobar'); widget.init({ @@ -119,12 +121,14 @@ describe('connectCurrentRefinedValues', () => { // clear current refined values + query expect(rendering).toBeCalled(); + expect(helper.hasRefinements('myFacet')).toBe(true); const [{ clearAllClick }] = rendering.mock.calls[0]; clearAllClick(); expect(helper.search).toBeCalled(); expect(helper.state.query).toBe(''); + expect(helper.hasRefinements('myFacet')).toBe(false); }); it('should provide the query as a refinement if clearsQuery is true', () => { diff --git a/src/connectors/current-refined-values/connectCurrentRefinedValues.js b/src/connectors/current-refined-values/connectCurrentRefinedValues.js index 5213487254..a36be3a0e6 100644 --- a/src/connectors/current-refined-values/connectCurrentRefinedValues.js +++ b/src/connectors/current-refined-values/connectCurrentRefinedValues.js @@ -12,8 +12,7 @@ import filter from 'lodash/filter'; import { getRefinements, - clearRefinementsFromState, - clearRefinementsAndSearch, + clearRefinements, checkRendering, } from '../../lib/utils.js'; @@ -187,7 +186,7 @@ export default function connectCurrentRefinedValues(renderFn, unmountFn) { } const attributeNames = map(attributes, attribute => attribute.name); - const restrictedTo = onlyListedAttributes ? attributeNames : []; + const restrictedTo = onlyListedAttributes ? attributeNames : undefined; const attributesObj = reduce( attributes, @@ -200,16 +199,22 @@ export default function connectCurrentRefinedValues(renderFn, unmountFn) { return { init({ helper, createURL, instantSearchInstance }) { - this._clearRefinementsAndSearch = clearRefinementsAndSearch.bind( - null, - helper, - restrictedTo, - clearsQuery - ); - - const clearAllURL = createURL( - clearRefinementsFromState(helper.state, restrictedTo, clearsQuery) - ); + this._clearRefinementsAndSearch = () => { + helper + .setState( + clearRefinements({ + helper, + whiteList: restrictedTo, + clearsQuery, + }) + ) + .search(); + }; + + this._createClearAllURL = () => + createURL( + clearRefinements({ helper, whiteList: restrictedTo, clearsQuery }) + ); const refinements = getFilteredRefinements( {}, @@ -228,7 +233,7 @@ export default function connectCurrentRefinedValues(renderFn, unmountFn) { { attributes: attributesObj, clearAllClick: this._clearRefinementsAndSearch, - clearAllURL, + clearAllURL: this._createClearAllURL(), refine: _clearRefinement, createURL: _createURL, refinements, @@ -240,10 +245,6 @@ export default function connectCurrentRefinedValues(renderFn, unmountFn) { }, render({ results, helper, state, createURL, instantSearchInstance }) { - const clearAllURL = createURL( - clearRefinementsFromState(state, restrictedTo, clearsQuery) - ); - const refinements = getFilteredRefinements( results, state, @@ -261,7 +262,7 @@ export default function connectCurrentRefinedValues(renderFn, unmountFn) { { attributes: attributesObj, clearAllClick: this._clearRefinementsAndSearch, - clearAllURL, + clearAllURL: this._createClearAllURL(), refine: _clearRefinement, createURL: _createURL, refinements, diff --git a/src/lib/__tests__/utils-test.js b/src/lib/__tests__/utils-test.js index aa2e97e107..2250d65877 100644 --- a/src/lib/__tests__/utils-test.js +++ b/src/lib/__tests__/utils-test.js @@ -1,6 +1,5 @@ import algoliasearchHelper from 'algoliasearch-helper'; import * as utils from '../utils'; -import isEmpty from 'lodash/isEmpty'; describe('utils.getContainerNode', () => { it('should be able to get a node from a node', () => { @@ -866,336 +865,6 @@ describe('utils.getRefinements', () => { }); }); -describe('utils.clearRefinementsFromState', () => { - let helper; - let state; - - beforeEach(() => { - helper = algoliasearchHelper({}, 'my_index', { - facets: ['facet1', 'facet2', 'numericFacet1', 'facetExclude1'], - disjunctiveFacets: ['disjunctiveFacet1', 'numericDisjunctiveFacet'], - hierarchicalFacets: [ - { - name: 'hierarchicalFacet1', - attributes: ['hierarchicalFacet1.lvl0', 'hierarchicalFacet1.lvl1'], - separator: ' > ', - }, - ], - }); - helper - .toggleRefinement('facet1', 'facet1val1') - .toggleRefinement('facet1', 'facet1val2') - .toggleRefinement('facet2', 'facet2val1') - .toggleRefinement('facet2', 'facet2val2') - .toggleRefinement('disjunctiveFacet1', 'facet1val1') - .toggleRefinement('disjunctiveFacet1', 'facet1val2') - .toggleExclude('facetExclude1', 'facetExclude1val1') - .toggleExclude('facetExclude1', 'facetExclude1val2') - .addNumericRefinement('numericFacet1', '>', '1') - .addNumericRefinement('numericFacet1', '>', '2') - .addNumericRefinement('numericDisjunctiveFacet1', '>', '1') - .addNumericRefinement('numericDisjunctiveFacet1', '>', '2') - .toggleRefinement('hierarchicalFacet1', 'hierarchicalFacet1lvl0val1') - .addTag('tag1') - .addTag('tag2'); - state = helper.state; - }); - - describe('without arguments', () => { - it('should clear everything', () => { - const newState = utils.clearRefinementsFromState(state); - expect(isEmpty(newState.facetsRefinements)).toBe( - true, - "state shouldn't have facetsRefinements" - ); - expect(isEmpty(newState.facetsExcludes)).toBe( - true, - "state shouldn't have facetsExcludes" - ); - expect(isEmpty(newState.disjunctiveFacetsRefinements)).toBe( - true, - "state shouldn't have disjunctiveFacetsRefinements" - ); - expect(isEmpty(newState.hierarchicalFacetsRefinements)).toBe( - true, - "state shouldn't have hierarchicalFacetsRefinements" - ); - expect(isEmpty(newState.numericRefinements)).toBe( - true, - "state shouldn't have numericRefinements" - ); - expect(isEmpty(newState.tagRefinements)).toBe( - true, - "state shouldn't have tagRefinements" - ); - }); - }); - - it('should clear one facet', () => { - const newState = utils.clearRefinementsFromState(state, ['facet1']); - expect(isEmpty(newState.facetsRefinements)).toBe( - false, - 'state should have facetsRefinements' - ); - expect(isEmpty(newState.facetsExcludes)).toBe( - false, - 'state should have facetsExcludes' - ); - expect(isEmpty(newState.disjunctiveFacetsRefinements)).toBe( - false, - 'state should have disjunctiveFacetsRefinements' - ); - expect(isEmpty(newState.hierarchicalFacetsRefinements)).toBe( - false, - 'state should have hierarchicalFacetsRefinements' - ); - expect(isEmpty(newState.numericRefinements)).toBe( - false, - 'state should have numericRefinements' - ); - expect(isEmpty(newState.tagRefinements)).toBe( - false, - 'state should have tagRefinements' - ); - }); - - it('should clear facets', () => { - const newState = utils.clearRefinementsFromState(state, [ - 'facet1', - 'facet2', - ]); - expect(isEmpty(newState.facetsRefinements)).toBe( - true, - "state shouldn't have facetsRefinements" - ); - expect(isEmpty(newState.facetsExcludes)).toBe( - false, - 'state should have facetsExcludes' - ); - expect(isEmpty(newState.disjunctiveFacetsRefinements)).toBe( - false, - 'state should have disjunctiveFacetsRefinements' - ); - expect(isEmpty(newState.hierarchicalFacetsRefinements)).toBe( - false, - 'state should have hierarchicalFacetsRefinements' - ); - expect(isEmpty(newState.numericRefinements)).toBe( - false, - 'state should have numericRefinements' - ); - expect(isEmpty(newState.tagRefinements)).toBe( - false, - 'state should have tagRefinements' - ); - }); - - it('should clear excludes', () => { - const newState = utils.clearRefinementsFromState(state, ['facetExclude1']); - expect(isEmpty(newState.facetsRefinements)).toBe( - false, - 'state should have facetsRefinements' - ); - expect(isEmpty(newState.facetsExcludes)).toBe( - true, - "state shouldn't have facetsExcludes" - ); - expect(isEmpty(newState.disjunctiveFacetsRefinements)).toBe( - false, - 'state should have disjunctiveFacetsRefinements' - ); - expect(isEmpty(newState.hierarchicalFacetsRefinements)).toBe( - false, - 'state should have hierarchicalFacetsRefinements' - ); - expect(isEmpty(newState.numericRefinements)).toBe( - false, - 'state should have numericRefinements' - ); - expect(isEmpty(newState.tagRefinements)).toBe( - false, - 'state should have tagRefinements' - ); - }); - - it('should clear disjunctive facets', () => { - const newState = utils.clearRefinementsFromState(state, [ - 'disjunctiveFacet1', - ]); - expect(isEmpty(newState.facetsRefinements)).toBe( - false, - 'state should have facetsRefinements' - ); - expect(isEmpty(newState.facetsExcludes)).toBe( - false, - 'state should have facetsExcludes' - ); - expect(isEmpty(newState.disjunctiveFacetsRefinements)).toBe( - true, - "state shouldn't have disjunctiveFacetsRefinements" - ); - expect(isEmpty(newState.hierarchicalFacetsRefinements)).toBe( - false, - 'state should have hierarchicalFacetsRefinements' - ); - expect(isEmpty(newState.numericRefinements)).toBe( - false, - 'state should have numericRefinements' - ); - expect(isEmpty(newState.tagRefinements)).toBe( - false, - 'state should have tagRefinements' - ); - }); - - it('should clear hierarchical facets', () => { - const newState = utils.clearRefinementsFromState(state, [ - 'hierarchicalFacet1', - ]); - expect(isEmpty(newState.facetsRefinements)).toBe( - false, - 'state should have facetsRefinements' - ); - expect(isEmpty(newState.facetsExcludes)).toBe( - false, - 'state should have facetsExcludes' - ); - expect(isEmpty(newState.disjunctiveFacetsRefinements)).toBe( - false, - 'state should have disjunctiveFacetsRefinements' - ); - expect(isEmpty(newState.hierarchicalFacetsRefinements)).toBe( - true, - "state shouldn't have hierarchicalFacetsRefinements" - ); - expect(isEmpty(newState.numericRefinements)).toBe( - false, - 'state should have numericRefinements' - ); - expect(isEmpty(newState.tagRefinements)).toBe( - false, - 'state should have tagRefinements' - ); - }); - - it('should clear one numeric facet', () => { - const newState = utils.clearRefinementsFromState(state, ['numericFacet1']); - expect(isEmpty(newState.facetsRefinements)).toBe( - false, - 'state should have facetsRefinements' - ); - expect(isEmpty(newState.facetsExcludes)).toBe( - false, - 'state should have facetsExcludes' - ); - expect(isEmpty(newState.disjunctiveFacetsRefinements)).toBe( - false, - 'state should have disjunctiveFacetsRefinements' - ); - expect(isEmpty(newState.hierarchicalFacetsRefinements)).toBe( - false, - 'state should have hierarchicalFacetsRefinements' - ); - expect(isEmpty(newState.numericRefinements)).toBe( - false, - 'state should have numericRefinements' - ); - expect(isEmpty(newState.tagRefinements)).toBe( - false, - 'state should have tagRefinements' - ); - }); - - it('should clear one numeric disjunctive facet', () => { - const newState = utils.clearRefinementsFromState(state, [ - 'numericDisjunctiveFacet1', - ]); - expect(isEmpty(newState.facetsRefinements)).toBe( - false, - 'state should have facetsRefinements' - ); - expect(isEmpty(newState.facetsExcludes)).toBe( - false, - 'state should have facetsExcludes' - ); - expect(isEmpty(newState.disjunctiveFacetsRefinements)).toBe( - false, - 'state should have disjunctiveFacetsRefinements' - ); - expect(isEmpty(newState.hierarchicalFacetsRefinements)).toBe( - false, - 'state should have hierarchicalFacetsRefinements' - ); - expect(isEmpty(newState.numericRefinements)).toBe( - false, - 'state should have numericRefinements' - ); - expect(isEmpty(newState.tagRefinements)).toBe( - false, - 'state should have tagRefinements' - ); - }); - - it('should clear numeric facets', () => { - const newState = utils.clearRefinementsFromState(state, [ - 'numericFacet1', - 'numericDisjunctiveFacet1', - ]); - expect(isEmpty(newState.facetsRefinements)).toBe( - false, - 'state should have facetsRefinements' - ); - expect(isEmpty(newState.facetsExcludes)).toBe( - false, - 'state should have facetsExcludes' - ); - expect(isEmpty(newState.disjunctiveFacetsRefinements)).toBe( - false, - 'state should have disjunctiveFacetsRefinements' - ); - expect(isEmpty(newState.hierarchicalFacetsRefinements)).toBe( - false, - 'state should have hierarchicalFacetsRefinements' - ); - expect(isEmpty(newState.numericRefinements)).toBe( - true, - "state shouldn't have numericRefinements" - ); - expect(isEmpty(newState.tagRefinements)).toBe( - false, - 'state should have tagRefinements' - ); - }); - - it('should clear tags', () => { - const newState = utils.clearRefinementsFromState(state, ['_tags']); - expect(isEmpty(newState.facetsRefinements)).toBe( - false, - 'state should have facetsRefinements' - ); - expect(isEmpty(newState.facetsExcludes)).toBe( - false, - 'state should have facetsExcludes' - ); - expect(isEmpty(newState.disjunctiveFacetsRefinements)).toBe( - false, - 'state should have disjunctiveFacetsRefinements' - ); - expect(isEmpty(newState.hierarchicalFacetsRefinements)).toBe( - false, - 'state should have hierarchicalFacetsRefinements' - ); - expect(isEmpty(newState.numericRefinements)).toBe( - false, - 'state should have numericRefinements' - ); - expect(isEmpty(newState.tagRefinements)).toBe( - true, - "state shouldn't have tagRefinements" - ); - }); -}); - describe('utils.deprecate', () => { const sum = (...args) => args.reduce((acc, _) => acc + _, 0); @@ -1252,3 +921,218 @@ describe('utils.parseAroundLatLngFromString', () => { }); }); }); + +describe('utils.clearRefinements', () => { + const initHelperWithRefinements = () => { + const helper = algoliasearchHelper({}, 'index', { + facets: ['conjFacet'], + disjunctiveFacets: ['disjFacet'], + }); + + helper.toggleRefinement('conjFacet', 'value'); + helper.toggleRefinement('disjFacet', 'otherValue'); + helper.toggleTag('taG'); + + helper.setQuery('a query'); + + return helper; + }; + + describe('Without clearsQuery', () => { + it('can clear all the parameters refined', () => { + const helper = initHelperWithRefinements(); + + const finalState = utils.clearRefinements({ + helper, + }); + + expect(finalState.query).toBe(helper.state.query); + expect(finalState.facetsRefinements).toEqual({}); + expect(finalState.disjunctiveFacetsRefinements).toEqual({}); + expect(finalState.tagRefinements).toEqual([]); + }); + + it('can clear all the parameters defined in the whiteList', () => { + const helper = initHelperWithRefinements(); + + const finalState = utils.clearRefinements({ + helper, + whiteList: ['conjFacet'], + }); + + expect(finalState.query).toBe(helper.state.query); + expect(finalState.facetsRefinements).toEqual({}); + expect(finalState.disjunctiveFacetsRefinements).toEqual( + helper.state.disjunctiveFacetsRefinements + ); + expect(finalState.tagRefinements).toEqual(helper.state.tagRefinements); + }); + + it('can clear all the parameters refined but the ones in the black list', () => { + const helper = initHelperWithRefinements(); + + const finalState = utils.clearRefinements({ + helper, + blackList: ['conjFacet'], + }); + + expect(finalState.query).toBe(helper.state.query); + expect(finalState.facetsRefinements).toEqual( + helper.state.facetsRefinements + ); + expect(finalState.disjunctiveFacetsRefinements).toEqual({}); + expect(finalState.tagRefinements).toEqual([]); + }); + + it('can clear all the parameters in the whitelist except the ones in the black list', () => { + const helper = initHelperWithRefinements(); + + const finalState = utils.clearRefinements({ + helper, + whiteList: ['conjFacet', 'disjFacet'], + blackList: ['conjFacet'], + }); + + expect(finalState.query).toBe(helper.state.query); + expect(finalState.facetsRefinements).toEqual( + helper.state.facetsRefinements + ); + expect(finalState.disjunctiveFacetsRefinements).toEqual({}); + expect(finalState.tagRefinements).toEqual(finalState.tagRefinements); + }); + + it('can clear tags only (whitelisting tags)', () => { + const helper = initHelperWithRefinements(); + + const finalState = utils.clearRefinements({ + helper, + whiteList: ['_tags'], + }); + + expect(finalState.query).toBe(helper.state.query); + expect(finalState.facetsRefinements).toEqual( + helper.state.facetsRefinements + ); + expect(finalState.disjunctiveFacetsRefinements).toEqual( + finalState.disjunctiveFacetsRefinements + ); + expect(finalState.tagRefinements).toEqual([]); + }); + + it('can clear everything but the tags (blacklisting tags)', () => { + const helper = initHelperWithRefinements(); + + const finalState = utils.clearRefinements({ + helper, + blackList: ['_tags'], + }); + + expect(finalState.query).toBe(helper.state.query); + expect(finalState.facetsRefinements).toEqual({}); + expect(finalState.disjunctiveFacetsRefinements).toEqual({}); + expect(finalState.tagRefinements).toEqual(finalState.tagRefinements); + }); + }); + + describe('With clearsQuery', () => { + it('can clear all the parameters refined', () => { + const helper = initHelperWithRefinements(); + + const finalState = utils.clearRefinements({ + helper, + clearsQuery: true, + }); + + expect(finalState.query).toBe(''); + expect(finalState.facetsRefinements).toEqual({}); + expect(finalState.disjunctiveFacetsRefinements).toEqual({}); + expect(finalState.tagRefinements).toEqual([]); + }); + + it('can clear all the parameters defined in the whiteList', () => { + const helper = initHelperWithRefinements(); + + const finalState = utils.clearRefinements({ + helper, + whiteList: ['conjFacet'], + clearsQuery: true, + }); + + expect(finalState.query).toBe(''); + expect(finalState.facetsRefinements).toEqual({}); + expect(finalState.disjunctiveFacetsRefinements).toEqual( + helper.state.disjunctiveFacetsRefinements + ); + expect(finalState.tagRefinements).toEqual(helper.state.tagRefinements); + }); + + it('can clear all the parameters refined but the ones in the black list', () => { + const helper = initHelperWithRefinements(); + + const finalState = utils.clearRefinements({ + helper, + blackList: ['conjFacet'], + clearsQuery: true, + }); + + expect(finalState.query).toBe(''); + expect(finalState.facetsRefinements).toEqual( + helper.state.facetsRefinements + ); + expect(finalState.disjunctiveFacetsRefinements).toEqual({}); + expect(finalState.tagRefinements).toEqual([]); + }); + + it('can clear all the parameters in the whitelist except the ones in the black list', () => { + const helper = initHelperWithRefinements(); + + const finalState = utils.clearRefinements({ + helper, + whiteList: ['conjFacet', 'disjFacet'], + blackList: ['conjFacet'], + clearsQuery: true, + }); + + expect(finalState.query).toBe(''); + expect(finalState.facetsRefinements).toEqual( + helper.state.facetsRefinements + ); + expect(finalState.disjunctiveFacetsRefinements).toEqual({}); + expect(finalState.tagRefinements).toEqual(finalState.tagRefinements); + }); + + it('can clear tags only (whitelisting tags)', () => { + const helper = initHelperWithRefinements(); + + const finalState = utils.clearRefinements({ + helper, + whiteList: ['_tags'], + clearsQuery: true, + }); + + expect(finalState.query).toBe(''); + expect(finalState.facetsRefinements).toEqual( + helper.state.facetsRefinements + ); + expect(finalState.disjunctiveFacetsRefinements).toEqual( + finalState.disjunctiveFacetsRefinements + ); + expect(finalState.tagRefinements).toEqual([]); + }); + + it('can clear everything but the tags (blacklisting tags)', () => { + const helper = initHelperWithRefinements(); + + const finalState = utils.clearRefinements({ + helper, + blackList: ['_tags'], + clearsQuery: true, + }); + + expect(finalState.query).toBe(''); + expect(finalState.facetsRefinements).toEqual({}); + expect(finalState.disjunctiveFacetsRefinements).toEqual({}); + expect(finalState.tagRefinements).toEqual(finalState.tagRefinements); + }); + }); +}); diff --git a/src/lib/utils.js b/src/lib/utils.js index 9fe4aa36d9..9186e6bc61 100644 --- a/src/lib/utils.js +++ b/src/lib/utils.js @@ -2,7 +2,6 @@ import reduce from 'lodash/reduce'; import forEach from 'lodash/forEach'; import find from 'lodash/find'; import get from 'lodash/get'; -import isEmpty from 'lodash/isEmpty'; import keys from 'lodash/keys'; import uniq from 'lodash/uniq'; import mapKeys from 'lodash/mapKeys'; @@ -18,8 +17,8 @@ export { isSpecialClick, isDomElement, getRefinements, - clearRefinementsFromState, - clearRefinementsAndSearch, + getAttributesToClear, + clearRefinements, prefixKeys, escapeRefinement, unescapeRefinement, @@ -301,44 +300,64 @@ function getRefinements(results, state, clearsQuery) { return res; } -function clearRefinementsFromState( - inputState, - attributeNames, - clearsQuery = false -) { - let state = inputState; - - if (clearsQuery) { - state = state.setQuery(''); - } +/** + * Clears the refinements of a SearchParameters object based on rules provided. + * The white list is first used then the black list is applied. If no white list + * is provided, all the current refinements are used. + * @param {object} $0 parameters + * @param {Helper} $0.helper instance of the Helper + * @param {string[]} [$0.whiteList] list of parameters to clear + * @param {string[]} [$0.blackList=[]] list of parameters not to remove (will impact the white list) + * @param {boolean} [$0.clearsQuery=false] clears the query if need be + * @returns {SearchParameters} search parameters with refinements cleared + */ +function clearRefinements({ + helper, + whiteList, + blackList = [], + clearsQuery = false, +}) { + const attributesToClear = getAttributesToClear({ + helper, + whiteList, + blackList, + }); - if (isEmpty(attributeNames)) { - state = state.clearTags(); - state = state.clearRefinements(); - return state; - } + let finalState = helper.state; - forEach(attributeNames, attributeName => { - if (attributeName === '_tags') { - state = state.clearTags(); + attributesToClear.forEach(attribute => { + if (attribute === '_tags') { + finalState = finalState.clearTags(); } else { - state = state.clearRefinements(attributeName); + finalState = finalState.clearRefinements(attribute); } }); - return state; + if (clearsQuery) { + finalState = finalState.setQuery(''); + } + + return finalState; } -function clearRefinementsAndSearch( - helper, - attributeNames, - clearsQuery = false -) { - helper - .setState( - clearRefinementsFromState(helper.state, attributeNames, clearsQuery) - ) - .search(); +/** + * Computes the list of attributes (conjunctive, disjunctive, hierarchical facet + numerical attributes) + * to clear based on a optional white and black lists. The white list is applied first then the black list. + * @param {object} $0 parameters + * @param {Helper} $0.helper instance of the Helper + * @param {string[]} [$0.whiteList] attributes to clear (defaults to all attributes) + * @param {string[]} [$0.blackList=[]] attributes to keep, will override the white list + * @returns {string[]} the list of attributes to clear based on the rules + */ +function getAttributesToClear({ helper, whiteList, blackList }) { + const lastResults = helper.lastResults || {}; + const attributesToClear = + whiteList || + getRefinements(lastResults, helper.state).map(one => one.attributeName); + + return attributesToClear.filter( + attribute => blackList.indexOf(attribute) === -1 + ); } function prefixKeys(prefix, obj) { diff --git a/src/widgets/clear-all/__tests__/clear-all-test.js b/src/widgets/clear-all/__tests__/clear-all-test.js index 420fac24f9..0562ab3e4b 100644 --- a/src/widgets/clear-all/__tests__/clear-all-test.js +++ b/src/widgets/clear-all/__tests__/clear-all-test.js @@ -56,7 +56,7 @@ describe('clearAll()', () => { }; widget.init({ helper, - createURL: () => {}, + createURL, instantSearchInstance: { templatesConfig: {}, }, diff --git a/src/widgets/current-refined-values/__tests__/current-refined-values-test.js b/src/widgets/current-refined-values/__tests__/current-refined-values-test.js index 2ba0237483..65c8a36530 100644 --- a/src/widgets/current-refined-values/__tests__/current-refined-values-test.js +++ b/src/widgets/current-refined-values/__tests__/current-refined-values-test.js @@ -431,9 +431,11 @@ describe('currentRefinedValues()', () => { .toggleTag('tag1') .toggleTag('tag2'); + const createURL = () => '#cleared'; + initParameters = { helper, - createURL: () => '', + createURL, instantSearchInstance: { templatesConfig: { randomAttributeNeverUsed: 'value' }, }, @@ -500,7 +502,7 @@ describe('currentRefinedValues()', () => { helper, state: helper.state, templatesConfig: { randomAttributeNeverUsed: 'value' }, - createURL: () => '#cleared', + createURL, }; refinements = [