[Explore] Handle empty metrics control data #5241
Conversation
| @@ -223,7 +223,7 @@ export default class AdhocFilterControl extends React.Component { | |||
| } else if (option instanceof AdhocMetric) { | |||
| return { ...option, filterOptionName: '_adhocmetric_' + option.label }; | |||
| } | |||
| return null; | |||
| return ''; | |||
There was a problem hiding this comment.
Do we really want to return an empty string here? Generally checking for null is preferable to an empty string.
There was a problem hiding this comment.
The caller for these 2 optionsForSelect functions expecting a string array result, and don't handle null in the array. if i return null and caller brought result to do sort, another exception will be thrown.
So i come up another solution that combines filter (empty metrics) + map metrics to new attribute name.
graceguo-supercat
force-pushed
the
gg-ExploreControlsNotifyJS
branch
from
2be21b9
to
938f89f
Compare
graceguo-supercat
force-pushed
the
gg-ExploreControlsNotifyJS
branch
from
938f89f
to
b1c6771
Compare
Codecov Report
@@ Coverage Diff @@
## master #5241 +/- ##
==========================================
+ Coverage 60.76% 60.78% +0.01%
==========================================
Files 258 258
Lines 19701 19700 -1
Branches 1970 1970
==========================================
+ Hits 11972 11974 +2
+ Misses 7720 7717 -3
Partials 9 9
Continue to review full report at Codecov.
|
(cherry picked from commit 13cbf80)
mistercrunch
added
🏷️ bot
when metrics is empty, Explore controls throw exceptions:
I think js bundle for notify library is also problematic. We probably should remove the usage and replace with other library. But it's out of scope of this issue.
@GabeLoins @timifasubaa @michellethomas @mistercrunch