Skip to content
New issue

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

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

Already on GitHub? # to your account

chore(dashboard): Ignore empty json value for overwrite confirm #22214

Conversation

justinpark
Copy link
Member

SUMMARY

Minor regression from #21819
When user change a single layout but no filter_scope updates, there's unrelated overwrite confirmation kept popping up because the default json value is not empty string but an empty object {}.

This commit updates the default json value to skip the unnecessary overwrite warning.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

ignore-empty-json-overwrite.mov

After:

No overwrite confirm

TESTING INSTRUCTIONS

  • Enable CONFIRM_DASHBOARD_DIFF
  • Edit dashboard and change one chart position
  • Save and check the confirmation

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

cc: @ktmud @john-bodley

@codecov
Copy link

codecov bot commented Nov 24, 2022

Codecov Report

Merging #22214 (265564c) into master (888f43c) will decrease coverage by 0.03%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##           master   #22214      +/-   ##
==========================================
- Coverage   66.96%   66.92%   -0.04%     
==========================================
  Files        1835     1835              
  Lines       69927    69988      +61     
  Branches     7590     7612      +22     
==========================================
+ Hits        46826    46839      +13     
- Misses      21135    21183      +48     
  Partials     1966     1966              
Flag Coverage Δ
javascript 53.75% <50.00%> (-0.06%) ⬇️

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

Impacted Files Coverage Δ
...t-frontend/src/dashboard/util/getOverwriteItems.ts 90.90% <50.00%> (ø)
...ntend/plugins/plugin-chart-echarts/src/defaults.ts 20.00% <0.00%> (-80.00%) ⬇️
.../src/explore/components/ControlPanelsContainer.tsx 75.35% <0.00%> (-2.03%) ⬇️
...tend/plugins/plugin-chart-echarts/src/Pie/types.ts 100.00% <0.00%> (ø)
...tend/plugins/plugin-chart-echarts/src/constants.ts 100.00% <0.00%> (ø)
...nd/plugins/plugin-chart-echarts/src/Graph/types.ts 100.00% <0.00%> (ø)
...nd/plugins/plugin-chart-echarts/src/Radar/types.ts 100.00% <0.00%> (ø)
...d/plugins/plugin-chart-echarts/src/Funnel/types.ts 100.00% <0.00%> (ø)
.../plugins/plugin-chart-echarts/src/BoxPlot/types.ts 0.00% <0.00%> (ø)
.../plugins/plugin-chart-echarts/src/Treemap/types.ts 100.00% <0.00%> (ø)
... and 29 more

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

Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

LGTM

@zhaoyongjie zhaoyongjie merged commit cc2334e into apache:master Nov 26, 2022
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 and removed 🚢 2.1.3 labels Mar 13, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/XS 🚢 2.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants