diff --git a/.github/workflows/bashlib.sh b/.github/workflows/bashlib.sh index 0f9a8fd10e01b..32e89be43174d 100644 --- a/.github/workflows/bashlib.sh +++ b/.github/workflows/bashlib.sh @@ -38,10 +38,10 @@ default-setup-command() { } apt-get-install() { - say "::group::apt-get install dependencies" - sudo apt-get update && sudo apt-get install --yes \ - libsasl2-dev - say "::endgroup::" + say "::group::apt-get install dependencies" + sudo apt-get update && sudo apt-get install --yes \ + libsasl2-dev + say "::endgroup::" } pip-upgrade() { @@ -161,7 +161,7 @@ cypress-run() { if [[ -z $CYPRESS_KEY ]]; then $cypress --spec "cypress/integration/$page" --browser "$browser" else - export CYPRESS_RECORD_KEY=`echo $CYPRESS_KEY | base64 --decode` + export CYPRESS_RECORD_KEY=$(echo $CYPRESS_KEY | base64 --decode) # additional flags for Cypress dashboard recording $cypress --spec "cypress/integration/$page" --browser "$browser" \ --record --group "$group" --tag "${GITHUB_REPOSITORY},${GITHUB_EVENT_NAME}" \ @@ -190,8 +190,8 @@ cypress-run-all() { cat "$flasklog" say "::endgroup::" - # Rerun SQL Lab tests with backend persist enabled - export SUPERSET_CONFIG=tests.integration_tests.superset_test_config_sqllab_backend_persist + # Rerun SQL Lab tests with backend persist disabled + export SUPERSET_CONFIG=tests.integration_tests.superset_test_config_sqllab_backend_persist_off # Restart Flask with new configs kill $flaskProcessId diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8731b8aa3d1bb..1f29891dfddc9 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -20,7 +20,7 @@ repos: hooks: - id: isort - repo: https://github.com/pre-commit/mirrors-mypy - rev: v0.910 + rev: v0.941 hooks: - id: mypy additional_dependencies: [types-all] diff --git a/RELEASING/changelog.py b/RELEASING/changelog.py index 0cf600280b799..441e3092d047e 100644 --- a/RELEASING/changelog.py +++ b/RELEASING/changelog.py @@ -381,12 +381,12 @@ def change_log( with open(csv, "w") as csv_file: log_items = list(logs) field_names = log_items[0].keys() - writer = lib_csv.DictWriter( # type: ignore + writer = lib_csv.DictWriter( csv_file, delimiter=",", quotechar='"', quoting=lib_csv.QUOTE_ALL, - fieldnames=field_names, # type: ignore + fieldnames=field_names, ) writer.writeheader() for log in logs: diff --git a/UPDATING.md b/UPDATING.md index ea9f02094a52c..07193a462d3be 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -24,6 +24,7 @@ assists people when migrating to a new version. ## Next +- [19046](https://github.com/apache/superset/pull/19046): Enables the drag and drop interface in Explore control panel by default. Flips `ENABLE_EXPLORE_DRAG_AND_DROP` and `ENABLE_DND_WITH_CLICK_UX` feature flags to `True`. - [18936](https://github.com/apache/superset/pull/18936): Removes legacy SIP-15 interm logic/flags—specifically the `SIP_15_ENABLED`, `SIP_15_GRACE_PERIOD_END`, `SIP_15_DEFAULT_TIME_RANGE_ENDPOINTS`, and `SIP_15_TOAST_MESSAGE` flags. Time range endpoints are no longer configurable and strictly adhere to the `[start, end)` paradigm, i.e., inclusive of the start and exclusive of the end. Additionally this change removes the now obsolete `time_range_endpoints` from the form-data and resulting in the cache being busted. ### Breaking Changes @@ -34,9 +35,10 @@ assists people when migrating to a new version. - [17984](https://github.com/apache/superset/pull/17984): Default Flask SECRET_KEY has changed for security reasons. You should always override with your own secret. Set `PREVIOUS_SECRET_KEY` (ex: PREVIOUS_SECRET_KEY = "\2\1thisismyscretkey\1\2\\e\\y\\y\\h") with your previous key and use `superset re-encrypt-secrets` to rotate you current secrets - [15254](https://github.com/apache/superset/pull/15254): Previously `QUERY_COST_FORMATTERS_BY_ENGINE`, `SQL_VALIDATORS_BY_ENGINE` and `SCHEDULED_QUERIES` were expected to be defined in the feature flag dictionary in the `config.py` file. These should now be defined as a top-level config, with the feature flag dictionary being reserved for boolean only values. - [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) -- [18970](https://github.com/apache/superset/pull/18970): Changes feature -flag for the legacy datasource editor (DISABLE_LEGACY_DATASOURCE_EDITOR) in config.py to True, thus disabling the feature from being shown in the client. +- [18970](https://github.com/apache/superset/pull/18970): Changes feature flag for the legacy datasource editor (DISABLE_LEGACY_DATASOURCE_EDITOR) in config.py to True, thus disabling the feature from being shown in the client. - [19017](https://github.com/apache/superset/pull/19017): Removes Python 3.7 support. +- [19142](https://github.com/apache/superset/pull/19142): Changes feature flag for versioned export(VERSIONED_EXPORT) to be true. +- [19107](https://github.com/apache/superset/pull/19107): Feature flag `SQLLAB_BACKEND_PERSISTENCE` is now on by default, which enables persisting SQL Lab tabs in the backend instead of the browser's `localStorage`. ### Potential Downtime @@ -49,11 +51,12 @@ flag for the legacy datasource editor (DISABLE_LEGACY_DATASOURCE_EDITOR) in conf ### Deprecations +- [19078](https://github.com/apache/superset/pull/19078): Creation of old shorturl links has been deprecated in favor of a new permalink feature that solves the long url problem (old shorturls will still work, though!). By default, new permalinks use UUID4 as the key. However, to use serial ids similar to the old shorturls, add the following to your `superset_config.py`: `PERMALINK_KEY_TYPE = "id"`. - [18960](https://github.com/apache/superset/pull/18960): Persisting URL params in chart metadata is no longer supported. To set a default value for URL params in Jinja code, use the optional second argument: `url_param("my-param", "my-default-value")`. ### Other -- [17589](https://github.com/apache/incubator-superset/pull/17589): It is now possible to limit access to users' recent activity data by setting the `ENABLE_BROAD_ACTIVITY_ACCESS` config flag to false, or customizing the `raise_for_user_activity_access` method in the security manager. +- [17589](https://github.com/apache/superset/pull/17589): It is now possible to limit access to users' recent activity data by setting the `ENABLE_BROAD_ACTIVITY_ACCESS` config flag to false, or customizing the `raise_for_user_activity_access` method in the security manager. - [17536](https://github.com/apache/superset/pull/17536): introduced a key-value endpoint to store dashboard filter state. This endpoint is backed by Flask-Caching and the default configuration assumes that the values will be stored in the file system. If you are already using another cache backend like Redis or Memchached, you'll probably want to change this setting in `superset_config.py`. The key is `FILTER_STATE_CACHE_CONFIG` and the available settings can be found in Flask-Caching [docs](https://flask-caching.readthedocs.io/en/latest/). - [17882](https://github.com/apache/superset/pull/17882): introduced a key-value endpoint to store Explore form data. This endpoint is backed by Flask-Caching and the default configuration assumes that the values will be stored in the file system. If you are already using another cache backend like Redis or Memchached, you'll probably want to change this setting in `superset_config.py`. The key is `EXPLORE_FORM_DATA_CACHE_CONFIG` and the available settings can be found in Flask-Caching [docs](https://flask-caching.readthedocs.io/en/latest/). diff --git a/docs/static/resources/openapi.json b/docs/static/resources/openapi.json index 86d07ad8264c5..1e8c6129f1d44 100644 --- a/docs/static/resources/openapi.json +++ b/docs/static/resources/openapi.json @@ -2248,6 +2248,9 @@ "allows_virtual_table_explore": { "type": "boolean" }, + "disable_data_preview": { + "type": "boolean" + }, "backend": { "type": "string" }, @@ -2472,6 +2475,9 @@ "allows_virtual_table_explore": { "readOnly": true }, + "disable_data_preview": { + "readOnly": true + }, "backend": { "readOnly": true }, @@ -2571,7 +2577,7 @@ "type": "boolean" }, "extra": { - "description": "

JSON string containing extra configuration elements.
1. The engine_params object gets unpacked into the sqlalchemy.create_engine call, while the metadata_params gets unpacked into the sqlalchemy.MetaData call.
2. The metadata_cache_timeout is a cache timeout setting in seconds for metadata fetch of this database. Specify it as \"metadata_cache_timeout\": {\"schema_cache_timeout\": 600, \"table_cache_timeout\": 600}. If unset, cache will not be enabled for the functionality. A timeout of 0 indicates that the cache never expires.
3. The schemas_allowed_for_csv_upload is a comma separated list of schemas that CSVs are allowed to upload to. Specify it as \"schemas_allowed_for_csv_upload\": [\"public\", \"csv_upload\"]. If database flavor does not support schema or any schema is allowed to be accessed, just leave the list empty
4. the version field is a string specifying the this db's version. This should be used with Presto DBs so that the syntax is correct
5. The allows_virtual_table_explore field is a boolean specifying whether or not the Explore button in SQL Lab results is shown.

", + "description": "

JSON string containing extra configuration elements.
1. The engine_params object gets unpacked into the sqlalchemy.create_engine call, while the metadata_params gets unpacked into the sqlalchemy.MetaData call.
2. The metadata_cache_timeout is a cache timeout setting in seconds for metadata fetch of this database. Specify it as \"metadata_cache_timeout\": {\"schema_cache_timeout\": 600, \"table_cache_timeout\": 600}. If unset, cache will not be enabled for the functionality. A timeout of 0 indicates that the cache never expires.
3. The schemas_allowed_for_csv_upload is a comma separated list of schemas that CSVs are allowed to upload to. Specify it as \"schemas_allowed_for_csv_upload\": [\"public\", \"csv_upload\"]. If database flavor does not support schema or any schema is allowed to be accessed, just leave the list empty
4. the version field is a string specifying the this db's version. This should be used with Presto DBs so that the syntax is correct
5. The allows_virtual_table_explore field is a boolean specifying whether or not the Explore button in SQL Lab results is shown
6. The disable_data_preview field is a boolean specifying whether or not data preview queries will be run when fetching table metadata in SQL Lab.

", "type": "string" }, "force_ctas_schema": { @@ -2663,7 +2669,7 @@ "type": "boolean" }, "extra": { - "description": "

JSON string containing extra configuration elements.
1. The engine_params object gets unpacked into the sqlalchemy.create_engine call, while the metadata_params gets unpacked into the sqlalchemy.MetaData call.
2. The metadata_cache_timeout is a cache timeout setting in seconds for metadata fetch of this database. Specify it as \"metadata_cache_timeout\": {\"schema_cache_timeout\": 600, \"table_cache_timeout\": 600}. If unset, cache will not be enabled for the functionality. A timeout of 0 indicates that the cache never expires.
3. The schemas_allowed_for_csv_upload is a comma separated list of schemas that CSVs are allowed to upload to. Specify it as \"schemas_allowed_for_csv_upload\": [\"public\", \"csv_upload\"]. If database flavor does not support schema or any schema is allowed to be accessed, just leave the list empty
4. the version field is a string specifying the this db's version. This should be used with Presto DBs so that the syntax is correct
5. The allows_virtual_table_explore field is a boolean specifying whether or not the Explore button in SQL Lab results is shown.

", + "description": "

JSON string containing extra configuration elements.
1. The engine_params object gets unpacked into the sqlalchemy.create_engine call, while the metadata_params gets unpacked into the sqlalchemy.MetaData call.
2. The metadata_cache_timeout is a cache timeout setting in seconds for metadata fetch of this database. Specify it as \"metadata_cache_timeout\": {\"schema_cache_timeout\": 600, \"table_cache_timeout\": 600}. If unset, cache will not be enabled for the functionality. A timeout of 0 indicates that the cache never expires.
3. The schemas_allowed_for_csv_upload is a comma separated list of schemas that CSVs are allowed to upload to. Specify it as \"schemas_allowed_for_csv_upload\": [\"public\", \"csv_upload\"]. If database flavor does not support schema or any schema is allowed to be accessed, just leave the list empty
4. the version field is a string specifying the this db's version. This should be used with Presto DBs so that the syntax is correct
5. The allows_virtual_table_explore field is a boolean specifying whether or not the Explore button in SQL Lab results is shown
6. The disable_data_preview field is a boolean specifying whether or not data preview queries will be run when fetching table metadata in SQL Lab.

", "type": "string" }, "force_ctas_schema": { @@ -2720,7 +2726,7 @@ "type": "string" }, "extra": { - "description": "

JSON string containing extra configuration elements.
1. The engine_params object gets unpacked into the sqlalchemy.create_engine call, while the metadata_params gets unpacked into the sqlalchemy.MetaData call.
2. The metadata_cache_timeout is a cache timeout setting in seconds for metadata fetch of this database. Specify it as \"metadata_cache_timeout\": {\"schema_cache_timeout\": 600, \"table_cache_timeout\": 600}. If unset, cache will not be enabled for the functionality. A timeout of 0 indicates that the cache never expires.
3. The schemas_allowed_for_csv_upload is a comma separated list of schemas that CSVs are allowed to upload to. Specify it as \"schemas_allowed_for_csv_upload\": [\"public\", \"csv_upload\"]. If database flavor does not support schema or any schema is allowed to be accessed, just leave the list empty
4. the version field is a string specifying the this db's version. This should be used with Presto DBs so that the syntax is correct
5. The allows_virtual_table_explore field is a boolean specifying whether or not the Explore button in SQL Lab results is shown.

", + "description": "

JSON string containing extra configuration elements.
1. The engine_params object gets unpacked into the sqlalchemy.create_engine call, while the metadata_params gets unpacked into the sqlalchemy.MetaData call.
2. The metadata_cache_timeout is a cache timeout setting in seconds for metadata fetch of this database. Specify it as \"metadata_cache_timeout\": {\"schema_cache_timeout\": 600, \"table_cache_timeout\": 600}. If unset, cache will not be enabled for the functionality. A timeout of 0 indicates that the cache never expires.
3. The schemas_allowed_for_csv_upload is a comma separated list of schemas that CSVs are allowed to upload to. Specify it as \"schemas_allowed_for_csv_upload\": [\"public\", \"csv_upload\"]. If database flavor does not support schema or any schema is allowed to be accessed, just leave the list empty
4. the version field is a string specifying the this db's version. This should be used with Presto DBs so that the syntax is correct
5. The allows_virtual_table_explore field is a boolean specifying whether or not the Explore button in SQL Lab results is shown
6. The disable_data_preview field is a boolean specifying whether or not data preview queries will be run when fetching table metadata in SQL Lab.

", "type": "string" }, "impersonate_user": { @@ -2768,7 +2774,7 @@ "type": "string" }, "extra": { - "description": "

JSON string containing extra configuration elements.
1. The engine_params object gets unpacked into the sqlalchemy.create_engine call, while the metadata_params gets unpacked into the sqlalchemy.MetaData call.
2. The metadata_cache_timeout is a cache timeout setting in seconds for metadata fetch of this database. Specify it as \"metadata_cache_timeout\": {\"schema_cache_timeout\": 600, \"table_cache_timeout\": 600}. If unset, cache will not be enabled for the functionality. A timeout of 0 indicates that the cache never expires.
3. The schemas_allowed_for_csv_upload is a comma separated list of schemas that CSVs are allowed to upload to. Specify it as \"schemas_allowed_for_csv_upload\": [\"public\", \"csv_upload\"]. If database flavor does not support schema or any schema is allowed to be accessed, just leave the list empty
4. the version field is a string specifying the this db's version. This should be used with Presto DBs so that the syntax is correct
5. The allows_virtual_table_explore field is a boolean specifying whether or not the Explore button in SQL Lab results is shown.

", + "description": "

JSON string containing extra configuration elements.
1. The engine_params object gets unpacked into the sqlalchemy.create_engine call, while the metadata_params gets unpacked into the sqlalchemy.MetaData call.
2. The metadata_cache_timeout is a cache timeout setting in seconds for metadata fetch of this database. Specify it as \"metadata_cache_timeout\": {\"schema_cache_timeout\": 600, \"table_cache_timeout\": 600}. If unset, cache will not be enabled for the functionality. A timeout of 0 indicates that the cache never expires.
3. The schemas_allowed_for_csv_upload is a comma separated list of schemas that CSVs are allowed to upload to. Specify it as \"schemas_allowed_for_csv_upload\": [\"public\", \"csv_upload\"]. If database flavor does not support schema or any schema is allowed to be accessed, just leave the list empty
4. the version field is a string specifying the this db's version. This should be used with Presto DBs so that the syntax is correct
5. The allows_virtual_table_explore field is a boolean specifying whether or not the Explore button in SQL Lab results is shown
6. The disable_data_preview field is a boolean specifying whether or not data preview queries will be run when fetching table metadata in SQL Lab.

", "type": "string" }, "impersonate_user": { diff --git a/superset-frontend/cypress-base/cypress.json b/superset-frontend/cypress-base/cypress.json index 8e023d8a1a24b..f9729be1c3c91 100644 --- a/superset-frontend/cypress-base/cypress.json +++ b/superset-frontend/cypress-base/cypress.json @@ -1,7 +1,7 @@ { "baseUrl": "http://localhost:8088", "chromeWebSecurity": false, - "defaultCommandTimeout": 5000, + "defaultCommandTimeout": 8000, "numTestsKeptInMemory": 0, "experimentalFetchPolyfill": true, "requestTimeout": 10000, diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/key_value.test.ts b/superset-frontend/cypress-base/cypress/integration/dashboard/key_value.test.ts index ba27bf30163a2..24b6ff0aa7a62 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/key_value.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/key_value.test.ts @@ -27,16 +27,19 @@ interface QueryString { native_filters_key: string; } -describe('nativefiler url param key', () => { +xdescribe('nativefiler url param key', () => { // const urlParams = { param1: '123', param2: 'abc' }; before(() => { cy.login(); - cy.visit(WORLD_HEALTH_DASHBOARD); - WORLD_HEALTH_CHARTS.forEach(waitForChartLoad); - cy.wait(1000); // wait for key to be published (debounced) }); + let initialFilterKey: string; it('should have cachekey in nativefilter param', () => { + // things in `before` will not retry and the `waitForChartLoad` check is + // especically flaky and may need more retries + cy.visit(WORLD_HEALTH_DASHBOARD); + WORLD_HEALTH_CHARTS.forEach(waitForChartLoad); + cy.wait(1000); // wait for key to be published (debounced) cy.location().then(loc => { const queryParams = qs.parse(loc.search) as QueryString; expect(typeof queryParams.native_filters_key).eq('string'); @@ -44,6 +47,9 @@ describe('nativefiler url param key', () => { }); it('should have different key when page reloads', () => { + cy.visit(WORLD_HEALTH_DASHBOARD); + WORLD_HEALTH_CHARTS.forEach(waitForChartLoad); + cy.wait(1000); // wait for key to be published (debounced) cy.location().then(loc => { const queryParams = qs.parse(loc.search) as QueryString; expect(queryParams.native_filters_key).not.equal(initialFilterKey); diff --git a/superset-frontend/cypress-base/cypress/integration/sqllab/tabs.test.js b/superset-frontend/cypress-base/cypress/integration/sqllab/tabs.test.js deleted file mode 100644 index 24dd074992b02..0000000000000 --- a/superset-frontend/cypress-base/cypress/integration/sqllab/tabs.test.js +++ /dev/null @@ -1,65 +0,0 @@ -/** - * 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. - */ -describe('SqlLab query tabs', () => { - beforeEach(() => { - cy.login(); - cy.visit('/superset/sqllab'); - }); - - it('allows you to create a tab', () => { - cy.get('[data-test="sql-editor-tabs"]').then(tabList => { - const initialTabCount = tabList.length; - // add tab - cy.get('[data-test="add-tab-icon"]').first().click(); - // wait until we find the new tab - cy.get('[data-test="sql-editor-tabs"]') - .children() - .eq(0) - .contains(`Untitled Query ${initialTabCount}`); - cy.get('[data-test="sql-editor-tabs"]') - .children() - .eq(0) - .contains(`Untitled Query ${initialTabCount + 1}`); - }); - }); - - it('allows you to close a tab', () => { - cy.get('[data-test="sql-editor-tabs"]') - .children() - .then(tabListA => { - const initialTabCount = tabListA.length; - - // open the tab dropdown to remove - cy.get('[data-test="dropdown-toggle-button"]') - .children() - .first() - .click({ - force: true, - }); - - // first item is close - cy.get('[data-test="close-tab-menu-option"]').click(); - - cy.get('[data-test="sql-editor-tabs"]').should( - 'have.length', - initialTabCount - 1, - ); - }); - }); -}); diff --git a/superset-frontend/cypress-base/cypress/integration/sqllab/tabs.test.ts b/superset-frontend/cypress-base/cypress/integration/sqllab/tabs.test.ts new file mode 100644 index 0000000000000..0e85664cb785a --- /dev/null +++ b/superset-frontend/cypress-base/cypress/integration/sqllab/tabs.test.ts @@ -0,0 +1,60 @@ +/** + * 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. + */ +describe('SqlLab query tabs', () => { + beforeEach(() => { + cy.login(); + cy.visit('/superset/sqllab'); + }); + + it('allows you to create and close a tab', () => { + const tablistSelector = '[data-test="sql-editor-tabs"] > [role="tablist"]'; + const tabSelector = `${tablistSelector} [role="tab"]`; + cy.get(tabSelector).then(tabs => { + const initialTabCount = tabs.length; + const initialUntitledCount = Math.max( + 0, + ...tabs + .map((i, tabItem) => + Number(tabItem.textContent?.match(/Untitled Query (\d+)/)?.[1]), + ) + .toArray(), + ); + + // add two new tabs + cy.get('[data-test="add-tab-icon"]:visible:last').click(); + cy.contains('[role="tab"]', `Untitled Query ${initialUntitledCount + 1}`); + cy.get(tabSelector).should('have.length', initialTabCount + 1); + + cy.get('[data-test="add-tab-icon"]:visible:last').click(); + cy.contains('[role="tab"]', `Untitled Query ${initialUntitledCount + 2}`); + cy.get(tabSelector).should('have.length', initialTabCount + 2); + + // close the tabs + cy.get(`${tabSelector}:last [data-test="dropdown-trigger"]`).click({ + force: true, + }); + cy.get('[data-test="close-tab-menu-option"]').click(); + cy.get(tabSelector).should('have.length', initialTabCount + 1); + cy.contains('[role="tab"]', `Untitled Query ${initialUntitledCount + 1}`); + + cy.get(`${tablistSelector} [aria-label="remove"]:last`).click(); + cy.get(tabSelector).should('have.length', initialTabCount); + }); + }); +}); diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/index.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/index.tsx index 4021fa2c38597..0419c55ed3184 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/index.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/index.tsx @@ -95,6 +95,11 @@ type Control = { default?: unknown; }; +type SelectDefaultOption = { + label: string; + value: string; +}; + const groupByControl: SharedControlConfig<'SelectControl', ColumnMeta> = { type: 'SelectControl', label: t('Group by'), @@ -434,29 +439,36 @@ const size: SharedControlConfig<'MetricsControl'> = { default: null, }; -const y_axis_format: SharedControlConfig<'SelectControl'> = { - type: 'SelectControl', - freeForm: true, - label: t('Y Axis Format'), - renderTrigger: true, - default: DEFAULT_NUMBER_FORMAT, - choices: D3_FORMAT_OPTIONS, - description: D3_FORMAT_DOCS, - mapStateToProps: state => { - const showWarning = state.controls?.comparison_type?.value === 'percentage'; - return { - warning: showWarning - ? t( - 'When `Calculation type` is set to "Percentage change", the Y ' + - 'Axis Format is forced to `.1%`', - ) - : null, - disabled: showWarning, - }; - }, -}; - -const x_axis_time_format: SharedControlConfig<'SelectControl'> = { +const y_axis_format: SharedControlConfig<'SelectControl', SelectDefaultOption> = + { + type: 'SelectControl', + freeForm: true, + label: t('Y Axis Format'), + renderTrigger: true, + default: DEFAULT_NUMBER_FORMAT, + choices: D3_FORMAT_OPTIONS, + description: D3_FORMAT_DOCS, + filterOption: ({ data: option }, search) => + option.label.includes(search) || option.value.includes(search), + mapStateToProps: state => { + const showWarning = + state.controls?.comparison_type?.value === 'percentage'; + return { + warning: showWarning + ? t( + 'When `Calculation type` is set to "Percentage change", the Y ' + + 'Axis Format is forced to `.1%`', + ) + : null, + disabled: showWarning, + }; + }, + }; + +const x_axis_time_format: SharedControlConfig< + 'SelectControl', + SelectDefaultOption +> = { type: 'SelectControl', freeForm: true, label: t('Time format'), @@ -464,6 +476,8 @@ const x_axis_time_format: SharedControlConfig<'SelectControl'> = { default: DEFAULT_TIME_FORMAT, choices: D3_TIME_FORMAT_OPTIONS, description: D3_TIME_FORMAT_DOCS, + filterOption: ({ data: option }, search) => + option.label.includes(search) || option.value.includes(search), }; const adhoc_filters: SharedControlConfig<'AdhocFilterControl'> = { diff --git a/superset-frontend/packages/superset-ui-core/src/chart/models/ChartMetadata.ts b/superset-frontend/packages/superset-ui-core/src/chart/models/ChartMetadata.ts index 1013eeee2d3b3..7a25fe86208d1 100644 --- a/superset-frontend/packages/superset-ui-core/src/chart/models/ChartMetadata.ts +++ b/superset-frontend/packages/superset-ui-core/src/chart/models/ChartMetadata.ts @@ -43,10 +43,11 @@ export interface ChartMetadataConfig { exampleGallery?: ExampleImage[]; tags?: string[]; category?: string | null; - label?: { - name?: ChartLabel; - description?: string; - } | null; + // deprecated: true hides a chart from all viz picker interactions. + deprecated?: boolean; + // label: ChartLabel.DEPRECATED which will display a "deprecated" label on the chart. + label?: ChartLabel | null; + labelExplanation?: string | null; } export default class ChartMetadata { @@ -80,10 +81,11 @@ export default class ChartMetadata { category: string | null; - label?: { - name?: ChartLabel; - description?: string; - } | null; + deprecated?: boolean; + + label?: ChartLabel | null; + + labelExplanation?: string | null; constructor(config: ChartMetadataConfig) { const { @@ -101,7 +103,9 @@ export default class ChartMetadata { exampleGallery = [], tags = [], category = null, + deprecated = false, label = null, + labelExplanation = null, } = config; this.name = name; @@ -127,7 +131,9 @@ export default class ChartMetadata { this.exampleGallery = exampleGallery; this.tags = tags; this.category = category; + this.deprecated = deprecated; this.label = label; + this.labelExplanation = labelExplanation; } canBeAnnotationType(type: string): boolean { diff --git a/superset-frontend/packages/superset-ui-core/src/chart/types/Base.ts b/superset-frontend/packages/superset-ui-core/src/chart/types/Base.ts index aad547ca2aa5d..0bfae7777e7df 100644 --- a/superset-frontend/packages/superset-ui-core/src/chart/types/Base.ts +++ b/superset-frontend/packages/superset-ui-core/src/chart/types/Base.ts @@ -53,18 +53,21 @@ export interface PlainObject { } export enum ChartLabel { - VERIFIED = 'VERIFIED', DEPRECATED = 'DEPRECATED', FEATURED = 'FEATURED', } -export const ChartLabelWeight = { +export const chartLabelExplanations: Record = { + [ChartLabel.DEPRECATED]: + 'This chart uses features or modules which are no longer actively maintained. It will eventually be replaced or removed.', + [ChartLabel.FEATURED]: + 'This chart was tested and verified, so the overall experience should be stable.', +}; + +export const chartLabelWeight: Record = { [ChartLabel.DEPRECATED]: { weight: -0.1, }, - [ChartLabel.VERIFIED]: { - weight: 0.2, - }, [ChartLabel.FEATURED]: { weight: 0.1, }, diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index f89a6a8535df7..e13e4263a36cd 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -1018,28 +1018,13 @@ function getTableMetadata(table, query, dispatch) { ), }) .then(({ json }) => { - const dataPreviewQuery = { - id: shortid.generate(), - dbId: query.dbId, - sql: json.selectStar, - tableName: table.name, - sqlEditorId: null, - tab: '', - runAsync: false, - ctas: false, - isDataPreview: true, - }; const newTable = { ...table, ...json, expanded: true, isMetadataLoading: false, - dataPreviewQueryId: dataPreviewQuery.id, }; - Promise.all([ - dispatch(mergeTable(newTable, dataPreviewQuery)), // Merge table to tables in state - dispatch(runQuery(dataPreviewQuery)), // Run query to get preview data for table - ]); + dispatch(mergeTable(newTable)); // Merge table to tables in state return newTable; }) .catch(() => @@ -1082,7 +1067,7 @@ function getTableExtendedMetadata(table, query, dispatch) { ); } -export function addTable(query, tableName, schemaName) { +export function addTable(query, database, tableName, schemaName) { return function (dispatch) { const table = { dbId: query.dbId, @@ -1110,6 +1095,32 @@ export function addTable(query, tableName, schemaName) { }) : Promise.resolve({ json: { id: shortid.generate() } }); + if (!database.disable_data_preview && database.id === query.dbId) { + const dataPreviewQuery = { + id: shortid.generate(), + dbId: query.dbId, + sql: newTable.selectStar, + tableName: table.name, + sqlEditorId: null, + tab: '', + runAsync: database.allow_run_async, + ctas: false, + isDataPreview: true, + }; + Promise.all([ + dispatch( + mergeTable( + { + ...newTable, + dataPreviewQueryId: dataPreviewQuery.id, + }, + dataPreviewQuery, + ), + ), + dispatch(runQuery(dataPreviewQuery)), + ]); + } + return sync .then(({ json: resultJson }) => dispatch(mergeTable({ ...table, id: resultJson.id })), diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js index d04d8b90ab1a8..789ae986bfbe7 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js @@ -727,28 +727,60 @@ describe('async actions', () => { it('updates the table schema state in the backend', () => { expect.assertions(5); + const database = { disable_data_preview: true }; + const tableName = 'table'; + const schemaName = 'schema'; + const store = mockStore({}); + const expectedActionTypes = [ + actions.MERGE_TABLE, // addTable + actions.MERGE_TABLE, // getTableMetadata + actions.MERGE_TABLE, // getTableExtendedMetadata + actions.MERGE_TABLE, // addTable + ]; + return store + .dispatch(actions.addTable(query, database, tableName, schemaName)) + .then(() => { + expect(store.getActions().map(a => a.type)).toEqual( + expectedActionTypes, + ); + expect(fetchMock.calls(updateTableSchemaEndpoint)).toHaveLength(1); + expect(fetchMock.calls(getTableMetadataEndpoint)).toHaveLength(1); + expect(fetchMock.calls(getExtraTableMetadataEndpoint)).toHaveLength( + 1, + ); + + // tab state is not updated, since no query was run + expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(0); + }); + }); + + it('updates and runs data preview query when configured', () => { + expect.assertions(5); + const results = { data: mockBigNumber, - query: { sqlEditorId: 'null' }, + query: { sqlEditorId: 'null', dbId: 1 }, query_id: 'efgh', }; fetchMock.post(runQueryEndpoint, JSON.stringify(results), { overwriteRoutes: true, }); + const database = { disable_data_preview: false, id: 1 }; const tableName = 'table'; const schemaName = 'schema'; const store = mockStore({}); const expectedActionTypes = [ actions.MERGE_TABLE, // addTable actions.MERGE_TABLE, // getTableMetadata - actions.START_QUERY, // runQuery (data preview) actions.MERGE_TABLE, // getTableExtendedMetadata - actions.QUERY_SUCCESS, // querySuccess + actions.MERGE_TABLE, // addTable (data preview) + actions.START_QUERY, // runQuery (data preview) actions.MERGE_TABLE, // addTable + actions.QUERY_SUCCESS, // querySuccess ]; return store - .dispatch(actions.addTable(query, tableName, schemaName)) + .dispatch(actions.addTable(query, database, tableName, schemaName)) .then(() => { expect(store.getActions().map(a => a.type)).toEqual( expectedActionTypes, @@ -758,7 +790,6 @@ describe('async actions', () => { expect(fetchMock.calls(getExtraTableMetadataEndpoint)).toHaveLength( 1, ); - // tab state is not updated, since the query is a data preview expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(0); }); diff --git a/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx b/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx index ce201e89d904c..25da49137ad60 100644 --- a/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx +++ b/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx @@ -43,11 +43,17 @@ interface Props { actions: { queryEditorSetSelectedText: (edit: any, text: null | string) => void; queryEditorSetFunctionNames: (queryEditor: object, dbId: number) => void; - addTable: (queryEditor: any, value: any, schema: any) => void; + addTable: ( + queryEditor: any, + database: any, + value: any, + schema: any, + ) => void; }; autocomplete: boolean; onBlur: (sql: string) => void; sql: string; + database: any; schemas: any[]; tables: any[]; functionNames: string[]; @@ -171,17 +177,20 @@ class AceEditorWrapper extends React.PureComponent { meta: 'schema', })); const columns = {}; - const tables = props.extendedTables || props.tables || []; + + const tables = props.tables || []; + const extendedTables = props.extendedTables || []; const tableWords = tables.map(t => { - const tableName = t.name; - const cols = t.columns || []; + const tableName = t.value; + const extendedTable = extendedTables.find(et => et.name === tableName); + const cols = (extendedTable && extendedTable.columns) || []; cols.forEach(col => { columns[col.name] = null; // using an object as a unique set }); return { - name: tableName, + name: t.label, value: tableName, score: TABLE_AUTOCOMPLETE_SCORE, meta: 'table', @@ -207,6 +216,7 @@ class AceEditorWrapper extends React.PureComponent { if (data.meta === 'table') { this.props.actions.addTable( this.props.queryEditor, + this.props.database, data.value, this.props.queryEditor.schema, ); diff --git a/superset-frontend/src/SqlLab/components/QueryAutoRefresh/QueryAutoRefresh.test.jsx b/superset-frontend/src/SqlLab/components/QueryAutoRefresh/QueryAutoRefresh.test.jsx index 93cea6d08d65f..06bf187e1185a 100644 --- a/superset-frontend/src/SqlLab/components/QueryAutoRefresh/QueryAutoRefresh.test.jsx +++ b/superset-frontend/src/SqlLab/components/QueryAutoRefresh/QueryAutoRefresh.test.jsx @@ -17,14 +17,12 @@ * under the License. */ import React from 'react'; -import { render } from 'spec/helpers/testing-library'; -import { ThemeProvider, supersetTheme } from '@superset-ui/core'; +import { shallow } from 'enzyme'; +import sinon from 'sinon'; import thunk from 'redux-thunk'; import configureStore from 'redux-mock-store'; import QueryAutoRefresh from 'src/SqlLab/components/QueryAutoRefresh'; import { initialState, runningQuery } from 'src/SqlLab/fixtures'; -import fetchMock from 'fetch-mock'; -import * as actions from 'src/SqlLab/actions/sqlLab'; describe('QueryAutoRefresh', () => { const middlewares = [thunk]; @@ -40,29 +38,31 @@ describe('QueryAutoRefresh', () => { sqlLab, }; const store = mockStore(state); - const setup = (overrides = {}) => ( - - - - ); - - const mockFetch = fetchMock.get('glob:*/superset/queries/*', {}); + const getWrapper = () => + shallow() + .dive() + .dive(); + let wrapper; it('shouldCheckForQueries', () => { - render(setup(), { - useRedux: true, - }); - - expect(mockFetch.called()).toBe(true); + wrapper = getWrapper(); + expect(wrapper.instance().shouldCheckForQueries()).toBe(true); }); it('setUserOffline', () => { - const spy = jest.spyOn(actions, 'setUserOffline'); + wrapper = getWrapper(); + const spy = sinon.spy(wrapper.instance().props.actions, 'setUserOffline'); - render(setup(), { - useRedux: true, + // state not changed + wrapper.setState({ + offline: false, }); + expect(spy.called).toBe(false); - expect(spy).toHaveBeenCalled(); + // state is changed + wrapper.setState({ + offline: true, + }); + expect(spy.callCount).toBe(1); }); }); diff --git a/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.jsx b/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.jsx index 43f6c5d8a7d6e..b54936b691efe 100644 --- a/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.jsx +++ b/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.jsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { useState, useEffect } from 'react'; +import React from 'react'; import PropTypes from 'prop-types'; import { bindActionCreators } from 'redux'; import { connect } from 'react-redux'; @@ -28,12 +28,31 @@ const QUERY_UPDATE_BUFFER_MS = 5000; const MAX_QUERY_AGE_TO_POLL = 21600000; const QUERY_TIMEOUT_LIMIT = 10000; -function QueryAutoRefresh({ offline, queries, queriesLastUpdate, actions }) { - const [offlineState, setOfflineState] = useState(offline); - let timer = null; +class QueryAutoRefresh extends React.PureComponent { + constructor(props) { + super(props); + this.state = { + offline: props.offline, + }; + } + + UNSAFE_componentWillMount() { + this.startTimer(); + } + + componentDidUpdate(prevProps) { + if (prevProps.offline !== this.state.offline) { + this.props.actions.setUserOffline(this.state.offline); + } + } + + componentWillUnmount() { + this.stopTimer(); + } - const shouldCheckForQueries = () => { + shouldCheckForQueries() { // if there are started or running queries, this method should return true + const { queries } = this.props; const now = new Date().getTime(); const isQueryRunning = q => ['running', 'started', 'pending', 'fetching'].indexOf(q.state) >= 0; @@ -41,57 +60,46 @@ function QueryAutoRefresh({ offline, queries, queriesLastUpdate, actions }) { return Object.values(queries).some( q => isQueryRunning(q) && now - q.startDttm < MAX_QUERY_AGE_TO_POLL, ); - }; + } + + startTimer() { + if (!this.timer) { + this.timer = setInterval(this.stopwatch.bind(this), QUERY_UPDATE_FREQ); + } + } - const stopwatch = () => { + stopTimer() { + clearInterval(this.timer); + this.timer = null; + } + + stopwatch() { // only poll /superset/queries/ if there are started or running queries - if (shouldCheckForQueries()) { + if (this.shouldCheckForQueries()) { SupersetClient.get({ endpoint: `/superset/queries/${ - queriesLastUpdate - QUERY_UPDATE_BUFFER_MS + this.props.queriesLastUpdate - QUERY_UPDATE_BUFFER_MS }`, timeout: QUERY_TIMEOUT_LIMIT, }) .then(({ json }) => { if (Object.keys(json).length > 0) { - actions.refreshQueries(json); + this.props.actions.refreshQueries(json); } - - setOfflineState(false); + this.setState({ offline: false }); }) .catch(() => { - setOfflineState(true); + this.setState({ offline: true }); }); } else { - setOfflineState(false); + this.setState({ offline: false }); } - }; - - const startTimer = () => { - if (!timer) { - timer = setInterval(stopwatch(), QUERY_UPDATE_FREQ); - } - }; - - const stopTimer = () => { - clearInterval(timer); - timer = null; - }; - - useEffect(() => { - startTimer(); - return () => { - stopTimer(); - }; - }, []); + } - useEffect(() => { - actions.setUserOffline(offlineState); - }, [offlineState]); - - return null; + render() { + return null; + } } - QueryAutoRefresh.propTypes = { offline: PropTypes.bool.isRequired, queries: PropTypes.object.isRequired, diff --git a/superset-frontend/src/hooks/useUrlShortener.ts b/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx similarity index 52% rename from superset-frontend/src/hooks/useUrlShortener.ts rename to superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx index 33cb636b4527c..782b147839186 100644 --- a/superset-frontend/src/hooks/useUrlShortener.ts +++ b/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx @@ -16,24 +16,35 @@ * specific language governing permissions and limitations * under the License. */ -import { useState, useEffect } from 'react'; -import { getShortUrl as getShortUrlUtil } from 'src/utils/urlUtils'; +import React from 'react'; +import { render, screen } from 'spec/helpers/testing-library'; +import QueryHistory from 'src/SqlLab/components/QueryHistory'; -export function useUrlShortener(url: string): Function { - const [update, setUpdate] = useState(false); - const [shortUrl, setShortUrl] = useState(''); +const NOOP = () => {}; +const mockedProps = { + queries: [], + actions: { + queryEditorSetSql: NOOP, + cloneQueryToNewTab: NOOP, + fetchQueryResults: NOOP, + clearQueryResults: NOOP, + removeQuery: NOOP, + }, + displayLimit: 1000, +}; - async function getShortUrl(urlOverride?: string) { - if (update) { - const newShortUrl = await getShortUrlUtil(urlOverride || url); - setShortUrl(newShortUrl); - setUpdate(false); - return newShortUrl; - } - return shortUrl; - } +const setup = (overrides = {}) => ( + +); - useEffect(() => setUpdate(true), [url]); +describe('QueryHistory', () => { + it('Renders an empty state for query history', () => { + render(setup()); - return getShortUrl; -} + const emptyStateText = screen.getByText( + /run a query to display query history/i, + ); + + expect(emptyStateText).toBeVisible(); + }); +}); diff --git a/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx b/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx index e2d0453bb297a..7cf9d6ba657dd 100644 --- a/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx +++ b/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx @@ -17,8 +17,8 @@ * under the License. */ import React from 'react'; -import Alert from 'src/components/Alert'; -import { t } from '@superset-ui/core'; +import { EmptyStateMedium } from 'src/components/EmptyState'; +import { t, styled } from '@superset-ui/core'; import { Query } from 'src/SqlLab/types'; import QueryTable from 'src/SqlLab/components/QueryTable'; @@ -34,6 +34,17 @@ interface QueryHistoryProps { displayLimit: number; } +const StyledEmptyStateWrapper = styled.div` + height: 100%; + .ant-empty-image img { + margin-right: 28px; + } + + p { + margin-right: 28px; + } +`; + const QueryHistory = ({ queries, actions, displayLimit }: QueryHistoryProps) => queries.length > 0 ? ( displayLimit={displayLimit} /> ) : ( - + + + ); export default QueryHistory; diff --git a/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.jsx b/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.jsx index dbf81cfcf282d..1786a6cf313a6 100644 --- a/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.jsx +++ b/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.jsx @@ -20,11 +20,15 @@ import React from 'react'; import configureStore from 'redux-mock-store'; import thunk from 'redux-thunk'; import { styledShallow as shallow } from 'spec/helpers/theming'; +import { render, screen, act } from 'spec/helpers/testing-library'; import SouthPaneContainer from 'src/SqlLab/components/SouthPane/state'; import ResultSet from 'src/SqlLab/components/ResultSet'; import '@testing-library/jest-dom/extend-expect'; import { STATUS_OPTIONS } from 'src/SqlLab/constants'; import { initialState } from 'src/SqlLab/fixtures'; +import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes'; + +const NOOP = () => {}; const mockedProps = { editorQueries: [ @@ -71,13 +75,36 @@ const mockedProps = { offline: false, }; +const mockedEmptyProps = { + editorQueries: [], + latestQueryId: '', + dataPreviewQueries: [], + actions: { + queryEditorSetSql: NOOP, + cloneQueryToNewTab: NOOP, + fetchQueryResults: NOOP, + clearQueryResults: NOOP, + removeQuery: NOOP, + setActiveSouthPaneTab: NOOP, + }, + activeSouthPaneTab: '', + height: 100, + databases: '', + offline: false, + displayLimit: 100, + user: UserWithPermissionsAndRoles, + defaultQueryLimit: 100, +}; + const middlewares = [thunk]; const mockStore = configureStore(middlewares); const store = mockStore(initialState); +const setup = (overrides = {}) => ( + +); -describe('SouthPane', () => { - const getWrapper = () => - shallow().dive(); +describe('SouthPane - Enzyme', () => { + const getWrapper = () => shallow(setup()).dive(); let wrapper; @@ -95,3 +122,20 @@ describe('SouthPane', () => { ); }); }); + +describe('SouthPane - RTL', () => { + const renderAndWait = overrides => { + const mounted = act(async () => { + render(setup(overrides)); + }); + + return mounted; + }; + it('Renders an empty state for results', async () => { + await renderAndWait(mockedEmptyProps); + + const emptyStateText = screen.getByText(/run a query to display results/i); + + expect(emptyStateText).toBeVisible(); + }); +}); diff --git a/superset-frontend/src/SqlLab/components/SouthPane/index.tsx b/superset-frontend/src/SqlLab/components/SouthPane/index.tsx index f7efc04f34725..3fb0f9c5261e4 100644 --- a/superset-frontend/src/SqlLab/components/SouthPane/index.tsx +++ b/superset-frontend/src/SqlLab/components/SouthPane/index.tsx @@ -20,6 +20,7 @@ import React, { createRef } from 'react'; import shortid from 'shortid'; import Alert from 'src/components/Alert'; import Tabs from 'src/components/Tabs'; +import { EmptyStateMedium } from 'src/components/EmptyState'; import { t, styled } from '@superset-ui/core'; import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags'; @@ -93,6 +94,17 @@ const StyledPane = styled.div` } `; +const StyledEmptyStateWrapper = styled.div` + height: 100%; + .ant-empty-image img { + margin-right: 28px; + } + + p { + margin-right: 28px; + } +`; + export default function SouthPane({ editorQueries, latestQueryId, @@ -161,7 +173,12 @@ export default function SouthPane({ } } else { results = ( - + + + ); } return results; diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx b/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx index 168a53d52b1a0..7899cbf71908a 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx +++ b/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx @@ -514,6 +514,7 @@ class SqlEditor extends React.PureComponent { onChange={this.onSqlChanged} queryEditor={this.props.queryEditor} sql={this.props.queryEditor.sql} + database={this.props.database} schemas={this.props.queryEditor.schemaOptions} tables={this.props.queryEditor.tableOptions} functionNames={this.props.queryEditor.functionNames} diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx index 7bbdfcf6345ec..f9e8c2da9f98f 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx +++ b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import React from 'react'; +import React, { useEffect, useRef, useCallback } from 'react'; import Button from 'src/components/Button'; import { t, styled, css, SupersetTheme } from '@superset-ui/core'; import Collapse from 'src/components/Collapse'; @@ -36,12 +36,15 @@ interface actionsTypes { queryEditorSetFunctionNames: (queryEditor: QueryEditor, dbId: number) => void; collapseTable: (table: Table) => void; expandTable: (table: Table) => void; - addTable: (queryEditor: any, value: any, schema: any) => void; + addTable: (queryEditor: any, database: any, value: any, schema: any) => void; setDatabases: (arg0: any) => {}; addDangerToast: (msg: string) => void; queryEditorSetSchema: (queryEditor: QueryEditor, schema?: string) => void; queryEditorSetSchemaOptions: () => void; - queryEditorSetTableOptions: (options: Array) => void; + queryEditorSetTableOptions: ( + queryEditor: QueryEditor, + options: Array, + ) => void; resetState: () => void; } @@ -86,6 +89,13 @@ export default function SqlEditorLeftBar({ tables = [], height = 500, }: SqlEditorLeftBarProps) { + // Ref needed to avoid infinite rerenders on handlers + // that require and modify the queryEditor + const queryEditorRef = useRef(queryEditor); + useEffect(() => { + queryEditorRef.current = queryEditor; + }, [queryEditor]); + const onDbChange = ({ id: dbId }: { id: number }) => { actions.queryEditorSetDb(queryEditor, dbId); actions.queryEditorSetFunctionNames(queryEditor, dbId); @@ -93,7 +103,7 @@ export default function SqlEditorLeftBar({ const onTableChange = (tableName: string, schemaName: string) => { if (tableName && schemaName) { - actions.addTable(queryEditor, tableName, schemaName); + actions.addTable(queryEditor, database, tableName, schemaName); } }; @@ -132,9 +142,23 @@ export default function SqlEditorLeftBar({ const shouldShowReset = window.location.search === '?reset=1'; const tableMetaDataHeight = height - 130; // 130 is the height of the selects above - const onSchemaChange = (schema: string) => { - actions.queryEditorSetSchema(queryEditor, schema); - }; + const handleSchemaChange = useCallback( + (schema: string) => { + if (queryEditorRef.current) { + actions.queryEditorSetSchema(queryEditorRef.current, schema); + } + }, + [actions], + ); + + const handleTablesLoad = React.useCallback( + (options: Array) => { + if (queryEditorRef.current) { + actions.queryEditorSetTableOptions(queryEditorRef.current, options); + } + }, + [actions], + ); return (
@@ -143,10 +167,10 @@ export default function SqlEditorLeftBar({ getDbList={actions.setDatabases} handleError={actions.addDangerToast} onDbChange={onDbChange} - onSchemaChange={onSchemaChange} + onSchemaChange={handleSchemaChange} onSchemasLoad={actions.queryEditorSetSchemaOptions} onTableChange={onTableChange} - onTablesLoad={actions.queryEditorSetTableOptions} + onTablesLoad={handleTablesLoad} schema={queryEditor.schema} sqlLabMode /> diff --git a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx index 11c6fa8b6c097..8c20a493b0876 100644 --- a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx +++ b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx @@ -386,9 +386,7 @@ class TabbedSqlEditors extends React.PureComponent { ); const tabHeader = ( -
- -
+ {qe.title} {' '}
); diff --git a/superset-frontend/src/components/AnchorLink/AnchorLink.test.jsx b/superset-frontend/src/components/AnchorLink/AnchorLink.test.jsx index 9f0c05a8eb87a..3f05416b1c0c5 100644 --- a/superset-frontend/src/components/AnchorLink/AnchorLink.test.jsx +++ b/superset-frontend/src/components/AnchorLink/AnchorLink.test.jsx @@ -25,6 +25,7 @@ import URLShortLinkButton from 'src/components/URLShortLinkButton'; describe('AnchorLink', () => { const props = { anchorLinkId: 'CHART-123', + dashboardId: 10, }; const globalLocation = window.location; @@ -64,8 +65,9 @@ describe('AnchorLink', () => { expect(wrapper.find(URLShortLinkButton)).toExist(); expect(wrapper.find(URLShortLinkButton)).toHaveProp({ placement: 'right' }); - const targetUrl = wrapper.find(URLShortLinkButton).prop('url'); - const hash = targetUrl.slice(targetUrl.indexOf('#') + 1); - expect(hash).toBe(props.anchorLinkId); + const anchorLinkId = wrapper.find(URLShortLinkButton).prop('anchorLinkId'); + const dashboardId = wrapper.find(URLShortLinkButton).prop('dashboardId'); + expect(anchorLinkId).toBe(props.anchorLinkId); + expect(dashboardId).toBe(props.dashboardId); }); }); diff --git a/superset-frontend/src/components/AnchorLink/index.jsx b/superset-frontend/src/components/AnchorLink/index.jsx index 743cb3a3c6493..71ba76dff7a07 100644 --- a/superset-frontend/src/components/AnchorLink/index.jsx +++ b/superset-frontend/src/components/AnchorLink/index.jsx @@ -21,11 +21,11 @@ import PropTypes from 'prop-types'; import { t } from '@superset-ui/core'; import URLShortLinkButton from 'src/components/URLShortLinkButton'; -import getDashboardUrl from 'src/dashboard/util/getDashboardUrl'; import getLocationHash from 'src/dashboard/util/getLocationHash'; const propTypes = { anchorLinkId: PropTypes.string.isRequired, + dashboardId: PropTypes.number, filters: PropTypes.object, showShortLinkButton: PropTypes.bool, inFocus: PropTypes.bool, @@ -70,17 +70,14 @@ class AnchorLink extends React.PureComponent { } render() { - const { anchorLinkId, filters, showShortLinkButton, placement } = + const { anchorLinkId, dashboardId, showShortLinkButton, placement } = this.props; return ( {showShortLinkButton && ( void; style?: React.CSSProperties; @@ -49,5 +49,3 @@ export default function Checkbox({ checked, onChange, style }: CheckboxProps) { ); } - -export type { CheckboxProps }; diff --git a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx index d8d4e23eb1651..2387c2e2517fe 100644 --- a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx +++ b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx @@ -76,6 +76,7 @@ beforeEach(() => { allows_cost_estimate: 'Allows Cost Estimate', allows_subquery: 'Allows Subquery', allows_virtual_table_explore: 'Allows Virtual Table Explore', + disable_data_preview: 'Disables SQL Lab Data Preview', backend: 'Backend', changed_on: 'Changed On', changed_on_delta_humanized: 'Changed On Delta Humanized', @@ -97,6 +98,7 @@ beforeEach(() => { 'allows_cost_estimate', 'allows_subquery', 'allows_virtual_table_explore', + 'disable_data_preview', 'backend', 'changed_on', 'changed_on_delta_humanized', @@ -130,6 +132,7 @@ beforeEach(() => { allows_cost_estimate: null, allows_subquery: true, allows_virtual_table_explore: true, + disable_data_preview: false, backend: 'postgresql', changed_on: '2021-03-09T19:02:07.141095', changed_on_delta_humanized: 'a day ago', @@ -150,6 +153,7 @@ beforeEach(() => { allows_cost_estimate: null, allows_subquery: true, allows_virtual_table_explore: true, + disable_data_preview: false, backend: 'mysql', changed_on: '2021-03-09T19:02:07.141095', changed_on_delta_humanized: 'a day ago', diff --git a/superset-frontend/src/components/Dropdown/index.tsx b/superset-frontend/src/components/Dropdown/index.tsx index fdfa9f945c6c2..e5d5f9f8526c5 100644 --- a/superset-frontend/src/components/Dropdown/index.tsx +++ b/superset-frontend/src/components/Dropdown/index.tsx @@ -72,7 +72,7 @@ export interface DropdownProps { export const Dropdown = ({ overlay, ...rest }: DropdownProps) => ( - + diff --git a/superset-frontend/src/components/Popover/index.tsx b/superset-frontend/src/components/Popover/index.tsx index 880e457913a2e..bccc31c35c4bb 100644 --- a/superset-frontend/src/components/Popover/index.tsx +++ b/superset-frontend/src/components/Popover/index.tsx @@ -18,6 +18,9 @@ */ import { Popover } from 'antd'; +export { PopoverProps } from 'antd/lib/popover'; +export { TooltipPlacement } from 'antd/lib/tooltip'; + // Eventually Popover can be wrapped and customized in this file // for now we're just redirecting export default Popover; diff --git a/superset-frontend/src/components/Select/utils.ts b/superset-frontend/src/components/Select/utils.ts index f62b93ade3668..f3880f52f7d01 100644 --- a/superset-frontend/src/components/Select/utils.ts +++ b/superset-frontend/src/components/Select/utils.ts @@ -60,8 +60,10 @@ export function findValue( return (Array.isArray(value) ? value : [value]).map(find); } -export function getValue(option: string | number | { value: string | number }) { - return typeof option === 'object' ? option.value : option; +export function getValue( + option: string | number | { value: string | number | null } | null, +) { + return option && typeof option === 'object' ? option.value : option; } type LabeledValue = { label?: ReactNode; value?: V }; diff --git a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx index cd7b51d4b124d..013e937edeb41 100644 --- a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx +++ b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx @@ -20,12 +20,13 @@ import React from 'react'; import { render, screen, waitFor } from 'spec/helpers/testing-library'; import { SupersetClient } from '@superset-ui/core'; +import { act } from 'react-dom/test-utils'; import userEvent from '@testing-library/user-event'; import TableSelector from '.'; const SupersetClientGet = jest.spyOn(SupersetClient, 'get'); -const createProps = () => ({ +const createProps = (props = {}) => ({ database: { id: 1, database_name: 'main', @@ -34,23 +35,33 @@ const createProps = () => ({ }, schema: 'test_schema', handleError: jest.fn(), + ...props, }); -beforeAll(() => { - SupersetClientGet.mockImplementation( - async () => - ({ - json: { - options: [ - { label: 'table_a', value: 'table_a' }, - { label: 'table_b', value: 'table_b' }, - ], - }, - } as any), - ); +afterEach(() => { + jest.clearAllMocks(); }); +const getSchemaMockFunction = async () => + ({ + json: { + result: ['schema_a', 'schema_b'], + }, + } as any); + +const getTableMockFunction = async () => + ({ + json: { + options: [ + { label: 'table_a', value: 'table_a' }, + { label: 'table_b', value: 'table_b' }, + ], + }, + } as any); + test('renders with default props', async () => { + SupersetClientGet.mockImplementation(getTableMockFunction); + const props = createProps(); render(, { useRedux: true }); const databaseSelect = screen.getByRole('combobox', { @@ -70,6 +81,8 @@ test('renders with default props', async () => { }); test('renders table options', async () => { + SupersetClientGet.mockImplementation(getTableMockFunction); + const props = createProps(); render(, { useRedux: true }); const tableSelect = screen.getByRole('combobox', { @@ -85,6 +98,8 @@ test('renders table options', async () => { }); test('renders disabled without schema', async () => { + SupersetClientGet.mockImplementation(getTableMockFunction); + const props = createProps(); render(, { useRedux: true }); const tableSelect = screen.getByRole('combobox', { @@ -94,3 +109,42 @@ test('renders disabled without schema', async () => { expect(tableSelect).toBeDisabled(); }); }); + +test('table options are notified after schema selection', async () => { + SupersetClientGet.mockImplementation(getSchemaMockFunction); + + const callback = jest.fn(); + const props = createProps({ + onTablesLoad: callback, + schema: undefined, + }); + render(, { useRedux: true }); + + const schemaSelect = screen.getByRole('combobox', { + name: 'Select schema or type schema name', + }); + expect(schemaSelect).toBeInTheDocument(); + expect(callback).not.toHaveBeenCalled(); + + userEvent.click(schemaSelect); + + expect( + await screen.findByRole('option', { name: 'schema_a' }), + ).toBeInTheDocument(); + expect( + await screen.findByRole('option', { name: 'schema_b' }), + ).toBeInTheDocument(); + + SupersetClientGet.mockImplementation(getTableMockFunction); + + act(() => { + userEvent.click(screen.getAllByText('schema_a')[1]); + }); + + await waitFor(() => { + expect(callback).toHaveBeenCalledWith([ + { label: 'table_a', value: 'table_a' }, + { label: 'table_b', value: 'table_b' }, + ]); + }); +}); diff --git a/superset-frontend/src/components/TableSelector/index.tsx b/superset-frontend/src/components/TableSelector/index.tsx index 88ac9cefba47f..50804f7d920ce 100644 --- a/superset-frontend/src/components/TableSelector/index.tsx +++ b/superset-frontend/src/components/TableSelector/index.tsx @@ -208,15 +208,14 @@ const TableSelector: FunctionComponent = ({ currentTable = option; } }); - if (onTablesLoad) { - onTablesLoad(json.options); - } + + onTablesLoad?.(json.options); setTableOptions(options); setCurrentTable(currentTable); setLoadingTables(false); if (forceRefresh) addSuccessToast('List updated'); }) - .catch(e => { + .catch(() => { setLoadingTables(false); handleError(t('There was an error loading the tables')); }); diff --git a/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.stories.tsx b/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.stories.tsx index cf9d1d6e730ed..6bf0d438daca6 100644 --- a/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.stories.tsx +++ b/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.stories.tsx @@ -18,7 +18,7 @@ */ import React from 'react'; import { useArgs } from '@storybook/client-api'; -import TimezoneSelector, { TimezoneProps } from './index'; +import TimezoneSelector, { TimezoneSelectorProps } from './index'; export default { title: 'TimezoneSelector', @@ -26,7 +26,7 @@ export default { }; // eslint-disable-next-line @typescript-eslint/no-unused-vars -export const InteractiveTimezoneSelector = (args: TimezoneProps) => { +export const InteractiveTimezoneSelector = (args: TimezoneSelectorProps) => { const [{ timezone }, updateArgs] = useArgs(); const onTimezoneChange = (value: string) => { updateArgs({ timezone: value }); diff --git a/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx b/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx index 035cff842c9e2..19c713adf4f13 100644 --- a/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx +++ b/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx @@ -20,21 +20,42 @@ import React from 'react'; import moment from 'moment-timezone'; import { render, screen, waitFor } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; -import TimezoneSelector from './index'; +import type { TimezoneSelectorProps } from './index'; -jest.spyOn(moment.tz, 'guess').mockReturnValue('America/New_York'); +const loadComponent = (mockCurrentTime?: string) => { + if (mockCurrentTime) { + jest.useFakeTimers('modern'); + jest.setSystemTime(new Date(mockCurrentTime)); + } + return new Promise>(resolve => { + jest.isolateModules(() => { + const { default: TimezoneSelector } = module.require('./index'); + resolve(TimezoneSelector); + jest.useRealTimers(); + }); + }); +}; const getSelectOptions = () => waitFor(() => document.querySelectorAll('.ant-select-item-option-content')); -it('use the timezone from `moment` if no timezone provided', () => { +const openSelectMenu = async () => { + const searchInput = screen.getByRole('combobox'); + userEvent.click(searchInput); +}; + +jest.spyOn(moment.tz, 'guess').mockReturnValue('America/New_York'); + +test('use the timezone from `moment` if no timezone provided', async () => { + const TimezoneSelector = await loadComponent('2022-01-01'); const onTimezoneChange = jest.fn(); render(); expect(onTimezoneChange).toHaveBeenCalledTimes(1); expect(onTimezoneChange).toHaveBeenCalledWith('America/Nassau'); }); -it('update to closest deduped timezone when timezone is provided', async () => { +test('update to closest deduped timezone when timezone is provided', async () => { + const TimezoneSelector = await loadComponent('2022-01-01'); const onTimezoneChange = jest.fn(); render( { expect(onTimezoneChange).toHaveBeenLastCalledWith('America/Vancouver'); }); -it('use the default timezone when an invalid timezone is provided', async () => { +test('use the default timezone when an invalid timezone is provided', async () => { + const TimezoneSelector = await loadComponent('2022-01-01'); const onTimezoneChange = jest.fn(); render( , @@ -55,7 +77,8 @@ it('use the default timezone when an invalid timezone is provided', async () => expect(onTimezoneChange).toHaveBeenLastCalledWith('Africa/Abidjan'); }); -it('can select a timezone values and returns canonical value', async () => { +test('render timezones in correct oder for standard time', async () => { + const TimezoneSelector = await loadComponent('2022-01-01'); const onTimezoneChange = jest.fn(); render( { timezone="America/Nassau" />, ); - - const searchInput = screen.getByRole('combobox', { - name: 'Timezone selector', - }); - expect(searchInput).toBeInTheDocument(); - userEvent.click(searchInput); - const isDaylight = moment(moment.now()).isDST(); - - const selectedTimezone = isDaylight - ? 'GMT -04:00 (Eastern Daylight Time)' - : 'GMT -05:00 (Eastern Standard Time)'; - - // selected option ranks first + await openSelectMenu(); const options = await getSelectOptions(); - expect(options[0]).toHaveTextContent(selectedTimezone); - - // others are ranked by offset + expect(options[0]).toHaveTextContent('GMT -05:00 (Eastern Standard Time)'); expect(options[1]).toHaveTextContent('GMT -11:00 (Pacific/Pago_Pago)'); expect(options[2]).toHaveTextContent('GMT -10:00 (Hawaii Standard Time)'); expect(options[3]).toHaveTextContent('GMT -10:00 (America/Adak)'); +}); + +test('render timezones in correct order for daylight saving time', async () => { + const TimezoneSelector = await loadComponent('2022-07-01'); + const onTimezoneChange = jest.fn(); + render( + , + ); + await openSelectMenu(); + const options = await getSelectOptions(); + // first option is always current timezone + expect(options[0]).toHaveTextContent('GMT -04:00 (Eastern Daylight Time)'); + expect(options[1]).toHaveTextContent('GMT -11:00 (Pacific/Pago_Pago)'); + expect(options[2]).toHaveTextContent('GMT -10:00 (Hawaii Standard Time)'); + expect(options[3]).toHaveTextContent('GMT -09:30 (Pacific/Marquesas)'); +}); +test('can select a timezone values and returns canonical timezone name', async () => { + const TimezoneSelector = await loadComponent('2022-01-01'); + const onTimezoneChange = jest.fn(); + render( + , + ); + + await openSelectMenu(); + + const searchInput = screen.getByRole('combobox'); // search for mountain time await userEvent.type(searchInput, 'mou', { delay: 10 }); - - const findTitle = isDaylight - ? 'GMT -06:00 (Mountain Daylight Time)' - : 'GMT -07:00 (Mountain Standard Time)'; + const findTitle = 'GMT -07:00 (Mountain Standard Time)'; const selectOption = await screen.findByTitle(findTitle); - expect(selectOption).toBeInTheDocument(); userEvent.click(selectOption); expect(onTimezoneChange).toHaveBeenCalledTimes(1); expect(onTimezoneChange).toHaveBeenLastCalledWith('America/Cambridge_Bay'); }); -it('can update props and rerender with different values', async () => { +test('can update props and rerender with different values', async () => { + const TimezoneSelector = await loadComponent('2022-01-01'); const onTimezoneChange = jest.fn(); const { rerender } = render( { ); }; -export interface TimezoneProps { - onTimezoneChange: (value: string) => void; - timezone?: string | null; -} - const ALL_ZONES = moment.tz .countries() .map(country => moment.tz.zonesForCountry(country, true)) @@ -106,7 +101,15 @@ const matchTimezoneToOptions = (timezone: string) => TIMEZONE_OPTIONS.find(option => option.offsets === getOffsetKey(timezone)) ?.value || DEFAULT_TIMEZONE.value; -const TimezoneSelector = ({ onTimezoneChange, timezone }: TimezoneProps) => { +export type TimezoneSelectorProps = { + onTimezoneChange: (value: string) => void; + timezone?: string | null; +}; + +export default function TimezoneSelector({ + onTimezoneChange, + timezone, +}: TimezoneSelectorProps) { const validTimezone = useMemo( () => matchTimezoneToOptions(timezone || moment.tz.guess()), [timezone], @@ -129,6 +132,4 @@ const TimezoneSelector = ({ onTimezoneChange, timezone }: TimezoneProps) => { sortComparator={TIMEZONE_OPTIONS_SORT_COMPARATOR} /> ); -}; - -export default TimezoneSelector; +} diff --git a/superset-frontend/src/components/URLShortLinkButton/URLShortLinkButton.test.tsx b/superset-frontend/src/components/URLShortLinkButton/URLShortLinkButton.test.tsx index f54a2ba364fbb..36ffc9e339432 100644 --- a/superset-frontend/src/components/URLShortLinkButton/URLShortLinkButton.test.tsx +++ b/superset-frontend/src/components/URLShortLinkButton/URLShortLinkButton.test.tsx @@ -23,48 +23,76 @@ import fetchMock from 'fetch-mock'; import URLShortLinkButton from 'src/components/URLShortLinkButton'; import ToastContainer from 'src/components/MessageToasts/ToastContainer'; -const fakeUrl = 'http://fakeurl.com'; +const DASHBOARD_ID = 10; +const PERMALINK_PAYLOAD = { + key: '123', + url: 'http://fakeurl.com/123', +}; +const FILTER_STATE_PAYLOAD = { + value: '{}', +}; -fetchMock.post('glob:*/r/shortner/', fakeUrl); +const props = { + dashboardId: DASHBOARD_ID, +}; + +fetchMock.get( + `glob:*/api/v1/dashboard/${DASHBOARD_ID}/filter_state*`, + FILTER_STATE_PAYLOAD, +); + +fetchMock.post( + `glob:*/api/v1/dashboard/${DASHBOARD_ID}/permalink`, + PERMALINK_PAYLOAD, +); test('renders with default props', () => { - render(, { useRedux: true }); + render(, { useRedux: true }); expect(screen.getByRole('button')).toBeInTheDocument(); }); test('renders overlay on click', async () => { - render(, { useRedux: true }); + render(, { useRedux: true }); userEvent.click(screen.getByRole('button')); expect(await screen.findByRole('tooltip')).toBeInTheDocument(); }); test('obtains short url', async () => { - render(, { useRedux: true }); + render(, { useRedux: true }); userEvent.click(screen.getByRole('button')); - expect(await screen.findByRole('tooltip')).toHaveTextContent(fakeUrl); + expect(await screen.findByRole('tooltip')).toHaveTextContent( + PERMALINK_PAYLOAD.url, + ); }); test('creates email anchor', async () => { const subject = 'Subject'; const content = 'Content'; - render(, { - useRedux: true, - }); + render( + , + { + useRedux: true, + }, + ); - const href = `mailto:?Subject=${subject}%20&Body=${content}${fakeUrl}`; + const href = `mailto:?Subject=${subject}%20&Body=${content}${PERMALINK_PAYLOAD.url}`; userEvent.click(screen.getByRole('button')); expect(await screen.findByRole('link')).toHaveAttribute('href', href); }); test('renders error message on short url error', async () => { - fetchMock.mock('glob:*/r/shortner/', 500, { + fetchMock.mock(`glob:*/api/v1/dashboard/${DASHBOARD_ID}/permalink`, 500, { overwriteRoutes: true, }); render( <> - + , { useRedux: true }, diff --git a/superset-frontend/src/components/URLShortLinkButton/index.jsx b/superset-frontend/src/components/URLShortLinkButton/index.jsx index 1678471b61f79..35795f81a11fa 100644 --- a/superset-frontend/src/components/URLShortLinkButton/index.jsx +++ b/superset-frontend/src/components/URLShortLinkButton/index.jsx @@ -21,14 +21,17 @@ import PropTypes from 'prop-types'; import { t } from '@superset-ui/core'; import Popover from 'src/components/Popover'; import CopyToClipboard from 'src/components/CopyToClipboard'; -import { getShortUrl } from 'src/utils/urlUtils'; +import { getDashboardPermalink, getUrlParam } from 'src/utils/urlUtils'; import withToasts from 'src/components/MessageToasts/withToasts'; +import { URL_PARAMS } from 'src/constants'; +import { getFilterValue } from 'src/dashboard/components/nativeFilters/FilterBar/keyValue'; const propTypes = { - url: PropTypes.string, + addDangerToast: PropTypes.func.isRequired, + anchorLinkId: PropTypes.string, + dashboardId: PropTypes.number, emailSubject: PropTypes.string, emailContent: PropTypes.string, - addDangerToast: PropTypes.func.isRequired, placement: PropTypes.oneOf(['right', 'left', 'top', 'bottom']), }; @@ -50,9 +53,20 @@ class URLShortLinkButton extends React.Component { getCopyUrl(e) { e.stopPropagation(); - getShortUrl(this.props.url) - .then(this.onShortUrlSuccess) - .catch(this.props.addDangerToast); + const nativeFiltersKey = getUrlParam(URL_PARAMS.nativeFiltersKey); + if (this.props.dashboardId) { + getFilterValue(this.props.dashboardId, nativeFiltersKey) + .then(filterState => + getDashboardPermalink( + String(this.props.dashboardId), + filterState, + this.props.anchorLinkId, + ) + .then(this.onShortUrlSuccess) + .catch(this.props.addDangerToast), + ) + .catch(this.props.addDangerToast); + } } renderPopover() { @@ -96,7 +110,6 @@ class URLShortLinkButton extends React.Component { } URLShortLinkButton.defaultProps = { - url: window.location.href.substring(window.location.origin.length), placement: 'left', emailSubject: '', emailContent: '', diff --git a/superset-frontend/src/constants.ts b/superset-frontend/src/constants.ts index b54fc1173c28f..777d5f2a4e434 100644 --- a/superset-frontend/src/constants.ts +++ b/superset-frontend/src/constants.ts @@ -71,8 +71,24 @@ export const URL_PARAMS = { name: 'force', type: 'boolean', }, + permalinkKey: { + name: 'permalink_key', + type: 'string', + }, } as const; +export const RESERVED_CHART_URL_PARAMS: string[] = [ + URL_PARAMS.formDataKey.name, + URL_PARAMS.sliceId.name, + URL_PARAMS.datasetId.name, +]; +export const RESERVED_DASHBOARD_URL_PARAMS: string[] = [ + URL_PARAMS.nativeFilters.name, + URL_PARAMS.nativeFiltersKey.name, + URL_PARAMS.permalinkKey.name, + URL_PARAMS.preselectFilters.name, +]; + /** * Faster debounce delay for inputs without expensive operation. */ diff --git a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/HeaderActionsDropdown.test.tsx b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/HeaderActionsDropdown.test.tsx index 8996645079472..d1f87ec999e0c 100644 --- a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/HeaderActionsDropdown.test.tsx +++ b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/HeaderActionsDropdown.test.tsx @@ -135,8 +135,8 @@ test('should show the share actions', async () => { }; render(setup(canShareProps)); await openDropdown(); - expect(screen.getByText('Copy dashboard URL')).toBeInTheDocument(); - expect(screen.getByText('Share dashboard by email')).toBeInTheDocument(); + expect(screen.getByText('Copy permalink to clipboard')).toBeInTheDocument(); + expect(screen.getByText('Share permalink by email')).toBeInTheDocument(); }); test('should render the "Save Modal" when user can save', async () => { diff --git a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx index b7d368ec32db2..9375c684af90a 100644 --- a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx @@ -257,8 +257,8 @@ class HeaderActionsDropdown extends React.PureComponent { {userCanShare && ( ; onExploreChart: () => void; forceRefresh: (sliceId: number, dashboardId: number) => void; @@ -309,8 +310,8 @@ class SliceHeaderControls extends React.PureComponent< {supersetCanShare && ( = 5 ? 'left' : 'right'} diff --git a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx index da7d196bd8b50..579f9d4b69077 100644 --- a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx +++ b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx @@ -31,7 +31,7 @@ const DASHBOARD_ID = '26'; const createProps = () => ({ addDangerToast: jest.fn(), addSuccessToast: jest.fn(), - url: `/superset/dashboard/${DASHBOARD_ID}/?preselect_filters=%7B%7D`, + url: `/superset/dashboard/${DASHBOARD_ID}`, copyMenuItemTitle: 'Copy dashboard URL', emailMenuItemTitle: 'Share dashboard by email', emailSubject: 'Superset dashboard COVID Vaccine Dashboard', @@ -45,10 +45,10 @@ beforeAll((): void => { // @ts-ignore delete window.location; fetchMock.post( - 'http://localhost/r/shortner/', - { body: 'http://localhost:8088/r/3' }, + `http://localhost/api/v1/dashboard/${DASHBOARD_ID}/permalink`, + { key: '123', url: 'http://localhost/superset/dashboard/p/123/' }, { - sendAsJson: false, + sendAsJson: true, }, ); }); @@ -104,7 +104,7 @@ test('Click on "Copy dashboard URL" and succeed', async () => { await waitFor(() => { expect(spy).toBeCalledTimes(1); - expect(spy).toBeCalledWith('http://localhost:8088/r/3'); + expect(spy).toBeCalledWith('http://localhost/superset/dashboard/p/123/'); expect(props.addSuccessToast).toBeCalledTimes(1); expect(props.addSuccessToast).toBeCalledWith('Copied to clipboard!'); expect(props.addDangerToast).toBeCalledTimes(0); @@ -130,7 +130,7 @@ test('Click on "Copy dashboard URL" and fail', async () => { await waitFor(() => { expect(spy).toBeCalledTimes(1); - expect(spy).toBeCalledWith('http://localhost:8088/r/3'); + expect(spy).toBeCalledWith('http://localhost/superset/dashboard/p/123/'); expect(props.addSuccessToast).toBeCalledTimes(0); expect(props.addDangerToast).toBeCalledTimes(1); expect(props.addDangerToast).toBeCalledWith( @@ -159,14 +159,14 @@ test('Click on "Share dashboard by email" and succeed', async () => { await waitFor(() => { expect(props.addDangerToast).toBeCalledTimes(0); expect(window.location.href).toBe( - 'mailto:?Subject=Superset%20dashboard%20COVID%20Vaccine%20Dashboard%20&Body=Check%20out%20this%20dashboard%3A%20http%3A%2F%2Flocalhost%3A8088%2Fr%2F3', + 'mailto:?Subject=Superset%20dashboard%20COVID%20Vaccine%20Dashboard%20&Body=Check%20out%20this%20dashboard%3A%20http%3A%2F%2Flocalhost%2Fsuperset%2Fdashboard%2Fp%2F123%2F', ); }); }); test('Click on "Share dashboard by email" and fail', async () => { fetchMock.post( - 'http://localhost/r/shortner/', + `http://localhost/api/v1/dashboard/${DASHBOARD_ID}/permalink`, { status: 404 }, { overwriteRoutes: true }, ); diff --git a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx index cb31503ac8611..c70e47dc3d01d 100644 --- a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx +++ b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx @@ -17,19 +17,16 @@ * under the License. */ import React from 'react'; -import { useUrlShortener } from 'src/hooks/useUrlShortener'; import copyTextToClipboard from 'src/utils/copy'; -import { t, logging } from '@superset-ui/core'; +import { t, logging, QueryFormData } from '@superset-ui/core'; import { Menu } from 'src/components/Menu'; -import { getUrlParam } from 'src/utils/urlUtils'; -import { postFormData } from 'src/explore/exploreUtils/formData'; -import { useTabId } from 'src/hooks/useTabId'; -import { URL_PARAMS } from 'src/constants'; -import { mountExploreUrl } from 'src/explore/exploreUtils'; import { - createFilterKey, - getFilterValue, -} from 'src/dashboard/components/nativeFilters/FilterBar/keyValue'; + getChartPermalink, + getDashboardPermalink, + getUrlParam, +} from 'src/utils/urlUtils'; +import { RESERVED_DASHBOARD_URL_PARAMS, URL_PARAMS } from 'src/constants'; +import { getFilterValue } from 'src/dashboard/components/nativeFilters/FilterBar/keyValue'; interface ShareMenuItemProps { url?: string; @@ -40,12 +37,11 @@ interface ShareMenuItemProps { addDangerToast: Function; addSuccessToast: Function; dashboardId?: string; - formData?: { slice_id: number; datasource: string }; + formData?: Pick; } const ShareMenuItems = (props: ShareMenuItemProps) => { const { - url, copyMenuItemTitle, emailMenuItemTitle, emailSubject, @@ -57,47 +53,25 @@ const ShareMenuItems = (props: ShareMenuItemProps) => { ...rest } = props; - const tabId = useTabId(); - - const getShortUrl = useUrlShortener(url || ''); - - async function getCopyUrl() { - const risonObj = getUrlParam(URL_PARAMS.nativeFilters); - if (typeof risonObj === 'object' || !dashboardId) return null; - const prevData = await getFilterValue( - dashboardId, - getUrlParam(URL_PARAMS.nativeFiltersKey), - ); - const newDataMaskKey = await createFilterKey( - dashboardId, - JSON.stringify(prevData), - tabId, - ); - const newUrl = new URL(`${window.location.origin}${url}`); - newUrl.searchParams.set(URL_PARAMS.nativeFilters.name, newDataMaskKey); - return `${newUrl.pathname}${newUrl.search}`; - } - async function generateUrl() { + // chart if (formData) { - const key = await postFormData( - parseInt(formData.datasource.split('_')[0], 10), - formData, - formData.slice_id, - tabId, - ); - return `${window.location.origin}${mountExploreUrl(null, { - [URL_PARAMS.formDataKey.name]: key, - [URL_PARAMS.sliceId.name]: formData.slice_id, - })}`; + // we need to remove reserved dashboard url params + return getChartPermalink(formData, RESERVED_DASHBOARD_URL_PARAMS); + } + // dashboard + const nativeFiltersKey = getUrlParam(URL_PARAMS.nativeFiltersKey); + let filterState = {}; + if (nativeFiltersKey && dashboardId) { + filterState = await getFilterValue(dashboardId, nativeFiltersKey); } - const copyUrl = await getCopyUrl(); - return getShortUrl(copyUrl); + return getDashboardPermalink(String(dashboardId), filterState); } async function onCopyLink() { try { - await copyTextToClipboard(await generateUrl()); + const url = await generateUrl(); + await copyTextToClipboard(url); addSuccessToast(t('Copied to clipboard!')); } catch (error) { logging.error(error); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index 73a589312acc6..309d75dac9a80 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -165,6 +165,11 @@ export interface FiltersBarProps { offset: number; } +const EXCLUDED_URL_PARAMS: string[] = [ + URL_PARAMS.nativeFilters.name, + URL_PARAMS.permalinkKey.name, +]; + const publishDataMask = debounce( async ( history, @@ -177,9 +182,9 @@ const publishDataMask = debounce( const { search } = location; const previousParams = new URLSearchParams(search); const newParams = new URLSearchParams(); - let dataMaskKey: string; + let dataMaskKey: string | null; previousParams.forEach((value, key) => { - if (key !== URL_PARAMS.nativeFilters.name) { + if (!EXCLUDED_URL_PARAMS.includes(key)) { newParams.append(key, value); } }); @@ -200,7 +205,9 @@ const publishDataMask = debounce( } else { dataMaskKey = await createFilterKey(dashboardId, dataMask, tabId); } - newParams.set(URL_PARAMS.nativeFiltersKey.name, dataMaskKey); + if (dataMaskKey) { + newParams.set(URL_PARAMS.nativeFiltersKey.name, dataMaskKey); + } // pathname could be updated somewhere else through window.history // keep react router history in sync with window history diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/keyValue.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/keyValue.tsx index 9682fdb7b8f0e..ec9735f091690 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/keyValue.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/keyValue.tsx @@ -17,6 +17,7 @@ * under the License. */ import { SupersetClient, logging } from '@superset-ui/core'; +import { DashboardPermalinkValue } from 'src/dashboard/types'; const assembleEndpoint = ( dashId: string | number, @@ -58,7 +59,7 @@ export const createFilterKey = ( endpoint: assembleEndpoint(dashId, undefined, tabId), jsonPayload: { value }, }) - .then(r => r.json.key) + .then(r => r.json.key as string) .catch(err => { logging.error(err); return null; @@ -73,3 +74,13 @@ export const getFilterValue = (dashId: string | number, key: string) => logging.error(err); return null; }); + +export const getPermalinkValue = (key: string) => + SupersetClient.get({ + endpoint: `/api/v1/dashboard/permalink/${key}`, + }) + .then(({ json }) => json as DashboardPermalinkValue) + .catch(err => { + logging.error(err); + return null; + }); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.tsx index 188655355ca45..7a4827c80bcf0 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.tsx @@ -57,7 +57,7 @@ const DragIcon = styled(Icons.Drag, { interface FilterTabTitleProps { index: number; filterIds: string[]; - onRearrage: (dragItemIndex: number, targetIndex: number) => void; + onRearrange: (dragItemIndex: number, targetIndex: number) => void; } interface DragItem { @@ -68,7 +68,7 @@ interface DragItem { export const DraggableFilter: React.FC = ({ index, - onRearrage, + onRearrange, filterIds, children, }) => { @@ -120,7 +120,7 @@ export const DraggableFilter: React.FC = ({ return; } - onRearrage(dragIndex, hoverIndex); + onRearrange(dragIndex, hoverIndex); // Note: we're mutating the monitor item here. // Generally it's better to avoid mutations, // but it's good here for the sake of performance diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx index 78c4d77da1918..3742d536326fb 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx @@ -22,6 +22,9 @@ import { buildNativeFilter } from 'spec/fixtures/mockNativeFilters'; import { act, fireEvent, render, screen } from 'spec/helpers/testing-library'; import FilterConfigPane from './FilterConfigurePane'; +const scrollMock = jest.fn(); +Element.prototype.scroll = scrollMock; + const defaultProps = { children: jest.fn(), getFilterTitle: (id: string) => id, @@ -56,6 +59,10 @@ function defaultRender(initialState: any = defaultState, props = defaultProps) { }); } +beforeEach(() => { + scrollMock.mockClear(); +}); + test('renders form', async () => { await act(async () => { defaultRender(); @@ -65,7 +72,7 @@ test('renders form', async () => { test('drag and drop', async () => { defaultRender(); - // Drag the state and contry filter above the product filter + // Drag the state and country filter above the product filter const [countryStateFilter, productFilter] = document.querySelectorAll( 'div[draggable=true]', ); @@ -132,3 +139,41 @@ test('add divider', async () => { }); expect(defaultProps.onAdd).toHaveBeenCalledWith('DIVIDER'); }); + +test('filter container should scroll to bottom when adding items', async () => { + const state = { + dashboardInfo: { + metadata: { + native_filter_configuration: new Array(35) + .fill(0) + .map((_, index) => + buildNativeFilter(`NATIVE_FILTER-${index}`, `filter-${index}`, []), + ), + }, + }, + dashboardLayout, + }; + const props = { + ...defaultProps, + filters: new Array(35).fill(0).map((_, index) => `NATIVE_FILTER-${index}`), + }; + + defaultRender(state, props); + + const addButton = screen.getByText('Add filters and dividers')!; + fireEvent.mouseOver(addButton); + + const addFilterButton = await screen.findByText('Filter'); + await act(async () => { + fireEvent( + addFilterButton, + new MouseEvent('click', { + bubbles: true, + cancelable: true, + }), + ); + }); + + const containerElement = screen.getByTestId('filter-title-container'); + expect(containerElement.scroll).toHaveBeenCalled(); +}); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx index a65e167fdf39b..dba7e6bb30250 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx @@ -50,7 +50,7 @@ const TitlesContainer = styled.div` border-right: 1px solid ${({ theme }) => theme.colors.grayscale.light2}; `; -const FiltureConfigurePane: React.FC = ({ +const FilterConfigurePane: React.FC = ({ getFilterTitle, onChange, onRemove, @@ -75,7 +75,7 @@ const FiltureConfigurePane: React.FC = ({ getFilterTitle={getFilterTitle} onChange={onChange} onAdd={(type: NativeFilterType) => onAdd(type)} - onRearrage={onRearrange} + onRearrange={onRearrange} onRemove={(id: string) => onRemove(id)} restoreFilter={restoreFilter} /> @@ -98,4 +98,4 @@ const FiltureConfigurePane: React.FC = ({ ); }; -export default FiltureConfigurePane; +export default FilterConfigurePane; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitleContainer.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitleContainer.tsx index 6ef40d8303b57..f5fe459e4b260 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitleContainer.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitleContainer.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import React from 'react'; +import React, { forwardRef } from 'react'; import { styled, t } from '@superset-ui/core'; import Icons from 'src/components/Icons'; import { FilterRemoval } from './types'; @@ -72,124 +72,134 @@ interface Props { removedFilters: Record; onRemove: (id: string) => void; restoreFilter: (id: string) => void; - onRearrage: (dragIndex: number, targetIndex: number) => void; + onRearrange: (dragIndex: number, targetIndex: number) => void; filters: string[]; erroredFilters: string[]; } -const FilterTitleContainer: React.FC = ({ - getFilterTitle, - onChange, - onRemove, - restoreFilter, - onRearrage, - currentFilterId, - removedFilters, - filters, - erroredFilters = [], -}) => { - const renderComponent = (id: string) => { - const isRemoved = !!removedFilters[id]; - const isErrored = erroredFilters.includes(id); - const isActive = currentFilterId === id; - const classNames = []; - if (isErrored) { - classNames.push('errored'); - } - if (isActive) { - classNames.push('active'); - } - return ( - onChange(id)} - className={classNames.join(' ')} - > -
-
- {isRemoved ? t('(Removed)') : getFilterTitle(id)} +const FilterTitleContainer = forwardRef( + ( + { + getFilterTitle, + onChange, + onRemove, + restoreFilter, + onRearrange, + currentFilterId, + removedFilters, + filters, + erroredFilters = [], + }, + ref, + ) => { + const renderComponent = (id: string) => { + const isRemoved = !!removedFilters[id]; + const isErrored = erroredFilters.includes(id); + const isActive = currentFilterId === id; + const classNames = []; + if (isErrored) { + classNames.push('errored'); + } + if (isActive) { + classNames.push('active'); + } + return ( + onChange(id)} + className={classNames.join(' ')} + > +
+
+ {isRemoved ? t('(Removed)') : getFilterTitle(id)} +
+ {!removedFilters[id] && isErrored && ( + + )} + {isRemoved && ( + { + e.preventDefault(); + restoreFilter(id); + }} + > + {t('Undo?')} + + )}
- {!removedFilters[id] && isErrored && ( - - )} - {isRemoved && ( - { - e.preventDefault(); - restoreFilter(id); - }} - > - {t('Undo?')} - - )} -
-
- {isRemoved ? null : ( - { - event.stopPropagation(); - onRemove(id); - }} - alt="RemoveFilter" - /> - )} -
- - ); - }; - const recursivelyRender = ( - elementId: string, - nodeList: Array<{ id: string; parentId: string | null }>, - rendered: Array, - ): React.ReactNode => { - const didAlreadyRender = rendered.indexOf(elementId) >= 0; - if (didAlreadyRender) { - return null; - } - let parent = null; - const element = nodeList.filter(el => el.id === elementId)[0]; - if (!element) { - return null; - } +
+ {isRemoved ? null : ( + { + event.stopPropagation(); + onRemove(id); + }} + alt="RemoveFilter" + /> + )} +
+ + ); + }; + const recursivelyRender = ( + elementId: string, + nodeList: Array<{ id: string; parentId: string | null }>, + rendered: Array, + ): React.ReactNode => { + const didAlreadyRender = rendered.indexOf(elementId) >= 0; + if (didAlreadyRender) { + return null; + } + let parent = null; + const element = nodeList.filter(el => el.id === elementId)[0]; + if (!element) { + return null; + } + + rendered.push(elementId); + if (element.parentId) { + parent = recursivelyRender(element.parentId, nodeList, rendered); + } + const children = nodeList + .filter(item => item.parentId === elementId) + .map(item => recursivelyRender(item.id, nodeList, rendered)); + return ( + <> + {parent} + {renderComponent(elementId)} + {children} + + ); + }; + + const renderFilterGroups = () => { + const items: React.ReactNode[] = []; + filters.forEach((item, index) => { + items.push( + + {renderComponent(item)} + , + ); + }); + return items; + }; - rendered.push(elementId); - if (element.parentId) { - parent = recursivelyRender(element.parentId, nodeList, rendered); - } - const children = nodeList - .filter(item => item.parentId === elementId) - .map(item => recursivelyRender(item.id, nodeList, rendered)); return ( - <> - {parent} - {renderComponent(elementId)} - {children} - + + {renderFilterGroups()} + ); - }; - - const renderFilterGroups = () => { - const items: React.ReactNode[] = []; - filters.forEach((item, index) => { - items.push( - - {renderComponent(item)} - , - ); - }); - return items; - }; - return {renderFilterGroups()}; -}; + }, +); export default FilterTitleContainer; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitlePane.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitlePane.tsx index 5681a41717666..79dc4148349aa 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitlePane.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitlePane.tsx @@ -16,8 +16,8 @@ * specific language governing permissions and limitations * under the License. */ +import React, { useRef } from 'react'; import { NativeFilterType, styled, t, useTheme } from '@superset-ui/core'; -import React from 'react'; import { AntdDropdown } from 'src/components'; import { MainNav as Menu } from 'src/components/Menu'; import FilterTitleContainer from './FilterTitleContainer'; @@ -26,7 +26,7 @@ import { FilterRemoval } from './types'; interface Props { restoreFilter: (id: string) => void; getFilterTitle: (id: string) => string; - onRearrage: (dragIndex: number, targetIndex: number) => void; + onRearrange: (dragIndex: number, targetIndex: number) => void; onRemove: (id: string) => void; onChange: (id: string) => void; onAdd: (type: NativeFilterType) => void; @@ -52,23 +52,26 @@ const TabsContainer = styled.div` flex-direction: column; `; +const options = [ + { label: 'Filter', type: NativeFilterType.NATIVE_FILTER }, + { label: 'Divider', type: NativeFilterType.DIVIDER }, +]; + const FilterTitlePane: React.FC = ({ getFilterTitle, onChange, onAdd, onRemove, - onRearrage, + onRearrange, restoreFilter, currentFilterId, filters, removedFilters, erroredFilters, }) => { + const filtersContainerRef = useRef(null); const theme = useTheme(); - const options = [ - { label: 'Filter', type: NativeFilterType.NATIVE_FILTER }, - { label: 'Divider', type: NativeFilterType.DIVIDER }, - ]; + const handleOnAdd = (type: NativeFilterType) => { onAdd(type); setTimeout(() => { @@ -77,6 +80,11 @@ const FilterTitlePane: React.FC = ({ const navList = element.getElementsByClassName('ant-tabs-nav-list')[0]; navList.scrollTop = navList.scrollHeight; } + + filtersContainerRef?.current?.scroll?.({ + top: filtersContainerRef.current.scrollHeight, + behavior: 'smooth', + }); }, 0); }; const menu = ( @@ -109,6 +117,7 @@ const FilterTitlePane: React.FC = ({ }} > = ({ erroredFilters={erroredFilters} onChange={onChange} onRemove={onRemove} - onRearrage={onRearrage} + onRearrange={onRearrange} restoreFilter={restoreFilter} />
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx index fd3ac06ea6db9..d258b34fa7489 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx @@ -38,7 +38,7 @@ import ErrorBoundary from 'src/components/ErrorBoundary'; import { StyledModal } from 'src/components/Modal'; import { testWithId } from 'src/utils/testUtils'; import { useFilterConfigMap, useFilterConfiguration } from '../state'; -import FiltureConfigurePane from './FilterConfigurePane'; +import FilterConfigurePane from './FilterConfigurePane'; import FiltersConfigForm, { FilterPanels, } from './FiltersConfigForm/FiltersConfigForm'; @@ -379,7 +379,7 @@ export function FiltersConfigModal({ handleConfirmCancel(); } }; - const onRearrage = (dragIndex: number, targetIndex: number) => { + const onRearrange = (dragIndex: number, targetIndex: number) => { const newOrderedFilter = [...orderedFilters]; const removed = newOrderedFilter.splice(dragIndex, 1)[0]; newOrderedFilter.splice(targetIndex, 0, removed); @@ -522,7 +522,7 @@ export function FiltersConfigModal({ onValuesChange={onValuesChange} layout="vertical" > - {(id: string) => getForm(id)} - + diff --git a/superset-frontend/src/dashboard/containers/DashboardPage.tsx b/superset-frontend/src/dashboard/containers/DashboardPage.tsx index 878f3d68695ae..e5fff328724d5 100644 --- a/superset-frontend/src/dashboard/containers/DashboardPage.tsx +++ b/superset-frontend/src/dashboard/containers/DashboardPage.tsx @@ -49,7 +49,10 @@ import { URL_PARAMS } from 'src/constants'; import { getUrlParam } from 'src/utils/urlUtils'; import { canUserEditDashboard } from 'src/dashboard/util/findPermission'; import { getFilterSets } from '../actions/nativeFilters'; -import { getFilterValue } from '../components/nativeFilters/FilterBar/keyValue'; +import { + getFilterValue, + getPermalinkValue, +} from '../components/nativeFilters/FilterBar/keyValue'; import { filterCardPopoverStyle } from '../styles'; export const MigrationContext = React.createContext( @@ -161,12 +164,17 @@ const DashboardPage: FC = () => { useEffect(() => { // eslint-disable-next-line consistent-return async function getDataMaskApplied() { + const permalinkKey = getUrlParam(URL_PARAMS.permalinkKey); const nativeFilterKeyValue = getUrlParam(URL_PARAMS.nativeFiltersKey); let dataMaskFromUrl = nativeFilterKeyValue || {}; const isOldRison = getUrlParam(URL_PARAMS.nativeFilters); - // check if key from key_value api and get datamask - if (nativeFilterKeyValue) { + if (permalinkKey) { + const permalinkValue = await getPermalinkValue(permalinkKey); + if (permalinkValue) { + dataMaskFromUrl = permalinkValue.state.filterState; + } + } else if (nativeFilterKeyValue) { dataMaskFromUrl = await getFilterValue(id, nativeFilterKeyValue); } if (isOldRison) { diff --git a/superset-frontend/src/dashboard/types.ts b/superset-frontend/src/dashboard/types.ts index fbdf362eea709..dffbd9fbe0be8 100644 --- a/superset-frontend/src/dashboard/types.ts +++ b/superset-frontend/src/dashboard/types.ts @@ -144,3 +144,11 @@ type ActiveFilter = { export type ActiveFilters = { [key: string]: ActiveFilter; }; + +export type DashboardPermalinkValue = { + dashboardId: string; + state: { + filterState: DataMaskStateWithId; + hash: string; + }; +}; diff --git a/superset-frontend/src/explore/components/EmbedCodeButton.jsx b/superset-frontend/src/explore/components/EmbedCodeButton.jsx index 57e6d30de4532..71f77a4621fa2 100644 --- a/superset-frontend/src/explore/components/EmbedCodeButton.jsx +++ b/superset-frontend/src/explore/components/EmbedCodeButton.jsx @@ -25,6 +25,7 @@ import Icons from 'src/components/Icons'; import { Tooltip } from 'src/components/Tooltip'; import CopyToClipboard from 'src/components/CopyToClipboard'; import { URL_PARAMS } from 'src/constants'; +import { getChartPermalink } from 'src/utils/urlUtils'; export default class EmbedCodeButton extends React.Component { constructor(props) { @@ -32,8 +33,11 @@ export default class EmbedCodeButton extends React.Component { this.state = { height: '400', width: '600', + url: '', + errorMessage: '', }; this.handleInputChange = this.handleInputChange.bind(this); + this.updateUrl = this.updateUrl.bind(this); } handleInputChange(e) { @@ -43,8 +47,21 @@ export default class EmbedCodeButton extends React.Component { this.setState(data); } + updateUrl() { + this.setState({ url: '' }); + getChartPermalink(this.props.formData) + .then(url => this.setState({ errorMessage: '', url })) + .catch(() => { + this.setState({ errorMessage: t('Error') }); + this.props.addDangerToast( + t('Sorry, something went wrong. Try again later.'), + ); + }); + } + generateEmbedHTML() { - const srcLink = `${window.location.href}&${URL_PARAMS.standalone.name}=1&height=${this.state.height}`; + if (!this.state.url) return ''; + const srcLink = `${this.state.url}?${URL_PARAMS.standalone.name}=1&height=${this.state.height}`; return ( '
@@ -67,7 +86,8 @@ export default class EmbedCodeButton extends React.Component {