fix: Duplicated options in Select when using numerical values #24906

michael-s-molina · 2023-08-07T15:05:20Z

SUMMARY

We noticed that Select options were being duplicated when searching for a value in numerical filters. It turns out that Ant Design Select can't have options with numerical values when mode equals tags. It displays the following warning:

Warning: value of Option should not use number type when mode is tags or combobox

To overcome this limitation, this PR changes the mode to multiple and implement missing features such as allowing new values and copy and paste.

Fixes #24877

TESTING INSTRUCTIONS

I added a test to the component but we need to check the general use of selects in the application because there are wrappers that override the component behavior such as SelectControl and SelectFilterPlugin.

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

codecov · 2023-08-07T16:04:40Z

Codecov Report

Merging #24906 (51a7dbd) into master (aee2695) will increase coverage by 10.38%.
Report is 13 commits behind head on master.
The diff coverage is 74.19%.

❗ Current head 51a7dbd differs from pull request most recent head 1c63c8f. Consider uploading reports for the commit 1c63c8f to get more accurate results

@@             Coverage Diff             @@
##           master   #24906       +/-   ##
===========================================
+ Coverage   58.45%   68.84%   +10.38%     
===========================================
  Files        1906     1906               
  Lines       74141    74144        +3     
  Branches     8208     8207        -1     
===========================================
+ Hits        43337    51041     +7704     
+ Misses      28683    20971     -7712     
- Partials     2121     2132       +11

Flag	Coverage Δ
hive	`?`
javascript	`55.82% <70.37%> (-0.02%)`	⬇️
presto	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
...s/legacy-plugin-chart-country-map/src/countries.ts	`100.00% <ø> (ø)`
...c/features/datasets/AddDataset/LeftPanel/index.tsx	`89.47% <ø> (ø)`
superset-frontend/src/features/datasets/styles.ts	`92.00% <50.00%> (-8.00%)`	⬇️
superset-frontend/src/components/Select/Select.tsx	`84.65% <69.23%> (-1.14%)`	⬇️
...set-frontend/src/components/Select/AsyncSelect.tsx	`84.79% <72.72%> (-4.53%)`	⬇️
...tend/src/features/datasets/DatasetLayout/index.tsx	`100.00% <100.00%> (ø)`
superset/config.py	`92.20% <100.00%> (+0.58%)`	⬆️
superset/security/manager.py	`94.43% <100.00%> (+33.95%)`	⬆️
superset/utils/core.py	`90.39% <100.00%> (+22.64%)`	⬆️

... and 291 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

michael-s-molina · 2023-08-07T18:31:57Z

/testenv up

github-actions · 2023-08-07T18:34:14Z

@michael-s-molina Ephemeral environment spinning up at http://34.221.157.238:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

kgabryje · 2023-08-10T15:01:00Z

@michael-s-molina Have you considered an alternative approach of stringifying the numeric values when using them as option values and then parsing the numbers in onSelect callback? If it's possible, it could save us some added complexity of the features added for parity with tag mode

kgabryje · 2023-08-10T15:01:47Z

superset-frontend/src/components/Select/AsyncSelect.test.tsx

-  expect(options[1]?.textContent).toEqual('CAc');
-  expect(options[2]?.textContent).toEqual('abac');
-  expect(options[3]?.textContent).toEqual('Cac');
+  await waitFor(() => {


Are all those test changes relevant to the fix or are those just some bycatches?

These changes are the result of debouncing handleOnSearch.

michael-s-molina · 2023-08-10T15:57:23Z

@michael-s-molina Have you considered an alternative approach of stringifying the numeric values when using them as option values and then parsing the numbers in onSelect callback? If it's possible, it could save us some added complexity of the features added for parity with tag mode

I did consider this approach but rejected for the following reasons:

It goes against the Typescript definition of the values
It has performance implications when dealing with a big number of options
Numbers are supported in single and multiple modes.

kgabryje · 2023-08-10T18:04:48Z

I did consider this approach but rejected for the following reasons:

It goes against the Typescript definition of the values

It has performance implications when dealing with a big number of options

Numbers are supported in single and multiple modes.

Got it, thanks for explanation 👍

kgabryje

LGTM

geido

Code LGTM. We will run regression testing on this one to catch any possible side effect. Thanks!

github-actions · 2023-08-11T17:22:46Z

Ephemeral environment shutdown and build artifacts deleted.

(cherry picked from commit b621ee9)

…#24906)

fix: Duplicated options in Select when using numerical values

7a49393

michael-s-molina requested review from yousoph and jinghua-qa August 7, 2023 15:05

michael-s-molina requested review from geido and kgabryje as code owners August 7, 2023 15:05

pull-request-size bot added the size/L label Aug 7, 2023

Fixes E2E tests

1c63c8f

michael-s-molina requested review from eschutho, rusackas and betodealmeida as code owners August 7, 2023 16:44

rusackas added review:checkpoint Last PR reviewed during the daily review standup and removed review:checkpoint Last PR reviewed during the daily review standup labels Aug 7, 2023

kgabryje reviewed Aug 10, 2023

View reviewed changes

kgabryje approved these changes Aug 11, 2023

View reviewed changes

geido approved these changes Aug 11, 2023

View reviewed changes

michael-s-molina merged commit b621ee9 into apache:master Aug 11, 2023

michael-s-molina added the v3.0 Label added by the release manager to track PRs to be included in the 3.0 branch label Aug 11, 2023

eschutho added v2.1 2.1.1 labels Aug 12, 2023

michael-s-molina added a commit that referenced this pull request Aug 14, 2023

fix: Duplicated options in Select when using numerical values (#24906)

dd53b33

(cherry picked from commit b621ee9)

mistercrunch added 🍒 3.0.0 🍒 3.0.1 🍒 3.0.2 labels Mar 8, 2024

mistercrunch added 🍒 3.0.3 🍒 3.0.4 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 3.1.0 labels Mar 8, 2024

vinothkumar66 pushed a commit to vinothkumar66/superset that referenced this pull request Nov 11, 2024

fix: Duplicated options in Select when using numerical values (apache…

1bd89ef

…#24906)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Duplicated options in Select when using numerical values #24906

fix: Duplicated options in Select when using numerical values #24906

michael-s-molina commented Aug 7, 2023

codecov bot commented Aug 7, 2023 •

edited

Loading

michael-s-molina commented Aug 7, 2023

github-actions bot commented Aug 7, 2023

kgabryje commented Aug 10, 2023

kgabryje Aug 10, 2023

michael-s-molina Aug 10, 2023

michael-s-molina commented Aug 10, 2023 •

edited

Loading

kgabryje commented Aug 10, 2023

kgabryje left a comment

geido left a comment

github-actions bot commented Aug 11, 2023

fix: Duplicated options in Select when using numerical values #24906

fix: Duplicated options in Select when using numerical values #24906

Conversation

michael-s-molina commented Aug 7, 2023

SUMMARY

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov bot commented Aug 7, 2023 • edited Loading

Codecov Report

michael-s-molina commented Aug 7, 2023

github-actions bot commented Aug 7, 2023

kgabryje commented Aug 10, 2023

kgabryje Aug 10, 2023

Choose a reason for hiding this comment

michael-s-molina Aug 10, 2023

Choose a reason for hiding this comment

michael-s-molina commented Aug 10, 2023 • edited Loading

kgabryje commented Aug 10, 2023

kgabryje left a comment

Choose a reason for hiding this comment

geido left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 11, 2023

codecov bot commented Aug 7, 2023 •

edited

Loading

michael-s-molina commented Aug 10, 2023 •

edited

Loading