From 485852d7009cf2077ebb093a758b82f05be31a04 Mon Sep 17 00:00:00 2001 From: Daniel Vaz Gaspar Date: Mon, 13 Dec 2021 12:03:41 +0000 Subject: [PATCH 01/14] ci: temp fix for mysqlclient on an OS regression bug (#17724) --- scripts/python_tests.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scripts/python_tests.sh b/scripts/python_tests.sh index b9ef2cee21048..1681cc8af7ecd 100755 --- a/scripts/python_tests.sh +++ b/scripts/python_tests.sh @@ -18,6 +18,11 @@ # set -e +# Temporary fix, probably related with https://bugs.launchpad.net/ubuntu/+source/opencv/+bug/1890170 +# MySQL was failling with: +# from . import _mysql +# ImportError: /lib/x86_64-linux-gnu/libstdc++.so.6: cannot allocate memory in static TLS block +export LD_PRELOAD=/lib/x86_64-linux-gnu/libstdc++.so.6 export SUPERSET_CONFIG=${SUPERSET_CONFIG:-tests.integration_tests.superset_test_config} export SUPERSET_TESTENV=true echo "Superset config module: $SUPERSET_CONFIG" From 0d2299cb60b2b646a4280f648f1ca3a2ee44a9a2 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Mon, 13 Dec 2021 13:04:35 -0800 Subject: [PATCH 02/14] fix: migration out-of-scope bind (#17728) * fix: migration out-of-scope bind * Bypass flaky test --- ...abe27eaf93db_add_extra_config_column_to_alerts.py | 6 +++--- tests/integration_tests/celery_tests.py | 12 ++++++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/superset/migrations/versions/abe27eaf93db_add_extra_config_column_to_alerts.py b/superset/migrations/versions/abe27eaf93db_add_extra_config_column_to_alerts.py index c0cce0ac9e650..a956058434118 100644 --- a/superset/migrations/versions/abe27eaf93db_add_extra_config_column_to_alerts.py +++ b/superset/migrations/versions/abe27eaf93db_add_extra_config_column_to_alerts.py @@ -31,17 +31,17 @@ from sqlalchemy import String from sqlalchemy.sql import column, table -connection = op.get_bind() - report_schedule = table("report_schedule", column("extra", String)) def upgrade(): + bind = op.get_bind() + with op.batch_alter_table("report_schedule") as batch_op: batch_op.add_column( sa.Column("extra", sa.Text(), nullable=True, default="{}",), ) - connection.execute(report_schedule.update().values({"extra": "{}"})) + bind.execute(report_schedule.update().values({"extra": "{}"})) with op.batch_alter_table("report_schedule") as batch_op: batch_op.alter_column("extra", existing_type=sa.Text(), nullable=False) diff --git a/tests/integration_tests/celery_tests.py b/tests/integration_tests/celery_tests.py index c7465bcbe871e..bfcb73271b634 100644 --- a/tests/integration_tests/celery_tests.py +++ b/tests/integration_tests/celery_tests.py @@ -249,8 +249,8 @@ def test_run_sync_query_cta_config(setup_sqllab, ctas_method): lambda d, u, s, sql: CTAS_SCHEMA_NAME, ) def test_run_async_query_cta_config(setup_sqllab, ctas_method): - if backend() == "sqlite": - # sqlite doesn't support schemas + if backend() in {"sqlite", "mysql"}: + # sqlite doesn't support schemas, mysql is flaky return tmp_table_name = f"{TEST_ASYNC_CTA_CONFIG}_{ctas_method.lower()}" result = run_sql( @@ -272,6 +272,10 @@ def test_run_async_query_cta_config(setup_sqllab, ctas_method): @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") @pytest.mark.parametrize("ctas_method", [CtasMethod.TABLE, CtasMethod.VIEW]) def test_run_async_cta_query(setup_sqllab, ctas_method): + if backend() == "mysql": + # failing + return + table_name = f"{TEST_ASYNC_CTA}_{ctas_method.lower()}" result = run_sql( QUERY, cta=True, ctas_method=ctas_method, async_=True, tmp_table=table_name @@ -294,6 +298,10 @@ def test_run_async_cta_query(setup_sqllab, ctas_method): @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") @pytest.mark.parametrize("ctas_method", [CtasMethod.TABLE, CtasMethod.VIEW]) def test_run_async_cta_query_with_lower_limit(setup_sqllab, ctas_method): + if backend() == "mysql": + # failing + return + tmp_table = f"{TEST_ASYNC_LOWER_LIMIT}_{ctas_method.lower()}" result = run_sql( QUERY, cta=True, ctas_method=ctas_method, async_=True, tmp_table=tmp_table From e6db62c469b9dcf391015e7bb768a73316d9efbc Mon Sep 17 00:00:00 2001 From: cccs-joel Date: Mon, 13 Dec 2021 17:28:21 -0500 Subject: [PATCH 03/14] fix: Change datatype of column type in BaseColumn to allow larger datatype names for complexed columns (#17360) * Change datatype of column type in BaseColumn to allow larger datatype names for complexed columns * Fixed formatting * Added an entry in the UPDATING.md file as a potential downtime * Update UPDATING.md Accept proposed changes. Co-authored-by: Beto Dealmeida * Updated down revision number to reflect new head Co-authored-by: cccs-joel Co-authored-by: Beto Dealmeida --- UPDATING.md | 1 + superset/connectors/base/models.py | 2 +- superset/datasets/schemas.py | 2 +- ...5_change_datatype_of_type_in_basecolumn.py | 46 +++++++++++++++++++ 4 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 superset/migrations/versions/3ba29ecbaac5_change_datatype_of_type_in_basecolumn.py diff --git a/UPDATING.md b/UPDATING.md index 2533c1ffd0d44..1b44f078292d6 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -38,6 +38,7 @@ assists people when migrating to a new version. - [17539](https://github.com/apache/superset/pull/17539): all Superset CLI commands (init, load_examples and etc) require setting the FLASK_APP environment variable (which is set by default when .flaskenv is loaded) +- [17360](https://github.com/apache/superset/pull/17360): changes the column type from `VARCHAR(32)` to `TEXT` in table `table_columns`, potentially requiring a table lock on MySQL dbs or taking some time to complete on large deployments. ### Deprecations diff --git a/superset/connectors/base/models.py b/superset/connectors/base/models.py index 47d0a21657f19..f8a3261805e7c 100644 --- a/superset/connectors/base/models.py +++ b/superset/connectors/base/models.py @@ -578,7 +578,7 @@ class BaseColumn(AuditMixinNullable, ImportExportMixin): column_name = Column(String(255), nullable=False) verbose_name = Column(String(1024)) is_active = Column(Boolean, default=True) - type = Column(String(32)) + type = Column(Text) groupby = Column(Boolean, default=True) filterable = Column(Boolean, default=True) description = Column(Text) diff --git a/superset/datasets/schemas.py b/superset/datasets/schemas.py index 58258b1dda158..637e282a2b91d 100644 --- a/superset/datasets/schemas.py +++ b/superset/datasets/schemas.py @@ -42,7 +42,7 @@ def validate_python_date_format(value: str) -> None: class DatasetColumnsPutSchema(Schema): id = fields.Integer() column_name = fields.String(required=True, validate=Length(1, 255)) - type = fields.String(validate=Length(1, 32)) + type = fields.String(allow_none=True) verbose_name = fields.String(allow_none=True, Length=(1, 1024)) description = fields.String(allow_none=True) expression = fields.String(allow_none=True) diff --git a/superset/migrations/versions/3ba29ecbaac5_change_datatype_of_type_in_basecolumn.py b/superset/migrations/versions/3ba29ecbaac5_change_datatype_of_type_in_basecolumn.py new file mode 100644 index 0000000000000..15f81488a310c --- /dev/null +++ b/superset/migrations/versions/3ba29ecbaac5_change_datatype_of_type_in_basecolumn.py @@ -0,0 +1,46 @@ +# 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. +"""Change datatype of type in BaseColumn + +Revision ID: 3ba29ecbaac5 +Revises: abe27eaf93db +Create Date: 2021-11-02 17:44:51.792138 + +""" + +# revision identifiers, used by Alembic. +revision = "3ba29ecbaac5" +down_revision = "abe27eaf93db" + +import sqlalchemy as sa +from alembic import op +from sqlalchemy.dialects import postgresql + + +def upgrade(): + + with op.batch_alter_table("table_columns") as batch_op: + batch_op.alter_column( + "type", existing_type=sa.VARCHAR(length=32), type_=sa.TEXT() + ) + + +def downgrade(): + with op.batch_alter_table("table_columns") as batch_op: + batch_op.alter_column( + "type", existing_type=sa.TEXT(), type_=sa.VARCHAR(length=32) + ) From 67fdeffe346ae20d6ad57b0125c93c0d42b84740 Mon Sep 17 00:00:00 2001 From: Jason Cahela <49314834+jcahela@users.noreply.github.com> Date: Mon, 13 Dec 2021 17:14:18 -0600 Subject: [PATCH 04/14] fixed misspelling of apprear to appear (#17735) --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 564a04a5ddb84..a14653688e284 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1341,7 +1341,7 @@ Chart parameters are stored as a JSON encoded string the `slices.params` column The following tables provide a non-exhausive list of the various fields which can be present in the JSON object grouped by the Explorer pane sections. These values were obtained by extracting the distinct fields from a legacy deployment consisting of tens of thousands of charts and thus some fields may be missing whilst others may be deprecated. -Note not all fields are correctly categorized. The fields vary based on visualization type and may apprear in different sections depending on the type. Verified deprecated columns may indicate a missing migration and/or prior migrations which were unsuccessful and thus future work may be required to clean up the form-data. +Note not all fields are correctly categorized. The fields vary based on visualization type and may appear in different sections depending on the type. Verified deprecated columns may indicate a missing migration and/or prior migrations which were unsuccessful and thus future work may be required to clean up the form-data. ### Datasource & Chart Type From 12d307935507b52ca876ec3d45cf301d48978f4d Mon Sep 17 00:00:00 2001 From: Josue Lugaro <82119536+JosueLugaro@users.noreply.github.com> Date: Mon, 13 Dec 2021 17:32:01 -0600 Subject: [PATCH 05/14] chore: fixed spelling error on line 1342 of CONTRIBUTING.md (#17737) --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a14653688e284..3c5a02c7e501b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1339,7 +1339,7 @@ More information on the async query feature can be found in [SIP-39](https://git Chart parameters are stored as a JSON encoded string the `slices.params` column and are often referenced throughout the code as form-data. Currently the form-data is neither versioned nor typed as thus is somewhat free-formed. Note in the future there may be merit in using something like [JSON Schema](https://json-schema.org/) to both annotate and validate the JSON object in addition to using a Mypy `TypedDict` (introduced in Python 3.8) for typing the form-data in the backend. This section serves as a potential primer for that work. -The following tables provide a non-exhausive list of the various fields which can be present in the JSON object grouped by the Explorer pane sections. These values were obtained by extracting the distinct fields from a legacy deployment consisting of tens of thousands of charts and thus some fields may be missing whilst others may be deprecated. +The following tables provide a non-exhaustive list of the various fields which can be present in the JSON object grouped by the Explorer pane sections. These values were obtained by extracting the distinct fields from a legacy deployment consisting of tens of thousands of charts and thus some fields may be missing whilst others may be deprecated. Note not all fields are correctly categorized. The fields vary based on visualization type and may appear in different sections depending on the type. Verified deprecated columns may indicate a missing migration and/or prior migrations which were unsuccessful and thus future work may be required to clean up the form-data. From fceabf6bc5f9eacb082f321185aa2c1cbe657a40 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Mon, 13 Dec 2021 16:20:55 -0800 Subject: [PATCH 06/14] fix: import dash with missing immune ID (#17732) --- .../dashboards/commands/importers/v1/utils.py | 4 +- tests/unit_tests/dashboards/__init__.py | 16 +++++ .../dashboards/commands/__init__.py | 16 +++++ .../dashboards/commands/importers/__init__.py | 16 +++++ .../commands/importers/v1/__init__.py | 16 +++++ .../commands/importers/v1/utils_test.py | 71 +++++++++++++++++++ 6 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 tests/unit_tests/dashboards/__init__.py create mode 100644 tests/unit_tests/dashboards/commands/__init__.py create mode 100644 tests/unit_tests/dashboards/commands/importers/__init__.py create mode 100644 tests/unit_tests/dashboards/commands/importers/v1/__init__.py create mode 100644 tests/unit_tests/dashboards/commands/importers/v1/utils_test.py diff --git a/superset/dashboards/commands/importers/v1/utils.py b/superset/dashboards/commands/importers/v1/utils.py index fd5b4ee51bd66..0067c2c524c2a 100644 --- a/superset/dashboards/commands/importers/v1/utils.py +++ b/superset/dashboards/commands/importers/v1/utils.py @@ -90,7 +90,9 @@ def update_id_refs( # pylint: disable=too-many-locals for columns in metadata["filter_scopes"].values(): for attributes in columns.values(): attributes["immune"] = [ - id_map[old_id] for old_id in attributes["immune"] + id_map[old_id] + for old_id in attributes["immune"] + if old_id in id_map ] if "expanded_slices" in metadata: diff --git a/tests/unit_tests/dashboards/__init__.py b/tests/unit_tests/dashboards/__init__.py new file mode 100644 index 0000000000000..13a83393a9124 --- /dev/null +++ b/tests/unit_tests/dashboards/__init__.py @@ -0,0 +1,16 @@ +# 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. diff --git a/tests/unit_tests/dashboards/commands/__init__.py b/tests/unit_tests/dashboards/commands/__init__.py new file mode 100644 index 0000000000000..13a83393a9124 --- /dev/null +++ b/tests/unit_tests/dashboards/commands/__init__.py @@ -0,0 +1,16 @@ +# 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. diff --git a/tests/unit_tests/dashboards/commands/importers/__init__.py b/tests/unit_tests/dashboards/commands/importers/__init__.py new file mode 100644 index 0000000000000..13a83393a9124 --- /dev/null +++ b/tests/unit_tests/dashboards/commands/importers/__init__.py @@ -0,0 +1,16 @@ +# 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. diff --git a/tests/unit_tests/dashboards/commands/importers/v1/__init__.py b/tests/unit_tests/dashboards/commands/importers/v1/__init__.py new file mode 100644 index 0000000000000..13a83393a9124 --- /dev/null +++ b/tests/unit_tests/dashboards/commands/importers/v1/__init__.py @@ -0,0 +1,16 @@ +# 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. diff --git a/tests/unit_tests/dashboards/commands/importers/v1/utils_test.py b/tests/unit_tests/dashboards/commands/importers/v1/utils_test.py new file mode 100644 index 0000000000000..7ff829437b486 --- /dev/null +++ b/tests/unit_tests/dashboards/commands/importers/v1/utils_test.py @@ -0,0 +1,71 @@ +# 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. +# pylint: disable=import-outside-toplevel, unused-argument + +from typing import Any, Dict + + +def test_update_id_refs_immune_missing( # pylint: disable=invalid-name + app_context: None, +): + """ + Test that missing immune charts are ignored. + + A chart might be removed from a dashboard but still remain in the list of charts + immune to filters. The missing chart ID should be simply ignored when the + dashboard is imported. + """ + from superset.dashboards.commands.importers.v1.utils import update_id_refs + + config = { + "position": { + "CHART1": { + "id": "CHART1", + "meta": {"chartId": 101, "uuid": "uuid1",}, + "type": "CHART", + }, + "CHART2": { + "id": "CHART2", + "meta": {"chartId": 102, "uuid": "uuid2",}, + "type": "CHART", + }, + }, + "metadata": { + "filter_scopes": {"101": {"filter_name": {"immune": [102, 103],},},}, + }, + "native_filter_configuration": [], + } + chart_ids = {"uuid1": 1, "uuid2": 2} + dataset_info: Dict[str, Dict[str, Any]] = {} # not used + + fixed = update_id_refs(config, chart_ids, dataset_info) + assert fixed == { + "position": { + "CHART1": { + "id": "CHART1", + "meta": {"chartId": 1, "uuid": "uuid1"}, + "type": "CHART", + }, + "CHART2": { + "id": "CHART2", + "meta": {"chartId": 2, "uuid": "uuid2"}, + "type": "CHART", + }, + }, + "metadata": {"filter_scopes": {"1": {"filter_name": {"immune": [2]}}}}, + "native_filter_configuration": [], + } From 89d0d38ed0eb211d44de8067bd091392a0f84f85 Mon Sep 17 00:00:00 2001 From: Yahya Kayani <42173629+Yahyakiani@users.noreply.github.com> Date: Tue, 14 Dec 2021 11:41:04 +0500 Subject: [PATCH 07/14] fix(Mixed Timeseries Chart): Custom Metric Label (#17649) * fix(Mixed Timeseries Chart): Custom Metric Label * Fixed Formatting * Fixed Type mismatch from queryFormData * Reverted type change and used extracted datasource * Type fix for mapping --- .../src/MixedTimeseries/transformProps.ts | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts index 3d7b8174bee7e..1f39a4ade9209 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts @@ -33,6 +33,7 @@ import { DEFAULT_FORM_DATA, EchartsMixedTimeseriesFormData, EchartsMixedTimeseriesChartTransformedProps, + EchartsMixedTimeseriesProps, } from './types'; import { ForecastSeriesEnum } from '../types'; import { parseYAxisBound } from '../utils/controls'; @@ -63,14 +64,22 @@ import { import { TIMESERIES_CONSTANTS } from '../constants'; export default function transformProps( - chartProps: EchartsMixedTimeseriesFormData, + chartProps: EchartsMixedTimeseriesProps, ): EchartsMixedTimeseriesChartTransformedProps { - const { width, height, formData, queriesData, hooks, filterState } = - chartProps; + const { + width, + height, + formData, + queriesData, + hooks, + filterState, + datasource, + } = chartProps; const { annotation_data: annotationData_ } = queriesData[0]; const annotationData = annotationData_ || {}; - const data1: TimeseriesDataRecord[] = queriesData[0].data || []; - const data2: TimeseriesDataRecord[] = queriesData[1].data || []; + const { verboseMap = {} } = datasource; + const data1 = (queriesData[0].data || []) as TimeseriesDataRecord[]; + const data2 = (queriesData[1].data || []) as TimeseriesDataRecord[]; const { area, @@ -121,10 +130,12 @@ export default function transformProps( }: EchartsMixedTimeseriesFormData = { ...DEFAULT_FORM_DATA, ...formData }; const colorScale = CategoricalColorNamespace.getScale(colorScheme as string); - const rawSeriesA = extractTimeseriesSeries(rebaseTimeseriesDatum(data1), { + const rebasedDataA = rebaseTimeseriesDatum(data1, verboseMap); + const rawSeriesA = extractTimeseriesSeries(rebasedDataA, { fillNeighborValue: stack ? 0 : undefined, }); - const rawSeriesB = extractTimeseriesSeries(rebaseTimeseriesDatum(data2), { + const rebasedDataB = rebaseTimeseriesDatum(data2, verboseMap); + const rawSeriesB = extractTimeseriesSeries(rebasedDataB, { fillNeighborValue: stackB ? 0 : undefined, }); From 07bbe8448b2a63a6b22cb4b9670fa37f984ee5c8 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Tue, 14 Dec 2021 16:19:55 +0800 Subject: [PATCH 08/14] refactor(monorepo): change coverage of core to 100% (#17698) --- .codecov.yml | 2 +- superset-frontend/jest.config.js | 7 +-- .../src/color/CategoricalColorNamespace.ts | 7 +-- .../src/connection/SupersetClientClass.ts | 15 +++--- .../superset-ui-core/src/utils/random.ts | 9 ++-- .../connection/SupersetClientClass.test.ts | 49 +++++++++++++++---- .../test/translation/Translator.test.ts | 6 +-- .../translation/TranslatorSingleton.test.ts | 21 +++++--- .../test/translation/index.test.ts | 2 +- .../test/translation/languagePacks/en.ts | 2 +- .../test/translation/languagePacks/zh.ts | 2 +- .../test/utils/logging.test.ts | 39 ++++++++------- 12 files changed, 95 insertions(+), 66 deletions(-) diff --git a/.codecov.yml b/.codecov.yml index 4d86439e9969d..149042367ab18 100644 --- a/.codecov.yml +++ b/.codecov.yml @@ -16,7 +16,7 @@ coverage: target: auto threshold: 0% core-packages-ts: - target: 95% + target: 100% paths: - 'superset-frontend/packages' - '!superset-frontend/packages/**/*.jsx' diff --git a/superset-frontend/jest.config.js b/superset-frontend/jest.config.js index bc961ad3d16ce..90af1ee6cd343 100644 --- a/superset-frontend/jest.config.js +++ b/superset-frontend/jest.config.js @@ -56,9 +56,10 @@ module.exports = { coverageReporters: ['lcov', 'json-summary', 'html'], transform: { '^.+\\.jsx?$': 'babel-jest', - // ts-jest can't load plugin 'babel-plugin-typescript-to-proptypes' - 'reactify\\.tsx$': 'babel-jest', - '^.+\\.tsx?$': 'ts-jest', + // ts-jest doesn't work with `--coverage`. @superset-ui/core should + // 100% coverage, so we use babel-jest in packages and plugins. + '(plugins|packages)\\/.+\\.tsx?$': 'babel-jest', + '(((?!(plugins|packages)).)*)\\/.+\\.tsx?$': 'ts-jest', }, moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json'], snapshotSerializers: ['@emotion/jest/enzyme-serializer'], diff --git a/superset-frontend/packages/superset-ui-core/src/color/CategoricalColorNamespace.ts b/superset-frontend/packages/superset-ui-core/src/color/CategoricalColorNamespace.ts index 7e28cae6babac..9c56d5114b9d9 100644 --- a/superset-frontend/packages/superset-ui-core/src/color/CategoricalColorNamespace.ts +++ b/superset-frontend/packages/superset-ui-core/src/color/CategoricalColorNamespace.ts @@ -40,12 +40,7 @@ export default class CategoricalColorNamespace { getScale(schemeId?: string) { const id = schemeId ?? getCategoricalSchemeRegistry().getDefaultKey() ?? ''; const scheme = getCategoricalSchemeRegistry().get(id); - const newScale = new CategoricalColorScale( - scheme?.colors ?? [], - this.forcedItems, - ); - - return newScale; + return new CategoricalColorScale(scheme?.colors ?? [], this.forcedItems); } /** diff --git a/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts b/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts index 4959fb1bf2d5b..ef52134f31376 100644 --- a/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts +++ b/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts @@ -33,13 +33,6 @@ import { } from './types'; import { DEFAULT_FETCH_RETRY_OPTIONS, DEFAULT_BASE_URL } from './constants'; -function redirectUnauthorized() { - // the next param will be picked by flask to redirect the user after the login - setTimeout(() => { - window.location.href = `/login?next=${window.location.href}`; - }); -} - export default class SupersetClientClass { credentials: Credentials; @@ -159,8 +152,8 @@ export default class SupersetClientClass { timeout: timeout ?? this.timeout, fetchRetryOptions: fetchRetryOptions ?? this.fetchRetryOptions, }).catch(res => { - if (res && res.status === 401) { - redirectUnauthorized(); + if (res?.status === 401) { + this.redirectUnauthorized(); } return Promise.reject(res); }); @@ -226,4 +219,8 @@ export default class SupersetClientClass { endpoint[0] === '/' ? endpoint.slice(1) : endpoint }`; } + + redirectUnauthorized() { + window.location.href = `/login?next=${window.location.href}`; + } } diff --git a/superset-frontend/packages/superset-ui-core/src/utils/random.ts b/superset-frontend/packages/superset-ui-core/src/utils/random.ts index 38dbdbd10bba2..0edddfd178a31 100644 --- a/superset-frontend/packages/superset-ui-core/src/utils/random.ts +++ b/superset-frontend/packages/superset-ui-core/src/utils/random.ts @@ -16,15 +16,12 @@ * specific language governing permissions and limitations * under the License. */ -import seedrandom from 'seedrandom'; - -let random = seedrandom('superset-ui'); +import _seedrandom from 'seedrandom'; export function seed(seed: string) { - random = seedrandom(seed); - return random; + return _seedrandom(seed); } export function seedRandom() { - return random(); + return _seedrandom('superset-ui')(); } diff --git a/superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts b/superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts index 2edb0f46df83f..4b2307c9d4f26 100644 --- a/superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts @@ -17,10 +17,7 @@ * under the License. */ import fetchMock from 'fetch-mock'; -import { - SupersetClientClass, - ClientConfig, -} from '@superset-ui/core/src/connection'; +import { SupersetClientClass, ClientConfig, CallApi } from '@superset-ui/core'; import { LOGIN_GLOB } from './fixtures/constants'; describe('SupersetClientClass', () => { @@ -321,7 +318,7 @@ describe('SupersetClientClass', () => { await client.init(); await client.get({ url: mockGetUrl }); - const fetchRequest = fetchMock.calls(mockGetUrl)[0][1]; + const fetchRequest = fetchMock.calls(mockGetUrl)[0][1] as CallApi; expect(fetchRequest.mode).toBe(clientConfig.mode); expect(fetchRequest.credentials).toBe(clientConfig.credentials); expect(fetchRequest.headers).toEqual( @@ -378,7 +375,7 @@ describe('SupersetClientClass', () => { await client.init(); await client.get({ url: mockGetUrl, ...overrideConfig }); - const fetchRequest = fetchMock.calls(mockGetUrl)[0][1]; + const fetchRequest = fetchMock.calls(mockGetUrl)[0][1] as CallApi; expect(fetchRequest.mode).toBe(overrideConfig.mode); expect(fetchRequest.credentials).toBe(overrideConfig.credentials); expect(fetchRequest.headers).toEqual( @@ -423,7 +420,7 @@ describe('SupersetClientClass', () => { await client.init(); await client.post({ url: mockPostUrl, ...overrideConfig }); - const fetchRequest = fetchMock.calls(mockPostUrl)[0][1]; + const fetchRequest = fetchMock.calls(mockPostUrl)[0][1] as CallApi; expect(fetchRequest.mode).toBe(overrideConfig.mode); expect(fetchRequest.credentials).toBe(overrideConfig.credentials); @@ -454,7 +451,8 @@ describe('SupersetClientClass', () => { await client.init(); await client.post({ url: mockPostUrl, postPayload }); - const formData = fetchMock.calls(mockPostUrl)[0][1].body as FormData; + const fetchRequest = fetchMock.calls(mockPostUrl)[0][1] as CallApi; + const formData = fetchRequest.body as FormData; expect(fetchMock.calls(mockPostUrl)).toHaveLength(1); Object.entries(postPayload).forEach(([key, value]) => { @@ -470,7 +468,8 @@ describe('SupersetClientClass', () => { await client.init(); await client.post({ url: mockPostUrl, postPayload, stringify: false }); - const formData = fetchMock.calls(mockPostUrl)[0][1].body as FormData; + const fetchRequest = fetchMock.calls(mockPostUrl)[0][1] as CallApi; + const formData = fetchRequest.body as FormData; expect(fetchMock.calls(mockPostUrl)).toHaveLength(1); Object.entries(postPayload).forEach(([key, value]) => { @@ -479,4 +478,36 @@ describe('SupersetClientClass', () => { }); }); }); + + it('should redirect Unauthorized', async () => { + const mockRequestUrl = 'https://host/get/url'; + const { location } = window; + // @ts-ignore + delete window.location; + // @ts-ignore + window.location = { href: mockRequestUrl }; + const authSpy = jest + .spyOn(SupersetClientClass.prototype, 'ensureAuth') + .mockImplementation(); + const rejectValue = { status: 401 }; + fetchMock.get(mockRequestUrl, () => Promise.reject(rejectValue), { + overwriteRoutes: true, + }); + + const client = new SupersetClientClass({}); + + let error; + try { + await client.request({ url: mockRequestUrl, method: 'GET' }); + } catch (err) { + error = err; + } finally { + const redirectURL = window.location.href; + expect(redirectURL).toBe(`/login?next=${mockRequestUrl}`); + expect(error.status).toBe(401); + } + + authSpy.mockReset(); + window.location = location; + }); }); diff --git a/superset-frontend/packages/superset-ui-core/test/translation/Translator.test.ts b/superset-frontend/packages/superset-ui-core/test/translation/Translator.test.ts index bab9081a7cdb0..703d9a5d309c0 100644 --- a/superset-frontend/packages/superset-ui-core/test/translation/Translator.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/translation/Translator.test.ts @@ -17,16 +17,16 @@ * under the License. */ -import { logging } from '@superset-ui/core'; -import Translator from '@superset-ui/core/src/translation/Translator'; import { + logging, configure, t, tn, addLocaleData, addTranslation, addTranslations, -} from '@superset-ui/core/src/translation/TranslatorSingleton'; +} from '@superset-ui/core'; +import Translator from '../../src/translation/Translator'; import languagePackZh from './languagePacks/zh'; import languagePackEn from './languagePacks/en'; diff --git a/superset-frontend/packages/superset-ui-core/test/translation/TranslatorSingleton.test.ts b/superset-frontend/packages/superset-ui-core/test/translation/TranslatorSingleton.test.ts index 24ccd260d4fa1..af0d6f5915524 100644 --- a/superset-frontend/packages/superset-ui-core/test/translation/TranslatorSingleton.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/translation/TranslatorSingleton.test.ts @@ -19,13 +19,8 @@ /* eslint no-console: 0 */ import mockConsole from 'jest-mock-console'; -import Translator from '@superset-ui/core/src/translation/Translator'; -import { - configure, - resetTranslation, - t, - tn, -} from '@superset-ui/core/src/translation/TranslatorSingleton'; +import { configure, resetTranslation, t, tn } from '@superset-ui/core'; +import Translator from '../../src/translation/Translator'; import languagePackEn from './languagePacks/en'; import languagePackZh from './languagePacks/zh'; @@ -76,4 +71,16 @@ describe('TranslatorSingleton', () => { }); }); }); + it('should be reset translation setting', () => { + configure(); + expect(t('second')).toEqual('second'); + + resetTranslation(); + const restoreConsole = mockConsole(); + expect(t('second')).toEqual('second'); + resetTranslation(); + expect(t('second')).toEqual('second'); + expect(console.warn).toBeCalledTimes(2); + restoreConsole(); + }); }); diff --git a/superset-frontend/packages/superset-ui-core/test/translation/index.test.ts b/superset-frontend/packages/superset-ui-core/test/translation/index.test.ts index aecc3e6fafcfb..2c0f0d6f92d5b 100644 --- a/superset-frontend/packages/superset-ui-core/test/translation/index.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/translation/index.test.ts @@ -17,7 +17,7 @@ * under the License. */ -import { configure, t, tn } from '@superset-ui/core/src/translation'; +import { configure, t, tn } from '@superset-ui/core'; describe('index', () => { it('exports configure()', () => { diff --git a/superset-frontend/packages/superset-ui-core/test/translation/languagePacks/en.ts b/superset-frontend/packages/superset-ui-core/test/translation/languagePacks/en.ts index 0efcc12645bcd..00d52be8a2728 100644 --- a/superset-frontend/packages/superset-ui-core/test/translation/languagePacks/en.ts +++ b/superset-frontend/packages/superset-ui-core/test/translation/languagePacks/en.ts @@ -17,7 +17,7 @@ * under the License. */ -import { LanguagePack } from '@superset-ui/core/src/translation'; +import { LanguagePack } from '@superset-ui/core'; const languagePack: LanguagePack = { domain: 'superset', diff --git a/superset-frontend/packages/superset-ui-core/test/translation/languagePacks/zh.ts b/superset-frontend/packages/superset-ui-core/test/translation/languagePacks/zh.ts index 105a65431e564..420a2bda1e417 100644 --- a/superset-frontend/packages/superset-ui-core/test/translation/languagePacks/zh.ts +++ b/superset-frontend/packages/superset-ui-core/test/translation/languagePacks/zh.ts @@ -17,7 +17,7 @@ * under the License. */ -import { LanguagePack } from '@superset-ui/core/src/translation'; +import { LanguagePack } from '@superset-ui/core'; const languagePack: LanguagePack = { domain: 'superset', diff --git a/superset-frontend/packages/superset-ui-core/test/utils/logging.test.ts b/superset-frontend/packages/superset-ui-core/test/utils/logging.test.ts index c5e158c946355..cdf4df89cf377 100644 --- a/superset-frontend/packages/superset-ui-core/test/utils/logging.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/utils/logging.test.ts @@ -21,40 +21,41 @@ describe('logging', () => { beforeEach(() => { jest.resetModules(); - // Explicit is better than implicit - console.warn = console.error = function mockedConsole(message) { - throw new Error(message); - }; + jest.resetAllMocks(); }); + it('should pipe to `console` methods', () => { - const { logging } = require('@superset-ui/core/src'); + const { logging } = require('@superset-ui/core'); + jest.spyOn(logging, 'debug').mockImplementation(); + jest.spyOn(logging, 'log').mockImplementation(); + jest.spyOn(logging, 'info').mockImplementation(); expect(() => { logging.debug(); logging.log(); logging.info(); }).not.toThrow(); - expect(() => { - logging.warn('warn'); - }).toThrow('warn'); - expect(() => { - logging.error('error'); - }).toThrow('error'); - // to support: npx jest --silent - const spy = jest.spyOn(logging, 'trace'); - spy.mockImplementation(() => { + jest.spyOn(logging, 'warn').mockImplementation(() => { + throw new Error('warn'); + }); + expect(() => logging.warn()).toThrow('warn'); + + jest.spyOn(logging, 'error').mockImplementation(() => { + throw new Error('error'); + }); + expect(() => logging.error()).toThrow('error'); + + jest.spyOn(logging, 'trace').mockImplementation(() => { throw new Error('Trace:'); }); - expect(() => { - logging.trace(); - }).toThrow('Trace:'); - spy.mockRestore(); + expect(() => logging.trace()).toThrow('Trace:'); }); + it('should use noop functions when console unavailable', () => { const { console } = window; Object.assign(window, { console: undefined }); - const { logging } = require('@superset-ui/core/src'); + const { logging } = require('@superset-ui/core'); afterAll(() => { Object.assign(window, { console }); From e2e79923d305da3995c35022bff41a7d68bdfb8b Mon Sep 17 00:00:00 2001 From: Daniel Vaz Gaspar Date: Tue, 14 Dec 2021 14:19:52 +0000 Subject: [PATCH 09/14] chore: bump FAB to 3.4.1 (#17723) * chore: bump FAB to 3.4.1 * test --- requirements/base.txt | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements/base.txt b/requirements/base.txt index 239d53adc43c3..575e200035ebc 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -77,7 +77,7 @@ flask==1.1.4 # flask-openid # flask-sqlalchemy # flask-wtf -flask-appbuilder==3.4.1rc2 +flask-appbuilder==3.4.1 # via apache-superset flask-babel==1.0.0 # via flask-appbuilder diff --git a/setup.py b/setup.py index 5788db80f729a..44cf9b067eeb9 100644 --- a/setup.py +++ b/setup.py @@ -75,7 +75,7 @@ def get_git_sha() -> str: "cryptography>=3.3.2", "deprecation>=2.1.0, <2.2.0", "flask>=1.1.0, <2.0.0", - "flask-appbuilder>=3.4.1rc2, <4.0.0", + "flask-appbuilder>=3.4.1, <4.0.0", "flask-caching>=1.10.0", "flask-compress", "flask-talisman", From 2633bcccc372f0059a23cd5a8367a983f388dcb2 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Tue, 14 Dec 2021 07:23:55 -0800 Subject: [PATCH 10/14] fix: import dashboard stale filter_scopes (#17741) --- superset/dashboards/commands/importers/v1/utils.py | 2 ++ .../dashboards/commands/importers/v1/utils_test.py | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/superset/dashboards/commands/importers/v1/utils.py b/superset/dashboards/commands/importers/v1/utils.py index 0067c2c524c2a..c3dee1578e306 100644 --- a/superset/dashboards/commands/importers/v1/utils.py +++ b/superset/dashboards/commands/importers/v1/utils.py @@ -84,6 +84,7 @@ def update_id_refs( # pylint: disable=too-many-locals metadata["filter_scopes"] = { str(id_map[int(old_id)]): columns for old_id, columns in metadata["filter_scopes"].items() + if int(old_id) in id_map } # now update columns to use new IDs: @@ -107,6 +108,7 @@ def update_id_refs( # pylint: disable=too-many-locals { str(id_map[int(old_id)]): value for old_id, value in default_filters.items() + if int(old_id) in id_map } ) diff --git a/tests/unit_tests/dashboards/commands/importers/v1/utils_test.py b/tests/unit_tests/dashboards/commands/importers/v1/utils_test.py index 7ff829437b486..d0ee3157f6b2f 100644 --- a/tests/unit_tests/dashboards/commands/importers/v1/utils_test.py +++ b/tests/unit_tests/dashboards/commands/importers/v1/utils_test.py @@ -45,7 +45,10 @@ def test_update_id_refs_immune_missing( # pylint: disable=invalid-name }, }, "metadata": { - "filter_scopes": {"101": {"filter_name": {"immune": [102, 103],},},}, + "filter_scopes": { + "101": {"filter_name": {"immune": [102, 103]}}, + "104": {"filter_name": {"immune": [102, 103]}}, + }, }, "native_filter_configuration": [], } From 215ee08a475c1ba0e49e58213ce2dbec14bf1b16 Mon Sep 17 00:00:00 2001 From: "Hugh A. Miles II" Date: Tue, 14 Dec 2021 12:46:57 -0500 Subject: [PATCH 11/14] feat: Update makefile with frontend build (#17734) * Update Makefile * vibess --- Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Makefile b/Makefile index 51c2b0397d89c..e253d37786f46 100644 --- a/Makefile +++ b/Makefile @@ -46,6 +46,9 @@ superset: # Load some data to play with superset load-examples + # Install node packages + cd superset-frontend; npm install + update: update-py update-js update-py: From 63d9693f21786431ba7e2ec11d6658bcd3a1f9e9 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Tue, 14 Dec 2021 14:47:08 -0800 Subject: [PATCH 12/14] feat: add main datetime column to dataset editor (#17739) * feat: add main dttm col to dataset editor * Add tests --- .../spec/fixtures/mockDatasource.js | 1 + .../Datasource/DatasourceEditor.jsx | 56 ++++++++++++++++--- .../Datasource/DatasourceEditor.test.jsx | 26 +++++++++ .../datasets/examples/cleaned_sales_data.yaml | 2 +- 4 files changed, 77 insertions(+), 8 deletions(-) diff --git a/superset-frontend/spec/fixtures/mockDatasource.js b/superset-frontend/spec/fixtures/mockDatasource.js index ca60a712b2088..30513fc126748 100644 --- a/superset-frontend/spec/fixtures/mockDatasource.js +++ b/superset-frontend/spec/fixtures/mockDatasource.js @@ -167,6 +167,7 @@ export default { column_types: [0, 1, 2], id, granularity_sqla: [['ds', 'ds']], + main_dttm_col: 'ds', name: 'birth_names', owners: [{ first_name: 'joe', last_name: 'man', id: 1 }], database: { diff --git a/superset-frontend/src/components/Datasource/DatasourceEditor.jsx b/superset-frontend/src/components/Datasource/DatasourceEditor.jsx index cd215190b1fdd..001fc3cba7332 100644 --- a/superset-frontend/src/components/Datasource/DatasourceEditor.jsx +++ b/superset-frontend/src/components/Datasource/DatasourceEditor.jsx @@ -156,7 +156,9 @@ CollectionTabTitle.propTypes = { function ColumnCollectionTable({ columns, - onChange, + datasource, + onColumnsChange, + onDatasourceChange, editableColumnName, showExpression, allowAddItem, @@ -166,8 +168,22 @@ function ColumnCollectionTable({ return ( editableColumnName ? ( @@ -310,6 +327,25 @@ function ColumnCollectionTable({ {v} ), + main_dttm_col: (value, _onItemChange, _label, record) => { + const checked = datasource.main_dttm_col === record.column_name; + const disabled = !columns.find( + column => column.column_name === record.column_name, + ).is_dttm; + return ( + + onDatasourceChange({ + ...datasource, + main_dttm_col: record.column_name, + }) + } + /> + ); + }, type: d => (d ? : null), is_dttm: checkboxGenerator, filterable: checkboxGenerator, @@ -320,7 +356,9 @@ function ColumnCollectionTable({ } ColumnCollectionTable.propTypes = { columns: PropTypes.array.isRequired, - onChange: PropTypes.func.isRequired, + datasource: PropTypes.object.isRequired, + onColumnsChange: PropTypes.func.isRequired, + onDatasourceChange: PropTypes.func.isRequired, editableColumnName: PropTypes.bool, showExpression: PropTypes.bool, allowAddItem: PropTypes.bool, @@ -1227,9 +1265,11 @@ class DatasourceEditor extends React.PureComponent { + datasource={datasource} + onColumnsChange={databaseColumns => this.setColumns({ databaseColumns }) } + onDatasourceChange={this.onDatasourceChange} /> {this.state.metadataLoading && } @@ -1245,9 +1285,11 @@ class DatasourceEditor extends React.PureComponent { > + onColumnsChange={calculatedColumns => this.setColumns({ calculatedColumns }) } + onDatasourceChange={this.onDatasourceChange} + datasource={datasource} editableColumnName showExpression allowAddItem diff --git a/superset-frontend/src/components/Datasource/DatasourceEditor.test.jsx b/superset-frontend/src/components/Datasource/DatasourceEditor.test.jsx index a0d46a6f1ac0b..8d634930119b6 100644 --- a/superset-frontend/src/components/Datasource/DatasourceEditor.test.jsx +++ b/superset-frontend/src/components/Datasource/DatasourceEditor.test.jsx @@ -230,4 +230,30 @@ describe('DatasourceEditor RTL', () => { userEvent.type(certificationDetails, 'I am typing something new'); expect(certificationDetails.value).toEqual('I am typing something new'); }); + it('shows the default datetime column', async () => { + render(, { useRedux: true }); + const metricButton = screen.getByTestId('collection-tab-Columns'); + userEvent.click(metricButton); + + const dsDefaultDatetimeRadio = screen.getByTestId('radio-default-dttm-ds'); + expect(dsDefaultDatetimeRadio).toBeChecked(); + + const genderDefaultDatetimeRadio = screen.getByTestId( + 'radio-default-dttm-gender', + ); + expect(genderDefaultDatetimeRadio).not.toBeChecked(); + }); + it('allows choosing only temporal columns as the default datetime', async () => { + render(, { useRedux: true }); + const metricButton = screen.getByTestId('collection-tab-Columns'); + userEvent.click(metricButton); + + const dsDefaultDatetimeRadio = screen.getByTestId('radio-default-dttm-ds'); + expect(dsDefaultDatetimeRadio).toBeEnabled(); + + const genderDefaultDatetimeRadio = screen.getByTestId( + 'radio-default-dttm-gender', + ); + expect(genderDefaultDatetimeRadio).toBeDisabled(); + }); }); diff --git a/superset/examples/configs/datasets/examples/cleaned_sales_data.yaml b/superset/examples/configs/datasets/examples/cleaned_sales_data.yaml index 8832cc529d0f7..fcf91bc1a2104 100644 --- a/superset/examples/configs/datasets/examples/cleaned_sales_data.yaml +++ b/superset/examples/configs/datasets/examples/cleaned_sales_data.yaml @@ -15,7 +15,7 @@ # specific language governing permissions and limitations # under the License. table_name: cleaned_sales_data -main_dttm_col: OrderDate +main_dttm_col: order_date description: null default_endpoint: null offset: 0 From 2a6e5e5e5c0d35eae1879fb8d07586262d61a3ca Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Tue, 14 Dec 2021 18:32:57 -0800 Subject: [PATCH 13/14] fix: import DB errors (#17748) --- .../src/views/CRUD/utils.test.tsx | 28 +++++++++++++++++++ superset-frontend/src/views/CRUD/utils.tsx | 9 ++++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/superset-frontend/src/views/CRUD/utils.test.tsx b/superset-frontend/src/views/CRUD/utils.test.tsx index 24cb0ce97fc23..2b95319d85d5b 100644 --- a/superset-frontend/src/views/CRUD/utils.test.tsx +++ b/superset-frontend/src/views/CRUD/utils.test.tsx @@ -143,3 +143,31 @@ test('detects if the error message is terminal or if it requires uses interventi isTerminal = hasTerminalValidation(passwordNeededErrors.errors); expect(isTerminal).toBe(false); }); + +test('does not ask for password when the import type is wrong', () => { + const error = { + errors: [ + { + message: 'Error importing database', + error_type: 'GENERIC_COMMAND_ERROR', + level: 'warning', + extra: { + 'metadata.yaml': { + type: ['Must be equal to Database.'], + }, + 'databases/examples.yaml': { + _schema: ['Must provide a password for the database'], + }, + issue_codes: [ + { + code: 1010, + message: + 'Issue 1010 - Superset encountered an error while running a command.', + }, + ], + }, + }, + ], + }; + expect(hasTerminalValidation(error.errors)).toBe(true); +}); diff --git a/superset-frontend/src/views/CRUD/utils.tsx b/superset-frontend/src/views/CRUD/utils.tsx index f261e1cd69c6b..64a92e08c63de 100644 --- a/superset-frontend/src/views/CRUD/utils.tsx +++ b/superset-frontend/src/views/CRUD/utils.tsx @@ -373,7 +373,10 @@ export const getAlreadyExists = (errors: Record[]) => export const hasTerminalValidation = (errors: Record[]) => errors.some( error => - !Object.values(error.extra).some( - payload => isNeedsPassword(payload) || isAlreadyExists(payload), - ), + !Object.entries(error.extra) + .filter(([key, _]) => key !== 'issue_codes') + .every( + ([_, payload]) => + isNeedsPassword(payload) || isAlreadyExists(payload), + ), ); From 37cc2c4d1568ac35d145a88ce8e27d8d2d108478 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Tue, 14 Dec 2021 18:33:52 -0800 Subject: [PATCH 14/14] fix: column extra in import/export (#17738) --- superset/datasets/commands/export.py | 15 +- .../datasets/commands/importers/v1/utils.py | 32 +-- superset/datasets/schemas.py | 3 +- tests/unit_tests/conftest.py | 41 +++- tests/unit_tests/datasets/__init__.py | 16 ++ .../unit_tests/datasets/commands/__init__.py | 16 ++ .../datasets/commands/export_test.py | 192 ++++++++++++++++++ .../datasets/commands/importers/__init__.py | 16 ++ .../commands/importers/v1/__init__.py | 16 ++ .../commands/importers/v1/import_test.py | 190 +++++++++++++++++ 10 files changed, 512 insertions(+), 25 deletions(-) create mode 100644 tests/unit_tests/datasets/__init__.py create mode 100644 tests/unit_tests/datasets/commands/__init__.py create mode 100644 tests/unit_tests/datasets/commands/export_test.py create mode 100644 tests/unit_tests/datasets/commands/importers/__init__.py create mode 100644 tests/unit_tests/datasets/commands/importers/v1/__init__.py create mode 100644 tests/unit_tests/datasets/commands/importers/v1/import_test.py diff --git a/superset/datasets/commands/export.py b/superset/datasets/commands/export.py index 84ae6c486c08d..45460f36e3455 100644 --- a/superset/datasets/commands/export.py +++ b/superset/datasets/commands/export.py @@ -59,12 +59,15 @@ def _export(model: SqlaTable) -> Iterator[Tuple[str, str]]: payload[key] = json.loads(payload[key]) except json.decoder.JSONDecodeError: logger.info("Unable to decode `%s` field: %s", key, payload[key]) - for metric in payload.get("metrics", []): - if metric.get("extra"): - try: - metric["extra"] = json.loads(metric["extra"]) - except json.decoder.JSONDecodeError: - logger.info("Unable to decode `extra` field: %s", metric["extra"]) + for key in ("metrics", "columns"): + for attributes in payload.get(key, []): + if attributes.get("extra"): + try: + attributes["extra"] = json.loads(attributes["extra"]) + except json.decoder.JSONDecodeError: + logger.info( + "Unable to decode `extra` field: %s", attributes["extra"] + ) payload["version"] = EXPORT_VERSION payload["database_uuid"] = str(model.database.uuid) diff --git a/superset/datasets/commands/importers/v1/utils.py b/superset/datasets/commands/importers/v1/utils.py index 37522da28c2d2..92687324aab8c 100644 --- a/superset/datasets/commands/importers/v1/utils.py +++ b/superset/datasets/commands/importers/v1/utils.py @@ -30,7 +30,6 @@ from superset.connectors.sqla.models import SqlaTable from superset.models.core import Database -from superset.utils.core import get_example_database logger = logging.getLogger(__name__) @@ -96,13 +95,17 @@ def import_dataset( config[key] = json.dumps(config[key]) except TypeError: logger.info("Unable to encode `%s` field: %s", key, config[key]) - for metric in config.get("metrics", []): - if metric.get("extra") is not None: - try: - metric["extra"] = json.dumps(metric["extra"]) - except TypeError: - logger.info("Unable to encode `extra` field: %s", metric["extra"]) - metric["extra"] = None + for key in ("metrics", "columns"): + for attributes in config.get(key, []): + # should be a dictionary, but in initial exports this was a string + if isinstance(attributes.get("extra"), dict): + try: + attributes["extra"] = json.dumps(attributes["extra"]) + except TypeError: + logger.info( + "Unable to encode `extra` field: %s", attributes["extra"] + ) + attributes["extra"] = None # should we delete columns and metrics not present in the current import? sync = ["columns", "metrics"] if overwrite else [] @@ -127,9 +130,8 @@ def import_dataset( if dataset.id is None: session.flush() - example_database = get_example_database() try: - table_exists = example_database.has_table_by_name(dataset.table_name) + table_exists = dataset.database.has_table_by_name(dataset.table_name) except Exception: # pylint: disable=broad-except # MySQL doesn't play nice with GSheets table names logger.warning( @@ -139,7 +141,7 @@ def import_dataset( if data_uri and (not table_exists or force_data): logger.info("Downloading data from %s", data_uri) - load_data(data_uri, dataset, example_database, session) + load_data(data_uri, dataset, dataset.database, session) if hasattr(g, "user") and g.user: dataset.owners.append(g.user) @@ -148,7 +150,7 @@ def import_dataset( def load_data( - data_uri: str, dataset: SqlaTable, example_database: Database, session: Session + data_uri: str, dataset: SqlaTable, database: Database, session: Session ) -> None: data = request.urlopen(data_uri) # pylint: disable=consider-using-with if data_uri.endswith(".gz"): @@ -162,14 +164,12 @@ def load_data( df[column_name] = pd.to_datetime(df[column_name]) # reuse session when loading data if possible, to make import atomic - if example_database.sqlalchemy_uri == current_app.config.get( - "SQLALCHEMY_DATABASE_URI" - ) or not current_app.config.get("SQLALCHEMY_EXAMPLES_URI"): + if database.sqlalchemy_uri == current_app.config.get("SQLALCHEMY_DATABASE_URI"): logger.info("Loading data inside the import transaction") connection = session.connection() else: logger.warning("Loading data outside the import transaction") - connection = example_database.get_sqla_engine() + connection = database.get_sqla_engine() df.to_sql( dataset.table_name, diff --git a/superset/datasets/schemas.py b/superset/datasets/schemas.py index 637e282a2b91d..e8e30c65769ce 100644 --- a/superset/datasets/schemas.py +++ b/superset/datasets/schemas.py @@ -131,7 +131,8 @@ class DatasetRelatedObjectsResponse(Schema): class ImportV1ColumnSchema(Schema): column_name = fields.String(required=True) - extra = fields.Dict(allow_none=True) + # extra was initially exported incorrectly as a string + extra = fields.Raw(allow_none=True) verbose_name = fields.String(allow_none=True) is_dttm = fields.Boolean(default=False, allow_none=True) is_active = fields.Boolean(default=True, allow_none=True) diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index 4700cd19f8d7d..8522877c28865 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -14,25 +14,62 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +# pylint: disable=redefined-outer-name + +from typing import Iterator import pytest +from pytest_mock import MockFixture +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker +from sqlalchemy.orm.session import Session from superset.app import SupersetApp from superset.initialization import SupersetAppInitializer +@pytest.fixture() +def session() -> Iterator[Session]: + """ + Create an in-memory SQLite session to test models. + """ + engine = create_engine("sqlite://") + Session_ = sessionmaker(bind=engine) # pylint: disable=invalid-name + in_memory_session = Session_() + + # flask calls session.remove() + in_memory_session.remove = lambda: None + + yield in_memory_session + + @pytest.fixture -def app_context(): +def app(mocker: MockFixture, session: Session) -> Iterator[SupersetApp]: """ - A fixture for running the test inside an app context. + A fixture that generates a Superset app. """ app = SupersetApp(__name__) app.config.from_object("superset.config") app.config["SQLALCHEMY_DATABASE_URI"] = "sqlite://" + app.config["FAB_ADD_SECURITY_VIEWS"] = False app_initializer = app.config.get("APP_INITIALIZER", SupersetAppInitializer)(app) app_initializer.init_app() + # patch session + mocker.patch( + "superset.security.SupersetSecurityManager.get_session", return_value=session, + ) + mocker.patch("superset.db.session", session) + + yield app + + +@pytest.fixture +def app_context(app: SupersetApp) -> Iterator[None]: + """ + A fixture that yields and application context. + """ with app.app_context(): yield diff --git a/tests/unit_tests/datasets/__init__.py b/tests/unit_tests/datasets/__init__.py new file mode 100644 index 0000000000000..13a83393a9124 --- /dev/null +++ b/tests/unit_tests/datasets/__init__.py @@ -0,0 +1,16 @@ +# 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. diff --git a/tests/unit_tests/datasets/commands/__init__.py b/tests/unit_tests/datasets/commands/__init__.py new file mode 100644 index 0000000000000..13a83393a9124 --- /dev/null +++ b/tests/unit_tests/datasets/commands/__init__.py @@ -0,0 +1,16 @@ +# 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. diff --git a/tests/unit_tests/datasets/commands/export_test.py b/tests/unit_tests/datasets/commands/export_test.py new file mode 100644 index 0000000000000..5c67ab2e145ad --- /dev/null +++ b/tests/unit_tests/datasets/commands/export_test.py @@ -0,0 +1,192 @@ +# 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. +# pylint: disable=import-outside-toplevel, unused-argument, unused-import + +import json + +from sqlalchemy.orm.session import Session + + +def test_export(app_context: None, session: Session) -> None: + """ + Test exporting a dataset. + """ + from superset.connectors.sqla.models import SqlaTable, SqlMetric, TableColumn + from superset.datasets.commands.export import ExportDatasetsCommand + from superset.models.core import Database + + engine = session.get_bind() + SqlaTable.metadata.create_all(engine) # pylint: disable=no-member + + database = Database(database_name="my_database", sqlalchemy_uri="sqlite://") + session.add(database) + session.flush() + + columns = [ + TableColumn(column_name="ds", is_dttm=1, type="TIMESTAMP"), + TableColumn(column_name="user_id", type="INTEGER"), + TableColumn(column_name="revenue", type="INTEGER"), + TableColumn(column_name="expenses", type="INTEGER"), + TableColumn( + column_name="profit", + type="INTEGER", + expression="revenue-expenses", + extra=json.dumps({"certified_by": "User"}), + ), + ] + metrics = [ + SqlMetric(metric_name="cnt", expression="COUNT(*)"), + ] + + sqla_table = SqlaTable( + table_name="my_table", + columns=columns, + metrics=metrics, + main_dttm_col="ds", + database=database, + offset=-8, + description="This is the description", + is_featured=1, + cache_timeout=3600, + schema="my_schema", + sql=None, + params=json.dumps( + {"remote_id": 64, "database_name": "examples", "import_time": 1606677834,} + ), + perm=None, + filter_select_enabled=1, + fetch_values_predicate="foo IN (1, 2)", + is_sqllab_view=0, # no longer used? + template_params=json.dumps({"answer": "42"}), + schema_perm=None, + extra=json.dumps({"warning_markdown": "*WARNING*"}), + ) + + export = list( + ExportDatasetsCommand._export(sqla_table) # pylint: disable=protected-access + ) + assert export == [ + ( + "datasets/my_database/my_table.yaml", + f"""table_name: my_table +main_dttm_col: ds +description: This is the description +default_endpoint: null +offset: -8 +cache_timeout: 3600 +schema: my_schema +sql: null +params: + remote_id: 64 + database_name: examples + import_time: 1606677834 +template_params: + answer: '42' +filter_select_enabled: 1 +fetch_values_predicate: foo IN (1, 2) +extra: '{{\"warning_markdown\": \"*WARNING*\"}}' +uuid: null +metrics: +- metric_name: cnt + verbose_name: null + metric_type: null + expression: COUNT(*) + description: null + d3format: null + extra: null + warning_text: null +columns: +- column_name: profit + verbose_name: null + is_dttm: null + is_active: null + type: INTEGER + groupby: null + filterable: null + expression: revenue-expenses + description: null + python_date_format: null + extra: + certified_by: User +- column_name: ds + verbose_name: null + is_dttm: 1 + is_active: null + type: TIMESTAMP + groupby: null + filterable: null + expression: null + description: null + python_date_format: null + extra: null +- column_name: user_id + verbose_name: null + is_dttm: null + is_active: null + type: INTEGER + groupby: null + filterable: null + expression: null + description: null + python_date_format: null + extra: null +- column_name: expenses + verbose_name: null + is_dttm: null + is_active: null + type: INTEGER + groupby: null + filterable: null + expression: null + description: null + python_date_format: null + extra: null +- column_name: revenue + verbose_name: null + is_dttm: null + is_active: null + type: INTEGER + groupby: null + filterable: null + expression: null + description: null + python_date_format: null + extra: null +version: 1.0.0 +database_uuid: {database.uuid} +""", + ), + ( + "databases/my_database.yaml", + f"""database_name: my_database +sqlalchemy_uri: sqlite:// +cache_timeout: null +expose_in_sqllab: true +allow_run_async: false +allow_ctas: false +allow_cvas: false +allow_file_upload: false +extra: + metadata_params: {{}} + engine_params: {{}} + metadata_cache_timeout: {{}} + schemas_allowed_for_file_upload: [] +uuid: {database.uuid} +version: 1.0.0 +""", + ), + ] diff --git a/tests/unit_tests/datasets/commands/importers/__init__.py b/tests/unit_tests/datasets/commands/importers/__init__.py new file mode 100644 index 0000000000000..13a83393a9124 --- /dev/null +++ b/tests/unit_tests/datasets/commands/importers/__init__.py @@ -0,0 +1,16 @@ +# 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. diff --git a/tests/unit_tests/datasets/commands/importers/v1/__init__.py b/tests/unit_tests/datasets/commands/importers/v1/__init__.py new file mode 100644 index 0000000000000..13a83393a9124 --- /dev/null +++ b/tests/unit_tests/datasets/commands/importers/v1/__init__.py @@ -0,0 +1,16 @@ +# 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. diff --git a/tests/unit_tests/datasets/commands/importers/v1/import_test.py b/tests/unit_tests/datasets/commands/importers/v1/import_test.py new file mode 100644 index 0000000000000..99fa42d072f3d --- /dev/null +++ b/tests/unit_tests/datasets/commands/importers/v1/import_test.py @@ -0,0 +1,190 @@ +# 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. +# pylint: disable=import-outside-toplevel, unused-argument, unused-import, invalid-name + +import json +import uuid +from typing import Any, Dict + +from sqlalchemy.orm.session import Session + + +def test_import_(app_context: None, session: Session) -> None: + """ + Test importing a dataset. + """ + from superset.connectors.sqla.models import SqlaTable, SqlMetric, TableColumn + from superset.datasets.commands.importers.v1.utils import import_dataset + from superset.models.core import Database + + engine = session.get_bind() + SqlaTable.metadata.create_all(engine) # pylint: disable=no-member + + database = Database(database_name="my_database", sqlalchemy_uri="sqlite://") + session.add(database) + session.flush() + + dataset_uuid = uuid.uuid4() + config = { + "table_name": "my_table", + "main_dttm_col": "ds", + "description": "This is the description", + "default_endpoint": None, + "offset": -8, + "cache_timeout": 3600, + "schema": "my_schema", + "sql": None, + "params": { + "remote_id": 64, + "database_name": "examples", + "import_time": 1606677834, + }, + "template_params": {"answer": "42",}, + "filter_select_enabled": True, + "fetch_values_predicate": "foo IN (1, 2)", + "extra": '{"warning_markdown": "*WARNING*"}', + "uuid": dataset_uuid, + "metrics": [ + { + "metric_name": "cnt", + "verbose_name": None, + "metric_type": None, + "expression": "COUNT(*)", + "description": None, + "d3format": None, + "extra": None, + "warning_text": None, + } + ], + "columns": [ + { + "column_name": "profit", + "verbose_name": None, + "is_dttm": None, + "is_active": None, + "type": "INTEGER", + "groupby": None, + "filterable": None, + "expression": "revenue-expenses", + "description": None, + "python_date_format": None, + "extra": {"certified_by": "User",}, + } + ], + "database_uuid": database.uuid, + "database_id": database.id, + } + + sqla_table = import_dataset(session, config) + assert sqla_table.table_name == "my_table" + assert sqla_table.main_dttm_col == "ds" + assert sqla_table.description == "This is the description" + assert sqla_table.default_endpoint is None + assert sqla_table.offset == -8 + assert sqla_table.cache_timeout == 3600 + assert sqla_table.schema == "my_schema" + assert sqla_table.sql is None + assert sqla_table.params == json.dumps( + {"remote_id": 64, "database_name": "examples", "import_time": 1606677834} + ) + assert sqla_table.template_params == json.dumps({"answer": "42"}) + assert sqla_table.filter_select_enabled is True + assert sqla_table.fetch_values_predicate == "foo IN (1, 2)" + assert sqla_table.extra == '{"warning_markdown": "*WARNING*"}' + assert sqla_table.uuid == dataset_uuid + assert len(sqla_table.metrics) == 1 + assert sqla_table.metrics[0].metric_name == "cnt" + assert sqla_table.metrics[0].verbose_name is None + assert sqla_table.metrics[0].metric_type is None + assert sqla_table.metrics[0].expression == "COUNT(*)" + assert sqla_table.metrics[0].description is None + assert sqla_table.metrics[0].d3format is None + assert sqla_table.metrics[0].extra is None + assert sqla_table.metrics[0].warning_text is None + assert len(sqla_table.columns) == 1 + assert sqla_table.columns[0].column_name == "profit" + assert sqla_table.columns[0].verbose_name is None + assert sqla_table.columns[0].is_dttm is False + assert sqla_table.columns[0].is_active is True + assert sqla_table.columns[0].type == "INTEGER" + assert sqla_table.columns[0].groupby is True + assert sqla_table.columns[0].filterable is True + assert sqla_table.columns[0].expression == "revenue-expenses" + assert sqla_table.columns[0].description is None + assert sqla_table.columns[0].python_date_format is None + assert sqla_table.columns[0].extra == '{"certified_by": "User"}' + assert sqla_table.database.uuid == database.uuid + assert sqla_table.database.id == database.id + + +def test_import_column_extra_is_string(app_context: None, session: Session) -> None: + """ + Test importing a dataset when the column extra is a string. + """ + from superset.connectors.sqla.models import SqlaTable, SqlMetric, TableColumn + from superset.datasets.commands.importers.v1.utils import import_dataset + from superset.models.core import Database + + engine = session.get_bind() + SqlaTable.metadata.create_all(engine) # pylint: disable=no-member + + database = Database(database_name="my_database", sqlalchemy_uri="sqlite://") + session.add(database) + session.flush() + + dataset_uuid = uuid.uuid4() + config: Dict[str, Any] = { + "table_name": "my_table", + "main_dttm_col": "ds", + "description": "This is the description", + "default_endpoint": None, + "offset": -8, + "cache_timeout": 3600, + "schema": "my_schema", + "sql": None, + "params": { + "remote_id": 64, + "database_name": "examples", + "import_time": 1606677834, + }, + "template_params": {"answer": "42",}, + "filter_select_enabled": True, + "fetch_values_predicate": "foo IN (1, 2)", + "extra": '{"warning_markdown": "*WARNING*"}', + "uuid": dataset_uuid, + "metrics": [], + "columns": [ + { + "column_name": "profit", + "verbose_name": None, + "is_dttm": None, + "is_active": None, + "type": "INTEGER", + "groupby": None, + "filterable": None, + "expression": "revenue-expenses", + "description": None, + "python_date_format": None, + "extra": '{"certified_by": "User"}', + } + ], + "database_uuid": database.uuid, + "database_id": database.id, + } + + sqla_table = import_dataset(session, config) + assert sqla_table.columns[0].extra == '{"certified_by": "User"}'