You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by gr...@apache.org on 2018/06/21 23:42:35 UTC
[incubator-superset] branch dashboard-builder updated: [fix] layout
converter fix (#5218)
This is an automated email from the ASF dual-hosted git repository.
graceguo pushed a commit to branch dashboard-builder
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/dashboard-builder by this push:
new ec1c879 [fix] layout converter fix (#5218)
ec1c879 is described below
commit ec1c87927626dbf90fa5ca8482b03241dfd98f63
Author: Grace Guo <gr...@airbnb.com>
AuthorDate: Thu Jun 21 16:42:32 2018 -0700
[fix] layout converter fix (#5218)
* [fix] layout converter fix
* add changed_on into initial sliceEntity data
* add unit tests for SliceAdder component
* remove old fixtures file
---
.../dashboard/components/Dashboard_spec.jsx | 2 +-
.../dashboard/components/SliceAdder_spec.jsx | 154 +++++++++++++++++++
.../components/gridComponents/Chart_spec.jsx | 5 +-
.../assets/spec/javascripts/dashboard/fixtures.jsx | 171 ---------------------
.../dashboard/fixtures/mockSliceEntities.js | 140 ++++++++++++++++-
.../assets/src/dashboard/containers/SliceAdder.js | 28 ----
.../src/dashboard/reducers/getInitialState.js | 1 +
.../src/dashboard/util/dashboardLayoutConverter.js | 6 +-
superset/assets/src/dashboard/util/propShapes.jsx | 4 +-
superset/models/core.py | 1 +
tests/core_tests.py | 8 +
11 files changed, 313 insertions(+), 207 deletions(-)
diff --git a/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx b/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx
index 2094040..e855009 100644
--- a/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx
+++ b/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx
@@ -13,7 +13,7 @@ import datasources from '../fixtures/mockDatasource';
import dashboardInfo from '../fixtures/mockDashboardInfo';
import { dashboardLayout } from '../fixtures/mockDashboardLayout';
import dashboardState from '../fixtures/mockDashboardState';
-import sliceEntities from '../fixtures/mockSliceEntities';
+import { sliceEntitiesForChart as sliceEntities } from '../fixtures/mockSliceEntities';
import { CHART_TYPE } from '../../../../src/dashboard/util/componentTypes';
import newComponentFactory from '../../../../src/dashboard/util/newComponentFactory';
diff --git a/superset/assets/spec/javascripts/dashboard/components/SliceAdder_spec.jsx b/superset/assets/spec/javascripts/dashboard/components/SliceAdder_spec.jsx
new file mode 100644
index 0000000..da0f7df
--- /dev/null
+++ b/superset/assets/spec/javascripts/dashboard/components/SliceAdder_spec.jsx
@@ -0,0 +1,154 @@
+import React from 'react';
+import { shallow } from 'enzyme';
+import { describe, it, beforeEach, afterEach } from 'mocha';
+import sinon from 'sinon';
+import { expect } from 'chai';
+
+import { List } from 'react-virtualized';
+
+import SliceAdder from '../../../../src/dashboard/components/SliceAdder';
+import { sliceEntitiesForDashboard as mockSliceEntities } from '../fixtures/mockSliceEntities';
+
+describe('SliceAdder', () => {
+ const mockEvent = {
+ key: 'Enter',
+ target: {
+ value: 'mock event target',
+ },
+ preventDefault: () => {},
+ };
+ const props = {
+ ...mockSliceEntities,
+ fetchAllSlices: () => {},
+ selectedSliceIds: [127, 128],
+ userId: '1',
+ height: 100,
+ };
+ const errorProps = {
+ ...props,
+ errorMessage: 'this is error',
+ };
+
+ describe('SliceAdder.sortByComparator', () => {
+ it('should sort by timestamp descending', () => {
+ const sortedTimestamps = Object.values(props.slices)
+ .sort(SliceAdder.sortByComparator('changed_on'))
+ .map(slice => slice.changed_on);
+ expect(
+ sortedTimestamps.every((currentTimestamp, index) => {
+ if (index === 0) {
+ return true;
+ }
+ return currentTimestamp < sortedTimestamps[index - 1];
+ }),
+ ).to.equal(true);
+ });
+
+ it('should sort by slice_name', () => {
+ const sortedNames = Object.values(props.slices)
+ .sort(SliceAdder.sortByComparator('slice_name'))
+ .map(slice => slice.slice_name);
+ const expectedNames = Object.values(props.slices)
+ .map(slice => slice.slice_name)
+ .sort();
+ expect(sortedNames).to.deep.equal(expectedNames);
+ });
+ });
+
+ it('render List', () => {
+ const wrapper = shallow(<SliceAdder {...props} />);
+ wrapper.setState({ filteredSlices: Object.values(props.slices) });
+ expect(wrapper.find(List)).to.have.length(1);
+ });
+
+ it('render error', () => {
+ const wrapper = shallow(<SliceAdder {...errorProps} />);
+ wrapper.setState({ filteredSlices: Object.values(props.slices) });
+ expect(wrapper.text()).to.have.string(errorProps.errorMessage);
+ });
+
+ it('componentDidMount', () => {
+ sinon.spy(SliceAdder.prototype, 'componentDidMount');
+ sinon.spy(props, 'fetchAllSlices');
+
+ shallow(<SliceAdder {...props} />, {
+ lifecycleExperimental: true,
+ });
+ expect(SliceAdder.prototype.componentDidMount.calledOnce).to.equal(true);
+ expect(props.fetchAllSlices.calledOnce).to.equal(true);
+
+ SliceAdder.prototype.componentDidMount.restore();
+ props.fetchAllSlices.restore();
+ });
+
+ describe('componentWillReceiveProps', () => {
+ let wrapper;
+ beforeEach(() => {
+ wrapper = shallow(<SliceAdder {...props} />);
+ wrapper.setState({ filteredSlices: Object.values(props.slices) });
+ sinon.spy(wrapper.instance(), 'setState');
+ });
+ afterEach(() => {
+ wrapper.instance().setState.restore();
+ });
+
+ it('fetch slices should update state', () => {
+ wrapper.instance().componentWillReceiveProps({
+ ...props,
+ lastUpdated: new Date().getTime(),
+ });
+ expect(wrapper.instance().setState.calledOnce).to.equal(true);
+
+ const stateKeys = Object.keys(
+ wrapper.instance().setState.lastCall.args[0],
+ );
+ expect(stateKeys).to.include('filteredSlices');
+ });
+
+ it('select slices should update state', () => {
+ wrapper.instance().componentWillReceiveProps({
+ ...props,
+ selectedSliceIds: [127],
+ });
+ expect(wrapper.instance().setState.calledOnce).to.equal(true);
+
+ const stateKeys = Object.keys(
+ wrapper.instance().setState.lastCall.args[0],
+ );
+ expect(stateKeys).to.include('selectedSliceIdsSet');
+ });
+ });
+
+ describe('should rerun filter and sort', () => {
+ let wrapper;
+ let spy;
+ beforeEach(() => {
+ wrapper = shallow(<SliceAdder {...props} />);
+ wrapper.setState({ filteredSlices: Object.values(props.slices) });
+ spy = sinon.spy(wrapper.instance(), 'getFilteredSortedSlices');
+ });
+ afterEach(() => {
+ spy.restore();
+ });
+
+ it('searchUpdated', () => {
+ const newSearchTerm = 'new search term';
+ wrapper.instance().searchUpdated(newSearchTerm);
+ expect(spy.calledOnce).to.equal(true);
+ expect(spy.lastCall.args[0]).to.equal(newSearchTerm);
+ });
+
+ it('handleSelect', () => {
+ const newSortBy = 1;
+ wrapper.instance().handleSelect(newSortBy);
+ expect(spy.calledOnce).to.equal(true);
+ expect(spy.lastCall.args[1]).to.equal(newSortBy);
+ });
+
+ it('handleKeyPress', () => {
+ wrapper.instance().handleKeyPress(mockEvent);
+ expect(spy.calledOnce).to.equal(true);
+ expect(spy.lastCall.args[0]).to.equal(mockEvent.target.value);
+ });
+ });
+});
diff --git a/superset/assets/spec/javascripts/dashboard/components/gridComponents/Chart_spec.jsx b/superset/assets/spec/javascripts/dashboard/components/gridComponents/Chart_spec.jsx
index 5fff313..dcd7119 100644
--- a/superset/assets/spec/javascripts/dashboard/components/gridComponents/Chart_spec.jsx
+++ b/superset/assets/spec/javascripts/dashboard/components/gridComponents/Chart_spec.jsx
@@ -9,7 +9,10 @@ import SliceHeader from '../../../../../src/dashboard/components/SliceHeader';
import ChartContainer from '../../../../../src/chart/ChartContainer';
import mockDatasource from '../../fixtures/mockDatasource';
-import sliceEntities, { sliceId } from '../../fixtures/mockSliceEntities';
+import {
+ sliceEntitiesForChart as sliceEntities,
+ sliceId,
+} from '../../fixtures/mockSliceEntities';
import chartQueries, {
sliceId as queryId,
} from '../../fixtures/mockChartQueries';
diff --git a/superset/assets/spec/javascripts/dashboard/fixtures.jsx b/superset/assets/spec/javascripts/dashboard/fixtures.jsx
deleted file mode 100644
index 7a12454..0000000
--- a/superset/assets/spec/javascripts/dashboard/fixtures.jsx
+++ /dev/null
@@ -1,171 +0,0 @@
-import getInitialState from '../../../src/dashboard/reducers/getInitialState';
-
-export const defaultFilters = {
- 256: { region: [] },
- 257: { country_name: ['United States'] },
-};
-export const regionFilter = {
- datasource: null,
- description: null,
- description_markeddown: '',
- edit_url: '/slicemodelview/edit/256',
- form_data: {
- datasource: '2__table',
- date_filter: false,
- filters: [
- {
- col: 'country_name',
- op: 'in',
- val: ['United States', 'France', 'Japan'],
- },
- ],
- granularity_sqla: null,
- groupby: ['region', 'country_name'],
- having: '',
- instant_filtering: true,
- metric: 'sum__SP_POP_TOTL',
- show_druid_time_granularity: false,
- show_druid_time_origin: false,
- show_sqla_time_column: false,
- show_sqla_time_granularity: false,
- since: '100 years ago',
- slice_id: 256,
- time_grain_sqla: null,
- until: 'now',
- viz_type: 'filter_box',
- where: '',
- },
- slice_id: 256,
- slice_name: 'Region Filters',
- slice_url:
- '/superset/explore/table/2/?form_data=%7B%22slice_id%22%3A%20256%7D',
-};
-export const countryFilter = {
- datasource: null,
- description: null,
- description_markeddown: '',
- edit_url: '/slicemodelview/edit/257',
- form_data: {
- datasource: '2__table',
- date_filter: false,
- filters: [],
- granularity_sqla: null,
- groupby: ['country_name'],
- having: '',
- instant_filtering: true,
- metric: 'sum__SP_POP_TOTL',
- show_druid_time_granularity: false,
- show_druid_time_origin: false,
- show_sqla_time_column: false,
- show_sqla_time_granularity: false,
- since: '100 years ago',
- slice_id: 257,
- time_grain_sqla: null,
- until: 'now',
- viz_type: 'filter_box',
- where: '',
- },
- slice_id: 257,
- slice_name: 'Country Filters',
- slice_url:
- '/superset/explore/table/2/?form_data=%7B%22slice_id%22%3A%20257%7D',
-};
-export const slice = {
- datasource: null,
- description: null,
- description_markeddown: '',
- edit_url: '/slicemodelview/edit/248',
- form_data: {
- annotation_layers: [],
- bottom_margin: 'auto',
- color_scheme: 'bnbColors',
- contribution: false,
- datasource: '2__table',
- filters: [],
- granularity_sqla: null,
- groupby: [],
- having: '',
- left_margin: 'auto',
- limit: 50,
- line_interpolation: 'linear',
- metrics: ['sum__SP_POP_TOTL'],
- num_period_compare: '',
- order_desc: true,
- period_ratio_type: 'growth',
- resample_fillmethod: null,
- resample_how: null,
- resample_rule: null,
- rich_tooltip: true,
- rolling_type: 'None',
- show_brush: false,
- show_legend: true,
- show_markers: false,
- since: '1961-01-01T00:00:00',
- slice_id: 248,
- time_compare: null,
- time_grain_sqla: null,
- timeseries_limit_metric: null,
- until: '2014-12-31T00:00:00',
- viz_type: 'line',
- where: '',
- x_axis_format: 'smart_date',
- x_axis_label: '',
- x_axis_showminmax: true,
- y_axis_bounds: [null, null],
- y_axis_format: '.3s',
- y_axis_label: '',
- y_axis_showminmax: true,
- y_log_scale: false,
- },
- slice_id: 248,
- slice_name: 'Filtered Population',
- slice_url:
- '/superset/explore/table/2/?form_data=%7B%22slice_id%22%3A%20248%7D',
-};
-
-const mockDashboardData = {
- css: '',
- dash_edit_perm: true,
- dash_save_perm: true,
- dashboard_title: 'Births',
- id: 2,
- metadata: {
- default_filters: JSON.stringify(defaultFilters),
- filter_immune_slices: [256],
- timed_refresh_immune_slices: [],
- filter_immune_slice_fields: {},
- expanded_slices: {},
- },
- position_json: [
- {
- size_x: 4,
- slice_id: '256',
- row: 0,
- size_y: 4,
- col: 5,
- },
- {
- size_x: 4,
- slice_id: '248',
- row: 0,
- size_y: 4,
- col: 1,
- },
- ],
- slug: 'births',
- slices: [regionFilter, slice, countryFilter],
- standalone_mode: false,
-};
-export const {
- dashboardState,
- dashboardInfo,
- charts,
- datasources,
- sliceEntities,
- dashboardLayout,
-} = getInitialState({
- common: {},
- dashboard_data: mockDashboardData,
- datasources: {},
- user_id: '1',
-});
diff --git a/superset/assets/spec/javascripts/dashboard/fixtures/mockSliceEntities.js b/superset/assets/spec/javascripts/dashboard/fixtures/mockSliceEntities.js
index 7c43bea..3644c10 100644
--- a/superset/assets/spec/javascripts/dashboard/fixtures/mockSliceEntities.js
+++ b/superset/assets/spec/javascripts/dashboard/fixtures/mockSliceEntities.js
@@ -3,7 +3,7 @@ import { datasourceId } from './mockDatasource';
export const sliceId = id;
-export default {
+export const sliceEntitiesForChart = {
slices: {
[sliceId]: {
slice_id: sliceId,
@@ -37,3 +37,141 @@ export default {
errorMessage: null,
lastUpdated: 0,
};
+
+export const sliceEntitiesForDashboard = {
+ slices: {
+ 127: {
+ slice_id: 127,
+ slice_url: '/superset/explore/?form_data=%7B%22slice_id%22%3A%20127%7D',
+ slice_name: 'Region Filter',
+ form_data: {},
+ edit_url: '/slicemodelview/edit/127',
+ viz_type: 'filter_box',
+ datasource: '2__table',
+ description: null,
+ description_markeddown: '',
+ modified: '23 hours ago',
+ changed_on: 1529453332615,
+ },
+ 128: {
+ slice_id: 128,
+ slice_url: '/superset/explore/?form_data=%7B%22slice_id%22%3A%20128%7D',
+ slice_name: "World's Population",
+ form_data: {},
+ edit_url: '/slicemodelview/edit/128',
+ viz_type: 'big_number',
+ datasource: '2__table',
+ description: null,
+ description_markeddown: '',
+ modified: '23 hours ago',
+ changed_on: 1529453332628,
+ },
+ 129: {
+ slice_id: 129,
+ slice_url: '/superset/explore/?form_data=%7B%22slice_id%22%3A%20129%7D',
+ slice_name: 'Most Populated Countries',
+ form_data: {},
+ edit_url: '/slicemodelview/edit/129',
+ viz_type: 'table',
+ datasource: '2__table',
+ description: null,
+ description_markeddown: '',
+ modified: '23 hours ago',
+ changed_on: 1529453332637,
+ },
+ 130: {
+ slice_id: 130,
+ slice_url: '/superset/explore/?form_data=%7B%22slice_id%22%3A%20130%7D',
+ slice_name: 'Growth Rate',
+ form_data: {},
+ edit_url: '/slicemodelview/edit/130',
+ viz_type: 'line',
+ datasource: '2__table',
+ description: null,
+ description_markeddown: '',
+ modified: '23 hours ago',
+ changed_on: 1529453332645,
+ },
+ 131: {
+ slice_id: 131,
+ slice_url: '/superset/explore/?form_data=%7B%22slice_id%22%3A%20131%7D',
+ slice_name: '% Rural',
+ form_data: {},
+ edit_url: '/slicemodelview/edit/131',
+ viz_type: 'world_map',
+ datasource: '2__table',
+ description: null,
+ description_markeddown: '',
+ modified: '23 hours ago',
+ changed_on: 1529453332654,
+ },
+ 132: {
+ slice_id: 132,
+ slice_url: '/superset/explore/?form_data=%7B%22slice_id%22%3A%20132%7D',
+ slice_name: 'Life Expectancy VS Rural %',
+ form_data: {},
+ edit_url: '/slicemodelview/edit/132',
+ viz_type: 'bubble',
+ datasource: '2__table',
+ description: null,
+ description_markeddown: '',
+ modified: '23 hours ago',
+ changed_on: 1529453332663,
+ },
+ 133: {
+ slice_id: 133,
+ slice_url: '/superset/explore/?form_data=%7B%22slice_id%22%3A%20133%7D',
+ slice_name: 'Rural Breakdown',
+ form_data: {},
+ edit_url: '/slicemodelview/edit/133',
+ viz_type: 'sunburst',
+ datasource: '2__table',
+ description: null,
+ description_markeddown: '',
+ modified: '23 hours ago',
+ changed_on: 1529453332673,
+ },
+ 134: {
+ slice_id: 134,
+ slice_url: '/superset/explore/?form_data=%7B%22slice_id%22%3A%20134%7D',
+ slice_name: "World's Pop Growth",
+ form_data: {},
+ edit_url: '/slicemodelview/edit/134',
+ viz_type: 'area',
+ datasource: '2__table',
+ description: null,
+ description_markeddown: '',
+ modified: '23 hours ago',
+ changed_on: 1529453332680,
+ },
+ 135: {
+ slice_id: 135,
+ slice_url: '/superset/explore/?form_data=%7B%22slice_id%22%3A%20135%7D',
+ slice_name: 'Box plot',
+ form_data: {},
+ edit_url: '/slicemodelview/edit/135',
+ viz_type: 'box_plot',
+ datasource: '2__table',
+ description: null,
+ description_markeddown: '',
+ modified: '23 hours ago',
+ changed_on: 1529453332688,
+ },
+ 136: {
+ slice_id: 136,
+ slice_url: '/superset/explore/?form_data=%7B%22slice_id%22%3A%20136%7D',
+ slice_name: 'Treemap',
+ form_data: {},
+ edit_url: '/slicemodelview/edit/136',
+ viz_type: 'treemap',
+ datasource: '2__table',
+ description: null,
+ description_markeddown: '',
+ modified: '23 hours ago',
+ changed_on: 1529453332700,
+ },
+ },
+ isLoading: false,
+ errorMessage: null,
+ lastUpdated: 0,
+};
diff --git a/superset/assets/src/dashboard/containers/SliceAdder.js b/superset/assets/src/dashboard/containers/SliceAdder.js
deleted file mode 100644
index e3d931d..0000000
--- a/superset/assets/src/dashboard/containers/SliceAdder.js
+++ /dev/null
@@ -1,28 +0,0 @@
-import { bindActionCreators } from 'redux';
-import { connect } from 'react-redux';
-
-import { fetchAllSlices } from '../actions/sliceEntities';
-import SliceAdder from '../components/SliceAdder';
-
-function mapStateToProps({ sliceEntities, dashboardInfo, dashboardState }) {
- return {
- userId: dashboardInfo.userId,
- selectedSliceIds: dashboardState.sliceIds,
- slices: sliceEntities.slices,
- isLoading: sliceEntities.isLoading,
- errorMessage: sliceEntities.errorMessage,
- lastUpdated: sliceEntities.lastUpdated,
- editMode: dashboardState.editMode,
- };
-}
-
-function mapDispatchToProps(dispatch) {
- return bindActionCreators(
- {
- fetchAllSlices,
- },
- dispatch,
- );
-}
-
-export default connect(mapStateToProps, mapDispatchToProps)(SliceAdder);
diff --git a/superset/assets/src/dashboard/reducers/getInitialState.js b/superset/assets/src/dashboard/reducers/getInitialState.js
index e529bf4..13283ec 100644
--- a/superset/assets/src/dashboard/reducers/getInitialState.js
+++ b/superset/assets/src/dashboard/reducers/getInitialState.js
@@ -93,6 +93,7 @@ export default function(bootstrapData) {
description: slice.description,
description_markeddown: slice.description_markeddown,
modified: slice.modified ? slice.modified.replace(/<[^>]*>/g, '') : '',
+ changed_on: new Date(slice.changed_on).getTime(),
};
sliceIds.add(key);
diff --git a/superset/assets/src/dashboard/util/dashboardLayoutConverter.js b/superset/assets/src/dashboard/util/dashboardLayoutConverter.js
index c1d855c..9b66e41 100644
--- a/superset/assets/src/dashboard/util/dashboardLayoutConverter.js
+++ b/superset/assets/src/dashboard/util/dashboardLayoutConverter.js
@@ -398,12 +398,12 @@ export function convertToLayout(positions) {
function mergePosition(position, bottomLine, maxColumn) {
const { col, size_x, size_y } = position;
const endColumn = col + size_x > maxColumn ? bottomLine.length : col + size_x;
- const nextSectionStart =
+ const sectionLength =
bottomLine.slice(col).findIndex(value => value > bottomLine[col]) + 1;
const currentBottom =
- nextSectionStart > 0 && nextSectionStart < size_x
- ? Math.max.apply(null, bottomLine.slice(col, col + size_x + 1))
+ sectionLength > 0 && sectionLength < size_x
+ ? Math.max.apply(null, bottomLine.slice(col, col + size_x))
: bottomLine[col];
bottomLine.fill(currentBottom + size_y, col, endColumn);
}
diff --git a/superset/assets/src/dashboard/util/propShapes.jsx b/superset/assets/src/dashboard/util/propShapes.jsx
index 3427520..f155190 100644
--- a/superset/assets/src/dashboard/util/propShapes.jsx
+++ b/superset/assets/src/dashboard/util/propShapes.jsx
@@ -59,8 +59,8 @@ export const slicePropShape = PropTypes.shape({
datasource: PropTypes.string,
datasource_name: PropTypes.string,
datasource_link: PropTypes.string,
- changedOn: PropTypes.number,
- modified: PropTypes.string,
+ changed_on: PropTypes.number.isRequired,
+ modified: PropTypes.string.isRequired,
viz_type: PropTypes.string.isRequired,
description: PropTypes.string,
description_markeddown: PropTypes.string,
diff --git a/superset/models/core.py b/superset/models/core.py
index 000e0ae..e9d14be 100644
--- a/superset/models/core.py
+++ b/superset/models/core.py
@@ -189,6 +189,7 @@ class Slice(Model, AuditMixinNullable, ImportMixin):
'slice_name': self.slice_name,
'slice_url': self.slice_url,
'modified': self.modified(),
+ 'changed_on': self.changed_on.isoformat(),
}
@property
diff --git a/tests/core_tests.py b/tests/core_tests.py
index 34d8a64..3ea783a 100644
--- a/tests/core_tests.py
+++ b/tests/core_tests.py
@@ -226,6 +226,14 @@ class CoreTests(SupersetTestCase):
assert len(resp) > 0
assert 'Carbon Dioxide' in resp
+ def test_slice_data(self):
+ # slice data should have some required attributes
+ self.login(username='admin')
+ slc = self.get_slice('Girls', db.session)
+ slc_data_attributes = slc.data.keys()
+ assert('changed_on' in slc_data_attributes)
+ assert('modified' in slc_data_attributes)
+
def test_slices(self):
# Testing by hitting the two supported end points for all slices
self.login(username='admin')