fix(tagging): adding tags containing a “:” to dashboards (#26324)

will add more tests in a separated PR

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

@@ -353,7 +353,7 @@ function TagList(props: TagListProps) {
                 className="tags-list-view"
                 columns={columns}
                 count={tagCount}
-                data={tags.filter(tag => !tag.name.includes(':'))}
+                data={tags}
                 disableBulkSelect={toggleBulkSelect}
                 refreshData={refreshData}
                 emptyState={emptyState}

superset/daos/tag.py

@@ -24,7 +24,7 @@ from sqlalchemy.exc import SQLAlchemyError
 from superset.commands.tag.exceptions import TagNotFoundError
 from superset.commands.tag.utils import to_object_type
 from superset.daos.base import BaseDAO
-from superset.daos.exceptions import DAOCreateFailedError, DAODeleteFailedError
+from superset.daos.exceptions import DAODeleteFailedError
 from superset.exceptions import MissingUserContextException
 from superset.extensions import db
 from superset.models.dashboard import Dashboard
@@ -46,24 +46,12 @@ logger = logging.getLogger(__name__)
 class TagDAO(BaseDAO[Tag]):
     # base_filter = TagAccessFilter
 
-    @staticmethod
-    def validate_tag_name(tag_name: str) -> bool:
-        invalid_characters = [":", ","]
-        for invalid_character in invalid_characters:
-            if invalid_character in tag_name:
-                return False
-        return True
-
     @staticmethod
     def create_custom_tagged_objects(
         object_type: ObjectType, object_id: int, tag_names: list[str]
     ) -> None:
         tagged_objects = []
         for name in tag_names:
-            if not TagDAO.validate_tag_name(name):
-                raise DAOCreateFailedError(
-                    message="Invalid Tag Name (cannot contain ':' or ',')"
-                )
             type_ = TagType.custom
             tag_name = name.strip()
             tag = TagDAO.get_by_name(tag_name, type_)

superset/tags/models.py

@@ -19,6 +19,7 @@ from __future__ import annotations
 import enum
 from typing import TYPE_CHECKING
 
+from flask import escape
 from flask_appbuilder import Model
 from sqlalchemy import Column, Enum, ForeignKey, Integer, orm, String, Table, Text
 from sqlalchemy.engine.base import Connection
@@ -115,7 +116,7 @@ def get_tag(name: str, session: orm.Session, type_: TagType) -> Tag:
     tag_name = name.strip()
     tag = session.query(Tag).filter_by(name=tag_name, type=type_).one_or_none()
     if tag is None:
-        tag = Tag(name=tag_name, type=type_)
+        tag = Tag(name=escape(tag_name), type=type_)
         session.add(tag)
         session.commit()
     return tag

tests/integration_tests/tags/dao_tests.py

@@ -124,13 +124,19 @@ class TestTagsDAO(SupersetTestCase):
     @pytest.mark.usefixtures("with_tagging_system_feature")
     # test create tag
     def test_create_tagged_objects(self):
-        # test that a tag cannot be added if it has ':' in it
-        with pytest.raises(DAOCreateFailedError):
-            TagDAO.create_custom_tagged_objects(
-                object_type=ObjectType.dashboard.name,
-                object_id=1,
-                tag_names=["invalid:example tag 1"],
-            )
+        # test that a tag can be added if it has ':' in it
+        TagDAO.create_custom_tagged_objects(
+            object_type=ObjectType.dashboard.name,
+            object_id=1,
+            tag_names=["valid:example tag 1"],
+        )
+
+        # test that a tag can be added if it has ',' in it
+        TagDAO.create_custom_tagged_objects(
+            object_type=ObjectType.dashboard.name,
+            object_id=1,
+            tag_names=["example,tag,1"],
+        )
 
         # test that a tag can be added if it has a valid name
         TagDAO.create_custom_tagged_objects(
@@ -320,11 +326,3 @@ class TestTagsDAO(SupersetTestCase):
             .first()
         )
         assert tagged_object is None
-
-    @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
-    @pytest.mark.usefixtures("with_tagging_system_feature")
-    def test_validate_tag_name(self):
-        assert TagDAO.validate_tag_name("example_tag_name") is True
-        assert TagDAO.validate_tag_name("invalid:tag_name") is False
-        db.session.query(TaggedObject).delete()
-        db.session.query(Tag).delete()