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/07 00:00:55 UTC

[GitHub] williaster closed pull request #5141: allow add chart from explore view

williaster closed pull request #5141: allow add chart from explore view
URL: https://github.com/apache/incubator-superset/pull/5141
 
 
   

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/util/findFirstParentContainer_spec.js b/superset/assets/spec/javascripts/dashboard/util/findFirstParentContainer_spec.js
new file mode 100644
index 0000000000..4ab29bda06
--- /dev/null
+++ b/superset/assets/spec/javascripts/dashboard/util/findFirstParentContainer_spec.js
@@ -0,0 +1,116 @@
+import { describe, it } from 'mocha';
+import { expect } from 'chai';
+
+import findFirstParentContainerId from '../../../../src/dashboard/util/findFirstParentContainer';
+import {
+  DASHBOARD_GRID_ID,
+  DASHBOARD_ROOT_ID,
+} from '../../../../src/dashboard/util/constants';
+
+describe('findFirstParentContainer', () => {
+  const mockGridLayout = {
+    DASHBOARD_VERSION_KEY: 'v2',
+    DASHBOARD_ROOT_ID: {
+      type: 'DASHBOARD_ROOT_TYPE',
+      id: 'DASHBOARD_ROOT_ID',
+      children: ['DASHBOARD_GRID_ID'],
+    },
+    DASHBOARD_GRID_ID: {
+      type: 'DASHBOARD_GRID_TYPE',
+      id: 'DASHBOARD_GRID_ID',
+      children: ['DASHBOARD_ROW_TYPE-Bk45URrlQ'],
+    },
+    'DASHBOARD_ROW_TYPE-Bk45URrlQ': {
+      type: 'DASHBOARD_ROW_TYPE',
+      id: 'DASHBOARD_ROW_TYPE-Bk45URrlQ',
+      children: ['DASHBOARD_CHART_TYPE-ryxVc8RHlX'],
+    },
+    'DASHBOARD_CHART_TYPE-ryxVc8RHlX': {
+      type: 'DASHBOARD_CHART_TYPE',
+      id: 'DASHBOARD_CHART_TYPE-ryxVc8RHlX',
+      children: [],
+    },
+    DASHBOARD_HEADER_ID: {
+      id: 'DASHBOARD_HEADER_ID',
+      type: 'DASHBOARD_HEADER_TYPE',
+    },
+  };
+  const mockTabsLayout = {
+    'DASHBOARD_CHART_TYPE-S1gilYABe7': {
+      children: [],
+      id: 'DASHBOARD_CHART_TYPE-S1gilYABe7',
+      type: 'DASHBOARD_CHART_TYPE',
+    },
+    'DASHBOARD_CHART_TYPE-SJli5K0HlQ': {
+      children: [],
+      id: 'DASHBOARD_CHART_TYPE-SJli5K0HlQ',
+      type: 'DASHBOARD_CHART_TYPE',
+    },
+    DASHBOARD_GRID_ID: {
+      children: [],
+      id: 'DASHBOARD_GRID_ID',
+      type: 'DASHBOARD_GRID_TYPE',
+    },
+    DASHBOARD_HEADER_ID: {
+      id: 'DASHBOARD_HEADER_ID',
+      type: 'DASHBOARD_HEADER_TYPE',
+    },
+    DASHBOARD_ROOT_ID: {
+      children: ['DASHBOARD_TABS_TYPE-SkgJ5t0Bem'],
+      id: 'DASHBOARD_ROOT_ID',
+      type: 'DASHBOARD_ROOT_TYPE',
+    },
+    'DASHBOARD_ROW_TYPE-S1B8-JLgX': {
+      children: ['DASHBOARD_CHART_TYPE-SJli5K0HlQ'],
+      id: 'DASHBOARD_ROW_TYPE-S1B8-JLgX',
+      type: 'DASHBOARD_ROW_TYPE',
+    },
+    'DASHBOARD_ROW_TYPE-S1bUb1Ilm': {
+      children: ['DASHBOARD_CHART_TYPE-S1gilYABe7'],
+      id: 'DASHBOARD_ROW_TYPE-S1bUb1Ilm',
+      type: 'DASHBOARD_ROW_TYPE',
+    },
+    'DASHBOARD_TABS_TYPE-ByeLSWyLe7': {
+      children: ['DASHBOARD_TAB_TYPE-BJbLSZ1UeQ'],
+      id: 'DASHBOARD_TABS_TYPE-ByeLSWyLe7',
+      type: 'DASHBOARD_TABS_TYPE',
+    },
+    'DASHBOARD_TABS_TYPE-SkgJ5t0Bem': {
+      children: [
+        'DASHBOARD_TAB_TYPE-HkWJcFCHxQ',
+        'DASHBOARD_TAB_TYPE-ByDBbkLlQ',
+      ],
+      id: 'DASHBOARD_TABS_TYPE-SkgJ5t0Bem',
+      meta: {},
+      type: 'DASHBOARD_TABS_TYPE',
+    },
+    'DASHBOARD_TAB_TYPE-BJbLSZ1UeQ': {
+      children: ['DASHBOARD_ROW_TYPE-S1bUb1Ilm'],
+      id: 'DASHBOARD_TAB_TYPE-BJbLSZ1UeQ',
+      type: 'DASHBOARD_TAB_TYPE',
+    },
+    'DASHBOARD_TAB_TYPE-ByDBbkLlQ': {
+      children: ['DASHBOARD_ROW_TYPE-S1B8-JLgX'],
+      id: 'DASHBOARD_TAB_TYPE-ByDBbkLlQ',
+      type: 'DASHBOARD_TAB_TYPE',
+    },
+    'DASHBOARD_TAB_TYPE-HkWJcFCHxQ': {
+      children: ['DASHBOARD_TABS_TYPE-ByeLSWyLe7'],
+      id: 'DASHBOARD_TAB_TYPE-HkWJcFCHxQ',
+      type: 'DASHBOARD_TAB_TYPE',
+    },
+    DASHBOARD_VERSION_KEY: 'v2',
+  };
+
+  it('should return grid root', () => {
+    expect(findFirstParentContainerId(mockGridLayout)).to.equal(
+      DASHBOARD_GRID_ID,
+    );
+  });
+
+  it('should return first tab', () => {
+    const tabsId = mockTabsLayout[DASHBOARD_ROOT_ID].children[0];
+    const firstTabId = mockTabsLayout[tabsId].children[0];
+    expect(findFirstParentContainerId(mockTabsLayout)).to.equal(firstTabId);
+  });
+});
diff --git a/superset/assets/src/dashboard/reducers/getInitialState.js b/superset/assets/src/dashboard/reducers/getInitialState.js
index f4e091e9ac..7378c7b56f 100644
--- a/superset/assets/src/dashboard/reducers/getInitialState.js
+++ b/superset/assets/src/dashboard/reducers/getInitialState.js
@@ -6,9 +6,16 @@ import { initSliceEntities } from './sliceEntities';
 import { getParam } from '../../modules/utils';
 import { applyDefaultFormData } from '../../explore/store';
 import { getColorFromScheme } from '../../modules/colors';
