fix: dashboard filter scope bug (#13695)

* fix: dashboard filter scope bug * fix comments * fix function/variables name
apache · Mar 19, 2021 · fa072cd · fa072cd
1 parent 87d1afc
commit fa072cd
Show file tree

Hide file tree

Showing 2 changed files with 129 additions and 1 deletion.
diff --git a/superset-frontend/spec/javascripts/dashboard/util/getFilterScopeFromNodesTree_spec.js b/superset-frontend/spec/javascripts/dashboard/util/getFilterScopeFromNodesTree_spec.js
@@ -381,5 +381,96 @@ describe('getFilterScopeFromNodesTree', () => {
         immune: [105, 103, 102, 101, 108, 104],
       });
     });
+
+    it('exclude nested sub-tab', () => {
+      // another layout for test:
+      // - filter_109
+      // - chart_106
+      // - Tab 1
+      //   - Nested_Tab1
+      //     - chart_101
+      //     - chart_102
+      //   - Nested_Tab2
+      //     - chart_103
+      //     - chart_104
+      const nodes3 = [
+        {
+          label: 'All dashboard',
+          type: 'ROOT',
+          value: 'ROOT_ID',
+          children: [
+            {
+              label: 'Time Filter',
+              showCheckbox: true,
+              type: 'CHART',
+              value: 109,
+            },
+            {
+              label: "World's Pop Growth",
+              showCheckbox: true,
+              type: 'CHART',
+              value: 106,
+            },
+            {
+              label: 'Row Tab 1',
+              type: 'TAB',
+              value: 'TAB-w5Fp904Rs',
+              children: [
+                {
+                  label: 'Nested Tab 1',
+                  type: 'TAB',
+                  value: 'TAB-E4mJaZ-uQM',
+                  children: [
+                    {
+                      value: 104,
+                      label: 'Rural Breakdown',
+                      type: 'CHART',
+                      showCheckbox: true,
+                    },
+                    {
+                      value: 103,
+                      label: '% Rural',
+                      type: 'CHART',
+                      showCheckbox: true,
+                    },
+                  ],
+                },
+                {
+                  value: 'TAB-rLYu-Cryu',
+                  label: 'Nested Tab 2',
+                  type: 'TAB',
+                  children: [
+                    {
+                      value: 102,
+                      label: 'Most Populated Countries',
+                      type: 'CHART',
+                      showCheckbox: true,
+                    },
+                    {
+                      value: 101,
+                      label: "World's Population",
+                      type: 'CHART',
+                      showCheckbox: true,
+                    },
+                  ],
+                },
+              ],
+            },
+          ],
+        },
+      ];
+
+      const checkedChartIds = [103, 104, 106];
+      expect(
+        getFilterScopeFromNodesTree({
+          filterKey: '109___time_range',
+          nodes: nodes3,
+          checkedChartIds,
+        }),
+      ).toEqual({
+        scope: ['ROOT_ID'],
+        immune: [102, 101],
+      });
+    });
   });
 });
diff --git a/superset-frontend/src/dashboard/util/getFilterScopeFromNodesTree.js b/superset-frontend/src/dashboard/util/getFilterScopeFromNodesTree.js
@@ -22,11 +22,29 @@ import { flatMap, isEmpty } from 'lodash';
 import { CHART_TYPE, TAB_TYPE } from './componentTypes';
 import { getChartIdAndColumnFromFilterKey } from './getDashboardFilterKey';
 
+function getImmuneChartIdsFromTabsNotInScope({ tabs = [], tabsInScope = [] }) {
+  const chartsNotInScope = [];
+  tabs.forEach(({ value: tab, children: tabChildren }) => {
+    if (tabChildren && !tabsInScope.includes(tab)) {
+      tabChildren.forEach(({ value: subTab, children: subTabChildren }) => {
+        if (subTabChildren && !tabsInScope.includes(subTab)) {
+          chartsNotInScope.push(
+            ...subTabChildren.filter(({ type }) => type === CHART_TYPE),
+          );
+        }
+      });
+    }
+  });
+
+  // return chartId only
+  return chartsNotInScope.map(({ value }) => value);
+}
 function getTabChildrenScope({
   tabScopes,
   parentNodeValue,
   forceAggregate = false,
   hasChartSiblings = false,
+  tabChildren = [],
   immuneChartSiblings = [],
 }) {
   // if all sub-tabs are in scope, or forceAggregate =  true
@@ -38,9 +56,26 @@ function getTabChildrenScope({
         ([key, { scope }]) => scope && scope.length && key === scope[0],
       ))
   ) {
+    // get all charts from tabChildren that is not in scope
+    const immuneChartIdsFromTabsNotInScope = getImmuneChartIdsFromTabsNotInScope(
+      {
+        tabs: tabChildren,
+        tabsInScope: flatMap(tabScopes, ({ scope }) => scope),
+      },
+    );
+    const immuneChartIdsFromTabsInScope = flatMap(
+      Object.values(tabScopes),
+      ({ immune }) => immune,
+    );
+    const immuneCharts = [
+      ...new Set([
+        ...immuneChartIdsFromTabsNotInScope,
+        ...immuneChartIdsFromTabsInScope,
+      ]),
+    ];
     return {
       scope: [parentNodeValue],
-      immune: flatMap(Object.values(tabScopes), ({ immune }) => immune),
+      immune: immuneCharts,
     };
   }
 
@@ -96,6 +131,7 @@ function traverse({ currentNode = {}, filterId, checkedChartIds = [] }) {
       tabScopes,
       parentNodeValue: currentValue,
       forceAggregate: true,
+      tabChildren,
     });
     return {
       scope,
@@ -109,6 +145,7 @@ function traverse({ currentNode = {}, filterId, checkedChartIds = [] }) {
       tabScopes,
       parentNodeValue: currentValue,
       hasChartSiblings: !isEmpty(chartChildren),
+      tabChildren,
       immuneChartSiblings: chartsImmune,
     });
   }