john-bodley commented on code in PR #24350: URL: https://github.com/apache/superset/pull/24350#discussion_r1225589330
########## superset/views/core.py: ########## @@ -1641,76 +1639,57 @@ def favstar( # pylint: disable=no-self-use @has_access @expose("/dashboard/<dashboard_id_or_slug>/") @event_logger.log_this_with_extra_payload - @check_dashboard_access( - on_error=lambda msg: redirect_with_flash(DASHBOARD_LIST_URL, msg, "danger") - ) def dashboard( self, - dashboard_id_or_slug: str, # pylint: disable=unused-argument + dashboard_id_or_slug: str, add_extra_log_payload: Callable[..., None] = lambda **kwargs: None, - dashboard: Dashboard | None = None, ) -> FlaskResponse: """ - Server side rendering for a dashboard - :param dashboard_id_or_slug: identifier for dashboard. used in the decorators + Server side rendering for a dashboard. + + :param dashboard_id_or_slug: identifier for dashboard :param add_extra_log_payload: added by `log_this_with_manual_updates`, set a default value to appease pylint - :param dashboard: added by `check_dashboard_access` """ + + dashboard = Dashboard.get(dashboard_id_or_slug) + if not dashboard: abort(404) - assert dashboard is not None - - has_access_ = False - for datasource in dashboard.datasources: - datasource = DatasourceDAO.get_datasource( - datasource_type=DatasourceType(datasource.type), - datasource_id=datasource.id, - session=db.session(), + try: + security_manager.raise_for_dashboard_access(dashboard) + except DashboardAccessDeniedError as ex: + return redirect_with_flash( + url="/dashboard/list/", + message=utils.error_msg_from_exception(ex), + category="danger", ) - if datasource and security_manager.can_access_datasource( - datasource=datasource, - ): - has_access_ = True - - if has_access_: - break - - if dashboard.datasources and not has_access_: - flash(DashboardAccessDeniedError.message, "danger") - return redirect(DASHBOARD_LIST_URL) - - dash_edit_perm = security_manager.is_owner( - dashboard - ) and security_manager.can_access("can_save_dash", "Superset") - edit_mode = ( - request.args.get(utils.ReservedUrlParameters.EDIT_MODE.value) == "true" - ) - - standalone_mode = ReservedUrlParameters.is_standalone_mode() add_extra_log_payload( dashboard_id=dashboard.id, dashboard_version="v2", - dash_edit_perm=dash_edit_perm, - edit_mode=edit_mode, + dash_edit_perm=( + security_manager.is_owner(dashboard) + and security_manager.can_access("can_save_dash", "Superset") + ), + edit_mode=( + request.args.get(ReservedUrlParameters.EDIT_MODE.value) == "true" + ), ) - bootstrap_data = { Review Comment: This is inlined later. ########## superset/views/core.py: ########## @@ -1641,76 +1648,54 @@ def favstar( # pylint: disable=no-self-use @has_access @expose("/dashboard/<dashboard_id_or_slug>/") @event_logger.log_this_with_extra_payload - @check_dashboard_access( - on_error=lambda msg: redirect_with_flash(DASHBOARD_LIST_URL, msg, "danger") - ) def dashboard( self, - dashboard_id_or_slug: str, # pylint: disable=unused-argument + dashboard_id_or_slug: str, add_extra_log_payload: Callable[..., None] = lambda **kwargs: None, - dashboard: Dashboard | None = None, ) -> FlaskResponse: """ - Server side rendering for a dashboard - :param dashboard_id_or_slug: identifier for dashboard. used in the decorators + Server side rendering for a dashboard. + + :param dashboard_id_or_slug: identifier for dashboard :param add_extra_log_payload: added by `log_this_with_manual_updates`, set a default value to appease pylint - :param dashboard: added by `check_dashboard_access` """ - if not dashboard: - abort(404) - assert dashboard is not None - - has_access_ = False - for datasource in dashboard.datasources: - datasource = DatasourceDAO.get_datasource( - datasource_type=DatasourceType(datasource.type), - datasource_id=datasource.id, - session=db.session(), + try: + dashboard = DashboardDAO.get_by_id_or_slug(dashboard_id_or_slug) + except DashboardNotFoundError: + abort(404) + except DashboardAccessDeniedError as ex: + return redirect_with_flash( + url="/dashboard/list/", + message=utils.error_msg_from_exception(ex), + category="danger", ) - if datasource and security_manager.can_access_datasource( - datasource=datasource, - ): - has_access_ = True - - if has_access_: - break - - if dashboard.datasources and not has_access_: - flash(DashboardAccessDeniedError.message, "danger") - return redirect(DASHBOARD_LIST_URL) - - dash_edit_perm = security_manager.is_owner( - dashboard - ) and security_manager.can_access("can_save_dash", "Superset") - edit_mode = ( - request.args.get(utils.ReservedUrlParameters.EDIT_MODE.value) == "true" - ) - - standalone_mode = ReservedUrlParameters.is_standalone_mode() add_extra_log_payload( dashboard_id=dashboard.id, dashboard_version="v2", - dash_edit_perm=dash_edit_perm, - edit_mode=edit_mode, + dash_edit_perm=( + security_manager.is_owner(dashboard) + and security_manager.can_access("can_save_dash", "Superset") + ), + edit_mode=( + request.args.get(ReservedUrlParameters.EDIT_MODE.value) == "true" + ), ) - bootstrap_data = { - "user": bootstrap_user_data(g.user, include_perms=True), - "common": common_bootstrap_payload(g.user), - } - return self.render_template( "superset/spa.html", entry="spa", - # dashboard title is always visible - title=dashboard.dashboard_title, + title=dashboard.dashboard_title, # dashboard title is always visible bootstrap_data=json.dumps( - bootstrap_data, default=utils.pessimistic_json_iso_dttm_ser + { Review Comment: Inlined logic from above. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org