Skip to content

Commit

Permalink
fix(refinementList): fix facet exhaustivity check (#2554)
Browse files Browse the repository at this point in the history
* 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

* chore(style): fix according to prettier
  • Loading branch information
bobylito authored Nov 6, 2017
1 parent ec810fa commit 0f1bf08
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -512,13 +512,94 @@ describe('connectRefinementList', () => {
facets: {
category: {
c1: 880,
c2: 880,
},
},
},
{
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,
},
},
},
Expand All @@ -539,6 +620,7 @@ describe('connectRefinementList', () => {
c1: 880,
c2: 34,
c3: 440,
c4: 440,
},
},
},
Expand All @@ -548,6 +630,7 @@ describe('connectRefinementList', () => {
c1: 880,
c2: 34,
c3: 440,
c4: 440,
},
},
},
Expand Down
15 changes: 14 additions & 1 deletion src/connectors/refinement-list/connectRefinementList.js
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,20 @@ 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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: {} };

Expand Down Expand Up @@ -57,7 +59,7 @@ describe('refinementList()', () => {
.stub()
.returns([{ name: 'foo' }, { name: 'bar' }]),
};
state = { toggleRefinement: sinon.spy() };
state = SearchParameters.make({});
createURL = () => '#';
});

Expand Down

0 comments on commit 0f1bf08

Please sign in to comment.