villebro commented on a change in pull request #14015: URL: https://github.com/apache/superset/pull/14015#discussion_r709322817
########## File path: tests/integration_tests/dashboards/filter_sets/conftest.py ########## @@ -0,0 +1,322 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import json +from typing import Any, Dict, Generator, List, TYPE_CHECKING + +import pytest + +from superset import security_manager as sm +from superset.dashboards.filter_sets.consts import ( + DESCRIPTION_FIELD, + JSON_METADATA_FIELD, + NAME_FIELD, + OWNER_ID_FIELD, + OWNER_TYPE_FIELD, + USER_OWNER_TYPE, +) +from superset.models.dashboard import Dashboard +from superset.models.filter_set import FilterSet +from tests.integration_tests.dashboards.filter_sets.consts import ( + ADMIN_USERNAME_FOR_TEST, + DASHBOARD_OWNER_USERNAME, + FILTER_SET_OWNER_USERNAME, + REGULAR_USER, +) +from tests.integration_tests.dashboards.superset_factory_util import ( + create_dashboard, + create_database, + create_datasource_table, + create_slice, +) +from tests.integration_tests.test_app import app + +if TYPE_CHECKING: + from flask.ctx import AppContext + from flask.testing import FlaskClient + from flask_appbuilder.security.sqla.models import ( + Role, + User, + ViewMenu, + PermissionView, + ) + from flask_appbuilder.security.manager import BaseSecurityManager + from sqlalchemy.orm import Session + from superset.models.slice import Slice + from superset.connectors.sqla.models import SqlaTable + from superset.models.core import Database + + +security_manager: BaseSecurityManager = sm + + +# @pytest.fixture(autouse=True, scope="session") +# def setup_sample_data() -> Any: +# pass + + [email protected](autouse=True) +def expire_on_commit_true() -> Generator[None, None, None]: + ctx: AppContext + with app.app_context() as ctx: + ctx.app.appbuilder.get_session.configure(expire_on_commit=False) + yield + ctx.app.appbuilder.get_session.configure(expire_on_commit=True) + + [email protected](autouse=True, scope="module") +def test_users() -> Generator[Dict[str, int], None, None]: + usernames = [ + ADMIN_USERNAME_FOR_TEST, + DASHBOARD_OWNER_USERNAME, + FILTER_SET_OWNER_USERNAME, + REGULAR_USER, + ] + with app.app_context(): + filter_set_role = build_filter_set_role() + admin_role: Role = security_manager.find_role("Admin") + usernames_to_ids = create_test_users(admin_role, filter_set_role, usernames) + yield usernames_to_ids + ctx: AppContext + delete_users(usernames_to_ids) + + +def delete_users(usernames_to_ids: Dict[str, int]) -> None: + with app.app_context() as ctx: + session: Session = ctx.app.appbuilder.get_session + for username in usernames_to_ids.keys(): + session.delete(security_manager.find_user(username)) + session.commit() + + +def create_test_users( + admin_role: Role, filter_set_role: Role, usernames: List[str] +) -> Dict[str, int]: + users: List[User] = [] + for username in usernames: + user = build_user(username, filter_set_role, admin_role) + users.append(user) + return {user.username: user.id for user in users} + + +def build_user(username: str, filter_set_role: Role, admin_role: Role) -> User: + roles_to_add = ( + [admin_role] if username == ADMIN_USERNAME_FOR_TEST else [filter_set_role] + ) + user: User = security_manager.add_user( + username, "test", "test", username, roles_to_add, password="general" + ) + if not user: + user = security_manager.find_user(username) + if user is None: + raise Exception("Failed to build the user {}".format(username)) + return user + + +def build_filter_set_role() -> Role: + filter_set_role: Role = security_manager.add_role("filter_set_role") + filterset_view_name: ViewMenu = security_manager.find_view_menu("FilterSets") + all_datasource_view_name: ViewMenu = security_manager.find_view_menu( + "all_datasource_access" + ) + pvms: List[PermissionView] = security_manager.find_permissions_view_menu( + filterset_view_name + ) + security_manager.find_permissions_view_menu(all_datasource_view_name) + for pvm in pvms: + security_manager.add_permission_role(filter_set_role, pvm) + return filter_set_role + + [email protected] +def client() -> Generator[FlaskClient[Any], None, None]: + with app.test_client() as client: + yield client + + [email protected] +def dashboard() -> Generator[Dashboard, None, None]: + dashboard: Dashboard + slice_: Slice + datasource: SqlaTable + database: Database + session: Session + try: + with app.app_context() as ctx: + dashboard_owner_user = security_manager.find_user(DASHBOARD_OWNER_USERNAME) + database = create_database("test_database_filter_sets") + datasource = create_datasource_table( + name="test_datasource", database=database, owners=[dashboard_owner_user] + ) + slice_ = create_slice( + datasource=datasource, name="test_slice", owners=[dashboard_owner_user] + ) + dashboard = create_dashboard( + dashboard_title="test_dashboard", + published=True, + slices=[slice_], + owners=[dashboard_owner_user], + ) + session = ctx.app.appbuilder.get_session + session.add(dashboard) + session.commit() + yield dashboard + except Exception as ex: + print(str(ex)) + yield Dashboard.get(dashboard.slug) Review comment: I don't understand this logic - is this to work around flakiness? I'd prefer for this step to fail if there's an exception (also I don't usually like catching the broad `Exception` type) ########## File path: tests/integration_tests/dashboards/filter_sets/conftest.py ########## @@ -0,0 +1,322 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import json +from typing import Any, Dict, Generator, List, TYPE_CHECKING + +import pytest + +from superset import security_manager as sm +from superset.dashboards.filter_sets.consts import ( + DESCRIPTION_FIELD, + JSON_METADATA_FIELD, + NAME_FIELD, + OWNER_ID_FIELD, + OWNER_TYPE_FIELD, + USER_OWNER_TYPE, +) +from superset.models.dashboard import Dashboard +from superset.models.filter_set import FilterSet +from tests.integration_tests.dashboards.filter_sets.consts import ( + ADMIN_USERNAME_FOR_TEST, + DASHBOARD_OWNER_USERNAME, + FILTER_SET_OWNER_USERNAME, + REGULAR_USER, +) +from tests.integration_tests.dashboards.superset_factory_util import ( + create_dashboard, + create_database, + create_datasource_table, + create_slice, +) +from tests.integration_tests.test_app import app + +if TYPE_CHECKING: + from flask.ctx import AppContext + from flask.testing import FlaskClient + from flask_appbuilder.security.sqla.models import ( + Role, + User, + ViewMenu, + PermissionView, + ) + from flask_appbuilder.security.manager import BaseSecurityManager + from sqlalchemy.orm import Session + from superset.models.slice import Slice + from superset.connectors.sqla.models import SqlaTable + from superset.models.core import Database + + +security_manager: BaseSecurityManager = sm + + +# @pytest.fixture(autouse=True, scope="session") +# def setup_sample_data() -> Any: +# pass + + [email protected](autouse=True) +def expire_on_commit_true() -> Generator[None, None, None]: + ctx: AppContext + with app.app_context() as ctx: + ctx.app.appbuilder.get_session.configure(expire_on_commit=False) + yield + ctx.app.appbuilder.get_session.configure(expire_on_commit=True) + + [email protected](autouse=True, scope="module") +def test_users() -> Generator[Dict[str, int], None, None]: + usernames = [ + ADMIN_USERNAME_FOR_TEST, + DASHBOARD_OWNER_USERNAME, + FILTER_SET_OWNER_USERNAME, + REGULAR_USER, + ] + with app.app_context(): + filter_set_role = build_filter_set_role() + admin_role: Role = security_manager.find_role("Admin") + usernames_to_ids = create_test_users(admin_role, filter_set_role, usernames) + yield usernames_to_ids + ctx: AppContext + delete_users(usernames_to_ids) + + +def delete_users(usernames_to_ids: Dict[str, int]) -> None: + with app.app_context() as ctx: + session: Session = ctx.app.appbuilder.get_session + for username in usernames_to_ids.keys(): + session.delete(security_manager.find_user(username)) + session.commit() + + +def create_test_users( + admin_role: Role, filter_set_role: Role, usernames: List[str] +) -> Dict[str, int]: + users: List[User] = [] + for username in usernames: + user = build_user(username, filter_set_role, admin_role) + users.append(user) + return {user.username: user.id for user in users} + + +def build_user(username: str, filter_set_role: Role, admin_role: Role) -> User: + roles_to_add = ( + [admin_role] if username == ADMIN_USERNAME_FOR_TEST else [filter_set_role] + ) + user: User = security_manager.add_user( + username, "test", "test", username, roles_to_add, password="general" + ) + if not user: + user = security_manager.find_user(username) + if user is None: + raise Exception("Failed to build the user {}".format(username)) + return user + + +def build_filter_set_role() -> Role: + filter_set_role: Role = security_manager.add_role("filter_set_role") + filterset_view_name: ViewMenu = security_manager.find_view_menu("FilterSets") + all_datasource_view_name: ViewMenu = security_manager.find_view_menu( + "all_datasource_access" + ) + pvms: List[PermissionView] = security_manager.find_permissions_view_menu( + filterset_view_name + ) + security_manager.find_permissions_view_menu(all_datasource_view_name) + for pvm in pvms: + security_manager.add_permission_role(filter_set_role, pvm) + return filter_set_role + + [email protected] +def client() -> Generator[FlaskClient[Any], None, None]: + with app.test_client() as client: + yield client + + [email protected] +def dashboard() -> Generator[Dashboard, None, None]: + dashboard: Dashboard + slice_: Slice + datasource: SqlaTable + database: Database + session: Session + try: + with app.app_context() as ctx: + dashboard_owner_user = security_manager.find_user(DASHBOARD_OWNER_USERNAME) + database = create_database("test_database_filter_sets") + datasource = create_datasource_table( + name="test_datasource", database=database, owners=[dashboard_owner_user] + ) + slice_ = create_slice( + datasource=datasource, name="test_slice", owners=[dashboard_owner_user] + ) + dashboard = create_dashboard( + dashboard_title="test_dashboard", + published=True, + slices=[slice_], + owners=[dashboard_owner_user], + ) + session = ctx.app.appbuilder.get_session + session.add(dashboard) + session.commit() + yield dashboard + except Exception as ex: + print(str(ex)) + yield Dashboard.get(dashboard.slug) + finally: + with app.app_context() as ctx: + session = ctx.app.appbuilder.get_session + try: + dashboard.owners = [] + slice_.owners = [] + datasource.owners = [] + session.merge(dashboard) + session.merge(slice_) + session.merge(datasource) + session.commit() + session.delete(dashboard) + session.delete(slice_) + session.delete(datasource) + session.delete(database) + session.commit() + except Exception as ex: + print(str(ex)) + + [email protected] +def dashboard_id(dashboard) -> int: + return dashboard.id Review comment: Do we need this fixture? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
