Translate string to array for multi fields in getControlsState #5057
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5057 +/- ##
=======================================
Coverage 77.53% 77.53%
=======================================
Files 44 44
Lines 8712 8712
=======================================
Hits 6755 6755
Misses 1957 1957Continue to review full report at Codecov.
|
superset/assets/src/explore/store.js
Outdated
| @@ -56,6 +56,10 @@ export function getControlsState(state, form_data) { | |||
| delete control.mapStateToProps; | |||
| } | |||
|
|
|||
| if (control.multi && typeof formData[k] === 'string') { | |||
There was a problem hiding this comment.
nit: formData[k] = (control.multi && typeof formData[k] === 'string') ? [formData[k]] : formData[k]
There was a problem hiding this comment.
Would !Array.isArray(formData[k]) be better?
There was a problem hiding this comment.
I think we'd want to stick with typeof formData[k] === 'string' because formData[k] might be undefined or null and we would want to leave it as null.
There was a problem hiding this comment.
Though I think it can be a int / float too.
There was a problem hiding this comment.
Ah ok, updated it to formData[k] && !Array.isArray(formData[k]).
|
🚢 |
michellethomas
force-pushed
the
fixing_histogram_add_multi
branch
from
f5badc8
to
d955c13
Compare
|
@mistercrunch @graceguo-supercat can one of you merge if |
|
Actually after thinking a bit more I'm thinking while this fixes the problem in the explore view, I don't think it fixes the issue in the dashboard view. |
|
@michellethomas maybe we need to refactor some sort of |
…e#5057) * Translate string to array for multi fields in getControlsState * Updating format to fit on one line
…e#5057) * Translate string to array for multi fields in getControlsState * Updating format to fit on one line
mistercrunch
added
🏷️ bot
In the explore store.js, I'm adding logic that will handle a string value being passed to a multi control by making it an array. This would handle errors with existing slices when making a SelectControl multi.
Fixes #5011
@mistercrunch @graceguo-supercat