From 86463d26f32db5b8ceaf52d51029aca0638b13f8 Mon Sep 17 00:00:00 2001 From: Jeffrey Wang Date: Thu, 22 Mar 2018 10:43:14 -0400 Subject: [PATCH 1/3] Add NullOption, and use it for Filters --- .../components/AlteredSliceTag.jsx | 4 +-- .../javascripts/components/NullOption.jsx | 26 +++++++++++++++ .../explore/components/controls/Filter.jsx | 8 ++++- .../components/controls/FilterControl.jsx | 2 +- .../components/controls/SelectControl.jsx | 11 +++++-- .../components/NullOption_spec.jsx | 33 +++++++++++++++++++ 6 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 superset/assets/javascripts/components/NullOption.jsx create mode 100644 superset/assets/spec/javascripts/components/NullOption_spec.jsx diff --git a/superset/assets/javascripts/components/AlteredSliceTag.jsx b/superset/assets/javascripts/components/AlteredSliceTag.jsx index eb24424e8d41f..131d56eab7248 100644 --- a/superset/assets/javascripts/components/AlteredSliceTag.jsx +++ b/superset/assets/javascripts/components/AlteredSliceTag.jsx @@ -61,7 +61,7 @@ export default class AlteredSliceTag extends React.Component { return '[]'; } return value.map((v) => { - const filterVal = v.val.constructor === Array ? `[${v.val.join(', ')}]` : v.val; + const filterVal = Array.isArray(v.val) ? `[${v.val.join(', ')}]` : v.val; return `${v.col} ${v.op} ${filterVal}`; }).join(', '); } else if (controls[key] && controls[key].type === 'BoundsControl') { @@ -70,7 +70,7 @@ export default class AlteredSliceTag extends React.Component { return value.map(v => JSON.stringify(v)).join(', '); } else if (typeof value === 'boolean') { return value ? 'true' : 'false'; - } else if (value.constructor === Array) { + } else if (Array.isArray(value)) { return value.length ? value.join(', ') : '[]'; } else if (typeof value === 'string' || typeof value === 'number') { return value; diff --git a/superset/assets/javascripts/components/NullOption.jsx b/superset/assets/javascripts/components/NullOption.jsx new file mode 100644 index 0000000000000..6dd30a6df98b2 --- /dev/null +++ b/superset/assets/javascripts/components/NullOption.jsx @@ -0,0 +1,26 @@ +import React from 'react'; +import PropTypes from 'prop-types'; + +import InfoTooltipWithTrigger from './InfoTooltipWithTrigger'; + +const propTypes = { + option: PropTypes.object.isRequired, +}; + +export default function NullOption({ option }) { + if (option.value === null) { + return ( + + {'NULL'} + + + ); + } + return {option.label}; +} + +NullOption.propTypes = propTypes; diff --git a/superset/assets/javascripts/explore/components/controls/Filter.jsx b/superset/assets/javascripts/explore/components/controls/Filter.jsx index 49a9751f3be00..ca92566fd80b5 100644 --- a/superset/assets/javascripts/explore/components/controls/Filter.jsx +++ b/superset/assets/javascripts/explore/components/controls/Filter.jsx @@ -3,6 +3,7 @@ import PropTypes from 'prop-types'; import Select from 'react-select'; import { Button, Row, Col } from 'react-bootstrap'; import SelectControl from './SelectControl'; +import NullOption from '../../../components/NullOption'; import { t } from '../../../locales'; const operatorsArr = [ @@ -91,13 +92,18 @@ export default class Filter extends React.Component { renderFilterFormControl(filter) { const operator = operators[filter.op]; if (operator.useSelect && !this.props.having) { + const value = Array.isArray(filter.val) ? + filter.val.map(v => ({ value: v, label: v })) : { value: filter.val, label: filter.val }; // TODO should use a simple Select, not a control here... return ( } + optionRenderer={v => } isLoading={this.props.valuesLoading} choices={this.props.valueChoices} onChange={this.changeSelect.bind(this)} diff --git a/superset/assets/javascripts/explore/components/controls/FilterControl.jsx b/superset/assets/javascripts/explore/components/controls/FilterControl.jsx index 041dd6fe2c181..47357f603eeeb 100644 --- a/superset/assets/javascripts/explore/components/controls/FilterControl.jsx +++ b/superset/assets/javascripts/explore/components/controls/FilterControl.jsx @@ -98,7 +98,7 @@ export default class FilterControl extends React.Component { } // Clear selected values and refresh upon column change if (control === 'col') { - if (modifiedFilter.val.constructor === Array) { + if (Array.isArray(modifiedFilter.val)) { modifiedFilter.val = []; } else if (typeof modifiedFilter.val === 'string') { modifiedFilter.val = ''; diff --git a/superset/assets/javascripts/explore/components/controls/SelectControl.jsx b/superset/assets/javascripts/explore/components/controls/SelectControl.jsx index 8b78fc027be0a..f268ba6e71cc2 100644 --- a/superset/assets/javascripts/explore/components/controls/SelectControl.jsx +++ b/superset/assets/javascripts/explore/components/controls/SelectControl.jsx @@ -16,9 +16,15 @@ const propTypes = { label: PropTypes.string, multi: PropTypes.bool, name: PropTypes.string.isRequired, + nullable: PropTypes.bool, onChange: PropTypes.func, onFocus: PropTypes.func, - value: PropTypes.oneOfType([PropTypes.string, PropTypes.number, PropTypes.array]), + value: PropTypes.oneOfType([ + PropTypes.string, + PropTypes.number, + PropTypes.array, + PropTypes.object, + ]), showHeader: PropTypes.bool, optionRenderer: PropTypes.func, valueRenderer: PropTypes.func, @@ -35,6 +41,7 @@ const defaultProps = { isLoading: false, label: null, multi: false, + nullable: false, onChange: () => {}, onFocus: () => {}, showHeader: true, @@ -90,7 +97,7 @@ export default class SelectControl extends React.PureComponent { if (props.freeForm) { // For FreeFormSelect, insert value into options if not exist const values = options.map(c => c.value); - if (props.value) { + if (props.value || (props.nullable && props.value === null)) { let valuesToAdd = props.value; if (!Array.isArray(valuesToAdd)) { valuesToAdd = [valuesToAdd]; diff --git a/superset/assets/spec/javascripts/components/NullOption_spec.jsx b/superset/assets/spec/javascripts/components/NullOption_spec.jsx new file mode 100644 index 0000000000000..e65fcaf193de0 --- /dev/null +++ b/superset/assets/spec/javascripts/components/NullOption_spec.jsx @@ -0,0 +1,33 @@ +import React from 'react'; +import { expect } from 'chai'; +import { describe, it } from 'mocha'; +import { shallow } from 'enzyme'; + +import InfoTooltipWithTrigger from '../../../javascripts/components/InfoTooltipWithTrigger'; +import NullOption from '../../../javascripts/components/NullOption'; + +describe('NullOption', () => { + const defaultProps = { + option: { + value: 'foo', + label: 'foo', + }, + }; + + let wrapper; + const factory = o => ; + beforeEach(() => { + wrapper = shallow(factory(defaultProps)); + }); + it('is a valid element', () => { + expect(React.isValidElement()).to.equal(true); + }); + it('renders label', () => { + expect(wrapper.find('span').text()).to.equal(defaultProps.option.label); + }); + it('renders null label with tooltip', () => { + wrapper = shallow(factory({ option: { value: null, label: null } })); + expect(wrapper.find(InfoTooltipWithTrigger)).to.have.length(1); + expect(wrapper.find('.option-label').text()).to.equal('NULL'); + }); +}); From 871c4a267e3da876375efaee1711442b24d75418 Mon Sep 17 00:00:00 2001 From: Jeffrey Wang Date: Thu, 22 Mar 2018 15:09:38 -0400 Subject: [PATCH 2/3] trim form data non-recursively --- superset/assets/javascripts/explore/exploreUtils.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/superset/assets/javascripts/explore/exploreUtils.js b/superset/assets/javascripts/explore/exploreUtils.js index 1c1271b0458c2..6805632b9b2ca 100644 --- a/superset/assets/javascripts/explore/exploreUtils.js +++ b/superset/assets/javascripts/explore/exploreUtils.js @@ -1,6 +1,16 @@ /* eslint camelcase: 0 */ import URI from 'urijs'; +export function trimFormData(formData) { + const cleaned = { ...formData }; + Object.entries(formData).forEach(([k, v]) => { + if (v === null || v === undefined) { + delete cleaned[k]; + } +}); + return cleaned; +} + export function getChartKey(explore) { const slice = explore.slice; return slice ? ('slice_' + slice.slice_id) : 'slice'; @@ -14,8 +24,7 @@ export function getAnnotationJsonUrl(slice_id, form_data, isNative) { const endpoint = isNative ? 'annotation_json' : 'slice_json'; return uri.pathname(`/superset/${endpoint}/${slice_id}`) .search({ - form_data: JSON.stringify(form_data, - (key, value) => value === null ? undefined : value), + form_data: JSON.stringify(trimFormData(form_data)), }).toString(); } From 18eb303346a4e56a8908b6de81b0258ae02a7624 Mon Sep 17 00:00:00 2001 From: Jeffrey Wang Date: Thu, 22 Mar 2018 16:07:57 -0400 Subject: [PATCH 3/3] lint --- superset/assets/javascripts/explore/exploreUtils.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/superset/assets/javascripts/explore/exploreUtils.js b/superset/assets/javascripts/explore/exploreUtils.js index 6805632b9b2ca..adfe41f1b2032 100644 --- a/superset/assets/javascripts/explore/exploreUtils.js +++ b/superset/assets/javascripts/explore/exploreUtils.js @@ -5,9 +5,9 @@ export function trimFormData(formData) { const cleaned = { ...formData }; Object.entries(formData).forEach(([k, v]) => { if (v === null || v === undefined) { - delete cleaned[k]; - } -}); + delete cleaned[k]; + } + }); return cleaned; }