+import findFirstParentContainerId from '../util/findFirstParentContainer';
 import layoutConverter from '../util/dashboardLayoutConverter';
+import getEmptyLayout from '../util/getEmptyLayout';
+import newComponentFactory from '../util/newComponentFactory';
 import { DASHBOARD_VERSION_KEY, DASHBOARD_HEADER_ID } from '../util/constants';
-import { DASHBOARD_HEADER_TYPE, CHART_TYPE } from '../util/componentTypes';
+import {
+  DASHBOARD_HEADER_TYPE,
+  CHART_TYPE,
+  ROW_TYPE,
+} from '../util/componentTypes';
 
 export default function(bootstrapData) {
   const {
@@ -48,22 +55,9 @@ export default function(bootstrapData) {
   const shouldConvertToV2 =
     !positionJson || positionJson[DASHBOARD_VERSION_KEY] !== 'v2';
 
-  const layout = shouldConvertToV2 ? layoutConverter(dashboard) : positionJson;
-
-  // store the header as a layout component so we can undo/redo changes
-  layout[DASHBOARD_HEADER_ID] = {
-    id: DASHBOARD_HEADER_ID,
-    type: DASHBOARD_HEADER_TYPE,
-    meta: {
-      text: dashboard.dashboard_title,
-    },
-  };
-
-  const dashboardLayout = {
-    past: [],
-    present: layout,
-    future: [],
-  };
+  const layout = shouldConvertToV2
+    ? layoutConverter(dashboard)
+    : positionJson || getEmptyLayout();
 
   // create a lookup to sync layout names with slice names
   const chartIdToLayoutId = {};
@@ -73,6 +67,9 @@ export default function(bootstrapData) {
     }
   });
 
+  // find root level chart container node for newly-added slices
+  const parentId = findFirstParentContainerId(layout);
+  let hasUnsavedChanges = false;
   const chartQueries = {};
   const slices = {};
   const sliceIds = new Set();
@@ -99,6 +96,23 @@ export default function(bootstrapData) {
       };
 
       sliceIds.add(key);
+
+      // if chart is newly added from explore view, add a row in layout
+      if (!chartIdToLayoutId[key] && layout[parentId]) {
+        const parent = layout[parentId];
+        const rowContainer = newComponentFactory(ROW_TYPE);
+        layout[rowContainer.id] = rowContainer;
+        parent.children.push(rowContainer.id);
+
+        const chartHolder = newComponentFactory(CHART_TYPE, {
+          chartId: slice.slice_id,
+        });
+
+        layout[chartHolder.id] = chartHolder;
+        rowContainer.children.push(chartHolder.id);
+        chartIdToLayoutId[chartHolder.meta.chartId] = chartHolder.id;
+        hasUnsavedChanges = true;
+      }
     }
 
     // sync layout names with current slice names in case a slice was edited
