[incubator-superset] branch master updated: [SIP-5] QueryBuilder in the client for granularity and groupby in word cloud (#6377)

[cc...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

ccwilliams pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 8b2cae0  [SIP-5] QueryBuilder in the client for granularity and groupby in word cloud (#6377)
8b2cae0 is described below

commit 8b2cae007d0b38217bd9f227c7079cca690fafa8
Author: Christine Chambers <ch...@gmail.com>
AuthorDate: Fri Nov 16 15:51:50 2018 -0800

    [SIP-5] QueryBuilder in the client for granularity and groupby in word cloud (#6377)
    
    * [SIP-5] QueryBuilder in the client for groupby field
    
    - Lay the structure of the QueryContext builder in the client
    - QueryContext builder composes different portions of the queryContext object (to be sent to server from the client) from the datasourceBuilder and the queryObjectBuilder.
    - The datasourceBuilder builds the datasource id and type by parsing them from the datasource field in formdata
    - The queryObjectBuilder currently only builds the groupby field. It will further compose the queryObject from sub query builders in future PRs.
    - Create a buildQuery method for WordCloud and override the groupby value with the value of series from formdata.
    
    * Addressing PR comments
    - Rename query builder files and their default exports
    - Move tests into spec/javascripts/superset-ui for easy mass migration to superset-uiin the future
    - Define viz specific formData and export formData for each viz type as intersection type of the generic formData and the viz specific one
    - Specify the type signature for sqla and druid based data sources
    
    * Addressing additional PR comments.
    - Introduce a Datasource class and leverage Typescript's declaration merging of the interface
    by the same name.
    - Let Typescript infer the return type of various builders instead of specifying them explicitly
    
    * Further tweaking the generic buildQueryContext method and viz speicific buildQuery methodes per PR comments.
    
    * git mv a renamed file.
    
    * addressing additional pr comments
---
 superset/assets/jest.config.js                     |  2 +-
 .../superset-ui/WordCloudBuildQuery.test.ts        | 15 +++++++++++
 .../superset-ui/buildQueryContext.test.ts          | 22 ++++++++++++++++
 .../superset-ui/buildQueryObject.test.ts           | 13 ++++++++++
 superset/assets/src/query/DatasourceKey.ts         | 29 ++++++++++++++++++++++
 superset/assets/src/query/FormData.ts              | 21 ++++++++++++++++
 superset/assets/src/query/buildQueryContext.ts     | 15 +++++++++++
 superset/assets/src/query/buildQueryObject.ts      | 18 ++++++++++++++
 superset/assets/src/query/index.ts                 |  3 +++
 .../src/visualizations/wordcloud/FormData.ts       | 11 ++++++++
 .../wordcloud/WordCloudChartPlugin.js              |  2 ++
 .../src/visualizations/wordcloud/buildQuery.ts     | 10 ++++++++
 12 files changed, 160 insertions(+), 1 deletion(-)

diff --git a/superset/assets/jest.config.js b/superset/assets/jest.config.js
index 64dcb6f..858f6f9 100644
--- a/superset/assets/jest.config.js
+++ b/superset/assets/jest.config.js
@@ -1,5 +1,5 @@
 module.exports = {
-  testRegex: '\\/spec\\/.*_spec\\.(j|t)sx?$',
+  testRegex: '\\/spec\\/.*(_spec|\\.test)\\.(j|t)sx?$',
   moduleNameMapper: {
     '\\.(css|less)$': '<rootDir>/spec/__mocks__/styleMock.js',
     '\\.(gif|ttf|eot|svg)$': '<rootDir>/spec/__mocks__/fileMock.js',
diff --git a/superset/assets/spec/javascripts/superset-ui/WordCloudBuildQuery.test.ts b/superset/assets/spec/javascripts/superset-ui/WordCloudBuildQuery.test.ts
new file mode 100644
index 0000000..a6edfbc
--- /dev/null
+++ b/superset/assets/spec/javascripts/superset-ui/WordCloudBuildQuery.test.ts
@@ -0,0 +1,15 @@
+import buildQuery from 'src/visualizations/wordcloud/buildQuery';
+
+describe('WordCloud buildQuery', () => {
+  const formData = {
+    datasource: '5__table',
+    granularity_sqla: 'ds',
+    series: 'foo',
+  };
+
+  it('should build groupby with series in form data', () => {
+    const queryContext = buildQuery(formData);
+    const [ query ] = queryContext.queries;
+    expect(query.groupby).toEqual(['foo']);
+  });
+});
diff --git a/superset/assets/spec/javascripts/superset-ui/buildQueryContext.test.ts b/superset/assets/spec/javascripts/superset-ui/buildQueryContext.test.ts
new file mode 100644
index 0000000..b6fd995
--- /dev/null
+++ b/superset/assets/spec/javascripts/superset-ui/buildQueryContext.test.ts
@@ -0,0 +1,22 @@
+import build from 'src/query/buildQueryContext';
+import * as queryObjectBuilder from 'src/query/buildQueryObject';
+
+describe('queryContextBuilder', () => {
+  it('should build datasource for table sources', () => {
+    const queryContext = build({ datasource: '5__table', granularity_sqla: 'ds'});
+    expect(queryContext.datasource.id).toBe(5);
+    expect(queryContext.datasource.type).toBe('table');
+  });
+
+  it('should build datasource for druid sources', () => {
+    const queryContext = build({ datasource: '5__druid', granularity: 'ds'});
+    expect(queryContext.datasource.id).toBe(5);
+    expect(queryContext.datasource.type).toBe('druid');
+  });
+
+  it('should call queryObjectBuilder to build queries', () => {
+    const buildQueryObjectSpy = jest.spyOn(queryObjectBuilder, 'default');
+    build({ datasource: '5__table', granularity_sqla: 'ds'});
+    expect(buildQueryObjectSpy).toHaveBeenCalledTimes(1);
+  });
+});
diff --git a/superset/assets/spec/javascripts/superset-ui/buildQueryObject.test.ts b/superset/assets/spec/javascripts/superset-ui/buildQueryObject.test.ts
new file mode 100644
index 0000000..ec111df
--- /dev/null
+++ b/superset/assets/spec/javascripts/superset-ui/buildQueryObject.test.ts
@@ -0,0 +1,13 @@
+import build from 'src/query/buildQueryObject';
+
+describe('queryObjectBuilder', () => {
+  it('should build granularity for sql alchemy datasources', () => {
+    const query = build({datasource: '5__table', granularity_sqla: 'ds'});
+    expect(query.granularity).toEqual('ds');
+  });
+
+  it('should build granularity for sql alchemy datasources', () => {
+    const query = build({datasource: '5__druid', granularity: 'ds'});
+    expect(query.granularity).toEqual('ds');
+  });
+});
diff --git a/superset/assets/src/query/DatasourceKey.ts b/superset/assets/src/query/DatasourceKey.ts
new file mode 100644
index 0000000..a13c121
--- /dev/null
+++ b/superset/assets/src/query/DatasourceKey.ts
@@ -0,0 +1,29 @@
+enum DatasourceType {
+  Table = 'table',
+  Druid = 'druid',
+}
+
+export default interface DatasourceKey {
+  id: number;
+  type: DatasourceType;
+}
+
+// Declaration merging with the interface above. No need to redeclare id and type.
+export default class DatasourceKey {
+  constructor(key: string) {
+    const [ idStr, typeStr ] = key.split('__');
+    this.id = parseInt(idStr, 10);
+    this.type = typeStr === 'table' ? DatasourceType.Table : DatasourceType.Druid;
+  }
+
+  public toString() {
+    return `${this.id}__${this.type}`;
+  }
+
+  public toObject() {
+    return {
+      id: this.id,
+      type: this.type,
+    };
+  }
+}
diff --git a/superset/assets/src/query/FormData.ts b/superset/assets/src/query/FormData.ts
new file mode 100644
index 0000000..daa7abf
--- /dev/null
+++ b/superset/assets/src/query/FormData.ts
@@ -0,0 +1,21 @@
+// Type signature and utility functions for formData shared by all viz types
+// It will be gradually filled out as we build out the query object
+interface BaseFormData {
+  datasource: string;
+}
+
+// FormData is either sqla-based or druid-based
+interface SqlaFormData extends BaseFormData {
+  granularity_sqla: string;
+}
+
+interface DruidFormData extends BaseFormData {
+  granularity: string;
+}
+
+type FormData = SqlaFormData | DruidFormData;
+export default FormData;
+
+export function getGranularity(formData: FormData): string {
+  return 'granularity_sqla' in formData ? formData.granularity_sqla : formData.granularity;
+}
diff --git a/superset/assets/src/query/buildQueryContext.ts b/superset/assets/src/query/buildQueryContext.ts
new file mode 100644
index 0000000..8ce0464
--- /dev/null
+++ b/superset/assets/src/query/buildQueryContext.ts
@@ -0,0 +1,15 @@
+import buildQueryObject, { QueryObject } from './buildQueryObject';
+import DatasourceKey from './DatasourceKey';
+import FormData from './FormData';
+
+const WRAP_IN_ARRAY = (baseQueryObject: QueryObject) => [baseQueryObject];
+
+// Note: let TypeScript infer the return type
+export default function buildQueryContext(
+  formData: FormData,
+  buildQuery: (baseQueryObject: QueryObject) => QueryObject[] = WRAP_IN_ARRAY) {
+  return {
+    datasource: new DatasourceKey(formData.datasource).toObject(),
+    queries: buildQuery(buildQueryObject(formData)),
+  };
+}
diff --git a/superset/assets/src/query/buildQueryObject.ts b/superset/assets/src/query/buildQueryObject.ts
new file mode 100644
index 0000000..578d9ae
--- /dev/null
+++ b/superset/assets/src/query/buildQueryObject.ts
@@ -0,0 +1,18 @@
+import FormData, { getGranularity } from './FormData';
+
+// TODO: fill out the rest of the query object
+export interface QueryObject {
+  granularity: string;
+  groupby?: string[];
+}
+
+// Build the common segments of all query objects (e.g. the granularity field derived from
+// either sql alchemy or druid). The segments specific to each viz type is constructed in the
+// buildQuery method for each viz type (see `wordcloud/buildQuery.ts` for an example).
+// Note the type of the formData argument passed in here is the type of the formData for a
+// specific viz, which is a subtype of the generic formData shared among all viz types.
+export default function buildQueryObject<T extends FormData>(formData: T): QueryObject {
+  return {
+    granularity: getGranularity(formData),
+  };
+}
diff --git a/superset/assets/src/query/index.ts b/superset/assets/src/query/index.ts
new file mode 100644
index 0000000..cda7126
--- /dev/null
+++ b/superset/assets/src/query/index.ts
@@ -0,0 +1,3 @@
+// Public API of the query module
+export { default } from './buildQueryContext';
+export { default as FormData } from './FormData';
diff --git a/superset/assets/src/visualizations/wordcloud/FormData.ts b/superset/assets/src/visualizations/wordcloud/FormData.ts
new file mode 100644
index 0000000..55f8131
--- /dev/null
+++ b/superset/assets/src/visualizations/wordcloud/FormData.ts
@@ -0,0 +1,11 @@
+import { FormData as GenericFormData } from 'src/query';
+
+// FormData specific to the wordcloud viz
+interface WordCloudFormData {
+  series: string;
+}
+
+// FormData for wordcloud contains both common properties of all form data
+// and properties specific to wordcloud vizzes
+type FormData = GenericFormData & WordCloudFormData;
+export default FormData;
diff --git a/superset/assets/src/visualizations/wordcloud/WordCloudChartPlugin.js b/superset/assets/src/visualizations/wordcloud/WordCloudChartPlugin.js
index b828820..19b43c5 100644
--- a/superset/assets/src/visualizations/wordcloud/WordCloudChartPlugin.js
+++ b/superset/assets/src/visualizations/wordcloud/WordCloudChartPlugin.js
@@ -1,5 +1,6 @@
 import { t } from '@superset-ui/translation';
 import { ChartMetadata, ChartPlugin } from '@superset-ui/chart';
+import buildQuery from './buildQuery';
 import transformProps from './transformProps';
 import thumbnail from './images/thumbnail.png';
 
@@ -14,6 +15,7 @@ export default class WordCloudChartPlugin extends ChartPlugin {
   constructor() {
     super({
       metadata,
+      buildQuery,
       transformProps,
       loadChart: () => import('./ReactWordCloud.js'),
     });
diff --git a/superset/assets/src/visualizations/wordcloud/buildQuery.ts b/superset/assets/src/visualizations/wordcloud/buildQuery.ts
new file mode 100644
index 0000000..2aa0f2c
--- /dev/null
+++ b/superset/assets/src/visualizations/wordcloud/buildQuery.ts
@@ -0,0 +1,10 @@
+import buildQueryContext from 'src/query';
+import FormData from './FormData';
+
+export default function buildQuery(formData: FormData) {
+  // Set the single QueryObject's groupby field with series in formData
+  return buildQueryContext(formData, (baseQueryObject) => [{
+    ...baseQueryObject,
+    groupby: [formData.series],
+  }]);
+}