topcoat-data / topcoat-public

TopCoat components, visualizations and themes
MIT License
1 stars 3 forks source link

fix: TMultiSelector filters as array #276

Closed egcastro closed 1 year ago

egcastro commented 1 year ago

This will introduce a breaking change. I'm thinking on a way to keep backwards compatibility.

What this does

Filters are going to be treated as array of string no matter the amount of filters. For example: ["one-value"] or ["one-value", "two-value", "n-value"].

This will avoid the special character used as a token to separate filters |. Usually, that token is valid as a separator for query string parameters if your values are integers like ids for example. In our case, those values are arbitrary and controlled by our users. That's the reason that a label with a | on it was breaking the whole filtering experience.

Notes for the reviewer

This is a breaking change. It fixes a bug and it solves a Zendesk escalation but it changes the way that multi-value filters are managed by the TMultiSelector component. Before filters were concatenated with | and now they are treated as arrays.

Before: filter1|filter2|filter3|

After: ["filter1", "filter2", "filter3"]

Depends on this topcoat-core PR: https://github.com/snyk/topcoat-core/pull/788

More information

Screenshots / GIFs

Before After
Screenshot 2023-10-25 at 15 53 12 Screenshot 2023-10-25 at 15 55 29

Current filters' situation

This is the state of the art related with filters in our current reporting setup:

There are three different filter versions:

Single key, single value

Interface: filter=value Example: issue_status=Open Components generating this kind of filter:

<t-multi-selector> (when there is only one value selected)
    key=value

<t-date-picker>
    introduced_start=2023-10-23
    introduced_end=2023-10-24
    introduced_range_preset=custom

<t-range-selector>
    min=317
    max=1000

<t-table>
    table_issues_detail_sort=' PROJECT_NAME ASC,  EXPLOIT_MATURITY ASC

Single key, multiple values:

Interface: filter=value1|value2|value3 Example: table_issues_detail_cols=SCORE|CVE|CWE|PROJECT|EXPLOIT MATURITY|AUTO FIXABLE|INTRODUCED|SNYK PRODUCT Components generating this kind of filter:

<t-multi-selector> (when there is more than one value selected)
    key=value1|value2|value3

<t-table>
    table_issues_detail_cols=SCORE|CVE|CWE|PROJECT|EXPLOIT MATURITY|AUTO FIXABLE|INTRODUCED|SNYK PRODUCT

Multiple keys, multiple values:

Interface: filter={key-a: [value1, value2, value3], key-b: [value1, value2, value3]} Example: package_name={"@angular/core":["10.2.5","8.2.14"],"@babel/traverse":["7.6.0"]} Components generating this kind of filter:

<t-tags-selector>
    key={key: ["value1", "value2"], key2: ["value1", "value2"]}

With this PR <TMultiSelector> will be generating a single key - multiple values filter, for example: key=["value1"] or key=["value1", "value2", "valueN"].

Then the filters generated by <TMultiSelector> will be as follows:

Single key, multiple values:

Interface: filter=["value1", "value2", "value3"] Example: issue_status=["Open", "Ignored"]

<TTable> will still generate a single key - multiple values with the old interface:

Interface: filter=value1|value2|value3 Example: table_issues_detail_cols=SCORE|CVE|CWE|PROJECT|EXPLOIT MATURITY|AUTO FIXABLE|INTRODUCED|SNYK PRODUCT

The main difference remains in that we control those strings and are hardcoded in the table column data.

The reason we haven't changed those now is to keep the change smaller and reduce the surface of the breaking change.

Backwards compatibility

By using the new feature introduced on topcoat-core by this PR: https://github.com/snyk/topcoat-core/pull/788 about URL versioning, the <TMultiSelector> component can implement a method to detect and infer in which version of the filters the current URL is and migrate it to the latest.

The only case that will need a new URL generation is the one that is already broken by the old approach. This is the minimum user base that will be not auto fixable but since those experiences are already broken, once they re-generate the URL by clicking on the filters by hand, the fix will be applied and the new versioning will take care of keeping that working.

Ideal technical solution

Ideally, we should build a service that can handle all kinds of object serialization and we should only have a query string parameter that would point to the serialized object: q=md5OrShaHash