chore(newchart): update chart creation dataset selection help text, styles

superset-frontend/src/addSlice/AddSliceContainer.test.tsx

@@ -27,61 +27,97 @@ import AddSliceContainer, {
 import VizTypeGallery from 'src/explore/components/controls/VizTypeControl/VizTypeGallery';
 import { styledMount as mount } from 'spec/helpers/theming';
 import { act } from 'spec/helpers/testing-library';
+import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
 
 const datasource = {
  value: '1',
  label: 'table',
 };
 
-describe('AddSliceContainer', () => {
- let wrapper: ReactWrapper<
+const mockUser: UserWithPermissionsAndRoles = {
+ createdOn: '2021-04-27T18:12:38.952304',
+ email: 'admin',
+ firstName: 'admin',
+ isActive: true,
+ lastName: 'admin',
+ permissions: {},
+ roles: { Admin: Array(173) },
+ userId: 1,
+ username: 'admin',
+ isAnonymous: false,
+};
+
+const mockUserWithDatasetWrite: UserWithPermissionsAndRoles = {
+ createdOn: '2021-04-27T18:12:38.952304',
+ email: 'admin',
+ firstName: 'admin',
+ isActive: true,
+ lastName: 'admin',
+ permissions: {},
+ roles: { Admin: [['can_write', 'Dataset']] },
+ userId: 1,
+ username: 'admin',
+ isAnonymous: false,
+};
+
+async function getWrapper(user = mockUser) {
+ const wrapper = mount(<AddSliceContainer user={user} />) as ReactWrapper<
  AddSliceContainerProps,
  AddSliceContainerState,
  AddSliceContainer
  >;
+ await act(() => new Promise(resolve => setTimeout(resolve, 0)));
+ return wrapper;
+}
 
- beforeEach(async () => {
- wrapper = mount(<AddSliceContainer />) as ReactWrapper<
- AddSliceContainerProps,
- AddSliceContainerState,
- AddSliceContainer
- >;
- // suppress a warning caused by some unusual async behavior in Icon
- await act(() => new Promise(resolve => setTimeout(resolve, 0)));
- });
+it('renders a select and a VizTypeControl', async () => {
+ const wrapper = await getWrapper();
+ expect(wrapper.find(Select)).toExist();
+ expect(wrapper.find(VizTypeGallery)).toExist();
+});
 
- it('renders a select and a VizTypeControl', () => {
- expect(wrapper.find(Select)).toExist();
- expect(wrapper.find(VizTypeGallery)).toExist();
- });
+it('renders dataset help text when user lacks dataset write permissions', async () => {
+ const wrapper = await getWrapper();
+ expect(wrapper.find('[data-test="dataset-write"]')).not.toExist();
+ expect(wrapper.find('[data-test="no-dataset-write"]')).toExist();
+});
 
- it('renders a button', () => {
- expect(wrapper.find(Button)).toExist();
- });
+it('renders dataset help text when user has dataset write permissions', async () => {
+ const wrapper = await getWrapper(mockUserWithDatasetWrite);
+ expect(wrapper.find('[data-test="dataset-write"]')).toExist();
+ expect(wrapper.find('[data-test="no-dataset-write"]')).not.toExist();
+});
 
- it('renders a disabled button if no datasource is selected', () => {
- expect(
- wrapper.find(Button).find({ disabled: true }).hostNodes(),
- ).toHaveLength(1);
- });
+it('renders a button', async () => {
+ const wrapper = await getWrapper();
+ expect(wrapper.find(Button)).toExist();
+});
 
- it('renders an enabled button if datasource and viz type is selected', () => {
- wrapper.setState({
- datasource,
- visType: 'table',
- });
- expect(
- wrapper.find(Button).find({ disabled: true }).hostNodes(),
- ).toHaveLength(0);
+it('renders a disabled button if no datasource is selected', async () => {
+ const wrapper = await getWrapper();
+ expect(
+ wrapper.find(Button).find({ disabled: true }).hostNodes(),
+ ).toHaveLength(1);
+});
+
+it('renders an enabled button if datasource and viz type is selected', async () => {
+ const wrapper = await getWrapper();
+ wrapper.setState({
+ datasource,
+ visType: 'table',
  });
+ expect(
+ wrapper.find(Button).find({ disabled: true }).hostNodes(),
+ ).toHaveLength(0);
+});
 
- it('formats explore url', () => {
- wrapper.setState({
- datasource,
- visType: 'table',
- });
- const formattedUrl =
- '/superset/explore/?form_data=%7B%22viz_type%22%3A%22table%22%2C%22datasource%22%3A%221%22%7D';
- expect(wrapper.instance().exploreUrl()).toBe(formattedUrl);
+it('formats explore url', async () => {
+ const wrapper = await getWrapper();
+ wrapper.setState({
+ datasource,
+ visType: 'table',
  });
+ const formattedUrl =
+ '/superset/explore/?form_data=%7B%22viz_type%22%3A%22table%22%2C%22datasource%22%3A%221%22%7D';
+ expect(wrapper.instance().exploreUrl()).toBe(formattedUrl);
 });

superset-frontend/src/addSlice/AddSliceContainer.tsx

@@ -24,12 +24,13 @@ import { URL_PARAMS } from 'src/constants';
 import { isNullish } from 'src/utils/common';
 import Button from 'src/components/Button';
 import { Select, Steps } from 'src/components';
-import { FormLabel } from 'src/components/Form';
 import { Tooltip } from 'src/components/Tooltip';
 
 import VizTypeGallery, {
  MAX_ADVISABLE_VIZ_GALLERY_WIDTH,
 } from 'src/explore/components/controls/VizTypeControl/VizTypeGallery';
+import findPermission from 'src/dashboard/util/findPermission';
+import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
 
 type Dataset = {
  id: number;
@@ -38,11 +39,14 @@ type Dataset = {
  datasource_type: string;
 };
 
-export type AddSliceContainerProps = {};
+export type AddSliceContainerProps = {
+ user: UserWithPermissionsAndRoles;
+};
 
 export type AddSliceContainerState = {
  datasource?: { label: string; value: string };
  visType: string | null;
+ canCreateDataset: boolean;
 };
 
 const ESTIMATED_NAV_HEIGHT = 56;
@@ -73,7 +77,6 @@ const StyledContainer = styled.div`
  display: flex;
  flex-direction: row;
  align-items: center;
- margin-bottom: ${theme.gridUnit * 2}px;
 
  & > div {
  min-width: 200px;
@@ -180,6 +183,24 @@ const StyledLabel = styled.span`
  `}
 `;
 
+const StyledStepTitle = styled.span`
+ ${({
+ theme: {
+ typography: { sizes, weights },
+ },
+ }) => `
+ font-size: ${sizes.m}px;
+ font-weight: ${weights.bold};
+ `}
+`;
+
+const StyledStepDescription = styled.div`
+ ${({ theme: { gridUnit } }) => `
+ margin-top: ${gridUnit * 4}px;
+ margin-bottom: ${gridUnit * 3}px;
+ `}
+`;
+
 export default class AddSliceContainer extends React.PureComponent<
  AddSliceContainerProps,
  AddSliceContainerState
@@ -188,6 +209,11 @@ export default class AddSliceContainer extends React.PureComponent<
  super(props);
  this.state = {
  visType: null,
+ canCreateDataset: findPermission(
+ 'can_write',
+ 'Dataset',
+ props.user.roles,
+ ),
  };
 
  this.changeDatasource = this.changeDatasource.bind(this);
@@ -276,15 +302,49 @@ export default class AddSliceContainer extends React.PureComponent<
 
  render() {
  const isButtonDisabled = this.isBtnDisabled();
+ const datasetHelpText = this.state.canCreateDataset ? (
+ <span data-test="dataset-write">
+ <a
+ href="/tablemodelview/list/#create"
+ rel="noopener noreferrer"
+ target="_blank"
+ >
+ {t('Add a dataset')}
+ </a>
+ {` ${t('or')} `}
+ <a
+ href="https://superset.apache.org/docs/creating-charts-dashboards/creating-your-first-dashboard/#registering-a-new-table"
+ rel="noopener noreferrer"
+ target="_blank"
+ >
+ {`${t('view instructions')} `}
+ <i className="fa fa-external-link" />
+ </a>
+ .
+ </span>
+ ) : (
+ <span data-test="no-dataset-write">
+ <a
+ href="https://superset.apache.org/docs/creating-charts-dashboards/creating-your-first-dashboard/#registering-a-new-table"
+ rel="noopener noreferrer"
+ target="_blank"
+ >
+ {`${t('View instructions')} `}
+ <i className="fa fa-external-link" />
+ </a>
+ .
+ </span>
+ );
+
  return (
  <StyledContainer>
  <h3>{t('Create a new chart')}</h3>
  <Steps direction="vertical" size="small">
  <Steps.Step
- title={<FormLabel>{t('Choose a dataset')}</FormLabel>}
+ title={<StyledStepTitle>{t('Choose a dataset')}</StyledStepTitle>}
  status={this.state.datasource?.value ? 'finish' : 'process'}
  description={
- <div className="dataset">
+ <StyledStepDescription className="dataset">
  <Select
  autoFocus
  ariaLabel={t('Dataset')}
@@ -296,30 +356,21 @@ export default class AddSliceContainer extends React.PureComponent<
  showSearch
  value={this.state.datasource}
  />
- <span>
- {t(
- 'Instructions to add a dataset are available in the Superset tutorial.',
- )}{' '}
- <a
- href="https://superset.apache.org/docs/creating-charts-dashboards/creating-your-first-dashboard/#registering-a-new-table"
- rel="noopener noreferrer"
- target="_blank"
- >
- <i className="fa fa-external-link" />
- </a>
- </span>
- </div>
+ {datasetHelpText}
+ </StyledStepDescription>
  }
  />
  <Steps.Step
- title={<FormLabel>{t('Choose chart type')}</FormLabel>}
+ title={<StyledStepTitle>{t('Choose chart type')}</StyledStepTitle>}
  status={this.state.visType ? 'finish' : 'process'}
  description={
- <VizTypeGallery
- className="viz-gallery"
- onChange={this.changeVisType}
- selectedViz={this.state.visType}
- />
+ <StyledStepDescription>
+ <VizTypeGallery
+ className="viz-gallery"
+ onChange={this.changeVisType}
+ selectedViz={this.state.visType}
+ />
+ </StyledStepDescription>
  }
  />
  </Steps>

superset-frontend/src/addSlice/App.tsx

@@ -41,7 +41,7 @@ const App = () => (
  <ThemeProvider theme={theme}>
  <GlobalStyles />
  <DynamicPluginProvider>
- <AddSliceContainer />
+ <AddSliceContainer user={bootstrapData.user} />
  </DynamicPluginProvider>
  </ThemeProvider>
 );

superset-frontend/src/views/CRUD/data/dataset/DatasetList.test.jsx

@@ -183,6 +183,12 @@ describe('DatasetList', () => {
  });
 });
 
+jest.mock('react-router-dom', () => ({
+ ...jest.requireActual('react-router-dom'),
+ useLocation: () => ({}),
+ useHistory: () => ({}),
+}));
+
 describe('RTL', () => {
  async function renderAndWait() {
  const mounted = act(async () => {
@@ -191,7 +197,7 @@ describe('RTL', () => {
  <QueryParamProvider>
  <DatasetList {...mockedProps} user={mockUser} />
  </QueryParamProvider>,
- { useRedux: true },
+ { useRedux: true, useRouter: true },
  );
  });
 

superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx

@@ -22,8 +22,10 @@ import React, {
  useState,
  useMemo,
  useCallback,
+ useEffect,
 } from 'react';
 import rison from 'rison';
+import { useHistory, useLocation } from 'react-router-dom';
 import {
  createFetchRelated,
  createFetchDistinct,
@@ -553,14 +555,34 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
  });
  }
 
+ const CREATE_HASH = '#create';
+ const location = useLocation();
+ const history = useHistory();
+
+ // Sync Dataset Add modal with #create hash
+ useEffect(() => {
+ const modalOpen = location.hash === CREATE_HASH && canCreate;
+ setDatasetAddModalOpen(modalOpen);
+ }, [canCreate, location.hash]);
+
+ // Add #create hash
+ const openDatasetAddModal = useCallback(() => {
+ history.replace(`${location.pathname}${location.search}${CREATE_HASH}`);
+ }, [history, location.pathname, location.search]);
+
+ // Remove #create hash
+ const closeDatasetAddModal = useCallback(() => {
+ history.replace(`${location.pathname}${location.search}`);
+ }, [history, location.pathname, location.search]);
+
  if (canCreate) {
  buttonArr.push({
  name: (
  <>
  <i className="fa fa-plus" /> {t('Dataset')}{' '}
  </>
  ),
- onClick: () => setDatasetAddModalOpen(true),
+ onClick: openDatasetAddModal,
  buttonStyle: 'primary',
  });
 
@@ -631,7 +653,7 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
  <SubMenu {...menuData} />
  <AddDatasetModal
  show={datasetAddModalOpen}
- onHide={() => setDatasetAddModalOpen(false)}
+ onHide={closeDatasetAddModal}
  onDatasetAdd={refreshData}
  />
  {datasetCurrentlyDeleting && (