You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ma...@apache.org on 2019/07/23 05:30:40 UTC
[incubator-superset] branch master updated: Set owner to dashboards
and charts on import (#7894)
This is an automated email from the ASF dual-hosted git repository.
maximebeauchemin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new 648f9fa Set owner to dashboards and charts on import (#7894)
648f9fa is described below
commit 648f9fa54b90ce4cca84737a21214fd92c0e9fcc
Author: Maxim Sukharev <ma...@smacker.ru>
AuthorDate: Tue Jul 23 07:30:28 2019 +0200
Set owner to dashboards and charts on import (#7894)
* Allow to pass user for dashboard import cli
Dashboard import assign current user from flask context during import.
But in case of cli import there is no flask user and imported charts
don't have an owner which prevents ability to edit them.
* Reset ownership on dashboard import
For overriding existing charts it requires `owners` property to be set.
* Add tests for reset ownership
* Use ORM to decode dashboards json
Creating instances using ORM allows to normally work with relations
* Fix test_import_dashboard_1_slice test
Previously tests used side-effect of slices import which kept id from
json on insert into db.
---
superset/cli.py | 11 +++++-
superset/models/core.py | 13 +++---
superset/models/helpers.py | 15 ++++++-
superset/utils/core.py | 20 +++-------
tests/import_export_tests.py | 94 +++++++++++++++++++++++++++++++++++++++++++-
5 files changed, 128 insertions(+), 25 deletions(-)
diff --git a/superset/cli.py b/superset/cli.py
index c4f8309..cc66f58 100755
--- a/superset/cli.py
+++ b/superset/cli.py
@@ -23,6 +23,7 @@ from sys import stdout
import click
from colorama import Fore, Style
+from flask import g
from pathlib2 import Path
import yaml
@@ -184,7 +185,13 @@ def refresh_druid(datasource, merge):
default=False,
help="recursively search the path for json files",
)
-def import_dashboards(path, recursive):
+@click.option(
+ "--username",
+ "-u",
+ default=None,
+ help="Specify the user name to assign dashboards to",
+)
+def import_dashboards(path, recursive, username):
"""Import dashboards from JSON"""
p = Path(path)
files = []
@@ -194,6 +201,8 @@ def import_dashboards(path, recursive):
files.extend(p.glob("*.json"))
elif p.exists() and recursive:
files.extend(p.rglob("*.json"))
+ if username is not None:
+ g.user = security_manager.find_user(username=username)
for f in files:
logging.info("Importing dashboard from file %s", f)
try:
diff --git a/superset/models/core.py b/superset/models/core.py
index 59d38eb..bfc080b 100755
--- a/superset/models/core.py
+++ b/superset/models/core.py
@@ -361,6 +361,7 @@ class Slice(Model, AuditMixinNullable, ImportMixin):
slc_to_import.alter_params(remote_id=slc_to_import.id, import_time=import_time)
slc_to_import = slc_to_import.copy()
+ slc_to_import.reset_ownership()
params = slc_to_import.params_dict
slc_to_import.datasource_id = ConnectorRegistry.get_datasource_by_name(
session,
@@ -617,7 +618,9 @@ class Dashboard(Model, AuditMixinNullable, ImportMixin):
):
existing_dashboard = dash
+ dashboard_to_import = dashboard_to_import.copy()
dashboard_to_import.id = None
+ dashboard_to_import.reset_ownership()
# position_json can be empty for dashboards
# with charts added from chart-edit page and without re-arranging
if dashboard_to_import.position_json:
@@ -646,14 +649,10 @@ class Dashboard(Model, AuditMixinNullable, ImportMixin):
session.flush()
return existing_dashboard.id
else:
- # session.add(dashboard_to_import) causes sqlachemy failures
- # related to the attached users / slices. Creating new object
- # allows to avoid conflicts in the sql alchemy state.
- copied_dash = dashboard_to_import.copy()
- copied_dash.slices = new_slices
- session.add(copied_dash)
+ dashboard_to_import.slices = new_slices
+ session.add(dashboard_to_import)
session.flush()
- return copied_dash.id
+ return dashboard_to_import.id
@classmethod
def export_dashboards(cls, dashboard_ids):
diff --git a/superset/models/helpers.py b/superset/models/helpers.py
index 7c39c1c..472ad74 100644
--- a/superset/models/helpers.py
+++ b/superset/models/helpers.py
@@ -21,7 +21,7 @@ import json
import logging
import re
-from flask import escape, Markup
+from flask import escape, g, Markup
from flask_appbuilder.models.decorators import renders
from flask_appbuilder.models.mixins import AuditMixin
import humanize
@@ -265,6 +265,19 @@ class ImportMixin(object):
d.update(kwargs)
self.params = json.dumps(d)
+ def reset_ownership(self):
+ """ object will belong to the user the current user """
+ # make sure the object doesn't have relations to a user
+ # it will be filled by appbuilder on save
+ self.created_by = None
+ self.changed_by = None
+ # flask global context might not exist (in cli or tests for example)
+ try:
+ if g.user:
+ self.owners = [g.user]
+ except Exception:
+ self.owners = []
+
@property
def params_dict(self):
return json_to_dict(self.params)
diff --git a/superset/utils/core.py b/superset/utils/core.py
index 8f0de13..b7d3370 100644
--- a/superset/utils/core.py
+++ b/superset/utils/core.py
@@ -254,25 +254,15 @@ def decode_dashboards(o):
from superset.connectors.sqla.models import SqlaTable, SqlMetric, TableColumn
if "__Dashboard__" in o:
- d = models.Dashboard()
- d.__dict__.update(o["__Dashboard__"])
- return d
+ return models.Dashboard(**o["__Dashboard__"])
elif "__Slice__" in o:
- d = models.Slice()
- d.__dict__.update(o["__Slice__"])
- return d
+ return models.Slice(**o["__Slice__"])
elif "__TableColumn__" in o:
- d = TableColumn()
- d.__dict__.update(o["__TableColumn__"])
- return d
+ return TableColumn(**o["__TableColumn__"])
elif "__SqlaTable__" in o:
- d = SqlaTable()
- d.__dict__.update(o["__SqlaTable__"])
- return d
+ return SqlaTable(**o["__SqlaTable__"])
elif "__SqlMetric__" in o:
- d = SqlMetric()
- d.__dict__.update(o["__SqlMetric__"])
- return d
+ return SqlMetric(**o["__SqlMetric__"])
elif "__datetime__" in o:
return datetime.strptime(o["__datetime__"], "%Y-%m-%dT%H:%M:%S")
else:
diff --git a/tests/import_export_tests.py b/tests/import_export_tests.py
index 03068a4..30a55ce 100644
--- a/tests/import_export_tests.py
+++ b/tests/import_export_tests.py
@@ -18,9 +18,10 @@
import json
import unittest
+from flask import Flask, g
from sqlalchemy.orm.session import make_transient
-from superset import db
+from superset import db, security_manager
from superset.connectors.druid.models import DruidColumn, DruidDatasource, DruidMetric
from superset.connectors.sqla.models import SqlaTable, SqlMetric, TableColumn
from superset.models import core as models
@@ -375,6 +376,10 @@ class ImportExportTests(SupersetTestCase):
)
expected_position = dash_with_1_slice.position
+ # new slice id (auto-incremental) assigned on insert
+ # id from json is used only for updating position with new id
+ meta = expected_position["DASHBOARD_CHART_TYPE-10006"]["meta"]
+ meta["chartId"] = imported_dash.slices[0].id
self.assertEquals(expected_position, imported_dash.position)
def test_import_dashboard_2_slices(self):
@@ -453,6 +458,93 @@ class ImportExportTests(SupersetTestCase):
json.loads(imported_dash.json_metadata),
)
+ def test_import_new_dashboard_slice_reset_ownership(self):
+ app = Flask("test_import_dashboard_slice_set_user")
+ with app.app_context():
+ admin_user = security_manager.find_user(username="admin")
+ self.assertTrue(admin_user)
+ gamma_user = security_manager.find_user(username="gamma")
+ self.assertTrue(gamma_user)
+ g.user = gamma_user
+
+ dash_with_1_slice = self._create_dashboard_for_import(id_=10200)
+ # set another user as an owner of importing dashboard
+ dash_with_1_slice.created_by = admin_user
+ dash_with_1_slice.changed_by = admin_user
+ dash_with_1_slice.owners = [admin_user]
+
+ imported_dash_id = models.Dashboard.import_obj(dash_with_1_slice)
+ imported_dash = self.get_dash(imported_dash_id)
+ self.assertEqual(imported_dash.created_by, gamma_user)
+ self.assertEqual(imported_dash.changed_by, gamma_user)
+ self.assertEqual(imported_dash.owners, [gamma_user])
+
+ imported_slc = imported_dash.slices[0]
+ self.assertEqual(imported_slc.created_by, gamma_user)
+ self.assertEqual(imported_slc.changed_by, gamma_user)
+ self.assertEqual(imported_slc.owners, [gamma_user])
+
+ def test_import_override_dashboard_slice_reset_ownership(self):
+ app = Flask("test_import_dashboard_slice_set_user")
+ with app.app_context():
+ admin_user = security_manager.find_user(username="admin")
+ self.assertTrue(admin_user)
+ gamma_user = security_manager.find_user(username="gamma")
+ self.assertTrue(gamma_user)
+ g.user = gamma_user
+
+ dash_with_1_slice = self._create_dashboard_for_import(id_=10300)
+
+ imported_dash_id = models.Dashboard.import_obj(dash_with_1_slice)
+ imported_dash = self.get_dash(imported_dash_id)
+ self.assertEqual(imported_dash.created_by, gamma_user)
+ self.assertEqual(imported_dash.changed_by, gamma_user)
+ self.assertEqual(imported_dash.owners, [gamma_user])
+
+ imported_slc = imported_dash.slices[0]
+ self.assertEqual(imported_slc.created_by, gamma_user)
+ self.assertEqual(imported_slc.changed_by, gamma_user)
+ self.assertEqual(imported_slc.owners, [gamma_user])
+
+ # re-import with another user shouldn't change the permissions
+ g.user = admin_user
+
+ dash_with_1_slice = self._create_dashboard_for_import(id_=10300)
+
+ imported_dash_id = models.Dashboard.import_obj(dash_with_1_slice)
+ imported_dash = self.get_dash(imported_dash_id)
+ self.assertEqual(imported_dash.created_by, gamma_user)
+ self.assertEqual(imported_dash.changed_by, gamma_user)
+ self.assertEqual(imported_dash.owners, [gamma_user])
+
+ imported_slc = imported_dash.slices[0]
+ self.assertEqual(imported_slc.created_by, gamma_user)
+ self.assertEqual(imported_slc.changed_by, gamma_user)
+ self.assertEqual(imported_slc.owners, [gamma_user])
+
+ def _create_dashboard_for_import(self, id_=10100):
+ slc = self.create_slice("health_slc" + str(id_), id=id_ + 1)
+ dash_with_1_slice = self.create_dashboard(
+ "dash_with_1_slice" + str(id_), slcs=[slc], id=id_ + 2
+ )
+ dash_with_1_slice.position_json = """
+ {{"DASHBOARD_VERSION_KEY": "v2",
+ "DASHBOARD_CHART_TYPE-{0}": {{
+ "type": "DASHBOARD_CHART_TYPE",
+ "id": {0},
+ "children": [],
+ "meta": {{
+ "width": 4,
+ "height": 50,
+ "chartId": {0}
+ }}
+ }}
+ }}
+ """.format(
+ slc.id
+ )
+ return dash_with_1_slice
+
def test_import_table_no_metadata(self):
table = self.create_table("pure_table", id=10001)
imported_id = SqlaTable.import_obj(table, import_time=1989)