superset-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From maximebeauche...@apache.org
Subject [incubator-superset] branch master updated: [Bugfix] Issues with table filtering (#4073)
Date Sun, 17 Dec 2017 23:43:37 GMT
This is an automated email from the ASF dual-hosted git repository.

maximebeauchemin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 1e79e9c  [Bugfix] Issues with table filtering (#4073)
1e79e9c is described below

commit 1e79e9cd2a064c8a6daa77bb74e549e0b12cf840
Author: Jeff Niu <jeffniu22@gmail.com>
AuthorDate: Sun Dec 17 15:43:34 2017 -0800

    [Bugfix] Issues with table filtering (#4073)
    
    * Fixing some issues with table filtering
    
    * Added some comments
---
 superset/assets/javascripts/chart/Chart.jsx       |  4 +-
 superset/assets/javascripts/dashboard/actions.js  |  4 +-
 superset/assets/javascripts/dashboard/reducers.js | 46 +++++++++++------------
 superset/assets/visualizations/table.js           |  7 ++++
 4 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/superset/assets/javascripts/chart/Chart.jsx b/superset/assets/javascripts/chart/Chart.jsx
index 3dd0355..958d353 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 25fa117..6da6b43 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 84215db..0ee7964 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)) {
+          // 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
           // 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,18 @@ 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;
+      // 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 };
       }
-      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 755ac1c..6af73ca 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,12 @@ function tableVis(slice, payload) {
     .attr('data-sort', function (d) {
       return (d.isMetric) ? d.val : null;
     })
+    // 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);

-- 
To stop receiving notification emails like this one, please contact
['"commits@superset.apache.org" <commits@superset.apache.org>'].

Mime
View raw message