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: use dashboard id for stable cache key #11293

Merged
merged 1 commit into from
Oct 16, 2020
Merged

Conversation

ktmud
Copy link
Member

@ktmud ktmud commented Oct 15, 2020

SUMMARY

This fixes a bug where dashboard cache used while visiting dashboard via slug is not updated when editing an embedded chart.

I used dashboard.slug in the default dashboard __repr__ string because I wanted to make it human readable, but since this is mostly used in cache and not all dashboards have slugs, human readability doesn't really matter. It's more important to have stable cache keys.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TEST PLAN

  1. Enable DASHBOARD_CACHE feature flag
  2. Visit a dashboard with slug
  3. Edit a chart in the dashboard
  4. In base you will not see the chart updates when you return to dashboard and refresh. After the fix, the chart in dashboard should be updated.

ADDITIONAL INFORMATION

@codecov-io
Copy link

codecov-io commented Oct 15, 2020

Codecov Report

Merging #11293 into master will decrease coverage by 4.22%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #11293      +/-   ##
==========================================
- Coverage   65.61%   61.38%   -4.23%     
==========================================
  Files         834      834              
  Lines       39565    39562       -3     
  Branches     3610     3610              
==========================================
- Hits        25960    24286    -1674     
- Misses      13496    15095    +1599     
- Partials      109      181      +72     
Flag Coverage Δ
#cypress ?
#javascript 62.71% <ø> (ø)
#python 60.59% <100.00%> (-0.19%) ⬇️

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

Impacted Files Coverage Δ
superset/models/dashboard.py 80.63% <100.00%> (ø)
superset-frontend/src/SqlLab/App.jsx 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/explore/App.jsx 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/dashboard/App.jsx 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/explore/index.jsx 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/dashboard/index.jsx 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/setup/setupColors.js 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/chart/ChartContainer.jsx 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/setup/setupFormatters.js 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/explore/reducers/index.js 0.00% <0.00%> (-100.00%) ⬇️
... and 171 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 233e67a...c45bb83. Read the comment docs.

Copy link

@graceguo-supercat graceguo-supercat left a comment

Choose a reason for hiding this comment

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

LGTM

@ktmud ktmud merged commit dde69d8 into apache:master Oct 16, 2020
graceguo-supercat pushed a commit to airbnb/superset-fork that referenced this pull request Oct 16, 2020
villebro pushed a commit to preset-io/superset that referenced this pull request Oct 19, 2020
suddjian pushed a commit that referenced this pull request Oct 19, 2020
…11325)

