refactor: remove obsolete Flask flash messaging system (#35237)

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Maxime Beauchemin
2025-09-25 00:05:16 -07:00
committed by GitHub
parent 927cc1cda1
commit abc2d46fed
19 changed files with 85 additions and 274 deletions

View File

@@ -49,7 +49,6 @@ export default {
dash_edit_perm: true,
dash_save_perm: true,
common: {
flash_messages: [],
conf: { SUPERSET_WEBSERVER_TIMEOUT: 60 },
},
filterBarOrientation: FilterBarOrientation.Vertical,

View File

@@ -1,65 +0,0 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { render, screen } from 'spec/helpers/testing-library';
import { Provider } from 'react-redux';
import { store } from 'src/views/store';
import type { FlashMessage } from './types';
import { FlashProvider } from '.';
test('Rerendering correctly with default props', () => {
const messages: FlashMessage[] = [];
render(
<FlashProvider messages={messages}>
<div data-test="my-component">My Component</div>
</FlashProvider>,
{ store },
);
expect(screen.getByTestId('my-component')).toBeInTheDocument();
});
test('messages should only be inserted in the State when the component is mounted', () => {
const messages: FlashMessage[] = [
['info', 'teste message 01'],
['info', 'teste message 02'],
];
expect(store.getState().messageToasts).toEqual([]);
const { rerender } = render(
<Provider store={store}>
<FlashProvider messages={messages}>
<div data-teste="my-component">My Component</div>
</FlashProvider>
</Provider>,
);
const fistRender = store.getState().messageToasts;
expect(fistRender).toHaveLength(2);
expect(fistRender[1].text).toBe(messages[0][1]);
expect(fistRender[0].text).toBe(messages[1][1]);
rerender(
<Provider store={store}>
<FlashProvider messages={[...messages, ['info', 'teste message 03']]}>
<div data-teste="my-component">My Component</div>
</FlashProvider>
</Provider>,
);
const secondRender = store.getState().messageToasts;
expect(secondRender).toEqual(fistRender);
});

View File

@@ -1,51 +0,0 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { useToasts } from 'src/components/MessageToasts/withToasts';
import { useComponentDidMount } from '@superset-ui/core';
import type { FlashMessage } from './types';
interface Props {
children: JSX.Element;
messages: FlashMessage[];
}
const flashObj = {
info: 'addInfoToast',
alert: 'addDangerToast',
danger: 'addDangerToast',
warning: 'addWarningToast',
success: 'addSuccessToast',
};
export function FlashProvider({ children, messages }: Props) {
const toasts = useToasts();
useComponentDidMount(() => {
messages.forEach(message => {
const [type, text] = message;
const flash = flashObj[type];
const toast = toasts[flash as keyof typeof toasts];
if (toast) {
toast(text);
}
});
});
return children;
}
export type { FlashMessage };

View File

@@ -1,20 +0,0 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
type FlashMessageType = 'info' | 'alert' | 'danger' | 'warning' | 'success';
export type FlashMessage = [FlashMessageType, string];

View File

@@ -38,7 +38,6 @@ export * from './ErrorMessage';
export { ImportModal, type ImportModelsModalProps } from './ImportModal';
export { ErrorBoundary, type ErrorBoundaryProps } from './ErrorBoundary';
export * from './GenericLink';
export { FlashProvider, type FlashMessage } from './FlashProvider';
export { GridTable, type TableProps } from './GridTable';
export * from './Tag';
export * from './TagsList';

View File

@@ -122,7 +122,6 @@ export const RESERVED_DASHBOARD_URL_PARAMS: string[] = [
export const DEFAULT_COMMON_BOOTSTRAP_DATA: CommonBootstrapData = {
application_root: '/',
static_assets_prefix: '',
flash_messages: [],
conf: {},
locale: 'en',
feature_flags: {},

View File

@@ -274,7 +274,6 @@ export const hydrateDashboard =
superset_can_csv: findPermission('can_csv', 'Superset', roles),
common: {
// legacy, please use state.common instead
flash_messages: common?.flash_messages,
conf: common?.conf,
},
filterBarOrientation:

View File

@@ -22,13 +22,12 @@ import { Provider as ReduxProvider } from 'react-redux';
import { QueryParamProvider } from 'use-query-params';
import { DndProvider } from 'react-dnd';
import { HTML5Backend } from 'react-dnd-html5-backend';
import { FlashProvider, DynamicPluginProvider } from 'src/components';
import { DynamicPluginProvider } from 'src/components';
import { EmbeddedUiConfigProvider } from 'src/components/UiConfigContext';
import { SupersetThemeProvider } from 'src/theme/ThemeProvider';
import { ThemeController } from 'src/theme/ThemeController';
import type { ThemeStorage } from '@superset-ui/core';
import { store } from 'src/views/store';
import getBootstrapData from 'src/utils/getBootstrapData';
/**
* In-memory implementation of ThemeStorage interface for embedded contexts.
@@ -56,7 +55,6 @@ const themeController = new ThemeController({
export const getThemeController = (): ThemeController => themeController;
const { common } = getBootstrapData();
const extensionsRegistry = getExtensionsRegistry();
export const EmbeddedContextProviders: React.FC = ({ children }) => {
@@ -68,24 +66,22 @@ export const EmbeddedContextProviders: React.FC = ({ children }) => {
<SupersetThemeProvider themeController={themeController}>
<ReduxProvider store={store}>
<DndProvider backend={HTML5Backend}>
<FlashProvider messages={common.flash_messages}>
<EmbeddedUiConfigProvider>
<DynamicPluginProvider>
<QueryParamProvider
ReactRouterRoute={Route}
stringifyOptions={{ encode: false }}
>
{RootContextProviderExtension ? (
<RootContextProviderExtension>
{children}
</RootContextProviderExtension>
) : (
children
)}
</QueryParamProvider>
</DynamicPluginProvider>
</EmbeddedUiConfigProvider>
</FlashProvider>
<EmbeddedUiConfigProvider>
<DynamicPluginProvider>
<QueryParamProvider
ReactRouterRoute={Route}
stringifyOptions={{ encode: false }}
>
{RootContextProviderExtension ? (
<RootContextProviderExtension>
{children}
</RootContextProviderExtension>
) : (
children
)}
</QueryParamProvider>
</DynamicPluginProvider>
</EmbeddedUiConfigProvider>
</DndProvider>
</ReduxProvider>
</SupersetThemeProvider>

View File

@@ -98,7 +98,6 @@ export interface ExploreResponsePayload {
export interface ExplorePageState {
user: UserWithPermissionsAndRoles;
common: {
flash_messages: string[];
conf: JsonObject;
locale: string;
};

View File

@@ -33,7 +33,7 @@ jest.mock('src/utils/getBootstrapData', () => ({
}));
test('should render login form elements', () => {
render(<Login />);
render(<Login />, { useRedux: true });
expect(screen.getByTestId('login-form')).toBeInTheDocument();
expect(screen.getByTestId('username-input')).toBeInTheDocument();
expect(screen.getByTestId('password-input')).toBeInTheDocument();
@@ -42,13 +42,13 @@ test('should render login form elements', () => {
});
test('should render username and password labels', () => {
render(<Login />);
render(<Login />, { useRedux: true });
expect(screen.getByText('Username:')).toBeInTheDocument();
expect(screen.getByText('Password:')).toBeInTheDocument();
});
test('should render form instruction text', () => {
render(<Login />);
render(<Login />, { useRedux: true });
expect(
screen.getByText('Enter your login and password below:'),
).toBeInTheDocument();

View File

@@ -27,8 +27,10 @@ import {
Typography,
Icons,
} from '@superset-ui/core/components';
import { useState } from 'react';
import { useState, useEffect } from 'react';
import { capitalize } from 'lodash/fp';
import { addDangerToast } from 'src/components/MessageToasts/actions';
import { useDispatch } from 'react-redux';
import getBootstrapData from 'src/utils/getBootstrapData';
type OAuthProvider = {
@@ -77,6 +79,7 @@ const StyledLabel = styled(Typography.Text)`
export default function Login() {
const [form] = Form.useForm<LoginForm>();
const [loading, setLoading] = useState(false);
const dispatch = useDispatch();
const bootstrapData = getBootstrapData();
@@ -85,11 +88,28 @@ export default function Login() {
const authRegistration: boolean =
bootstrapData.common.conf.AUTH_USER_REGISTRATION;
// TODO: This is a temporary solution for showing login errors after form submission.
// Should be replaced with proper SPA-style authentication (JSON API with error responses)
// when Flask-AppBuilder is updated or we implement a custom login endpoint.
useEffect(() => {
const loginAttempted = sessionStorage.getItem('login_attempted');
if (loginAttempted === 'true') {
sessionStorage.removeItem('login_attempted');
dispatch(addDangerToast(t('Invalid username or password')));
// Clear password field for security
form.setFieldsValue({ password: '' });
}
}, [dispatch, form]);
const onFinish = (values: LoginForm) => {
setLoading(true);
SupersetClient.postForm('/login/', values, '').finally(() => {
setLoading(false);
});
// Mark that we're attempting login (for error detection after redirect)
sessionStorage.setItem('login_attempted', 'true');
// Use standard form submission for Flask-AppBuilder compatibility
SupersetClient.postForm('/login/', values, '');
};
const getAuthIconElement = (

View File

@@ -83,7 +83,6 @@ const createMockBootstrapData = (
common: {
application_root: '/',
static_assets_prefix: '/static/assets/',
flash_messages: [],
conf: {},
locale: 'en',
feature_flags: {},
@@ -391,7 +390,6 @@ describe('ThemeController', () => {
common: {
application_root: '/',
static_assets_prefix: '/static/assets/',
flash_messages: [],
conf: {},
locale: 'en',
feature_flags: {},

View File

@@ -20,7 +20,6 @@ import { FormatLocaleDefinition } from 'd3-format';
import { TimeLocaleDefinition } from 'd3-time-format';
import { isPlainObject } from 'lodash';
import { Languages } from 'src/features/home/LanguagePicker';
import type { FlashMessage } from 'src/components';
import type {
AnyThemeConfig,
ColorSchemeConfig,
@@ -154,7 +153,6 @@ export interface BootstrapThemeDataConfig {
export interface CommonBootstrapData {
application_root: string;
static_assets_prefix: string;
flash_messages: FlashMessage[];
conf: JsonObject;
locale: Locale;
feature_flags: FeatureFlagMap;

View File

@@ -23,8 +23,7 @@ import { Provider as ReduxProvider } from 'react-redux';
import { QueryParamProvider } from 'use-query-params';
import { DndProvider } from 'react-dnd';
import { HTML5Backend } from 'react-dnd-html5-backend';
import getBootstrapData from 'src/utils/getBootstrapData';
import { FlashProvider, DynamicPluginProvider } from 'src/components';
import { DynamicPluginProvider } from 'src/components';
import { EmbeddedUiConfigProvider } from 'src/components/UiConfigContext';
import { SupersetThemeProvider } from 'src/theme/ThemeProvider';
import { ThemeController } from 'src/theme/ThemeController';
@@ -32,7 +31,6 @@ import { ExtensionsProvider } from 'src/extensions/ExtensionsContext';
import { store } from './store';
import '../preamble';
const { common } = getBootstrapData();
const themeController = new ThemeController();
const extensionsRegistry = getExtensionsRegistry();
@@ -45,26 +43,24 @@ export const RootContextProviders: React.FC = ({ children }) => {
<SupersetThemeProvider themeController={themeController}>
<ReduxProvider store={store}>
<DndProvider backend={HTML5Backend}>
<FlashProvider messages={common.flash_messages}>
<EmbeddedUiConfigProvider>
<DynamicPluginProvider>
<QueryParamProvider
ReactRouterRoute={Route}
stringifyOptions={{ encode: false }}
>
<ExtensionsProvider>
{RootContextProviderExtension ? (
<RootContextProviderExtension>
{children}
</RootContextProviderExtension>
) : (
children
)}
</ExtensionsProvider>
</QueryParamProvider>
</DynamicPluginProvider>
</EmbeddedUiConfigProvider>
</FlashProvider>
<EmbeddedUiConfigProvider>
<DynamicPluginProvider>
<QueryParamProvider
ReactRouterRoute={Route}
stringifyOptions={{ encode: false }}
>
<ExtensionsProvider>
{RootContextProviderExtension ? (
<RootContextProviderExtension>
{children}
</RootContextProviderExtension>
) : (
children
)}
</ExtensionsProvider>
</QueryParamProvider>
</DynamicPluginProvider>
</EmbeddedUiConfigProvider>
</DndProvider>
</ReduxProvider>
</SupersetThemeProvider>

View File

@@ -18,9 +18,8 @@
import logging
from typing import Optional
from flask import flash, g, redirect
from flask import g, redirect
from flask_appbuilder import expose
from flask_appbuilder._compat import as_unicode
from flask_appbuilder.const import LOGMSG_ERR_SEC_NO_REGISTER_HASH
from flask_appbuilder.security.decorators import no_cache
from flask_appbuilder.security.views import AuthView, WerkzeugResponse
@@ -66,7 +65,7 @@ class SupersetRegisterUserView(BaseSupersetView):
reg = self.appbuilder.sm.find_register_user(activation_hash)
if not reg:
logger.error(LOGMSG_ERR_SEC_NO_REGISTER_HASH, activation_hash)
flash(as_unicode(self.false_error_message), "danger")
logger.error("Registration activation failed: %s", self.false_error_message)
return redirect(self.appbuilder.get_url_for_index)
if not self.appbuilder.sm.add_user(
username=reg.username,
@@ -78,7 +77,7 @@ class SupersetRegisterUserView(BaseSupersetView):
),
hashed_password=reg.password,
):
flash(as_unicode(self.error_message), "danger")
logger.error("User registration failed: %s", self.error_message)
return redirect(self.appbuilder.get_url_for_index)
else:
self.appbuilder.sm.del_register_user(reg)

View File

@@ -27,9 +27,7 @@ from babel import Locale
from flask import (
abort,
current_app as app,
flash,
g,
get_flashed_messages,
redirect,
Response,
session,
@@ -499,10 +497,7 @@ def cached_common_bootstrap_data( # pylint: disable=unused-argument
def common_bootstrap_payload() -> dict[str, Any]:
return {
**cached_common_bootstrap_data(utils.get_user_id(), get_locale()),
"flash_messages": get_flashed_messages(with_categories=True),
}
return cached_common_bootstrap_data(utils.get_user_id(), get_locale())
def get_spa_payload(extra_data: dict[str, Any] | None = None) -> dict[str, Any]:
@@ -597,7 +592,7 @@ class DeleteMixin: # pylint: disable=too-few-public-methods
try:
self.pre_delete(item)
except Exception as ex: # pylint: disable=broad-except
flash(str(ex), "danger")
logger.error("Pre-delete error: %s", str(ex))
else:
view_menu = security_manager.find_view_menu(item.get_perm())
pvs = (
@@ -617,7 +612,6 @@ class DeleteMixin: # pylint: disable=too-few-public-methods
db.session.commit() # pylint: disable=consider-using-transaction
flash(*self.datamodel.message)
self.update_redirect()
@action(
@@ -630,7 +624,7 @@ class DeleteMixin: # pylint: disable=too-few-public-methods
try:
self.pre_delete(item)
except Exception as ex: # pylint: disable=broad-except
flash(str(ex), "danger")
logger.error("Pre-delete error: %s", str(ex))
else:
self._delete(item.id)
self.update_redirect()

View File

@@ -28,7 +28,6 @@ from urllib import parse
from flask import (
abort,
current_app as app,
flash,
g,
redirect,
request,
@@ -109,7 +108,6 @@ from superset.views.utils import (
get_form_data,
get_viz,
loads_request_json,
redirect_with_flash,
sanitize_datasource_data,
)
from superset.viz import BaseViz
@@ -415,7 +413,6 @@ class Superset(BaseSupersetView):
initial_form_data = {}
form_data_key = request.args.get("form_data_key")
if key is not None:
command = GetExplorePermalinkCommand(key)
try:
@@ -430,9 +427,10 @@ class Superset(BaseSupersetView):
_("Error: permalink state not found"), status=404
)
except (ChartNotFoundError, ExplorePermalinkGetFailedError) as ex:
flash(__("Error: %(msg)s", msg=ex.message), "danger")
return redirect(url_for("SliceModelView.list"))
elif form_data_key:
return json_error_response(
__("Error: %(msg)s", msg=ex.message), status=404
)
elif form_data_key := request.args.get("form_data_key"):
parameters = CommandParameters(key=form_data_key)
value = GetFormDataCommand(parameters).run()
initial_form_data = json.loads(value) if value else {}
@@ -442,18 +440,8 @@ class Superset(BaseSupersetView):
dataset_id = request.args.get("dataset_id")
if slice_id:
initial_form_data["slice_id"] = slice_id
if form_data_key:
flash(
_("Form data not found in cache, reverting to chart metadata.")
)
elif dataset_id:
initial_form_data["datasource"] = f"{dataset_id}__table"
if form_data_key:
flash(
_(
"Form data not found in cache, reverting to dataset metadata." # noqa: E501
)
)
form_data, slc = get_form_data(
use_slice_data=True, initial_form_data=initial_form_data
@@ -626,13 +614,9 @@ class Superset(BaseSupersetView):
if action == "saveas" and slice_add_perm:
ChartDAO.create(slc)
db.session.commit() # pylint: disable=consider-using-transaction
msg = _("Chart [{}] has been saved").format(slc.slice_name)
flash(msg, "success")
elif action == "overwrite" and slice_overwrite_perm:
ChartDAO.update(slc)
db.session.commit() # pylint: disable=consider-using-transaction
msg = _("Chart [{}] has been overwritten").format(slc.slice_name)
flash(msg, "success")
# Adding slice to a dashboard if requested
dash: Dashboard | None = None
@@ -654,13 +638,6 @@ class Superset(BaseSupersetView):
_("You don't have the rights to alter this dashboard"),
status=403,
)
flash(
_("Chart [{}] was added to dashboard [{}]").format(
slc.slice_name, dash.dashboard_title
),
"success",
)
elif new_dashboard_name:
# Creating and adding to a new dashboard
# check create dashboard permissions
@@ -675,12 +652,6 @@ class Superset(BaseSupersetView):
dashboard_title=request.args.get("new_dashboard_name"),
owners=[g.user] if g.user else [],
)
flash(
_(
"Dashboard [{}] just got created and chart [{}] was added to it"
).format(dash.dashboard_title, slc.slice_name),
"success",
)
if dash and slc not in dash.slices:
dash.slices.append(slc)
@@ -798,19 +769,9 @@ class Superset(BaseSupersetView):
try:
dashboard.raise_for_access()
except SupersetSecurityException as ex:
# anonymous users should get the login screen, others should go to dashboard list # noqa: E501
if g.user is None or g.user.is_anonymous:
redirect_url = f"{appbuilder.get_url_for_login}?next={request.url}"
warn_msg = "Users must be logged in to view this dashboard."
else:
redirect_url = url_for("DashboardModelView.list")
warn_msg = utils.error_msg_from_exception(ex)
return redirect_with_flash(
url=redirect_url,
message=warn_msg,
category="danger",
)
except SupersetSecurityException:
# Return 404 to avoid revealing dashboard existence
return Response(status=404)
add_extra_log_payload(
dashboard_id=dashboard.id,
dashboard_version="v2",
@@ -841,12 +802,8 @@ class Superset(BaseSupersetView):
) -> FlaskResponse:
try:
value = GetDashboardPermalinkCommand(key).run()
except DashboardPermalinkGetFailedError as ex:
flash(__("Error: %(msg)s", msg=ex.message), "danger")
return redirect(url_for("DashboardModelView.list"))
except DashboardAccessDeniedError as ex:
flash(__("Error: %(msg)s", msg=ex.message), "danger")
return redirect(url_for("DashboardModelView.list"))
except (DashboardPermalinkGetFailedError, DashboardAccessDeniedError) as ex:
return json_error_response(__("Error: %(msg)s", msg=ex.message), status=404)
if not value:
return json_error_response(_("permalink state not found"), status=404)

View File

@@ -22,12 +22,11 @@ from typing import Any, Callable, DefaultDict, Optional, Union
import msgpack
import pyarrow as pa
from flask import current_app as app, flash, g, has_request_context, redirect, request
from flask import current_app as app, g, has_request_context, request
from flask_appbuilder.security.sqla import models as ab_models
from flask_appbuilder.security.sqla.models import User
from flask_babel import _
from sqlalchemy.exc import NoResultFound
from werkzeug.wrappers.response import Response
from superset import dataframe, db, result_set, viz
from superset.common.db_query_status import QueryStatus
@@ -551,8 +550,3 @@ def get_cta_schema_name(
if not func:
return None
return func(database, user, schema, sql)
def redirect_with_flash(url: str, message: str, category: str) -> Response:
flash(message=message, category=category)
return redirect(url)

View File

@@ -108,7 +108,7 @@ class TestDashboardRoleBasedSecurity(BaseTestDashboardSecurity):
# act
response = self.get_dashboard_view_response(dashboard_to_access)
assert response.status_code == 302
assert response.status_code == 404
request_payload = get_query_context("birth_names")
rv = self.post_assert_metric(CHART_DATA_URI, request_payload, "data")
@@ -129,7 +129,7 @@ class TestDashboardRoleBasedSecurity(BaseTestDashboardSecurity):
response = self.get_dashboard_view_response(dashboard_to_access)
# assert
assert response.status_code == 302
assert response.status_code == 404
# post
revoke_access_to_dashboard(dashboard_to_access, new_role) # noqa: F405
@@ -147,9 +147,9 @@ class TestDashboardRoleBasedSecurity(BaseTestDashboardSecurity):
dashboard = create_dashboard_to_db(published=True, slices=[slice])
self.login(GAMMA_USERNAME)
# assert redirect on regular rbac access denied
# assert 404 on regular rbac access denied (prevents information leakage)
response = self.get_dashboard_view_response(dashboard)
assert response.status_code == 302
assert response.status_code == 404
request_payload = get_query_context("birth_names")
rv = self.post_assert_metric(CHART_DATA_URI, request_payload, "data")
@@ -221,7 +221,7 @@ class TestDashboardRoleBasedSecurity(BaseTestDashboardSecurity):
response = self.get_dashboard_view_response(dashboard_to_access)
# assert
assert response.status_code == 302
assert response.status_code == 404
@pytest.mark.usefixtures("public_role_like_gamma")
def test_get_dashboard_view__public_user_with_dashboard_permission_can_not_access_draft( # noqa: E501
@@ -234,7 +234,7 @@ class TestDashboardRoleBasedSecurity(BaseTestDashboardSecurity):
response = self.get_dashboard_view_response(dashboard_to_access)
# assert
assert response.status_code == 302
assert response.status_code == 404
# post
revoke_access_to_dashboard(dashboard_to_access, "Public") # noqa: F405