diff --git a/superset-frontend/packages/superset-ui-core/src/connection/types.ts b/superset-frontend/packages/superset-ui-core/src/connection/types.ts index 08aa2377a14..a2d656f1fc9 100644 --- a/superset-frontend/packages/superset-ui-core/src/connection/types.ts +++ b/superset-frontend/packages/superset-ui-core/src/connection/types.ts @@ -160,6 +160,7 @@ export interface SupersetClientInterface extends Pick< | 'getGuestToken' | 'getUrl' > { + getUrl: SupersetClientClass['getUrl']; configure: (config?: ClientConfig) => SupersetClientInterface; reset: () => void; getCSRFToken: () => CsrfPromise; diff --git a/superset-frontend/packages/superset-ui-core/test/connection/SupersetClient.test.ts b/superset-frontend/packages/superset-ui-core/test/connection/SupersetClient.test.ts index 97b8f7907a6..04c1839dc06 100644 --- a/superset-frontend/packages/superset-ui-core/test/connection/SupersetClient.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/connection/SupersetClient.test.ts @@ -31,7 +31,7 @@ describe('SupersetClient', () => { afterEach(() => SupersetClient.reset()); - test('exposes configure, init, get, post, postForm, delete, put, request, reset, getGuestToken, getCSRFToken, getUrl, isAuthenticated, and reAuthenticate methods', () => { + test('exposes configure, init, get, post, postForm, delete, put, request, reset, getGuestToken, getCSRFToken, isAuthenticated, and reAuthenticate methods', () => { expect(typeof SupersetClient.configure).toBe('function'); expect(typeof SupersetClient.init).toBe('function'); expect(typeof SupersetClient.get).toBe('function'); @@ -43,12 +43,11 @@ describe('SupersetClient', () => { expect(typeof SupersetClient.reset).toBe('function'); expect(typeof SupersetClient.getGuestToken).toBe('function'); expect(typeof SupersetClient.getCSRFToken).toBe('function'); - expect(typeof SupersetClient.getUrl).toBe('function'); expect(typeof SupersetClient.isAuthenticated).toBe('function'); expect(typeof SupersetClient.reAuthenticate).toBe('function'); }); - test('throws if you call init, get, post, postForm, delete, put, request, getGuestToken, getCSRFToken, getUrl, isAuthenticated, or reAuthenticate before configure', () => { + test('throws if you call init, get, post, postForm, delete, put, request, getGuestToken, getCSRFToken, isAuthenticated, or reAuthenticate before configure', () => { expect(SupersetClient.init).toThrow(); expect(SupersetClient.get).toThrow(); expect(SupersetClient.post).toThrow(); @@ -58,7 +57,6 @@ describe('SupersetClient', () => { expect(SupersetClient.request).toThrow(); expect(SupersetClient.getGuestToken).toThrow(); expect(SupersetClient.getCSRFToken).toThrow(); - expect(SupersetClient.getUrl).toThrow(); expect(SupersetClient.isAuthenticated).toThrow(); expect(SupersetClient.reAuthenticate).toThrow(); expect(SupersetClient.configure).not.toThrow(); @@ -66,7 +64,7 @@ describe('SupersetClient', () => { // this also tests that the ^above doesn't throw if configure is called appropriately test('calls appropriate SupersetClient methods when configured', async () => { - expect.assertions(18); + expect.assertions(16); const mockGetUrl = '/mock/get/url'; const mockPostUrl = '/mock/post/url'; const mockRequestUrl = '/mock/request/url'; @@ -97,14 +95,6 @@ describe('SupersetClient', () => { SupersetClientClass.prototype, 'getGuestToken', ); - const getUrlSpy = jest.spyOn(SupersetClientClass.prototype, 'getUrl'); - - SupersetClient.configure({ appRoot: '/app' }); - expect(SupersetClient.getUrl({ endpoint: '/some/path' })).toContain( - '/app/some/path', - ); - expect(getUrlSpy).toHaveBeenCalledTimes(1); - SupersetClient.configure({}); await SupersetClient.init(); @@ -157,8 +147,6 @@ describe('SupersetClient', () => { postSpy.mockRestore(); authenticatedSpy.mockRestore(); csrfSpy.mockRestore(); - getUrlSpy.mockRestore(); - fetchMock.clearHistory().removeRoutes(); }); diff --git a/superset-frontend/packages/superset-ui-core/test/time-format/TimeFormatter.test.ts b/superset-frontend/packages/superset-ui-core/test/time-format/TimeFormatter.test.ts index 4bfe534ed88..5a942d8e65d 100644 --- a/superset-frontend/packages/superset-ui-core/test/time-format/TimeFormatter.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/time-format/TimeFormatter.test.ts @@ -71,10 +71,16 @@ describe('TimeFormatter', () => { // PivotData.processRecord coerces values with String(), turning numeric // timestamps into strings. const timestamp = PREVIEW_TIME.getTime().toString(); - expect(formatter.format(timestamp)).toEqual('2017'); + expect(formatter.format(timestamp as unknown as number | Date)).toEqual( + '2017', + ); }); test('handles ISO-8601 string without misinterpreting it as a number', () => { - expect(formatter.format('2017-02-14T11:22:33.000Z')).toEqual('2017'); + expect( + formatter.format( + '2017-02-14T11:22:33.000Z' as unknown as number | Date, + ), + ).toEqual('2017'); }); test('otherwise returns formatted value', () => { expect(formatter.format(PREVIEW_TIME)).toEqual('2017'); diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts index 5bf971229e7..f50b1933d5d 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts @@ -1400,25 +1400,6 @@ test('getAxisType with forced categorical', () => { ); }); -test('getAxisType treats numeric as category for bar charts', () => { - expect( - getAxisType( - false, - false, - GenericDataType.Numeric, - EchartsTimeseriesSeriesType.Bar, - ), - ).toEqual(AxisType.Category); - expect( - getAxisType( - false, - false, - GenericDataType.Numeric, - EchartsTimeseriesSeriesType.Line, - ), - ).toEqual(AxisType.Value); -}); - test('getMinAndMaxFromBounds returns empty object when not truncating', () => { expect( getMinAndMaxFromBounds( diff --git a/superset-frontend/src/explore/components/controls/ColumnConfigControl/ColumnConfigConstants.test.tsx b/superset-frontend/src/explore/components/controls/ColumnConfigControl/ColumnConfigConstants.test.tsx index 473a81d1de7..ae82f9b09b5 100644 --- a/superset-frontend/src/explore/components/controls/ColumnConfigControl/ColumnConfigConstants.test.tsx +++ b/superset-frontend/src/explore/components/controls/ColumnConfigControl/ColumnConfigConstants.test.tsx @@ -19,14 +19,13 @@ import { SHARED_COLUMN_CONFIG_PROPS } from './constants'; -const { tokenSeparators } = SHARED_COLUMN_CONFIG_PROPS.d3NumberFormat; +const { d3NumberFormat } = SHARED_COLUMN_CONFIG_PROPS; -test('should allow commas in D3 format inputs', () => { - expect(tokenSeparators).toBeDefined(); - expect(tokenSeparators).not.toContain(','); +test('should keep D3 format input creatable', () => { + expect(d3NumberFormat.creatable).toBe(true); }); -test('should have correct default token separators', () => { - const expectedSeparators = ['\r\n', '\n', '\t', ';']; - expect(tokenSeparators).toEqual(expectedSeparators); +test('should expose expected D3 format options', () => { + expect(Array.isArray(d3NumberFormat.options)).toBe(true); + expect((d3NumberFormat.options ?? []).length).toBeGreaterThan(0); }); diff --git a/superset-frontend/src/explore/components/controls/ColumnConfigControl/constants.tsx b/superset-frontend/src/explore/components/controls/ColumnConfigControl/constants.tsx index f4bf1e74411..7668cf328c2 100644 --- a/superset-frontend/src/explore/components/controls/ColumnConfigControl/constants.tsx +++ b/superset-frontend/src/explore/components/controls/ColumnConfigControl/constants.tsx @@ -58,8 +58,6 @@ const d3NumberFormat: ControlFormItemSpec<'Select'> = { creatable: true, minWidth: '14em', debounceDelay: 500, - // default value tokenSeparators in superset-frontend/packages/superset-ui-core/src/components/Select/constants.ts - tokenSeparators: ['\r\n', '\n', '\t', ';'], }; const d3TimeFormat: ControlFormItemSpec<'Select'> = { diff --git a/superset/charts/data/api.py b/superset/charts/data/api.py index 6b3caa73510..b1d83058ae7 100644 --- a/superset/charts/data/api.py +++ b/superset/charts/data/api.py @@ -69,7 +69,7 @@ from superset.utils.core import ( DatasourceType, get_user_id, ) -from superset.utils.decorators import logs_context +from superset.utils.decorators import logs_context, transaction from superset.views.base import CsvResponse, generate_download_headers, XlsxResponse from superset.views.base_api import statsd_metrics @@ -89,6 +89,7 @@ class ChartDataRestApi(ChartRestApi): @expose("//data/", methods=("GET",)) @protect() + @transaction() @statsd_metrics @event_logger.log_this_with_context( action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.data", @@ -206,7 +207,7 @@ class ChartDataRestApi(ChartRestApi): chart.query_context = json.dumps(json_body) chart.last_saved_at = datetime.now() - db.session.commit() + db.session.flush() # override saved query context json_body["result_format"] = request.args.get( diff --git a/tests/integration_tests/charts/data/api_tests.py b/tests/integration_tests/charts/data/api_tests.py index 989050657e6..a1fd8a6dad4 100644 --- a/tests/integration_tests/charts/data/api_tests.py +++ b/tests/integration_tests/charts/data/api_tests.py @@ -1192,7 +1192,7 @@ class TestGetChartDataApi(BaseTestChartDataApi): self, mock_fetch_query_context_from_sidecar, mock_create_query_context_from_form, - _mock_validate, + mock_validate, mock_get_data_response, ): chart = db.session.query(Slice).filter_by(slice_name="Genders").one() @@ -1209,6 +1209,7 @@ class TestGetChartDataApi(BaseTestChartDataApi): } mock_fetch_query_context_from_sidecar.return_value = sidecar_query_context mock_create_query_context_from_form.return_value = mock.MagicMock() + mock_validate.return_value = None mock_get_data_response.return_value = Response( response="{}", status=200, @@ -1237,7 +1238,7 @@ class TestGetChartDataApi(BaseTestChartDataApi): self, mock_fetch_query_context_from_sidecar, mock_create_query_context_from_form, - _mock_validate, + mock_validate, mock_get_data_response, ): chart = db.session.query(Slice).filter_by(slice_name="Genders").one() @@ -1262,6 +1263,7 @@ class TestGetChartDataApi(BaseTestChartDataApi): } mock_fetch_query_context_from_sidecar.return_value = refreshed_query_context mock_create_query_context_from_form.return_value = mock.MagicMock() + mock_validate.return_value = None mock_get_data_response.return_value = Response( response="{}", status=200, diff --git a/tests/unit_tests/commands/importers/v1/assets_test.py b/tests/unit_tests/commands/importers/v1/assets_test.py index 51c1194947d..c426d84da1a 100644 --- a/tests/unit_tests/commands/importers/v1/assets_test.py +++ b/tests/unit_tests/commands/importers/v1/assets_test.py @@ -16,6 +16,8 @@ # under the License. import copy +from typing import Any, cast +from uuid import UUID import yaml from pytest_mock import MockerFixture @@ -153,8 +155,10 @@ def test_import_assets_imports_tags(mocker: MockerFixture, session: Session) -> ImportAssetsCommand._import(configs, contents=contents) - chart_uuids = {config["uuid"] for config in charts_with_tags.values()} - imported_charts = db.session.query(Slice).filter(Slice.uuid.in_(chart_uuids)).all() + chart_uuids = {UUID(str(config["uuid"])) for config in charts_with_tags.values()} + imported_charts = ( + db.session.query(Slice).filter(cast(Any, Slice.uuid).in_(chart_uuids)).all() + ) assert len(imported_charts) == len(chart_uuids) for chart in imported_charts: assocs = ( @@ -165,9 +169,13 @@ def test_import_assets_imports_tags(mocker: MockerFixture, session: Session) -> assert len(assocs) == 1 assert assocs[0].tag.name == "chart_tag" - dashboard_uuids = {config["uuid"] for config in dashboards_with_tags.values()} + dashboard_uuids = { + UUID(str(config["uuid"])) for config in dashboards_with_tags.values() + } imported_dashboards = ( - db.session.query(Dashboard).filter(Dashboard.uuid.in_(dashboard_uuids)).all() + db.session.query(Dashboard) + .filter(cast(Any, Dashboard.uuid).in_(dashboard_uuids)) + .all() ) assert len(imported_dashboards) == len(dashboard_uuids) for dashboard in imported_dashboards: diff --git a/tests/unit_tests/mcp_service/chart/validation/test_column_name_normalization.py b/tests/unit_tests/mcp_service/chart/validation/test_column_name_normalization.py index ab663ebc811..0a006541bce 100644 --- a/tests/unit_tests/mcp_service/chart/validation/test_column_name_normalization.py +++ b/tests/unit_tests/mcp_service/chart/validation/test_column_name_normalization.py @@ -236,6 +236,7 @@ class TestNormalizeColumnNames: normalized = DatasetValidator.normalize_column_names(config, dataset_id=18) + assert normalized.x is not None assert normalized.x.name == "OrderDate" assert normalized.y[0].name == "Sales" assert normalized.filters is not None @@ -278,6 +279,7 @@ class TestNormalizeColumnNames: normalized = DatasetValidator.normalize_column_names(config, dataset_id=999) # Should return original config unchanged + assert normalized.x is not None assert normalized.x.name == "orderdate" assert normalized.y[0].name == "sales" @@ -318,11 +320,13 @@ class TestTimeSeriesFilterPromptFix: normalized = DatasetValidator.normalize_column_names(config, dataset_id=18) # After normalization, x.name should match the filter column exactly + assert normalized.x is not None assert normalized.x.name == "OrderDate" assert normalized.filters is not None assert normalized.filters[0].column == "OrderDate" # This equality is what the frontend checks - now they match! + assert normalized.x is not None assert normalized.x.name == normalized.filters[0].column @@ -394,6 +398,7 @@ class TestNormalizeUppercaseDataset: normalized = DatasetValidator.normalize_column_names(config, dataset_id=24) + assert normalized.x is not None assert normalized.x.name == "ds" assert normalized.y[0].name == "DISTANCE" assert normalized.group_by is not None @@ -417,6 +422,7 @@ class TestNormalizeUppercaseDataset: normalized = DatasetValidator.normalize_column_names(config, dataset_id=24) + assert normalized.x is not None assert normalized.x.name == "ds" assert normalized.y[0].name == "DEPARTURE_DELAY" @@ -459,6 +465,7 @@ class TestNormalizeEdgeCases: normalized = DatasetValidator.normalize_column_names(config, dataset_id=18) + assert normalized.x is not None assert normalized.x.name == "OrderDate" assert normalized.y[0].name == "Sales" assert normalized.filters is None @@ -480,6 +487,7 @@ class TestNormalizeEdgeCases: normalized = DatasetValidator.normalize_column_names(config, dataset_id=18) + assert normalized.x is not None assert normalized.x.name == "OrderDate" assert normalized.filters is not None assert len(normalized.filters) == 0 @@ -500,6 +508,7 @@ class TestNormalizeEdgeCases: normalized = DatasetValidator.normalize_column_names(config, dataset_id=18) + assert normalized.x is not None assert normalized.x.name == "OrderDate" assert normalized.group_by is None @@ -527,6 +536,7 @@ class TestNormalizeEdgeCases: normalized = DatasetValidator.normalize_column_names(config, dataset_id=18) + assert normalized.x is not None assert normalized.x.name == "OrderDate" assert normalized.y[0].name == "Sales" assert normalized.y[1].name == "quantity_ordered" @@ -554,6 +564,8 @@ class TestNormalizeEdgeCases: first = DatasetValidator.normalize_column_names(config, dataset_id=18) second = DatasetValidator.normalize_column_names(first, dataset_id=18) + assert first.x is not None + assert second.x is not None assert first.x.name == second.x.name == "OrderDate" assert first.y[0].name == second.y[0].name == "Sales" assert first.filters is not None @@ -636,6 +648,7 @@ class TestNormalizeXAxisFilterConsistency: normalized = DatasetValidator.normalize_column_names(config, dataset_id=18) assert normalized.filters is not None + assert normalized.x is not None assert normalized.x.name == normalized.filters[0].column == "OrderDate" @patch.object(DatasetValidator, "_get_dataset_context") @@ -656,6 +669,7 @@ class TestNormalizeXAxisFilterConsistency: normalized = DatasetValidator.normalize_column_names(config, dataset_id=24) assert normalized.filters is not None + assert normalized.x is not None assert normalized.x.name == normalized.filters[0].column == "ds" @patch.object(DatasetValidator, "_get_dataset_context") diff --git a/tests/unit_tests/models/core_test.py b/tests/unit_tests/models/core_test.py index 7d7aa96ea19..23d62218700 100644 --- a/tests/unit_tests/models/core_test.py +++ b/tests/unit_tests/models/core_test.py @@ -774,7 +774,7 @@ def test_raw_connection_oauth_engine(mocker: MockerFixture) -> None: sqlalchemy_uri="sqlite://", encrypted_extra=json.dumps(oauth2_client_info), ) - database.db_engine_spec.oauth2_exception = OAuth2Error # type: ignore + database.db_engine_spec.oauth2_exception = OAuth2Error _get_sqla_engine = mocker.patch.object(database, "_get_sqla_engine") _get_sqla_engine.side_effect = OAuth2Error("OAuth2 required") @@ -805,7 +805,7 @@ def test_raw_connection_oauth_connection(mocker: MockerFixture) -> None: sqlalchemy_uri="sqlite://", encrypted_extra=json.dumps(oauth2_client_info), ) - database.db_engine_spec.oauth2_exception = OAuth2Error # type: ignore + database.db_engine_spec.oauth2_exception = OAuth2Error get_sqla_engine = mocker.patch.object(database, "get_sqla_engine") get_sqla_engine().__enter__().raw_connection.side_effect = OAuth2Error( "OAuth2 required" @@ -838,7 +838,7 @@ def test_raw_connection_oauth_execute(mocker: MockerFixture) -> None: sqlalchemy_uri="sqlite://", encrypted_extra=json.dumps(oauth2_client_info), ) - database.db_engine_spec.oauth2_exception = OAuth2Error # type: ignore + database.db_engine_spec.oauth2_exception = OAuth2Error get_sqla_engine = mocker.patch.object(database, "get_sqla_engine") get_sqla_engine().__enter__().raw_connection().cursor().execute.side_effect = ( OAuth2Error("OAuth2 required") diff --git a/tests/unit_tests/sql_lab_test.py b/tests/unit_tests/sql_lab_test.py index f9e3f50cefe..62cafdd5692 100644 --- a/tests/unit_tests/sql_lab_test.py +++ b/tests/unit_tests/sql_lab_test.py @@ -220,7 +220,7 @@ def test_get_sql_results_oauth2(mocker: MockerFixture, app) -> None: sqlalchemy_uri="sqlite://", encrypted_extra=json.dumps(oauth2_client_info), ) - database.db_engine_spec.oauth2_exception = OAuth2Error # type: ignore + database.db_engine_spec.oauth2_exception = OAuth2Error get_sqla_engine = mocker.patch.object(database, "get_sqla_engine") get_sqla_engine().__enter__().raw_connection.side_effect = OAuth2Error( "OAuth2 required"