From 64057bf1936f4d5e9299e91a8bf822752e29fc2d Mon Sep 17 00:00:00 2001 From: Lily Kuang Date: Fri, 22 Dec 2023 10:30:08 -0800 Subject: [PATCH] =?UTF-8?q?fix(tagging):=20adding=20tags=20containing=20a?= =?UTF-8?q?=20=E2=80=9C:=E2=80=9D=20to=20dashboards=20(#26324)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit will add more tests in a separated PR (cherry picked from commit 3391e290934b61e4a508ddee36ca002bee7e4c64) --- 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 d395ce7cde25a..b0c998c3f32f8 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 60362bfbbd3bd..e4aa89181644d 100644 --- a/superset/daos/tag.py +++ b/superset/daos/tag.py @@ -24,7 +24,7 @@ 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 @@ 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 7a77677a367ad..bae4417507bd4 100644 --- a/superset/tags/models.py +++ b/superset/tags/models.py @@ -19,6 +19,7 @@ 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 272ba43ed324a..38bb4d0904d74 100644 --- a/tests/integration_tests/tags/dao_tests.py +++ b/tests/integration_tests/tags/dao_tests.py @@ -124,13 +124,19 @@ def create_tagged_objects(self): @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 @@ def test_delete_tagged_object(self): .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()