[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #8522: [WIP][dashboard scoped filter] part 3: merge filter scope settings into dashboard redux state

[GitBox <gi...@apache.org>]

graceguo-supercat commented on a change in pull request #8522: [WIP][dashboard scoped filter] part 3: merge filter scope settings into dashboard redux state
URL: https://github.com/apache/incubator-superset/pull/8522#discussion_r344634706
 
 

 ##########
 File path: superset/assets/src/dashboard/components/Dashboard.jsx
 ##########
 @@ -122,27 +119,36 @@ class Dashboard extends React.PureComponent {
     // do not apply filter when dashboard in edit mode
     if (!editMode && safeStringify(appliedFilters) !== safeStringify(filters)) {
       // refresh charts if a filter was removed, added, or changed
-      let changedFilterKey = null;
       const currFilterKeys = Object.keys(filters);
       const appliedFilterKeys = Object.keys(appliedFilters);
 
-      currFilterKeys.forEach(key => {
-        if (
-          // filter was added or changed
-          typeof appliedFilters[key] === 'undefined' ||
-          !areObjectsEqual(appliedFilters[key], filters[key])
+      const allKeys = new Set(currFilterKeys.concat(appliedFilterKeys));
+      [...allKeys].forEach(filterKey => {
+        const affectedChartIds = [];
+        if (!currFilterKeys.includes(filterKey)) {
+          // removed filter?
+          [].push.apply(affectedChartIds, appliedFilters[filterKey].scope);
+        } else if (!appliedFilterKeys.includes(filterKey)) {
+          // added filter?
+          [].push.apply(affectedChartIds, filters[filterKey].scope);
+        } else if (
+          safeStringify(filters[filterKey].values) !==
+            safeStringify(appliedFilters[filterKey].values) ||
+          safeStringify(filters[filterKey].scope) !==
+            safeStringify(appliedFilters[filterKey].scope)
         ) {
-          changedFilterKey = key;
+          // changed filter field value?
+          const affectedScope = filters[filterKey].scope.concat(
+            appliedFilters[filterKey].scope,
+          );
+          [].push.apply(affectedChartIds, affectedScope);
         }
+
+        const idSet = new Set(affectedChartIds);
+        this.refreshCharts([...idSet]);
 
 Review comment:
   yes. convert to Set is to de-dupe. I am not sure there is no easier way to add an array of values into Set?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org