From 11254b68ebc405504d3ffe0dd5f245c18b7c0219 Mon Sep 17 00:00:00 2001 From: Alex S Date: Mon, 6 Nov 2017 14:11:58 +0100 Subject: [PATCH 1/2] fix(refinementList): fix facet exhausistivity check This check depends on wether or not there is another widget setting the max value per facet to a bigger value, we can't be sure that if there is the same number of items than the limit that it is exhaustive. Fixes the other part of #2552 --- .../__tests__/connectRefinementList-test.js | 89 ++++++++++++++++++- .../refinement-list/connectRefinementList.js | 12 ++- .../__tests__/refinement-list-test.js | 4 +- 3 files changed, 102 insertions(+), 3 deletions(-) diff --git a/src/connectors/refinement-list/__tests__/connectRefinementList-test.js b/src/connectors/refinement-list/__tests__/connectRefinementList-test.js index 115f928337..ca4d27302f 100644 --- a/src/connectors/refinement-list/__tests__/connectRefinementList-test.js +++ b/src/connectors/refinement-list/__tests__/connectRefinementList-test.js @@ -483,7 +483,7 @@ describe('connectRefinementList', () => { ]); }); - it('hasExhaustiveItems indicates if the items provided are exhaustive', () => { + it('hasExhaustiveItems indicates if the items provided are exhaustive - without other widgets making the maxValuesPerFacet bigger', () => { const widget = makeWidget({ attributeName: 'category', limit: 2, @@ -512,6 +512,7 @@ describe('connectRefinementList', () => { facets: { category: { c1: 880, + c2: 880, }, }, }, @@ -519,6 +520,90 @@ describe('connectRefinementList', () => { facets: { category: { c1: 880, + c2: 880, + }, + }, + }, + ]), + state: helper.state, + helper, + createURL: () => '#', + }); + + // this one is `false` because we're not sure that what we asked is the actual number of facet values + expect(rendering.lastCall.args[0].hasExhaustiveItems).toEqual(false); + + widget.render({ + results: new SearchResults(helper.state, [ + { + hits: [], + facets: { + category: { + c1: 880, + c2: 34, + c3: 440, + }, + }, + }, + { + facets: { + category: { + c1: 880, + c2: 34, + c3: 440, + }, + }, + }, + ]), + state: helper.state, + helper, + createURL: () => '#', + }); + + expect(rendering.lastCall.args[0].hasExhaustiveItems).toEqual(false); + }); + + it('hasExhaustiveItems indicates if the items provided are exhaustive - with an other widgets making the maxValuesPerFacet bigger', () => { + const widget = makeWidget({ + attributeName: 'category', + limit: 2, + }); + + const helper = algoliasearchHelper( + fakeClient, + '', + { + ...widget.getConfiguration({}), + maxValuesPerFacet: 3 + } + ); + helper.search = sinon.stub(); + + widget.init({ + helper, + state: helper.state, + createURL: () => '#', + onHistoryChange: () => {}, + }); + + expect(rendering.lastCall.args[0].hasExhaustiveItems).toEqual(true); + + widget.render({ + results: new SearchResults(helper.state, [ + { + hits: [], + facets: { + category: { + c1: 880, + c2: 880, + }, + }, + }, + { + facets: { + category: { + c1: 880, + c2: 880, }, }, }, @@ -539,6 +624,7 @@ describe('connectRefinementList', () => { c1: 880, c2: 34, c3: 440, + c4: 440, }, }, }, @@ -548,6 +634,7 @@ describe('connectRefinementList', () => { c1: 880, c2: 34, c3: 440, + c4: 440, }, }, }, diff --git a/src/connectors/refinement-list/connectRefinementList.js b/src/connectors/refinement-list/connectRefinementList.js index 4be4bd870c..1444504805 100644 --- a/src/connectors/refinement-list/connectRefinementList.js +++ b/src/connectors/refinement-list/connectRefinementList.js @@ -334,7 +334,17 @@ export default function connectRefinementList(renderFn) { const facetValues = results.getFacetValues(attributeName, { sortBy }); const items = facetValues.slice(0, this.getLimit()).map(formatItems); - const hasExhaustiveItems = facetValues.length <= this.getLimit(); + const maxValuesPerFacetConfig = state.getQueryParameter('maxValuesPerFacet'); + const currentLimit = this.getLimit(); + // If the limit is the max number of facet retrieved it is impossible to know + // if the facets are exhaustives. The only moment we are sure it is exhaustive + // is when it is strictly under the number requested unless we know that another + // widget has requested more values (maxValuesPerFacet > getLimit()). + // Because this is used for making the search of facets unable or not, it is important + // to be conservative here. + const hasExhaustiveItems = maxValuesPerFacetConfig > currentLimit + ? facetValues.length <= currentLimit + : facetValues.length < currentLimit; lastResultsFromMainSearch = items; diff --git a/src/widgets/refinement-list/__tests__/refinement-list-test.js b/src/widgets/refinement-list/__tests__/refinement-list-test.js index fc048b9917..791edff5a0 100644 --- a/src/widgets/refinement-list/__tests__/refinement-list-test.js +++ b/src/widgets/refinement-list/__tests__/refinement-list-test.js @@ -3,6 +3,8 @@ import sinon from 'sinon'; import expectJSX from 'expect-jsx'; expect.extend(expectJSX); +import algoliasearchHelper from 'algoliasearch-helper'; +const SearchParameters = algoliasearchHelper.SearchParameters; import refinementList from '../refinement-list.js'; const instantSearchInstance = { templatesConfig: {} }; @@ -57,7 +59,7 @@ describe('refinementList()', () => { .stub() .returns([{ name: 'foo' }, { name: 'bar' }]), }; - state = { toggleRefinement: sinon.spy() }; + state = SearchParameters.make({}); createURL = () => '#'; }); From 445a3b156cdc3798a531ce68b02eb4fd2d7c41f2 Mon Sep 17 00:00:00 2001 From: Alex S Date: Mon, 6 Nov 2017 16:11:50 +0100 Subject: [PATCH 2/2] chore(style): fix according to prettier --- .../__tests__/connectRefinementList-test.js | 12 ++++-------- .../refinement-list/connectRefinementList.js | 11 +++++++---- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/connectors/refinement-list/__tests__/connectRefinementList-test.js b/src/connectors/refinement-list/__tests__/connectRefinementList-test.js index ca4d27302f..c206dcd367 100644 --- a/src/connectors/refinement-list/__tests__/connectRefinementList-test.js +++ b/src/connectors/refinement-list/__tests__/connectRefinementList-test.js @@ -569,14 +569,10 @@ describe('connectRefinementList', () => { limit: 2, }); - const helper = algoliasearchHelper( - fakeClient, - '', - { - ...widget.getConfiguration({}), - maxValuesPerFacet: 3 - } - ); + const helper = algoliasearchHelper(fakeClient, '', { + ...widget.getConfiguration({}), + maxValuesPerFacet: 3, + }); helper.search = sinon.stub(); widget.init({ diff --git a/src/connectors/refinement-list/connectRefinementList.js b/src/connectors/refinement-list/connectRefinementList.js index 1444504805..998f2bf7f2 100644 --- a/src/connectors/refinement-list/connectRefinementList.js +++ b/src/connectors/refinement-list/connectRefinementList.js @@ -334,7 +334,9 @@ export default function connectRefinementList(renderFn) { const facetValues = results.getFacetValues(attributeName, { sortBy }); const items = facetValues.slice(0, this.getLimit()).map(formatItems); - const maxValuesPerFacetConfig = state.getQueryParameter('maxValuesPerFacet'); + const maxValuesPerFacetConfig = state.getQueryParameter( + 'maxValuesPerFacet' + ); const currentLimit = this.getLimit(); // If the limit is the max number of facet retrieved it is impossible to know // if the facets are exhaustives. The only moment we are sure it is exhaustive @@ -342,9 +344,10 @@ export default function connectRefinementList(renderFn) { // widget has requested more values (maxValuesPerFacet > getLimit()). // Because this is used for making the search of facets unable or not, it is important // to be conservative here. - const hasExhaustiveItems = maxValuesPerFacetConfig > currentLimit - ? facetValues.length <= currentLimit - : facetValues.length < currentLimit; + const hasExhaustiveItems = + maxValuesPerFacetConfig > currentLimit + ? facetValues.length <= currentLimit + : facetValues.length < currentLimit; lastResultsFromMainSearch = items;