* fix: use dashboard id for stable cache key (#11293)

* fix: button translations missing (#11187)

* button translations missing

* blank space before text

* feat: update time_compare description and choices (#11294)

* feat: update time_compare description and choices

* Update sections.jsx

* fix(extra-filters): add logic for identifying applied extra filters

* lint

Co-authored-by: Jesse Yang <[email protected]>
Co-authored-by: rubenSastre <[email protected]>
Co-authored-by: Erik Ritter <[email protected]>
suddjian added a commit that referenced this pull request Oct 28, 2020
* Initial commit of new filters badge.

* refactor applied/rejected filters code

* finished filter indicators

* filter badge tested

* unnecessary imports

* formatting and types

* fixes

* license

* code quality tweaks

* state management for showing focused filter scope

* clean up filter key extraction code

* remove unnecessary styles

* temp css to demonstrate highlighting

* fix focused filter logic

* no more color badges

* new toys for highlighting dash components (#11144)

* tweak style for the filter chart when filter is focused

* style: Filters p0 css2 (#11151)

* nixing background tweak

* src paths

* another quick theme color

* src paths, adjusting pill icon color, changing icons, showing applied/busted counts

* linting stuff

* fixing and tweaking tests

* show filter indicator when filters are not active

* chart title bar cleanup

* open the right panel when popover opens

* unused import

* fix EditableTitle tests

* margin on dashboard header

* show the chart dropdown menu

* fix blur filter breaking dropdowns

* style tweak - no pointer events when irrelevant charts are blurred

* fix box shadow on filter highlight

* it's an array

* attempt fixing e2e

* style: filters p0 icon churn (#11215)

* new filters icon

* icon styling

* bigger icons in list views

* better sizing of table actions and favStars

* more icon sizing...

* fixing more button size jankiness

* linting

* Filters performance (#11255)

* fixing time filter "ok" button

* making unset filter menu collapsible

* sort alphabetically

* fix highlighting when removing items

* try a flex layout (for browser render perf)

* more specific transitioning

* temp: comment out some code as a test

* temp: comment out more code

* temp: remove possibly expensive computations from ChartHolder

* Revert "temp: comment out some code as a test"

This reverts commit 309b880.

* Revert "temp: comment out more code"

This reverts commit 64c88b2.

* Revert "temp: remove possibly expensive computations from ChartHolder"

This reverts commit 37ce021.

* experiment: upgrade react-select to v3

* Revert "experiment: upgrade react-select to v3"

This reverts commit c3972ba.

* fix the damn problem

* remove code used for testing purposes

* awful hack to avoid adding a class to a container

* approaching infinity... and not beyond!

* fix ref forwarding

* add theme to tests as necessary

* fix(extra-filters): add logic for identifying applied extra filters (#11325)

* fix: use dashboard id for stable cache key (#11293)

* fix: button translations missing (#11187)

* button translations missing

* blank space before text

* feat: update time_compare description and choices (#11294)

* feat: update time_compare description and choices

* Update sections.jsx

* fix(extra-filters): add logic for identifying applied extra filters

* lint

Co-authored-by: Jesse Yang <[email protected]>
Co-authored-by: rubenSastre <[email protected]>
Co-authored-by: Erik Ritter <[email protected]>

* address design feedback

* slight tweak to panel logic, keep panels open that user has opened

* rearrange code to be more graceful

* fix: bump superset-ui/core (#11385)

* use is_dttm instead of is_temporal

* types, names

* only show unset filter panel if there are unset filters

* fix highlighting the filter control

* fix filterbox layout

* translations

* fix cypress

* actually add the test attribute

* Update superset-frontend/src/dashboard/components/DashboardBuilder.jsx

Co-authored-by: Evan Rusackas <[email protected]>

* Update superset-frontend/src/dashboard/components/DashboardBuilder.jsx

Co-authored-by: Evan Rusackas <[email protected]>

* formatting

* add link comment to hack

* Update superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx

Co-authored-by: Evan Rusackas <[email protected]>

* stop importing lodash

* Update superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx

Co-authored-by: Evan Rusackas <[email protected]>

* Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx

Co-authored-by: Evan Rusackas <[email protected]>

* Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx

Co-authored-by: Evan Rusackas <[email protected]>

* Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx

Co-authored-by: Evan Rusackas <[email protected]>

* Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx

Co-authored-by: Evan Rusackas <[email protected]>

* skip broken test

* Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx

Co-authored-by: Evan Rusackas <[email protected]>

* Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx

Co-authored-by: Evan Rusackas <[email protected]>

* adjust colors of titles

* linting

* no indicators when chart is loading

* support all time fields

* fix lock file

Co-authored-by: Natalie Ruhe <[email protected]>
Co-authored-by: Evan Rusackas <[email protected]>
Co-authored-by: Ville Brofeldt <[email protected]>
Co-authored-by: Jesse Yang <[email protected]>
Co-authored-by: rubenSastre <[email protected]>
Co-authored-by: Erik Ritter <[email protected]>
Co-authored-by: Ville Brofeldt <[email protected]>
auxten pushed a commit to auxten/incubator-superset that referenced this pull request Nov 20, 2020
auxten pushed a commit to auxten/incubator-superset that referenced this pull request Nov 20, 2020
* Initial commit of new filters badge.

* refactor applied/rejected filters code

* finished filter indicators

* filter badge tested

* unnecessary imports

* formatting and types

* fixes

* license

* code quality tweaks

* state management for showing focused filter scope

* clean up filter key extraction code

* remove unnecessary styles

* temp css to demonstrate highlighting

* fix focused filter logic

* no more color badges

* new toys for highlighting dash components (apache#11144)

* tweak style for the filter chart when filter is focused

* style: Filters p0 css2 (apache#11151)

* nixing background tweak

* src paths

* another quick theme color

* src paths, adjusting pill icon color, changing icons, showing applied/busted counts

* linting stuff

* fixing and tweaking tests

* show filter indicator when filters are not active

* chart title bar cleanup

* open the right panel when popover opens

* unused import

* fix EditableTitle tests

* margin on dashboard header

* show the chart dropdown menu

* fix blur filter breaking dropdowns

* style tweak - no pointer events when irrelevant charts are blurred

* fix box shadow on filter highlight

* it's an array

* attempt fixing e2e

* style: filters p0 icon churn (apache#11215)

* new filters icon

* icon styling

* bigger icons in list views

* better sizing of table actions and favStars

* more icon sizing...

* fixing more button size jankiness

* linting

* Filters performance (apache#11255)

* fixing time filter "ok" button

* making unset filter menu collapsible

* sort alphabetically

* fix highlighting when removing items

* try a flex layout (for browser render perf)

* more specific transitioning

* temp: comment out some code as a test

* temp: comment out more code

* temp: remove possibly expensive computations from ChartHolder

* Revert "temp: comment out some code as a test"

This reverts commit 309b880.

* Revert "temp: comment out more code"

This reverts commit 64c88b2.

* Revert "temp: remove possibly expensive computations from ChartHolder"

This reverts commit 37ce021.

* experiment: upgrade react-select to v3

* Revert "experiment: upgrade react-select to v3"

This reverts commit c3972ba.

* fix the damn problem

* remove code used for testing purposes

* awful hack to avoid adding a class to a container

* approaching infinity... and not beyond!

* fix ref forwarding

* add theme to tests as necessary

* fix(extra-filters): add logic for identifying applied extra filters (apache#11325)

* fix: use dashboard id for stable cache key (apache#11293)

* fix: button translations missing (apache#11187)

* button translations missing

* blank space before text

* feat: update time_compare description and choices (apache#11294)

* feat: update time_compare description and choices

* Update sections.jsx

* fix(extra-filters): add logic for identifying applied extra filters

* lint

Co-authored-by: Jesse Yang <[email protected]>
Co-authored-by: rubenSastre <[email protected]>
Co-authored-by: Erik Ritter <[email protected]>

* address design feedback

* slight tweak to panel logic, keep panels open that user has opened

* rearrange code to be more graceful

* fix: bump superset-ui/core (apache#11385)

* use is_dttm instead of is_temporal

* types, names

* only show unset filter panel if there are unset filters

* fix highlighting the filter control

* fix filterbox layout

* translations

* fix cypress

* actually add the test attribute

* Update superset-frontend/src/dashboard/components/DashboardBuilder.jsx

Co-authored-by: Evan Rusackas <[email protected]>

* Update superset-frontend/src/dashboard/components/DashboardBuilder.jsx

Co-authored-by: Evan Rusackas <[email protected]>

* formatting

* add link comment to hack

* Update superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx

Co-authored-by: Evan Rusackas <[email protected]>

* stop importing lodash

* Update superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx

Co-authored-by: Evan Rusackas <[email protected]>

* Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx

Co-authored-by: Evan Rusackas <[email protected]>

* Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx

Co-authored-by: Evan Rusackas <[email protected]>

* Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx

Co-authored-by: Evan Rusackas <[email protected]>

* Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx

Co-authored-by: Evan Rusackas <[email protected]>

* skip broken test

* Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx

Co-authored-by: Evan Rusackas <[email protected]>

* Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx

Co-authored-by: Evan Rusackas <[email protected]>

* adjust colors of titles

* linting

* no indicators when chart is loading

* support all time fields

* fix lock file

Co-authored-by: Natalie Ruhe <[email protected]>
Co-authored-by: Evan Rusackas <[email protected]>
Co-authored-by: Ville Brofeldt <[email protected]>
Co-authored-by: Jesse Yang <[email protected]>
Co-authored-by: rubenSastre <[email protected]>
Co-authored-by: Erik Ritter <[email protected]>
Co-authored-by: Ville Brofeldt <[email protected]>
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 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 size/XS 🚢 1.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants