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.
apache · Jun 17, 2022 · 33090fe · 33090fe
1 parent 679c171
commit 33090fe
Show file tree

Hide file tree

Showing 4 changed files with 123 additions and 61 deletions.
diff --git 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(<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);
 });
diff --git a/superset-frontend/src/addSlice/AddSliceContainer.tsx b/superset-frontend/src/addSlice/AddSliceContainer.tsx
@@ -29,6 +29,8 @@ 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;
@@ -37,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;
@@ -204,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);
@@ -292,6 +302,40 @@ 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>
@@ -312,25 +356,7 @@ export default class AddSliceContainer extends React.PureComponent<
                   showSearch
                   value={this.state.datasource}
                 />
-                <span>
-                  <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>
+                {datasetHelpText}
               </StyledStepDescription>
             }
           />

diff --git a/superset-frontend/src/addSlice/App.tsx b/superset-frontend/src/addSlice/App.tsx
@@ -41,7 +41,7 @@ const App = () => (
   <ThemeProvider theme={theme}>
     <GlobalStyles />
     <DynamicPluginProvider>
-      <AddSliceContainer />
+      <AddSliceContainer user={bootstrapData.user} />
     </DynamicPluginProvider>
   </ThemeProvider>
 );

diff --git a/superset/views/chart/views.py b/superset/views/chart/views.py
@@ -63,7 +63,7 @@ def pre_delete(self, item: "SliceModelView") -> None:
     def add(self) -> FlaskResponse:
         payload = {
             "common": common_bootstrap_payload(),
-            "user": bootstrap_user_data(g.user),
+            "user": bootstrap_user_data(g.user, include_perms=True),
         }
         return self.render_template(
             "superset/add_slice.html", bootstrap_data=json.dumps(payload)