Fixed filter removal bug (#3458)
* Fixed bugs when removing filter, switching operators, and switching columns * Fixed lint errors for code style * Added more unit tests for FilterControl * Code format changes to meet standards
This commit is contained in:
Jeff Niu
Maxime Beauchemin
parent
ad604aed09
commit
dd72048320
|
|
@ -4,8 +4,6 @@ import Select from 'react-select';
|
|||
import { Button, Row, Col } from 'react-bootstrap';
|
||||
import SelectControl from './SelectControl';
|
||||
|
||||
const $ = window.$ = require('jquery');
|
||||
|
||||
const operatorsArr = [
|
||||
{ val: 'in', type: 'array', useSelect: true, multi: true },
|
||||
{ val: 'not in', type: 'array', useSelect: true, multi: true },
|
||||
|
|
@ -29,6 +27,8 @@ const propTypes = {
|
|||
filter: PropTypes.object.isRequired,
|
||||
datasource: PropTypes.object,
|
||||
having: PropTypes.bool,
|
||||
valuesLoading: PropTypes.bool,
|
||||
valueChoices: PropTypes.array,
|
||||
};
|
||||
|
||||
const defaultProps = {
|
||||
|
|
@ -36,61 +36,57 @@ const defaultProps = {
|
|||
removeFilter: () => {},
|
||||
datasource: null,
|
||||
having: false,
|
||||
valuesLoading: false,
|
||||
valueChoices: [],
|
||||
};
|
||||
|
||||
export default class Filter extends React.Component {
|
||||
constructor(props) {
|
||||
super(props);
|
||||
this.state = {
|
||||
valuesLoading: false,
|
||||
};
|
||||
}
|
||||
componentDidMount() {
|
||||
this.fetchFilterValues(this.props.filter.col);
|
||||
}
|
||||
fetchFilterValues(col) {
|
||||
const datasource = this.props.datasource;
|
||||
if (col && this.props.datasource && this.props.datasource.filter_select && !this.props.having) {
|
||||
this.setState({ valuesLoading: true });
|
||||
$.ajax({
|
||||
type: 'GET',
|
||||
url: `/superset/filter/${datasource.type}/${datasource.id}/${col}/`,
|
||||
success: (data) => {
|
||||
this.setState({ valuesLoading: false, valueChoices: data });
|
||||
},
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
switchFilterValue(prevOp, nextOp) {
|
||||
if (operators[prevOp].type !== operators[nextOp].type) {
|
||||
const val = this.props.filter.value;
|
||||
// Switch from array to string or vice versa
|
||||
const val = this.props.filter.val;
|
||||
let newVal;
|
||||
// switch from array to string
|
||||
if (operators[nextOp].type === 'string' && val && val.length > 0) {
|
||||
newVal = val[0];
|
||||
} else if (operators[nextOp].type === 'string' && val) {
|
||||
newVal = [val];
|
||||
if (operators[nextOp].type === 'string') {
|
||||
if (!val || !val.length) {
|
||||
newVal = '';
|
||||
} else {
|
||||
newVal = val[0];
|
||||
}
|
||||
} else if (operators[nextOp].type === 'array') {
|
||||
if (!val || !val.length) {
|
||||
newVal = [];
|
||||
} else {
|
||||
newVal = [val];
|
||||
}
|
||||
}
|
||||
this.props.changeFilter('val', newVal);
|
||||
this.props.changeFilter(['val', 'op'], [newVal, nextOp]);
|
||||
} else {
|
||||
// No value type change
|
||||
this.props.changeFilter('op', nextOp);
|
||||
}
|
||||
}
|
||||
|
||||
changeText(event) {
|
||||
this.props.changeFilter('val', event.target.value);
|
||||
}
|
||||
|
||||
changeSelect(value) {
|
||||
this.props.changeFilter('val', value);
|
||||
}
|
||||
|
||||
changeColumn(event) {
|
||||
this.props.changeFilter('col', event.value);
|
||||
this.fetchFilterValues(event.value);
|
||||
}
|
||||
|
||||
changeOp(event) {
|
||||
this.switchFilterValue(this.props.filter.op, event.value);
|
||||
this.props.changeFilter('op', event.value);
|
||||
}
|
||||
|
||||
removeFilter(filter) {
|
||||
this.props.removeFilter(filter);
|
||||
}
|
||||
|
||||
renderFilterFormControl(filter) {
|
||||
const operator = operators[filter.op];
|
||||
if (operator.useSelect && !this.props.having) {
|
||||
|
|
@ -101,8 +97,8 @@ export default class Filter extends React.Component {
|
|||
freeForm
|
||||
name="filter-value"
|
||||
value={filter.val}
|
||||
isLoading={this.state.valuesLoading}
|
||||
choices={this.state.valueChoices}
|
||||
isLoading={this.props.valuesLoading}
|
||||
choices={this.props.valueChoices}
|
||||
onChange={this.changeSelect.bind(this)}
|
||||
showHeader={false}
|
||||
/>
|
||||
|
|
|
|||
|
|
@ -3,6 +3,8 @@ import PropTypes from 'prop-types';
|
|||
import { Button, Row, Col } from 'react-bootstrap';
|
||||
import Filter from './Filter';
|
||||
|
||||
const $ = window.$ = require('jquery');
|
||||
|
||||
const propTypes = {
|
||||
name: PropTypes.string,
|
||||
onChange: PropTypes.func,
|
||||
|
|
@ -16,6 +18,46 @@ const defaultProps = {
|
|||
};
|
||||
|
||||
export default class FilterControl extends React.Component {
|
||||
|
||||
constructor(props) {
|
||||
super(props);
|
||||
const initialFilters = props.value.map(() => ({
|
||||
valuesLoading: false,
|
||||
valueChoices: [],
|
||||
}));
|
||||
this.state = {
|
||||
filters: initialFilters,
|
||||
};
|
||||
}
|
||||
|
||||
componentDidMount() {
|
||||
this.state.filters.forEach((filter, index) => this.fetchFilterValues(index));
|
||||
}
|
||||
|
||||
fetchFilterValues(index, column) {
|
||||
const datasource = this.props.datasource;
|
||||
const col = column || this.props.value[index].col;
|
||||
const having = this.props.name === 'having_filters';
|
||||
if (col && this.props.datasource && this.props.datasource.filter_select && !having) {
|
||||
this.setState((prevState) => {
|
||||
const newStateFilters = Object.assign([], prevState.filters);
|
||||
newStateFilters[index].valuesLoading = true;
|
||||
return { filters: newStateFilters };
|
||||
});
|
||||
$.ajax({
|
||||
type: 'GET',
|
||||
url: `/superset/filter/${datasource.type}/${datasource.id}/${col}/`,
|
||||
success: (data) => {
|
||||
this.setState((prevState) => {
|
||||
const newStateFilters = Object.assign([], prevState.filters);
|
||||
newStateFilters[index] = { valuesLoading: false, valueChoices: data };
|
||||
return { filters: newStateFilters };
|
||||
});
|
||||
},
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
addFilter() {
|
||||
const newFilters = Object.assign([], this.props.value);
|
||||
const col = this.props.datasource && this.props.datasource.filterable_cols.length > 0 ?
|
||||
|
|
@ -27,7 +69,15 @@ export default class FilterControl extends React.Component {
|
|||
val: this.props.datasource.filter_select ? [] : '',
|
||||
});
|
||||
this.props.onChange(newFilters);
|
||||
const nextIndex = this.state.filters.length;
|
||||
this.setState((prevState) => {
|
||||
const newStateFilters = Object.assign([], prevState.filters);
|
||||
newStateFilters.push({ valuesLoading: false, valueChoices: [] });
|
||||
return { filters: newStateFilters };
|
||||
});
|
||||
this.fetchFilterValues(nextIndex, col);
|
||||
}
|
||||
|
||||
changeFilter(index, control, value) {
|
||||
const newFilters = Object.assign([], this.props.value);
|
||||
const modifiedFilter = Object.assign({}, newFilters[index]);
|
||||
|
|
@ -38,12 +88,28 @@ export default class FilterControl extends React.Component {
|
|||
modifiedFilter[c] = value[i];
|
||||
});
|
||||
}
|
||||
// Clear selected values and refresh upon column change
|
||||
if (control === 'col') {
|
||||
if (modifiedFilter.val.constructor === Array) {
|
||||
modifiedFilter.val = [];
|
||||
} else if (typeof modifiedFilter.val === 'string') {
|
||||
modifiedFilter.val = '';
|
||||
}
|
||||
this.fetchFilterValues(index, value);
|
||||
}
|
||||
newFilters.splice(index, 1, modifiedFilter);
|
||||
this.props.onChange(newFilters);
|
||||
}
|
||||
|
||||
removeFilter(index) {
|
||||
this.props.onChange(this.props.value.filter((f, i) => i !== index));
|
||||
this.setState((prevState) => {
|
||||
const newStateFilters = Object.assign([], prevState.filters);
|
||||
newStateFilters.splice(index, 1);
|
||||
return { filters: newStateFilters };
|
||||
});
|
||||
}
|
||||
|
||||
render() {
|
||||
const filters = this.props.value.map((filter, i) => (
|
||||
<div key={i}>
|
||||
|
|
@ -53,6 +119,8 @@ export default class FilterControl extends React.Component {
|
|||
datasource={this.props.datasource}
|
||||
removeFilter={this.removeFilter.bind(this, i)}
|
||||
changeFilter={this.changeFilter.bind(this, i)}
|
||||
valuesLoading={this.state.filters[i].valuesLoading}
|
||||
valueChoices={this.state.filters[i].valueChoices}
|
||||
/>
|
||||
</div>
|
||||
));
|
||||
|
|
|
|||
|
|
@ -8,16 +8,32 @@ import { shallow } from 'enzyme';
|
|||
import FilterControl from '../../../../javascripts/explore/components/controls/FilterControl';
|
||||
import Filter from '../../../../javascripts/explore/components/controls/Filter';
|
||||
|
||||
const $ = window.$ = require('jquery');
|
||||
|
||||
const defaultProps = {
|
||||
choices: ['country_name'],
|
||||
prefix: 'flt',
|
||||
name: 'not_having_filters',
|
||||
onChange: sinon.spy(),
|
||||
value: [],
|
||||
value: [
|
||||
{
|
||||
col: 'col1',
|
||||
op: 'in',
|
||||
val: ['a', 'b', 'd'],
|
||||
},
|
||||
{
|
||||
col: 'col2',
|
||||
op: '==',
|
||||
val: 'Z',
|
||||
},
|
||||
],
|
||||
datasource: {
|
||||
id: 1,
|
||||
type: 'table',
|
||||
filter_select: false,
|
||||
filterable_cols: ['country_name'],
|
||||
type: 'qtable',
|
||||
filter_select: true,
|
||||
filterable_cols: [['col1', 'col2']],
|
||||
metrics_combo: [
|
||||
['m1', 'v1'],
|
||||
['m2', 'v2'],
|
||||
],
|
||||
},
|
||||
};
|
||||
|
||||
|
|
@ -26,6 +42,23 @@ describe('FilterControl', () => {
|
|||
|
||||
beforeEach(() => {
|
||||
wrapper = shallow(<FilterControl {...defaultProps} />);
|
||||
wrapper.setState({
|
||||
filters: [
|
||||
{
|
||||
valuesLoading: false,
|
||||
valueChoices: ['a', 'b', 'c', 'd', 'e', 'f'],
|
||||
},
|
||||
{
|
||||
valuesLoading: false,
|
||||
valueChoices: ['X', 'Y', 'Z'],
|
||||
},
|
||||
// Need a duplicate since onChange calls are not changing props
|
||||
{
|
||||
valuesLoading: false,
|
||||
valueChoices: ['X', 'Y', 'Z'],
|
||||
},
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
it('renders Filters', () => {
|
||||
|
|
@ -34,15 +67,151 @@ describe('FilterControl', () => {
|
|||
).to.equal(true);
|
||||
});
|
||||
|
||||
it('renders one button', () => {
|
||||
expect(wrapper.find(Filter)).to.have.lengthOf(0);
|
||||
it('renders one button and two filters', () => {
|
||||
expect(wrapper.find(Filter)).to.have.lengthOf(2);
|
||||
expect(wrapper.find(Button)).to.have.lengthOf(1);
|
||||
});
|
||||
|
||||
it('add a filter when Add Filter button is clicked', () => {
|
||||
it('adds filter when clicking Add Filter', () => {
|
||||
const addButton = wrapper.find('#add-button');
|
||||
expect(addButton).to.have.lengthOf(1);
|
||||
addButton.simulate('click');
|
||||
expect(defaultProps.onChange).to.have.property('callCount', 1);
|
||||
expect(defaultProps.onChange.getCall(0).args[0]).to.deep.equal([
|
||||
{
|
||||
col: 'col1',
|
||||
op: 'in',
|
||||
val: ['a', 'b', 'd'],
|
||||
},
|
||||
{
|
||||
col: 'col2',
|
||||
op: '==',
|
||||
val: 'Z',
|
||||
},
|
||||
{
|
||||
col: 'col1',
|
||||
op: 'in',
|
||||
val: [],
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it('removes a the second filter when its delete button is clicked', () => {
|
||||
expect(wrapper.find(Filter)).to.have.lengthOf(2);
|
||||
wrapper.instance().removeFilter(1);
|
||||
expect(defaultProps.onChange).to.have.property('callCount', 2);
|
||||
expect(defaultProps.onChange.getCall(1).args[0]).to.deep.equal([
|
||||
{
|
||||
col: 'col1',
|
||||
op: 'in',
|
||||
val: ['a', 'b', 'd'],
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
after(() => {
|
||||
$.ajax.restore();
|
||||
});
|
||||
|
||||
it('makes a GET request to retrieve value choices', () => {
|
||||
sinon.stub($, 'ajax');
|
||||
wrapper.instance().fetchFilterValues(0, 'col1');
|
||||
expect($.ajax.getCall(0).args[0].type).to.deep.equal('GET');
|
||||
expect($.ajax.getCall(0).args[0].url).to.deep.equal('/superset/filter/qtable/1/col1/');
|
||||
});
|
||||
|
||||
it('changes filter values when one is removed', () => {
|
||||
wrapper.instance().changeFilter(0, 'val', ['a', 'b']);
|
||||
expect(defaultProps.onChange).to.have.property('callCount', 3);
|
||||
expect(defaultProps.onChange.getCall(2).args[0]).to.deep.equal([
|
||||
{
|
||||
col: 'col1',
|
||||
op: 'in',
|
||||
val: ['a', 'b'],
|
||||
},
|
||||
{
|
||||
col: 'col2',
|
||||
op: '==',
|
||||
val: 'Z',
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it('changes filter values when one is added', () => {
|
||||
wrapper.instance().changeFilter(0, 'val', ['a', 'b', 'd', 'e']);
|
||||
expect(defaultProps.onChange).to.have.property('callCount', 4);
|
||||
expect(defaultProps.onChange.getCall(3).args[0]).to.deep.equal([
|
||||
{
|
||||
col: 'col1',
|
||||
op: 'in',
|
||||
val: ['a', 'b', 'd', 'e'],
|
||||
},
|
||||
{
|
||||
col: 'col2',
|
||||
op: '==',
|
||||
val: 'Z',
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it('changes op and transforms values', () => {
|
||||
wrapper.instance().changeFilter(0, ['val', 'op'], ['a', '==']);
|
||||
wrapper.instance().changeFilter(1, ['val', 'op'], [['Z'], 'in']);
|
||||
expect(defaultProps.onChange).to.have.property('callCount', 6);
|
||||
expect(defaultProps.onChange.getCall(4).args[0]).to.deep.equal([
|
||||
{
|
||||
col: 'col1',
|
||||
op: '==',
|
||||
val: 'a',
|
||||
},
|
||||
{
|
||||
col: 'col2',
|
||||
op: '==',
|
||||
val: 'Z',
|
||||
},
|
||||
]);
|
||||
expect(defaultProps.onChange.getCall(5).args[0]).to.deep.equal([
|
||||
{
|
||||
col: 'col1',
|
||||
op: 'in',
|
||||
val: ['a', 'b', 'd'],
|
||||
},
|
||||
{
|
||||
col: 'col2',
|
||||
op: 'in',
|
||||
val: ['Z'],
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it('changes column and clears invalid values', () => {
|
||||
wrapper.instance().changeFilter(0, 'col', 'col2');
|
||||
expect(defaultProps.onChange).to.have.property('callCount', 7);
|
||||
expect(defaultProps.onChange.getCall(6).args[0]).to.deep.equal([
|
||||
{
|
||||
col: 'col2',
|
||||
op: 'in',
|
||||
val: [],
|
||||
},
|
||||
{
|
||||
col: 'col2',
|
||||
op: '==',
|
||||
val: 'Z',
|
||||
},
|
||||
]);
|
||||
wrapper.instance().changeFilter(1, 'col', 'col1');
|
||||
expect(defaultProps.onChange).to.have.property('callCount', 8);
|
||||
expect(defaultProps.onChange.getCall(7).args[0]).to.deep.equal([
|
||||
{
|
||||
col: 'col1',
|
||||
op: 'in',
|
||||
val: ['a', 'b', 'd'],
|
||||
},
|
||||
{
|
||||
col: 'col1',
|
||||
op: '==',
|
||||
val: '',
|
||||
},
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue