fix(disjunctive): prefer values of main query for facet count (#6445)

* fix(disjunctive): prefer values of main query for facet count This is relevant in two cases: - second query is non-exhaustive - second query is not correct due to facet count after distinct There's still more edge cases left (first query non exhaustive, but second isn't, but that's unlikely) and those are not worse after this PR. * Update packages/algoliasearch-helper/test/spec/SearchResults/getFacetValues.js * same for hierarchical * fix, but only when persistHierarchicalRootCount is true --------- Co-authored-by: Dhaya <[email protected]>
algolia · Dec 9, 2024 · 9bc841a · 9bc841a
1 parent 2f1f397
commit 9bc841a
Show file tree

Hide file tree

Showing 6 changed files with 248 additions and 18 deletions.
diff --git a/packages/algoliasearch-helper/src/SearchParameters/RefinementList.js b/packages/algoliasearch-helper/src/SearchParameters/RefinementList.js
@@ -39,7 +39,7 @@ var lib = {
 
     mod[attribute] = facetRefinement;
 
-    return defaultsPure({}, mod, refinementList);
+    return defaultsPure(mod, refinementList);
   },
   /**
    * Removes refinement(s) for an attribute:

diff --git a/packages/algoliasearch-helper/src/SearchParameters/index.js b/packages/algoliasearch-helper/src/SearchParameters/index.js
@@ -1213,7 +1213,6 @@ SearchParameters.prototype = {
 
     return this.setQueryParameters({
       hierarchicalFacetsRefinements: defaultsPure(
-        {},
         mod,
         this.hierarchicalFacetsRefinements
       ),
@@ -1241,7 +1240,6 @@ SearchParameters.prototype = {
     mod[facet] = [path];
     return this.setQueryParameters({
       hierarchicalFacetsRefinements: defaultsPure(
-        {},
         mod,
         this.hierarchicalFacetsRefinements
       ),
@@ -1262,7 +1260,6 @@ SearchParameters.prototype = {
     mod[facet] = [];
     return this.setQueryParameters({
       hierarchicalFacetsRefinements: defaultsPure(
-        {},
         mod,
         this.hierarchicalFacetsRefinements
       ),

diff --git a/packages/algoliasearch-helper/src/SearchResults/index.js b/packages/algoliasearch-helper/src/SearchResults/index.js
@@ -6,7 +6,6 @@ var fv = require('../functions/escapeFacetValue');
 var find = require('../functions/find');
 var findIndex = require('../functions/findIndex');
 var formatSort = require('../functions/formatSort');
-var merge = require('../functions/merge');
 var orderBy = require('../functions/orderBy');
 var escapeFacetValue = fv.escapeFacetValue;
 var unescapeFacetValue = fv.unescapeFacetValue;
@@ -244,12 +243,9 @@ function SearchResults(state, results, options) {
   });
 
   // Make every key of the result options reachable from the instance
-  var opts = merge(
-    {
-      persistHierarchicalRootCount: false,
-    },
-    options
-  );
+  var opts = defaultsPure(options, {
+    persistHierarchicalRootCount: false,
+  });
   Object.keys(opts).forEach(function (key) {
     self[key] = opts[key];
   });
@@ -516,11 +512,16 @@ function SearchResults(state, results, options) {
           return;
         }
 
-        self.hierarchicalFacets[position][attributeIndex].data = merge(
-          {},
-          self.hierarchicalFacets[position][attributeIndex].data,
-          facetResults
-        );
+        self.hierarchicalFacets[position][attributeIndex].data =
+          self.persistHierarchicalRootCount
+            ? defaultsPure(
+                self.hierarchicalFacets[position][attributeIndex].data,
+                facetResults
+              )
+            : defaultsPure(
+                facetResults,
+                self.hierarchicalFacets[position][attributeIndex].data
+              );
       } else {
         position = disjunctiveFacetsIndices[dfacet];
 
@@ -529,7 +530,7 @@ function SearchResults(state, results, options) {
 
         self.disjunctiveFacets[position] = {
           name: dfacet,
-          data: defaultsPure({}, facetResults, dataFromMainRequest),
+          data: defaultsPure(dataFromMainRequest, facetResults),
           exhaustive: result.exhaustiveFacetsCount,
         };
         assignFacetStats(
@@ -927,7 +928,7 @@ SearchResults.prototype.getFacetValues = function (attribute, opts) {
     return undefined;
   }
 
-  var options = defaultsPure({}, opts, {
+  var options = defaultsPure(opts, {
     sortBy: SearchResults.DEFAULT_SORT,
     // if no sortBy is given, attempt to sort based on facetOrdering
     // if it is given, we still allow to sort via facet ordering first

diff --git a/packages/algoliasearch-helper/test/spec/SearchResults/getFacetValues.js b/packages/algoliasearch-helper/test/spec/SearchResults/getFacetValues.js
@@ -729,3 +729,108 @@ test('getFacetValues(hierarchical) when current state is different to constructo
 
   expect(facetValues).toEqual(expected);
 });
+
+test('getFacetValues(facetName) prefers the "main" facet result (disjunctive)', function () {
+  var data = require('./getFacetValues/disjunctive-non-exhaustive.json');
+  var searchParams = new SearchParameters(data.state);
+  var result = new SearchResults(searchParams, data.content.results);
+
+  var facetValues = result.getFacetValues('brand');
+
+  var expected = [
+    { count: 1000, isRefined: true, name: 'Apple', escapedValue: 'Apple' },
+    {
+      count: 551,
+      isRefined: false,
+      name: 'Insignia™',
+      escapedValue: 'Insignia™',
+    },
+    { count: 511, isRefined: false, name: 'Samsung', escapedValue: 'Samsung' },
+  ];
+
+  expect(facetValues).toEqual(expected);
+});
+
+test('getFacetValues(facetName) prefers the "main" facet result (hierarchical) (persistHierarchicalRootCount: true)', function () {
+  var data = require('./getFacetValues/hierarchical-non-exhaustive.json');
+  var searchParams = new SearchParameters(data.state);
+  var result = new SearchResults(searchParams, data.content.results, {
+    persistHierarchicalRootCount: true,
+  });
+
+  var facetValues = result.getFacetValues('brand').data;
+
+  var expected = [
+    {
+      count: 1000,
+      data: null,
+      isRefined: true,
+      name: 'Apple',
+      escapedValue: 'Apple',
+      path: 'Apple',
+      exhaustive: true,
+    },
+    {
+      count: 551,
+      data: null,
+      isRefined: false,
+      name: 'Insignia™',
+      escapedValue: 'Insignia™',
+      path: 'Insignia™',
+      exhaustive: true,
+    },
+    {
+      count: 551,
+      data: null,
+      isRefined: false,
+      name: 'Samsung',
+      escapedValue: 'Samsung',
+      path: 'Samsung',
+      exhaustive: true,
+    },
+  ];
+
+  expect(facetValues).toEqual(expected);
+});
+
+test('getFacetValues(facetName) prefers the "main" facet result (hierarchical) (persistHierarchicalRootCount: false)', function () {
+  var data = require('./getFacetValues/hierarchical-non-exhaustive.json');
+  var searchParams = new SearchParameters(data.state);
+  var result = new SearchResults(searchParams, data.content.results, {
+    persistHierarchicalRootCount: false,
+  });
+
+  var facetValues = result.getFacetValues('brand').data;
+
+  var expected = [
+    {
+      count: 50,
+      data: null,
+      isRefined: true,
+      name: 'Apple',
+      escapedValue: 'Apple',
+      path: 'Apple',
+      exhaustive: true,
+    },
+    {
+      count: 551,
+      data: null,
+      isRefined: false,
+      name: 'Insignia™',
+      escapedValue: 'Insignia™',
+      path: 'Insignia™',
+      exhaustive: true,
+    },
+    {
+      count: 551,
+      data: null,
+      isRefined: false,
+      name: 'Samsung',
+      escapedValue: 'Samsung',
+      path: 'Samsung',
+      exhaustive: true,
+    },
+  ];
+
+  expect(facetValues).toEqual(expected);
+});
diff --git a/...goliasearch-helper/test/spec/SearchResults/getFacetValues/disjunctive-non-exhaustive.json b/...goliasearch-helper/test/spec/SearchResults/getFacetValues/disjunctive-non-exhaustive.json
@@ -0,0 +1,64 @@
+{
+  "content": {
+    "results": [
+      {
+        "hits": [],
+        "nbHits": 1000,
+        "page": 0,
+        "nbPages": 20,
+        "hitsPerPage": 0,
+        "processingTimeMS": 1,
+        "facets": {
+          "brand": {
+            "Apple": 1000
+          }
+        },
+        "exhaustiveFacetsCount": true,
+        "query": "",
+        "params": "query=&maxValuesPerFacet=3&page=0&facets=%5B%22brand%22%5D&tagFilters=&facetFilters=%5B%5B%22brand%3AApple%22%5D%5D",
+        "index": "instant_search"
+      },
+      {
+        "hits": [
+          {
+            "objectID": "1696302"
+          }
+        ],
+        "nbHits": 10000,
+        "page": 0,
+        "nbPages": 1000,
+        "hitsPerPage": 1,
+        "processingTimeMS": 1,
+        "facets": {
+          "brand": {
+            "Insignia™": 551,
+            "Samsung": 511,
+            "Apple": 1
+          }
+        },
+        "exhaustiveFacetsCount": false,
+        "query": "",
+        "params": "query=&maxValuesPerFacet=3&page=0&hitsPerPage=1&attributesToRetrieve=%5B%5D&attributesToHighlight=%5B%5D&attributesToSnippet=%5B%5D&tagFilters=&facets=brand",
+        "index": "instant_search"
+      }
+    ]
+  },
+  "state": {
+    "index": "instant_search",
+    "query": "",
+    "facets": [],
+    "disjunctiveFacets": ["brand"],
+    "hierarchicalFacets": [],
+    "facetsRefinements": {},
+    "facetsExcludes": {},
+    "disjunctiveFacetsRefinements": {
+      "brand": ["Apple"]
+    },
+    "numericRefinements": {},
+    "tagRefinements": [],
+    "hierarchicalFacetsRefinements": {},
+    "maxValuesPerFacet": 3,
+    "page": 0
+  },
+  "error": null
+}
diff --git a/...oliasearch-helper/test/spec/SearchResults/getFacetValues/hierarchical-non-exhaustive.json b/...oliasearch-helper/test/spec/SearchResults/getFacetValues/hierarchical-non-exhaustive.json
@@ -0,0 +1,63 @@
+{
+  "content": {
+    "results": [
+      {
+        "hits": [],
+        "nbHits": 1000,
+        "page": 0,
+        "nbPages": 16,
+        "hitsPerPage": 8,
+        "facets": {
+          "brand": {
+            "Apple": 1000
+          }
+        },
+        "exhaustiveFacetsCount": true,
+        "exhaustiveNbHits": true,
+        "query": "",
+        "index": "instant_search"
+      },
+      {
+        "hits": [],
+        "nbHits": 3988,
+        "page": 0,
+        "nbPages": 0,
+        "hitsPerPage": 0,
+        "facets": {
+          "brand": {
+            "Apple": 50,
+            "Insignia™": 551,
+            "Samsung": 551
+          }
+        },
+        "exhaustiveFacetsCount": true,
+        "exhaustiveNbHits": true,
+        "query": "",
+        "index": "instant_search"
+      }
+    ]
+  },
+  "state": {
+    "index": "instant_search",
+    "query": "",
+    "facets": [],
+    "disjunctiveFacets": [],
+    "hierarchicalFacets": [
+      {
+        "name": "brand",
+        "attributes": ["brand"]
+      }
+    ],
+    "facetsRefinements": {},
+    "facetsExcludes": {},
+    "disjunctiveFacetsRefinements": {},
+    "numericRefinements": {},
+    "tagRefinements": [],
+    "hierarchicalFacetsRefinements": {
+      "brand": ["Apple"]
+    },
+    "maxValuesPerFacet": 3,
+    "page": 0
+  },
+  "error": null
+}