You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by li...@apache.org on 2023/12/22 18:30:17 UTC
(superset) branch master updated: fix(tagging): adding tags containing a “:” to dashboards (#26324)
This is an automated email from the ASF dual-hosted git repository.
lilykuang 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 3391e29093 fix(tagging): adding tags containing a “:” to dashboards (#26324)
3391e29093 is described below
commit 3391e290934b61e4a508ddee36ca002bee7e4c64
Author: Lily Kuang <li...@preset.io>
AuthorDate: Fri Dec 22 10:30:08 2023 -0800
fix(tagging): adding tags containing a “:” to dashboards (#26324)
will add more tests in a separated PR
---
superset-frontend/src/pages/Tags/index.tsx | 2 +-
superset/daos/tag.py | 14 +-------------
superset/tags/models.py | 3 ++-
tests/integration_tests/tags/dao_tests.py | 28 +++++++++++++---------------
4 files changed, 17 insertions(+), 30 deletions(-)
diff --git a/superset-frontend/src/pages/Tags/index.tsx b/superset-frontend/src/pages/Tags/index.tsx
index d395ce7cde..b0c998c3f3 100644
--- a/superset-frontend/src/pages/Tags/index.tsx
+++ b/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}
diff --git a/superset/daos/tag.py b/superset/daos/tag.py
index 60362bfbbd..e4aa891816 100644
--- a/superset/daos/tag.py
+++ b/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_)
diff --git a/superset/tags/models.py b/superset/tags/models.py
index 7a77677a36..bae4417507 100644
--- a/superset/tags/models.py
+++ b/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
diff --git a/tests/integration_tests/tags/dao_tests.py b/tests/integration_tests/tags/dao_tests.py
index 272ba43ed3..38bb4d0904 100644
--- a/tests/integration_tests/tags/dao_tests.py
+++ b/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()