allow add chart from explore view

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);
+  });
+});

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

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

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;
+}

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: [],
+    },
+  };
+}