You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by hu...@apache.org on 2023/10/13 17:14:07 UTC

[superset] branch master updated: fix(tags): Polish + Better messaging for skipped tags with bad permissions (#25578)

This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 9074f72959 fix(tags): Polish + Better messaging for skipped tags with bad permissions (#25578)
9074f72959 is described below

commit 9074f72959956701bc46406ffb503670d9859b22
Author: Hugh A. Miles II <hu...@gmail.com>
AuthorDate: Fri Oct 13 13:13:59 2023 -0400

    fix(tags): Polish + Better messaging for skipped tags with bad permissions (#25578)
---
 .../src/features/tags/BulkTagModal.tsx             | 13 +++++-
 superset-frontend/src/pages/AllEntities/index.tsx  | 31 +++++++++------
 .../src/pages/ChartList/ChartList.test.jsx         | 42 ++++++++++++++++----
 superset-frontend/src/pages/ChartList/index.tsx    |  9 ++++-
 .../src/pages/DashboardList/DashboardList.test.jsx | 41 ++++++++++++++-----
 .../src/pages/DashboardList/index.tsx              | 10 ++++-
 .../pages/SavedQueryList/SavedQueryList.test.jsx   | 39 ++++++++++++++----
 .../src/pages/SavedQueryList/index.tsx             | 29 ++++++++++----
 superset/tags/api.py                               | 21 +++++++++-
 superset/tags/commands/create.py                   | 34 ++++++++++------
 superset/tags/commands/update.py                   |  6 +--
 tests/integration_tests/tags/api_tests.py          | 46 +++++++++++++++++++++-
 12 files changed, 255 insertions(+), 66 deletions(-)

diff --git a/superset-frontend/src/features/tags/BulkTagModal.tsx b/superset-frontend/src/features/tags/BulkTagModal.tsx
index 9854cf3d2a..643dcdb432 100644
--- a/superset-frontend/src/features/tags/BulkTagModal.tsx
+++ b/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'));
diff --git a/superset-frontend/src/pages/AllEntities/index.tsx b/superset-frontend/src/pages/AllEntities/index.tsx
index e131e7e6fb..ca815795d6 100644
--- a/superset-frontend/src/pages/AllEntities/index.tsx
+++ b/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
diff --git a/superset-frontend/src/pages/ChartList/ChartList.test.jsx b/superset-frontend/src/pages/ChartList/ChartList.test.jsx
index 6169c2a67e..bb3c5b79c1 100644
--- a/superset-frontend/src/pages/ChartList/ChartList.test.jsx
+++ b/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,6 +127,12 @@ describe('ChartList', () => {
   afterAll(() => {
     isFeatureEnabledMock.restore();
   });
+
+  beforeEach(() => {
+    // setup a DOM element as a render target
+    useSelectorMock.mockClear();
+  });
+
   const mockedProps = {};
 
   let wrapper;
@@ -114,9 +140,9 @@ describe('ChartList', () => {
   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>,
     );
 
diff --git a/superset-frontend/src/pages/ChartList/index.tsx b/superset-frontend/src/pages/ChartList/index.tsx
index 14ff62695f..d13113158e 100644
--- a/superset-frontend/src/pages/ChartList/index.tsx
+++ b/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',
diff --git a/superset-frontend/src/pages/DashboardList/DashboardList.test.jsx b/superset-frontend/src/pages/DashboardList/DashboardList.test.jsx
index 811f51afba..fe307ee791 100644
--- a/superset-frontend/src/pages/DashboardList/DashboardList.test.jsx
+++ b/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,6 +123,11 @@ describe('DashboardList', () => {
     isFeatureEnabledMock.restore();
   });
 
+  beforeEach(() => {
+    // setup a DOM element as a render target
+    useSelectorMock.mockClear();
+  });
+
   const mockedProps = {};
   let wrapper;
 
@@ -112,9 +135,9 @@ describe('DashboardList', () => {
     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>,
     );
 
diff --git a/superset-frontend/src/pages/DashboardList/index.tsx b/superset-frontend/src/pages/DashboardList/index.tsx
index 97898b3720..6542d85129 100644
--- a/superset-frontend/src/pages/DashboardList/index.tsx
+++ b/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',
diff --git a/superset-frontend/src/pages/SavedQueryList/SavedQueryList.test.jsx b/superset-frontend/src/pages/SavedQueryList/SavedQueryList.test.jsx
index 3804010e26..56fe3808d9 100644
--- a/superset-frontend/src/pages/SavedQueryList/SavedQueryList.test.jsx
+++ b/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);
   });
diff --git a/superset-frontend/src/pages/SavedQueryList/index.tsx b/superset-frontend/src/pages/SavedQueryList/index.tsx
index f9af490bb4..3ee62c2ce6 100644
--- a/superset-frontend/src/pages/SavedQueryList/index.tsx
+++ b/superset-frontend/src/pages/SavedQueryList/index.tsx
@@ -33,6 +33,7 @@ import {
   createFetchDistinct,
   createErrorHandler,
 } from 'src/views/CRUD/utils';
+import { useSelector } from 'react-redux';
 import Popover from 'src/components/Popover';
 import withToasts from 'src/components/MessageToasts/withToasts';
 import { useListViewResource } from 'src/views/CRUD/hooks';
@@ -55,8 +56,12 @@ import copyTextToClipboard from 'src/utils/copy';
 import Tag from 'src/types/TagType';
 import ImportModelsModal from 'src/components/ImportModal/index';
 import Icons from 'src/components/Icons';
-import { BootstrapUser } from 'src/types/bootstrapTypes';
+import {
+  BootstrapUser,
+  UserWithPermissionsAndRoles,
+} from 'src/types/bootstrapTypes';
 import SavedQueryPreviewModal from 'src/features/queries/SavedQueryPreviewModal';
+import { findPermission } from 'src/utils/findPermission';
 
 const PAGE_SIZE = 25;
 const PASSWORDS_NEEDED_MESSAGE = t(
@@ -111,6 +116,10 @@ function SavedQueryList({
     t('Saved queries'),
     addDangerToast,
   );
+  const { roles } = useSelector<any, UserWithPermissionsAndRoles>(
+    state => state.user,
+  );
+  const canReadTag = findPermission('can_read', 'Tag', roles);
   const [queryCurrentlyDeleting, setQueryCurrentlyDeleting] =
     useState<SavedQueryObject | null>(null);
   const [savedQueryCurrentlyPreviewing, setSavedQueryCurrentlyPreviewing] =
@@ -488,13 +497,7 @@ function SavedQueryList({
         ),
         paginate: true,
       },
-      {
-        Header: t('Tags'),
-        id: 'tags',
-        key: 'tags',
-        input: 'search',
-        operator: FilterOperator.savedQueryTags,
-      },
+
       {
         Header: t('Search'),
         id: 'label',
@@ -506,6 +509,16 @@ function SavedQueryList({
     [addDangerToast],
   );
 
+  if (isFeatureEnabled(FeatureFlag.TAGGING_SYSTEM) && canReadTag) {
+    filters.push({
+      Header: t('Tags'),
+      id: 'tags',
+      key: 'tags',
+      input: 'search',
+      operator: FilterOperator.savedQueryTags,
+    });
+  }
+
   return (
     <>
       <SubMenu {...menuData} />
diff --git a/superset/tags/api.py b/superset/tags/api.py
index 7224d1423a..e9842f5a6a 100644
--- a/superset/tags/api.py
+++ b/superset/tags/api.py
@@ -258,6 +258,8 @@ class TagRestApi(BaseSupersetModelRestApi):
         except ValidationError as error:
             return self.response_400(message=error.messages)
         try:
+            all_tagged_objects: set[tuple[str, int]] = set()
+            all_skipped_tagged_objects: set[tuple[str, int]] = set()
             for tag in item.get("tags"):
                 tagged_item: dict[str, Any] = self.add_model_schema.load(
                     {
@@ -265,10 +267,25 @@ class TagRestApi(BaseSupersetModelRestApi):
                         "objects_to_tag": tag.get("objects_to_tag"),
                     }
                 )
-                CreateCustomTagWithRelationshipsCommand(
+                (
+                    objects_tagged,
+                    objects_skipped,
+                ) = CreateCustomTagWithRelationshipsCommand(
                     tagged_item, bulk_create=True
                 ).run()
-            return self.response(201)
+                all_tagged_objects = all_tagged_objects | objects_tagged
+                all_skipped_tagged_objects = (
+                    all_skipped_tagged_objects | objects_skipped
+                )
+            return self.response(
+                200,
+                result={
+                    "objects_tagged": list(
+                        all_tagged_objects - all_skipped_tagged_objects
+                    ),
+                    "objects_skipped": list(all_skipped_tagged_objects),
+                },
+            )
         except TagNotFoundError:
             return self.response_404()
         except TagInvalidError as ex:
diff --git a/superset/tags/commands/create.py b/superset/tags/commands/create.py
index 883c498bc3..cd3bcc176b 100644
--- a/superset/tags/commands/create.py
+++ b/superset/tags/commands/create.py
@@ -69,8 +69,9 @@ class CreateCustomTagWithRelationshipsCommand(CreateMixin, BaseCommand):
     def __init__(self, data: dict[str, Any], bulk_create: bool = False):
         self._properties = data.copy()
         self._bulk_create = bulk_create
+        self._skipped_tagged_objects: set[tuple[str, int]] = set()
 
-    def run(self) -> None:
+    def run(self) -> tuple[set[tuple[str, int]], set[tuple[str, int]]]:
         self.validate()
 
         try:
@@ -86,6 +87,8 @@ class CreateCustomTagWithRelationshipsCommand(CreateMixin, BaseCommand):
 
             db.session.commit()
 
+            return set(self._properties["objects_to_tag"]), self._skipped_tagged_objects
+
         except DAOCreateFailedError as ex:
             logger.exception(ex.exception)
             raise TagCreateFailedError() from ex
@@ -93,20 +96,27 @@ class CreateCustomTagWithRelationshipsCommand(CreateMixin, BaseCommand):
     def validate(self) -> None:
         exceptions = []
         objects_to_tag = set(self._properties.get("objects_to_tag", []))
-        skipped_tagged_objects: set[tuple[str, int]] = set()
         for obj_type, obj_id in objects_to_tag:
             object_type = to_object_type(obj_type)
 
-            if not object_type:
-                exceptions.append(TagInvalidError(f"invalid object type {object_type}"))
-            try:
-                model = to_object_model(object_type, obj_id)  # type: ignore
-                security_manager.raise_for_ownership(model)
-            except SupersetSecurityException:
-                # skip the object if the user doesn't have access
-                skipped_tagged_objects.add((obj_type, obj_id))
-
-        self._properties["objects_to_tag"] = objects_to_tag - skipped_tagged_objects
+            # Validate object type
+            for obj_type, obj_id in objects_to_tag:
+                object_type = to_object_type(obj_type)
+
+                if not object_type:
+                    exceptions.append(
+                        TagInvalidError(f"invalid object type {object_type}")
+                    )
+                try:
+                    if model := to_object_model(object_type, obj_id):  # type: ignore
+                        security_manager.raise_for_ownership(model)
+                except SupersetSecurityException:
+                    # skip the object if the user doesn't have access
+                    self._skipped_tagged_objects.add((obj_type, obj_id))
+
+            self._properties["objects_to_tag"] = (
+                set(objects_to_tag) - self._skipped_tagged_objects
+            )
 
         if exceptions:
             raise TagInvalidError(exceptions=exceptions)
diff --git a/superset/tags/commands/update.py b/superset/tags/commands/update.py
index cc1c9a2be7..182376438b 100644
--- a/superset/tags/commands/update.py
+++ b/superset/tags/commands/update.py
@@ -38,14 +38,12 @@ class UpdateTagCommand(UpdateMixin, BaseCommand):
     def run(self) -> Model:
         self.validate()
         if self._model:
+            self._model.name = self._properties["name"]
             TagDAO.create_tag_relationship(
                 objects_to_tag=self._properties.get("objects_to_tag", []),
                 tag=self._model,
             )
-            if description := self._properties.get("description"):
-                self._model.description = description
-            if tag_name := self._properties.get("name"):
-                self._model.name = tag_name
+            self._model.description = self._properties.get("description")
 
             db.session.add(self._model)
             db.session.commit()
diff --git a/tests/integration_tests/tags/api_tests.py b/tests/integration_tests/tags/api_tests.py
index 444d52078e..e7f35c6b5d 100644
--- a/tests/integration_tests/tags/api_tests.py
+++ b/tests/integration_tests/tags/api_tests.py
@@ -550,7 +550,7 @@ class TestTagApi(SupersetTestCase):
             },
         )
 
-        self.assertEqual(rv.status_code, 201)
+        self.assertEqual(rv.status_code, 200)
 
         result = TagDAO.get_tagged_objects_for_tags(tags, ["dashboard"])
         assert len(result) == 1
@@ -569,3 +569,47 @@ class TestTagApi(SupersetTestCase):
             TaggedObject.object_type == ObjectTypes.chart,
         )
         assert tagged_objects.count() == 2
+
+    @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
+    def test_post_bulk_tag_skipped_tags_perm(self):
+        alpha = self.get_user("alpha")
+        self.insert_dashboard("titletag", "slugtag", [alpha.id])
+        self.login(username="alpha")
+        uri = "api/v1/tag/bulk_create"
+        dashboard = (
+            db.session.query(Dashboard)
+            .filter(Dashboard.dashboard_title == "World Bank's Data")
+            .first()
+        )
+        alpha_dash = (
+            db.session.query(Dashboard)
+            .filter(Dashboard.dashboard_title == "titletag")
+            .first()
+        )
+        chart = db.session.query(Slice).first()
+        rv = self.client.post(
+            uri,
+            json={
+                "tags": [
+                    {
+                        "name": "tag1",
+                        "objects_to_tag": [
+                            ["dashboard", alpha_dash.id],
+                        ],
+                    },
+                    {
+                        "name": "tag2",
+                        "objects_to_tag": [["dashboard", dashboard.id]],
+                    },
+                    {
+                        "name": "tag3",
+                        "objects_to_tag": [["chart", chart.id]],
+                    },
+                ]
+            },
+        )
+
+        self.assertEqual(rv.status_code, 200)
+        result = rv.json["result"]
+        assert len(result["objects_tagged"]) == 2
+        assert len(result["objects_skipped"]) == 1