diff --git a/.github/workflows/bump-python-package.yml b/.github/workflows/bump-python-package.yml index f3356bf6ea26..50cc0d2a805c 100644 --- a/.github/workflows/bump-python-package.yml +++ b/.github/workflows/bump-python-package.yml @@ -48,7 +48,7 @@ jobs: python-version: "3.11" - name: Install uv - uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0 + uses: astral-sh/setup-uv@ae62891fec2bb8e7d6c99fc78c9fec3a63790f8d # v10.0.0 with: python-version: "3.11" enable-cache: true diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index 6b48d65a953a..357098f954cf 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -20612,7 +20612,7 @@ "version": "0.8.0", "resolved": "https://registry.npmjs.org/expect-playwright/-/expect-playwright-0.8.0.tgz", "integrity": "sha512-+kn8561vHAY+dt+0gMqqj1oY+g5xWrsuGMk4QGxotT2WS545nVqqjs37z6hrYfIuucwqthzwJfCJUEYqixyljg==", - "deprecated": "⚠️ The 'expect-playwright' package is deprecated. The Playwright core assertions (via @playwright/test) now cover the same functionality. Please migrate to built-in expect. See https://playwright.dev/docs/test-assertions for migration.", + "deprecated": "\u26a0\ufe0f The 'expect-playwright' package is deprecated. The Playwright core assertions (via @playwright/test) now cover the same functionality. Please migrate to built-in expect. See https://playwright.dev/docs/test-assertions for migration.", "dev": true, "license": "MIT" }, @@ -26023,7 +26023,7 @@ "version": "0.4.0", "resolved": "https://registry.npmjs.org/jest-process-manager/-/jest-process-manager-0.4.0.tgz", "integrity": "sha512-80Y6snDyb0p8GG83pDxGI/kQzwVTkCxc7ep5FPe/F6JYdvRDhwr6RzRmPSP7SEwuLhxo80lBS/NqOdUIbHIfhw==", - "deprecated": "⚠️ The 'jest-process-manager' package is deprecated. Please migrate to Playwright's built-in test runner (@playwright/test) which now includes full Jest-style features and parallel testing. See https://playwright.dev/docs/intro for details.", + "deprecated": "\u26a0\ufe0f The 'jest-process-manager' package is deprecated. Please migrate to Playwright's built-in test runner (@playwright/test) which now includes full Jest-style features and parallel testing. See https://playwright.dev/docs/intro for details.", "dev": true, "license": "MIT", "dependencies": { @@ -43429,22 +43429,13 @@ "license": "Apache-2.0", "dependencies": { "d3": "^3.5.17", - "prop-types": "^15.8.1", - "react": "^19.2.7" + "prop-types": "^15.8.1" }, "peerDependencies": { "@apache-superset/core": "*", "@superset-ui/chart-controls": "*", - "@superset-ui/core": "*" - } - }, - "plugins/plugin-chart-chord/node_modules/react": { - "version": "19.2.8", - "resolved": "https://registry.npmjs.org/react/-/react-19.2.8.tgz", - "integrity": "sha512-PWaYA1L/q9u2u7xYQi+Y3L3Yfnie7XyLeaJICV1MGD6LprsBxcAqGjYyr0eY3p+QdsA+x/Irkt4Qif8D63+Sbw==", - "license": "MIT", - "engines": { - "node": ">=0.10.0" + "@superset-ui/core": "*", + "react": "^18.3.0" } }, "plugins/plugin-chart-country-map": { diff --git a/superset-frontend/playwright.config.ts b/superset-frontend/playwright.config.ts index 6b67bb3975b3..edbe94b326a6 100644 --- a/superset-frontend/playwright.config.ts +++ b/superset-frontend/playwright.config.ts @@ -47,6 +47,10 @@ export default defineConfig({ // Retry logic - 2 retries in CI, 0 locally retries: process.env.CI ? 2 : 0, + // Disable capturing Git commit info as the project's history is increasingly dense + // and breach Playwright's default 3-seconds `git` command timeout limit + captureGitInfo: { commit: false, diff: false }, + // Reporter configuration - multiple reporters for better visibility reporter: process.env.CI ? [ diff --git a/superset-frontend/plugins/plugin-chart-chord/package.json b/superset-frontend/plugins/plugin-chart-chord/package.json index 2570e2372120..c1972537c56e 100644 --- a/superset-frontend/plugins/plugin-chart-chord/package.json +++ b/superset-frontend/plugins/plugin-chart-chord/package.json @@ -30,12 +30,12 @@ }, "dependencies": { "d3": "^3.5.17", - "prop-types": "^15.8.1", - "react": "^19.2.7" + "prop-types": "^15.8.1" }, "peerDependencies": { "@apache-superset/core": "*", "@superset-ui/chart-controls": "*", - "@superset-ui/core": "*" + "@superset-ui/core": "*", + "react": "^18.3.0" } } diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.test.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.test.tsx index e201d96176c1..dd7a495ee89a 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.test.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.test.tsx @@ -17,6 +17,10 @@ * under the License. */ +import { getNumberFormatter } from '@superset-ui/core'; +import { render, fireEvent } from '../../../../spec/helpers/testing-library'; +import BigNumberVis from './BigNumberViz'; + /** * Tests for the color threshold formatter logic in BigNumberViz. * @@ -83,3 +87,33 @@ describe('BigNumberViz color formatters', () => { expect(getColorFromValue).not.toHaveBeenCalled(); }); }); + +describe('BigNumberViz context menu', () => { + test('invokes onContextMenu and stops the event bubbling to ancestor handlers', () => { + const onContextMenu = jest.fn(); + const ancestorHandler = jest.fn(); + + const { container } = render( +
+ +
, + ); + + const headerLine = container.querySelector('.header-line'); + fireEvent.contextMenu(headerLine!, { clientX: 10, clientY: 20 }); + + expect(onContextMenu).toHaveBeenCalledWith(10, 20); + expect(ancestorHandler).not.toHaveBeenCalled(); + }); +}); diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx index dc7d3cd2e856..ea4e89cd0ee7 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx @@ -224,6 +224,7 @@ function BigNumberVis({ const handleContextMenu = (e: MouseEvent) => { if (onContextMenu) { e.preventDefault(); + e.stopPropagation(); onContextMenu(e.nativeEvent.clientX, e.nativeEvent.clientY); } }; diff --git a/superset-frontend/src/dashboard/components/SliceAdder.tsx b/superset-frontend/src/dashboard/components/SliceAdder.tsx index f260bc0a2d58..690d1ad6d4de 100644 --- a/superset-frontend/src/dashboard/components/SliceAdder.tsx +++ b/superset-frontend/src/dashboard/components/SliceAdder.tsx @@ -468,7 +468,7 @@ function SliceAdder({ {({ height, width }: { height: number; width: number }) => ( { }); expect(adhocFilter.comparator).toBe(undefined); }); + // Charts saved before #32701 persisted `==` as the operation for IS_TRUE and + // IS_FALSE, alongside a boolean comparator. `translateToSql` and the backend + // both key off `operator`, so dropping the comparator would render such a + // filter as `col =` and query it as `col IS NULL`. + test('keeps the legacy boolean comparator for IS_TRUE', () => { + const adhocFilter = new AdhocFilter({ + expressionType: ExpressionTypes.Simple, + subject: 'col', + operator: '==', + operatorId: Operators.IsTrue, + comparator: true, + clause: Clauses.Where, + }); + expect(adhocFilter.operator).toBe('=='); + expect(adhocFilter.comparator).toBe(true); + expect(adhocFilter.translateToSql()).toBe("col = 'TRUE'"); + }); + test('keeps the legacy boolean comparator for IS_FALSE', () => { + const adhocFilter = new AdhocFilter({ + expressionType: ExpressionTypes.Simple, + subject: 'col', + operator: '==', + operatorId: Operators.IsFalse, + comparator: false, + clause: Clauses.Where, + }); + expect(adhocFilter.operator).toBe('=='); + expect(adhocFilter.comparator).toBe(false); + expect(adhocFilter.translateToSql()).toBe("col = 'FALSE'"); + }); + test('restores the boolean even when the stored comparator is missing', () => { + const adhocFilter = new AdhocFilter({ + expressionType: ExpressionTypes.Simple, + subject: 'col', + operator: '==', + operatorId: Operators.IsTrue, + clause: Clauses.Where, + }); + expect(adhocFilter.comparator).toBe(true); + }); + test('keeps a legacy boolean filter intact when the control re-posts it', () => { + const stored = { + expressionType: ExpressionTypes.Simple, + subject: 'col', + operator: '==', + operatorId: Operators.IsTrue, + comparator: true, + clause: Clauses.Where, + }; + // DndFilterSelect wraps props.value and hands those instances to onChange + const posted = JSON.parse(JSON.stringify(new AdhocFilter(stored))); + expect(posted.operator).toBe('=='); + expect(posted.comparator).toBe(true); + expect(posted.operatorId).toBe(Operators.IsTrue); + }); + test('leaves a genuine equality filter on a boolean value alone', () => { + const adhocFilter = new AdhocFilter({ + expressionType: ExpressionTypes.Simple, + subject: 'col', + operator: '==', + operatorId: Operators.Equals, + comparator: true, + clause: Clauses.Where, + }); + expect(adhocFilter.operator).toBe('=='); + expect(adhocFilter.comparator).toBe(true); + expect(adhocFilter.translateToSql()).toBe("col = 'TRUE'"); + }); test('sets the label properly if subject is a string', () => { const adhocFilter = new AdhocFilter({ expressionType: ExpressionTypes.Simple, diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.ts b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.ts index 31c544ffc4a3..649b8135ba37 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.ts +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.ts @@ -30,6 +30,15 @@ const CUSTOM_OPERATIONS = [...CUSTOM_OPERATORS].map( op => OPERATOR_ENUM_TO_OPERATOR_TYPE[op].operation, ); +// Charts saved before #32701 store `==` for IS_TRUE/IS_FALSE with the boolean +// in the comparator; blanking it makes them query `col IS NULL`. Restoring it +// leaves the emitted SQL untouched -- reconciling `operator` to `IS TRUE` +// would not, and Druid rejects that predicate on VARCHAR columns. +const LEGACY_BOOLEAN_COMPARATORS = new Map([ + [Operators.IsTrue, true], + [Operators.IsFalse, false], +]); + interface AdhocFilterInput { expressionType?: string; subject?: string | { column_name?: string; [key: string]: unknown } | null; @@ -77,6 +86,16 @@ export default class AdhocFilter { ) { this.comparator = undefined; } + if ( + this.operator === + OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.Equals].operation && + adhocFilter.operatorId && + LEGACY_BOOLEAN_COMPARATORS.has(adhocFilter.operatorId) + ) { + this.comparator = LEGACY_BOOLEAN_COMPARATORS.get( + adhocFilter.operatorId, + ); + } this.clause = adhocFilter.clause || Clauses.Where; this.sqlExpression = null; } else if (this.expressionType === ExpressionTypes.Sql) { diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx index b86c8f526e04..f9687b491723 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx @@ -515,6 +515,28 @@ test('will not display boolean operators when column type is string', () => { }); }); +test.each(['STRING', 'DATE'])( + 'will not display boolean operators when an expression column declares type %s', + type => { + const props = setup({ + datasource: { + type: 'table' as const, + datasource_name: 'table1', + schema: 'schema', + columns: [{ column_name: 'value', type, expression: '"value"' }], + }, + adhocFilter: simpleAdhocFilter, + }); + const { isOperatorRelevant } = useSimpleTabFilterProps( + props as unknown as Props, + ); + const booleanOnlyOperators = [Operators.IsTrue, Operators.IsFalse]; + booleanOnlyOperators.forEach(operator => { + expect(isOperatorRelevant(operator, 'value')).toBe(false); + }); + }, +); + test('will display boolean operators when column is an expression', () => { const props = setup({ datasource: { diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx index 4e0e7eb21456..c093e703acf2 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx @@ -160,7 +160,11 @@ export const useSimpleTabFilterProps = (props: Props) => { ].includes(operator); } if (operator === Operators.IsTrue || operator === Operators.IsFalse) { - return isColumnBoolean || isColumnNumber || isColumnFunction; + // An expression column may evaluate to a boolean, but that is only a + // safe assumption while its type is unknown; a declared type wins. + return ( + isColumnBoolean || isColumnNumber || (isColumnFunction && !column?.type) + ); } if (isColumnBoolean) { return operator === Operators.IsNull || operator === Operators.IsNotNull; diff --git a/superset-frontend/src/features/users/UserListModal.tsx b/superset-frontend/src/features/users/UserListModal.tsx index ca398cb7ea7b..045b727ac9aa 100644 --- a/superset-frontend/src/features/users/UserListModal.tsx +++ b/superset-frontend/src/features/users/UserListModal.tsx @@ -31,7 +31,12 @@ import { import { Group, Role, UserObject } from 'src/pages/UsersList/types'; import { Actions } from 'src/constants'; import { BaseUserListModalProps, FormValues } from './types'; -import { createUser, updateUser, atLeastOneRoleOrGroup } from './utils'; +import { + createUser, + updateUser, + atLeastOneRoleOrGroup, + handleUserError, +} from './utils'; export interface UserModalProps extends BaseUserListModalProps { roles: Role[]; @@ -51,36 +56,6 @@ function UserListModal({ }: UserModalProps) { const { addDangerToast, addSuccessToast } = useToasts(); const handleFormSubmit = async (values: FormValues) => { - const handleError = async ( - err: any, - action: Actions.CREATE | Actions.UPDATE, - ) => { - let errorMessage = - action === Actions.CREATE - ? t('There was an error creating the user. Please, try again.') - : t('There was an error updating the user. Please, try again.'); - - if (err.status === 422) { - const errorData = await err.json(); - const detail = errorData?.message || ''; - - if (detail.includes('duplicate key value')) { - if (detail.includes('ab_user_username_key')) { - errorMessage = t( - 'This username is already taken. Please choose another one.', - ); - } else if (detail.includes('ab_user_email_key')) { - errorMessage = t( - 'This email is already associated with an account. Please choose another one.', - ); - } - } - } - - addDangerToast(errorMessage); - throw err; - }; - if (isEditMode) { if (!user) { throw new Error('User is required in edit mode'); @@ -89,14 +64,14 @@ function UserListModal({ await updateUser(user.id, values); addSuccessToast(t('The user has been updated successfully.')); } catch (err) { - await handleError(err, Actions.UPDATE); + await handleUserError(err as Response, Actions.UPDATE, addDangerToast); } } else { try { await createUser(values); addSuccessToast(t('The user has been created successfully.')); } catch (err) { - await handleError(err, Actions.CREATE); + await handleUserError(err as Response, Actions.CREATE, addDangerToast); } } }; diff --git a/superset-frontend/src/features/users/utils.test.ts b/superset-frontend/src/features/users/utils.test.ts new file mode 100644 index 000000000000..bd9c80efa390 --- /dev/null +++ b/superset-frontend/src/features/users/utils.test.ts @@ -0,0 +1,99 @@ +/** + * 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 { Actions } from 'src/constants'; +import { handleUserError } from './utils'; + +test('shows the password validation message from a 400 response', async () => { + const error = new Response( + JSON.stringify({ + message: { + password: ['Password must be at least 8 characters long.'], + }, + }), + { status: 400 }, + ); + const addDangerToast = jest.fn(); + + await expect( + handleUserError(error, Actions.CREATE, addDangerToast), + ).rejects.toBe(error); + expect(addDangerToast).toHaveBeenCalledWith( + 'Password must be at least 8 characters long.', + ); +}); + +test('shows a plain string message from a 400 response', async () => { + const error = new Response( + JSON.stringify({ message: 'User must have at least one role or group!' }), + { status: 400 }, + ); + const addDangerToast = jest.fn(); + + await expect( + handleUserError(error, Actions.UPDATE, addDangerToast), + ).rejects.toBe(error); + expect(addDangerToast).toHaveBeenCalledWith( + 'User must have at least one role or group!', + ); +}); + +test('keeps the duplicate username message for a 422 response', async () => { + const error = new Response( + JSON.stringify({ + message: + 'duplicate key value violates unique constraint "ab_user_username_key"', + }), + { status: 422 }, + ); + const addDangerToast = jest.fn(); + + await expect( + handleUserError(error, Actions.CREATE, addDangerToast), + ).rejects.toBe(error); + expect(addDangerToast).toHaveBeenCalledWith( + 'This username is already taken. Please choose another one.', + ); +}); + +test('shows the generic message when a 422 response has no message', async () => { + const error = new Response(JSON.stringify({ foo: 'bar' }), { status: 422 }); + const addDangerToast = jest.fn(); + + await expect( + handleUserError(error, Actions.CREATE, addDangerToast), + ).rejects.toBe(error); + expect(addDangerToast).toHaveBeenCalledWith( + 'There was an error creating the user. Please, try again.', + ); +}); + +test('shows the generic message when a 400 response is not JSON', async () => { + const error = new Response('Bad request', { + status: 400, + headers: { 'Content-Type': 'text/html' }, + }); + const addDangerToast = jest.fn(); + + await expect( + handleUserError(error, Actions.CREATE, addDangerToast), + ).rejects.toBe(error); + expect(addDangerToast).toHaveBeenCalledWith( + 'There was an error creating the user. Please, try again.', + ); +}); diff --git a/superset-frontend/src/features/users/utils.ts b/superset-frontend/src/features/users/utils.ts index 5450bdca49fa..4baa60fbf3ce 100644 --- a/superset-frontend/src/features/users/utils.ts +++ b/superset-frontend/src/features/users/utils.ts @@ -17,10 +17,49 @@ * under the License. */ import { t } from '@apache-superset/core/translation'; -import { SupersetClient } from '@superset-ui/core'; +import { getClientErrorObject, SupersetClient } from '@superset-ui/core'; import { SelectOption } from 'src/components/ListView'; +import { Actions } from 'src/constants'; import { FormValues } from './types'; +type AddDangerToast = (message: string) => void; + +export const handleUserError = async ( + err: Response, + action: Actions.CREATE | Actions.UPDATE, + addDangerToast: AddDangerToast, +): Promise => { + let errorMessage = + action === Actions.CREATE + ? t('There was an error creating the user. Please, try again.') + : t('There was an error updating the user. Please, try again.'); + + if (err.status === 400 || err.status === 422) { + const errorData = await getClientErrorObject(err); + const message: unknown = errorData.message; + + if (err.status === 400 && message && errorData.error) { + errorMessage = errorData.error; + } else if ( + err.status === 422 && + errorData.error?.includes('duplicate key value') + ) { + if (errorData.error.includes('ab_user_username_key')) { + errorMessage = t( + 'This username is already taken. Please choose another one.', + ); + } else if (errorData.error.includes('ab_user_email_key')) { + errorMessage = t( + 'This email is already associated with an account. Please choose another one.', + ); + } + } + } + + addDangerToast(errorMessage); + throw err; +}; + export const createUser = async (values: FormValues) => { const { confirmPassword: _confirmPassword, ...payload } = values; if (payload.active == null) {