From 092ef5bdfca2f77a64a2edf88087a142fb8846f3 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 15 Sep 2021 12:27:02 -0700 Subject: [PATCH] fix: set importer as owner (#16656) * fix: set importer as owner * Fix tests --- superset/charts/commands/importers/v1/utils.py | 4 ++++ superset/dashboards/commands/importers/v1/utils.py | 4 ++++ superset/datasets/commands/importers/v1/utils.py | 5 ++++- tests/integration_tests/charts/api_tests.py | 6 ++++++ tests/integration_tests/charts/commands_tests.py | 11 +++++++++-- tests/integration_tests/dashboards/commands_tests.py | 10 +++++++++- tests/integration_tests/databases/api_tests.py | 4 ++++ tests/integration_tests/datasets/api_tests.py | 4 ++++ tests/integration_tests/datasets/commands_tests.py | 12 +++++++++++- 9 files changed, 55 insertions(+), 5 deletions(-) diff --git a/superset/charts/commands/importers/v1/utils.py b/superset/charts/commands/importers/v1/utils.py index b3d4237f2b5da..d4c69d3ada0b5 100644 --- a/superset/charts/commands/importers/v1/utils.py +++ b/superset/charts/commands/importers/v1/utils.py @@ -18,6 +18,7 @@ import json from typing import Any, Dict +from flask import g from sqlalchemy.orm import Session from superset.models.slice import Slice @@ -39,4 +40,7 @@ def import_chart( if chart.id is None: session.flush() + if hasattr(g, "user") and g.user: + chart.owners.append(g.user) + return chart diff --git a/superset/dashboards/commands/importers/v1/utils.py b/superset/dashboards/commands/importers/v1/utils.py index 9a0a6b23eb96b..fd5b4ee51bd66 100644 --- a/superset/dashboards/commands/importers/v1/utils.py +++ b/superset/dashboards/commands/importers/v1/utils.py @@ -19,6 +19,7 @@ import logging from typing import Any, Dict, Set +from flask import g from sqlalchemy.orm import Session from superset.models.dashboard import Dashboard @@ -155,4 +156,7 @@ def import_dashboard( if dashboard.id is None: session.flush() + if hasattr(g, "user") and g.user: + dashboard.owners.append(g.user) + return dashboard diff --git a/superset/datasets/commands/importers/v1/utils.py b/superset/datasets/commands/importers/v1/utils.py index 43df8b6ae5ec3..04650d069b8e4 100644 --- a/superset/datasets/commands/importers/v1/utils.py +++ b/superset/datasets/commands/importers/v1/utils.py @@ -22,7 +22,7 @@ from urllib import request import pandas as pd -from flask import current_app +from flask import current_app, g from sqlalchemy import BigInteger, Boolean, Date, DateTime, Float, String, Text from sqlalchemy.orm import Session from sqlalchemy.sql.visitors import VisitableType @@ -127,6 +127,9 @@ def import_dataset( if data_uri and (not table_exists or force_data): load_data(data_uri, dataset, example_database, session) + if hasattr(g, "user") and g.user: + dataset.owners.append(g.user) + return dataset diff --git a/tests/integration_tests/charts/api_tests.py b/tests/integration_tests/charts/api_tests.py index 7499e9e3a459d..ca0e45ed520a0 100644 --- a/tests/integration_tests/charts/api_tests.py +++ b/tests/integration_tests/charts/api_tests.py @@ -1715,6 +1715,9 @@ def test_import_chart(self): chart = db.session.query(Slice).filter_by(uuid=chart_config["uuid"]).one() assert chart.table == dataset + chart.owners = [] + dataset.owners = [] + database.owners = [] db.session.delete(chart) db.session.delete(dataset) db.session.delete(database) @@ -1784,6 +1787,9 @@ def test_import_chart_overwrite(self): dataset = database.tables[0] chart = db.session.query(Slice).filter_by(uuid=chart_config["uuid"]).one() + chart.owners = [] + dataset.owners = [] + database.owners = [] db.session.delete(chart) db.session.delete(dataset) db.session.delete(database) diff --git a/tests/integration_tests/charts/commands_tests.py b/tests/integration_tests/charts/commands_tests.py index a54e50ad81f0c..bad8530488913 100644 --- a/tests/integration_tests/charts/commands_tests.py +++ b/tests/integration_tests/charts/commands_tests.py @@ -134,8 +134,10 @@ def test_export_chart_command_key_order(self, mock_g): class TestImportChartsCommand(SupersetTestCase): - def test_import_v1_chart(self): + @patch("superset.charts.commands.importers.v1.utils.g") + def test_import_v1_chart(self, mock_g): """Test that we can import a chart""" + mock_g.user = security_manager.find_user("admin") contents = { "metadata.yaml": yaml.safe_dump(chart_metadata_config), "databases/imported_database.yaml": yaml.safe_dump(database_config), @@ -224,6 +226,11 @@ def test_import_v1_chart(self): assert database.database_name == "imported_database" assert chart.table.database == database + assert chart.owners == [mock_g.user] + + chart.owners = [] + dataset.owners = [] + database.owners = [] db.session.delete(chart) db.session.delete(dataset) db.session.delete(database) @@ -332,7 +339,7 @@ def test_update_v1_response(self, mock_sm_g, mock_g): @patch("superset.security.manager.g") @pytest.mark.usefixtures("load_energy_table_with_slice") def test_query_context_update_command(self, mock_sm_g, mock_g): - """ " + """ Test that a user can generate the chart query context payloadwithout affecting owners """ diff --git a/tests/integration_tests/dashboards/commands_tests.py b/tests/integration_tests/dashboards/commands_tests.py index 596dc404035b3..63d56eed0b8db 100644 --- a/tests/integration_tests/dashboards/commands_tests.py +++ b/tests/integration_tests/dashboards/commands_tests.py @@ -463,8 +463,10 @@ def test_import_v0_dashboard_cli_export(self): db.session.delete(dataset) db.session.commit() - def test_import_v1_dashboard(self): + @patch("superset.dashboards.commands.importers.v1.utils.g") + def test_import_v1_dashboard(self, mock_g): """Test that we can import a dashboard""" + mock_g.user = security_manager.find_user("admin") contents = { "metadata.yaml": yaml.safe_dump(dashboard_metadata_config), "databases/imported_database.yaml": yaml.safe_dump(database_config), @@ -544,6 +546,12 @@ def test_import_v1_dashboard(self): database = dataset.database assert str(database.uuid) == database_config["uuid"] + assert dashboard.owners == [mock_g.user] + + dashboard.owners = [] + chart.owners = [] + dataset.owners = [] + database.owners = [] db.session.delete(dashboard) db.session.delete(chart) db.session.delete(dataset) diff --git a/tests/integration_tests/databases/api_tests.py b/tests/integration_tests/databases/api_tests.py index bf87120c1b108..a7b28b8f6ab7a 100644 --- a/tests/integration_tests/databases/api_tests.py +++ b/tests/integration_tests/databases/api_tests.py @@ -1188,6 +1188,8 @@ def test_import_database(self): assert dataset.table_name == "imported_dataset" assert str(dataset.uuid) == dataset_config["uuid"] + dataset.owners = [] + database.owners = [] db.session.delete(dataset) db.session.delete(database) db.session.commit() @@ -1257,6 +1259,8 @@ def test_import_database_overwrite(self): db.session.query(Database).filter_by(uuid=database_config["uuid"]).one() ) dataset = database.tables[0] + dataset.owners = [] + database.owners = [] db.session.delete(dataset) db.session.delete(database) db.session.commit() diff --git a/tests/integration_tests/datasets/api_tests.py b/tests/integration_tests/datasets/api_tests.py index 6a5905db88e58..bf689ee50beec 100644 --- a/tests/integration_tests/datasets/api_tests.py +++ b/tests/integration_tests/datasets/api_tests.py @@ -1534,6 +1534,8 @@ def test_import_dataset(self): assert dataset.table_name == "imported_dataset" assert str(dataset.uuid) == dataset_config["uuid"] + dataset.owners = [] + database.owners = [] db.session.delete(dataset) db.session.delete(database) db.session.commit() @@ -1626,6 +1628,8 @@ def test_import_dataset_overwrite(self): ) dataset = database.tables[0] + dataset.owners = [] + database.owners = [] db.session.delete(dataset) db.session.delete(database) db.session.commit() diff --git a/tests/integration_tests/datasets/commands_tests.py b/tests/integration_tests/datasets/commands_tests.py index 834d91f516276..b7f61fa7b69c2 100644 --- a/tests/integration_tests/datasets/commands_tests.py +++ b/tests/integration_tests/datasets/commands_tests.py @@ -292,8 +292,11 @@ def test_import_v0_dataset_ui_export(self): db.session.delete(dataset) db.session.commit() - def test_import_v1_dataset(self): + @patch("superset.datasets.commands.importers.v1.utils.g") + @pytest.mark.usefixtures("load_energy_table_with_slice") + def test_import_v1_dataset(self, mock_g): """Test that we can import a dataset""" + mock_g.user = security_manager.find_user("admin") contents = { "metadata.yaml": yaml.safe_dump(dataset_metadata_config), "databases/imported_database.yaml": yaml.safe_dump(database_config), @@ -319,6 +322,9 @@ def test_import_v1_dataset(self): assert dataset.fetch_values_predicate is None assert dataset.extra == "dttm > sysdate() -10 " + # user should be included as one of the owners + assert dataset.owners == [mock_g.user] + # database is also imported assert str(dataset.database.uuid) == "b8a1ccd3-779d-4ab7-8ad8-9ab119d7fe89" @@ -346,6 +352,8 @@ def test_import_v1_dataset(self): assert column.description is None assert column.python_date_format is None + dataset.owners = [] + dataset.database.owners = [] db.session.delete(dataset) db.session.delete(dataset.database) db.session.commit() @@ -467,6 +475,8 @@ def test_import_v1_dataset_existing_database(self): ) assert len(database.tables) == 1 + database.tables[0].owners = [] + database.owners = [] db.session.delete(database.tables[0]) db.session.delete(database) db.session.commit()