chore(getConfiguration): burn it to the ground 🔥 #4083

Haroenv · 2019-08-29T15:49:30Z

remove getConfiguration in every widget
remove getConfiguration from types & expectations
change tests which were using getConfiguration to getWidgetSearchParameters

Haroenv · 2019-08-29T15:52:23Z

src/widgets/range-slider/__tests__/range-slider-test.js

@@ -62,15 +62,20 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/range-slide
    });

    describe('min option', () => {
-      it('refines when no previous configuration', () => {
+      // @TODO: gWSP does not yet take min & max in account
+      it.skip('refines when no previous configuration', () => {


these need to be fixed in a separate PR, cc @samouss

Haroenv · 2019-08-29T15:56:04Z

src/connectors/range/connectRange.js

+        // can not use setQueryParameters || removeNumericRefinements, because
+        // they both keep the old value. This isn't immutable, but it is fine
+        // since it's already a copy.
+        stateWithoutDisjunctive.numericRefinements = {


I'm not sure if we can find a better solution for this

IIRC you can use removeNumericRefinements with the attribute only, it will perform a clear under the hood.

It doesn’t actually remove the refinement, I havent found a solution which doesn’t break for refining to an empty state

src/widgets/range-slider/__tests__/range-slider-test.js

samouss

I went through the changes didn't read everything carefully but it looks good overall.

samouss · 2019-08-30T10:20:58Z

src/types/widget.ts

  getWidgetState?(
    uiState: IndexUiState,
    widgetStateOptions: WidgetStateOptions
  ): IndexUiState;
+  /**
+   * This function is required for a widget to behave correctly when a URL is


This is half true since we know use this function to sets the configuration too.

Do you have a better description? “Also used to keep track (or merge) of refinements when other widgets are added/removed”?

src/connectors/menu/__tests__/connectMenu-test.js

src/connectors/hierarchical-menu/__tests__/connectHierarchicalMenu-test.js

samouss · 2019-08-30T10:40:10Z

src/connectors/range/connectRange.js

+        // can not use setQueryParameters || removeNumericRefinements, because
+        // they both keep the old value. This isn't immutable, but it is fine
+        // since it's already a copy.
+        stateWithoutDisjunctive.numericRefinements = {


IIRC you can use removeNumericRefinements with the attribute only, it will perform a clear under the hood.

src/connectors/range/connectRange.js

tkrugg · 2019-09-04T13:27:34Z

src/connectors/breadcrumb/__tests__/connectBreadcrumb-test.js

-        ],
-      })
-    );
+    it('does not remove refinement', () => {


not sure I get this part.
Removing the widget should remove the refinement, no?

no, a breadcrumb's refinement is set by a configure or hierarchical facet, and should thus stay. This didn't change in this PR by the way, it was simply not tested

tkrugg · 2019-09-04T13:30:45Z

src/connectors/hits-per-page/__tests__/connectHitsPerPage-test.js

    ).toEqual(
      new SearchParameters({
-        hitsPerPage: 20,
+        hitsPerPage: 10,


isn't this supposed to be 20?
this doesn't look like it overrides it, right?

Or is it the undefined value that overrides the 20, so we end up fallbacking to the default 10 ?

we override the previously set hPP with the "default" one, this was defined by #4069

Co-Authored-By: Yannick Croissant <yannick.croissant@gmail.com>

* fix(connectRange): apply min/max * fix(connectRange): throw an error when max is lower than min

* chore: remove enhanceConfiguration * chore: remove getConfiguration from types * chore: remove getConfiguration from widgets * test(autocomplete): replace getConfiguration tests * chore: remove enhanceConfiguration test * test(refinementlist): replace getConfiguration tests * test(pagination): replace getConfiguration test * test(searchbox): replace getConfiguration tests * test(configure): replace getConfiguration in tests * test(ratingMenu): update usage of getConfiguration * test(breadcrumb): move getConfiguration tests * test(hitsPerPage): migrate getConfiguration tests * test(infinitehits): remove duplicate getConfiguration tests * test(stats): remove useless (common) test * chore: update comment * test(toggleRefinement): swap getConfiguration usages * test(refinementlist): migrate getConfiguration tests * test(clear): remove useless assertion * test(current): remove useless assertion * test(menu): update calls * test(pagination): !fixup * test(hierarchical-menu): remove redundant tests * test(sortBy): remove redundant tests * test(hits): remove redundant tests * test: remove redundant tests * test(range): update getConfiguration tests (with caveats) * !fixup linting * chore(test): remove commented code * chore: fix message Co-Authored-By: Yannick Croissant <yannick.croissant@gmail.com> * fix(range): apply min/max to getWidgetSearchParameters (#4088) * fix(connectRange): apply min/max * fix(connectRange): throw an error when max is lower than min * test(hpp): cleaner test without snapshots

Haroenv requested review from francoischalifour, samouss and a team August 29, 2019 15:50

ghost removed their request for review August 29, 2019 15:50

Haroenv commented Aug 29, 2019

View reviewed changes

Haroenv commented Aug 30, 2019

View reviewed changes

src/widgets/range-slider/__tests__/range-slider-test.js Show resolved Hide resolved

samouss reviewed Aug 30, 2019

View reviewed changes

Haroenv requested review from yannickcr and tkrugg and removed request for francoischalifour September 2, 2019 08:16

Haroenv force-pushed the chore/getConfiguration branch from 34516f7 to 18b7730 Compare September 2, 2019 09:22

yannickcr approved these changes Sep 2, 2019

View reviewed changes

src/connectors/range/connectRange.js Outdated Show resolved Hide resolved

tkrugg reviewed Sep 4, 2019

View reviewed changes

tkrugg approved these changes Sep 4, 2019

View reviewed changes

Haroenv added 14 commits September 4, 2019 15:55

chore: remove enhanceConfiguration

b4dca01

chore: remove getConfiguration from types

6b6b45d

chore: remove getConfiguration from widgets

b585084

test(autocomplete): replace getConfiguration tests

1ea9740

chore: remove enhanceConfiguration test

913d2f7

test(refinementlist): replace getConfiguration tests

47d094c

test(pagination): replace getConfiguration test

0e335c5

test(searchbox): replace getConfiguration tests

c218de3

test(configure): replace getConfiguration in tests

a573a23

test(ratingMenu): update usage of getConfiguration

aa6bcfb

test(breadcrumb): move getConfiguration tests

19620af

test(hitsPerPage): migrate getConfiguration tests

e873ecd

test(infinitehits): remove duplicate getConfiguration tests

d2308d3

test(stats): remove useless (common) test

9ee60e0

Haroenv and others added 17 commits September 4, 2019 15:55

chore: update comment

b0fc5d6

test(toggleRefinement): swap getConfiguration usages

a1e2e5b

test(refinementlist): migrate getConfiguration tests

d64e3de

test(clear): remove useless assertion

f9b9cb0

test(current): remove useless assertion

500b5fe

test(menu): update calls

93f0b98

test(pagination): !fixup

8e40cf2

test(hierarchical-menu): remove redundant tests

7759444

test(sortBy): remove redundant tests

8f1923c

test(hits): remove redundant tests

bdacde0

test: remove redundant tests

d075e90

test(range): update getConfiguration tests (with caveats)

88bf2f3

!fixup linting

5426006

chore(test): remove commented code

ac5cc70

chore: fix message

9cd6d5f

Co-Authored-By: Yannick Croissant <yannick.croissant@gmail.com>

fix(range): apply min/max to getWidgetSearchParameters (#4088)

f2dccbf

* fix(connectRange): apply min/max * fix(connectRange): throw an error when max is lower than min

test(hpp): cleaner test without snapshots

7466f90

Haroenv force-pushed the chore/getConfiguration branch from 7e0ae3d to 7466f90 Compare September 4, 2019 13:56

Haroenv merged commit a38fb6f into next Sep 4, 2019

Haroenv deleted the chore/getConfiguration branch September 4, 2019 14:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(getConfiguration): burn it to the ground 🔥 #4083

chore(getConfiguration): burn it to the ground 🔥 #4083

Haroenv commented Aug 29, 2019

Haroenv Aug 29, 2019

Haroenv Sep 2, 2019

Haroenv Aug 29, 2019

samouss Aug 30, 2019

Haroenv Aug 30, 2019

samouss left a comment

samouss Aug 30, 2019

Haroenv Aug 30, 2019

samouss Aug 30, 2019

tkrugg Sep 4, 2019

Haroenv Sep 4, 2019

tkrugg Sep 4, 2019 •

edited

Loading

Haroenv Sep 4, 2019

chore(getConfiguration): burn it to the ground 🔥 #4083

chore(getConfiguration): burn it to the ground 🔥 #4083

Conversation

Haroenv commented Aug 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samouss left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkrugg Sep 4, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkrugg Sep 4, 2019 •

edited

Loading