-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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): metric label disappearing in some scenarios #16190
Conversation
Codecov Report
@@ Coverage Diff @@
## master #16190 +/- ##
==========================================
- Coverage 76.83% 76.82% -0.01%
==========================================
Files 996 996
Lines 53060 53062 +2
Branches 6766 6767 +1
==========================================
Hits 40766 40766
- Misses 12066 12068 +2
Partials 228 228
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
/testenv up FEATURE_ENABLE_EXPLORE_DRAG_AND_DROP=true |
@kgabryje Ephemeral environment spinning up at http://35.155.136.177:8080. Credentials are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM and tested to work well!
Ephemeral environment shutdown and build artifacts deleted. |
🏷 2021.31 |
(cherry picked from commit 98fc29c)
(cherry picked from commit 98fc29c)
(cherry picked from commit 98fc29c)
(cherry picked from commit 98fc29c)
(cherry picked from commit eed3c58)
SUMMARY
When user added an adhoc metric, then opened a popover to edit that metric and clicked "Save" without doing any changes, the metric label would disappear. This PR adds a check if old and edited metric are equal - if they are, no actions are taken.
Basically what happened was that when we saved a metric without any changes, we didn't call
coerceAdhocMetrics
(which converts an object to AdhocMetric), but we did callonChange
. The result was that we passed a normal object instead of AdhocMetric instance to value renderer, and in value renderer we checked for instanceof AdhocMetricBEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before: see linked issue
After:
https://user-images.githubusercontent.com/15073128/129003100-1701671f-b658-49ad-bb2e-3ed13c0e5207.mov
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION
CC @jinghua-qa @junlincc