From 96b53674feebd815cb52513fbc67521b2dbcfa33 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Fri, 9 Sep 2022 14:01:23 -0700 Subject: [PATCH] fix(explore): missing data on expired form key --- superset-frontend/src/constants.ts | 11 +++-- .../getParsedExploreURLParams.test.ts | 11 ++++- .../exploreUtils/getParsedExploreURLParams.ts | 47 +++++++++---------- superset-frontend/src/explore/fixtures.tsx | 4 +- superset/connectors/sqla/models.py | 3 +- superset/explore/commands/get.py | 17 ++++--- 6 files changed, 54 insertions(+), 39 deletions(-) diff --git a/superset-frontend/src/constants.ts b/superset-frontend/src/constants.ts index 2f0aeff01e570..27cce425ad0ff 100644 --- a/superset-frontend/src/constants.ts +++ b/superset-frontend/src/constants.ts @@ -69,14 +69,18 @@ export const URL_PARAMS = { }, datasourceId: { name: 'datasource_id', + type: 'number', + }, + datasourceType: { + name: 'datasource_type', type: 'string', }, datasetId: { name: 'dataset_id', - type: 'string', + type: 'number', }, - datasourceType: { - name: 'datasource_type', + datasetType: { + name: 'dataset_type', type: 'string', }, dashboardId: { @@ -115,6 +119,7 @@ export const RESERVED_CHART_URL_PARAMS: string[] = [ URL_PARAMS.datasourceId.name, URL_PARAMS.datasourceType.name, URL_PARAMS.datasetId.name, + URL_PARAMS.datasetType.name, ]; export const RESERVED_DASHBOARD_URL_PARAMS: string[] = [ URL_PARAMS.nativeFilters.name, diff --git a/superset-frontend/src/explore/exploreUtils/getParsedExploreURLParams.test.ts b/superset-frontend/src/explore/exploreUtils/getParsedExploreURLParams.test.ts index 5039d3421fb19..6d6484d020a76 100644 --- a/superset-frontend/src/explore/exploreUtils/getParsedExploreURLParams.test.ts +++ b/superset-frontend/src/explore/exploreUtils/getParsedExploreURLParams.test.ts @@ -31,7 +31,7 @@ test('get form_data_key and slice_id from search params - url when moving from d `${EXPLORE_BASE_URL}?form_data_key=yrLXmyE9fmhQ11lM1KgaD1PoPSBpuLZIJfqdyIdw9GoBwhPFRZHeIgeFiNZljbpd&slice_id=56`, ); expect(getParsedExploreURLParams().toString()).toEqual( - 'slice_id=56&form_data_key=yrLXmyE9fmhQ11lM1KgaD1PoPSBpuLZIJfqdyIdw9GoBwhPFRZHeIgeFiNZljbpd', + 'form_data_key=yrLXmyE9fmhQ11lM1KgaD1PoPSBpuLZIJfqdyIdw9GoBwhPFRZHeIgeFiNZljbpd&slice_id=56', ); }); @@ -49,6 +49,15 @@ test('get datasource and viz type from form_data search param - url when creatin ); }); +test('parse legacy url where datasource_id instead of dataset_id is used', () => { + setupLocation( + `${EXPLORE_BASE_URL}?viz_type=big_number&datasource_id=2&datasource_type=query`, + ); + expect(getParsedExploreURLParams().toString()).toEqual( + 'viz_type=big_number&dataset_id=2&dataset_type=query', + ); +}); + test('get permalink key from path params', () => { setupLocation(`${EXPLORE_BASE_URL}p/kpOqweaMY9R/`); expect(getParsedExploreURLParams().toString()).toEqual( diff --git a/superset-frontend/src/explore/exploreUtils/getParsedExploreURLParams.ts b/superset-frontend/src/explore/exploreUtils/getParsedExploreURLParams.ts index 1e5007875a189..b6212d6ddbf12 100644 --- a/superset-frontend/src/explore/exploreUtils/getParsedExploreURLParams.ts +++ b/superset-frontend/src/explore/exploreUtils/getParsedExploreURLParams.ts @@ -17,15 +17,29 @@ * under the License. */ +import { URL_PARAMS } from 'src/constants'; + export interface Location { search: string; pathname: string; } -// mapping { url_param: v1_explore_request_param } const EXPLORE_URL_SEARCH_PARAMS = { + // Current supported URL params + [URL_PARAMS.formDataKey.name]: {}, + [URL_PARAMS.sliceId.name]: {}, + [URL_PARAMS.vizType.name]: {}, + [URL_PARAMS.datasetId.name]: {}, + [URL_PARAMS.datasetType.name]: {}, + [URL_PARAMS.datasourceId.name]: { + aliasOf: URL_PARAMS.datasetId.name, + }, + [URL_PARAMS.datasourceType.name]: { + aliasOf: URL_PARAMS.datasetType.name, + }, + + // legacy URL params form_data: { - name: 'form_data', parser: (formData: string) => { const formDataObject = JSON.parse(formData); if (formDataObject.datasource) { @@ -38,34 +52,16 @@ const EXPLORE_URL_SEARCH_PARAMS = { return formDataObject; }, }, - slice_id: { - name: 'slice_id', - }, - dataset_id: { - name: 'dataset_id', - }, - dataset_type: { - name: 'dataset_type', - }, datasource: { - name: 'datasource', parser: (datasource: string) => { const [dataset_id, dataset_type] = datasource.split('__'); return { dataset_id, dataset_type }; }, }, - form_data_key: { - name: 'form_data_key', - }, - permalink_key: { - name: 'permalink_key', - }, - viz_type: { - name: 'viz_type', - }, - dashboard_id: { - name: 'dashboard_id', - }, + + // URL path params + permalink_key: {}, + dashboard_id: {}, }; const EXPLORE_URL_PATH_PARAMS = { @@ -95,7 +91,8 @@ const getParsedExploreURLSearchParams = (search: string) => { } return { ...acc, - [EXPLORE_URL_SEARCH_PARAMS[currentParam].name]: parsedParamValue, + [EXPLORE_URL_SEARCH_PARAMS[currentParam].aliasOf || currentParam]: + parsedParamValue, }; }, {}); }; diff --git a/superset-frontend/src/explore/fixtures.tsx b/superset-frontend/src/explore/fixtures.tsx index 755985be2bda9..c4e06e044fa49 100644 --- a/superset-frontend/src/explore/fixtures.tsx +++ b/superset-frontend/src/explore/fixtures.tsx @@ -159,8 +159,8 @@ export const fallbackExploreInitialData: ExplorePageInitialData = { verbose_map: {}, main_dttm_col: '', owners: [], - datasource_name: 'missing_datasource', - name: 'missing_datasource', + datasource_name: '[Missing Dataset]', + name: '[Missing Dataset]', description: null, }, slice: null, diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index a72d07f2d0741..e10314e372145 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -126,6 +126,7 @@ from superset.tables.models import Table as NewTable from superset.utils import core as utils from superset.utils.core import ( + DatasourceType, GenericDataType, get_column_name, get_username, @@ -676,7 +677,7 @@ def _process_sql_expression( class SqlaTable(Model, BaseDatasource): # pylint: disable=too-many-public-methods """An ORM object for SqlAlchemy table references""" - type = "table" + type = DatasourceType.TABLE query_language = "sql" is_rls_supported = True columns: List[TableColumn] = [] diff --git a/superset/explore/commands/get.py b/superset/explore/commands/get.py index 3a656ea2b9f8a..de0799819a39e 100644 --- a/superset/explore/commands/get.py +++ b/superset/explore/commands/get.py @@ -38,7 +38,11 @@ ) from superset.explore.permalink.commands.get import GetExplorePermalinkCommand from superset.explore.permalink.exceptions import ExplorePermalinkGetFailedError -from superset.utils import core as utils +from superset.utils.core import ( + DatasourceType, + convert_legacy_filters_into_adhoc, + merge_extra_filters, +) from superset.views.utils import ( get_datasource_info, get_form_data, @@ -79,7 +83,6 @@ def run(self) -> Optional[Dict[str, Any]]: initial_form_data = json.loads(value) if value else {} message = None - if not initial_form_data: if self._slice_id: initial_form_data["slice_id"] = self._slice_id @@ -90,7 +93,7 @@ def run(self) -> Optional[Dict[str, Any]]: elif self._dataset_id: initial_form_data[ "datasource" - ] = f"{self._dataset_id}__{self._dataset_type}" + ] = f"{self._dataset_id}__{self._dataset_type or DatasourceType.TABLE}" if self._form_data_key: message = _( "Form data not found in cache, reverting to dataset metadata." @@ -105,8 +108,8 @@ def run(self) -> Optional[Dict[str, Any]]: ) except SupersetException: self._dataset_id = None - # fallback unkonw datasource to table type - self._dataset_type = SqlaTable.type + # fallback unknown datasource to table type + self._dataset_type = DatasourceType.TABLE dataset: Optional[BaseDatasource] = None if self._dataset_id is not None: @@ -138,8 +141,8 @@ def run(self) -> Optional[Dict[str, Any]]: ) # On explore, merge legacy and extra filters into the form data - utils.convert_legacy_filters_into_adhoc(form_data) - utils.merge_extra_filters(form_data) + convert_legacy_filters_into_adhoc(form_data) + merge_extra_filters(form_data) dummy_dataset_data: Dict[str, Any] = { "type": self._dataset_type,