[GitHub] [incubator-superset] villebro commented on a change in pull request #10118: feat: add database dropdown to dashboard import

[GitBox <gi...@apache.org>]

villebro commented on a change in pull request #10118:
URL: https://github.com/apache/incubator-superset/pull/10118#discussion_r443323616



##########
File path: superset/connectors/sqla/models.py
##########
@@ -1218,7 +1218,10 @@ def fetch_metadata(self, commit: bool = True) -> None:
 
     @classmethod
     def import_obj(
-        cls, i_datasource: "SqlaTable", import_time: Optional[int] = None
+        cls,
+        i_datasource: "SqlaTable",

Review comment:
       Not yours, but the name `i_datasource` makes me uncomfortable 😬 

##########
File path: superset/models/dashboard.py
##########
@@ -311,6 +311,10 @@ def alter_positions(
         # copy slices object as Slice.import_slice will mutate the slice
         # and will remove the existing dashboard - slice association
         slices = copy(dashboard_to_import.slices)
+
+        # Clearing the slug to avoid conflicts
+        dashboard_to_import.slug = None

Review comment:
       Removing the slug might be unexpected behavior for the end user. I wonder if we should raise an alert toast if the slug has been removed until we figure out a more permanent solution?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org