jeffreythewang closed pull request #4668: Add NullOption, and use it for Filters
URL: https://github.com/apache/incubator-superset/pull/4668
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/superset/assets/javascripts/components/AlteredSliceTag.jsx
b/superset/assets/javascripts/components/AlteredSliceTag.jsx
index eb24424e8d..131d56eab7 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 0000000000..6dd30a6df9
--- /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 (
+ <span>
+ <span className="m-r-5 option-label">{'NULL'}</span>
+ <InfoTooltipWithTrigger
+ className="m-r-5 text-muted"
+ tooltip="Denotes `null` value for filter (e.g. `where col_name is
null`)"
+ label="null-value"
+ />
+ </span>
+ );
+ }
+ return <span>{option.label}</span>;
+}
+
+NullOption.propTypes = propTypes;
diff --git a/superset/assets/javascripts/explore/components/controls/Filter.jsx
b/superset/assets/javascripts/explore/components/controls/Filter.jsx
index 49a9751f3b..ca92566fd8 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 (
<SelectControl
multi={operator.multi}
freeForm
name="filter-value"
- value={filter.val}
+ nullable
+ value={value}
+ valueRenderer={v => <NullOption option={v} />}
+ optionRenderer={v => <NullOption option={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 041dd6fe2c..47357f603e 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 8b78fc027b..f268ba6e71 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 0000000000..e65fcaf193
--- /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 => <NullOption {...o} />;
+ beforeEach(() => {
+ wrapper = shallow(factory(defaultProps));
+ });
+ it('is a valid element', () => {
+ expect(React.isValidElement(<NullOption {...defaultProps}
/>)).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');
+ });
+});
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
