Skip to content

Commit

Permalink
fix(plugin-chart-pivot-table): Invalid Formats Date Fields
Browse files Browse the repository at this point in the history
When a custom date field value converted in a string format, some aggregators truncated to the first four digits.
 This is caused by the parseFloat function which converts to first matched number rather than NaN(Not-A-Number) value.
 This commit replaces the parseFloat by Number wrapper to handle this case correctly.
  • Loading branch information
justinpark committed Jul 28, 2022
1 parent d50784d commit c48afc0
Show file tree
Hide file tree
Showing 3 changed files with 206 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import React from 'react';
import { withKnobs } from '@storybook/addon-knobs';
import { SuperChart } from '@superset-ui/core';
import { PivotTableChartPlugin } from '@superset-ui/plugin-chart-pivot-table';
import { basicFormData, basicData } from './testData';
import { withResizableChartDemo } from '../../../shared/components/ResizableChartDemo';

export default {
title: 'Chart Plugins/plugin-chart-pivot-table',
decorators: [withKnobs, withResizableChartDemo],
};

new PivotTableChartPlugin().configure({ key: 'pivot_table_v2' }).register();

export const basic = ({ width, height }) => (
<SuperChart
chartType="pivot_table_v2"
datasource={{
columnFormats: {},
}}
width={width}
height={height}
queriesData={[basicData]}
formData={basicFormData}
/>
);
basic.story = {
parameters: {
initialSize: {
width: 680,
height: 420,
},
},
};

export const MaximumAggregation = ({ width, height }) => (
<SuperChart
chartType="pivot_table_v2"
datasource={{
columnFormats: {},
}}
width={width}
height={height}
queriesData={[basicData]}
formData={{ ...basicFormData, aggregateFunction: 'Maximum' }}
/>
);
basic.story = {
parameters: {
initialSize: {
width: 680,
height: 420,
},
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
export const basicFormData = {
datasource: '1__table',
viz_type: 'pivot_table_v2',
granularity_sqla: 'ts',
groupbyColumns: ['location'],
groupbyRows: ['program_language'],
metrics: [
{
expressionType: 'SIMPLE',
column: {
id: 1,
column_name: 'count',
description: null,
expression: null,
groupby: true,
is_dttm: false,
python_date_format: null,
type: 'BIGINT',
type_generic: 0,
},
aggregate: 'SUM',
sqlExpression: null,
isNew: false,
hasCustomLabel: true,
label: 'Count',
},
{
expressionType: 'SIMPLE',
column: {
id: 2,
column_name: 'ts',
description: null,
expression: "DATE_PARSE(ds || ' ' || hr, '%Y-%m-%d %H')",
groupby: true,
is_dttm: true,
type: 'TIMESTAMP',
type_generic: 2,
python_date_format: null,
},
aggregate: 'MAX',
sqlExpression: null,
isNew: false,
hasCustomLabel: true,
label: 'Most Recent Data',
},
],
metricsLayout: 'COLUMNS',
order_desc: true,
aggregateFunction: 'Sum',
valueFormat: '~g',
date_format: 'smart_date',
rowOrder: 'key_a_to_z',
colOrder: 'key_a_to_z',
};

export const basicData = {
cache_key: 'f2cd2a37b6977e3619ce6c07d0027972',
cached_dttm: '2022-07-27T17:42:39',
cache_timeout: 129600,
applied_template_filters: [],
annotation_data: {},
error: null,
is_cached: true,
query: 'SELECT \nFROM\nWHERE',
status: 'success',
stacktrace: null,
rowcount: 5,
from_dttm: 1658426268000,
to_dttm: 1659031068000,
colnames: ['location', 'program_language', 'Count', 'Most Recent Data'],
indexnames: [0, 1, 2, 3, 4],
coltypes: [1, 1, 0, 1],
data: [
{
location: 'AMEA',
program_language: 'Javscript',
Count: 134,
'Most Recent Data': '2022-07-25 13:00:00.000',
},
{
location: 'ASIA',
program_language: 'python',
Count: 19,
'Most Recent Data': '2022-07-25 16:00:00.000',
},
{
location: 'ASIA',
program_language: 'Java',
Count: 7,
'Most Recent Data': '2022-07-25 15:00:00.000',
},
{
location: 'ASIA',
program_language: 'C++',
Count: 1,
'Most Recent Data': '2022-07-25 02:00:00.000',
},
{
location: 'ASIA',
program_language: 'PHP',
Count: 1,
'Most Recent Data': '2022-07-24 00:00:00.000',
},
],
result_format: 'json',
applied_filters: [],
rejected_filters: [],
};
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ const baseAggregatorTemplates = {
return {
sum: 0,
push(record) {
if (Number.isNaN(parseFloat(record[attr]))) {
if (Number.isNaN(Number(record[attr]))) {
this.sum = record[attr];
} else {
this.sum += parseFloat(record[attr]);
Expand Down Expand Up @@ -259,7 +259,7 @@ const baseAggregatorTemplates = {
push(record) {
const x = record[attr];
if (['min', 'max'].includes(mode)) {
const coercedValue = parseFloat(x);
const coercedValue = Number(x);
if (Number.isNaN(coercedValue)) {
this.val =
!this.val ||
Expand Down Expand Up @@ -308,7 +308,7 @@ const baseAggregatorTemplates = {
strMap: {},
push(record) {
const val = record[attr];
const x = parseFloat(val);
const x = Number(val);

if (Number.isNaN(x)) {
this.strMap[val] = (this.strMap[val] || 0) + 1;
Expand Down Expand Up @@ -354,7 +354,7 @@ const baseAggregatorTemplates = {
s: 0.0,
strValue: null,
push(record) {
const x = parseFloat(record[attr]);
const x = Number(record[attr]);
if (Number.isNaN(x)) {
this.strValue =
typeof record[attr] === 'string' ? record[attr] : this.strValue;
Expand Down Expand Up @@ -405,10 +405,10 @@ const baseAggregatorTemplates = {
sumNum: 0,
sumDenom: 0,
push(record) {
if (!Number.isNaN(parseFloat(record[num]))) {
if (!Number.isNaN(Number(record[num]))) {
this.sumNum += parseFloat(record[num]);
}
if (!Number.isNaN(parseFloat(record[denom]))) {
if (!Number.isNaN(Number(record[denom]))) {
this.sumDenom += parseFloat(record[denom]);
}
},
Expand Down

0 comments on commit c48afc0

Please sign in to comment.