Skip to content

Conversation

@geido
Copy link
Member

@geido geido commented Jan 9, 2021

SUMMARY

Closes #12389

The debounce effect was retriggered all the time causing the fetch to be called in an infinite loop.

BEFORE

ezgif-3-1f57266d4bcb

AFTER

First-Time-Developer-Commute-Time

TEST PLAN

  1. Open a chart
  2. Change the dataset
  3. Search for a dataset
  4. Make sure the search does not retrigger all the time

ADDITIONAL INFORMATION

@codecov-io
Copy link

codecov-io commented Jan 9, 2021

Codecov Report

Merging #12390 (c565659) into master (5d04f7d) will decrease coverage by 0.00%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12390      +/-   ##
==========================================
- Coverage   66.88%   66.88%   -0.01%     
==========================================
  Files        1014     1014              
  Lines       49526    49527       +1     
  Branches     5073     5073              
==========================================
  Hits        33124    33124              
- Misses      16272    16273       +1     
  Partials      130      130              
Flag Coverage Δ
cypress 50.92% <16.66%> (-0.01%) ⬇️
javascript 60.69% <50.00%> (-0.01%) ⬇️
python 64.21% <ø> (ø)

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

Impacted Files Coverage Δ
...-frontend/src/datasource/ChangeDatasourceModal.tsx 80.24% <25.00%> (-1.01%) ⬇️
superset-frontend/src/explore/exploreUtils.js 69.32% <100.00%> (-0.62%) ⬇️
...ntend/src/dashboard/components/PropertiesModal.jsx 83.03% <0.00%> (ø)
...rset-frontend/src/explore/components/SaveModal.tsx 91.76% <0.00%> (+1.17%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d04f7d...c565659. Read the comment docs.

@junlincc
Copy link
Member

junlincc commented Jan 9, 2021

@geido thanks for the quick fix!
one more ask, when user is deleting search input, they should get a list of increasing options. can you add that functionality to search? we are using it in the Datapanel search as well.

ezgif-7-2d1a9923bb24

Copy link
Member

@junlincc junlincc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@geido let's open a new PR to implement the enhancement on search! many thanks for fixing this P0 issue!!!

FYI @eschutho @betodealmeida this issue is likely caused by #12006

@villebro villebro merged commit 3eb0470 into apache:master Jan 9, 2021
@junlincc junlincc added the explore:dataset Related to the dataset of Explore label Jan 9, 2021
@villebro villebro removed the v1.0.1 label Jan 25, 2021
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.0.0 First shipped in 1.0.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels explore:dataset Related to the dataset of Explore size/M 🚢 1.0.0 First shipped in 1.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Explore]flaky Change dataset modal

6 participants