From c2885a166e961af62c0e15cd541133fc44b9669d Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" <70410625+michael-s-molina@users.noreply.github.com> Date: Tue, 12 Nov 2024 13:54:58 -0300 Subject: [PATCH 01/34] fix: Exception handling for SQL Lab views (#30897) --- superset/views/sql_lab/views.py | 285 ++++++++++++++------------ tests/integration_tests/core_tests.py | 1 - 2 files changed, 157 insertions(+), 129 deletions(-) diff --git a/superset/views/sql_lab/views.py b/superset/views/sql_lab/views.py index 29d0fdba758e9..29cf640d8c034 100644 --- a/superset/views/sql_lab/views.py +++ b/superset/views/sql_lab/views.py @@ -29,11 +29,12 @@ from superset.models.sql_lab import Query, SavedQuery, TableSchema, TabState from superset.superset_typing import FlaskResponse from superset.utils import json -from superset.utils.core import get_user_id +from superset.utils.core import error_msg_from_exception, get_user_id from superset.views.base import ( BaseSupersetView, DeleteMixin, DeprecateModelViewMixin, + json_error_response, json_success, SupersetModelView, ) @@ -84,48 +85,56 @@ class TabStateView(BaseSupersetView): @has_access_api @expose("/", methods=("POST",)) def post(self) -> FlaskResponse: - query_editor = json.loads(request.form["queryEditor"]) - tab_state = TabState( - user_id=get_user_id(), - # This is for backward compatibility - label=query_editor.get("name") - or query_editor.get("title", __("Untitled Query")), - active=True, - database_id=query_editor["dbId"], - catalog=query_editor.get("catalog"), - schema=query_editor.get("schema"), - sql=query_editor.get("sql", "SELECT ..."), - query_limit=query_editor.get("queryLimit"), - hide_left_bar=query_editor.get("hideLeftBar"), - saved_query_id=query_editor.get("remoteId"), - template_params=query_editor.get("templateParams"), - ) - ( - db.session.query(TabState) - .filter_by(user_id=get_user_id()) - .update({"active": False}) - ) - db.session.add(tab_state) - db.session.commit() - return json_success(json.dumps({"id": tab_state.id})) + try: + query_editor = json.loads(request.form["queryEditor"]) + tab_state = TabState( + user_id=get_user_id(), + # This is for backward compatibility + label=query_editor.get("name") + or query_editor.get("title", __("Untitled Query")), + active=True, + database_id=query_editor["dbId"], + catalog=query_editor.get("catalog"), + schema=query_editor.get("schema"), + sql=query_editor.get("sql", "SELECT ..."), + query_limit=query_editor.get("queryLimit"), + hide_left_bar=query_editor.get("hideLeftBar"), + saved_query_id=query_editor.get("remoteId"), + template_params=query_editor.get("templateParams"), + ) + ( + db.session.query(TabState) + .filter_by(user_id=get_user_id()) + .update({"active": False}) + ) + db.session.add(tab_state) + db.session.commit() + return json_success(json.dumps({"id": tab_state.id})) + except Exception as ex: # pylint: disable=broad-except + db.session.rollback() + return json_error_response(error_msg_from_exception(ex), 400) @has_access_api @expose("/", methods=("DELETE",)) def delete(self, tab_state_id: int) -> FlaskResponse: - owner_id = _get_owner_id(tab_state_id) - if owner_id is None: - return Response(status=404) - if owner_id != get_user_id(): - return Response(status=403) - - db.session.query(TabState).filter(TabState.id == tab_state_id).delete( - synchronize_session=False - ) - db.session.query(TableSchema).filter( - TableSchema.tab_state_id == tab_state_id - ).delete(synchronize_session=False) - db.session.commit() - return json_success(json.dumps("OK")) + try: + owner_id = _get_owner_id(tab_state_id) + if owner_id is None: + return Response(status=404) + if owner_id != get_user_id(): + return Response(status=403) + + db.session.query(TabState).filter(TabState.id == tab_state_id).delete( + synchronize_session=False + ) + db.session.query(TableSchema).filter( + TableSchema.tab_state_id == tab_state_id + ).delete(synchronize_session=False) + db.session.commit() + return json_success(json.dumps("OK")) + except Exception as ex: # pylint: disable=broad-except + db.session.rollback() + return json_error_response(error_msg_from_exception(ex), 400) @has_access_api @expose("/", methods=("GET",)) @@ -146,19 +155,23 @@ def get(self, tab_state_id: int) -> FlaskResponse: @has_access_api @expose("/activate", methods=("POST",)) def activate(self, tab_state_id: int) -> FlaskResponse: - owner_id = _get_owner_id(tab_state_id) - if owner_id is None: - return Response(status=404) - if owner_id != get_user_id(): - return Response(status=403) - - ( - db.session.query(TabState) - .filter_by(user_id=get_user_id()) - .update({"active": TabState.id == tab_state_id}) - ) - db.session.commit() - return json_success(json.dumps(tab_state_id)) + try: + owner_id = _get_owner_id(tab_state_id) + if owner_id is None: + return Response(status=404) + if owner_id != get_user_id(): + return Response(status=403) + + ( + db.session.query(TabState) + .filter_by(user_id=get_user_id()) + .update({"active": TabState.id == tab_state_id}) + ) + db.session.commit() + return json_success(json.dumps(tab_state_id)) + except Exception as ex: # pylint: disable=broad-except + db.session.rollback() + return json_error_response(error_msg_from_exception(ex), 400) @has_access_api @expose("", methods=("PUT",)) @@ -169,102 +182,118 @@ def put(self, tab_state_id: int) -> FlaskResponse: if owner_id != get_user_id(): return Response(status=403) - fields = {k: json.loads(v) for k, v in request.form.to_dict().items()} - if client_id := fields.get("latest_query_id"): - query = db.session.query(Query).filter_by(client_id=client_id).one_or_none() - if not query: - return self.json_response({"error": "Bad request"}, status=400) - db.session.query(TabState).filter_by(id=tab_state_id).update(fields) - db.session.commit() - return json_success(json.dumps(tab_state_id)) + try: + fields = {k: json.loads(v) for k, v in request.form.to_dict().items()} + db.session.query(TabState).filter_by(id=tab_state_id).update(fields) + db.session.commit() + return json_success(json.dumps(tab_state_id)) + except Exception as ex: # pylint: disable=broad-except + db.session.rollback() + return json_error_response(error_msg_from_exception(ex), 400) @has_access_api @expose("/migrate_query", methods=("POST",)) def migrate_query(self, tab_state_id: int) -> FlaskResponse: - owner_id = _get_owner_id(tab_state_id) - if owner_id is None: - return Response(status=404) - if owner_id != get_user_id(): - return Response(status=403) - - client_id = json.loads(request.form["queryId"]) - db.session.query(Query).filter_by(client_id=client_id).update( - {"sql_editor_id": tab_state_id} - ) - db.session.commit() - return json_success(json.dumps(tab_state_id)) + try: + owner_id = _get_owner_id(tab_state_id) + if owner_id is None: + return Response(status=404) + if owner_id != get_user_id(): + return Response(status=403) + + client_id = json.loads(request.form["queryId"]) + db.session.query(Query).filter_by(client_id=client_id).update( + {"sql_editor_id": tab_state_id} + ) + db.session.commit() + return json_success(json.dumps(tab_state_id)) + except Exception as ex: # pylint: disable=broad-except + db.session.rollback() + return json_error_response(error_msg_from_exception(ex), 400) @has_access_api @expose("/query/", methods=("DELETE",)) def delete_query(self, tab_state_id: int, client_id: str) -> FlaskResponse: - # Before deleting the query, ensure it's not tied to any - # active tab as the last query. If so, replace the query - # with the latest one created in that tab - tab_state_query = db.session.query(TabState).filter_by( - id=tab_state_id, latest_query_id=client_id - ) - if tab_state_query.count(): - query = ( - db.session.query(Query) - .filter( - and_( - Query.client_id != client_id, - Query.user_id == get_user_id(), - Query.sql_editor_id == str(tab_state_id), - ), - ) - .order_by(Query.id.desc()) - .first() - ) - tab_state_query.update( - {"latest_query_id": query.client_id if query else None} + try: + # Before deleting the query, ensure it's not tied to any + # active tab as the last query. If so, replace the query + # with the latest one created in that tab + tab_state_query = db.session.query(TabState).filter_by( + id=tab_state_id, latest_query_id=client_id ) + if tab_state_query.count(): + query = ( + db.session.query(Query) + .filter( + and_( + Query.client_id != client_id, + Query.user_id == get_user_id(), + Query.sql_editor_id == str(tab_state_id), + ), + ) + .order_by(Query.id.desc()) + .first() + ) + tab_state_query.update( + {"latest_query_id": query.client_id if query else None} + ) - db.session.query(Query).filter_by( - client_id=client_id, - user_id=get_user_id(), - sql_editor_id=str(tab_state_id), - ).delete(synchronize_session=False) - db.session.commit() - return json_success(json.dumps("OK")) + db.session.query(Query).filter_by( + client_id=client_id, + user_id=get_user_id(), + sql_editor_id=str(tab_state_id), + ).delete(synchronize_session=False) + db.session.commit() + return json_success(json.dumps("OK")) + except Exception as ex: # pylint: disable=broad-except + db.session.rollback() + return json_error_response(error_msg_from_exception(ex), 400) class TableSchemaView(BaseSupersetView): @has_access_api @expose("/", methods=("POST",)) def post(self) -> FlaskResponse: - table = json.loads(request.form["table"]) - - # delete any existing table schema - db.session.query(TableSchema).filter( - TableSchema.tab_state_id == table["queryEditorId"], - TableSchema.database_id == table["dbId"], - TableSchema.catalog == table.get("catalog"), - TableSchema.schema == table["schema"], - TableSchema.table == table["name"], - ).delete(synchronize_session=False) - - table_schema = TableSchema( - tab_state_id=table["queryEditorId"], - database_id=table["dbId"], - catalog=table.get("catalog"), - schema=table["schema"], - table=table["name"], - description=json.dumps(table), - expanded=True, - ) - db.session.add(table_schema) - db.session.commit() - return json_success(json.dumps({"id": table_schema.id})) + try: + table = json.loads(request.form["table"]) + + # delete any existing table schema + db.session.query(TableSchema).filter( + TableSchema.tab_state_id == table["queryEditorId"], + TableSchema.database_id == table["dbId"], + TableSchema.catalog == table.get("catalog"), + TableSchema.schema == table["schema"], + TableSchema.table == table["name"], + ).delete(synchronize_session=False) + + table_schema = TableSchema( + tab_state_id=table["queryEditorId"], + database_id=table["dbId"], + catalog=table.get("catalog"), + schema=table["schema"], + table=table["name"], + description=json.dumps(table), + expanded=True, + ) + db.session.add(table_schema) + db.session.commit() + return json_success(json.dumps({"id": table_schema.id})) + except Exception as ex: # pylint: disable=broad-except + db.session.rollback() + return json_error_response(error_msg_from_exception(ex), 400) @has_access_api @expose("/", methods=("DELETE",)) def delete(self, table_schema_id: int) -> FlaskResponse: - db.session.query(TableSchema).filter(TableSchema.id == table_schema_id).delete( - synchronize_session=False - ) - db.session.commit() - return json_success(json.dumps("OK")) + try: + db.session.query(TableSchema).filter( + TableSchema.id == table_schema_id + ).delete(synchronize_session=False) + db.session.commit() + return json_success(json.dumps("OK")) + except Exception as ex: # pylint: disable=broad-except + db.session.rollback() + return json_error_response(error_msg_from_exception(ex), 400) @has_access_api @expose("//expanded", methods=("POST",)) diff --git a/tests/integration_tests/core_tests.py b/tests/integration_tests/core_tests.py index 4f989611b2a29..e100d10da613e 100644 --- a/tests/integration_tests/core_tests.py +++ b/tests/integration_tests/core_tests.py @@ -1013,7 +1013,6 @@ def test_tabstate_update(self): data = {"sql": json.dumps("select 1"), "latest_query_id": json.dumps(client_id)} response = self.client.put(f"/tabstateview/{tab_state_id}", data=data) assert response.status_code == 400 - assert response.json["error"] == "Bad request" # generate query db.session.add(Query(client_id=client_id, database_id=1)) db.session.commit() From ac3a10d8f192520580b8ce545cf418dc7928d27c Mon Sep 17 00:00:00 2001 From: Joe Li Date: Tue, 12 Nov 2024 09:54:19 -0800 Subject: [PATCH 02/34] fix: don't show metadata for embedded dashboards (#30875) --- .../components/Header/Header.test.tsx | 45 +++++++++++++++++++ .../src/dashboard/components/Header/index.jsx | 3 +- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/dashboard/components/Header/Header.test.tsx b/superset-frontend/src/dashboard/components/Header/Header.test.tsx index 4c9f7bd0139bd..9a2c414d00f7a 100644 --- a/superset-frontend/src/dashboard/components/Header/Header.test.tsx +++ b/superset-frontend/src/dashboard/components/Header/Header.test.tsx @@ -373,3 +373,48 @@ test('should render an extension component if one is supplied', () => { screen.getByText('dashboard.nav.right extension component'), ).toBeInTheDocument(); }); + +test('should NOT render MetadataBar when in edit mode', () => { + const mockedProps = { + ...createProps(), + editMode: true, + dashboardInfo: { + ...createProps().dashboardInfo, + userId: '123', + }, + }; + setup(mockedProps); + expect( + screen.queryByText(mockedProps.dashboardInfo.changed_on_delta_humanized), + ).not.toBeInTheDocument(); +}); + +test('should NOT render MetadataBar when embedded', () => { + const mockedProps = { + ...createProps(), + editMode: false, + dashboardInfo: { + ...createProps().dashboardInfo, + userId: undefined, + }, + }; + setup(mockedProps); + expect( + screen.queryByText(mockedProps.dashboardInfo.changed_on_delta_humanized), + ).not.toBeInTheDocument(); +}); + +test('should render MetadataBar when not in edit mode and not embedded', () => { + const mockedProps = { + ...createProps(), + editMode: false, + dashboardInfo: { + ...createProps().dashboardInfo, + userId: '123', + }, + }; + setup(mockedProps); + expect( + screen.getByText(mockedProps.dashboardInfo.changed_on_delta_humanized), + ).toBeInTheDocument(); +}); diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx index 9470f875ffd6f..39dcff8d4c0d0 100644 --- a/superset-frontend/src/dashboard/components/Header/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/index.jsx @@ -496,6 +496,7 @@ class Header extends PureComponent { const refreshWarning = dashboardInfo.common?.conf ?.SUPERSET_DASHBOARD_PERIODICAL_REFRESH_WARNING_MESSAGE; + const isEmbedded = !dashboardInfo?.userId; const handleOnPropertiesChange = updates => { const { dashboardInfoChanged, dashboardTitleChanged } = this.props; @@ -553,7 +554,7 @@ class Header extends PureComponent { visible={!editMode} /> ), - !editMode && ( + !editMode && !isEmbedded && ( Date: Tue, 12 Nov 2024 09:54:33 -0800 Subject: [PATCH 03/34] feat: add logging durations for screenshot async service (#30884) --- superset/utils/screenshots.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/superset/utils/screenshots.py b/superset/utils/screenshots.py index b31d9c0e4412b..96c0f40d6da51 100644 --- a/superset/utils/screenshots.py +++ b/superset/utils/screenshots.py @@ -24,6 +24,7 @@ from superset import feature_flag_manager from superset.dashboards.permalink.types import DashboardPermalinkState +from superset.extensions import event_logger from superset.utils.hashing import md5_sha_from_dict from superset.utils.urls import modify_url_query from superset.utils.webdriver import ( @@ -91,7 +92,8 @@ def get_screenshot( self, user: User, window_size: WindowSize | None = None ) -> bytes | None: driver = self.driver(window_size) - self.screenshot = driver.get_screenshot(self.url, self.element, user) + with event_logger.log_context("screenshot", screenshot_url=self.url): + self.screenshot = driver.get_screenshot(self.url, self.element, user) return self.screenshot def get( @@ -169,7 +171,10 @@ def compute_and_cache( # pylint: disable=too-many-arguments # Assuming all sorts of things can go wrong with Selenium try: - payload = self.get_screenshot(user=user, window_size=window_size) + with event_logger.log_context( + f"screenshot.compute.{self.thumbnail_type}", force=force + ): + payload = self.get_screenshot(user=user, window_size=window_size) except Exception as ex: # pylint: disable=broad-except logger.warning("Failed at generating thumbnail %s", ex, exc_info=True) @@ -182,7 +187,10 @@ def compute_and_cache( # pylint: disable=too-many-arguments if payload: logger.info("Caching thumbnail: %s", cache_key) - cache.set(cache_key, payload) + with event_logger.log_context( + f"screenshot.cache.{self.thumbnail_type}", force=force + ): + cache.set(cache_key, payload) logger.info("Done caching thumbnail") return payload From 6e665c3e07c1c585a244e83d75a3cc7f541b37fe Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Wed, 13 Nov 2024 13:13:43 +0300 Subject: [PATCH 04/34] refactor(Avatar): Migrate Avatar to Ant Design 5 (#30740) --- .../src/components/Avatar/Avatar.stories.tsx | 42 +++++++++++++++++++ .../src/components/Avatar/Avatar.test.tsx | 26 ++++++++++++ .../src/components/Avatar/index.tsx | 31 ++++++++++++++ .../src/components/FacePile/index.tsx | 32 +++----------- .../src/components/Tooltip/index.tsx | 18 +------- superset-frontend/src/components/index.ts | 1 - superset-frontend/src/theme/index.ts | 8 ++++ 7 files changed, 113 insertions(+), 45 deletions(-) create mode 100644 superset-frontend/src/components/Avatar/Avatar.stories.tsx create mode 100644 superset-frontend/src/components/Avatar/Avatar.test.tsx create mode 100644 superset-frontend/src/components/Avatar/index.tsx diff --git a/superset-frontend/src/components/Avatar/Avatar.stories.tsx b/superset-frontend/src/components/Avatar/Avatar.stories.tsx new file mode 100644 index 0000000000000..d9b6a5bcce59c --- /dev/null +++ b/superset-frontend/src/components/Avatar/Avatar.stories.tsx @@ -0,0 +1,42 @@ +/** + * 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 { Avatar, AvatarProps } from '.'; + +export default { + title: 'Avatar', + component: Avatar, +}; + +export const InteractiveAvatar = (args: AvatarProps) => ; + +InteractiveAvatar.args = { + alt: '', + gap: 4, + shape: 'circle', + size: 'default', + src: '', + draggable: false, +}; + +InteractiveAvatar.argTypes = { + shape: { + options: ['circle', 'square'], + control: { type: 'select' }, + }, +}; diff --git a/superset-frontend/src/components/Avatar/Avatar.test.tsx b/superset-frontend/src/components/Avatar/Avatar.test.tsx new file mode 100644 index 0000000000000..91cf1ef5e795b --- /dev/null +++ b/superset-frontend/src/components/Avatar/Avatar.test.tsx @@ -0,0 +1,26 @@ +/** + * 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 } from 'spec/helpers/testing-library'; +import { Avatar } from 'src/components/Avatar'; + +test('renders with default props', async () => { + const { container } = render(); + + expect(container).toBeInTheDocument(); +}); diff --git a/superset-frontend/src/components/Avatar/index.tsx b/superset-frontend/src/components/Avatar/index.tsx new file mode 100644 index 0000000000000..910c3eeada666 --- /dev/null +++ b/superset-frontend/src/components/Avatar/index.tsx @@ -0,0 +1,31 @@ +/** + * 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 { Avatar as AntdAvatar } from 'antd-v5'; +import { AvatarProps, GroupProps } from 'antd-v5/lib/avatar'; + +export function Avatar(props: AvatarProps) { + return ; +} + +export function AvatarGroup(props: GroupProps) { + return ; +} + +export type { AvatarProps, GroupProps }; diff --git a/superset-frontend/src/components/FacePile/index.tsx b/superset-frontend/src/components/FacePile/index.tsx index b5586304d3649..b4c12c48f4756 100644 --- a/superset-frontend/src/components/FacePile/index.tsx +++ b/superset-frontend/src/components/FacePile/index.tsx @@ -19,14 +19,12 @@ import type Owner from 'src/types/Owner'; import { getCategoricalSchemeRegistry, - styled, isFeatureEnabled, FeatureFlag, - SupersetTheme, } from '@superset-ui/core'; import getOwnerName from 'src/utils/getOwnerName'; import { Tooltip } from 'src/components/Tooltip'; -import { Avatar } from 'src/components'; +import { Avatar, AvatarGroup } from 'src/components/Avatar'; import { getRandomColor } from './utils'; interface FacePileProps { @@ -36,29 +34,9 @@ interface FacePileProps { const colorList = getCategoricalSchemeRegistry().get()?.colors ?? []; -const customAvatarStyler = (theme: SupersetTheme) => { - const size = theme.gridUnit * 8; - return ` - width: ${size}px; - height: ${size}px; - line-height: ${size}px; - font-size: ${theme.typography.sizes.s}px;`; -}; - -const StyledAvatar = styled(Avatar)` - ${({ theme }) => customAvatarStyler(theme)} -`; - -// to apply styling to the maxCount avatar -const StyledGroup = styled(Avatar.Group)` - .ant-avatar { - ${({ theme }) => customAvatarStyler(theme)} - } -`; - export default function FacePile({ users, maxCount = 4 }: FacePileProps) { return ( - + {users.map(user => { const { first_name, last_name, id } = user; const name = getOwnerName(user); @@ -69,7 +47,7 @@ export default function FacePile({ users, maxCount = 4 }: FacePileProps) { : undefined; return ( - {first_name?.[0]?.toLocaleUpperCase()} {last_name?.[0]?.toLocaleUpperCase()} - + ); })} - + ); } diff --git a/superset-frontend/src/components/Tooltip/index.tsx b/superset-frontend/src/components/Tooltip/index.tsx index 8752a47acda93..86c616bf959ba 100644 --- a/superset-frontend/src/components/Tooltip/index.tsx +++ b/superset-frontend/src/components/Tooltip/index.tsx @@ -16,13 +16,12 @@ * specific language governing permissions and limitations * under the License. */ -import { useTheme, css } from '@superset-ui/core'; +import { useTheme } from '@superset-ui/core'; import { Tooltip as AntdTooltip } from 'antd'; import { TooltipProps, TooltipPlacement as AntdTooltipPlacement, } from 'antd/lib/tooltip'; -import { Global } from '@emotion/react'; export type TooltipPlacement = AntdTooltipPlacement; @@ -30,21 +29,6 @@ export const Tooltip = (props: TooltipProps) => { const theme = useTheme(); return ( <> - {/* Safari hack to hide browser default tooltips */} - p { - margin: 0; - } - `} - /> Date: Wed, 13 Nov 2024 14:07:45 +0300 Subject: [PATCH 05/34] refactor(input): Migrate Input component to Ant Design 5 (#30730) --- .../cypress/e2e/explore/control.test.ts | 2 +- superset-frontend/src/GlobalStyles.tsx | 2 +- .../Chart/DrillBy/DrillByMenuItems.tsx | 28 ++-- .../Datasource/ChangeDatasourceModal.tsx | 4 +- .../src/components/Input/Input.stories.tsx | 138 ++++++++++++++++++ .../src/components/Input/Input.test.tsx | 35 +++++ .../src/components/Input/index.tsx | 19 +-- .../controls/BoundsControl.test.jsx | 3 +- .../components/controls/BoundsControl.tsx | 8 +- .../DateFilterControl/DateFilterLabel.tsx | 2 +- .../VizTypeControl/VizTypeGallery.tsx | 2 +- .../src/features/alerts/AlertReportModal.tsx | 10 +- .../components/AlertReportCronScheduler.tsx | 13 +- .../features/rls/RowLevelSecurityModal.tsx | 2 +- superset-frontend/src/theme/index.ts | 14 ++ 15 files changed, 237 insertions(+), 45 deletions(-) create mode 100644 superset-frontend/src/components/Input/Input.stories.tsx create mode 100644 superset-frontend/src/components/Input/Input.test.tsx diff --git a/superset-frontend/cypress-base/cypress/e2e/explore/control.test.ts b/superset-frontend/cypress-base/cypress/e2e/explore/control.test.ts index ccdfcd4512c7d..5fd49efa01908 100644 --- a/superset-frontend/cypress-base/cypress/e2e/explore/control.test.ts +++ b/superset-frontend/cypress-base/cypress/e2e/explore/control.test.ts @@ -234,7 +234,7 @@ describe('Time range filter', () => { cy.get('[data-test=time-range-trigger]').click(); cy.get('[data-test=custom-frame]').then(() => { - cy.get('.ant-input-number-input-wrap > input') + cy.get('.antd5-input-number-input-wrap > input') .invoke('attr', 'value') .should('eq', '7'); }); diff --git a/superset-frontend/src/GlobalStyles.tsx b/superset-frontend/src/GlobalStyles.tsx index b9cb0e55013e0..b0b57133dac15 100644 --- a/superset-frontend/src/GlobalStyles.tsx +++ b/superset-frontend/src/GlobalStyles.tsx @@ -70,7 +70,7 @@ export const GlobalStyles = () => ( } } .column-config-popover { - & .ant-input-number { + & .antd5-input-number { width: 100%; } && .btn-group svg { diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx index b80417a130079..47666db7802f0 100644 --- a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx +++ b/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx @@ -45,7 +45,6 @@ import { import rison from 'rison'; import { debounce } from 'lodash'; import { FixedSizeList as List } from 'react-window'; -import { AntdInput } from 'src/components'; import Icons from 'src/components/Icons'; import { Input } from 'src/components/Input'; import { useToasts } from 'src/components/MessageToasts/withToasts'; @@ -55,6 +54,7 @@ import { supersetGetCache, } from 'src/utils/cachedSupersetGet'; import { useVerboseMap } from 'src/hooks/apiResources/datasets'; +import { InputRef } from 'antd-v5'; import { MenuItemTooltip } from '../DisabledMenuItemTooltip'; import DrillByModal from './DrillByModal'; import { getSubmenuYOffset } from '../utils'; @@ -114,11 +114,12 @@ export const DrillByMenuItems = ({ const { addDangerToast } = useToasts(); const [isLoadingColumns, setIsLoadingColumns] = useState(true); const [searchInput, setSearchInput] = useState(''); + const [debouncedSearchInput, setDebouncedSearchInput] = useState(''); const [dataset, setDataset] = useState(); const [columns, setColumns] = useState([]); const [showModal, setShowModal] = useState(false); const [currentColumn, setCurrentColumn] = useState(); - const ref = useRef(null); + const ref = useRef(null); const showSearch = loadDrillByOptions || columns.length > SHOW_COLUMNS_SEARCH_THRESHOLD; const handleSelection = useCallback( @@ -138,11 +139,11 @@ export const DrillByMenuItems = ({ useEffect(() => { if (open) { - ref.current?.input.focus({ preventScroll: true }); + ref.current?.input?.focus({ preventScroll: true }); } else { // Reset search input when menu is closed - ref.current?.setValue(''); setSearchInput(''); + setDebouncedSearchInput(''); } }, [open]); @@ -207,19 +208,27 @@ export const DrillByMenuItems = ({ hasDrillBy, ]); - const handleInput = debounce( - (value: string) => setSearchInput(value), - FAST_DEBOUNCE, + const debouncedSetSearchInput = useMemo( + () => + debounce((value: string) => { + setDebouncedSearchInput(value); + }, FAST_DEBOUNCE), + [], ); + const handleInput = (value: string) => { + setSearchInput(value); + debouncedSetSearchInput(value); + }; + const filteredColumns = useMemo( () => columns.filter(column => (column.verbose_name || column.column_name) .toLowerCase() - .includes(searchInput.toLowerCase()), + .includes(debouncedSearchInput.toLowerCase()), ), - [columns, searchInput], + [columns, debouncedSearchInput], ); const submenuYOffset = useMemo( @@ -311,6 +320,7 @@ export const DrillByMenuItems = ({ margin: ${theme.gridUnit * 2}px ${theme.gridUnit * 3}px; box-shadow: none; `} + value={searchInput} /> )} {isLoadingColumns ? ( diff --git a/superset-frontend/src/components/Datasource/ChangeDatasourceModal.tsx b/superset-frontend/src/components/Datasource/ChangeDatasourceModal.tsx index ed39a362229b6..e02ad68a7f145 100644 --- a/superset-frontend/src/components/Datasource/ChangeDatasourceModal.tsx +++ b/superset-frontend/src/components/Datasource/ChangeDatasourceModal.tsx @@ -41,13 +41,13 @@ import Dataset from 'src/types/Dataset'; import { useDebouncedEffect } from 'src/explore/exploreUtils'; import { SLOW_DEBOUNCE } from 'src/constants'; import Loading from 'src/components/Loading'; -import { AntdInput } from 'src/components'; import { Input } from 'src/components/Input'; import { PAGE_SIZE as DATASET_PAGE_SIZE, SORT_BY as DATASET_SORT_BY, } from 'src/features/datasets/constants'; import withToasts from 'src/components/MessageToasts/withToasts'; +import { InputRef } from 'antd-v5'; import FacePile from '../FacePile'; const CONFIRM_WARNING_MESSAGE = t( @@ -115,7 +115,7 @@ const ChangeDatasourceModal: FunctionComponent = ({ const [sortBy, setSortBy] = useState(DATASET_SORT_BY); const [confirmChange, setConfirmChange] = useState(false); const [confirmedDataset, setConfirmedDataset] = useState(); - const searchRef = useRef(null); + const searchRef = useRef(null); const { state: { loading, resourceCollection, resourceCount }, diff --git a/superset-frontend/src/components/Input/Input.stories.tsx b/superset-frontend/src/components/Input/Input.stories.tsx new file mode 100644 index 0000000000000..4023c53d97e0f --- /dev/null +++ b/superset-frontend/src/components/Input/Input.stories.tsx @@ -0,0 +1,138 @@ +/** + * 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 { InputProps, TextAreaProps } from 'antd-v5/lib/input'; +import { InputNumberProps } from 'antd-v5/lib/input-number'; +import { AntdThemeProvider } from 'src/components/AntdThemeProvider'; +import { Input, TextArea, InputNumber } from '.'; + +export default { + title: 'Input', + component: Input, +}; + +export const InteractiveInput = (args: InputProps) => ( + + + +); + +export const InteractiveInputNumber = (args: InputNumberProps) => ( + + + +); + +export const InteractiveTextArea = (args: TextAreaProps) => ( + +