Skip to content

Commit

Permalink
Fixed filter removal bug (#3458)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@Mogball @mistercrunch
Mogball authored and mistercrunch committed Sep 15, 2017
1 parent ad604ae commit dd72048
Show file tree
Hide file tree
Showing 3 changed files with 277 additions and 44 deletions.
66 changes: 31 additions & 35 deletions superset/assets/javascripts/explore/components/controls/Filter.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand All @@ -29,68 +27,66 @@ const propTypes = {
filter: PropTypes.object.isRequired,
datasource: PropTypes.object,
having: PropTypes.bool,
valuesLoading: PropTypes.bool,
valueChoices: PropTypes.array,
};

const defaultProps = {
changeFilter: () => {},
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) {
Expand All @@ -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}
/>
Expand Down
68 changes: 68 additions & 0 deletions superset/assets/javascripts/explore/components/controls/FilterControl.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 ?
Expand All @@ -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]);
Expand All @@ -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}>
Expand All @@ -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>
));
Expand Down
Loading

0 comments on commit dd72048

Please sign in to comment.