chore(core): remove queryData and replace with queriesData (#874)

* feat: remove single queryData from api * feat: migrate old plugins to queriesData api property * test: fix tests * test: fix tests * fix: fix storybook * lint: fix lint * refactor: fix CR notes * refactor: remove queryData from Chart Provider
apache-superset · Dec 29, 2020 · 4bda6f5 · 4bda6f5 · vercel · Dec 29, 2020
1 parent 7e5af33
commit 4bda6f5
Show file tree

Hide file tree

Showing 158 changed files with 20,877 additions and 20,891 deletions.
diff --git a/packages/generator-superset/generators/plugin-chart/templates/README.erb b/packages/generator-superset/generators/plugin-chart/templates/README.erb
@@ -24,9 +24,9 @@ Then use it via `SuperChart`. See [storybook](https://apache-superset.github.io/
   width={600}
   height={600}
   formData={...}
-  queryData={{
+  queriesData={[{
     data: {...},
-  }}
+  }]}
 />
 ```
 

diff --git a/packages/generator-superset/generators/plugin-chart/templates/src/plugin/transformProps.erb b/packages/generator-superset/generators/plugin-chart/templates/src/plugin/transformProps.erb
@@ -35,8 +35,8 @@ export default function transformProps(chartProps: ChartProps) {
    *   the chart is located
    * - `formData`: the chart data request payload that was sent to the
    *   backend.
-   * - `queryData`: the chart data response payload that was received
-   *   from the backend. Some notable properties of `queryData`:
+   * - `queriesData`: the chart data response payload that was received
+   *   from the backend. Some notable properties of `queriesData`:
    *   - `data`: an array with data, each row with an object mapping
    *     the column/alias to its value. Example:
    *     `[{ col1: 'abc', metric1: 10 }, { col1: 'xyz', metric1: 20 }]`
@@ -48,9 +48,9 @@ export default function transformProps(chartProps: ChartProps) {
    * function during development with hot reloading, changes won't
    * be seen until restarting the development server.
    */
-  const { width, height, formData, queryData } = chartProps;
+  const { width, height, formData, queriesData } = chartProps;
   const { boldText, headerFontSize, headerText } = formData;
-  const data = queryData.data as TimeseriesDataRecord[];
+  const data = queriesData[0].data as TimeseriesDataRecord[];
 
   console.log('formData via TransformProps.ts', formData);
 

diff --git a/.../generator-superset/generators/plugin-chart/templates/test/plugin/transformProps.test.erb b/.../generator-superset/generators/plugin-chart/templates/test/plugin/transformProps.test.erb
@@ -16,9 +16,9 @@ describe('<%= packageLabel %> tranformProps', () => {
     formData,
     width: 800,
     height: 600,
-    queryData: {
+    queriesData: [{
       data: [{ name: 'Hulk', sum__num: 1<%if (chartType === 'timeseries') { %>, __timestamp: 599616000000<% } %> }],
-    },
+    }],
   });
 
   it('should tranform chart props for viz', () => {

diff --git a/packages/superset-ui-core/src/chart/clients/ChartClient.ts b/packages/superset-ui-core/src/chart/clients/ChartClient.ts
@@ -29,7 +29,7 @@ export interface ChartData {
   annotationData: AnnotationData;
   datasource: PlainObject;
   formData: QueryFormData;
-  queryData: QueryData;
+  queriesData: QueryData[];
 }
 
 export default class ChartClient {
@@ -76,7 +76,7 @@ export default class ChartClient {
   async loadQueryData(
     formData: QueryFormData,
     options?: Partial<RequestConfig>,
-  ): Promise<QueryData> {
+  ): Promise<QueryData[]> {
     const { viz_type: visType } = formData;
     const metaDataRegistry = getChartMetadataRegistry();
     const buildQueryRegistry = getChartBuildQueryRegistry();
@@ -101,8 +101,7 @@ export default class ChartClient {
           };
 
       return this.client.post(requestConfig).then(response => {
-        // let's assume response.json always has the shape of QueryData
-        return response.json as QueryData;
+        return Array.isArray(response.json) ? response.json : [response.json];
       });
     }
 
@@ -156,11 +155,11 @@ export default class ChartClient {
           this.loadAnnotations(formData.annotation_layers),
           this.loadDatasource(formData.datasource),
           this.loadQueryData(formData),
-        ]).then(([annotationData, datasource, queryData]) => ({
+        ]).then(([annotationData, datasource, queriesData]) => ({
           annotationData,
           datasource,
           formData,
-          queryData,
+          queriesData,
         })),
     );
   }

diff --git a/packages/superset-ui-core/src/chart/components/ChartDataProvider.tsx b/packages/superset-ui-core/src/chart/components/ChartDataProvider.tsx
@@ -6,7 +6,7 @@ import { QueryData } from '../types/QueryResponse';
 
 interface Payload {
   formData: Partial<QueryFormData>;
-  queryData: QueryData;
+  queriesData: QueryData[];
   datasource?: Datasource;
 }
 
@@ -33,7 +33,7 @@ export type ChartDataProviderProps =
     formDataRequestOptions?: Partial<RequestConfig>;
     /** Hook to override the datasource request config. */
     datasourceRequestOptions?: Partial<RequestConfig>;
-    /** Hook to override the queryData request config. */
+    /** Hook to override the queriesData request config. */
     queryRequestOptions?: Partial<RequestConfig>;
   };
 
@@ -90,12 +90,12 @@ class ChartDataProvider extends React.PureComponent<
                 : Promise.resolve(undefined),
               this.chartClient.loadQueryData(formData, queryRequestOptions),
             ]).then(
-              ([datasource, queryData]) =>
+              ([datasource, queriesData]) =>
                 // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
                 ({
                   datasource,
                   formData,
-                  queryData,
+                  queriesData,
                 } as Payload),
             ),
           )

diff --git a/packages/superset-ui-core/src/chart/components/SuperChart.tsx b/packages/superset-ui-core/src/chart/components/SuperChart.tsx
@@ -111,29 +111,23 @@ export default class SuperChart extends React.PureComponent<Props, {}> {
       FallbackComponent,
       onErrorBoundary,
       Wrapper,
-      queryData,
       queriesData,
       ...rest
     } = this.props as PropsWithDefault;
 
     const chartProps = this.createChartProps({
       ...rest,
-      queryData,
       queriesData,
       height,
       width,
     });
 
     let chart;
     // Render the no results component if the query data is null or empty
-    const noResultQuery =
-      !queryData ||
-      !queryData.data ||
-      (Array.isArray(queryData.data) && queryData.data.length === 0);
     const noResultQueries =
       !queriesData ||
       queriesData.every(({ data }) => !data || (Array.isArray(data) && data.length === 0));
-    if (noResultQuery && noResultQueries) {
+    if (noResultQueries) {
       chart = <NoResultsComponent id={id} className={className} height={height} width={width} />;
     } else {
       const chartWithoutWrapper = (

diff --git a/packages/superset-ui-core/src/chart/models/ChartProps.ts b/packages/superset-ui-core/src/chart/models/ChartProps.ts
@@ -47,10 +47,7 @@ export interface ChartPropsConfig {
   height?: number;
   /** Programmatic overrides such as event handlers, renderers */
   hooks?: Hooks;
-  /** Formerly called "payload". This property going to be deprecated because
-   * contains only first item in response data array (use `queriesData` instead) */
-  queryData?: QueryData;
-  /** Formerly called "payload" */
+  /** The data returned for all queries objects in the request */
   queriesData?: QueryData[];
   /** Chart width */
   width?: number;
@@ -78,8 +75,6 @@ export default class ChartProps {
 
   hooks: Hooks;
 
-  queryData: QueryData;
-
   queriesData: QueryData[];
 
   width: number;
@@ -91,7 +86,6 @@ export default class ChartProps {
       formData = {},
       hooks = {},
       initialValues = {},
-      queryData = {},
       queriesData = [],
       width = DEFAULT_WIDTH,
       height = DEFAULT_HEIGHT,
@@ -105,7 +99,6 @@ export default class ChartProps {
     this.rawFormData = formData;
     this.hooks = hooks;
     this.initialValues = initialValues;
-    this.queryData = queryData;
     this.queriesData = queriesData;
   }
 }
@@ -119,28 +112,16 @@ ChartProps.createSelector = function create(): ChartPropsSelector {
     input => input.height,
     input => input.hooks,
     input => input.initialValues,
-    input => input.queryData,
     input => input.queriesData,
     input => input.width,
-    (
-      annotationData,
-      datasource,
-      formData,
-      height,
-      hooks,
-      initialValues,
-      queryData,
-      queriesData,
-      width,
-    ) =>
+    (annotationData, datasource, formData, height, hooks, initialValues, queriesData, width) =>
       new ChartProps({
         annotationData,
         datasource,
         formData,
         height,
         hooks,
         initialValues,
-        queryData,
         queriesData,
         width,
       }),

diff --git a/packages/superset-ui-core/test/chart/clients/ChartClient.test.ts b/packages/superset-ui-core/test/chart/clients/ChartClient.test.ts
@@ -100,21 +100,25 @@ describe('ChartClient', () => {
       getChartBuildQueryRegistry().registerValue('word_cloud', (formData: QueryFormData) =>
         buildQueryContext(formData),
       );
-      fetchMock.post('glob:*/api/v1/chart/data', {
-        field1: 'abc',
-        field2: 'def',
-      });
+      fetchMock.post('glob:*/api/v1/chart/data', [
+        {
+          field1: 'abc',
+          field2: 'def',
+        },
+      ]);
 
       return expect(
         chartClient.loadQueryData({
           granularity: 'minute',
           viz_type: 'word_cloud',
           datasource: '1__table',
         }),
-      ).resolves.toEqual({
-        field1: 'abc',
-        field2: 'def',
-      });
+      ).resolves.toEqual([
+        {
+          field1: 'abc',
+          field2: 'def',
+        },
+      ]);
     });
     it('returns a promise that rejects for unknown chart type', () =>
       expect(
@@ -151,10 +155,12 @@ describe('ChartClient', () => {
           viz_type: 'word_cloud_legacy',
           datasource: '1__table',
         }),
-      ).resolves.toEqual({
-        field1: 'abc',
-        field2: 'def',
-      });
+      ).resolves.toEqual([
+        {
+          field1: 'abc',
+          field2: 'def',
+        },
+      ]);
     });
   });
 
@@ -259,11 +265,13 @@ describe('ChartClient', () => {
           datasource: '1__table',
           color: 'living-coral',
         },
-        queryData: {
-          lorem: 'ipsum',
-          dolor: 'sit',
-          amet: true,
-        },
+        queriesData: [
+          {
+            lorem: 'ipsum',
+            dolor: 'sit',
+            amet: true,
+          },
+        ],
       });
     });
   });

diff --git a/packages/superset-ui-core/test/chart/components/ChartDataProvider.test.tsx b/packages/superset-ui-core/test/chart/components/ChartDataProvider.test.tsx
@@ -17,8 +17,12 @@ function createPromise<T>(input: T) {
   return Promise.resolve(input);
 }
 
+function createArrayPromise<T>(input: T) {
+  return Promise.resolve([input]);
+}
+
 const mockLoadDatasource = jest.fn<Promise<unknown>, unknown[]>(createPromise);
-const mockLoadQueryData = jest.fn<Promise<unknown>, unknown[]>(createPromise);
+const mockLoadQueryData = jest.fn<Promise<unknown>, unknown[]>(createArrayPromise);
 
 // ChartClient is now a mock
 jest.mock('@superset-ui/core/src/chart/clients/ChartClient', () =>
@@ -202,7 +206,7 @@ describe('ChartDataProvider', () => {
             payload: {
               formData: props.formData,
               datasource: props.formData.datasource,
-              queryData: props.formData,
+              queriesData: [props.formData],
             },
           });
           done();
@@ -258,7 +262,7 @@ describe('ChartDataProvider', () => {
           expect(onLoaded.mock.calls[0][0]).toEqual({
             formData: props.formData,
             datasource: props.formData.datasource,
-            queryData: props.formData,
+            queriesData: [props.formData],
           });
           done();
         }, 0);