From 29d5be449b33bed6d2229fadad85a45eafcf9dda Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 13 Apr 2023 05:26:16 -0700 Subject: [PATCH 01/20] First draft --- .../src/fixtures.ts | 2 +- .../superset-ui-chart-controls/src/types.ts | 2 +- .../test/utils/columnChoices.test.tsx | 2 +- .../test/utils/defineSavedMetrics.test.tsx | 2 +- .../Datasource/ChangeDatasourceModal.test.jsx | 2 +- .../Datasource/ChangeDatasourceModal.tsx | 5 +- .../Datasource/DatasourceModal.test.jsx | 4 +- .../components/Datasource/DatasourceModal.tsx | 84 +++++++++++++------ superset-frontend/src/dashboard/constants.ts | 2 +- .../actions/datasourcesActions.test.ts | 4 +- .../DatasourceControl.test.tsx | 2 +- .../controls/DatasourceControl/index.jsx | 1 + .../controlUtils/controlUtils.test.tsx | 2 +- ...trolValuesCompatibleWithDatasource.test.ts | 2 +- superset-frontend/src/explore/fixtures.tsx | 4 +- .../src/utils/getDatasourceUid.test.ts | 2 +- superset/connectors/base/models.py | 29 ++++--- superset/connectors/sqla/models.py | 14 +++- superset/datasets/api.py | 8 ++ superset/datasets/commands/exceptions.py | 17 ++++ superset/datasets/commands/update.py | 11 +++ superset/views/datasource/views.py | 3 + tests/integration_tests/datasets/api_tests.py | 23 +++++ 23 files changed, 167 insertions(+), 60 deletions(-) diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/fixtures.ts b/superset-frontend/packages/superset-ui-chart-controls/src/fixtures.ts index 71c4dd31189ba..1e5152b2bd904 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/fixtures.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/src/fixtures.ts @@ -20,7 +20,7 @@ import { DatasourceType } from '@superset-ui/core'; import { Dataset } from './types'; export const TestDataset: Dataset = { - column_format: {}, + column_formats: {}, columns: [ { advanced_data_type: undefined, diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts index d4e91246ab11b..ddae9d48965ec 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts @@ -67,7 +67,7 @@ export interface Dataset { type: DatasourceType; columns: ColumnMeta[]; metrics: Metric[]; - column_format: Record; + column_formats: Record; verbose_map: Record; main_dttm_col: string; // eg. ['["ds", true]', 'ds [asc]'] diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/utils/columnChoices.test.tsx b/superset-frontend/packages/superset-ui-chart-controls/test/utils/columnChoices.test.tsx index 59f4796a44d21..e27fa95120848 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/utils/columnChoices.test.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/test/utils/columnChoices.test.tsx @@ -42,7 +42,7 @@ describe('columnChoices()', () => { }, ], verbose_map: {}, - column_format: { fiz: 'NUMERIC', about: 'STRING', foo: 'DATE' }, + column_formats: { fiz: 'NUMERIC', about: 'STRING', foo: 'DATE' }, datasource_name: 'my_datasource', description: 'this is my datasource', }), diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/utils/defineSavedMetrics.test.tsx b/superset-frontend/packages/superset-ui-chart-controls/test/utils/defineSavedMetrics.test.tsx index 48b000ed17ffa..765412d592c24 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/utils/defineSavedMetrics.test.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/test/utils/defineSavedMetrics.test.tsx @@ -39,7 +39,7 @@ describe('defineSavedMetrics', () => { time_grain_sqla: 'P1D', columns: [], verbose_map: {}, - column_format: {}, + column_formats: {}, datasource_name: 'my_datasource', description: 'this is my datasource', }; diff --git a/superset-frontend/src/components/Datasource/ChangeDatasourceModal.test.jsx b/superset-frontend/src/components/Datasource/ChangeDatasourceModal.test.jsx index 419af6e3cf752..a24c18c3770cd 100644 --- a/superset-frontend/src/components/Datasource/ChangeDatasourceModal.test.jsx +++ b/superset-frontend/src/components/Datasource/ChangeDatasourceModal.test.jsx @@ -49,7 +49,7 @@ const datasourceData = { const DATASOURCES_ENDPOINT = 'glob:*/api/v1/dataset/?q=(order_column:changed_on_delta_humanized,order_direction:desc,page:0,page_size:25)'; -const DATASOURCE_ENDPOINT = `glob:*/datasource/get/${datasourceData.type}/${datasourceData.id}`; +const DATASOURCE_ENDPOINT = `glob:*/api/v1/dataset/${datasourceData.id}`; const DATASOURCE_PAYLOAD = { new: 'data' }; const INFO_ENDPOINT = 'glob:*/api/v1/dataset/_info?*'; diff --git a/superset-frontend/src/components/Datasource/ChangeDatasourceModal.tsx b/superset-frontend/src/components/Datasource/ChangeDatasourceModal.tsx index b5b99e1089f4a..5ae8929a2355b 100644 --- a/superset-frontend/src/components/Datasource/ChangeDatasourceModal.tsx +++ b/superset-frontend/src/components/Datasource/ChangeDatasourceModal.tsx @@ -173,10 +173,11 @@ const ChangeDatasourceModal: FunctionComponent = ({ const handleChangeConfirm = () => { SupersetClient.get({ - endpoint: `/datasource/get/${confirmedDataset?.type}/${confirmedDataset?.id}/`, + endpoint: `/api/v1/dataset/${confirmedDataset?.id}`, }) .then(({ json }) => { - onDatasourceSave(json); + json.result.type = 'table'; + onDatasourceSave(json.result); onChange(`${confirmedDataset?.id}__table`); }) .catch(response => { diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx b/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx index 12be350521515..7b3395e2997d6 100644 --- a/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx +++ b/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx @@ -40,7 +40,7 @@ const datasource = mockDatasource['7__table']; const SAVE_ENDPOINT = 'glob:*/api/v1/dataset/7'; const SAVE_PAYLOAD = { new: 'data' }; -const SAVE_DATASOURCE_ENDPOINT = 'glob:*/datasource/save/'; +const SAVE_DATASOURCE_ENDPOINT = 'glob:*/api/v1/dataset/7'; const mockedProps = { datasource, @@ -111,7 +111,7 @@ describe('DatasourceModal', () => { okButton.simulate('click'); }); await waitForComponentToPaint(wrapper); - const expected = ['http://localhost/datasource/save/']; + const expected = ['http://localhost/api/v1/dataset/7']; expect(callsP._calls.map(call => call[0])).toEqual( expected, ); /* eslint no-underscore-dangle: 0 */ diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.tsx b/superset-frontend/src/components/Datasource/DatasourceModal.tsx index 90135f40f9ecd..ba23efef0f729 100644 --- a/superset-frontend/src/components/Datasource/DatasourceModal.tsx +++ b/superset-frontend/src/components/Datasource/DatasourceModal.tsx @@ -96,33 +96,63 @@ const DatasourceModal: FunctionComponent = ({ currentDatasource.schema; setIsSaving(true); - SupersetClient.post({ - endpoint: '/datasource/save/', - postPayload: { - data: { - ...currentDatasource, - cache_timeout: - currentDatasource.cache_timeout === '' - ? null - : currentDatasource.cache_timeout, - schema, - metrics: currentDatasource?.metrics?.map( - (metric: Record) => ({ - ...metric, - extra: buildExtraJsonObject(metric), - }), - ), - columns: currentDatasource?.columns?.map( - (column: Record) => ({ - ...column, - extra: buildExtraJsonObject(column), - }), - ), - type: currentDatasource.type || currentDatasource.datasource_type, - owners: currentDatasource.owners.map( - (o: Record) => o.value || o.id, - ), - }, + SupersetClient.put({ + endpoint: `/api/v1/dataset/${currentDatasource.id}`, + jsonPayload: { + table_name: currentDatasource.table_name, + database_id: currentDatasource.database?.id, + sql: currentDatasource.sql, + filter_select_enabled: currentDatasource.filter_select_enabled, + fetch_values_predicate: currentDatasource.fetch_values_predicate, + schema, + description: currentDatasource.description, + main_dttm_col: currentDatasource.main_dttm_col, + offset: currentDatasource.offset, + default_endpoint: currentDatasource.default_endpoint, + cache_timeout: + currentDatasource.cache_timeout === '' + ? null + : currentDatasource.cache_timeout, + is_sqllab_view: currentDatasource.is_sqllab_view, + template_params: currentDatasource.template_params, + extra: currentDatasource.extra, + is_managed_externally: currentDatasource.is_managed_externally, + external_url: currentDatasource.external_url, + metrics: currentDatasource?.metrics?.map( + (metric: Record) => ({ + id: metric.id, + expression: metric.expression, + description: metric.description, + metric_name: metric.metric_name, + metric_type: metric.metric_type, + d3format: metric.d3format, + verbose_name: metric.verbose_name, + warning_text: metric.warning_text, + uuid: metric.uuid, + extra: buildExtraJsonObject(metric), + }), + ), + columns: currentDatasource?.columns?.map( + (column: Record) => ({ + id: column.id, + column_name: column.column_name, + type: column.type, + advanced_data_type: column.advanced_data_type, + verbose_name: column.verbose_name, + description: column.description, + expression: column.expression, + filterable: column.filterable, + groupby: column.groupby, + is_active: column.is_active, + is_dttm: column.is_dttm, + python_date_format: column.python_date_format, + uuid: column.uuid, + extra: buildExtraJsonObject(column), + }), + ), + owners: currentDatasource.owners.map( + (o: Record) => o.value || o.id, + ), }, }) .then(({ json }) => { diff --git a/superset-frontend/src/dashboard/constants.ts b/superset-frontend/src/dashboard/constants.ts index 97753abe45c54..aa9f4d8bd3619 100644 --- a/superset-frontend/src/dashboard/constants.ts +++ b/superset-frontend/src/dashboard/constants.ts @@ -28,7 +28,7 @@ export const PLACEHOLDER_DATASOURCE: Datasource = { columns: [], column_types: [], metrics: [], - column_format: {}, + column_formats: {}, verbose_map: {}, main_dttm_col: '', description: '', diff --git a/superset-frontend/src/explore/actions/datasourcesActions.test.ts b/superset-frontend/src/explore/actions/datasourcesActions.test.ts index 6317f1f4a6d15..996758b262323 100644 --- a/superset-frontend/src/explore/actions/datasourcesActions.test.ts +++ b/superset-frontend/src/explore/actions/datasourcesActions.test.ts @@ -34,7 +34,7 @@ const CURRENT_DATASOURCE = { type: DatasourceType.Table, columns: [], metrics: [], - column_format: {}, + column_formats: {}, verbose_map: {}, main_dttm_col: '__timestamp', // eg. ['["ds", true]', 'ds [asc]'] @@ -47,7 +47,7 @@ const NEW_DATASOURCE = { type: DatasourceType.Table, columns: [], metrics: [], - column_format: {}, + column_formats: {}, verbose_map: {}, main_dttm_col: '__timestamp', // eg. ['["ds", true]', 'ds [asc]'] diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx index 2c094e72afba0..8113a35e2d61b 100644 --- a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx +++ b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx @@ -69,7 +69,7 @@ const createProps = (overrides: JsonObject = {}) => ({ }); async function openAndSaveChanges(datasource: any) { - fetchMock.post('glob:*/datasource/save/', datasource, { + fetchMock.post('glob:*/api/v1/dataset/*', datasource, { overwriteRoutes: true, }); userEvent.click(screen.getByTestId('datasource-menu-trigger')); diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx b/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx index c99193dd427d6..98b31c54f6744 100644 --- a/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx +++ b/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx @@ -173,6 +173,7 @@ class DatasourceControl extends React.PureComponent { } onDatasourceSave = datasource => { + console.log(datasource); this.props.actions.changeDatasource(datasource); const { temporalColumns, defaultTemporalColumn } = getTemporalColumns(datasource); diff --git a/superset-frontend/src/explore/controlUtils/controlUtils.test.tsx b/superset-frontend/src/explore/controlUtils/controlUtils.test.tsx index 5feefc3481aa1..e03aba06d11df 100644 --- a/superset-frontend/src/explore/controlUtils/controlUtils.test.tsx +++ b/superset-frontend/src/explore/controlUtils/controlUtils.test.tsx @@ -51,7 +51,7 @@ describe('controlUtils', () => { type: DatasourceType.Table, columns: [{ column_name: 'a' }], metrics: [{ metric_name: 'first' }, { metric_name: 'second' }], - column_format: {}, + column_formats: {}, verbose_map: {}, main_dttm_col: '', datasource_name: '1__table', diff --git a/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.test.ts b/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.test.ts index ad7ccc480121a..c8d34f749d3dc 100644 --- a/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.test.ts +++ b/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.test.ts @@ -34,7 +34,7 @@ const sampleDatasource: Dataset = { { column_name: 'sample_column_4' }, ], metrics: [{ metric_name: 'saved_metric_2' }], - column_format: {}, + column_formats: {}, verbose_map: {}, main_dttm_col: '', datasource_name: 'Sample Dataset', diff --git a/superset-frontend/src/explore/fixtures.tsx b/superset-frontend/src/explore/fixtures.tsx index 351e6f26b126b..a0f3c112ec1b8 100644 --- a/superset-frontend/src/explore/fixtures.tsx +++ b/superset-frontend/src/explore/fixtures.tsx @@ -135,7 +135,7 @@ export const exploreInitialData: ExplorePageInitialData = { type: DatasourceType.Table, columns: [{ column_name: 'a' }], metrics: [{ metric_name: 'first' }, { metric_name: 'second' }], - column_format: {}, + column_formats: {}, verbose_map: {}, main_dttm_col: '', datasource_name: '8__table', @@ -153,7 +153,7 @@ export const fallbackExploreInitialData: ExplorePageInitialData = { type: DatasourceType.Table, columns: [], metrics: [], - column_format: {}, + column_formats: {}, verbose_map: {}, main_dttm_col: '', owners: [], diff --git a/superset-frontend/src/utils/getDatasourceUid.test.ts b/superset-frontend/src/utils/getDatasourceUid.test.ts index 2d14fde26230c..d3a629efcfed3 100644 --- a/superset-frontend/src/utils/getDatasourceUid.test.ts +++ b/superset-frontend/src/utils/getDatasourceUid.test.ts @@ -25,7 +25,7 @@ const TEST_DATASOURCE = { type: DatasourceType.Table, columns: [], metrics: [], - column_format: {}, + column_formats: {}, verbose_map: {}, main_dttm_col: '__timestamp', // eg. ['["ds", true]', 'ds [asc]'] diff --git a/superset/connectors/base/models.py b/superset/connectors/base/models.py index a5bea92ffa11e..4d2318f911e27 100644 --- a/superset/connectors/base/models.py +++ b/superset/connectors/base/models.py @@ -19,7 +19,7 @@ import json from datetime import datetime from enum import Enum -from typing import Any, Dict, Hashable, List, Optional, Set, Type, TYPE_CHECKING, Union +from typing import Any, Dict, Hashable, List, Optional, Set, Tuple, Type, TYPE_CHECKING, Union from flask_appbuilder.security.sqla.models import User from flask_babel import gettext as __ @@ -242,26 +242,33 @@ def select_star(self) -> Optional[str]: pass @property - def data(self) -> Dict[str, Any]: - """Data representation of the datasource sent to the frontend""" - order_by_choices = [] + def order_by_choices(self) -> List[Tuple[str, str]]: + choices = [] # self.column_names return sorted column_names for column_name in self.column_names: column_name = str(column_name or "") - order_by_choices.append( + choices.append( (json.dumps([column_name, True]), f"{column_name} " + __("[asc]")) ) - order_by_choices.append( + choices.append( (json.dumps([column_name, False]), f"{column_name} " + __("[desc]")) ) + return choices - verbose_map = {"__timestamp": "Time"} - verbose_map.update( + @property + def verbose_map(self) -> Dict[str, str]: + verb_map = {"__timestamp": "Time"} + verb_map.update( {o.metric_name: o.verbose_name or o.metric_name for o in self.metrics} ) - verbose_map.update( + verb_map.update( {o.column_name: o.verbose_name or o.column_name for o in self.columns} ) + return verb_map + + @property + def data(self) -> Dict[str, Any]: + """Data representation of the datasource sent to the frontend""" return { # simple fields "id": self.id, @@ -288,9 +295,9 @@ def data(self) -> Dict[str, Any]: "columns": [o.data for o in self.columns], "metrics": [o.data for o in self.metrics], # TODO deprecate, move logic to JS - "order_by_choices": order_by_choices, + "order_by_choices": self.order_by_choices, "owners": [owner.id for owner in self.owners], - "verbose_map": verbose_map, + "verbose_map": self.verbose_map, "select_star": self.select_star, } diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index 593c5f853b935..36ba17ddb1bc4 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -786,14 +786,20 @@ def health_check_message(self) -> Optional[str]: check = config["DATASET_HEALTH_CHECK"] return check(self) if check else None + @property + def granularity_sqla(self) -> List[Tuple[Any, Any]]: + return utils.choicify(self.dttm_cols) + + @property + def time_grain_sqla(self) -> List[Tuple[Any, Any]]: + return [(g.duration, g.name) for g in self.database.grains() or []] + @property def data(self) -> Dict[str, Any]: data_ = super().data if self.type == "table": - data_["granularity_sqla"] = utils.choicify(self.dttm_cols) - data_["time_grain_sqla"] = [ - (g.duration, g.name) for g in self.database.grains() or [] - ] + data_["granularity_sqla"] = self.granularity_sqla + data_["time_grain_sqla"] = self.time_grain_sqla data_["main_dttm_col"] = self.main_dttm_col data_["fetch_values_predicate"] = self.fetch_values_predicate data_["template_params"] = self.template_params diff --git a/superset/datasets/api.py b/superset/datasets/api.py index 48c429d32d4b1..12de82c780d43 100644 --- a/superset/datasets/api.py +++ b/superset/datasets/api.py @@ -195,6 +195,14 @@ class DatasetRestApi(BaseSupersetModelRestApi): "database.backend", "columns.advanced_data_type", "is_managed_externally", + "uid", + "datasource_name", + "name", + "column_formats", + "granularity_sqla", + "time_grain_sqla", + "order_by_choices", + "verbose_map", ] add_model_schema = DatasetPostSchema() edit_model_schema = DatasetPutSchema() diff --git a/superset/datasets/commands/exceptions.py b/superset/datasets/commands/exceptions.py index 91af2fdde4e9c..727ce6e37c354 100644 --- a/superset/datasets/commands/exceptions.py +++ b/superset/datasets/commands/exceptions.py @@ -61,6 +61,23 @@ def __init__(self, table_name: str) -> None: ) +class DatasetEndpointUnsafeValidationError(ValidationError): + """ + Marshmallow validation error for unsafe dataset default endpoint + """ + + def __init__(self): + super().__init__( + [ + _( + "The submitted URL is not considered safe," + " only use URLs with the same domain as Superset." + ) + ], + field_name="default_endpoint", + ) + + class DatasetColumnNotFoundValidationError(ValidationError): """ Marshmallow validation error when dataset column for update does not exist diff --git a/superset/datasets/commands/update.py b/superset/datasets/commands/update.py index 483a98e76c3b8..a2e483ba93ddb 100644 --- a/superset/datasets/commands/update.py +++ b/superset/datasets/commands/update.py @@ -18,6 +18,7 @@ from collections import Counter from typing import Any, Dict, List, Optional +from flask import current_app from flask_appbuilder.models.sqla import Model from marshmallow import ValidationError @@ -30,6 +31,7 @@ DatasetColumnNotFoundValidationError, DatasetColumnsDuplicateValidationError, DatasetColumnsExistsValidationError, + DatasetEndpointUnsafeValidationError, DatasetExistsValidationError, DatasetForbiddenError, DatasetInvalidError, @@ -41,6 +43,7 @@ ) from superset.datasets.dao import DatasetDAO from superset.exceptions import SupersetSecurityException +from superset.utils.urls import is_safe_url logger = logging.getLogger(__name__) @@ -101,6 +104,14 @@ def validate(self) -> None: self._properties["owners"] = owners except ValidationError as ex: exceptions.append(ex) + # Validate default URL safety + default_endpoint = self._properties.get("default_endpoint") + if ( + default_endpoint + and not is_safe_url(default_endpoint) + and current_app.config["PREVENT_UNSAFE_DEFAULT_URLS_ON_DATASET"] + ): + exceptions.append(DatasetEndpointUnsafeValidationError()) # Validate columns columns = self._properties.get("columns") diff --git a/superset/views/datasource/views.py b/superset/views/datasource/views.py index 4f158e836936a..77cb47d19957a 100644 --- a/superset/views/datasource/views.py +++ b/superset/views/datasource/views.py @@ -44,6 +44,7 @@ from superset.views.base import ( api, BaseSupersetView, + deprecated, handle_api_exception, json_error_response, ) @@ -69,6 +70,7 @@ class Datasource(BaseSupersetView): @has_access_api @api @handle_api_exception + @deprecated(new_target="/api/v1/dataset/") def save(self) -> FlaskResponse: data = request.form.get("data") if not isinstance(data, str): @@ -133,6 +135,7 @@ def save(self) -> FlaskResponse: @has_access_api @api @handle_api_exception + @deprecated(new_target="/api/v1/dataset/") def get(self, datasource_type: str, datasource_id: int) -> FlaskResponse: datasource = DatasourceDAO.get_datasource( db.session, DatasourceType(datasource_type), datasource_id diff --git a/tests/integration_tests/datasets/api_tests.py b/tests/integration_tests/datasets/api_tests.py index b9d1eb84b6a55..caedcdf0f246f 100644 --- a/tests/integration_tests/datasets/api_tests.py +++ b/tests/integration_tests/datasets/api_tests.py @@ -1350,6 +1350,29 @@ def test_update_dataset_item_uniqueness(self): db.session.delete(ab_user) db.session.commit() + def test_update_dataset_unsafe_default_endpoint(self): + """ + Dataset API: Test unsafe default endpoint + """ + dataset = self.insert_default_dataset() + self.login(username="admin") + uri = f"api/v1/dataset/{dataset.id}" + table_data = {"default_endpoint": "http://www.google.com"} + rv = self.client.put(uri, json=table_data) + data = json.loads(rv.data.decode("utf-8")) + assert rv.status_code == 422 + expected_response = { + "message": { + "default_endpoint": [ + "The submitted URL is not considered safe," + " only use URLs with the same domain as Superset." + ] + } + } + assert data == expected_response + db.session.delete(dataset) + db.session.commit() + @patch("superset.datasets.dao.DatasetDAO.update") def test_update_dataset_sqlalchemy_error(self, mock_dao_update): """ From a67be31112be53c0942b69eb964f533cf2227e7c Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 13 Apr 2023 05:29:37 -0700 Subject: [PATCH 02/20] Formatting --- superset/connectors/base/models.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/superset/connectors/base/models.py b/superset/connectors/base/models.py index 4d2318f911e27..cf48f1464bb5f 100644 --- a/superset/connectors/base/models.py +++ b/superset/connectors/base/models.py @@ -19,7 +19,18 @@ import json from datetime import datetime from enum import Enum -from typing import Any, Dict, Hashable, List, Optional, Set, Tuple, Type, TYPE_CHECKING, Union +from typing import ( + Any, + Dict, + Hashable, + List, + Optional, + Set, + Tuple, + Type, + TYPE_CHECKING, + Union, +) from flask_appbuilder.security.sqla.models import User from flask_babel import gettext as __ From 99a08738765f3c522ae7f2b7856bd8e6b2a66d93 Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 13 Apr 2023 05:46:37 -0700 Subject: [PATCH 03/20] Lint --- .../src/components/Datasource/ChangeDatasourceModal.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/superset-frontend/src/components/Datasource/ChangeDatasourceModal.tsx b/superset-frontend/src/components/Datasource/ChangeDatasourceModal.tsx index 5ae8929a2355b..5e837f0a26846 100644 --- a/superset-frontend/src/components/Datasource/ChangeDatasourceModal.tsx +++ b/superset-frontend/src/components/Datasource/ChangeDatasourceModal.tsx @@ -176,6 +176,7 @@ const ChangeDatasourceModal: FunctionComponent = ({ endpoint: `/api/v1/dataset/${confirmedDataset?.id}`, }) .then(({ json }) => { + // eslint-disable-next-line no-param-reassign json.result.type = 'table'; onDatasourceSave(json.result); onChange(`${confirmedDataset?.id}__table`); From 81e52dea9615e6c8c2b710bb84756df59af90586 Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 13 Apr 2023 05:48:57 -0700 Subject: [PATCH 04/20] rm debugging code --- .../src/explore/components/controls/DatasourceControl/index.jsx | 1 - 1 file changed, 1 deletion(-) diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx b/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx index 98b31c54f6744..c99193dd427d6 100644 --- a/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx +++ b/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx @@ -173,7 +173,6 @@ class DatasourceControl extends React.PureComponent { } onDatasourceSave = datasource => { - console.log(datasource); this.props.actions.changeDatasource(datasource); const { temporalColumns, defaultTemporalColumn } = getTemporalColumns(datasource); From 5ab4fc1fb02314db34e45cf16133e330232ccc5a Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 13 Apr 2023 05:58:39 -0700 Subject: [PATCH 05/20] Fix mypy --- superset/datasets/commands/exceptions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/datasets/commands/exceptions.py b/superset/datasets/commands/exceptions.py index 727ce6e37c354..e06e92802f04c 100644 --- a/superset/datasets/commands/exceptions.py +++ b/superset/datasets/commands/exceptions.py @@ -66,7 +66,7 @@ class DatasetEndpointUnsafeValidationError(ValidationError): Marshmallow validation error for unsafe dataset default endpoint """ - def __init__(self): + def __init__(self) -> None: super().__init__( [ _( From 6da1bb9cfb9d2de62e626f0e3b902ea8d6126056 Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 13 Apr 2023 07:11:39 -0700 Subject: [PATCH 06/20] Fixes --- superset/datasets/schemas.py | 2 +- tests/integration_tests/datasets/api_tests.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/superset/datasets/schemas.py b/superset/datasets/schemas.py index 103359a2c3f03..1d49bc1cfb432 100644 --- a/superset/datasets/schemas.py +++ b/superset/datasets/schemas.py @@ -55,7 +55,7 @@ class DatasetColumnsPutSchema(Schema): extra = fields.String(allow_none=True) filterable = fields.Boolean() groupby = fields.Boolean() - is_active = fields.Boolean() + is_active = fields.Boolean(allow_none=True) is_dttm = fields.Boolean(default=False) python_date_format = fields.String( allow_none=True, validate=[Length(1, 255), validate_python_date_format] diff --git a/tests/integration_tests/datasets/api_tests.py b/tests/integration_tests/datasets/api_tests.py index caedcdf0f246f..3cc199d7c1df7 100644 --- a/tests/integration_tests/datasets/api_tests.py +++ b/tests/integration_tests/datasets/api_tests.py @@ -1354,6 +1354,9 @@ def test_update_dataset_unsafe_default_endpoint(self): """ Dataset API: Test unsafe default endpoint """ + if backend() == "sqlite": + return + dataset = self.insert_default_dataset() self.login(username="admin") uri = f"api/v1/dataset/{dataset.id}" From 37f53b26f0c425edca080ef287f32d5d48f7993d Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 13 Apr 2023 07:55:08 -0700 Subject: [PATCH 07/20] Fix metric adding --- .../components/Datasource/DatasourceModal.tsx | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.tsx b/superset-frontend/src/components/Datasource/DatasourceModal.tsx index ba23efef0f729..86f0cac7fef33 100644 --- a/superset-frontend/src/components/Datasource/DatasourceModal.tsx +++ b/superset-frontend/src/components/Datasource/DatasourceModal.tsx @@ -119,18 +119,23 @@ const DatasourceModal: FunctionComponent = ({ is_managed_externally: currentDatasource.is_managed_externally, external_url: currentDatasource.external_url, metrics: currentDatasource?.metrics?.map( - (metric: Record) => ({ - id: metric.id, - expression: metric.expression, - description: metric.description, - metric_name: metric.metric_name, - metric_type: metric.metric_type, - d3format: metric.d3format, - verbose_name: metric.verbose_name, - warning_text: metric.warning_text, - uuid: metric.uuid, - extra: buildExtraJsonObject(metric), - }), + (metric: Record) => { + const metricBody: any = { + expression: metric.expression, + description: metric.description, + metric_name: metric.metric_name, + metric_type: metric.metric_type, + d3format: metric.d3format, + verbose_name: metric.verbose_name, + warning_text: metric.warning_text, + uuid: metric.uuid, + extra: buildExtraJsonObject(metric), + }; + if (!isNaN(metric.id as number)) { + metricBody.id = metric.id; + } + return metricBody; + }, ), columns: currentDatasource?.columns?.map( (column: Record) => ({ From 1ffdc9664ccc9435f5d280467627eb2e3a9e402b Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 13 Apr 2023 08:05:41 -0700 Subject: [PATCH 08/20] Add new fields to dataset api test --- tests/integration_tests/datasets/api_tests.py | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/integration_tests/datasets/api_tests.py b/tests/integration_tests/datasets/api_tests.py index 3cc199d7c1df7..c219b0c24b3b5 100644 --- a/tests/integration_tests/datasets/api_tests.py +++ b/tests/integration_tests/datasets/api_tests.py @@ -347,7 +347,41 @@ def test_get_dataset_item(self): "sql": None, "table_name": "energy_usage", "template_params": None, + "uid": "2__table", + "datasource_name": "energy_usage", + "name": "public.energy_usage", + "column_formats": {}, + "granularity_sqla": [], + "time_grain_sqla": [ + ['PT1S', 'Second'], + ['PT1M', 'Minute'], + ['PT1H', 'Hour'], + ['P1D', 'Day'], + ['P1W', 'Week'], + ['P1M', 'Month'], + ['P3M', 'Quarter'], + ['P1Y', 'Year'], + ], + "order_by_choices": [ + ['["source", true]', 'source [asc]'], + ['["source", false]', 'source [desc]'], + ['["target", true]', 'target [asc]'], + ['["target", false]', 'target [desc]'], + ['["value", true]', 'value [asc]'], + ['["value", false]', 'value [desc]'], + ], + "verbose_map": { + '__timestamp': 'Time', + 'count': 'COUNT(*)', + 'source': 'source', + 'sum__value': 'sum__value', + 'target': 'target', + 'value': 'value', + }, } + import pprint + pp = pprint.PrettyPrinter(indent=4) + pp.pprint(response["result"]) if response["result"]["database"]["backend"] not in ("presto", "hive"): assert { k: v for k, v in response["result"].items() if k in expected_result From 1002ce65be9448eef59a426351dc4c931a4c505b Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 13 Apr 2023 08:06:18 -0700 Subject: [PATCH 09/20] Formatting --- tests/integration_tests/datasets/api_tests.py | 41 ++++++++++--------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/tests/integration_tests/datasets/api_tests.py b/tests/integration_tests/datasets/api_tests.py index c219b0c24b3b5..1ecde518c8fa9 100644 --- a/tests/integration_tests/datasets/api_tests.py +++ b/tests/integration_tests/datasets/api_tests.py @@ -353,33 +353,34 @@ def test_get_dataset_item(self): "column_formats": {}, "granularity_sqla": [], "time_grain_sqla": [ - ['PT1S', 'Second'], - ['PT1M', 'Minute'], - ['PT1H', 'Hour'], - ['P1D', 'Day'], - ['P1W', 'Week'], - ['P1M', 'Month'], - ['P3M', 'Quarter'], - ['P1Y', 'Year'], + ["PT1S", "Second"], + ["PT1M", "Minute"], + ["PT1H", "Hour"], + ["P1D", "Day"], + ["P1W", "Week"], + ["P1M", "Month"], + ["P3M", "Quarter"], + ["P1Y", "Year"], ], "order_by_choices": [ - ['["source", true]', 'source [asc]'], - ['["source", false]', 'source [desc]'], - ['["target", true]', 'target [asc]'], - ['["target", false]', 'target [desc]'], - ['["value", true]', 'value [asc]'], - ['["value", false]', 'value [desc]'], + ['["source", true]', "source [asc]"], + ['["source", false]', "source [desc]"], + ['["target", true]', "target [asc]"], + ['["target", false]', "target [desc]"], + ['["value", true]', "value [asc]"], + ['["value", false]', "value [desc]"], ], "verbose_map": { - '__timestamp': 'Time', - 'count': 'COUNT(*)', - 'source': 'source', - 'sum__value': 'sum__value', - 'target': 'target', - 'value': 'value', + "__timestamp": "Time", + "count": "COUNT(*)", + "source": "source", + "sum__value": "sum__value", + "target": "target", + "value": "value", }, } import pprint + pp = pprint.PrettyPrinter(indent=4) pp.pprint(response["result"]) if response["result"]["database"]["backend"] not in ("presto", "hive"): From 83cf76c2879534cf64cc584706fd2a3af47b8705 Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 13 Apr 2023 08:07:23 -0700 Subject: [PATCH 10/20] rm debugging code --- tests/integration_tests/datasets/api_tests.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/integration_tests/datasets/api_tests.py b/tests/integration_tests/datasets/api_tests.py index 1ecde518c8fa9..e0ec38d4d5f1c 100644 --- a/tests/integration_tests/datasets/api_tests.py +++ b/tests/integration_tests/datasets/api_tests.py @@ -379,10 +379,6 @@ def test_get_dataset_item(self): "value": "value", }, } - import pprint - - pp = pprint.PrettyPrinter(indent=4) - pp.pprint(response["result"]) if response["result"]["database"]["backend"] not in ("presto", "hive"): assert { k: v for k, v in response["result"].items() if k in expected_result From 8c37492f728d677e9bce8d96b073afa8a3a397fe Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 13 Apr 2023 08:09:49 -0700 Subject: [PATCH 11/20] Fix test --- .../controls/DatasourceControl/DatasourceControl.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx index 8113a35e2d61b..612e2f1feb253 100644 --- a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx +++ b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx @@ -69,7 +69,7 @@ const createProps = (overrides: JsonObject = {}) => ({ }); async function openAndSaveChanges(datasource: any) { - fetchMock.post('glob:*/api/v1/dataset/*', datasource, { + fetchMock.put('glob:*/api/v1/dataset/*', datasource, { overwriteRoutes: true, }); userEvent.click(screen.getByTestId('datasource-menu-trigger')); From a372dba37984d16fb851349f36d7e1a8c5e8dd5d Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 13 Apr 2023 08:28:47 -0700 Subject: [PATCH 12/20] Fix lint --- superset-frontend/src/components/Datasource/DatasourceModal.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.tsx b/superset-frontend/src/components/Datasource/DatasourceModal.tsx index 86f0cac7fef33..4cf4e3f502fe3 100644 --- a/superset-frontend/src/components/Datasource/DatasourceModal.tsx +++ b/superset-frontend/src/components/Datasource/DatasourceModal.tsx @@ -131,7 +131,7 @@ const DatasourceModal: FunctionComponent = ({ uuid: metric.uuid, extra: buildExtraJsonObject(metric), }; - if (!isNaN(metric.id as number)) { + if (!Number.isNaN(Number(metric.id))) { metricBody.id = metric.id; } return metricBody; From 1dbc15df08567638739e2ddc2cf538d1c3c907e0 Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 13 Apr 2023 09:04:54 -0700 Subject: [PATCH 13/20] Fix test for mysql --- tests/integration_tests/datasets/api_tests.py | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/tests/integration_tests/datasets/api_tests.py b/tests/integration_tests/datasets/api_tests.py index e0ec38d4d5f1c..2add5d7aac48e 100644 --- a/tests/integration_tests/datasets/api_tests.py +++ b/tests/integration_tests/datasets/api_tests.py @@ -19,7 +19,7 @@ import unittest from io import BytesIO from typing import List, Optional -from unittest.mock import patch +from unittest.mock import ANY, patch from zipfile import is_zipfile, ZipFile import prison @@ -352,16 +352,7 @@ def test_get_dataset_item(self): "name": "public.energy_usage", "column_formats": {}, "granularity_sqla": [], - "time_grain_sqla": [ - ["PT1S", "Second"], - ["PT1M", "Minute"], - ["PT1H", "Hour"], - ["P1D", "Day"], - ["P1W", "Week"], - ["P1M", "Month"], - ["P3M", "Quarter"], - ["P1Y", "Year"], - ], + "time_grain_sqla": ANY, "order_by_choices": [ ['["source", true]', "source [asc]"], ['["source", false]', "source [desc]"], From bbe1ab89b13b61c43bf9c0a7e5d021f733318808 Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 13 Apr 2023 09:34:22 -0700 Subject: [PATCH 14/20] Fix test for mysql again --- tests/integration_tests/datasets/api_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration_tests/datasets/api_tests.py b/tests/integration_tests/datasets/api_tests.py index 2add5d7aac48e..95cb2a33e2b3f 100644 --- a/tests/integration_tests/datasets/api_tests.py +++ b/tests/integration_tests/datasets/api_tests.py @@ -349,7 +349,7 @@ def test_get_dataset_item(self): "template_params": None, "uid": "2__table", "datasource_name": "energy_usage", - "name": "public.energy_usage", + "name": f"{get_example_default_schema()}.energy_usage", "column_formats": {}, "granularity_sqla": [], "time_grain_sqla": ANY, From 8470eac8cd3c0276f18e52ff2b7c1926756d8a62 Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 13 Apr 2023 11:19:08 -0700 Subject: [PATCH 15/20] Fix frontend unit tests --- .../src/components/Datasource/ChangeDatasourceModal.test.jsx | 2 +- .../src/components/Datasource/DatasourceModal.test.jsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/components/Datasource/ChangeDatasourceModal.test.jsx b/superset-frontend/src/components/Datasource/ChangeDatasourceModal.test.jsx index a24c18c3770cd..5dbc52c38a5ee 100644 --- a/superset-frontend/src/components/Datasource/ChangeDatasourceModal.test.jsx +++ b/superset-frontend/src/components/Datasource/ChangeDatasourceModal.test.jsx @@ -112,6 +112,6 @@ describe('ChangeDatasourceModal', () => { }); await waitForComponentToPaint(wrapper); - expect(fetchMock.calls(/datasource\/get\/table\/7/)).toHaveLength(1); + expect(fetchMock.calls(/api\/v1\/dataset\/7/)).toHaveLength(1); }); }); diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx b/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx index 7b3395e2997d6..36fe6ec1dbbb2 100644 --- a/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx +++ b/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx @@ -96,7 +96,7 @@ describe('DatasourceModal', () => { it('saves on confirm', async () => { const callsP = fetchMock.post(SAVE_ENDPOINT, SAVE_PAYLOAD); - fetchMock.post(SAVE_DATASOURCE_ENDPOINT, {}); + fetchMock.put(SAVE_DATASOURCE_ENDPOINT, {}); act(() => { wrapper .find('button[data-test="datasource-modal-save"]') From d37b12636ed4b58be16ca9a16f48e1653c98dc39 Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 13 Apr 2023 12:35:56 -0700 Subject: [PATCH 16/20] Attempt to fix failing cypress test --- .../cypress-base/cypress/integration/explore/control.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts index faee1f6f4ee41..a88c8e444f895 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts @@ -45,7 +45,7 @@ describe('Datasource control', () => { '[data-test="table-content-rows"] [data-test="editable-title-input"]', ) .first() - .click(); + .click({ force: true }); cy.get( '[data-test="table-content-rows"] [data-test="editable-title-input"]', From d5048561d9fa2e04addd57908d611af06d3bf0d6 Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 13 Apr 2023 14:03:25 -0700 Subject: [PATCH 17/20] Fetch from get endpoint after update to get all needed fields --- .../cypress/integration/explore/control.test.ts | 2 +- .../src/components/Datasource/DatasourceModal.tsx | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts index a88c8e444f895..faee1f6f4ee41 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts @@ -45,7 +45,7 @@ describe('Datasource control', () => { '[data-test="table-content-rows"] [data-test="editable-title-input"]', ) .first() - .click({ force: true }); + .click(); cy.get( '[data-test="table-content-rows"] [data-test="editable-title-input"]', diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.tsx b/superset-frontend/src/components/Datasource/DatasourceModal.tsx index 4cf4e3f502fe3..b5b698f8315bb 100644 --- a/superset-frontend/src/components/Datasource/DatasourceModal.tsx +++ b/superset-frontend/src/components/Datasource/DatasourceModal.tsx @@ -160,10 +160,17 @@ const DatasourceModal: FunctionComponent = ({ ), }, }) - .then(({ json }) => { + .then(() => { addSuccessToast(t('The dataset has been saved')); + return SupersetClient.get({ + endpoint: `/api/v1/dataset/${currentDatasource?.id}`, + }); + }) + .then(({ json }) => { + // eslint-disable-next-line no-param-reassign + json.result.type = 'table'; onDatasourceSave({ - ...json, + ...json.result, owners: currentDatasource.owners, }); onHide(); From ce23c548249759ef5f9593304b5d4dce5d6193cb Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 13 Apr 2023 15:44:25 -0700 Subject: [PATCH 18/20] Update frontend tests to account for put then get --- .../Datasource/DatasourceModal.test.jsx | 10 ++++++++-- .../DatasourceControl.test.tsx | 17 ++++++++++++++--- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx b/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx index 36fe6ec1dbbb2..73436ca7e3164 100644 --- a/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx +++ b/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx @@ -41,6 +41,7 @@ const datasource = mockDatasource['7__table']; const SAVE_ENDPOINT = 'glob:*/api/v1/dataset/7'; const SAVE_PAYLOAD = { new: 'data' }; const SAVE_DATASOURCE_ENDPOINT = 'glob:*/api/v1/dataset/7'; +const GET_DATASOURCE_ENDPOINT = SAVE_DATASOURCE_ENDPOINT; const mockedProps = { datasource, @@ -94,9 +95,10 @@ describe('DatasourceModal', () => { expect(wrapper.find(DatasourceEditor)).toExist(); }); - it('saves on confirm', async () => { + it.only('saves on confirm', async () => { const callsP = fetchMock.post(SAVE_ENDPOINT, SAVE_PAYLOAD); fetchMock.put(SAVE_DATASOURCE_ENDPOINT, {}); + fetchMock.get(GET_DATASOURCE_ENDPOINT, {}); act(() => { wrapper .find('button[data-test="datasource-modal-save"]') @@ -111,7 +113,11 @@ describe('DatasourceModal', () => { okButton.simulate('click'); }); await waitForComponentToPaint(wrapper); - const expected = ['http://localhost/api/v1/dataset/7']; + // one call to PUT, then one to GET + const expected = [ + 'http://localhost/api/v1/dataset/7', + 'http://localhost/api/v1/dataset/7', + ]; expect(callsP._calls.map(call => call[0])).toEqual( expected, ); /* eslint no-underscore-dangle: 0 */ diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx index 612e2f1feb253..73dd9f9c8ea8a 100644 --- a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx +++ b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx @@ -69,9 +69,20 @@ const createProps = (overrides: JsonObject = {}) => ({ }); async function openAndSaveChanges(datasource: any) { - fetchMock.put('glob:*/api/v1/dataset/*', datasource, { - overwriteRoutes: true, - }); + fetchMock.put( + 'glob:*/api/v1/dataset/*', + {}, + { + overwriteRoutes: true, + }, + ); + fetchMock.get( + 'glob:*/api/v1/dataset/*', + { result: datasource }, + { + overwriteRoutes: true, + }, + ); userEvent.click(screen.getByTestId('datasource-menu-trigger')); userEvent.click(await screen.findByTestId('edit-dataset')); userEvent.click(await screen.findByTestId('datasource-modal-save')); From 6140fb1651a2cbf1beec283559fd4d6360b051a1 Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 13 Apr 2023 15:53:00 -0700 Subject: [PATCH 19/20] Oops --- .../src/components/Datasource/DatasourceModal.test.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx b/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx index 73436ca7e3164..7d378902e6e93 100644 --- a/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx +++ b/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx @@ -95,7 +95,7 @@ describe('DatasourceModal', () => { expect(wrapper.find(DatasourceEditor)).toExist(); }); - it.only('saves on confirm', async () => { + it('saves on confirm', async () => { const callsP = fetchMock.post(SAVE_ENDPOINT, SAVE_PAYLOAD); fetchMock.put(SAVE_DATASOURCE_ENDPOINT, {}); fetchMock.get(GET_DATASOURCE_ENDPOINT, {}); From 389ce012b6ada678ffbf913902e539e1dc3c94c5 Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 13 Apr 2023 18:54:03 -0700 Subject: [PATCH 20/20] Break up the mock interference --- .../controls/DatasourceControl/DatasourceControl.test.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx index 73dd9f9c8ea8a..80cb84ac8de7a 100644 --- a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx +++ b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx @@ -165,7 +165,7 @@ test('Should show SQL Lab for sql_lab role', async () => { test('Click on Swap dataset option', async () => { const props = createProps(); - SupersetClientGet.mockImplementation( + SupersetClientGet.mockImplementationOnce( async ({ endpoint }: { endpoint: string }) => { if (endpoint.includes('_info')) { return { @@ -193,7 +193,7 @@ test('Click on Swap dataset option', async () => { test('Click on Edit dataset', async () => { const props = createProps(); - SupersetClientGet.mockImplementation( + SupersetClientGet.mockImplementationOnce( async () => ({ json: { result: [] } } as any), ); render(, { @@ -217,7 +217,7 @@ test('Edit dataset should be disabled when user is not admin', async () => { // @ts-expect-error props.user.roles = {}; props.datasource.owners = []; - SupersetClientGet.mockImplementation( + SupersetClientGet.mockImplementationOnce( async () => ({ json: { result: [] } } as any), );