fix(datasets): keep a metric's warning text when editing from Explore - #43177
Open
bikash-barnwal wants to merge 1 commit into
Open
fix(datasets): keep a metric's warning text when editing from Explore#43177bikash-barnwal wants to merge 1 commit into
bikash-barnwal wants to merge 1 commit into
Conversation
The Edit dataset modal reached from Explore always showed an empty Warning
field for a metric whose warning was set, while the same modal reached from
Datasets showed it correctly.
The two entry points hand the editor two different metric shapes. The dataset
API returns `extra` as a JSON string, but Explore's bootstrap payload goes
through `SqlMetric.data`, which flattens `extra` into `warning_markdown` and
drops the raw string. The editor unconditionally read the warning out of
`JSON.parse(metric.extra || '{}')` and then normalized it with `|| ''`, so on
the Explore path the flattened value was overwritten with an empty string —
the field rendered blank even though the value was persisted, which is why the
warning icon stayed put next to the metric name.
Extract the transform as `hydrateMetricExtra` and treat the parsed `extra` as
authoritative only when `extra` is actually present, so an already-flattened
warning survives. An empty warning inside a present `extra` still clears the
field.
Fixes apache#42704
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Code Review Agent Run #2f1171Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
dosubot
Bot
added
change:frontend
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #43177 +/- ##
=======================================
Coverage 66.70% 66.70%
=======================================
Files 2872 2872
Lines 163447 163451 +4
Branches 37727 37730 +3
=======================================
+ Hits 109024 109028 +4
Misses 52293 52293
Partials 2130 2130
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SUMMARY
The Edit dataset modal reached from Explore (dataset overflow menu → Edit dataset) always showed an empty Warning field for a metric whose warning was set, while the same modal reached from Datasets → Edit dataset showed the value correctly. The value was persisted the whole time — hence the warning icon staying put next to the metric name — it just never made it back into the field.
The two entry points hand
DatasourceEditortwo different metric shapes:extraas a JSON string;SqlMetric.data, which flattensextrainto a top-levelwarning_markdownand drops the rawextrastring.The editor unconditionally read the warning out of
JSON.parse(metric.extra || '{}')and normalized the result with|| '', so on the Explore path the already-flattened value was overwritten with an empty string.This extracts the transform as
hydrateMetricExtraand treats the parsedextraas authoritative only whenextrais actually present, so an already-flattened warning survives. An empty warning inside a presentextrastill clears the field, so clearing a warning from the Datasets page keeps working.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before: Warning field blank on reopen from Explore (warning icon still shown).
After: the saved warning text is rendered in the field.
TESTING INSTRUCTIONS
Manually:
...beside the dataset name → Edit dataset.ADDITIONAL INFORMATION
🤖 Generated with Claude Code