fix: add validation on tag name to have name + onDelete refresh list …

…view (apache#25831) Co-authored-by: Elizabeth Thompson <eschutho@gmail.com>
CybercentreCanada · Nov 4, 2023 · 5d3df92 · 5d3df92
1 parent 98905c6
commit 5d3df92
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 22 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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):