fix(hierarchicalFacets): prevent different rootPath on same attribute (…

…#3965) * fix(hierarchicalMenu): ignore configuration with different values * chore(stories): remove rootPath for selected item * chore(stories): rename header -> rootPath * fix(connectBreadcrumb): ignore configuration with different values * chore(stories): rename selected item -> rootPath * chore(stories): remove useless option on HierarchicalMenu * chore(stories): use SearchParameters rather than custom widget
algolia · Jul 19, 2019 · 5ee79fa · 5ee79fa
1 parent 5e37b25
commit 5ee79fa
Show file tree

Hide file tree

Showing 6 changed files with 602 additions and 252 deletions.
diff --git a/src/connectors/breadcrumb/__tests__/connectBreadcrumb-test.js b/src/connectors/breadcrumb/__tests__/connectBreadcrumb-test.js
@@ -1,4 +1,5 @@
 import jsHelper, { SearchResults } from 'algoliasearch-helper';
+import { warning } from '../../../lib/utils';
 import connectBreadcrumb from '../connectBreadcrumb';
 
 describe('connectBreadcrumb', () => {
@@ -38,73 +39,193 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/breadcrumb/
     });
   });
 
-  it('should compute getConfiguration() correctly', () => {
-    const rendering = jest.fn();
-    const makeWidget = connectBreadcrumb(rendering);
+  describe('getConfiguration', () => {
+    beforeEach(() => {
+      warning.cache = {};
+    });
 
-    const widget = makeWidget({ attributes: ['category', 'sub_category'] });
+    it('returns the default configuration', () => {
+      const render = () => {};
+      const makeWidget = connectBreadcrumb(render);
+      const widget = makeWidget({
+        attributes: ['category', 'sub_category'],
+      });
+
+      const previous = {};
+
+      const actual = widget.getConfiguration(previous);
 
-    // when there is no hierarchicalFacets into current configuration
-    {
-      const config = widget.getConfiguration({});
-      expect(config).toEqual({
+      expect(actual).toEqual({
         hierarchicalFacets: [
           {
-            attributes: ['category', 'sub_category'],
             name: 'category',
+            attributes: ['category', 'sub_category'],
             rootPath: null,
             separator: ' > ',
           },
         ],
       });
-    }
+    });
 
-    // when there is an identical hierarchicalFacets into current configuration
-    expect(() => {
-      const config = widget.getConfiguration({
-        hierarchicalFacets: [{ name: 'category' }],
+    it('returns the configuration with custom `separator`', () => {
+      const render = () => {};
+      const makeWidget = connectBreadcrumb(render);
+      const widget = makeWidget({
+        attributes: ['category', 'sub_category'],
+        separator: ' / ',
       });
 
-      expect(config).toEqual({});
-    }).toWarnDev();
+      const previous = {};
 
-    // when there is already a different hierarchicalFacets into current configuration
-    {
-      const config = widget.getConfiguration({
-        hierarchicalFacets: [{ name: 'foo' }],
+      const actual = widget.getConfiguration(previous);
+
+      expect(actual).toEqual({
+        hierarchicalFacets: [
+          {
+            name: 'category',
+            attributes: ['category', 'sub_category'],
+            rootPath: null,
+            separator: ' / ',
+          },
+        ],
+      });
+    });
+
+    it('returns the configuration with custom `rootPath`', () => {
+      const render = () => {};
+      const makeWidget = connectBreadcrumb(render);
+      const widget = makeWidget({
+        attributes: ['category', 'sub_category'],
+        rootPath: 'TopLevel > SubLevel',
       });
-      expect(config).toEqual({
+
+      const previous = {};
+
+      const actual = widget.getConfiguration(previous);
+
+      expect(actual).toEqual({
         hierarchicalFacets: [
           {
+            name: 'category',
             attributes: ['category', 'sub_category'],
+            rootPath: 'TopLevel > SubLevel',
+            separator: ' > ',
+          },
+        ],
+      });
+    });
+
+    it('returns the configuration with another `hierarchicalFacets` already defined', () => {
+      const render = () => {};
+      const makeWidget = connectBreadcrumb(render);
+      const widget = makeWidget({
+        attributes: ['category', 'sub_category'],
+      });
+
+      const previous = {
+        hierarchicalFacets: [
+          {
+            name: 'country',
+            attributes: ['country', 'sub_country'],
+            separator: ' > ',
+            rootPath: null,
+          },
+        ],
+      };
+
+      const actual = widget.getConfiguration(previous);
+
+      expect(actual).toEqual({
+        hierarchicalFacets: [
+          {
             name: 'category',
+            attributes: ['category', 'sub_category'],
+            separator: ' > ',
             rootPath: null,
+          },
+        ],
+      });
+    });
+
+    it('returns an empty configuration with the same `hierarchicalFacets` already defined', () => {
+      const render = () => {};
+      const makeWidget = connectBreadcrumb(render);
+      const widget = makeWidget({ attributes: ['category', 'sub_category'] });
+
+      const previous = {
+        hierarchicalFacets: [
+          {
+            name: 'category',
+            attributes: ['category', 'sub_category'],
             separator: ' > ',
+            rootPath: null,
           },
         ],
+      };
+
+      const actual = widget.getConfiguration(previous);
+
+      expect(actual).toEqual({});
+    });
+
+    it('warns with the same `hierarchicalFacets` already defined with different `attributes`', () => {
+      const render = () => {};
+      const makeWidget = connectBreadcrumb(render);
+      const widget = makeWidget({ attributes: ['category', 'sub_category'] });
+
+      const previous = {
+        hierarchicalFacets: [
+          {
+            name: 'category',
+            attributes: ['category', 'sub_category', 'sub_sub_category'],
+          },
+        ],
+      };
+
+      expect(() => widget.getConfiguration(previous)).toWarnDev();
+    });
+
+    it('warns with the same `hierarchicalFacets` already defined with different `separator`', () => {
+      const render = () => {};
+      const makeWidget = connectBreadcrumb(render);
+      const widget = makeWidget({
+        attributes: ['category', 'sub_category'],
+        separator: ' / ',
       });
-    }
-  });
 
-  it('should compute getConfiguration() correctly with a custom separator', () => {
-    const rendering = jest.fn();
-    const makeWidget = connectBreadcrumb(rendering);
+      const previous = {
+        hierarchicalFacets: [
+          {
+            name: 'category',
+            attributes: ['category', 'sub_category'],
+            separator: ' > ',
+          },
+        ],
+      };
 
-    const widget = makeWidget({
-      attributes: ['category', 'sub_category'],
-      separator: ' / ',
+      expect(() => widget.getConfiguration(previous)).toWarnDev();
     });
-    const widgetConfiguration = widget.getConfiguration({});
 
-    expect(widgetConfiguration).toEqual({
-      hierarchicalFacets: [
-        {
-          attributes: ['category', 'sub_category'],
-          name: 'category',
-          rootPath: null,
-          separator: ' / ',
-        },
-      ],
+    it('warns with the same `hierarchicalFacets` already defined with different `rootPath`', () => {
+      const render = () => {};
+      const makeWidget = connectBreadcrumb(render);
+      const widget = makeWidget({
+        attributes: ['category', 'sub_category'],
+        rootPath: 'TopLevel',
+      });
+
+      const previous = {
+        hierarchicalFacets: [
+          {
+            name: 'category',
+            attributes: ['category', 'sub_category'],
+            separator: ' > ',
+            rootPath: 'TopLevel > SubLevel',
+          },
+        ],
+      };
+
+      expect(() => widget.getConfiguration(previous)).toWarnDev();
     });
   });
 
@@ -193,7 +314,6 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/breadcrumb/
           name: 'category',
           rootPath: null,
           separator: ' > ',
-          showParentLevel: true,
         },
       ],
     });

diff --git a/src/connectors/breadcrumb/connectBreadcrumb.js b/src/connectors/breadcrumb/connectBreadcrumb.js
@@ -70,12 +70,15 @@ export default function connectBreadcrumb(renderFn, unmountFn = noop) {
             currentConfiguration.hierarchicalFacets,
             ({ name }) => name === hierarchicalFacetName
           );
+
           if (isFacetSet) {
             warning(
               isEqual(isFacetSet.attributes, attributes) &&
-                isFacetSet.separator === separator,
+                isFacetSet.separator === separator &&
+                isFacetSet.rootPath === rootPath,
               'Using Breadcrumb and HierarchicalMenu on the same facet with different options overrides the configuration of the HierarchicalMenu.'
             );
+
             return {};
           }
         }