@@ -110,6 +124,21 @@ export default function(bootstrapData) {
     }
   });
 
+  // store the header as a layout component so we can undo/redo changes
+  layout[DASHBOARD_HEADER_ID] = {
+    id: DASHBOARD_HEADER_ID,
+    type: DASHBOARD_HEADER_TYPE,
+    meta: {
+      text: dashboard.dashboard_title,
+    },
+  };
+
+  const dashboardLayout = {
+    past: [],
+    present: layout,
+    future: [],
+  };
+
   return {
     datasources,
     sliceEntities: { ...initSliceEntities, slices, isLoading: false },
@@ -143,7 +172,7 @@ export default function(bootstrapData) {
       css: dashboard.css || '',
       editMode: dashboard.dash_edit_perm && editMode,
       showBuilderPane: dashboard.dash_edit_perm && editMode,
-      hasUnsavedChanges: false,
+      hasUnsavedChanges,
       maxUndoHistoryExceeded: false,
       isV2Preview: shouldConvertToV2,
     },
diff --git a/superset/assets/src/dashboard/util/dashboardLayoutConverter.js b/superset/assets/src/dashboard/util/dashboardLayoutConverter.js
index e28e3be314..cf7a4939fc 100644
--- a/superset/assets/src/dashboard/util/dashboardLayoutConverter.js
+++ b/superset/assets/src/dashboard/util/dashboardLayoutConverter.js
@@ -1,32 +1,23 @@
 /* eslint-disable no-param-reassign */
 /* eslint-disable camelcase */
 /* eslint-disable no-loop-func */
+import shortid from 'shortid';
+
+import getEmptyLayout from './getEmptyLayout';
+
 import {
   ROW_TYPE,
   COLUMN_TYPE,
   CHART_TYPE,
   MARKDOWN_TYPE,
-  DASHBOARD_ROOT_TYPE,
   DASHBOARD_GRID_TYPE,
 } from './componentTypes';
 
-import {
-  DASHBOARD_GRID_ID,
-  DASHBOARD_ROOT_ID,
-  DASHBOARD_VERSION_KEY,
-} from './constants';
+import { DASHBOARD_GRID_ID } from './constants';
 
 const MAX_RECURSIVE_LEVEL = 6;
 const GRID_RATIO = 4;
 const ROW_HEIGHT = 8;
-const generateId = (() => {
-  let componentId = 1;
-  return () => {
-    const id = componentId;
-    componentId += 1;
-    return id;
-  };
-})();
 
 /**
  *
@@ -54,6 +45,10 @@ function getBoundary(positions) {
   };
 }
 
+function generateId() {
+  return shortid.generate();
+}
+
 function getRowContainer() {
   return {
     type: ROW_TYPE,
@@ -275,19 +270,7 @@ function doConvert(positions, level, parent, root) {
 }
 
 export function convertToLayout(positions) {
-  const root = {
-    [DASHBOARD_VERSION_KEY]: 'v2',
-    [DASHBOARD_ROOT_ID]: {
-      type: DASHBOARD_ROOT_TYPE,
-      id: DASHBOARD_ROOT_ID,
-      children: [DASHBOARD_GRID_ID],
-    },
-    [DASHBOARD_GRID_ID]: {
-      type: DASHBOARD_GRID_TYPE,
-      id: DASHBOARD_GRID_ID,
-      children: [],
-    },
-  };
+  const root = getEmptyLayout();
 
   doConvert(positions, 0, root[DASHBOARD_GRID_ID], root);
 
diff --git a/superset/assets/src/dashboard/util/findFirstParentContainer.js b/superset/assets/src/dashboard/util/findFirstParentContainer.js
new file mode 100644
index 0000000000..d4a75ce320
--- /dev/null
+++ b/superset/assets/src/dashboard/util/findFirstParentContainer.js
@@ -0,0 +1,19 @@
+import { TABS_TYPE } from './componentTypes';
+import { DASHBOARD_ROOT_ID } from './constants';
+
+export default function(layout = {}) {
+  // DASHBOARD_GRID_TYPE or TABS_TYPE?
+  let parent = layout[DASHBOARD_ROOT_ID];
+  if (
+    parent &&
+    parent.children.length &&
+    layout[parent.children[0]].type === TABS_TYPE
+  ) {
+    const tabs = layout[parent.children[0]];
+    parent = layout[tabs.children[0]];
+  } else {
+    parent = layout[parent.children[0]];
+  }
+
+  return parent.id;
+}
diff --git a/superset/assets/src/dashboard/util/getEmptyLayout.js b/superset/assets/src/dashboard/util/getEmptyLayout.js
new file mode 100644
index 0000000000..bc5fb71779
--- /dev/null
+++ b/superset/assets/src/dashboard/util/getEmptyLayout.js
@@ -0,0 +1,23 @@
+import { DASHBOARD_ROOT_TYPE, DASHBOARD_GRID_TYPE } from './componentTypes';
+
+import {
+  DASHBOARD_GRID_ID,
+  DASHBOARD_ROOT_ID,
+  DASHBOARD_VERSION_KEY,
+} from './constants';
+
+export default function() {
+  return {
+    [DASHBOARD_VERSION_KEY]: 'v2',
+    [DASHBOARD_ROOT_ID]: {
+      type: DASHBOARD_ROOT_TYPE,
+      id: DASHBOARD_ROOT_ID,
+      children: [DASHBOARD_GRID_ID],
+    },
+    [DASHBOARD_GRID_ID]: {
+      type: DASHBOARD_GRID_TYPE,
+      id: DASHBOARD_GRID_ID,
+      children: [],
+    },
+  };
+}


 

----------------------------------------------------------------
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