fix(explore): handle null control sections (#20142)

* fix(explore): handle null control sections * fix type and add null to test fixture
apache · May 20, 2022 · e766f8c · e766f8c
1 parent 0bcc21b
commit e766f8c
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 54 deletions.
diff --git a/superset-frontend/src/explore/controlUtils/getControlConfig.ts b/superset-frontend/src/explore/controlUtils/getControlConfig.ts
@@ -19,19 +19,22 @@
 import memoizeOne from 'memoize-one';
 import { getChartControlPanelRegistry } from '@superset-ui/core';
 import {
+  ControlPanelConfig,
   ControlPanelSectionConfig,
   expandControlConfig,
+  isControlPanelSectionConfig,
 } from '@superset-ui/chart-controls';
 
 /**
  * Find control item from control panel config.
  */
 export function findControlItem(
-  controlPanelSections: ControlPanelSectionConfig[],
+  controlPanelSections: (ControlPanelSectionConfig | null)[],
   controlKey: string,
 ) {
   return (
     controlPanelSections
+      .filter(isControlPanelSectionConfig)
       .map(section => section.controlSetRows)
       .flat(2)
       .find(
@@ -46,7 +49,7 @@ export function findControlItem(
 }
 
 const getMemoizedControlConfig = memoizeOne(
-  (controlKey, controlPanelConfig) => {
+  (controlKey, controlPanelConfig: ControlPanelConfig) => {
     const { controlOverrides = {}, controlPanelSections = [] } =
       controlPanelConfig;
     const control = expandControlConfig(
@@ -62,5 +65,10 @@ export const getControlConfig = function getControlConfig(
   vizType: string,
 ) {
   const controlPanelConfig = getChartControlPanelRegistry().get(vizType) || {};
-  return getMemoizedControlConfig(controlKey, controlPanelConfig);
+  return getMemoizedControlConfig(
+    controlKey,
+    // TODO: the ChartControlPanelRegistry is incorrectly typed and needs to
+    // be fixed
+    controlPanelConfig as ControlPanelConfig,
+  );
 };
diff --git a/superset-frontend/src/explore/fixtures.tsx b/superset-frontend/src/explore/fixtures.tsx
@@ -26,47 +26,49 @@ import {
   ControlPanelSectionConfig,
 } from '@superset-ui/chart-controls';
 
-export const controlPanelSectionsChartOptions: ControlPanelSectionConfig[] = [
-  {
-    label: t('Chart Options'),
-    expanded: true,
-    controlSetRows: [
-      [
-        'color_scheme',
-        {
-          name: 'rose_area_proportion',
-          config: {
-            type: 'CheckboxControl',
-            label: t('Use Area Proportions'),
-            description: t(
-              'Check if the Rose Chart should use segment area instead of ' +
-                'segment radius for proportioning',
-            ),
-            default: false,
-            renderTrigger: true,
+export const controlPanelSectionsChartOptions: (ControlPanelSectionConfig | null)[] =
+  [
+    null,
+    {
+      label: t('Chart Options'),
+      expanded: true,
+      controlSetRows: [
+        [
+          'color_scheme',
+          {
+            name: 'rose_area_proportion',
+            config: {
+              type: 'CheckboxControl',
+              label: t('Use Area Proportions'),
+              description: t(
+                'Check if the Rose Chart should use segment area instead of ' +
+                  'segment radius for proportioning',
+              ),
+              default: false,
+              renderTrigger: true,
+            },
           },
-        },
-      ],
-      [
-        {
-          name: 'stacked_style',
-          config: {
-            type: 'SelectControl',
-            label: t('Stacked Style'),
-            renderTrigger: true,
-            choices: [
-              ['stack', 'stack'],
-              ['stream', 'stream'],
-              ['expand', 'expand'],
-            ],
-            default: 'stack',
-            description: '',
+        ],
+        [
+          {
+            name: 'stacked_style',
+            config: {
+              type: 'SelectControl',
+              label: t('Stacked Style'),
+              renderTrigger: true,
+              choices: [
+                ['stack', 'stack'],
+                ['stream', 'stream'],
+                ['expand', 'expand'],
+              ],
+              default: 'stack',
+              description: '',
+            },
           },
-        },
+        ],
       ],
-    ],
-  },
-];
+    },
+  ];
 
 export const controlPanelSectionsChartOptionsOnlyColorScheme: ControlPanelSectionConfig[] =
   [

diff --git a/superset-frontend/src/utils/getControlsForVizType.js b/superset-frontend/src/utils/getControlsForVizType.js
@@ -18,26 +18,29 @@
  */
 
 import memoize from 'lodash/memoize';
+import { isControlPanelSectionConfig } from '@superset-ui/chart-controls';
 import { getChartControlPanelRegistry } from '@superset-ui/core';
 import { controls } from '../explore/controls';
 
 const memoizedControls = memoize((vizType, controlPanel) => {
   const controlsMap = {};
-  (controlPanel?.controlPanelSections || []).forEach(section => {
-    section.controlSetRows.forEach(row => {
-      row.forEach(control => {
-        if (!control) return;
-        if (typeof control === 'string') {
-          // For now, we have to look in controls.jsx to get the config for some controls.
-          // Once everything is migrated out, delete this if statement.
-          controlsMap[control] = controls[control];
-        } else if (control.name && control.config) {
-          // condition needed because there are elements, e.g. <hr /> in some control configs (I'm looking at you, FilterBox!)
-          controlsMap[control.name] = control.config;
-        }
+  (controlPanel?.controlPanelSections || [])
+    .filter(isControlPanelSectionConfig)
+    .forEach(section => {
+      section.controlSetRows.forEach(row => {
+        row.forEach(control => {
+          if (!control) return;
+          if (typeof control === 'string') {
+            // For now, we have to look in controls.jsx to get the config for some controls.
+            // Once everything is migrated out, delete this if statement.
+            controlsMap[control] = controls[control];
+          } else if (control.name && control.config) {
+            // condition needed because there are elements, e.g. <hr /> in some control configs (I'm looking at you, FilterBox!)
+            controlsMap[control.name] = control.config;
+          }
+        });
       });
     });
-  });
   return controlsMap;
 });