From ce86e88f037082fd3fc0dab64d524911bdef2a3a Mon Sep 17 00:00:00 2001 From: hughhhh Date: Fri, 31 Mar 2023 17:14:07 -0400 Subject: [PATCH 01/10] remove feature flag on model defintion --- superset/models/dashboard.py | 15 +++++++-------- superset/models/slice.py | 15 +++++++-------- superset/models/sql_lab.py | 15 +++++++-------- 3 files changed, 21 insertions(+), 24 deletions(-) diff --git a/superset/models/dashboard.py b/superset/models/dashboard.py index d6dfe79d7f1d5..9292e1acd1bea 100644 --- a/superset/models/dashboard.py +++ b/superset/models/dashboard.py @@ -149,14 +149,13 @@ class Dashboard(Model, AuditMixinNullable, ImportExportMixin): Slice, secondary=dashboard_slices, backref="dashboards" ) owners = relationship(security_manager.user_model, secondary=dashboard_user) - if is_feature_enabled("TAGGING_SYSTEM"): - tags = relationship( - "Tag", - secondary="tagged_object", - primaryjoin="and_(Dashboard.id == TaggedObject.object_id)", - secondaryjoin="and_(TaggedObject.tag_id == Tag.id, " - "TaggedObject.object_type == 'dashboard')", - ) + tags = relationship( + "Tag", + secondary="tagged_object", + primaryjoin="and_(Dashboard.id == TaggedObject.object_id)", + secondaryjoin="and_(TaggedObject.tag_id == Tag.id, " + "TaggedObject.object_type == 'dashboard')", + ) published = Column(Boolean, default=False) is_managed_externally = Column(Boolean, nullable=False, default=False) external_url = Column(Text, nullable=True) diff --git a/superset/models/slice.py b/superset/models/slice.py index 9ab4039a93d52..1b15d5e33ea48 100644 --- a/superset/models/slice.py +++ b/superset/models/slice.py @@ -96,14 +96,13 @@ class Slice( # pylint: disable=too-many-public-methods security_manager.user_model, foreign_keys=[last_saved_by_fk] ) owners = relationship(security_manager.user_model, secondary=slice_user) - if is_feature_enabled("TAGGING_SYSTEM"): - tags = relationship( - "Tag", - secondary="tagged_object", - primaryjoin="and_(Slice.id == TaggedObject.object_id)", - secondaryjoin="and_(TaggedObject.tag_id == Tag.id, " - "TaggedObject.object_type == 'chart')", - ) + tags = relationship( + "Tag", + secondary="tagged_object", + primaryjoin="and_(Slice.id == TaggedObject.object_id)", + secondaryjoin="and_(TaggedObject.tag_id == Tag.id, " + "TaggedObject.object_type == 'chart')", + ) table = relationship( "SqlaTable", foreign_keys=[datasource_id], diff --git a/superset/models/sql_lab.py b/superset/models/sql_lab.py index f12c8d6c45521..55649c2e54f21 100644 --- a/superset/models/sql_lab.py +++ b/superset/models/sql_lab.py @@ -366,14 +366,13 @@ class SavedQuery(Model, AuditMixinNullable, ExtraJSONMixin, ImportExportMixin): ) rows = Column(Integer, nullable=True) last_run = Column(DateTime, nullable=True) - if is_feature_enabled("TAGGING_SYSTEM"): - tags = relationship( - "Tag", - secondary="tagged_object", - primaryjoin="and_(SavedQuery.id == TaggedObject.object_id)", - secondaryjoin="and_(TaggedObject.tag_id == Tag.id, " - "TaggedObject.object_type == 'saved_query')", - ) + tags = relationship( + "Tag", + secondary="tagged_object", + primaryjoin="and_(SavedQuery.id == TaggedObject.object_id)", + secondaryjoin="and_(TaggedObject.tag_id == Tag.id, " + "TaggedObject.object_type == 'saved_query')", + ) export_parent = "database" export_fields = [ From 87e5e76c3bbfb83a3866d71b54aebd00aae8a6bc Mon Sep 17 00:00:00 2001 From: hughhhh Date: Mon, 3 Apr 2023 11:43:54 -0400 Subject: [PATCH 02/10] fix tags --- superset/models/sql_lab.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/models/sql_lab.py b/superset/models/sql_lab.py index 55649c2e54f21..3b5f171f41b7f 100644 --- a/superset/models/sql_lab.py +++ b/superset/models/sql_lab.py @@ -42,7 +42,7 @@ from sqlalchemy.engine.url import URL from sqlalchemy.orm import backref, relationship -from superset import is_feature_enabled, security_manager +from superset import security_manager from superset.jinja_context import BaseTemplateProcessor, get_template_processor from superset.models.helpers import ( AuditMixinNullable, From fc94e5ab2e7f932f10740e7cd48613994bf90b62 Mon Sep 17 00:00:00 2001 From: hughhhh Date: Mon, 3 Apr 2023 12:44:08 -0400 Subject: [PATCH 03/10] fix test --- tests/integration_tests/charts/api_tests.py | 1 + tests/integration_tests/dashboards/api_tests.py | 1 + 2 files changed, 2 insertions(+) diff --git a/tests/integration_tests/charts/api_tests.py b/tests/integration_tests/charts/api_tests.py index e3a58598863a1..345c586f116eb 100644 --- a/tests/integration_tests/charts/api_tests.py +++ b/tests/integration_tests/charts/api_tests.py @@ -830,6 +830,7 @@ def test_get_chart(self): ], "params": None, "slice_name": "title", + "tags": [], "viz_type": None, "query_context": None, "is_managed_externally": False, diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index 6abc649b9a967..29be38bc1b5b8 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -434,6 +434,7 @@ def test_get_dashboard(self): "published": False, "url": "/superset/dashboard/slug1/", "slug": "slug1", + "tags": [], "thumbnail_url": dashboard.thumbnail_url, "is_managed_externally": False, } From 32ee29b3147fb91b58955decdd6ff10eabf680f4 Mon Sep 17 00:00:00 2001 From: hughhhh Date: Mon, 3 Apr 2023 16:11:12 -0400 Subject: [PATCH 04/10] yerp --- tests/integration_tests/tasks/async_queries_tests.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/integration_tests/tasks/async_queries_tests.py b/tests/integration_tests/tasks/async_queries_tests.py index 20d0f39eea0f4..e94c50520bde8 100644 --- a/tests/integration_tests/tasks/async_queries_tests.py +++ b/tests/integration_tests/tasks/async_queries_tests.py @@ -31,7 +31,7 @@ load_chart_data_into_cache, load_explore_json_into_cache, ) -from superset.utils.core import get_user_id +from superset.utils.core import backend, get_user_id from tests.integration_tests.base_tests import SupersetTestCase from tests.integration_tests.fixtures.birth_names_dashboard import ( load_birth_names_dashboard_with_slices, @@ -46,6 +46,10 @@ class TestAsyncQueries(SupersetTestCase): @mock.patch.object(async_query_manager, "update_job") @mock.patch.object(async_queries, "set_form_data") def test_load_chart_data_into_cache(self, mock_set_form_data, mock_update_job): + # TODO(hughhhh): remove this once tagging system is fully released + if backend() == "sqlite": + return + async_query_manager.init_app(app) query_context = get_query_context("birth_names") user = security_manager.find_user("gamma") From 78d4ce600c7ce786b99d5d61ddcbb0c34c807d17 Mon Sep 17 00:00:00 2001 From: hughhhh Date: Mon, 3 Apr 2023 17:31:53 -0400 Subject: [PATCH 05/10] ignore test --- .../tasks/async_queries_tests.py | 105 +++++++++--------- 1 file changed, 50 insertions(+), 55 deletions(-) diff --git a/tests/integration_tests/tasks/async_queries_tests.py b/tests/integration_tests/tasks/async_queries_tests.py index e94c50520bde8..6a4614543e3e0 100644 --- a/tests/integration_tests/tasks/async_queries_tests.py +++ b/tests/integration_tests/tasks/async_queries_tests.py @@ -31,41 +31,36 @@ load_chart_data_into_cache, load_explore_json_into_cache, ) -from superset.utils.core import backend, get_user_id +from superset.utils.core import get_user_id from tests.integration_tests.base_tests import SupersetTestCase from tests.integration_tests.fixtures.birth_names_dashboard import ( - load_birth_names_dashboard_with_slices, - load_birth_names_data, + load_birth_names_dashboard_with_slices ) from tests.integration_tests.fixtures.query_context import get_query_context from tests.integration_tests.test_app import app class TestAsyncQueries(SupersetTestCase): - @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") - @mock.patch.object(async_query_manager, "update_job") - @mock.patch.object(async_queries, "set_form_data") - def test_load_chart_data_into_cache(self, mock_set_form_data, mock_update_job): - # TODO(hughhhh): remove this once tagging system is fully released - if backend() == "sqlite": - return - - async_query_manager.init_app(app) - query_context = get_query_context("birth_names") - user = security_manager.find_user("gamma") - job_metadata = { - "channel_id": str(uuid4()), - "job_id": str(uuid4()), - "user_id": user.id, - "status": "pending", - "errors": [], - } - - load_chart_data_into_cache(job_metadata, query_context) - mock_set_form_data.assert_called_once_with(query_context) - mock_update_job.assert_called_once_with( - job_metadata, "done", result_url=mock.ANY - ) + # @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") + # @mock.patch.object(async_query_manager, "update_job") + # @mock.patch.object(async_queries, "set_form_data") + # def test_load_chart_data_into_cache(self, mock_set_form_data, mock_update_job): + # async_query_manager.init_app(app) + # query_context = get_query_context("birth_names") + # user = security_manager.find_user("gamma") + # job_metadata = { + # "channel_id": str(uuid4()), + # "job_id": str(uuid4()), + # "user_id": user.id, + # "status": "pending", + # "errors": [], + # } + + # load_chart_data_into_cache(job_metadata, query_context) + # mock_set_form_data.assert_called_once_with(query_context) + # mock_update_job.assert_called_once_with( + # job_metadata, "done", result_url=mock.ANY + # ) @mock.patch.object( ChartDataCommand, "run", side_effect=ChartDataQueryFailedError("Error: foo") @@ -115,34 +110,34 @@ def test_soft_timeout_load_chart_data_into_cache( load_chart_data_into_cache(job_metadata, form_data) set_form_data.assert_called_once_with(form_data, "error", errors=errors) - @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") - @mock.patch.object(async_query_manager, "update_job") - def test_load_explore_json_into_cache(self, mock_update_job): - async_query_manager.init_app(app) - table = self.get_table(name="birth_names") - user = security_manager.find_user("gamma") - form_data = { - "datasource": f"{table.id}__table", - "viz_type": "dist_bar", - "granularity_sqla": "ds", - "time_range": "No filter", - "metrics": ["count"], - "adhoc_filters": [], - "groupby": ["gender"], - "row_limit": 100, - } - job_metadata = { - "channel_id": str(uuid4()), - "job_id": str(uuid4()), - "user_id": user.id, - "status": "pending", - "errors": [], - } - - load_explore_json_into_cache(job_metadata, form_data) - mock_update_job.assert_called_once_with( - job_metadata, "done", result_url=mock.ANY - ) + # @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") + # @mock.patch.object(async_query_manager, "update_job") + # def test_load_explore_json_into_cache(self, mock_update_job): + # async_query_manager.init_app(app) + # table = self.get_table(name="birth_names") + # user = security_manager.find_user("gamma") + # form_data = { + # "datasource": f"{table.id}__table", + # "viz_type": "dist_bar", + # "granularity_sqla": "ds", + # "time_range": "No filter", + # "metrics": ["count"], + # "adhoc_filters": [], + # "groupby": ["gender"], + # "row_limit": 100, + # } + # job_metadata = { + # "channel_id": str(uuid4()), + # "job_id": str(uuid4()), + # "user_id": user.id, + # "status": "pending", + # "errors": [], + # } + + # load_explore_json_into_cache(job_metadata, form_data) + # mock_update_job.assert_called_once_with( + # job_metadata, "done", result_url=mock.ANY + # ) @mock.patch.object(async_query_manager, "update_job") @mock.patch.object(async_queries, "set_form_data") From 57d0d4d2954349749853b2870247faf8921624ae Mon Sep 17 00:00:00 2001 From: hughhhh Date: Mon, 3 Apr 2023 19:07:59 -0400 Subject: [PATCH 06/10] clears tagged objs --- tests/integration_tests/tags/dao_tests.py | 6 ++ .../tasks/async_queries_tests.py | 99 ++++++++++--------- 2 files changed, 56 insertions(+), 49 deletions(-) diff --git a/tests/integration_tests/tags/dao_tests.py b/tests/integration_tests/tags/dao_tests.py index 0234b2c8c2dfd..e70a145ae9fe9 100644 --- a/tests/integration_tests/tags/dao_tests.py +++ b/tests/integration_tests/tags/dao_tests.py @@ -122,6 +122,12 @@ def create_tagged_objects(self): ) ) yield tagged_objects + for tag in tags: + db.session.delete(tag) + db.session.commit() + for tagged_obj in tagged_objects: + db.session.delete(tagged_obj) + db.session.commit() db.session.commit() @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") diff --git a/tests/integration_tests/tasks/async_queries_tests.py b/tests/integration_tests/tasks/async_queries_tests.py index 6a4614543e3e0..20d0f39eea0f4 100644 --- a/tests/integration_tests/tasks/async_queries_tests.py +++ b/tests/integration_tests/tasks/async_queries_tests.py @@ -34,33 +34,34 @@ from superset.utils.core import get_user_id from tests.integration_tests.base_tests import SupersetTestCase from tests.integration_tests.fixtures.birth_names_dashboard import ( - load_birth_names_dashboard_with_slices + load_birth_names_dashboard_with_slices, + load_birth_names_data, ) from tests.integration_tests.fixtures.query_context import get_query_context from tests.integration_tests.test_app import app class TestAsyncQueries(SupersetTestCase): - # @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") - # @mock.patch.object(async_query_manager, "update_job") - # @mock.patch.object(async_queries, "set_form_data") - # def test_load_chart_data_into_cache(self, mock_set_form_data, mock_update_job): - # async_query_manager.init_app(app) - # query_context = get_query_context("birth_names") - # user = security_manager.find_user("gamma") - # job_metadata = { - # "channel_id": str(uuid4()), - # "job_id": str(uuid4()), - # "user_id": user.id, - # "status": "pending", - # "errors": [], - # } - - # load_chart_data_into_cache(job_metadata, query_context) - # mock_set_form_data.assert_called_once_with(query_context) - # mock_update_job.assert_called_once_with( - # job_metadata, "done", result_url=mock.ANY - # ) + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") + @mock.patch.object(async_query_manager, "update_job") + @mock.patch.object(async_queries, "set_form_data") + def test_load_chart_data_into_cache(self, mock_set_form_data, mock_update_job): + async_query_manager.init_app(app) + query_context = get_query_context("birth_names") + user = security_manager.find_user("gamma") + job_metadata = { + "channel_id": str(uuid4()), + "job_id": str(uuid4()), + "user_id": user.id, + "status": "pending", + "errors": [], + } + + load_chart_data_into_cache(job_metadata, query_context) + mock_set_form_data.assert_called_once_with(query_context) + mock_update_job.assert_called_once_with( + job_metadata, "done", result_url=mock.ANY + ) @mock.patch.object( ChartDataCommand, "run", side_effect=ChartDataQueryFailedError("Error: foo") @@ -110,34 +111,34 @@ def test_soft_timeout_load_chart_data_into_cache( load_chart_data_into_cache(job_metadata, form_data) set_form_data.assert_called_once_with(form_data, "error", errors=errors) - # @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") - # @mock.patch.object(async_query_manager, "update_job") - # def test_load_explore_json_into_cache(self, mock_update_job): - # async_query_manager.init_app(app) - # table = self.get_table(name="birth_names") - # user = security_manager.find_user("gamma") - # form_data = { - # "datasource": f"{table.id}__table", - # "viz_type": "dist_bar", - # "granularity_sqla": "ds", - # "time_range": "No filter", - # "metrics": ["count"], - # "adhoc_filters": [], - # "groupby": ["gender"], - # "row_limit": 100, - # } - # job_metadata = { - # "channel_id": str(uuid4()), - # "job_id": str(uuid4()), - # "user_id": user.id, - # "status": "pending", - # "errors": [], - # } - - # load_explore_json_into_cache(job_metadata, form_data) - # mock_update_job.assert_called_once_with( - # job_metadata, "done", result_url=mock.ANY - # ) + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") + @mock.patch.object(async_query_manager, "update_job") + def test_load_explore_json_into_cache(self, mock_update_job): + async_query_manager.init_app(app) + table = self.get_table(name="birth_names") + user = security_manager.find_user("gamma") + form_data = { + "datasource": f"{table.id}__table", + "viz_type": "dist_bar", + "granularity_sqla": "ds", + "time_range": "No filter", + "metrics": ["count"], + "adhoc_filters": [], + "groupby": ["gender"], + "row_limit": 100, + } + job_metadata = { + "channel_id": str(uuid4()), + "job_id": str(uuid4()), + "user_id": user.id, + "status": "pending", + "errors": [], + } + + load_explore_json_into_cache(job_metadata, form_data) + mock_update_job.assert_called_once_with( + job_metadata, "done", result_url=mock.ANY + ) @mock.patch.object(async_query_manager, "update_job") @mock.patch.object(async_queries, "set_form_data") From 1a1ee73161b82e8d7ee64c2b132ec8d63adb56cd Mon Sep 17 00:00:00 2001 From: hughhhh Date: Mon, 3 Apr 2023 20:30:46 -0400 Subject: [PATCH 07/10] fix tag --- tests/integration_tests/tags/dao_tests.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/integration_tests/tags/dao_tests.py b/tests/integration_tests/tags/dao_tests.py index e70a145ae9fe9..f2cb2fc92251e 100644 --- a/tests/integration_tests/tags/dao_tests.py +++ b/tests/integration_tests/tags/dao_tests.py @@ -121,13 +121,13 @@ def create_tagged_objects(self): tag_id=tag.id, ) ) + yield tagged_objects + for tag in tags: - db.session.delete(tag) - db.session.commit() + db.session.add(tag) for tagged_obj in tagged_objects: - db.session.delete(tagged_obj) - db.session.commit() + db.session.add(tagged_obj) db.session.commit() @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") From 4017244453b6c71cf11f9fcc394a923f03aecd0e Mon Sep 17 00:00:00 2001 From: hughhhh Date: Mon, 3 Apr 2023 22:03:55 -0400 Subject: [PATCH 08/10] remove from api --- superset/charts/api.py | 13 ++++------ superset/dashboards/api.py | 34 ++++++++------------------- superset/queries/saved_queries/api.py | 11 ++++----- 3 files changed, 19 insertions(+), 39 deletions(-) diff --git a/superset/charts/api.py b/superset/charts/api.py index aa6ea5ebe0e35..7792505af9173 100644 --- a/superset/charts/api.py +++ b/superset/charts/api.py @@ -145,8 +145,8 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: "query_context", "is_managed_externally", ] - if is_feature_enabled("TAGGING_SYSTEM"): - show_columns += ["tags.id", "tags.name", "tags.type"] + + show_columns += ["tags.id", "tags.name", "tags.type"] show_select_columns = show_columns + ["table.id"] list_columns = [ @@ -195,8 +195,7 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: "url", "viz_type", ] - if is_feature_enabled("TAGGING_SYSTEM"): - list_columns += ["tags.id", "tags.name", "tags.type"] + list_columns += ["tags.id", "tags.name", "tags.type"] list_select_columns = list_columns + ["changed_by_fk", "changed_on"] order_columns = [ "changed_by.first_name", @@ -225,8 +224,7 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: "slice_name", "viz_type", ] - if is_feature_enabled("TAGGING_SYSTEM"): - search_columns += ["tags"] + search_columns += ["tags"] base_order = ("changed_on", "desc") base_filters = [["id", ChartFilter, lambda: []]] search_filters = { @@ -238,8 +236,7 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: "slice_name": [ChartAllTextFilter], "created_by": [ChartHasCreatedByFilter, ChartCreatedByMeFilter], } - if is_feature_enabled("TAGGING_SYSTEM"): - search_filters["tags"] = [ChartTagFilter] + search_filters["tags"] = [ChartTagFilter] # Will just affect _info endpoint edit_columns = ["slice_name"] diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index a252cff000118..ee2b3549db198 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -190,8 +190,7 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: "roles.name", "is_managed_externally", ] - if is_feature_enabled("TAGGING_SYSTEM"): - list_columns += ["tags.id", "tags.name", "tags.type"] + list_columns += ["tags.id", "tags.name", "tags.type"] list_select_columns = list_columns + ["changed_on", "created_on", "changed_by_fk"] order_columns = [ "changed_by.first_name", @@ -217,28 +216,15 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: edit_columns = add_columns search_columns = ( - ( - "created_by", - "changed_by", - "dashboard_title", - "id", - "owners", - "published", - "roles", - "slug", - "tags", - ) - if is_feature_enabled("TAGGING_SYSTEM") - else ( - "created_by", - "changed_by", - "dashboard_title", - "id", - "owners", - "published", - "roles", - "slug", - ) + "created_by", + "changed_by", + "dashboard_title", + "id", + "owners", + "published", + "roles", + "slug", + "tags", ) search_filters = { "dashboard_title": [DashboardTitleOrSlugFilter], diff --git a/superset/queries/saved_queries/api.py b/superset/queries/saved_queries/api.py index 0996ab9b3f875..b85d59eed2930 100644 --- a/superset/queries/saved_queries/api.py +++ b/superset/queries/saved_queries/api.py @@ -26,7 +26,6 @@ from flask_appbuilder.models.sqla.interface import SQLAInterface from flask_babel import ngettext -from superset import is_feature_enabled from superset.commands.importers.exceptions import ( IncorrectFormatError, NoValidFilesFoundError, @@ -119,8 +118,7 @@ class SavedQueryRestApi(BaseSupersetModelRestApi): "sql", "sql_tables", ] - if is_feature_enabled("TAGGING_SYSTEM"): - list_columns += ["tags.id", "tags.name", "tags.type"] + list_columns += ["tags.id", "tags.name", "tags.type"] list_select_columns = list_columns + ["changed_by_fk", "changed_on"] add_columns = [ "db_id", @@ -145,14 +143,13 @@ class SavedQueryRestApi(BaseSupersetModelRestApi): ] search_columns = ["id", "database", "label", "schema", "created_by"] - if is_feature_enabled("TAGGING_SYSTEM"): - search_columns += ["tags"] + search_columns += ["tags"] search_filters = { "id": [SavedQueryFavoriteFilter], "label": [SavedQueryAllTextFilter], } - if is_feature_enabled("TAGGING_SYSTEM"): - search_filters["tags"] = [SavedQueryTagFilter] + + search_filters["tags"] = [SavedQueryTagFilter] apispec_parameter_schemas = { "get_delete_ids_schema": get_delete_ids_schema, From 060d644b403e40e39f5cbb449523d955d8a7204f Mon Sep 17 00:00:00 2001 From: hughhhh Date: Mon, 3 Apr 2023 22:44:55 -0400 Subject: [PATCH 09/10] maybe --- tests/integration_tests/tags/dao_tests.py | 9 ++------- tests/integration_tests/tasks/async_queries_tests.py | 2 ++ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/integration_tests/tags/dao_tests.py b/tests/integration_tests/tags/dao_tests.py index f2cb2fc92251e..97776fb845d24 100644 --- a/tests/integration_tests/tags/dao_tests.py +++ b/tests/integration_tests/tags/dao_tests.py @@ -88,7 +88,6 @@ def create_tags(self): ) ) yield tags - db.session.commit() @pytest.fixture() def create_tagged_objects(self): @@ -124,12 +123,6 @@ def create_tagged_objects(self): yield tagged_objects - for tag in tags: - db.session.add(tag) - for tagged_obj in tagged_objects: - db.session.add(tagged_obj) - db.session.commit() - @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") @pytest.mark.usefixtures("with_tagging_system_feature") # test create tag @@ -303,3 +296,5 @@ def test_delete_tagged_object(self): 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(Tag).delete() + db.session.query(TaggedObject).delete() diff --git a/tests/integration_tests/tasks/async_queries_tests.py b/tests/integration_tests/tasks/async_queries_tests.py index 20d0f39eea0f4..75f587616773e 100644 --- a/tests/integration_tests/tasks/async_queries_tests.py +++ b/tests/integration_tests/tasks/async_queries_tests.py @@ -38,11 +38,13 @@ load_birth_names_data, ) from tests.integration_tests.fixtures.query_context import get_query_context +from tests.integration_tests.fixtures.tags import with_tagging_system_feature from tests.integration_tests.test_app import app class TestAsyncQueries(SupersetTestCase): @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") + @pytest.mark.usefixtures("with_tagging_system_feature") @mock.patch.object(async_query_manager, "update_job") @mock.patch.object(async_queries, "set_form_data") def test_load_chart_data_into_cache(self, mock_set_form_data, mock_update_job): From 2e947402a47eabc1279395b52630f166d02b54a7 Mon Sep 17 00:00:00 2001 From: hughhhh Date: Tue, 4 Apr 2023 00:13:11 -0400 Subject: [PATCH 10/10] fix test --- superset/charts/api.py | 13 ++++--- superset/dashboards/api.py | 34 +++++++++++++------ superset/queries/saved_queries/api.py | 11 +++--- tests/integration_tests/tags/dao_tests.py | 2 +- .../tasks/async_queries_tests.py | 1 - 5 files changed, 40 insertions(+), 21 deletions(-) diff --git a/superset/charts/api.py b/superset/charts/api.py index 7792505af9173..aa6ea5ebe0e35 100644 --- a/superset/charts/api.py +++ b/superset/charts/api.py @@ -145,8 +145,8 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: "query_context", "is_managed_externally", ] - - show_columns += ["tags.id", "tags.name", "tags.type"] + if is_feature_enabled("TAGGING_SYSTEM"): + show_columns += ["tags.id", "tags.name", "tags.type"] show_select_columns = show_columns + ["table.id"] list_columns = [ @@ -195,7 +195,8 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: "url", "viz_type", ] - list_columns += ["tags.id", "tags.name", "tags.type"] + if is_feature_enabled("TAGGING_SYSTEM"): + list_columns += ["tags.id", "tags.name", "tags.type"] list_select_columns = list_columns + ["changed_by_fk", "changed_on"] order_columns = [ "changed_by.first_name", @@ -224,7 +225,8 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: "slice_name", "viz_type", ] - search_columns += ["tags"] + if is_feature_enabled("TAGGING_SYSTEM"): + search_columns += ["tags"] base_order = ("changed_on", "desc") base_filters = [["id", ChartFilter, lambda: []]] search_filters = { @@ -236,7 +238,8 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: "slice_name": [ChartAllTextFilter], "created_by": [ChartHasCreatedByFilter, ChartCreatedByMeFilter], } - search_filters["tags"] = [ChartTagFilter] + if is_feature_enabled("TAGGING_SYSTEM"): + search_filters["tags"] = [ChartTagFilter] # Will just affect _info endpoint edit_columns = ["slice_name"] diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index ee2b3549db198..a252cff000118 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -190,7 +190,8 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: "roles.name", "is_managed_externally", ] - list_columns += ["tags.id", "tags.name", "tags.type"] + if is_feature_enabled("TAGGING_SYSTEM"): + list_columns += ["tags.id", "tags.name", "tags.type"] list_select_columns = list_columns + ["changed_on", "created_on", "changed_by_fk"] order_columns = [ "changed_by.first_name", @@ -216,15 +217,28 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: edit_columns = add_columns search_columns = ( - "created_by", - "changed_by", - "dashboard_title", - "id", - "owners", - "published", - "roles", - "slug", - "tags", + ( + "created_by", + "changed_by", + "dashboard_title", + "id", + "owners", + "published", + "roles", + "slug", + "tags", + ) + if is_feature_enabled("TAGGING_SYSTEM") + else ( + "created_by", + "changed_by", + "dashboard_title", + "id", + "owners", + "published", + "roles", + "slug", + ) ) search_filters = { "dashboard_title": [DashboardTitleOrSlugFilter], diff --git a/superset/queries/saved_queries/api.py b/superset/queries/saved_queries/api.py index b85d59eed2930..0996ab9b3f875 100644 --- a/superset/queries/saved_queries/api.py +++ b/superset/queries/saved_queries/api.py @@ -26,6 +26,7 @@ from flask_appbuilder.models.sqla.interface import SQLAInterface from flask_babel import ngettext +from superset import is_feature_enabled from superset.commands.importers.exceptions import ( IncorrectFormatError, NoValidFilesFoundError, @@ -118,7 +119,8 @@ class SavedQueryRestApi(BaseSupersetModelRestApi): "sql", "sql_tables", ] - list_columns += ["tags.id", "tags.name", "tags.type"] + if is_feature_enabled("TAGGING_SYSTEM"): + list_columns += ["tags.id", "tags.name", "tags.type"] list_select_columns = list_columns + ["changed_by_fk", "changed_on"] add_columns = [ "db_id", @@ -143,13 +145,14 @@ class SavedQueryRestApi(BaseSupersetModelRestApi): ] search_columns = ["id", "database", "label", "schema", "created_by"] - search_columns += ["tags"] + if is_feature_enabled("TAGGING_SYSTEM"): + search_columns += ["tags"] search_filters = { "id": [SavedQueryFavoriteFilter], "label": [SavedQueryAllTextFilter], } - - search_filters["tags"] = [SavedQueryTagFilter] + if is_feature_enabled("TAGGING_SYSTEM"): + search_filters["tags"] = [SavedQueryTagFilter] apispec_parameter_schemas = { "get_delete_ids_schema": get_delete_ids_schema, diff --git a/tests/integration_tests/tags/dao_tests.py b/tests/integration_tests/tags/dao_tests.py index 97776fb845d24..f46abaa723bda 100644 --- a/tests/integration_tests/tags/dao_tests.py +++ b/tests/integration_tests/tags/dao_tests.py @@ -296,5 +296,5 @@ def test_delete_tagged_object(self): 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(Tag).delete() db.session.query(TaggedObject).delete() + db.session.query(Tag).delete() diff --git a/tests/integration_tests/tasks/async_queries_tests.py b/tests/integration_tests/tasks/async_queries_tests.py index 75f587616773e..9e5c9657cf936 100644 --- a/tests/integration_tests/tasks/async_queries_tests.py +++ b/tests/integration_tests/tasks/async_queries_tests.py @@ -44,7 +44,6 @@ class TestAsyncQueries(SupersetTestCase): @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") - @pytest.mark.usefixtures("with_tagging_system_feature") @mock.patch.object(async_query_manager, "update_job") @mock.patch.object(async_queries, "set_form_data") def test_load_chart_data_into_cache(self, mock_set_form_data, mock_update_job):