[GitHub] [superset] jfrag1 commented on a diff in pull request #22789: chore: Migrate /superset/recent_activity// to /api/v1/

["jfrag1 (via GitHub)" <gi...@apache.org>]

jfrag1 commented on code in PR #22789:
URL: https://github.com/apache/superset/pull/22789#discussion_r1084830867


##########
superset/views/log/api.py:
##########
@@ -44,13 +56,82 @@ class LogRestApi(LogMixin, BaseSupersetModelRestApi):
         "referrer",
     ]
     show_columns = list_columns
+    apispec_parameter_schemas = {
+        "get_recent_activity_schema": get_recent_activity_schema,
+    }
+    openapi_spec_component_schemas = (
+        RecentActivityResponseSchema,
+        RecentActivitySchema,
+    )
 
     @staticmethod
     def is_enabled() -> bool:
         return app.config["FAB_ADD_SECURITY_VIEWS"] and app.config["SUPERSET_LOG_VIEW"]
 
-    @before_request
+    @before_request(only=["get_list", "get", "post"])
     def ensure_enabled(self) -> None:
         if not self.is_enabled():
             return self.response_404()
         return None
+
+    def get_user_activity_access_error(self, user_id: int) -> Optional[FlaskResponse]:
+        try:
+            security_manager.raise_for_user_activity_access(user_id)
+        except SupersetSecurityException as ex:
+            return self.response(403, message=ex.message)
+        return None
+
+    @expose("/recent_activity/<int:user_id>/", methods=["GET"])

Review Comment:
   With `ENABLE_BROAD_ACTIVITY_ACCESS` enabled, users can view other users' profiles, which will fetch the those users' recent activity.  I don't think we can make this change without breaking this behavior, which seems to me like something that would have to wait for 3.0 (correct me if I'm wrong).
   
   I can think of one alternative: we make this new endpoint only get the current logged in user activity, and have the frontend for the profile page call the deprecated endpoint until it's removed in 3.0.  Then in 3.0 the ability to view other users' profiles would be removed, and we can switch that page to use the new endpoint.
   
   However, I think that only makes sense if `ENABLE_BROAD_ACTIVITY_ACCESS` will be removed in 3.0, what do you think?



-- 
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: notifications-unsubscribe@superset.apache.org

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