Skip to content

fix(explore): recompute currency_formats when dataset is saved from chart editor - #42478

Closed
SkinnyPigeon wants to merge 1 commit into
apache:masterfrom
SkinnyPigeon:fix-charts-update-currency-in-chart-editor
Closed

fix(explore): recompute currency_formats when dataset is saved from chart editor#42478
SkinnyPigeon wants to merge 1 commit into
apache:masterfrom
SkinnyPigeon:fix-charts-update-currency-in-chart-editor

Conversation

@SkinnyPigeon

Copy link
Copy Markdown
Contributor

SUMMARY

When editing a metric's currency via Edit Dataset from the chart editor, the chart preview did not reflect the new currency until the page was refreshed.

Root cause: currency_formats — a derived map of metric_name → Currency used by all chart plugins (as camelCase currencyFormats) — is only computed once in hydrateExplore.ts at initial page load. When changeDatasource is dispatched after an inline dataset save, it passes the raw API response directly to Redux. The API response contains the updated metrics[].currency data but never carries the pre-computed currency_formats field, so chart plugins receive currencyFormats: {} and render without any currency formatting.

Why removal appeared to work: Removing a currency also results in currencyFormats: {}, which is the correct outcome (no currency → no formatting), so it appeared to work immediately.

Fix: In changeDatasource — the canonical entry point for all datasource updates — derive currency_formats from metrics[].currency before dispatching, exactly mirroring what hydrateExplore.ts does on initial page load. This ensures every caller gets consistent, up-to-date currency formatting without any component needing to know about this derived field.

Fixes #42468

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before: Changing a metric's currency in Edit Dataset has no effect on the chart preview; a full page refresh is required.

After: The chart preview immediately reflects the updated currency after saving the dataset.

Updating.Currency.mp4

TESTING INSTRUCTIONS

  1. Open an existing chart (e.g. a Big Number chart backed by a dataset with a metric).
  2. In Chart Source, click ... beside the dataset name → Edit Dataset.
  3. Open the Metrics tab, expand a metric, and change its currency (e.g. from USD to EUR).
  4. Save the dataset and return to the chart preview.
  5. Verify the chart immediately renders with the updated currency symbol — no page refresh required.

Automated: npx jest --testPathPatterns="datasourcesActions.test" — all 5 tests pass, including a new test that verifies currency_formats is correctly derived from metric currencies when changeDatasource is called.

ADDITIONAL INFORMATION

@dosubot dosubot Bot added change:frontend Requires changing the frontend explore Namespace | Anything related to Explore labels Jul 27, 2026
@bito-code-review

bito-code-review Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Bito Review Skipped - Source Branch Not Found

Bito didn’t review this change because the pull request is no longer valid. It may have been merged, or the source/target branch may no longer exist.

@SkinnyPigeon
SkinnyPigeon deleted the fix-charts-update-currency-in-chart-editor branch July 27, 2026 13:14
@netlify

netlify Bot commented Jul 27, 2026

Copy link
Copy Markdown

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit c0d7c3c
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/6a6759951735ab0008856804
😎 Deploy Preview https://deploy-preview-42478--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.24%. Comparing base (6856d0f) to head (c0d7c3c).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #42478      +/-   ##
==========================================
- Coverage   65.25%   65.24%   -0.01%     
==========================================
  Files        2795     2795              
  Lines      157639   157643       +4     
  Branches    36052    36053       +1     
==========================================
- Hits       102869   102856      -13     
- Misses      52793    52810      +17     
  Partials     1977     1977              
Flag Coverage Δ
javascript 71.32% <100.00%> (-0.02%) ⬇️

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

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:frontend Requires changing the frontend explore Namespace | Anything related to Explore size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add/Edit Chart: metric currency change in Edit Dataset is not reflected in chart preview until page refresh

1 participant