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

Author: bobylito

src/connectors/clear-all/__tests__/connectClearAll-test.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,68 +86,64 @@ 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,
           },
           true
         );
       },
 
-      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,
           },

src/connectors/clear-all/connectClearAll.js

@@ -1,23 +1,22 @@
-import sinon from 'sinon';
 import jsHelper from 'algoliasearch-helper';
 const SearchResults = jsHelper.SearchResults;
 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`
     });
 
     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,14 +99,17 @@ 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();
     const makeWidget = connectCurrentRefinedValues(rendering);
     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', () => {

src/connectors/current-refined-values/__tests__/connectCurrentRefinedValues-test.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,