inveniosoftware / react-searchkit

React component library for interacting with a REST API.
https://inveniosoftware.github.io/react-searchkit/
MIT License
78 stars 40 forks source link

selectedFilters doesn't work for Integer bucket keys #265

Open corovcam opened 4 months ago

corovcam commented 4 months ago

Describe the bug

selectedFilters.filter((filter) => filter[0] === aggName && filter[1] === value) This step uses strict comparison between value and type.

In case the bucket contains Integer value (key), i.e. year, age, etc., upon bucket click, the checkbox doesn't appear visually checked (the comparison fails). On the other hand, the filter is applied, so the search parameters (and URL) gets updated. The problem lies only in the unset isSelected prop that gets passed down to the Checkbox SemanticUI component.

The same problem could potentially happen with other data types: boolean, etc.

I don't think that it's meant for the bucket containing key as integer (or boolean), to also contain bucket.key_as_string property, as seen here:

const keyField = bucket.key_as_string ? bucket.key_as_string : bucket.key;
const isSelected = this._isSelected(this.aggName, keyField, selectedFilters);

For example for years or age, or any other number coming from backend.

Expected behavior

Using value comparison only, the issue can be easily solved: selectedFilters.filter((filter) => filter[0] == aggName && filter[1] == value)

zzacharo commented 3 months ago

Hello @corovcam , thanks for reporting this issue. I understand the issue with the strict type checking but it was done in purpose so we do not end up with difficult to track bugs due to JS value casting. Could you please elaborate a bit more on your ES filter structure and maybe also showcase what are the values of the selectedFilters that make the comparison above fail? I would like to understand e.g why in some point there is a comparison between 1 === "1" which makes the isSelected constant get assigned with the wrong value.

corovcam commented 2 months ago

Hi @zzacharo, thanks for your reply. For example, having clicked on a certain integer checkbox, selectedFilters gets assigned with:

selectedFilters: [
  ["metadata_restorationObject_creationPeriod_since", "1550"]
]

and the respective buckets being filtered:

"buckets": [
      {
        "key": 1550,
        "doc_count": 3,
        "label": 1550,
        "is_selected": false
      },
      {
        "key": 1700,
        "doc_count": 3,
        "label": 1700,
        "is_selected": false
      },
      {
        "key": 1728,
        "doc_count": 3,
        "label": 1728,
        "is_selected": false
      },
      {
        "key": 1880,
        "doc_count": 3,
        "label": 1880,
        "is_selected": false
      },
      {
        "key": 1500,
        "doc_count": 2,
        "label": 1500,
        "is_selected": false
      },
],

Also another possible solution could be:

selectedFilters.filter((filter) => filter[0] === aggName && filter[1] === value.toString())
zzacharo commented 2 months ago

What about trying to solve this on the Checkbox level i.e when you click on a value it should set as selectedFilter the integer value and not the string. Then the comparison would work as expected. Which component are you using to display the bucket values? The default one or do you have a custom one?