refactor(chart): remove and rename fields in ChartProps (#174)

* refactor(chart): remove and remove fields in ChartProps BREAKING CHANGE: ChartProps fields are removed and renamed. * test: update tests * fix: unit tests * fix: type
apache · Nov 26, 2021 · 3df26ae · 3df26ae
1 parent cb9e617
commit 3df26ae
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 83 deletions.
diff --git a/...end/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/models/ChartProps.ts b/...end/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/models/ChartProps.ts
@@ -9,59 +9,74 @@ type SnakeCaseDatasource = PlainObject;
 type CamelCaseFormData = PlainObject;
 type SnakeCaseFormData = PlainObject;
 export type QueryData = PlainObject;
-type Filters = PlainObject;
+/** Initial values for the visualizations, currently used by only filter_box and table */
+type InitialValues = PlainObject;
 type ChartPropsSelector = (c: ChartPropsConfig) => ChartProps;
 
+/** Optional field for event handlers, renderers */
+type Hooks = {
+  /** handle adding filters  */
+  onAddFilter?: HandlerFunction;
+  /** handle errors  */
+  onError?: HandlerFunction;
+  /** use the vis as control to update state */
+  setControlValue?: HandlerFunction;
+  /** handle tooltip */
+  setTooltip?: HandlerFunction;
+  [key: string]: any;
+};
+
+/**
+ * Preferred format for ChartProps config
+ */
 export interface ChartPropsConfig {
   annotationData?: AnnotationData;
+  /** Datasource metadata */
   datasource?: SnakeCaseDatasource;
-  filters?: Filters;
+  /**
+   * Formerly called "filters", which was misleading because it is actually
+   * initial values of the filter_box and table vis
+   */
+  initialValues?: InitialValues;
+  /** Main configuration of the chart */
   formData?: SnakeCaseFormData;
+  /** Chart height */
   height?: number;
-  hooks?: PlainObject;
-  onAddFilter?: HandlerFunction;
-  onError?: HandlerFunction;
-  payload?: QueryData;
-  setControlValue?: HandlerFunction;
-  setTooltip?: HandlerFunction;
+  /** Programmatic overrides such as event handlers, renderers */
+  hooks?: Hooks;
+  /** Formerly called "payload" */
+  queryData?: QueryData;
+  /** Chart width */
   width?: number;
 }
 
-function NOOP() {}
-
 const DEFAULT_WIDTH = 800;
 const DEFAULT_HEIGHT = 600;
 
-export default class ChartProps {
+export default class ChartProps<
+  FormDataType extends CamelCaseFormData | SnakeCaseFormData = CamelCaseFormData
+> {
   static createSelector: () => ChartPropsSelector;
 
   annotationData: AnnotationData;
   datasource: CamelCaseDatasource;
   rawDatasource: SnakeCaseDatasource;
-  filters: Filters;
+  initialValues: InitialValues;
   formData: CamelCaseFormData;
-  rawFormData: SnakeCaseFormData;
+  rawFormData: SnakeCaseFormData | CamelCaseFormData;
   height: number;
-  hooks: PlainObject;
-  onAddFilter: HandlerFunction;
-  onError: HandlerFunction;
-  payload: QueryData;
-  setControlValue: HandlerFunction;
-  setTooltip: HandlerFunction;
+  hooks: Hooks;
+  queryData: QueryData;
   width: number;
 
   constructor(config: ChartPropsConfig = {}) {
     const {
       annotationData = {},
       datasource = {},
-      filters = {},
-      formData = {},
+      formData = {} as FormDataType,
       hooks = {},
-      onAddFilter = NOOP,
-      onError = NOOP,
-      payload = {},
-      setControlValue = NOOP,
-      setTooltip = NOOP,
+      initialValues = [],
+      queryData = [],
       width = DEFAULT_WIDTH,
       height = DEFAULT_HEIGHT,
     } = config;
@@ -70,58 +85,33 @@ export default class ChartProps {
     this.annotationData = annotationData;
     this.datasource = convertKeysToCamelCase(datasource);
     this.rawDatasource = datasource;
-    this.filters = filters;
     this.formData = convertKeysToCamelCase(formData);
-    this.hooks = hooks;
     this.rawFormData = formData;
-    this.onAddFilter = onAddFilter;
-    this.onError = onError;
-    this.payload = payload;
-    this.setControlValue = setControlValue;
-    this.setTooltip = setTooltip;
+    this.hooks = hooks;
+    this.initialValues = initialValues;
+    this.queryData = queryData;
   }
 }
 
 ChartProps.createSelector = function create(): ChartPropsSelector {
   return createSelector(
     (input: ChartPropsConfig) => input.annotationData,
     input => input.datasource,
-    input => input.filters,
     input => input.formData,
     input => input.height,
     input => input.hooks,
-    input => input.onAddFilter,
-    input => input.onError,
-    input => input.payload,
-    input => input.setControlValue,
-    input => input.setTooltip,
+    input => input.initialValues,
+    input => input.queryData,
     input => input.width,
-    (
-      annotationData,
-      datasource,
-      filters,
-      formData,
-      height,
-      hooks,
-      onAddFilter,
-      onError,
-      payload,
-      setControlValue,
-      setTooltip,
-      width,
-    ) =>
+    (annotationData, datasource, formData, height, hooks, initialValues, queryData, width) =>
       new ChartProps({
         annotationData,
         datasource,
-        filters,
         formData,
         height,
         hooks,
-        onAddFilter,
-        onError,
-        payload,
-        setControlValue,
-        setTooltip,
+        initialValues,
+        queryData,
         width,
       }),
   );

diff --git a/...uperset_ui/superset-ui/packages/superset-ui-chart/test/components/SuperChartCore.test.tsx b/...uperset_ui/superset-ui/packages/superset-ui-chart/test/components/SuperChartCore.test.tsx
@@ -91,13 +91,13 @@ describe('SuperChartCore', () => {
     });
     it('uses preTransformProps when specified', () => {
       const chartPropsWithPayload = new ChartProps({
-        payload: { message: 'hulk' },
+        queryData: { message: 'hulk' },
       });
       const wrapper = shallow(
         <SuperChartCore
           chartType={ChartKeys.DILIGENT}
           preTransformProps={() => chartPropsWithPayload}
-          overrideTransformProps={props => props.payload}
+          overrideTransformProps={props => props.queryData}
         />,
       );
 

diff --git a/...mporary_superset_ui/superset-ui/packages/superset-ui-chart/test/models/ChartProps.test.ts b/...mporary_superset_ui/superset-ui/packages/superset-ui-chart/test/models/ChartProps.test.ts
@@ -8,7 +8,7 @@ const RAW_DATASOURCE = {
   another_field: 2,
 };
 
-const PAYLOAD = {};
+const QUERY_DATA = {};
 
 describe('ChartProps', () => {
   it('exists', () => {
@@ -20,31 +20,17 @@ describe('ChartProps', () => {
         width: 800,
         height: 600,
         formData: RAW_FORM_DATA,
-        payload: PAYLOAD,
+        queryData: QUERY_DATA,
       });
       expect(props).toBeInstanceOf(ChartProps);
     });
-    it('sets all handlers to NOOP by default', () => {
-      const props = new ChartProps({
-        width: 800,
-        height: 600,
-        formData: RAW_FORM_DATA,
-        payload: PAYLOAD,
-      });
-      expect(() => {
-        props.onAddFilter();
-        props.onError();
-        props.setControlValue();
-        props.setTooltip();
-      }).not.toThrow();
-    });
     it('processes formData and datasource to convert field names to camelCase', () => {
       const props = new ChartProps({
         width: 800,
         height: 600,
         datasource: RAW_DATASOURCE,
         formData: RAW_FORM_DATA,
-        payload: PAYLOAD,
+        queryData: QUERY_DATA,
       });
       expect(props.formData.someField).toEqual(1);
       expect(props.datasource.anotherField).toEqual(2);
@@ -63,14 +49,14 @@ describe('ChartProps', () => {
         height: 600,
         datasource: RAW_DATASOURCE,
         formData: RAW_FORM_DATA,
-        payload: PAYLOAD,
+        queryData: QUERY_DATA,
       });
       const props2 = selector({
         width: 800,
         height: 600,
         datasource: RAW_DATASOURCE,
         formData: RAW_FORM_DATA,
-        payload: PAYLOAD,
+        queryData: QUERY_DATA,
       });
       expect(props1).toBe(props2);
     });
@@ -80,21 +66,21 @@ describe('ChartProps', () => {
         height: 600,
         datasource: RAW_DATASOURCE,
         formData: RAW_FORM_DATA,
-        payload: PAYLOAD,
+        queryData: QUERY_DATA,
       });
       const props2 = selector({
         width: 800,
         height: 600,
         datasource: RAW_DATASOURCE,
         formData: { new_field: 3 },
-        payload: PAYLOAD,
+        queryData: QUERY_DATA,
       });
       const props3 = selector({
         width: 800,
         height: 600,
         datasource: RAW_DATASOURCE,
         formData: RAW_FORM_DATA,
-        payload: PAYLOAD,
+        queryData: QUERY_DATA,
       });
       expect(props1).not.toBe(props2);
       expect(props1).not.toBe(props3);