You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2018/06/21 23:42:34 UTC

[GitHub] graceguo-supercat closed pull request #5218: [fix] layout converter fix

graceguo-supercat closed pull request #5218: [fix] layout converter fix
URL: https://github.com/apache/incubator-superset/pull/5218
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx b/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx
index 209404092f..e855009592 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 0000000000..da0f7df754
--- /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 5fff31325d..dcd711947a 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 7a12454d39..0000000000
--- 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 7c43bea534..3644c100a2 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 e3d931dc51..0000000000
--- 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 e529bf4bc9..13283ec136 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 c1d855c203..9b66e41694 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 3427520644..f15519094b 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 000e0ae771..e9d14bebb2 100644
--- a/superset/models/core.py
+++ b/superset/models/core.py
@@ -189,6 +189,7 @@ def data(self):
             '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 34d8a64677..3ea783ae70 100644
--- a/tests/core_tests.py
+++ b/tests/core_tests.py
@@ -226,6 +226,14 @@ def test_filter_endpoint(self):
         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')


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org