Skip to content

Commit

Permalink
Fix for #6590: Numeric values in columns sometimes returned as quoted…
Browse files Browse the repository at this point in the history
… strings (#6591)

* Fix: #6590, also depends on apache-superset/superset-ui#71

* Bumped version of superset-ui-connector

* Added @babel/polyfill import

* Reset mock history before each test, not after each test

* Update CONTRIBUTING.md
  • Loading branch information
soboko authored and kristw committed Jan 14, 2019
1 parent f480a52 commit 207d952
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 15 deletions.
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ Make sure your machine meets the [OS dependencies](https://superset.incubator.ap

```bash
# Create a virtual environemnt and activate it (recommended)
virtualenv -p python3 venv . # setup a python3.6 virtualenv
virtualenv -p python3 venv # setup a python3.6 virtualenv
source venv/bin/activate

# Install external dependencies
Expand Down
9 changes: 5 additions & 4 deletions superset/assets/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion superset/assets/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
"@data-ui/xy-chart": "^0.0.61",
"@superset-ui/chart": "^0.7.0",
"@superset-ui/color": "^0.7.0",
"@superset-ui/connection": "^0.5.0",
"@superset-ui/connection": "^0.8.0",
"@superset-ui/core": "^0.7.0",
"@superset-ui/number-format": "^0.7.2",
"@superset-ui/time-format": "^0.7.2",
Expand Down
1 change: 1 addition & 0 deletions superset/assets/spec/helpers/shim.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* eslint no-native-reassign: 0 */
import '@babel/polyfill';
import 'abortcontroller-polyfill/dist/abortcontroller-polyfill-only';
import jsdom from 'jsdom';
import { configure } from 'enzyme';
Expand Down
20 changes: 18 additions & 2 deletions superset/assets/spec/javascripts/sqllab/actions/sqlLab_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import * as actions from '../../../../src/SqlLab/actions/sqlLab';
import { query } from '../fixtures';

describe('async actions', () => {
const mockBigNumber = '9223372036854775807';

let dispatch;

beforeEach(() => {
Expand Down Expand Up @@ -42,7 +44,7 @@ describe('async actions', () => {

describe('fetchQueryResults', () => {
const fetchQueryEndpoint = 'glob:*/superset/results/*';
fetchMock.get(fetchQueryEndpoint, '{ "data": "" }');
fetchMock.get(fetchQueryEndpoint, '{ "data": ' + mockBigNumber + ' }');

const makeRequest = () => {
const actionThunk = actions.fetchQueryResults(query);
Expand All @@ -65,6 +67,13 @@ describe('async actions', () => {
});
});

it('parses large number result without losing precision', () =>
makeRequest().then(() => {
expect(fetchMock.calls(fetchQueryEndpoint)).toHaveLength(1);
expect(dispatch.callCount).toBe(2);
expect(dispatch.getCall(1).lastArg.results.data.toString()).toBe(mockBigNumber);
}));

it('calls querySuccess on fetch success', () =>
makeRequest().then(() => {
expect(dispatch.callCount).toBe(2);
Expand All @@ -88,7 +97,7 @@ describe('async actions', () => {

describe('runQuery', () => {
const runQueryEndpoint = 'glob:*/superset/sql_json/*';
fetchMock.post(runQueryEndpoint, { data: '' });
fetchMock.post(runQueryEndpoint, '{ "data": ' + mockBigNumber + ' }');

const makeRequest = () => {
const request = actions.runQuery(query);
Expand All @@ -111,6 +120,13 @@ describe('async actions', () => {
});
});

it('parses large number result without losing precision', () =>
makeRequest().then(() => {
expect(fetchMock.calls(runQueryEndpoint)).toHaveLength(1);
expect(dispatch.callCount).toBe(2);
expect(dispatch.getCall(1).lastArg.results.data.toString()).toBe(mockBigNumber);
}));

it('calls querySuccess on fetch success', () => {
expect.assertions(3);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ function setup() {
}

describe('DashboardTable', () => {
afterEach(fetchMock.resetHistory);
beforeEach(fetchMock.resetHistory);

it('renders a Loading initially', () => {
const wrapper = setup();
Expand Down
7 changes: 2 additions & 5 deletions superset/assets/src/SqlLab/actions/sqlLab.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import shortid from 'shortid';
import JSONbig from 'json-bigint';
import { t } from '@superset-ui/translation';
import { SupersetClient } from '@superset-ui/connection';

Expand Down Expand Up @@ -111,11 +110,9 @@ export function fetchQueryResults(query) {

return SupersetClient.get({
endpoint: `/superset/results/${query.resultsKey}/`,
parseMethod: 'text',
})
.then(({ text = '{}' }) => {
const bigIntJson = JSONbig.parse(text);
dispatch(querySuccess(query, bigIntJson));
.then(({ json = {} }) => {
dispatch(querySuccess(query, json));
})
.catch(response =>
getClientErrorObject(response).then((error) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { List } from 'immutable';
import PropTypes from 'prop-types';
import JSONbig from 'json-bigint';
import React, { PureComponent } from 'react';
import {
Column,
Expand Down Expand Up @@ -85,7 +86,7 @@ export default class FilterableTable extends PureComponent {
if (['string', 'number'].indexOf(typeof (val)) >= 0) {
newRow[k] = val;
} else {
newRow[k] = JSON.stringify(val);
newRow[k] = JSONbig.stringify(val);
}
}
return newRow;
Expand Down

0 comments on commit 207d952

Please sign in to comment.