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

@@ -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:

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):

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)

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:

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)