dpgaspar commented on code in PR #21161:
URL: https://github.com/apache/superset/pull/21161#discussion_r959532309


##########
superset/security/manager.py:
##########
@@ -1106,83 +1173,417 @@ def _update_vm_datasources_access(  # pylint: 
disable=too-many-locals
                 .values(perm=new_dataset_vm_name)
             )
             self.on_view_menu_after_update(mapper, connection, 
new_dataset_view_menu)
-            
updated_view_menus.append(self.find_view_menu(new_dataset_view_menu))
+            updated_view_menus.append(new_dataset_view_menu)
         return updated_view_menus
 
-    def database_after_update(
+    def dataset_after_insert(
         self,
         mapper: Mapper,
         connection: Connection,
-        target: "Database",
+        target: "SqlaTable",
     ) -> None:
-        # Check if database name has changed
+        """
+        Handles permission creation when a dataset is inserted.
+        Triggered by a SQLAlchemy after_insert event.
+
+        We need to create:
+         - The dataset PVM and set local and schema perm
+
+        :param mapper: The SQLA mapper
+        :param connection: The SQLA connection
+        :param target: The changed dataset object
+        :return:
+        """
+        try:
+            dataset_perm = target.get_perm()
+        except DatasetInvalidPermissionEvaluationException:
+            logger.warning("Dataset has no database refusing to set 
permission")
+            return
+        dataset_table = target.__table__
+
+        self._insert_pvm_on_sqla_event(
+            mapper, connection, "datasource_access", dataset_perm
+        )
+        if target.perm != dataset_perm:
+            target.perm = dataset_perm
+            connection.execute(
+                dataset_table.update()
+                .where(dataset_table.c.id == target.id)
+                .values(perm=dataset_perm)
+            )

Review Comment:
   regarding shadow writes and permissions, yes we should talk about this. I've 
pinged @betodealmeida about it.
   
   Very good point, Just added the following tests to make sure everything 
rollback has expected:
   - test_after_insert_database_rollback
   - test_after_insert_dataset_rollback
   - test_after_update_database_rollback
   - test_after_update_dataset_rollback
   - test_after_delete_database_rollback
   - test_after_delete_dataset_rollback
   



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

Reply via email to