Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(explore): highlight Run button correctly when query is stale #15994

Merged
merged 1 commit into from
Aug 1, 2021

Conversation

suddjian
Copy link
Member

@suddjian suddjian commented Jul 30, 2021

SUMMARY

In the staleness logic, it was only looking at the changes in controls since the last render of the explore container, when actually it should look at changes since the last query.

This also fixes a number of other cases where changing the value of a control did not prompt running the query.

AFTER

Screen.Recording.2021-07-30.at.3.27.08.PM.mov

TESTING INSTRUCTIONS

Open explore, change controls around, see run button light up.

ADDITIONAL INFORMATION

@suddjian suddjian requested a review from kgabryje July 30, 2021 22:29
@suddjian suddjian changed the title fix(explore): calculate query staleness correctly fix(explore): highlight Run button correctly when query is stale Jul 30, 2021
@codecov
Copy link

codecov bot commented Jul 30, 2021

Codecov Report

Merging #15994 (b981e27) into master (f4739f4) will increase coverage by 0.13%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #15994      +/-   ##
==========================================
+ Coverage   76.98%   77.11%   +0.13%     
==========================================
  Files         988      988              
  Lines       52378    53529    +1151     
  Branches     6622     7093     +471     
==========================================
+ Hits        40322    41278     +956     
- Misses      11833    12020     +187     
- Partials      223      231       +8     
Flag Coverage Δ
hive 81.37% <ø> (ø)
javascript 71.75% <0.00%> (+0.19%) ⬆️
mysql 81.59% <ø> (-0.05%) ⬇️
postgres 81.65% <ø> (ø)
presto 81.49% <ø> (?)
python 82.18% <ø> (+0.15%) ⬆️
sqlite 81.29% <ø> (ø)

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

Impacted Files Coverage Δ
...nd/src/explore/components/ExploreViewContainer.jsx 2.11% <0.00%> (-0.04%) ⬇️
...d/src/dashboard/components/gridComponents/Tabs.jsx 86.50% <0.00%> (-2.39%) ⬇️
...rontend/src/views/CRUD/dashboard/DashboardCard.tsx 75.00% <0.00%> (-2.28%) ⬇️
...rc/dashboard/components/dnd/dragDroppableConfig.js 33.33% <0.00%> (-1.67%) ⬇️
...t-frontend/src/views/CRUD/welcome/SavedQueries.tsx 58.86% <0.00%> (-1.14%) ⬇️
...erset-frontend/src/SqlLab/components/SqlEditor.jsx 53.58% <0.00%> (-0.41%) ⬇️
...set-frontend/src/components/ListViewCard/index.tsx 100.00% <0.00%> (ø)
...eFilters/FiltersConfigModal/FiltersConfigModal.tsx 95.37% <0.00%> (+0.13%) ⬆️
superset/models/core.py 89.87% <0.00%> (+0.25%) ⬆️
...nts/controls/DateFilterControl/DateFilterLabel.tsx 67.92% <0.00%> (+0.98%) ⬆️
... and 17 more

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 f4739f4...b981e27. Read the comment docs.

@rusackas
Copy link
Member

/testenv up

@github-actions
Copy link
Contributor

@rusackas Container image not yet published for this PR. Please try again when build is complete.

@github-actions
Copy link
Contributor

@rusackas Ephemeral environment creation failed. Please check the Actions logs for details.

@github-actions
Copy link
Contributor

@suddjian Container image not yet published for this PR. Please try again when build is complete.

@github-actions
Copy link
Contributor

@suddjian Ephemeral environment creation failed. Please check the Actions logs for details.

@rusackas
Copy link
Member

/testenv up

@github-actions
Copy link
Contributor

@rusackas Ephemeral environment spinning up at http://54.68.119.214:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

@rusackas rusackas self-requested a review July 30, 2021 23:36
Copy link
Member

@rusackas rusackas left a comment

Choose a reason for hiding this comment

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

LGTM

@jinghua-qa
Copy link
Member

Can I test this PR with dd feature flag on? The previous reported issue is dd related

@rusackas
Copy link
Member

/testenv up FEATURE_ENABLE_EXPLORE_DRAG_AND_DROP=true

@github-actions
Copy link
Contributor

@rusackas Ephemeral environment spinning up at http://34.210.64.222:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

Copy link
Member

@jinghua-qa jinghua-qa left a comment

Choose a reason for hiding this comment

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

LGTM!

Run.button.mov

@rusackas rusackas merged commit 46188c1 into master Aug 1, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2021

Ephemeral environment shutdown and build artifacts deleted.

@villebro
Copy link
Member

villebro commented Aug 1, 2021

Awesome work!

@rusackas rusackas deleted the fix/explore-staleness-detection branch August 1, 2021 08:52
@junlincc junlincc added the #bug:blocking! Blocking issues with high priority label Aug 2, 2021
opus-42 pushed a commit to opus-42/incubator-superset that referenced this pull request Nov 14, 2021
cccs-RyanS pushed a commit to CybercentreCanada/superset that referenced this pull request Dec 17, 2021
QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 29, 2021
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.3.0 labels Mar 13, 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 #bug:blocking! Blocking issues with high priority preset-io size/M 🚢 1.3.0
Projects
None yet
6 participants