From 0f6a2431769d5504a8299c928482d6064f68c575 Mon Sep 17 00:00:00 2001 From: Jeff Niu Date: Sun, 17 Dec 2017 14:03:09 -0800 Subject: [PATCH 1/2] Fixing some issues with table filtering --- superset/assets/javascripts/chart/Chart.jsx | 4 +- .../assets/javascripts/dashboard/actions.js | 4 +- .../assets/javascripts/dashboard/reducers.js | 42 ++++++++----------- superset/assets/visualizations/table.js | 2 + 4 files changed, 24 insertions(+), 28 deletions(-) diff --git a/superset/assets/javascripts/chart/Chart.jsx b/superset/assets/javascripts/chart/Chart.jsx index 3dd0355ab08fc..958d353f95540 100644 --- a/superset/assets/javascripts/chart/Chart.jsx +++ b/superset/assets/javascripts/chart/Chart.jsx @@ -106,8 +106,8 @@ class Chart extends React.PureComponent { this.props.clearFilter(); } - removeFilter(col, vals) { - this.props.removeFilter(col, vals); + removeFilter(col, vals, refresh = true) { + this.props.removeFilter(col, vals, refresh); } clearError() { diff --git a/superset/assets/javascripts/dashboard/actions.js b/superset/assets/javascripts/dashboard/actions.js index 25fa117c47dfc..6da6b43ac773a 100644 --- a/superset/assets/javascripts/dashboard/actions.js +++ b/superset/assets/javascripts/dashboard/actions.js @@ -13,8 +13,8 @@ export function clearFilter(sliceId) { } export const REMOVE_FILTER = 'REMOVE_FILTER'; -export function removeFilter(sliceId, col, vals) { - return { type: REMOVE_FILTER, sliceId, col, vals }; +export function removeFilter(sliceId, col, vals, refresh = true) { + return { type: REMOVE_FILTER, sliceId, col, vals, refresh }; } export const UPDATE_DASHBOARD_LAYOUT = 'UPDATE_DASHBOARD_LAYOUT'; diff --git a/superset/assets/javascripts/dashboard/reducers.js b/superset/assets/javascripts/dashboard/reducers.js index 84215db26b26d..5fbd49d7c035f 100644 --- a/superset/assets/javascripts/dashboard/reducers.js +++ b/superset/assets/javascripts/dashboard/reducers.js @@ -138,27 +138,26 @@ export const dashboard = function (state = {}, action) { return state; } - let filters; + let filters = state.filters; const { sliceId, col, vals, merge, refresh } = action; const filterKeys = ['__from', '__to', '__time_col', '__time_grain', '__time_origin', '__granularity']; if (filterKeys.indexOf(col) >= 0 || selectedSlice.formData.groupby.indexOf(col) !== -1) { - if (!(sliceId in state.filters)) { - filters = { ...state.filters, [sliceId]: {} }; - } let newFilter = {}; - if (state.filters[sliceId] && !(col in state.filters[sliceId]) || !merge) { - newFilter = { ...state.filters[sliceId], [col]: vals }; + if (!(sliceId in filters)) { + newFilter = { [col]: vals } + } else if (filters[sliceId] && !(col in filters[sliceId]) || !merge) { + newFilter = { ...filters[sliceId], [col]: vals }; // d3.merge pass in array of arrays while some value form filter components // from and to filter box require string to be process and return - } else if (state.filters[sliceId][col] instanceof Array) { - newFilter[col] = d3.merge([state.filters[sliceId][col], vals]); + } else if (filters[sliceId][col] instanceof Array) { + newFilter[col] = d3.merge([filters[sliceId][col], vals]); } else { - newFilter[col] = d3.merge([[state.filters[sliceId][col]], vals])[0] || ''; + newFilter[col] = d3.merge([[filters[sliceId][col]], vals])[0] || ''; } - filters = { ...state.filters, [sliceId]: newFilter }; + filters = { ...filters, [sliceId]: newFilter }; } return { ...state, filters, refresh }; }, @@ -168,21 +167,16 @@ export const dashboard = function (state = {}, action) { return { ...state, filter: newFilters, refresh: true }; }, [actions.REMOVE_FILTER]() { - const newFilters = { ...state.filters }; - const { sliceId, col, vals } = action; - - if (sliceId in state.filters) { - if (col in state.filters[sliceId]) { - const a = []; - newFilters[sliceId][col].forEach(function (v) { - if (vals.indexOf(v) < 0) { - a.push(v); - } - }); - newFilters[sliceId][col] = a; - } + const { sliceId, col, vals, refresh } = action; + const excluded = new Set(vals); + const valFilter = (val) => !excluded.has(val); + + let filters = state.filters; + if (sliceId in state.filters && col in state.filters[sliceId]) { + const newFilter = filters[sliceId][col].filter(valFilter); + filters = { ...filters, [sliceId]: newFilter }; } - return { ...state, filter: newFilters, refresh: true }; + return { ...state, filters, refresh }; }, // slice reducer diff --git a/superset/assets/visualizations/table.js b/superset/assets/visualizations/table.js index 755ac1c7e6690..c4f10fdb51658 100644 --- a/superset/assets/visualizations/table.js +++ b/superset/assets/visualizations/table.js @@ -66,6 +66,7 @@ function tableVis(slice, payload) { return d; }); + const filters = slice.getFilters(); table.append('tbody') .selectAll('tr') .data(data.records) @@ -119,6 +120,7 @@ function tableVis(slice, payload) { .attr('data-sort', function (d) { return (d.isMetric) ? d.val : null; }) + .classed('filtered', d => filters && filters[d.col] && filters[d.col].indexOf(d.val) >= 0) .on('click', function (d) { if (!d.isMetric && fd.table_filter) { const td = d3.select(this); From 9fff98431ac0ca4241de8ad4e6fae572fca8312e Mon Sep 17 00:00:00 2001 From: Jeff Niu Date: Sun, 17 Dec 2017 14:08:00 -0800 Subject: [PATCH 2/2] Added some comments --- superset/assets/javascripts/dashboard/reducers.js | 8 +++++--- superset/assets/visualizations/table.js | 7 ++++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/superset/assets/javascripts/dashboard/reducers.js b/superset/assets/javascripts/dashboard/reducers.js index 5fbd49d7c035f..0ee7964ab1218 100644 --- a/superset/assets/javascripts/dashboard/reducers.js +++ b/superset/assets/javascripts/dashboard/reducers.js @@ -144,10 +144,10 @@ export const dashboard = function (state = {}, action) { '__time_grain', '__time_origin', '__granularity']; if (filterKeys.indexOf(col) >= 0 || selectedSlice.formData.groupby.indexOf(col) !== -1) { - let newFilter = {}; if (!(sliceId in filters)) { - newFilter = { [col]: vals } + // Straight up set the filters if none existed for the slice + newFilter = { [col]: vals }; } else if (filters[sliceId] && !(col in filters[sliceId]) || !merge) { newFilter = { ...filters[sliceId], [col]: vals }; // d3.merge pass in array of arrays while some value form filter components @@ -169,9 +169,11 @@ export const dashboard = function (state = {}, action) { [actions.REMOVE_FILTER]() { const { sliceId, col, vals, refresh } = action; const excluded = new Set(vals); - const valFilter = (val) => !excluded.has(val); + const valFilter = val => !excluded.has(val); let filters = state.filters; + // Have to be careful not to modify the dashboard state so that + // the render actually triggers if (sliceId in state.filters && col in state.filters[sliceId]) { const newFilter = filters[sliceId][col].filter(valFilter); filters = { ...filters, [sliceId]: newFilter }; diff --git a/superset/assets/visualizations/table.js b/superset/assets/visualizations/table.js index c4f10fdb51658..6af73ca46505c 100644 --- a/superset/assets/visualizations/table.js +++ b/superset/assets/visualizations/table.js @@ -120,7 +120,12 @@ function tableVis(slice, payload) { .attr('data-sort', function (d) { return (d.isMetric) ? d.val : null; }) - .classed('filtered', d => filters && filters[d.col] && filters[d.col].indexOf(d.val) >= 0) + // Check if the dashboard currently has a filter for each row + .classed('filtered', d => + filters && + filters[d.col] && + filters[d.col].indexOf(d.val) >= 0, + ) .on('click', function (d) { if (!d.isMetric && fd.table_filter) { const td = d3.select(this);