Skip to content

Commit

Permalink
minor filter select enhancements (apache#3933)
Browse files Browse the repository at this point in the history
* `values_for_column` configurable row limit

* `FilterControl` cancels active ajax request if any
  • Loading branch information
kkalyan authored and mistercrunch committed Nov 28, 2017
1 parent 045cd2e commit 3a5befb
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export default class FilterControl extends React.Component {
}));
this.state = {
filters: initialFilters,
activeRequest: null,
};
}

Expand All @@ -45,16 +46,22 @@ export default class FilterControl extends React.Component {
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 };
});
},
// if there is an outstanding request to fetch values, cancel it.
if (this.state.activeRequest) {
this.state.activeRequest.abort();
}
this.setState({
activeRequest: $.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, activeRequest: null };
});
},
}),
});
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,15 @@ describe('FilterControl', () => {
]);
});

before(() => {
sinon.stub($, 'ajax');
});

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/');
Expand Down Expand Up @@ -214,4 +217,31 @@ describe('FilterControl', () => {
},
]);
});

it('tracks an active filter select ajax request', () => {
const spyReq = sinon.spy();
$.ajax.reset();
$.ajax.onFirstCall().returns(spyReq);
wrapper.instance().fetchFilterValues(0, 'col1');
expect(wrapper.state().activeRequest).to.equal(spyReq);
// Sets active to null after success
$.ajax.getCall(0).args[0].success('choices');
expect(wrapper.state().filters[0].valuesLoading).to.equal(false);
expect(wrapper.state().filters[0].valueChoices).to.equal('choices');
expect(wrapper.state().activeRequest).to.equal(null);
});

it('cancels active request if another is submitted', () => {
const spyReq = sinon.spy();
spyReq.abort = sinon.spy();
$.ajax.reset();
$.ajax.onFirstCall().returns(spyReq);
wrapper.instance().fetchFilterValues(0, 'col1');
expect(wrapper.state().activeRequest).to.equal(spyReq);
const spyReq1 = sinon.spy();
$.ajax.onSecondCall().returns(spyReq1);
wrapper.instance().fetchFilterValues(1, 'col2');
expect(spyReq.abort.called).to.equal(true);
expect(wrapper.state().activeRequest).to.equal(spyReq1);
});
});
2 changes: 2 additions & 0 deletions superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@

ROW_LIMIT = 50000
VIZ_ROW_LIMIT = 10000
# max rows retrieved by filter select auto complete
FILTER_SELECT_ROW_LIMIT = 10000
SUPERSET_WORKERS = 2
SUPERSET_CELERY_WORKERS = 32

Expand Down
5 changes: 4 additions & 1 deletion superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -1171,7 +1171,10 @@ def filter(self, datasource_type, datasource_id, column):
return json_error_response(DATASOURCE_ACCESS_ERR)

payload = json.dumps(
datasource.values_for_column(column),
datasource.values_for_column(
column,
config.get('FILTER_SELECT_ROW_LIMIT', 10000),
),
default=utils.json_int_dttm_ser)
return json_success(payload)

Expand Down

0 comments on commit 3a5befb

Please sign in to comment.