From 44c5e2879b912b33c902b955eebdaa52ea6e5354 Mon Sep 17 00:00:00 2001 From: Cody Leff Date: Tue, 21 Jun 2022 11:01:43 -0600 Subject: [PATCH] chore(newchart): update chart creation dataset selection help text, styles (#20369) * Update dataset selection help text. * Update 'Create a new chart' flow styles. * Add support for linking directly to Create Dataset modal via URL hash. * Add support for linking directly to Create Dataset modal via URL hash. * Update dataset help text to not include spaces in translated strings and only include an 'Add dataset' link when user has permission to add dataset. * Clean up test file Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> --- .../src/addSlice/AddSliceContainer.test.tsx | 114 ++++++++++++------ .../src/addSlice/AddSliceContainer.tsx | 99 +++++++++++---- superset-frontend/src/addSlice/App.tsx | 2 +- .../CRUD/data/dataset/DatasetList.test.jsx | 8 +- .../views/CRUD/data/dataset/DatasetList.tsx | 26 +++- superset/views/chart/views.py | 2 +- 6 files changed, 183 insertions(+), 68 deletions(-) diff --git a/superset-frontend/src/addSlice/AddSliceContainer.test.tsx b/superset-frontend/src/addSlice/AddSliceContainer.test.tsx index 00e7276a5864..6187f574867c 100644 --- a/superset-frontend/src/addSlice/AddSliceContainer.test.tsx +++ b/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() as ReactWrapper< AddSliceContainerProps, AddSliceContainerState, AddSliceContainer >; + await act(() => new Promise(resolve => setTimeout(resolve, 0))); + return wrapper; +} - beforeEach(async () => { - wrapper = mount() as ReactWrapper< - AddSliceContainerProps, - AddSliceContainerState, - AddSliceContainer - >; - // suppress a warning caused by some unusual async behavior in Icon - await act(() => new Promise(resolve => setTimeout(resolve, 0))); - }); +test('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(); - }); +test('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(); - }); +test('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); - }); +test('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); +test('renders a disabled button if no datasource is selected', async () => { + const wrapper = await getWrapper(); + expect( + wrapper.find(Button).find({ disabled: true }).hostNodes(), + ).toHaveLength(1); +}); + +test('renders an enabled button if datasource and viz type are 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); +test('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); }); diff --git a/superset-frontend/src/addSlice/AddSliceContainer.tsx b/superset-frontend/src/addSlice/AddSliceContainer.tsx index fd22377314ac..66183219adbd 100644 --- a/superset-frontend/src/addSlice/AddSliceContainer.tsx +++ b/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 ? ( + + + {t('Add a dataset')} + + {` ${t('or')} `} + + {`${t('view instructions')} `} + + + . + + ) : ( + + + {`${t('View instructions')} `} + + + . + + ); + return (

{t('Create a new chart')}

{t('Choose a dataset')}} + title={{t('Choose a dataset')}} status={this.state.datasource?.value ? 'finish' : 'process'} description={ -
+