You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2024/03/19 13:09:53 UTC
(superset) branch 4.0 updated (017e0fc733 -> f52647ff48)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a change to branch 4.0
in repository https://gitbox.apache.org/repos/asf/superset.git
from 017e0fc733 fix: check if guest user modified query (#27484)
new 1016fd92f6 fix(postprocessing): resample with holes (#27487)
new 297849a8b5 fix(alerts/reports): implementing custom_width as an Antd number input (#27260)
new d4314a92d8 fix(explore): Allow only saved metrics and columns (#27539)
new 94f677850c feat: `improve _extract_tables_from_sql` (#26748)
new 35562198f8 fix: pass valid SQL to SM (#27464)
new f52647ff48 perf(sqllab): reduce bootstrap data delay by queries (#27488)
The 6 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../components/QueryHistory/QueryHistory.test.tsx | 73 ++++++-
.../src/SqlLab/components/QueryHistory/index.tsx | 106 ++++++++--
.../src/SqlLab/reducers/getInitialState.test.ts | 21 +-
.../src/SqlLab/reducers/getInitialState.ts | 7 +-
.../DndFilterSelect.test.tsx | 221 +++++++++++++++++++--
.../DndColumnSelectControl/DndFilterSelect.tsx | 41 +++-
.../DndMetricSelect.test.tsx | 121 +++++++++++
.../DndColumnSelectControl/DndMetricSelect.tsx | 32 ++-
superset-frontend/src/explore/types.ts | 1 +
.../src/features/alerts/AlertReportModal.tsx | 14 +-
.../src/hooks/apiResources/queries.test.ts | 154 ++++++++++++++
.../src/hooks/apiResources/queries.ts | 176 ++++++++++++++++
.../src/hooks/apiResources/queryApi.ts | 3 +-
superset/queries/api.py | 19 +-
superset/security/manager.py | 6 +-
superset/sql_parse.py | 18 +-
superset/sqllab/utils.py | 15 +-
superset/utils/pandas_postprocessing/resample.py | 5 +-
tests/integration_tests/sql_lab/api_tests.py | 22 +-
tests/integration_tests/sqllab_tests.py | 57 ++++++
.../unit_tests/commands/dataset}/__init__.py | 0
tests/unit_tests/jinja_context_test.py | 8 +
.../pandas_postprocessing/test_resample.py | 54 ++++-
tests/unit_tests/security/manager_test.py | 36 ++++
tests/unit_tests/sql_parse_tests.py | 34 +++-
25 files changed, 1137 insertions(+), 107 deletions(-)
create mode 100644 superset-frontend/src/hooks/apiResources/queries.test.ts
create mode 100644 superset-frontend/src/hooks/apiResources/queries.ts
copy {superset/advanced_data_type => tests/unit_tests/commands/dataset}/__init__.py (100%)
(superset) 04/06: feat: `improve _extract_tables_from_sql` (#26748)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 4.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 94f677850c19b96bb9bc072fda82ad9171f66e7e
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Mon Mar 18 13:02:58 2024 -0400
feat: `improve _extract_tables_from_sql` (#26748)
---
superset/sql_parse.py | 18 +++++++++++++---
tests/unit_tests/jinja_context_test.py | 8 ++++++++
tests/unit_tests/security/manager_test.py | 1 +
tests/unit_tests/sql_parse_tests.py | 34 ++++++++++++++++++++++++++-----
4 files changed, 53 insertions(+), 8 deletions(-)
diff --git a/superset/sql_parse.py b/superset/sql_parse.py
index c85afc9460..49ee4c491f 100644
--- a/superset/sql_parse.py
+++ b/superset/sql_parse.py
@@ -25,6 +25,7 @@ from dataclasses import dataclass
from typing import Any, cast, Optional
import sqlparse
+from flask_babel import gettext as __
from sqlalchemy import and_
from sqlglot import exp, parse, parse_one
from sqlglot.dialects import Dialects
@@ -55,7 +56,11 @@ from sqlparse.tokens import (
)
from sqlparse.utils import imt
-from superset.exceptions import QueryClauseValidationException
+from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
+from superset.exceptions import (
+ QueryClauseValidationException,
+ SupersetSecurityException,
+)
from superset.utils.backports import StrEnum
try:
@@ -287,9 +292,16 @@ class ParsedQuery:
"""
try:
statements = parse(self.stripped(), dialect=self._dialect)
- except SqlglotError:
+ except SqlglotError as ex:
logger.warning("Unable to parse SQL (%s): %s", self._dialect, self.sql)
- return set()
+ dialect = self._dialect or "generic"
+ raise SupersetSecurityException(
+ SupersetError(
+ error_type=SupersetErrorType.QUERY_SECURITY_ACCESS_ERROR,
+ message=__(f"Unable to parse SQL ({dialect}): {self.sql}"),
+ level=ErrorLevel.ERROR,
+ )
+ ) from ex
return {
table
diff --git a/tests/unit_tests/jinja_context_test.py b/tests/unit_tests/jinja_context_test.py
index e2a5e8cd49..bbd9bcf06c 100644
--- a/tests/unit_tests/jinja_context_test.py
+++ b/tests/unit_tests/jinja_context_test.py
@@ -90,6 +90,14 @@ def test_dataset_macro(mocker: MockFixture) -> None:
)
DatasetDAO = mocker.patch("superset.daos.dataset.DatasetDAO")
DatasetDAO.find_by_id.return_value = dataset
+ mocker.patch(
+ "superset.connectors.sqla.models.security_manager.get_guest_rls_filters",
+ return_value=[],
+ )
+ mocker.patch(
+ "superset.models.helpers.security_manager.get_guest_rls_filters",
+ return_value=[],
+ )
assert (
dataset_macro(1)
diff --git a/tests/unit_tests/security/manager_test.py b/tests/unit_tests/security/manager_test.py
index 5a06013a68..325531c25b 100644
--- a/tests/unit_tests/security/manager_test.py
+++ b/tests/unit_tests/security/manager_test.py
@@ -202,6 +202,7 @@ def test_raise_for_access_query_default_schema(
sm = SupersetSecurityManager(appbuilder)
mocker.patch.object(sm, "can_access_database", return_value=False)
mocker.patch.object(sm, "get_schema_perm", return_value="[PostgreSQL].[public]")
+ mocker.patch.object(sm, "is_guest_user", return_value=False)
SqlaTable = mocker.patch("superset.connectors.sqla.models.SqlaTable")
SqlaTable.query_datasources_by_name.return_value = []
diff --git a/tests/unit_tests/sql_parse_tests.py b/tests/unit_tests/sql_parse_tests.py
index 2fd23f7e8e..025108e9b5 100644
--- a/tests/unit_tests/sql_parse_tests.py
+++ b/tests/unit_tests/sql_parse_tests.py
@@ -25,7 +25,10 @@ from sqlalchemy import text
from sqlparse.sql import Identifier, Token, TokenList
from sqlparse.tokens import Name
-from superset.exceptions import QueryClauseValidationException
+from superset.exceptions import (
+ QueryClauseValidationException,
+ SupersetSecurityException,
+)
from superset.sql_parse import (
add_table_name,
extract_table_references,
@@ -265,13 +268,34 @@ def test_extract_tables_illdefined() -> None:
"""
Test that ill-defined tables return an empty set.
"""
- assert extract_tables("SELECT * FROM schemaname.") == set()
- assert extract_tables("SELECT * FROM catalogname.schemaname.") == set()
- assert extract_tables("SELECT * FROM catalogname..") == set()
+ with pytest.raises(SupersetSecurityException) as excinfo:
+ extract_tables("SELECT * FROM schemaname.")
+ assert (
+ str(excinfo.value) == "Unable to parse SQL (generic): SELECT * FROM schemaname."
+ )
+
+ with pytest.raises(SupersetSecurityException) as excinfo:
+ extract_tables("SELECT * FROM catalogname.schemaname.")
+ assert (
+ str(excinfo.value)
+ == "Unable to parse SQL (generic): SELECT * FROM catalogname.schemaname."
+ )
+
+ with pytest.raises(SupersetSecurityException) as excinfo:
+ extract_tables("SELECT * FROM catalogname..")
+ assert (
+ str(excinfo.value)
+ == "Unable to parse SQL (generic): SELECT * FROM catalogname.."
+ )
+
+ with pytest.raises(SupersetSecurityException) as excinfo:
+ extract_tables('SELECT * FROM "tbname')
+ assert str(excinfo.value) == 'Unable to parse SQL (generic): SELECT * FROM "tbname'
+
+ # odd edge case that works
assert extract_tables("SELECT * FROM catalogname..tbname") == {
Table(table="tbname", schema=None, catalog="catalogname")
}
- assert extract_tables('SELECT * FROM "tbname') == set()
def test_extract_tables_show_tables_from() -> None:
(superset) 02/06: fix(alerts/reports): implementing custom_width as an Antd number input (#27260)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 4.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 297849a8b507ae03a17a9f0001adf4d1a6b29558
Author: Jack <41...@users.noreply.github.com>
AuthorDate: Fri Mar 15 13:10:00 2024 -0500
fix(alerts/reports): implementing custom_width as an Antd number input (#27260)
(cherry picked from commit ad9024b040c3ccfd59ce531889b631049b67ea97)
---
superset-frontend/src/features/alerts/AlertReportModal.tsx | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/superset-frontend/src/features/alerts/AlertReportModal.tsx b/superset-frontend/src/features/alerts/AlertReportModal.tsx
index bc6908cac2..c297da2d71 100644
--- a/superset-frontend/src/features/alerts/AlertReportModal.tsx
+++ b/superset-frontend/src/features/alerts/AlertReportModal.tsx
@@ -36,7 +36,7 @@ import {
import rison from 'rison';
import { useSingleViewResource } from 'src/views/CRUD/hooks';
-import { Input } from 'src/components/Input';
+import { InputNumber } from 'src/components/Input';
import { Switch } from 'src/components/Switch';
import Modal from 'src/components/Modal';
import Collapse from 'src/components/Collapse';
@@ -873,6 +873,10 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
updateAlertState(name, parsedValue);
};
+ const onCustomWidthChange = (value: number | null | undefined) => {
+ updateAlertState('custom_width', value);
+ };
+
const onTimeoutVerifyChange = (
event: React.ChangeEvent<HTMLTextAreaElement | HTMLInputElement>,
) => {
@@ -1542,12 +1546,14 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
>
<div className="control-label">{t('Screenshot width')}</div>
<div className="input-container">
- <Input
+ <InputNumber
type="number"
name="custom_width"
- value={currentAlert?.custom_width || ''}
+ value={currentAlert?.custom_width || undefined}
+ min={600}
+ max={2400}
placeholder={t('Input custom width in pixels')}
- onChange={onInputChange}
+ onChange={onCustomWidthChange}
/>
</div>
</StyledInputContainer>
(superset) 05/06: fix: pass valid SQL to SM (#27464)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 4.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 35562198f81db82bd06e4611c0ba01c4b5593a1e
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Mon Mar 18 15:38:58 2024 -0400
fix: pass valid SQL to SM (#27464)
(cherry picked from commit 376bfd05bdba2bbc4bde2d209324105d0d408ee4)
---
superset/security/manager.py | 6 ++++-
tests/unit_tests/commands/dataset/__init__.py | 16 ++++++++++++
tests/unit_tests/security/manager_test.py | 35 +++++++++++++++++++++++++++
3 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/superset/security/manager.py b/superset/security/manager.py
index d56c0ad688..94719cb524 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -59,6 +59,7 @@ from superset.exceptions import (
DatasetInvalidPermissionEvaluationException,
SupersetSecurityException,
)
+from superset.jinja_context import get_template_processor
from superset.security.guest_token import (
GuestToken,
GuestTokenResources,
@@ -1956,11 +1957,14 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
return
if query:
+ # make sure the quuery is valid SQL by rendering any Jinja
+ processor = get_template_processor(database=query.database)
+ rendered_sql = processor.process_template(query.sql)
default_schema = database.get_default_schema_for_query(query)
tables = {
Table(table_.table, table_.schema or default_schema)
for table_ in sql_parse.ParsedQuery(
- query.sql,
+ rendered_sql,
engine=database.db_engine_spec.engine,
).tables
}
diff --git a/tests/unit_tests/commands/dataset/__init__.py b/tests/unit_tests/commands/dataset/__init__.py
new file mode 100644
index 0000000000..13a83393a9
--- /dev/null
+++ b/tests/unit_tests/commands/dataset/__init__.py
@@ -0,0 +1,16 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
diff --git a/tests/unit_tests/security/manager_test.py b/tests/unit_tests/security/manager_test.py
index 325531c25b..22ec0dda4a 100644
--- a/tests/unit_tests/security/manager_test.py
+++ b/tests/unit_tests/security/manager_test.py
@@ -27,6 +27,7 @@ from superset.exceptions import SupersetSecurityException
from superset.extensions import appbuilder
from superset.models.slice import Slice
from superset.security.manager import SupersetSecurityManager
+from superset.sql_parse import Table
from superset.superset_typing import AdhocMetric
from superset.utils.core import override_user
@@ -245,6 +246,40 @@ def test_raise_for_access_query_default_schema(
)
+def test_raise_for_access_jinja_sql(mocker: MockFixture, app_context: None) -> None:
+ """
+ Test that Jinja gets rendered to SQL.
+ """
+ sm = SupersetSecurityManager(appbuilder)
+ mocker.patch.object(sm, "can_access_database", return_value=False)
+ mocker.patch.object(sm, "get_schema_perm", return_value="[PostgreSQL].[public]")
+ mocker.patch.object(sm, "can_access", return_value=False)
+ mocker.patch.object(sm, "is_guest_user", return_value=False)
+ get_table_access_error_object = mocker.patch.object(
+ sm, "get_table_access_error_object"
+ )
+ SqlaTable = mocker.patch("superset.connectors.sqla.models.SqlaTable")
+ SqlaTable.query_datasources_by_name.return_value = []
+
+ database = mocker.MagicMock()
+ database.get_default_schema_for_query.return_value = "public"
+ query = mocker.MagicMock()
+ query.database = database
+ query.sql = "SELECT * FROM {% if True %}ab_user{% endif %} WHERE 1=1"
+
+ with pytest.raises(SupersetSecurityException):
+ sm.raise_for_access(
+ database=None,
+ datasource=None,
+ query=query,
+ query_context=None,
+ table=None,
+ viz=None,
+ )
+
+ get_table_access_error_object.assert_called_with({Table("ab_user", "public")})
+
+
def test_raise_for_access_chart_for_datasource_permission(
mocker: MockFixture,
app_context: None,
(superset) 06/06: perf(sqllab): reduce bootstrap data delay by queries (#27488)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 4.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit f52647ff48219482bc65191834e9815a7e1f6dc3
Author: JUST.in DO IT <ju...@airbnb.com>
AuthorDate: Mon Mar 18 12:52:23 2024 -0700
perf(sqllab): reduce bootstrap data delay by queries (#27488)
(cherry picked from commit f4bdcb5743d7f70048d922500975496f8f219dc7)
---
.../components/QueryHistory/QueryHistory.test.tsx | 73 ++++++++-
.../src/SqlLab/components/QueryHistory/index.tsx | 106 ++++++++++---
.../src/SqlLab/reducers/getInitialState.test.ts | 21 +--
.../src/SqlLab/reducers/getInitialState.ts | 7 +-
.../src/hooks/apiResources/queries.test.ts | 154 ++++++++++++++++++
.../src/hooks/apiResources/queries.ts | 176 +++++++++++++++++++++
.../src/hooks/apiResources/queryApi.ts | 3 +-
superset/queries/api.py | 19 ++-
superset/sqllab/utils.py | 15 +-
tests/integration_tests/sql_lab/api_tests.py | 22 +--
tests/integration_tests/sqllab_tests.py | 57 +++++++
11 files changed, 577 insertions(+), 76 deletions(-)
diff --git a/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx b/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx
index ad1881b5d9..110e7b4ae2 100644
--- a/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx
+++ b/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx
@@ -17,7 +17,10 @@
* under the License.
*/
import React from 'react';
-import { render, screen } from 'spec/helpers/testing-library';
+import fetchMock from 'fetch-mock';
+import * as uiCore from '@superset-ui/core';
+import { FeatureFlag, QueryState } from '@superset-ui/core';
+import { render, screen, waitFor } from 'spec/helpers/testing-library';
import QueryHistory from 'src/SqlLab/components/QueryHistory';
import { initialState } from 'src/SqlLab/fixtures';
@@ -27,18 +30,72 @@ const mockedProps = {
latestQueryId: 'yhMUZCGb',
};
+const fakeApiResult = {
+ count: 4,
+ ids: [692],
+ result: [
+ {
+ changed_on: '2024-03-12T20:01:02.497775',
+ client_id: 'b0ZDzRYzn',
+ database: {
+ database_name: 'examples',
+ id: 1,
+ },
+ end_time: '1710273662496.047852',
+ error_message: null,
+ executed_sql: 'SELECT * from "FCC 2018 Survey"\nLIMIT 1001',
+ id: 692,
+ limit: 1000,
+ limiting_factor: 'DROPDOWN',
+ progress: 100,
+ results_key: null,
+ rows: 443,
+ schema: 'main',
+ select_as_cta: false,
+ sql: 'SELECT * from "FCC 2018 Survey" ',
+ sql_editor_id: '22',
+ start_time: '1710273662445.992920',
+ status: QueryState.Success,
+ tab_name: 'Untitled Query 16',
+ tmp_table_name: null,
+ tracking_url: null,
+ user: {
+ first_name: 'admin',
+ id: 1,
+ last_name: 'user',
+ },
+ },
+ ],
+};
+
const setup = (overrides = {}) => (
<QueryHistory {...mockedProps} {...overrides} />
);
-describe('QueryHistory', () => {
- it('Renders an empty state for query history', () => {
- render(setup(), { useRedux: true, initialState });
+test('Renders an empty state for query history', () => {
+ render(setup(), { useRedux: true, initialState });
+
+ const emptyStateText = screen.getByText(
+ /run a query to display query history/i,
+ );
+
+ expect(emptyStateText).toBeVisible();
+});
- const emptyStateText = screen.getByText(
- /run a query to display query history/i,
+test('fetches the query history when the persistence mode is enabled', async () => {
+ const isFeatureEnabledMock = jest
+ .spyOn(uiCore, 'isFeatureEnabled')
+ .mockImplementation(
+ featureFlag => featureFlag === FeatureFlag.SqllabBackendPersistence,
);
- expect(emptyStateText).toBeVisible();
- });
+ const editorQueryApiRoute = `glob:*/api/v1/query/?q=*`;
+ fetchMock.get(editorQueryApiRoute, fakeApiResult);
+ render(setup(), { useRedux: true, initialState });
+ await waitFor(() =>
+ expect(fetchMock.calls(editorQueryApiRoute).length).toBe(1),
+ );
+ const queryResultText = screen.getByText(fakeApiResult.result[0].rows);
+ expect(queryResultText).toBeInTheDocument();
+ isFeatureEnabledMock.mockClear();
});
diff --git a/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx b/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx
index 311a125d55..e020c3302f 100644
--- a/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx
+++ b/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx
@@ -16,12 +16,23 @@
* specific language governing permissions and limitations
* under the License.
*/
-import React, { useMemo } from 'react';
+import React, { useEffect, useMemo, useState } from 'react';
import { shallowEqual, useSelector } from 'react-redux';
+import { useInView } from 'react-intersection-observer';
+import { omit } from 'lodash';
import { EmptyStateMedium } from 'src/components/EmptyState';
-import { t, styled } from '@superset-ui/core';
+import {
+ t,
+ styled,
+ css,
+ FeatureFlag,
+ isFeatureEnabled,
+} from '@superset-ui/core';
import QueryTable from 'src/SqlLab/components/QueryTable';
import { SqlLabRootState } from 'src/SqlLab/types';
+import { useEditorQueriesQuery } from 'src/hooks/apiResources/queries';
+import { Skeleton } from 'src/components';
+import useEffectEvent from 'src/hooks/useEffectEvent';
interface QueryHistoryProps {
queryEditorId: string | number;
@@ -40,39 +51,92 @@ const StyledEmptyStateWrapper = styled.div`
}
`;
+const getEditorQueries = (
+ queries: SqlLabRootState['sqlLab']['queries'],
+ queryEditorId: string | number,
+) =>
+ Object.values(queries).filter(
+ ({ sqlEditorId }) => String(sqlEditorId) === String(queryEditorId),
+ );
+
const QueryHistory = ({
queryEditorId,
displayLimit,
latestQueryId,
}: QueryHistoryProps) => {
+ const [ref, hasReachedBottom] = useInView({ threshold: 0 });
+ const [pageIndex, setPageIndex] = useState(0);
const queries = useSelector(
({ sqlLab: { queries } }: SqlLabRootState) => queries,
shallowEqual,
);
+ const { data, isLoading, isFetching } = useEditorQueriesQuery(
+ { editorId: `${queryEditorId}`, pageIndex },
+ {
+ skip: !isFeatureEnabled(FeatureFlag.SqllabBackendPersistence),
+ },
+ );
const editorQueries = useMemo(
() =>
- Object.values(queries).filter(
- ({ sqlEditorId }) => String(sqlEditorId) === String(queryEditorId),
- ),
- [queries, queryEditorId],
+ data
+ ? getEditorQueries(
+ omit(
+ queries,
+ data.result.map(({ id }) => id),
+ ),
+ queryEditorId,
+ )
+ .concat(data.result)
+ .reverse()
+ : getEditorQueries(queries, queryEditorId),
+ [queries, data, queryEditorId],
);
+ const loadNext = useEffectEvent(() => {
+ setPageIndex(pageIndex + 1);
+ });
+
+ const loadedDataCount = data?.result.length || 0;
+ const totalCount = data?.count || 0;
+
+ useEffect(() => {
+ if (hasReachedBottom && loadedDataCount < totalCount) {
+ loadNext();
+ }
+ }, [hasReachedBottom, loadNext, loadedDataCount, totalCount]);
+
+ if (!editorQueries.length && isLoading) {
+ return <Skeleton active />;
+ }
+
return editorQueries.length > 0 ? (
- <QueryTable
- columns={[
- 'state',
- 'started',
- 'duration',
- 'progress',
- 'rows',
- 'sql',
- 'results',
- 'actions',
- ]}
- queries={editorQueries}
- displayLimit={displayLimit}
- latestQueryId={latestQueryId}
- />
+ <>
+ <QueryTable
+ columns={[
+ 'state',
+ 'started',
+ 'duration',
+ 'progress',
+ 'rows',
+ 'sql',
+ 'results',
+ 'actions',
+ ]}
+ queries={editorQueries}
+ displayLimit={displayLimit}
+ latestQueryId={latestQueryId}
+ />
+ {data && loadedDataCount < totalCount && (
+ <div
+ ref={ref}
+ css={css`
+ position: relative;
+ top: -150px;
+ `}
+ />
+ )}
+ {isFetching && <Skeleton active />}
+ </>
) : (
<StyledEmptyStateWrapper>
<EmptyStateMedium
diff --git a/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts b/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts
index 1dd3220fcc..6d6e65bad3 100644
--- a/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts
+++ b/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts
@@ -25,7 +25,6 @@ const apiData = {
common: DEFAULT_COMMON_BOOTSTRAP_DATA,
tab_state_ids: [],
databases: [],
- queries: {},
user: {
userId: 1,
username: 'some name',
@@ -220,18 +219,20 @@ describe('getInitialState', () => {
}),
);
+ const latestQuery = {
+ ...runningQuery,
+ id: 'latestPersisted',
+ startDttm: Number(startDttmInStr),
+ endDttm: Number(endDttmInStr),
+ };
const initializedQueries = getInitialState({
- ...apiData,
- queries: {
- backendPersisted: {
- ...runningQuery,
- id: 'backendPersisted',
- startDttm: startDttmInStr,
- endDttm: endDttmInStr,
- },
+ ...apiDataWithTabState,
+ active_tab: {
+ ...apiDataWithTabState.active_tab,
+ latest_query: latestQuery,
},
}).sqlLab.queries;
- expect(initializedQueries.backendPersisted).toEqual(
+ expect(initializedQueries.latestPersisted).toEqual(
expect.objectContaining({
startDttm: Number(startDttmInStr),
endDttm: Number(endDttmInStr),
diff --git a/superset-frontend/src/SqlLab/reducers/getInitialState.ts b/superset-frontend/src/SqlLab/reducers/getInitialState.ts
index 04e7f4eca4..bcdc1f40c3 100644
--- a/superset-frontend/src/SqlLab/reducers/getInitialState.ts
+++ b/superset-frontend/src/SqlLab/reducers/getInitialState.ts
@@ -136,7 +136,12 @@ export default function getInitialState({
});
}
- const queries = { ...queries_ };
+ const queries = {
+ ...queries_,
+ ...(activeTab?.latest_query && {
+ [activeTab.latest_query.id]: activeTab.latest_query,
+ }),
+ };
/**
* If the `SQLLAB_BACKEND_PERSISTENCE` feature flag is off, or if the user
diff --git a/superset-frontend/src/hooks/apiResources/queries.test.ts b/superset-frontend/src/hooks/apiResources/queries.test.ts
new file mode 100644
index 0000000000..14d1f9ceac
--- /dev/null
+++ b/superset-frontend/src/hooks/apiResources/queries.test.ts
@@ -0,0 +1,154 @@
+/**
+ * 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.
+ */
+import rison from 'rison';
+import fetchMock from 'fetch-mock';
+import { act, renderHook } from '@testing-library/react-hooks';
+import {
+ createWrapper,
+ defaultStore as store,
+} from 'spec/helpers/testing-library';
+import { QueryState } from '@superset-ui/core';
+import { api } from 'src/hooks/apiResources/queryApi';
+import { mapQueryResponse, useEditorQueriesQuery } from './queries';
+
+const fakeApiResult = {
+ count: 4,
+ ids: [692],
+ result: [
+ {
+ changed_on: '2024-03-12T20:01:02.497775',
+ client_id: 'b0ZDzRYzn',
+ database: {
+ database_name: 'examples',
+ id: 1,
+ },
+ end_time: '1710273662496.047852',
+ error_message: null,
+ executed_sql: 'SELECT * from "FCC 2018 Survey"\nLIMIT 1001',
+ id: 692,
+ limit: 1000,
+ limiting_factor: 'DROPDOWN',
+ progress: 100,
+ results_key: null,
+ rows: 1000,
+ schema: 'main',
+ select_as_cta: false,
+ sql: 'SELECT * from "FCC 2018 Survey" ',
+ sql_editor_id: '22',
+ start_time: '1710273662445.992920',
+ status: QueryState.Success,
+ tab_name: 'Untitled Query 16',
+ tmp_table_name: null,
+ tracking_url: null,
+ user: {
+ first_name: 'admin',
+ id: 1,
+ last_name: 'user',
+ },
+ },
+ ],
+};
+
+afterEach(() => {
+ fetchMock.reset();
+ act(() => {
+ store.dispatch(api.util.resetApiState());
+ });
+});
+
+test('returns api response mapping camelCase keys', async () => {
+ const editorId = '23';
+ const editorQueryApiRoute = `glob:*/api/v1/query/?q=*`;
+ fetchMock.get(editorQueryApiRoute, fakeApiResult);
+ const { result, waitFor } = renderHook(
+ () => useEditorQueriesQuery({ editorId }),
+ {
+ wrapper: createWrapper({
+ useRedux: true,
+ store,
+ }),
+ },
+ );
+ await waitFor(() =>
+ expect(fetchMock.calls(editorQueryApiRoute).length).toBe(1),
+ );
+ const expectedResult = {
+ ...fakeApiResult,
+ result: fakeApiResult.result.map(mapQueryResponse),
+ };
+ expect(
+ rison.decode(fetchMock.calls(editorQueryApiRoute)[0][0].split('?q=')[1]),
+ ).toEqual(
+ expect.objectContaining({
+ order_column: 'start_time',
+ order_direction: 'desc',
+ page: 0,
+ page_size: 25,
+ filters: [
+ {
+ col: 'sql_editor_id',
+ opr: 'eq',
+ value: expect.stringContaining(editorId),
+ },
+ ],
+ }),
+ );
+ expect(result.current.data).toEqual(expectedResult);
+});
+
+test('merges paginated results', async () => {
+ const editorId = '23';
+ const editorQueryApiRoute = `glob:*/api/v1/query/?q=*`;
+ fetchMock.get(editorQueryApiRoute, fakeApiResult);
+ const { waitFor } = renderHook(() => useEditorQueriesQuery({ editorId }), {
+ wrapper: createWrapper({
+ useRedux: true,
+ store,
+ }),
+ });
+ await waitFor(() =>
+ expect(fetchMock.calls(editorQueryApiRoute).length).toBe(1),
+ );
+ const { result: paginatedResult } = renderHook(
+ () => useEditorQueriesQuery({ editorId, pageIndex: 1 }),
+ {
+ wrapper: createWrapper({
+ useRedux: true,
+ store,
+ }),
+ },
+ );
+ await waitFor(() =>
+ expect(fetchMock.calls(editorQueryApiRoute).length).toBe(2),
+ );
+ expect(
+ rison.decode(fetchMock.calls(editorQueryApiRoute)[1][0].split('?q=')[1]),
+ ).toEqual(
+ expect.objectContaining({
+ page: 1,
+ }),
+ );
+ expect(paginatedResult.current.data).toEqual({
+ ...fakeApiResult,
+ result: [
+ ...fakeApiResult.result.map(mapQueryResponse),
+ ...fakeApiResult.result.map(mapQueryResponse),
+ ],
+ });
+});
diff --git a/superset-frontend/src/hooks/apiResources/queries.ts b/superset-frontend/src/hooks/apiResources/queries.ts
new file mode 100644
index 0000000000..ad7ed8eb59
--- /dev/null
+++ b/superset-frontend/src/hooks/apiResources/queries.ts
@@ -0,0 +1,176 @@
+/**
+ * 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.
+ */
+import type { Query, QueryResponse } from '@superset-ui/core';
+import type { JsonResponse } from './queryApi';
+import { api } from './queryApi';
+
+export type QueryResult = {
+ count: number;
+ ids: Query['id'][];
+ result: QueryResponse[];
+};
+
+export type EditorQueriesParams = {
+ editorId: string;
+ pageIndex?: number;
+ pageSize?: number;
+};
+
+interface ResponseResult {
+ id: Query['queryId'];
+ client_id: Query['id'];
+ database: {
+ id: Query['dbId'];
+ database_name: string;
+ };
+ executed_sql: Query['executedSql'];
+ error_message: Query['errorMessage'];
+ limit: Query['queryLimit'];
+ limiting_factor: Query['limitingFactor'];
+ progress: Query['progress'];
+ rows: Query['rows'];
+ select_as_cta: Query['ctas'];
+ schema: Query['schema'];
+ sql: Query['sql'];
+ sql_editor_id: Query['sqlEditorId'];
+ status: Query['state'];
+ tab_name: Query['tab'];
+ user: {
+ id: Query['userId'];
+ };
+ start_time: string;
+ end_time: string;
+ tmp_table_name: Query['tempTable'] | null;
+ tracking_url: Query['trackingUrl'];
+ results_key: Query['resultsKey'];
+}
+
+export const mapQueryResponse = (
+ query: ResponseResult,
+): Omit<
+ Query,
+ | 'tempSchema'
+ | 'started'
+ | 'time'
+ | 'duration'
+ | 'templateParams'
+ | 'querylink'
+ | 'output'
+ | 'actions'
+ | 'type'
+ | 'columns'
+> => ({
+ queryId: query.id,
+ id: query.client_id,
+ dbId: query.database.id,
+ db: query.database,
+ executedSql: query.executed_sql,
+ errorMessage: query.error_message,
+ queryLimit: query.limit,
+ ctas: query.select_as_cta,
+ limitingFactor: query.limiting_factor,
+ progress: query.progress,
+ rows: query.rows,
+ schema: query.schema,
+ sql: query.sql,
+ sqlEditorId: query.sql_editor_id,
+ state: query.status,
+ tab: query.tab_name,
+ startDttm: Number(query.start_time),
+ endDttm: Number(query.end_time),
+ tempTable: query.tmp_table_name || '',
+ trackingUrl: query.tracking_url,
+ resultsKey: query.results_key,
+ userId: query.user.id,
+ cached: false,
+ extra: {
+ progress: null,
+ },
+ isDataPreview: false,
+ user: query.user,
+});
+
+const queryHistoryApi = api.injectEndpoints({
+ endpoints: builder => ({
+ editorQueries: builder.query<QueryResult, EditorQueriesParams>({
+ providesTags: ['EditorQueries'],
+ query: ({ editorId, pageIndex = 0, pageSize = 25 }) => ({
+ method: 'GET',
+ endpoint: `/api/v1/query/`,
+ urlParams: {
+ keys: ['none'],
+ columns: [
+ 'id',
+ 'client_id',
+ 'changed_on',
+ 'database.id',
+ 'database.database_name',
+ 'executed_sql',
+ 'error_message',
+ 'limit',
+ 'limiting_factor',
+ 'progress',
+ 'rows',
+ 'select_as_cta',
+ 'schema',
+ 'sql',
+ 'sql_editor_id',
+ 'status',
+ 'tab_name',
+ 'user.first_name',
+ 'user.id',
+ 'user.last_name',
+ 'start_time',
+ 'end_time',
+ 'tmp_table_name',
+ 'tmp_schema_name',
+ 'tracking_url',
+ 'results_key',
+ ],
+ order_column: 'start_time',
+ order_direction: 'desc',
+ page: pageIndex,
+ page_size: pageSize,
+ filters: [
+ {
+ col: 'sql_editor_id',
+ opr: 'eq',
+ value: editorId,
+ },
+ ],
+ },
+ headers: { 'Content-Type': 'application/json' },
+ transformResponse: ({ json }: JsonResponse) => ({
+ ...json,
+ result: json.result.map(mapQueryResponse),
+ }),
+ }),
+ serializeQueryArgs: ({ queryArgs: { editorId } }) => ({ editorId }),
+ // Refetch when the page arg changes
+ forceRefetch({ currentArg, previousArg }) {
+ return currentArg !== previousArg;
+ },
+ merge: (currentCache, newItems) => {
+ currentCache.result.push(...newItems.result);
+ },
+ }),
+ }),
+});
+
+export const { useEditorQueriesQuery } = queryHistoryApi;
diff --git a/superset-frontend/src/hooks/apiResources/queryApi.ts b/superset-frontend/src/hooks/apiResources/queryApi.ts
index 4099422b54..ae25f3cc56 100644
--- a/superset-frontend/src/hooks/apiResources/queryApi.ts
+++ b/superset-frontend/src/hooks/apiResources/queryApi.ts
@@ -37,7 +37,7 @@ export const supersetClientQuery: BaseQueryFn<
endpoint: string;
parseMethod?: ParseMethod;
transformResponse?: (response: SupersetClientResponse) => JsonValue;
- urlParams?: Record<string, number | string | undefined | boolean>;
+ urlParams?: Record<string, number | string | undefined | boolean | object>;
},
JsonValue,
ClientErrorObject
@@ -80,6 +80,7 @@ export const api = createApi({
'QueryValidations',
'TableMetadatas',
'SqlLabInitialState',
+ 'EditorQueries',
],
endpoints: () => ({}),
baseQuery: supersetClientQuery,
diff --git a/superset/queries/api.py b/superset/queries/api.py
index 9568542f76..0695946fe0 100644
--- a/superset/queries/api.py
+++ b/superset/queries/api.py
@@ -71,11 +71,19 @@ class QueryRestApi(BaseSupersetModelRestApi):
list_columns = [
"id",
"changed_on",
+ "client_id",
+ "database.id",
"database.database_name",
"executed_sql",
+ "error_message",
+ "limit",
+ "limiting_factor",
+ "progress",
"rows",
"schema",
+ "select_as_cta",
"sql",
+ "sql_editor_id",
"sql_tables",
"status",
"tab_name",
@@ -86,6 +94,7 @@ class QueryRestApi(BaseSupersetModelRestApi):
"end_time",
"tmp_table_name",
"tracking_url",
+ "results_key",
]
show_columns = [
"id",
@@ -143,7 +152,15 @@ class QueryRestApi(BaseSupersetModelRestApi):
"user": RelatedFieldFilter("first_name", FilterRelatedOwners),
}
- search_columns = ["changed_on", "database", "sql", "status", "user", "start_time"]
+ search_columns = [
+ "changed_on",
+ "database",
+ "sql",
+ "status",
+ "user",
+ "start_time",
+ "sql_editor_id",
+ ]
allowed_rel_fields = {"database", "user"}
allowed_distinct_fields = {"status"}
diff --git a/superset/sqllab/utils.py b/superset/sqllab/utils.py
index 989bd19cc3..ca331d1e34 100644
--- a/superset/sqllab/utils.py
+++ b/superset/sqllab/utils.py
@@ -23,7 +23,7 @@ import pyarrow as pa
from superset import db, is_feature_enabled
from superset.common.db_query_status import QueryStatus
from superset.daos.database import DatabaseDAO
-from superset.models.sql_lab import Query, TabState
+from superset.models.sql_lab import TabState
DATABASE_KEYS = [
"allow_file_upload",
@@ -87,7 +87,6 @@ def bootstrap_sqllab_data(user_id: int | None) -> dict[str, Any]:
k: v for k, v in database.to_json().items() if k in DATABASE_KEYS
}
databases[database.id]["backend"] = database.backend
- queries: dict[str, Any] = {}
# These are unnecessary if sqllab backend persistence is disabled
if is_feature_enabled("SQLLAB_BACKEND_PERSISTENCE"):
@@ -97,7 +96,6 @@ def bootstrap_sqllab_data(user_id: int | None) -> dict[str, Any]:
.filter_by(user_id=user_id)
.all()
)
- tab_state_ids = [str(tab_state[0]) for tab_state in tabs_state]
# return first active tab, or fallback to another one if no tab is active
active_tab = (
db.session.query(TabState)
@@ -105,20 +103,9 @@ def bootstrap_sqllab_data(user_id: int | None) -> dict[str, Any]:
.order_by(TabState.active.desc())
.first()
)
- # return all user queries associated with existing SQL editors
- user_queries = (
- db.session.query(Query)
- .filter_by(user_id=user_id)
- .filter(Query.sql_editor_id.in_(tab_state_ids))
- .all()
- )
- queries = {
- query.client_id: dict(query.to_dict().items()) for query in user_queries
- }
return {
"tab_state_ids": tabs_state,
"active_tab": active_tab.to_dict() if active_tab else None,
"databases": databases,
- "queries": queries,
}
diff --git a/tests/integration_tests/sql_lab/api_tests.py b/tests/integration_tests/sql_lab/api_tests.py
index 597f961346..25158d25c8 100644
--- a/tests/integration_tests/sql_lab/api_tests.py
+++ b/tests/integration_tests/sql_lab/api_tests.py
@@ -57,7 +57,6 @@ class TestSqlLabApi(SupersetTestCase):
data = json.loads(resp.data.decode("utf-8"))
result = data.get("result")
assert result["active_tab"] == None
- assert result["queries"] == {}
assert result["tab_state_ids"] == []
self.assertEqual(len(result["databases"]), 0)
@@ -87,7 +86,6 @@ class TestSqlLabApi(SupersetTestCase):
data = json.loads(resp.data.decode("utf-8"))
result = data.get("result")
assert result["active_tab"] == None
- assert result["queries"] == {}
assert result["tab_state_ids"] == []
@mock.patch.dict(
@@ -95,7 +93,7 @@ class TestSqlLabApi(SupersetTestCase):
{"SQLLAB_BACKEND_PERSISTENCE": True},
clear=True,
)
- def test_get_from_bootstrap_data_with_queries(self):
+ def test_get_from_bootstrap_data_with_latest_query(self):
username = "admin"
self.login(username)
@@ -115,27 +113,11 @@ class TestSqlLabApi(SupersetTestCase):
resp = self.get_json_resp("/tabstateview/", data=data)
tab_state_id = resp["id"]
- # run a query in the created tab
- self.run_sql(
- "SELECT name FROM birth_names",
- "client_id_1",
- username=username,
- raise_on_error=True,
- sql_editor_id=str(tab_state_id),
- )
- # run an orphan query (no tab)
- self.run_sql(
- "SELECT name FROM birth_names",
- "client_id_2",
- username=username,
- raise_on_error=True,
- )
-
# we should have only 1 query returned, since the second one is not
# associated with any tabs
resp = self.get_json_resp("/api/v1/sqllab/")
result = resp["result"]
- self.assertEqual(len(result["queries"]), 1)
+ self.assertEqual(result["active_tab"]["id"], tab_state_id)
@mock.patch.dict(
"superset.extensions.feature_flag_manager._feature_flags",
diff --git a/tests/integration_tests/sqllab_tests.py b/tests/integration_tests/sqllab_tests.py
index 0dc4e26aca..30b8401cc6 100644
--- a/tests/integration_tests/sqllab_tests.py
+++ b/tests/integration_tests/sqllab_tests.py
@@ -461,6 +461,63 @@ class TestSqlLab(SupersetTestCase):
db.session.commit()
+ def test_query_api_can_access_sql_editor_id_associated_queries(self) -> None:
+ """
+ Test query api with sql_editor_id filter to
+ gamma and make sure sql editor associated queries show up.
+ """
+ username = "gamma_sqllab"
+ self.login("gamma_sqllab")
+
+ # create a tab
+ data = {
+ "queryEditor": json.dumps(
+ {
+ "title": "Untitled Query 1",
+ "dbId": 1,
+ "schema": None,
+ "autorun": False,
+ "sql": "SELECT ...",
+ "queryLimit": 1000,
+ }
+ )
+ }
+ resp = self.get_json_resp("/tabstateview/", data=data)
+ tab_state_id = resp["id"]
+ # run a query in the created tab
+ self.run_sql(
+ "SELECT 1",
+ "client_id_1",
+ username=username,
+ raise_on_error=True,
+ sql_editor_id=str(tab_state_id),
+ )
+ self.run_sql(
+ "SELECT 2",
+ "client_id_2",
+ username=username,
+ raise_on_error=True,
+ sql_editor_id=str(tab_state_id),
+ )
+ # run an orphan query (no tab)
+ self.run_sql(
+ "SELECT 3",
+ "client_id_3",
+ username=username,
+ raise_on_error=True,
+ )
+
+ arguments = {
+ "filters": [
+ {"col": "sql_editor_id", "opr": "eq", "value": str(tab_state_id)}
+ ]
+ }
+ url = f"/api/v1/query/?q={prison.dumps(arguments)}"
+ self.assertEqual(
+ {"SELECT 1", "SELECT 2"},
+ {r.get("sql") for r in self.get_json_resp(url)["result"]},
+ )
+
def test_query_admin_can_access_all_queries(self) -> None:
"""
Test query api with all_query_access perm added to
(superset) 03/06: fix(explore): Allow only saved metrics and columns (#27539)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 4.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit d4314a92d8985c9cbdd1f7a146834b1893cbce6d
Author: JUST.in DO IT <ju...@airbnb.com>
AuthorDate: Mon Mar 18 09:51:02 2024 -0700
fix(explore): Allow only saved metrics and columns (#27539)
---
.../DndFilterSelect.test.tsx | 221 +++++++++++++++++++--
.../DndColumnSelectControl/DndFilterSelect.tsx | 41 +++-
.../DndMetricSelect.test.tsx | 121 +++++++++++
.../DndColumnSelectControl/DndMetricSelect.tsx | 32 ++-
superset-frontend/src/explore/types.ts | 1 +
5 files changed, 400 insertions(+), 16 deletions(-)
diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.test.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.test.tsx
index f47c3a0101..6be82472d9 100644
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.test.tsx
+++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.test.tsx
@@ -18,7 +18,6 @@
*/
import React from 'react';
import thunk from 'redux-thunk';
-import { Provider } from 'react-redux';
import configureStore from 'redux-mock-store';
import {
@@ -29,7 +28,13 @@ import {
import { ColumnMeta } from '@superset-ui/chart-controls';
import { TimeseriesDefaultFormData } from '@superset-ui/plugin-chart-echarts';
-import { render, screen } from 'spec/helpers/testing-library';
+import {
+ fireEvent,
+ render,
+ screen,
+ within,
+} from 'spec/helpers/testing-library';
+import type { AsyncAceEditorProps } from 'src/components/AsyncAceEditor';
import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetric';
import AdhocFilter from 'src/explore/components/controls/FilterControl/AdhocFilter';
import {
@@ -38,13 +43,21 @@ import {
} from 'src/explore/components/controls/DndColumnSelectControl/DndFilterSelect';
import { PLACEHOLDER_DATASOURCE } from 'src/dashboard/constants';
import { ExpressionTypes } from '../FilterControl/types';
+import { Datasource } from '../../../types';
+import { DndItemType } from '../../DndItemType';
+import DatasourcePanelDragOption from '../../DatasourcePanel/DatasourcePanelDragOption';
+
+jest.mock('src/components/AsyncAceEditor', () => ({
+ SQLEditor: (props: AsyncAceEditorProps) => (
+ <div data-test="react-ace">{props.value}</div>
+ ),
+}));
-const defaultProps: DndFilterSelectProps = {
+const defaultProps: Omit<DndFilterSelectProps, 'datasource'> = {
type: 'DndFilterSelect',
name: 'Filter',
value: [],
columns: [],
- datasource: PLACEHOLDER_DATASOURCE,
formData: null,
savedMetrics: [],
selectedMetrics: [],
@@ -64,25 +77,26 @@ function setup({
value = undefined,
formData = baseFormData,
columns = [],
+ datasource = PLACEHOLDER_DATASOURCE,
}: {
value?: AdhocFilter;
formData?: QueryFormData;
columns?: ColumnMeta[];
+ datasource?: Datasource;
} = {}) {
return (
- <Provider store={store}>
- <DndFilterSelect
- {...defaultProps}
- value={ensureIsArray(value)}
- formData={formData}
- columns={columns}
- />
- </Provider>
+ <DndFilterSelect
+ {...defaultProps}
+ datasource={datasource}
+ value={ensureIsArray(value)}
+ formData={formData}
+ columns={columns}
+ />
);
}
test('renders with default props', async () => {
- render(setup(), { useDnd: true });
+ render(setup(), { useDnd: true, store });
expect(
await screen.findByText('Drop columns/metrics here or click'),
).toBeInTheDocument();
@@ -95,6 +109,7 @@ test('renders with value', async () => {
});
render(setup({ value }), {
useDnd: true,
+ store,
});
expect(await screen.findByText('COUNT(*)')).toBeInTheDocument();
});
@@ -110,6 +125,7 @@ test('renders options with saved metric', async () => {
}),
{
useDnd: true,
+ store,
},
);
expect(
@@ -131,6 +147,7 @@ test('renders options with column', async () => {
}),
{
useDnd: true,
+ store,
},
);
expect(
@@ -153,9 +170,187 @@ test('renders options with adhoc metric', async () => {
}),
{
useDnd: true,
+ store,
},
);
expect(
await screen.findByText('Drop columns/metrics here or click'),
).toBeInTheDocument();
});
+
+test('cannot drop a column that is not part of the simple column selection', () => {
+ const adhocMetric = new AdhocMetric({
+ expression: 'AVG(birth_names.num)',
+ metric_name: 'avg__num',
+ });
+ const { getByTestId, getAllByTestId } = render(
+ <>
+ <DatasourcePanelDragOption
+ value={{ column_name: 'order_date' }}
+ type={DndItemType.Column}
+ />
+ <DatasourcePanelDragOption
+ value={{ column_name: 'address_line1' }}
+ type={DndItemType.Column}
+ />
+ <DatasourcePanelDragOption
+ value={{ metric_name: 'metric_a', expression: 'AGG(metric_a)' }}
+ type={DndItemType.Metric}
+ />
+ {setup({
+ formData: {
+ ...baseFormData,
+ ...TimeseriesDefaultFormData,
+ metrics: [adhocMetric],
+ },
+ columns: [{ column_name: 'order_date' }],
+ })}
+ </>,
+ {
+ useDnd: true,
+ store,
+ },
+ );
+
+ const selections = getAllByTestId('DatasourcePanelDragOption');
+ const acceptableColumn = selections[0];
+ const unacceptableColumn = selections[1];
+ const metricType = selections[2];
+ const currentMetric = getByTestId('dnd-labels-container');
+
+ fireEvent.dragStart(unacceptableColumn);
+ fireEvent.dragOver(currentMetric);
+ fireEvent.drop(currentMetric);
+
+ expect(screen.queryByTestId('filter-edit-popover')).not.toBeInTheDocument();
+
+ fireEvent.dragStart(acceptableColumn);
+ fireEvent.dragOver(currentMetric);
+ fireEvent.drop(currentMetric);
+
+ const filterConfigPopup = screen.getByTestId('filter-edit-popover');
+ expect(within(filterConfigPopup).getByText('order_date')).toBeInTheDocument();
+
+ fireEvent.keyDown(filterConfigPopup, {
+ key: 'Escape',
+ code: 'Escape',
+ keyCode: 27,
+ charCode: 27,
+ });
+ expect(screen.queryByTestId('filter-edit-popover')).not.toBeInTheDocument();
+
+ fireEvent.dragStart(metricType);
+ fireEvent.dragOver(currentMetric);
+ fireEvent.drop(currentMetric);
+
+ expect(
+ within(screen.getByTestId('filter-edit-popover')).getByTestId('react-ace'),
+ ).toHaveTextContent('AGG(metric_a)');
+});
+
+describe('when disallow_adhoc_metrics is set', () => {
+ test('can drop a column type from the simple column selection', () => {
+ const adhocMetric = new AdhocMetric({
+ expression: 'AVG(birth_names.num)',
+ metric_name: 'avg__num',
+ });
+ const { getByTestId } = render(
+ <>
+ <DatasourcePanelDragOption
+ value={{ column_name: 'column_b' }}
+ type={DndItemType.Column}
+ />
+ {setup({
+ formData: {
+ ...baseFormData,
+ ...TimeseriesDefaultFormData,
+ metrics: [adhocMetric],
+ },
+ datasource: {
+ ...PLACEHOLDER_DATASOURCE,
+ extra: '{ "disallow_adhoc_metrics": true }',
+ },
+ columns: [{ column_name: 'column_a' }, { column_name: 'column_b' }],
+ })}
+ </>,
+ {
+ useDnd: true,
+ store,
+ },
+ );
+
+ const acceptableColumn = getByTestId('DatasourcePanelDragOption');
+ const currentMetric = getByTestId('dnd-labels-container');
+
+ fireEvent.dragStart(acceptableColumn);
+ fireEvent.dragOver(currentMetric);
+ fireEvent.drop(currentMetric);
+
+ const filterConfigPopup = screen.getByTestId('filter-edit-popover');
+ expect(within(filterConfigPopup).getByText('column_b')).toBeInTheDocument();
+ });
+
+ test('cannot drop any other types of selections apart from simple column selection', () => {
+ const adhocMetric = new AdhocMetric({
+ expression: 'AVG(birth_names.num)',
+ metric_name: 'avg__num',
+ });
+ const { getByTestId, getAllByTestId } = render(
+ <>
+ <DatasourcePanelDragOption
+ value={{ column_name: 'column_c' }}
+ type={DndItemType.Column}
+ />
+ <DatasourcePanelDragOption
+ value={{ metric_name: 'metric_a' }}
+ type={DndItemType.Metric}
+ />
+ <DatasourcePanelDragOption
+ value={{ metric_name: 'avg__num' }}
+ type={DndItemType.AdhocMetricOption}
+ />
+ {setup({
+ formData: {
+ ...baseFormData,
+ ...TimeseriesDefaultFormData,
+ metrics: [adhocMetric],
+ },
+ datasource: {
+ ...PLACEHOLDER_DATASOURCE,
+ extra: '{ "disallow_adhoc_metrics": true }',
+ },
+ columns: [{ column_name: 'column_a' }, { column_name: 'column_c' }],
+ })}
+ </>,
+ {
+ useDnd: true,
+ store,
+ },
+ );
+
+ const selections = getAllByTestId('DatasourcePanelDragOption');
+ const acceptableColumn = selections[0];
+ const unacceptableMetric = selections[1];
+ const unacceptableType = selections[2];
+ const currentMetric = getByTestId('dnd-labels-container');
+
+ fireEvent.dragStart(unacceptableMetric);
+ fireEvent.dragOver(currentMetric);
+ fireEvent.drop(currentMetric);
+
+ expect(screen.queryByTestId('filter-edit-popover')).not.toBeInTheDocument();
+
+ fireEvent.dragStart(unacceptableType);
+ fireEvent.dragOver(currentMetric);
+ fireEvent.drop(currentMetric);
+
+ expect(screen.queryByTestId('filter-edit-popover')).not.toBeInTheDocument();
+
+ fireEvent.dragStart(acceptableColumn);
+ fireEvent.dragOver(currentMetric);
+ fireEvent.drop(currentMetric);
+
+ const filterConfigPopup = screen.getByTestId('filter-edit-popover');
+ expect(within(filterConfigPopup).getByText('column_c')).toBeInTheDocument();
+ });
+});
diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx
index 5295bd6dae..955c480724 100644
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx
+++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx
@@ -85,6 +85,16 @@ const DndFilterSelect = (props: DndFilterSelectProps) => {
canDelete,
} = props;
+ const extra = useMemo<{ disallow_adhoc_metrics?: boolean }>(() => {
+ let extra = {};
+ if (datasource?.extra) {
+ try {
+ extra = JSON.parse(datasource.extra);
+ } catch {} // eslint-disable-line no-empty
+ }
+ return extra;
+ }, [datasource?.extra]);
+
const propsValues = Array.from(props.value ?? []);
const [values, setValues] = useState(
propsValues.map((filter: OptionValueType) =>
@@ -149,6 +159,17 @@ const DndFilterSelect = (props: DndFilterSelectProps) => {
optionsForSelect(props.columns, props.formData),
);
+ const availableColumnSet = useMemo(
+ () =>
+ new Set(
+ options.map(
+ ({ column_name, filterOptionName }) =>
+ column_name ?? filterOptionName,
+ ),
+ ),
+ [options],
+ );
+
useEffect(() => {
if (datasource && datasource.type === 'table') {
const dbId = datasource.database?.id;
@@ -382,7 +403,25 @@ const DndFilterSelect = (props: DndFilterSelectProps) => {
return new AdhocFilter(config);
}, [droppedItem]);
- const canDrop = useCallback(() => true, []);
+ const canDrop = useCallback(
+ (item: DatasourcePanelDndItem) => {
+ if (
+ extra.disallow_adhoc_metrics &&
+ (item.type !== DndItemType.Column ||
+ !availableColumnSet.has((item.value as ColumnMeta).column_name))
+ ) {
+ return false;
+ }
+
+ if (item.type === DndItemType.Column) {
+ const columnName = (item.value as ColumnMeta).column_name;
+ return availableColumnSet.has(columnName);
+ }
+ return true;
+ },
+ [availableColumnSet, extra],
+ );
+
const handleDrop = useCallback(
(item: DatasourcePanelDndItem) => {
setDroppedItem(item.value);
diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx
index 30be2cebb0..9d6a7423f0 100644
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx
+++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx
@@ -28,6 +28,8 @@ import {
import { DndMetricSelect } from 'src/explore/components/controls/DndColumnSelectControl/DndMetricSelect';
import { AGGREGATES } from 'src/explore/constants';
import { EXPRESSION_TYPES } from '../MetricControl/AdhocMetric';
+import DatasourcePanelDragOption from '../../DatasourcePanel/DatasourcePanelDragOption';
+import { DndItemType } from '../../DndItemType';
const defaultProps = {
savedMetrics: [
@@ -307,6 +309,125 @@ test('can drag metrics', async () => {
expect(within(lastMetric).getByText('metric_a')).toBeVisible();
});
+test('cannot drop a duplicated item', () => {
+ const metricValues = ['metric_a'];
+ const { getByTestId } = render(
+ <>
+ <DatasourcePanelDragOption
+ value={{ metric_name: 'metric_a' }}
+ type={DndItemType.Metric}
+ />
+ <DndMetricSelect {...defaultProps} value={metricValues} multi />
+ </>,
+ {
+ useDnd: true,
+ },
+ );
+
+ const acceptableMetric = getByTestId('DatasourcePanelDragOption');
+ const currentMetric = getByTestId('dnd-labels-container');
+
+ const currentMetricSelection = currentMetric.children.length;
+
+ fireEvent.dragStart(acceptableMetric);
+ fireEvent.dragOver(currentMetric);
+ fireEvent.drop(currentMetric);
+
+ expect(currentMetric.children).toHaveLength(currentMetricSelection);
+ expect(currentMetric).toHaveTextContent('metric_a');
+});
+
+test('can drop a saved metric when disallow_adhoc_metrics', () => {
+ const metricValues = ['metric_b'];
+ const { getByTestId } = render(
+ <>
+ <DatasourcePanelDragOption
+ value={{ metric_name: 'metric_a' }}
+ type={DndItemType.Metric}
+ />
+ <DndMetricSelect
+ {...defaultProps}
+ value={metricValues}
+ multi
+ datasource={{ extra: '{ "disallow_adhoc_metrics": true }' }}
+ />
+ </>,
+ {
+ useDnd: true,
+ },
+ );
+
+ const acceptableMetric = getByTestId('DatasourcePanelDragOption');
+ const currentMetric = getByTestId('dnd-labels-container');
+
+ const currentMetricSelection = currentMetric.children.length;
+
+ fireEvent.dragStart(acceptableMetric);
+ fireEvent.dragOver(currentMetric);
+ fireEvent.drop(currentMetric);
+
+ expect(currentMetric.children).toHaveLength(currentMetricSelection + 1);
+ expect(currentMetric.children[1]).toHaveTextContent('metric_a');
+});
+
+test('cannot drop non-saved metrics when disallow_adhoc_metrics', () => {
+ const metricValues = ['metric_b'];
+ const { getByTestId, getAllByTestId } = render(
+ <>
+ <DatasourcePanelDragOption
+ value={{ metric_name: 'metric_a' }}
+ type={DndItemType.Metric}
+ />
+ <DatasourcePanelDragOption
+ value={{ metric_name: 'metric_c' }}
+ type={DndItemType.Metric}
+ />
+ <DatasourcePanelDragOption
+ value={{ column_name: 'column_1' }}
+ type={DndItemType.Column}
+ />
+ <DndMetricSelect
+ {...defaultProps}
+ value={metricValues}
+ multi
+ datasource={{ extra: '{ "disallow_adhoc_metrics": true }' }}
+ />
+ </>,
+ {
+ useDnd: true,
+ },
+ );
+
+ const selections = getAllByTestId('DatasourcePanelDragOption');
+ const acceptableMetric = selections[0];
+ const unacceptableMetric = selections[1];
+ const unacceptableType = selections[2];
+ const currentMetric = getByTestId('dnd-labels-container');
+
+ const currentMetricSelection = currentMetric.children.length;
+
+ fireEvent.dragStart(unacceptableMetric);
+ fireEvent.dragOver(currentMetric);
+ fireEvent.drop(currentMetric);
+
+ expect(currentMetric.children).toHaveLength(currentMetricSelection);
+ expect(currentMetric).not.toHaveTextContent('metric_c');
+
+ fireEvent.dragStart(unacceptableType);
+ fireEvent.dragOver(currentMetric);
+ fireEvent.drop(currentMetric);
+
+ expect(currentMetric.children).toHaveLength(currentMetricSelection);
+ expect(currentMetric).not.toHaveTextContent('column_1');
+
+ fireEvent.dragStart(acceptableMetric);
+ fireEvent.dragOver(currentMetric);
+ fireEvent.drop(currentMetric);
+
+ expect(currentMetric.children).toHaveLength(currentMetricSelection + 1);
+ expect(currentMetric).toHaveTextContent('metric_a');
+});
+
test('title changes on custom SQL text change', async () => {
let metricValues = [adhocMetricA, 'metric_b'];
const onChange = (val: any[]) => {
diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx
index 8f489773e8..2c98ea4c48 100644
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx
+++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx
@@ -105,7 +105,27 @@ const getOptionsForSavedMetrics = (
type ValueType = Metric | AdhocMetric | QueryFormMetric;
const DndMetricSelect = (props: any) => {
- const { onChange, multi } = props;
+ const { onChange, multi, datasource, savedMetrics } = props;
+
+ const extra = useMemo<{ disallow_adhoc_metrics?: boolean }>(() => {
+ let extra = {};
+ if (datasource?.extra) {
+ try {
+ extra = JSON.parse(datasource.extra);
+ } catch {} // eslint-disable-line no-empty
+ }
+ return extra;
+ }, [datasource?.extra]);
+
+ const savedMetricSet = useMemo(
+ () =>
+ new Set(
+ (savedMetrics as savedMetricType[]).map(
+ ({ metric_name }) => metric_name,
+ ),
+ ),
+ [savedMetrics],
+ );
const handleChange = useCallback(
opts => {
@@ -148,11 +168,19 @@ const DndMetricSelect = (props: any) => {
const canDrop = useCallback(
(item: DatasourcePanelDndItem) => {
+ if (
+ extra.disallow_adhoc_metrics &&
+ (item.type !== DndItemType.Metric ||
+ !savedMetricSet.has(item.value.metric_name))
+ ) {
+ return false;
+ }
+
const isMetricAlreadyInValues =
item.type === 'metric' ? value.includes(item.value.metric_name) : false;
return !isMetricAlreadyInValues;
},
- [value],
+ [value, extra, savedMetricSet],
);
const onNewMetric = useCallback(
diff --git a/superset-frontend/src/explore/types.ts b/superset-frontend/src/explore/types.ts
index 85240c919d..ee249e0fc3 100644
--- a/superset-frontend/src/explore/types.ts
+++ b/superset-frontend/src/explore/types.ts
@@ -68,6 +68,7 @@ export type Datasource = Dataset & {
datasource?: string;
schema?: string;
is_sqllab_view?: boolean;
+ extra?: string;
};
export interface ExplorePageInitialData {
(superset) 01/06: fix(postprocessing): resample with holes (#27487)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 4.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 1016fd92f665919ae27d3f9dfd143ffb235a9489
Author: Ville Brofeldt <33...@users.noreply.github.com>
AuthorDate: Thu Mar 14 12:02:01 2024 -0700
fix(postprocessing): resample with holes (#27487)
(cherry picked from commit 7f19d296b16d8463931b42c8258600b210b56475)
---
superset/utils/pandas_postprocessing/resample.py | 5 +-
.../pandas_postprocessing/test_resample.py | 54 +++++++++++++++++++++-
2 files changed, 57 insertions(+), 2 deletions(-)
diff --git a/superset/utils/pandas_postprocessing/resample.py b/superset/utils/pandas_postprocessing/resample.py
index a82d7031e9..a689895bd6 100644
--- a/superset/utils/pandas_postprocessing/resample.py
+++ b/superset/utils/pandas_postprocessing/resample.py
@@ -43,13 +43,16 @@ def resample(
raise InvalidPostProcessingError(_("Resample operation requires DatetimeIndex"))
if method not in RESAMPLE_METHOD:
raise InvalidPostProcessingError(
- _("Resample method should in ") + ", ".join(RESAMPLE_METHOD) + "."
+ _("Resample method should be in ") + ", ".join(RESAMPLE_METHOD) + "."
)
if method == "asfreq" and fill_value is not None:
_df = df.resample(rule).asfreq(fill_value=fill_value)
+ _df = _df.fillna(fill_value)
elif method == "linear":
_df = df.resample(rule).interpolate()
else:
_df = getattr(df.resample(rule), method)()
+ if method in ("ffill", "bfill"):
+ _df = _df.fillna(method=method)
return _df
diff --git a/tests/unit_tests/pandas_postprocessing/test_resample.py b/tests/unit_tests/pandas_postprocessing/test_resample.py
index b1414c5fe8..207863ab87 100644
--- a/tests/unit_tests/pandas_postprocessing/test_resample.py
+++ b/tests/unit_tests/pandas_postprocessing/test_resample.py
@@ -21,7 +21,11 @@ from pandas import to_datetime
from superset.exceptions import InvalidPostProcessingError
from superset.utils import pandas_postprocessing as pp
-from tests.unit_tests.fixtures.dataframes import categories_df, timeseries_df
+from tests.unit_tests.fixtures.dataframes import (
+ categories_df,
+ timeseries_df,
+ timeseries_with_gap_df,
+)
def test_resample_should_not_side_effect():
@@ -63,6 +67,29 @@ def test_resample():
)
+def test_resample_ffill_with_gaps():
+ post_df = pp.resample(df=timeseries_with_gap_df, rule="1D", method="ffill")
+ assert post_df.equals(
+ pd.DataFrame(
+ index=pd.to_datetime(
+ [
+ "2019-01-01",
+ "2019-01-02",
+ "2019-01-03",
+ "2019-01-04",
+ "2019-01-05",
+ "2019-01-06",
+ "2019-01-07",
+ ]
+ ),
+ data={
+ "label": ["x", "y", "y", "y", "z", "z", "q"],
+ "y": [1.0, 2.0, 2.0, 2.0, 2.0, 2.0, 4.0],
+ },
+ )
+ )
+
+
def test_resample_zero_fill():
post_df = pp.resample(df=timeseries_df, rule="1D", method="asfreq", fill_value=0)
assert post_df.equals(
@@ -86,6 +113,31 @@ def test_resample_zero_fill():
)
+def test_resample_zero_fill_with_gaps():
+ post_df = pp.resample(
+ df=timeseries_with_gap_df, rule="1D", method="asfreq", fill_value=0
+ )
+ assert post_df.equals(
+ pd.DataFrame(
+ index=pd.to_datetime(
+ [
+ "2019-01-01",
+ "2019-01-02",
+ "2019-01-03",
+ "2019-01-04",
+ "2019-01-05",
+ "2019-01-06",
+ "2019-01-07",
+ ]
+ ),
+ data={
+ "label": ["x", "y", 0, 0, "z", 0, "q"],
+ "y": [1.0, 2.0, 0, 0, 0, 0, 4.0],
+ },
+ )
+ )
+
+
def test_resample_after_pivot():
df = pd.DataFrame(
data={