fix(tags): Polish + Better messaging for skipped tags with bad permis…

…sions (apache#25578)

(cherry picked from commit 9074f72)

superset-frontend/src/features/tags/BulkTagModal.tsx

@@ -67,7 +67,18 @@ const BulkTagModal: React.FC<BulkTagModalProps> = ({
       },
     })
       .then(({ json = {} }) => {
-        addSuccessToast(t('Tagged %s %ss', selected.length, resourceName));
+        const skipped = json.result.objects_skipped;
+        const tagged = json.result.objects_tagged;
+        if (skipped.length > 0) {
+          addSuccessToast(
+            t(
+              '%s items could not be tagged because you don’t have edit permissions to all selected objects.',
+              skipped.length,
+              resourceName,
+            ),
+          );
+        }
+        addSuccessToast(t('Tagged %s %ss', tagged.length, resourceName));
       })
       .catch(err => {
         addDangerToast(t('Failed to tag items'));

superset-frontend/src/pages/AllEntities/index.tsx

@@ -164,21 +164,25 @@ function AllEntities() {
     );
   };
 
+  const fetchTag = (tagId: number) => {
+    fetchSingleTag(
+      tagId,
+      (tag: Tag) => {
+        setTag(tag);
+        setLoading(false);
+      },
+      (error: Response) => {
+        addDangerToast(t('Error Fetching Tagged Objects'));
+        setLoading(false);
+      },
+    );
+  };
+
   useEffect(() => {
     // fetch single tag met
     if (tagId) {
       setLoading(true);
-      fetchSingleTag(
-        tagId,
-        (tag: Tag) => {
-          setTag(tag);
-          setLoading(false);
-        },
-        (error: Response) => {
-          addDangerToast(t('Error Fetching Tagged Objects'));
-          setLoading(false);
-        },
-      );
+      fetchTag(tagId);
     }
   }, [tagId]);
 
@@ -197,7 +201,10 @@ function AllEntities() {
         editTag={tag}
         addSuccessToast={addSuccessToast}
         addDangerToast={addDangerToast}
-        refreshData={fetchTaggedObjects}
+        refreshData={() => {
+          fetchTaggedObjects();
+          if (tagId) fetchTag(tagId);
+        }}
       />
       <AllEntitiesNav>
         <PageHeaderWithActions

superset-frontend/src/pages/ChartList/ChartList.test.jsx

@@ -20,7 +20,7 @@ import React from 'react';
 import { MemoryRouter } from 'react-router-dom';
 import thunk from 'redux-thunk';
 import configureStore from 'redux-mock-store';
-import { Provider } from 'react-redux';
+import * as reactRedux from 'react-redux';
 import fetchMock from 'fetch-mock';
 import * as uiCore from '@superset-ui/core';
 import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint';
@@ -38,9 +38,6 @@ import ListViewCard from 'src/components/ListViewCard';
 import FaveStar from 'src/components/FaveStar';
 import TableCollection from 'src/components/TableCollection';
 import CardCollection from 'src/components/ListView/CardCollection';
-// store needed for withToasts(ChartTable)
-const mockStore = configureStore([thunk]);
-const store = mockStore({});
 
 const chartsInfoEndpoint = 'glob:*/api/v1/chart/_info*';
 const chartsOwnersEndpoint = 'glob:*/api/v1/chart/related/owners*';
@@ -99,6 +96,29 @@ fetchMock.get(datasetEndpoint, {});
 global.URL.createObjectURL = jest.fn();
 fetchMock.get('/thumbnail', { body: new Blob(), sendAsJson: false });
 
+const user = {
+  createdOn: '2021-04-27T18:12:38.952304',
+  email: 'admin',
+  firstName: 'admin',
+  isActive: true,
+  lastName: 'admin',
+  permissions: {},
+  roles: {
+    Admin: [
+      ['can_sqllab', 'Superset'],
+      ['can_write', 'Dashboard'],
+      ['can_write', 'Chart'],
+    ],
+  },
+  userId: 1,
+  username: 'admin',
+};
+
+// store needed for withToasts(DatabaseList)
+const mockStore = configureStore([thunk]);
+const store = mockStore({ user });
+const useSelectorMock = jest.spyOn(reactRedux, 'useSelector');
+
 describe('ChartList', () => {
   const isFeatureEnabledMock = jest
     .spyOn(uiCore, 'isFeatureEnabled')
@@ -107,16 +127,22 @@ describe('ChartList', () => {
   afterAll(() => {
     isFeatureEnabledMock.restore();
   });
+
+  beforeEach(() => {
+    // setup a DOM element as a render target
+    useSelectorMock.mockClear();
+  });
+
   const mockedProps = {};
 
   let wrapper;
 
   beforeAll(async () => {
     wrapper = mount(
       <MemoryRouter>
-        <Provider store={store}>
+        <reactRedux.Provider store={store}>
           <ChartList {...mockedProps} user={mockUser} />
-        </Provider>
+        </reactRedux.Provider>
       </MemoryRouter>,
     );
 
@@ -231,9 +257,9 @@ describe('ChartList - anonymous view', () => {
     fetchMock.resetHistory();
     wrapper = mount(
       <MemoryRouter>
-        <Provider store={store}>
+        <reactRedux.Provider store={store}>
           <ChartList {...mockedProps} user={mockUserLoggedOut} />
-        </Provider>
+        </reactRedux.Provider>
       </MemoryRouter>,
     );
 

superset-frontend/src/pages/ChartList/index.tsx

@@ -30,6 +30,7 @@ import React, { useState, useMemo, useCallback } from 'react';
 import rison from 'rison';
 import { uniqBy } from 'lodash';
 import moment from 'moment';
+import { useSelector } from 'react-redux';
 import {
   createErrorHandler,
   createFetchRelated,
@@ -71,6 +72,8 @@ import { GenericLink } from 'src/components/GenericLink/GenericLink';
 import Owner from 'src/types/Owner';
 import { loadTags } from 'src/components/Tags/utils';
 import ChartCard from 'src/features/charts/ChartCard';
+import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
+import { findPermission } from 'src/utils/findPermission';
 
 const FlexRowContainer = styled.div`
   align-items: center;
@@ -179,6 +182,10 @@ function ChartList(props: ChartListProps) {
   } = useListViewResource<Chart>('chart', t('chart'), addDangerToast);
 
   const chartIds = useMemo(() => charts.map(c => c.id), [charts]);
+  const { roles } = useSelector<any, UserWithPermissionsAndRoles>(
+    state => state.user,
+  );
+  const canReadTag = findPermission('can_read', 'Tag', roles);
 
   const [saveFavoriteStatus, favoriteStatus] = useFavoriteStatus(
     'chart',
@@ -701,7 +708,7 @@ function ChartList(props: ChartListProps) {
         ],
       },
     ] as Filters;
-    if (isFeatureEnabled(FeatureFlag.TAGGING_SYSTEM)) {
+    if (isFeatureEnabled(FeatureFlag.TAGGING_SYSTEM) && canReadTag) {
       filters_list.push({
         Header: t('Tags'),
         key: 'tags',

superset-frontend/src/pages/DashboardList/DashboardList.test.jsx

@@ -21,7 +21,7 @@ import { MemoryRouter } from 'react-router-dom';
 import thunk from 'redux-thunk';
 import configureStore from 'redux-mock-store';
 import fetchMock from 'fetch-mock';
-import { Provider } from 'react-redux';
+import * as reactRedux from 'react-redux';
 import * as uiCore from '@superset-ui/core';
 
 import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint';
@@ -40,10 +40,6 @@ import FaveStar from 'src/components/FaveStar';
 import TableCollection from 'src/components/TableCollection';
 import CardCollection from 'src/components/ListView/CardCollection';
 
-// store needed for withToasts(DashboardTable)
-const mockStore = configureStore([thunk]);
-const store = mockStore({});
-
 const dashboardsInfoEndpoint = 'glob:*/api/v1/dashboard/_info*';
 const dashboardOwnersEndpoint = 'glob:*/api/v1/dashboard/related/owners*';
 const dashboardCreatedByEndpoint =
@@ -95,6 +91,28 @@ fetchMock.get(dashboardEndpoint, {
 
 global.URL.createObjectURL = jest.fn();
 fetchMock.get('/thumbnail', { body: new Blob(), sendAsJson: false });
+const user = {
+  createdOn: '2021-04-27T18:12:38.952304',
+  email: 'admin',
+  firstName: 'admin',
+  isActive: true,
+  lastName: 'admin',
+  permissions: {},
+  roles: {
+    Admin: [
+      ['can_sqllab', 'Superset'],
+      ['can_write', 'Dashboard'],
+      ['can_write', 'Chart'],
+    ],
+  },
+  userId: 1,
+  username: 'admin',
+};
+
+// store needed for withToasts(DatabaseList)
+const mockStore = configureStore([thunk]);
+const store = mockStore({ user });
+const useSelectorMock = jest.spyOn(reactRedux, 'useSelector');
 
 describe('DashboardList', () => {
   const isFeatureEnabledMock = jest
@@ -105,16 +123,21 @@ describe('DashboardList', () => {
     isFeatureEnabledMock.restore();
   });
 
+  beforeEach(() => {
+    // setup a DOM element as a render target
+    useSelectorMock.mockClear();
+  });
+
   const mockedProps = {};
   let wrapper;
 
   beforeAll(async () => {
     fetchMock.resetHistory();
     wrapper = mount(
       <MemoryRouter>
-        <Provider store={store}>
+        <reactRedux.Provider store={store}>
           <DashboardList {...mockedProps} user={mockUser} />
-        </Provider>
+        </reactRedux.Provider>
       </MemoryRouter>,
     );
 
@@ -249,9 +272,9 @@ describe('DashboardList - anonymous view', () => {
     fetchMock.resetHistory();
     wrapper = mount(
       <MemoryRouter>
-        <Provider store={store}>
+        <reactRedux.Provider store={store}>
           <DashboardList {...mockedProps} user={mockUserLoggedOut} />
-        </Provider>
+        </reactRedux.Provider>
       </MemoryRouter>,
     );
 

superset-frontend/src/pages/DashboardList/index.tsx

@@ -23,6 +23,7 @@ import {
   SupersetClient,
   t,
 } from '@superset-ui/core';
+import { useSelector } from 'react-redux';
 import React, { useState, useMemo, useCallback } from 'react';
 import { Link } from 'react-router-dom';
 import rison from 'rison';
@@ -61,6 +62,8 @@ import CertifiedBadge from 'src/components/CertifiedBadge';
 import { loadTags } from 'src/components/Tags/utils';
 import DashboardCard from 'src/features/dashboards/DashboardCard';
 import { DashboardStatus } from 'src/features/dashboards/types';
+import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
+import { findPermission } from 'src/utils/findPermission';
 
 const PAGE_SIZE = 25;
 const PASSWORDS_NEEDED_MESSAGE = t(
@@ -111,6 +114,11 @@ function DashboardList(props: DashboardListProps) {
     user: { userId },
   } = props;
 
+  const { roles } = useSelector<any, UserWithPermissionsAndRoles>(
+    state => state.user,
+  );
+  const canReadTag = findPermission('can_read', 'Tag', roles);
+
   const {
     state: {
       loading,
@@ -578,7 +586,7 @@ function DashboardList(props: DashboardListProps) {
         ],
       },
     ] as Filters;
-    if (isFeatureEnabled(FeatureFlag.TAGGING_SYSTEM)) {
+    if (isFeatureEnabled(FeatureFlag.TAGGING_SYSTEM) && canReadTag) {
       filters_list.push({
         Header: t('Tags'),
         key: 'tags',

superset-frontend/src/pages/SavedQueryList/SavedQueryList.test.jsx

@@ -18,9 +18,9 @@
  */
 import React from 'react';
 import thunk from 'redux-thunk';
+import * as reactRedux from 'react-redux';
 import { BrowserRouter } from 'react-router-dom';
 import configureStore from 'redux-mock-store';
-import { Provider } from 'react-redux';
 import fetchMock from 'fetch-mock';
 import { styledMount as mount } from 'spec/helpers/theming';
 import { render, screen, cleanup, waitFor } from 'spec/helpers/testing-library';
@@ -38,10 +38,6 @@ import Button from 'src/components/Button';
 import IndeterminateCheckbox from 'src/components/IndeterminateCheckbox';
 import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint';
 
-// store needed for withToasts(DatabaseList)
-const mockStore = configureStore([thunk]);
-const store = mockStore({});
-
 const queriesInfoEndpoint = 'glob:*/api/v1/saved_query/_info*';
 const queriesEndpoint = 'glob:*/api/v1/saved_query/?*';
 const queryEndpoint = 'glob:*/api/v1/saved_query/*';
@@ -75,6 +71,30 @@ const mockqueries = [...new Array(3)].map((_, i) => ({
   ],
 }));
 
+const user = {
+  createdOn: '2021-04-27T18:12:38.952304',
+  email: 'admin',
+  firstName: 'admin',
+  isActive: true,
+  lastName: 'admin',
+  permissions: {},
+  roles: {
+    Admin: [
+      ['can_sqllab', 'Superset'],
+      ['can_write', 'Dashboard'],
+      ['can_write', 'Chart'],
+    ],
+  },
+  userId: 1,
+  username: 'admin',
+};
+
+// store needed for withToasts(DatabaseList)
+const mockStore = configureStore([thunk]);
+const store = mockStore({ user });
+
+const useSelectorMock = jest.spyOn(reactRedux, 'useSelector');
+
 // ---------- For import testing ----------
 // Create an one more mocked query than the original mocked query array
 const mockOneMoreQuery = [...new Array(mockqueries.length + 1)].map((_, i) => ({
@@ -137,11 +157,16 @@ jest.mock('src/views/CRUD/utils');
 
 describe('SavedQueryList', () => {
   const wrapper = mount(
-    <Provider store={store}>
+    <reactRedux.Provider store={store}>
       <SavedQueryList />
-    </Provider>,
+    </reactRedux.Provider>,
   );
 
+  beforeEach(() => {
+    // setup a DOM element as a render target
+    useSelectorMock.mockClear();
+  });
+
   beforeAll(async () => {
     await waitForComponentToPaint(wrapper);
   });