From 80cf710dbec6f1f2d3ddff8f1f391a155417f1b2 Mon Sep 17 00:00:00 2001
From: "Hugh A. Miles II" <hughmil3s@gmail.com>
Date: Sat, 4 Nov 2023 10:09:54 -0400
Subject: [PATCH] fix: add validation on tag name to have name + onDelete
 refresh list view (#25831)

Co-authored-by: Elizabeth Thompson <eschutho@gmail.com>
---
 .../src/features/tags/TagModal.tsx            | 22 ++++++++++------
 superset-frontend/src/pages/Tags/index.tsx    | 26 ++++++++++---------
 superset/tags/schemas.py                      |  4 +--
 tests/integration_tests/tags/api_tests.py     | 16 ++++++++++++
 4 files changed, 46 insertions(+), 22 deletions(-)

diff --git a/superset-frontend/src/features/tags/TagModal.tsx b/superset-frontend/src/features/tags/TagModal.tsx
index fde448d4b48fd..4339d69130792 100644
--- a/superset-frontend/src/features/tags/TagModal.tsx
+++ b/superset-frontend/src/features/tags/TagModal.tsx
@@ -222,10 +222,14 @@ const TagModal: React.FC<TagModalProps> = ({
           name: tagName,
           objects_to_tag: [...dashboards, ...charts, ...savedQueries],
         },
-      }).then(({ json = {} }) => {
-        refreshData();
-        addSuccessToast(t('Tag updated'));
-      });
+      })
+        .then(({ json = {} }) => {
+          refreshData();
+          addSuccessToast(t('Tag updated'));
+        })
+        .catch(err => {
+          addDangerToast(err.message || 'Error Updating Tag');
+        });
     } else {
       SupersetClient.post({
         endpoint: `/api/v1/tag/`,
@@ -234,10 +238,12 @@ const TagModal: React.FC<TagModalProps> = ({
           name: tagName,
           objects_to_tag: [...dashboards, ...charts, ...savedQueries],
         },
-      }).then(({ json = {} }) => {
-        refreshData();
-        addSuccessToast(t('Tag created'));
-      });
+      })
+        .then(({ json = {} }) => {
+          refreshData();
+          addSuccessToast(t('Tag created'));
+        })
+        .catch(err => addDangerToast(err.message || 'Error Creating Tag'));
     }
     onHide();
   };
diff --git a/superset-frontend/src/pages/Tags/index.tsx b/superset-frontend/src/pages/Tags/index.tsx
index e252e3d9f98c2..a66d7c7b61b0d 100644
--- a/superset-frontend/src/pages/Tags/index.tsx
+++ b/superset-frontend/src/pages/Tags/index.tsx
@@ -92,14 +92,18 @@ function TagList(props: TagListProps) {
 
   const initialSort = [{ id: 'changed_on_delta_humanized', desc: true }];
 
-  function handleTagsDelete(
-    tags: Tag[],
-    callback: (text: string) => void,
-    error: (text: string) => void,
-  ) {
-    // TODO what permissions need to be checked here?
-    deleteTags(tags, callback, error);
-    refreshData();
+  function handleTagsDelete(tags: Tag[]) {
+    deleteTags(
+      tags,
+      (msg: string) => {
+        addSuccessToast(msg);
+        refreshData();
+      },
+      msg => {
+        addDangerToast(msg);
+        refreshData();
+      },
+    );
   }
 
   const handleTagEdit = (tag: Tag) => {
@@ -178,8 +182,6 @@ function TagList(props: TagListProps) {
       },
       {
         Cell: ({ row: { original } }: any) => {
-          const handleDelete = () =>
-            handleTagsDelete([original], addSuccessToast, addDangerToast);
           const handleEdit = () => handleTagEdit(original);
           return (
             <Actions className="actions">
@@ -192,7 +194,7 @@ function TagList(props: TagListProps) {
                       <b>{original.dashboard_title}</b>?
                     </>
                   }
-                  onConfirm={handleDelete}
+                  onConfirm={() => handleTagsDelete([original])}
                 >
                   {confirmDelete => (
                     <Tooltip
@@ -318,7 +320,7 @@ function TagList(props: TagListProps) {
   });
 
   const handleBulkDelete = (tagsToDelete: Tag[]) =>
-    handleTagsDelete(tagsToDelete, addSuccessToast, addDangerToast);
+    handleTagsDelete(tagsToDelete);
 
   return (
     <>
diff --git a/superset/tags/schemas.py b/superset/tags/schemas.py
index a391fd2b8055a..38676b42949e4 100644
--- a/superset/tags/schemas.py
+++ b/superset/tags/schemas.py
@@ -15,7 +15,7 @@
 # specific language governing permissions and limitations
 # under the License.
 from marshmallow import fields, Schema
-from marshmallow.validate import Range
+from marshmallow.validate import Length, Range
 
 from superset.dashboards.schemas import UserSchema
 
@@ -58,7 +58,7 @@ class TaggedObjectEntityResponseSchema(Schema):
 
 
 class TagObjectSchema(Schema):
-    name = fields.String()
+    name = fields.String(validate=Length(min=1))
     description = fields.String(required=False, allow_none=True)
     objects_to_tag = fields.List(
         fields.Tuple((fields.String(), fields.Int(validate=Range(min=1)))),
diff --git a/tests/integration_tests/tags/api_tests.py b/tests/integration_tests/tags/api_tests.py
index e7f35c6b5d55b..33fa4902b26ca 100644
--- a/tests/integration_tests/tags/api_tests.py
+++ b/tests/integration_tests/tags/api_tests.py
@@ -485,6 +485,22 @@ def test_post_tag(self):
         )
         assert tag is not None
 
+    @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
+    def test_post_tag_no_name_400(self):
+        self.login(username="admin")
+        uri = f"api/v1/tag/"
+        dashboard = (
+            db.session.query(Dashboard)
+            .filter(Dashboard.dashboard_title == "World Bank's Data")
+            .first()
+        )
+        rv = self.client.post(
+            uri,
+            json={"name": "", "objects_to_tag": [["dashboard", dashboard.id]]},
+        )
+
+        self.assertEqual(rv.status_code, 400)
+
     @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
     @pytest.mark.usefixtures("create_tags")
     def test_put_tag(self):