fix(table-viz): format non-numeric metrics (apache#10234)

auxten · Nov 20, 2020 · 4d5dc7e · 4d5dc7e
1 parent 4099e68
commit 4d5dc7e
Show file tree

Hide file tree

Showing 4 changed files with 90 additions and 11 deletions.
diff --git a/superset-frontend/cypress-base/cypress/integration/explore/visualizations/shared.helper.js b/superset-frontend/cypress-base/cypress/integration/explore/visualizations/shared.helper.js
@@ -63,6 +63,52 @@ export const NUM_METRIC = {
   optionName: 'metric_1de0s4viy5d_ly7y8k6ghvk',
 };
 
+export const MAX_DS = {
+  aggregate: 'MAX',
+  column: {
+    column_name: 'ds',
+    description: null,
+    expression: null,
+    filterable: true,
+    groupby: true,
+    id: 333,
+    is_dttm: true,
+    optionName: '_col_ds',
+    python_date_format: null,
+    type: 'TIMESTAMP WITHOUT TIME ZONE',
+    verbose_name: null,
+  },
+  expressionType: 'SIMPLE',
+  hasCustomLabel: false,
+  isNew: false,
+  label: 'MAX(ds)',
+  optionName: 'metric_pbib7j9m15a_js80vs9vca',
+  sqlExpression: null,
+};
+
+export const MAX_STATE = {
+  expressionType: 'SIMPLE',
+  column: {
+    id: 337,
+    column_name: 'state',
+    verbose_name: null,
+    description: null,
+    expression: null,
+    filterable: true,
+    groupby: true,
+    is_dttm: false,
+    type: 'VARCHAR(10)',
+    python_date_format: null,
+    optionName: '_col_state',
+  },
+  aggregate: 'MAX',
+  sqlExpression: null,
+  isNew: false,
+  hasCustomLabel: false,
+  label: 'MAX(state)',
+  optionName: 'metric_kvval50pvbo_hewj3pzacb',
+};
+
 export const SIMPLE_FILTER = {
   expressionType: 'SIMPLE',
   subject: 'name',

diff --git a/superset-frontend/cypress-base/cypress/integration/explore/visualizations/table.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/visualizations/table.test.ts
@@ -16,7 +16,13 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { FORM_DATA_DEFAULTS, NUM_METRIC, SIMPLE_FILTER } from './shared.helper';
+import {
+  FORM_DATA_DEFAULTS,
+  NUM_METRIC,
+  MAX_DS,
+  MAX_STATE,
+  SIMPLE_FILTER,
+} from './shared.helper';
 import readResponseBlob from '../../../utils/readResponseBlob';
 
 // Table
@@ -29,13 +35,40 @@ describe('Visualization > Table', () => {
     cy.route('POST', '/superset/explore_json/**').as('getJson');
   });
 
-  it('Test table with adhoc metric', () => {
-    cy.visitChartByParams({ ...VIZ_DEFAULTS, metrics: NUM_METRIC });
-    cy.verifySliceSuccess({
-      waitAlias: '@getJson',
-      querySubstring: NUM_METRIC.label,
-      chartSelector: 'table',
+  it('Format non-numeric metrics correctly', () => {
+    cy.visitChartByParams({
+      ...VIZ_DEFAULTS,
+      include_time: true,
+      granularity_sqla: 'ds',
+      time_grain_sqla: 'P0.25Y',
+      metrics: [NUM_METRIC, MAX_DS, MAX_STATE],
+    });
+    // when format with smart_date, time column use format by granularity
+    cy.get('.chart-container td:nth-child(1)').contains('2008 Q1');
+    // other column with timestamp use raw timestamp
+    cy.get('.chart-container td:nth-child(3)').contains('2008-01-01T00:00:00');
+    cy.get('.chart-container td').contains('other');
+  });
+
+  it('Format with table_timestamp_format', () => {
+    cy.visitChartByParams({
+      ...VIZ_DEFAULTS,
+      include_time: true,
+      granularity_sqla: 'ds',
+      time_grain_sqla: 'P0.25Y',
+      table_timestamp_format: '%Y-%m-%d %H:%M',
+      metrics: [NUM_METRIC, MAX_DS, MAX_STATE],
     });
+    // time column and MAX(ds) metric column both use UTC time
+    cy.get('.chart-container td:nth-child(1)').contains('2008-01-01 00:00');
+    cy.get('.chart-container td:nth-child(3)').contains('2008-01-01 00:00');
+    cy.get('.chart-container td')
+      .contains('2008-01-01 08:00')
+      .should('not.exist');
+    // time column should not use time granularity when timestamp format is set
+    cy.get('.chart-container td').contains('2008 Q1').should('not.exist');
+    // other num numeric metric column should stay as string
+    cy.get('.chart-container td').contains('other');
   });
 
   it('Test table with groupby', () => {

diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json
diff --git a/superset-frontend/package.json b/superset-frontend/package.json
@@ -85,7 +85,7 @@
     "@superset-ui/legacy-plugin-chart-sankey": "^0.14.1",
     "@superset-ui/legacy-plugin-chart-sankey-loop": "^0.14.1",
     "@superset-ui/legacy-plugin-chart-sunburst": "^0.14.1",
-    "@superset-ui/plugin-chart-table": "^0.14.6",
+    "@superset-ui/plugin-chart-table": "^0.14.8",
     "@superset-ui/legacy-plugin-chart-treemap": "^0.14.1",
     "@superset-ui/legacy-plugin-chart-world-map": "^0.14.1",
     "@superset-ui/legacy-preset-chart-big-number": "^0.14.1",