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 2023/08/14 12:27:02 UTC
[superset] branch 3.0 updated (7a7fa748f5 -> 4b07b5d628)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a change to branch 3.0
in repository https://gitbox.apache.org/repos/asf/superset.git
from 7a7fa748f5 fix: remove unused file (#24946)
new 5d8c65ae6f fix(charts): View in SQL Lab with relevant perm (#24903)
new ff2ec23102 fix: calls to `_get_sqla_engine` (#24953)
new 52319201f9 chore: rate limit requests (#24324)
new 5c931b1951 fix: Tooltips don't disappear on the Heatmap chart (#24959)
new dd53b334d6 fix: Duplicated options in Select when using numerical values (#24906)
new 4b07b5d628 feat: Adds options to show subtotals in Pivot Table (#24960)
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:
.../cypress-base/cypress/e2e/dashboard/utils.ts | 2 +-
.../cypress/e2e/explore/advanced_analytics.test.ts | 1 +
.../src/ReactHeatmap.jsx | 7 +-
.../src/PivotTableChart.tsx | 6 +
.../src/plugin/controlPanel.tsx | 24 ++++
.../src/plugin/transformProps.ts | 4 +
.../src/react-pivottable/TableRenderers.jsx | 4 +-
.../plugins/plugin-chart-pivot-table/src/types.ts | 2 +
.../test/plugin/buildQuery.test.ts | 2 +
.../src/components/Select/AsyncSelect.stories.tsx | 34 ------
.../src/components/Select/AsyncSelect.test.tsx | 63 +++++++---
.../src/components/Select/AsyncSelect.tsx | 75 +++++++-----
.../src/components/Select/Select.stories.tsx | 5 +
.../src/components/Select/Select.test.tsx | 135 +++++++++++++++------
superset-frontend/src/components/Select/Select.tsx | 96 +++++++++------
.../src/dashboard/util/permissionUtils.test.ts | 51 +++++---
.../src/dashboard/util/permissionUtils.ts | 17 ++-
.../DatasourceControl/DatasourceControl.test.tsx | 2 +-
.../controls/DatasourceControl/index.jsx | 4 +-
superset-frontend/src/pages/Home/Home.test.tsx | 2 +-
superset-frontend/src/pages/Home/index.tsx | 8 +-
superset/config.py | 14 +++
superset/dashboards/api.py | 2 +-
superset/db_engine_specs/trino.py | 7 +-
.../migrations/shared/migrate_viz/processors.py | 1 +
superset/models/core.py | 51 ++++----
superset/models/dashboard.py | 3 +-
superset/utils/dashboard_import_export.py | 4 +-
superset/views/dashboard/views.py | 2 +-
tests/integration_tests/celery_tests.py | 5 +-
tests/integration_tests/charts/data/api_tests.py | 7 +-
tests/integration_tests/model_tests.py | 3 +-
tests/integration_tests/superset_test_config.py | 2 +
.../migrations/viz/pivot_table_v1_v2_test.py | 1 +
34 files changed, 422 insertions(+), 224 deletions(-)
[superset] 02/06: fix: calls to `_get_sqla_engine` (#24953)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit ff2ec231029c508196b4e40538c78b24e54493ba
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Fri Aug 11 03:54:01 2023 -0700
fix: calls to `_get_sqla_engine` (#24953)
(cherry picked from commit 6f24a4e7a84cd25185b911c079aa622fb085fc29)
---
superset/db_engine_specs/trino.py | 7 ++--
superset/models/core.py | 51 +++++++++++-------------
tests/integration_tests/celery_tests.py | 5 +--
tests/integration_tests/charts/data/api_tests.py | 7 ++--
tests/integration_tests/model_tests.py | 3 +-
5 files changed, 36 insertions(+), 37 deletions(-)
diff --git a/superset/db_engine_specs/trino.py b/superset/db_engine_specs/trino.py
index f05bd67ec3..c3bdccc775 100644
--- a/superset/db_engine_specs/trino.py
+++ b/superset/db_engine_specs/trino.py
@@ -86,9 +86,10 @@ class TrinoEngineSpec(PrestoBaseEngineSpec):
}
if database.has_view_by_name(table_name, schema_name):
- metadata["view"] = database.inspector.get_view_definition(
- table_name, schema_name
- )
+ with database.get_inspector_with_context() as inspector:
+ metadata["view"] = inspector.get_view_definition(
+ table_name, schema_name
+ )
return metadata
diff --git a/superset/models/core.py b/superset/models/core.py
index 21f8eddba4..e3f91e1379 100755
--- a/superset/models/core.py
+++ b/superset/models/core.py
@@ -563,7 +563,8 @@ class Database(
mutator: Callable[[pd.DataFrame], None] | None = None,
) -> pd.DataFrame:
sqls = self.db_engine_spec.parse_sql(sql)
- engine = self._get_sqla_engine(schema)
+ with self.get_sqla_engine_with_context(schema) as engine:
+ engine_url = engine.url
mutate_after_split = config["MUTATE_AFTER_SPLIT"]
sql_query_mutator = config["SQL_QUERY_MUTATOR"]
@@ -577,7 +578,7 @@ class Database(
def _log_query(sql: str) -> None:
if log_query:
log_query(
- engine.url,
+ engine_url,
sql,
schema,
__name__,
@@ -624,13 +625,12 @@ class Database(
return df
def compile_sqla_query(self, qry: Select, schema: str | None = None) -> str:
- engine = self._get_sqla_engine(schema=schema)
+ with self.get_sqla_engine_with_context(schema) as engine:
+ sql = str(qry.compile(engine, compile_kwargs={"literal_binds": True}))
- sql = str(qry.compile(engine, compile_kwargs={"literal_binds": True}))
-
- # pylint: disable=protected-access
- if engine.dialect.identifier_preparer._double_percents: # noqa
- sql = sql.replace("%%", "%")
+ # pylint: disable=protected-access
+ if engine.dialect.identifier_preparer._double_percents: # noqa
+ sql = sql.replace("%%", "%")
return sql
@@ -645,18 +645,18 @@ class Database(
cols: list[ResultSetColumnType] | None = None,
) -> str:
"""Generates a ``select *`` statement in the proper dialect"""
- eng = self._get_sqla_engine(schema=schema, source=utils.QuerySource.SQL_LAB)
- return self.db_engine_spec.select_star(
- self,
- table_name,
- schema=schema,
- engine=eng,
- limit=limit,
- show_cols=show_cols,
- indent=indent,
- latest_partition=latest_partition,
- cols=cols,
- )
+ with self.get_sqla_engine_with_context(schema) as engine:
+ return self.db_engine_spec.select_star(
+ self,
+ table_name,
+ schema=schema,
+ engine=engine,
+ limit=limit,
+ show_cols=show_cols,
+ indent=indent,
+ latest_partition=latest_partition,
+ cols=cols,
+ )
def apply_limit_to_sql(
self, sql: str, limit: int = 1000, force: bool = False
@@ -668,11 +668,6 @@ class Database(
def safe_sqlalchemy_uri(self) -> str:
return self.sqlalchemy_uri
- @property
- def inspector(self) -> Inspector:
- engine = self._get_sqla_engine()
- return sqla.inspect(engine)
-
@cache_util.memoized_func(
key="db:{self.id}:schema:{schema}:table_list",
cache=cache_manager.cache,
@@ -955,8 +950,10 @@ class Database(
return view_name in view_names
def has_view(self, view_name: str, schema: str | None = None) -> bool:
- engine = self._get_sqla_engine()
- return engine.run_callable(self._has_view, engine.dialect, view_name, schema)
+ with self.get_sqla_engine_with_context(schema) as engine:
+ return engine.run_callable(
+ self._has_view, engine.dialect, view_name, schema
+ )
def has_view_by_name(self, view_name: str, schema: str | None = None) -> bool:
return self.has_view(view_name=view_name, schema=schema)
diff --git a/tests/integration_tests/celery_tests.py b/tests/integration_tests/celery_tests.py
index 8693a88887..29a1f7a66a 100644
--- a/tests/integration_tests/celery_tests.py
+++ b/tests/integration_tests/celery_tests.py
@@ -120,9 +120,8 @@ def drop_table_if_exists(table_name: str, table_type: CtasMethod) -> None:
def quote_f(value: Optional[str]):
if not value:
return value
- return get_example_database().inspector.engine.dialect.identifier_preparer.quote_identifier(
- value
- )
+ with get_example_database().get_inspector_with_context() as inspector:
+ return inspector.engine.dialect.identifier_preparer.quote_identifier(value)
def cta_result(ctas_method: CtasMethod):
diff --git a/tests/integration_tests/charts/data/api_tests.py b/tests/integration_tests/charts/data/api_tests.py
index da3a28f1ba..dc82026986 100644
--- a/tests/integration_tests/charts/data/api_tests.py
+++ b/tests/integration_tests/charts/data/api_tests.py
@@ -113,9 +113,10 @@ class BaseTestChartDataApi(SupersetTestCase):
def quote_name(self, name: str):
if get_main_database().backend in {"presto", "hive"}:
- return get_example_database().inspector.engine.dialect.identifier_preparer.quote_identifier(
- name
- )
+ with get_example_database().get_inspector_with_context() as inspector: # E: Ne
+ return inspector.engine.dialect.identifier_preparer.quote_identifier(
+ name
+ )
return name
diff --git a/tests/integration_tests/model_tests.py b/tests/integration_tests/model_tests.py
index 3a5f7c0a77..5222c1cb34 100644
--- a/tests/integration_tests/model_tests.py
+++ b/tests/integration_tests/model_tests.py
@@ -296,7 +296,8 @@ class TestDatabaseModel(SupersetTestCase):
db = get_example_database()
table_name = "energy_usage"
sql = db.select_star(table_name, show_cols=False, latest_partition=False)
- quote = db.inspector.engine.dialect.identifier_preparer.quote_identifier
+ with db.get_sqla_engine_with_context() as engine:
+ quote = engine.dialect.identifier_preparer.quote_identifier
expected = (
textwrap.dedent(
f"""\
[superset] 06/06: feat: Adds options to show subtotals in Pivot Table (#24960)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 4b07b5d62874567c26ee31028b356f68e46dc26d
Author: Michael S. Molina <70...@users.noreply.github.com>
AuthorDate: Mon Aug 14 09:20:32 2023 -0300
feat: Adds options to show subtotals in Pivot Table (#24960)
(cherry picked from commit be1155679963a90c7a0d699a2ebdceade40fb5a9)
---
.../src/PivotTableChart.tsx | 6 ++++++
.../src/plugin/controlPanel.tsx | 24 ++++++++++++++++++++++
.../src/plugin/transformProps.ts | 4 ++++
.../src/react-pivottable/TableRenderers.jsx | 4 ++--
.../plugins/plugin-chart-pivot-table/src/types.ts | 2 ++
.../test/plugin/buildQuery.test.ts | 2 ++
.../migrations/shared/migrate_viz/processors.py | 1 +
.../migrations/viz/pivot_table_v1_v2_test.py | 1 +
8 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/PivotTableChart.tsx b/superset-frontend/plugins/plugin-chart-pivot-table/src/PivotTableChart.tsx
index f463990b1d..0912deea1d 100644
--- a/superset-frontend/plugins/plugin-chart-pivot-table/src/PivotTableChart.tsx
+++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/PivotTableChart.tsx
@@ -138,7 +138,9 @@ export default function PivotTableChart(props: PivotTableProps) {
rowSubtotalPosition,
colSubtotalPosition,
colTotals,
+ colSubTotals,
rowTotals,
+ rowSubTotals,
valueFormat,
emitCrossFilters,
setDataMask,
@@ -425,7 +427,9 @@ export default function PivotTableChart(props: PivotTableProps) {
clickRowHeaderCallback: toggleFilter,
clickColumnHeaderCallback: toggleFilter,
colTotals,
+ colSubTotals,
rowTotals,
+ rowSubTotals,
highlightHeaderCellsOnHover:
emitCrossFilters ||
isFeatureEnabled(FeatureFlag.DRILL_BY) ||
@@ -437,10 +441,12 @@ export default function PivotTableChart(props: PivotTableProps) {
}),
[
colTotals,
+ colSubTotals,
dateFormatters,
emitCrossFilters,
metricColorFormatters,
rowTotals,
+ rowSubTotals,
selectedFilters,
toggleFilter,
],
diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx
index d099406c55..2ba473358b 100644
--- a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx
+++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx
@@ -218,6 +218,18 @@ const config: ControlPanelConfig = {
},
},
],
+ [
+ {
+ name: 'rowSubTotals',
+ config: {
+ type: 'CheckboxControl',
+ label: t('Show rows subtotal'),
+ default: false,
+ renderTrigger: true,
+ description: t('Display row level subtotal'),
+ },
+ },
+ ],
[
{
name: 'colTotals',
@@ -230,6 +242,18 @@ const config: ControlPanelConfig = {
},
},
],
+ [
+ {
+ name: 'colSubTotals',
+ config: {
+ type: 'CheckboxControl',
+ label: t('Show columns subtotal'),
+ default: false,
+ renderTrigger: true,
+ description: t('Display column level subtotal'),
+ },
+ },
+ ],
[
{
name: 'transposePivot',
diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/transformProps.ts b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/transformProps.ts
index f335c6978e..829f0178be 100644
--- a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/transformProps.ts
@@ -96,7 +96,9 @@ export default function transformProps(chartProps: ChartProps<QueryFormData>) {
rowSubtotalPosition,
colSubtotalPosition,
colTotals,
+ colSubTotals,
rowTotals,
+ rowSubTotals,
valueFormat,
dateFormat,
metricsLayout,
@@ -155,7 +157,9 @@ export default function transformProps(chartProps: ChartProps<QueryFormData>) {
rowSubtotalPosition,
colSubtotalPosition,
colTotals,
+ colSubTotals,
rowTotals,
+ rowSubTotals,
valueFormat,
emitCrossFilters,
setDataMask,
diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.jsx b/superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.jsx
index 8915ed7e9c..760ff90c15 100644
--- a/superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.jsx
+++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.jsx
@@ -92,14 +92,14 @@ export class TableRenderer extends React.Component {
const colSubtotalDisplay = {
displayOnTop: false,
- enabled: rowTotals,
+ enabled: tableOptions.colSubTotals,
hideOnExpand: false,
...subtotalOptions.colSubtotalDisplay,
};
const rowSubtotalDisplay = {
displayOnTop: false,
- enabled: colTotals,
+ enabled: tableOptions.rowSubTotals,
hideOnExpand: false,
...subtotalOptions.rowSubtotalDisplay,
};
diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/types.ts b/superset-frontend/plugins/plugin-chart-pivot-table/src/types.ts
index dea5236666..8eeef30efa 100644
--- a/superset-frontend/plugins/plugin-chart-pivot-table/src/types.ts
+++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/types.ts
@@ -63,7 +63,9 @@ interface PivotTableCustomizeProps {
rowSubtotalPosition: boolean;
colSubtotalPosition: boolean;
colTotals: boolean;
+ colSubTotals: boolean;
rowTotals: boolean;
+ rowSubTotals: boolean;
valueFormat: string;
setDataMask: SetDataMaskHook;
emitCrossFilters?: boolean;
diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/test/plugin/buildQuery.test.ts b/superset-frontend/plugins/plugin-chart-pivot-table/test/plugin/buildQuery.test.ts
index 7bb47d785c..468a1d62ba 100644
--- a/superset-frontend/plugins/plugin-chart-pivot-table/test/plugin/buildQuery.test.ts
+++ b/superset-frontend/plugins/plugin-chart-pivot-table/test/plugin/buildQuery.test.ts
@@ -34,7 +34,9 @@ const formData: PivotTableQueryFormData = {
rowSubtotalPosition: true,
colSubtotalPosition: true,
colTotals: true,
+ colSubTotals: true,
rowTotals: true,
+ rowSubTotals: true,
valueFormat: 'SMART_NUMBER',
datasource: '5__table',
viz_type: 'my_chart',
diff --git a/superset/migrations/shared/migrate_viz/processors.py b/superset/migrations/shared/migrate_viz/processors.py
index 70c3c27055..d1978f33e1 100644
--- a/superset/migrations/shared/migrate_viz/processors.py
+++ b/superset/migrations/shared/migrate_viz/processors.py
@@ -95,6 +95,7 @@ class MigratePivotTable(MigrateViz):
def _pre_action(self) -> None:
if pivot_margins := self.data.get("pivot_margins"):
self.data["colTotals"] = pivot_margins
+ self.data["colSubTotals"] = pivot_margins
if pandas_aggfunc := self.data.get("pandas_aggfunc"):
self.data["pandas_aggfunc"] = self.aggregation_mapping[pandas_aggfunc]
diff --git a/tests/unit_tests/migrations/viz/pivot_table_v1_v2_test.py b/tests/unit_tests/migrations/viz/pivot_table_v1_v2_test.py
index ab357b62c3..1e2229ca83 100644
--- a/tests/unit_tests/migrations/viz/pivot_table_v1_v2_test.py
+++ b/tests/unit_tests/migrations/viz/pivot_table_v1_v2_test.py
@@ -40,6 +40,7 @@ TARGET_FORM_DATA = {
"any_other_key": "untouched",
"aggregateFunction": "Sum",
"colTotals": True,
+ "colSubTotals": True,
"combineMetric": True,
"form_data_bak": SOURCE_FORM_DATA,
"granularity_sqla": "ds",
[superset] 01/06: fix(charts): View in SQL Lab with relevant perm (#24903)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 5d8c65ae6f2c4827f8c6be0f59785c58be8e098d
Author: Rob Moore <gi...@users.noreply.github.com>
AuthorDate: Thu Aug 10 18:04:07 2023 +0100
fix(charts): View in SQL Lab with relevant perm (#24903)
(cherry picked from commit ce65a3b9cd56e4d9e1966e78e577ef7ec18d6412)
---
.../src/dashboard/util/permissionUtils.test.ts | 51 ++++++++++++++++------
.../src/dashboard/util/permissionUtils.ts | 17 +++++---
.../DatasourceControl/DatasourceControl.test.tsx | 2 +-
.../controls/DatasourceControl/index.jsx | 4 +-
superset-frontend/src/pages/Home/Home.test.tsx | 2 +-
superset-frontend/src/pages/Home/index.tsx | 8 ++--
6 files changed, 56 insertions(+), 28 deletions(-)
diff --git a/superset-frontend/src/dashboard/util/permissionUtils.test.ts b/superset-frontend/src/dashboard/util/permissionUtils.test.ts
index b9b0e071b4..c6d707dbb0 100644
--- a/superset-frontend/src/dashboard/util/permissionUtils.test.ts
+++ b/superset-frontend/src/dashboard/util/permissionUtils.test.ts
@@ -24,7 +24,7 @@ import {
import { Dashboard } from 'src/types/Dashboard';
import Owner from 'src/types/Owner';
import {
- canUserAccessSqlLab,
+ userHasPermission,
canUserEditDashboard,
canUserSaveAsDashboard,
isUserAdmin,
@@ -65,11 +65,18 @@ const owner: Owner = {
last_name: 'User',
};
+const sqlLabMenuAccessPermission: [string, string] = ['menu_access', 'SQL Lab'];
+
+const arbitraryPermissions: [string, string][] = [
+ ['can_write', 'AnArbitraryView'],
+ sqlLabMenuAccessPermission,
+];
+
const sqlLabUser: UserWithPermissionsAndRoles = {
...ownerUser,
roles: {
...ownerUser.roles,
- sql_lab: [],
+ sql_lab: [sqlLabMenuAccessPermission],
},
};
@@ -139,24 +146,40 @@ test('isUserAdmin returns false for non-admin user', () => {
expect(isUserAdmin(ownerUser)).toEqual(false);
});
-test('canUserAccessSqlLab returns true for admin user', () => {
- expect(canUserAccessSqlLab(adminUser)).toEqual(true);
-});
-
-test('canUserAccessSqlLab returns false for undefined', () => {
- expect(canUserAccessSqlLab(undefined)).toEqual(false);
+test('userHasPermission always returns true for admin user', () => {
+ arbitraryPermissions.forEach(permissionView => {
+ expect(
+ userHasPermission(adminUser, permissionView[1], permissionView[0]),
+ ).toEqual(true);
+ });
});
-test('canUserAccessSqlLab returns false for undefined user', () => {
- expect(canUserAccessSqlLab(undefinedUser)).toEqual(false);
+test('userHasPermission always returns false for undefined user', () => {
+ arbitraryPermissions.forEach(permissionView => {
+ expect(
+ userHasPermission(undefinedUser, permissionView[1], permissionView[0]),
+ ).toEqual(false);
+ });
});
-test('canUserAccessSqlLab returns false for non-sqllab role', () => {
- expect(canUserAccessSqlLab(ownerUser)).toEqual(false);
+test('userHasPermission returns false if user does not have permission', () => {
+ expect(
+ userHasPermission(
+ ownerUser,
+ sqlLabMenuAccessPermission[1],
+ sqlLabMenuAccessPermission[0],
+ ),
+ ).toEqual(false);
});
-test('canUserAccessSqlLab returns true for sqllab role', () => {
- expect(canUserAccessSqlLab(sqlLabUser)).toEqual(true);
+test('userHasPermission returns true if user has permission', () => {
+ expect(
+ userHasPermission(
+ sqlLabUser,
+ sqlLabMenuAccessPermission[1],
+ sqlLabMenuAccessPermission[0],
+ ),
+ ).toEqual(true);
});
describe('canUserSaveAsDashboard with RBAC feature flag disabled', () => {
diff --git a/superset-frontend/src/dashboard/util/permissionUtils.ts b/superset-frontend/src/dashboard/util/permissionUtils.ts
index 23744bde89..4b09710d59 100644
--- a/superset-frontend/src/dashboard/util/permissionUtils.ts
+++ b/superset-frontend/src/dashboard/util/permissionUtils.ts
@@ -28,7 +28,6 @@ import { findPermission } from 'src/utils/findPermission';
// this should really be a config value,
// but is hardcoded in backend logic already, so...
const ADMIN_ROLE_NAME = 'admin';
-const SQL_LAB_ROLE = 'sql_lab';
export const isUserAdmin = (
user?: UserWithPermissionsAndRoles | UndefinedUser,
@@ -53,15 +52,21 @@ export const canUserEditDashboard = (
(isUserAdmin(user) || isUserDashboardOwner(dashboard, user)) &&
findPermission('can_write', 'Dashboard', user?.roles);
-export function canUserAccessSqlLab(
- user?: UserWithPermissionsAndRoles | UndefinedUser,
+export function userHasPermission(
+ user: UserWithPermissionsAndRoles | UndefinedUser,
+ viewName: string,
+ permissionName: string,
) {
return (
isUserAdmin(user) ||
(isUserWithPermissionsAndRoles(user) &&
- Object.keys(user.roles || {}).some(
- role => role.toLowerCase() === SQL_LAB_ROLE,
- ))
+ Object.values(user.roles || {})
+ .flat()
+ .some(
+ permissionView =>
+ permissionView[0] === permissionName &&
+ permissionView[1] === viewName,
+ ))
);
}
diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx
index 80cb84ac8d..6def65d7d2 100644
--- a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx
+++ b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx
@@ -149,7 +149,7 @@ test('Should show SQL Lab for sql_lab role', async () => {
isActive: true,
lastName: 'sql',
permissions: {},
- roles: { Gamma: [], sql_lab: [] },
+ roles: { Gamma: [], sql_lab: [['menu_access', 'SQL Lab']] },
userId: 2,
username: 'sql',
},
diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx b/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx
index c99193dd42..bf85716206 100644
--- a/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx
+++ b/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx
@@ -43,7 +43,7 @@ import WarningIconWithTooltip from 'src/components/WarningIconWithTooltip';
import { URL_PARAMS } from 'src/constants';
import { getDatasourceAsSaveableDataset } from 'src/utils/datasourceUtils';
import {
- canUserAccessSqlLab,
+ userHasPermission,
isUserAdmin,
} from 'src/dashboard/util/permissionUtils';
import ModalTrigger from 'src/components/ModalTrigger';
@@ -283,7 +283,7 @@ class DatasourceControl extends React.PureComponent {
datasource.owners?.map(o => o.id || o.value).includes(user.userId) ||
isUserAdmin(user);
- const canAccessSqlLab = canUserAccessSqlLab(user);
+ const canAccessSqlLab = userHasPermission(user, 'SQL Lab', 'menu_access');
const editText = t('Edit dataset');
diff --git a/superset-frontend/src/pages/Home/Home.test.tsx b/superset-frontend/src/pages/Home/Home.test.tsx
index 9f0b4e3ca1..8de33f323e 100644
--- a/superset-frontend/src/pages/Home/Home.test.tsx
+++ b/superset-frontend/src/pages/Home/Home.test.tsx
@@ -109,7 +109,7 @@ const mockedProps = {
isAnonymous: false,
permissions: {},
roles: {
- sql_lab: [],
+ sql_lab: [['can_read', 'SavedQuery']],
},
},
};
diff --git a/superset-frontend/src/pages/Home/index.tsx b/superset-frontend/src/pages/Home/index.tsx
index cfeb4cd982..c61d15e2ab 100644
--- a/superset-frontend/src/pages/Home/index.tsx
+++ b/superset-frontend/src/pages/Home/index.tsx
@@ -50,7 +50,7 @@ import { AntdSwitch } from 'src/components';
import getBootstrapData from 'src/utils/getBootstrapData';
import { TableTab } from 'src/views/CRUD/types';
import SubMenu, { SubMenuProps } from 'src/features/home/SubMenu';
-import { canUserAccessSqlLab } from 'src/dashboard/util/permissionUtils';
+import { userHasPermission } from 'src/dashboard/util/permissionUtils';
import { WelcomePageLastTab } from 'src/features/home/types';
import ActivityTable from 'src/features/home/ActivityTable';
import ChartTable from 'src/features/home/ChartTable';
@@ -156,7 +156,7 @@ export const LoadingCards = ({ cover }: LoadingProps) => (
);
function Welcome({ user, addDangerToast }: WelcomeProps) {
- const canAccessSqlLab = canUserAccessSqlLab(user);
+ const canReadSavedQueries = userHasPermission(user, 'SavedQuery', 'can_read');
const userid = user.userId;
const id = userid!.toString(); // confident that user is not a guest user
const params = rison.encode({ page_size: 6 });
@@ -281,7 +281,7 @@ function Welcome({ user, addDangerToast }: WelcomeProps) {
addDangerToast(t('There was an issue fetching your chart: %s', err));
return Promise.resolve();
}),
- canAccessSqlLab
+ canReadSavedQueries
? getUserOwnedObjects(id, 'saved_query', ownSavedQueryFilters)
.then(r => {
setQueryData(r);
@@ -410,7 +410,7 @@ function Welcome({ user, addDangerToast }: WelcomeProps) {
/>
)}
</Collapse.Panel>
- {canAccessSqlLab && (
+ {canReadSavedQueries && (
<Collapse.Panel header={t('Saved queries')} key="4">
{!queryData ? (
<LoadingCards cover={checked} />
[superset] 03/06: chore: rate limit requests (#24324)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 52319201f9c5621f2b4f03ab2306af4b180d6e17
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Fri Aug 11 09:35:31 2023 -0700
chore: rate limit requests (#24324)
(cherry picked from commit 4bc46003b56e66c1a5af7b5ad50b869187623e7f)
---
superset/config.py | 14 ++++++++++++++
superset/dashboards/api.py | 2 +-
superset/models/dashboard.py | 3 ++-
superset/utils/dashboard_import_export.py | 4 ++--
superset/views/dashboard/views.py | 2 +-
tests/integration_tests/superset_test_config.py | 2 ++
6 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/superset/config.py b/superset/config.py
index ded4dc1404..93cef69ba8 100644
--- a/superset/config.py
+++ b/superset/config.py
@@ -268,6 +268,20 @@ PROXY_FIX_CONFIG = {"x_for": 1, "x_proto": 1, "x_host": 1, "x_port": 1, "x_prefi
# Configuration for scheduling queries from SQL Lab.
SCHEDULED_QUERIES: dict[str, Any] = {}
+# FAB Rate limiting: this is a security feature for preventing DDOS attacks. The
+# feature is on by default to make Superset secure by default, but you should
+# fine tune the limits to your needs. You can read more about the different
+# parameters here: https://flask-limiter.readthedocs.io/en/stable/configuration.html
+RATELIMIT_ENABLED = True
+RATELIMIT_APPLICATION = "50 per second"
+AUTH_RATE_LIMITED = True
+AUTH_RATE_LIMIT = "5 per second"
+# A storage location conforming to the scheme in storage-scheme. See the limits
+# library for allowed values: https://limits.readthedocs.io/en/stable/storage.html
+# RATELIMIT_STORAGE_URI = "redis://host:port"
+# A callable that returns the unique identity of the current request.
+# RATELIMIT_REQUEST_IDENTIFIER = flask.Request.endpoint
+
# ------------------------------
# GLOBALS FOR APP Builder
# ------------------------------
diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py
index 07f5ed6fbc..150c99e47c 100644
--- a/superset/dashboards/api.py
+++ b/superset/dashboards/api.py
@@ -822,7 +822,7 @@ class DashboardRestApi(BaseSupersetModelRestApi):
Dashboard.id.in_(requested_ids)
)
query = self._base_filters.apply_all(query)
- ids = [item.id for item in query.all()]
+ ids = {item.id for item in query.all()}
if not ids:
return self.response_404()
export = Dashboard.export_dashboards(ids)
diff --git a/superset/models/dashboard.py b/superset/models/dashboard.py
index 719a6df8e4..f837c76610 100644
--- a/superset/models/dashboard.py
+++ b/superset/models/dashboard.py
@@ -373,7 +373,8 @@ class Dashboard(Model, AuditMixinNullable, ImportExportMixin):
@classmethod
def export_dashboards( # pylint: disable=too-many-locals
- cls, dashboard_ids: list[int]
+ cls,
+ dashboard_ids: set[int],
) -> str:
copied_dashboards = []
datasource_ids = set()
diff --git a/superset/utils/dashboard_import_export.py b/superset/utils/dashboard_import_export.py
index fc61d0a422..eef8cbe6df 100644
--- a/superset/utils/dashboard_import_export.py
+++ b/superset/utils/dashboard_import_export.py
@@ -27,8 +27,8 @@ def export_dashboards(session: Session) -> str:
"""Returns all dashboards metadata as a json dump"""
logger.info("Starting export")
dashboards = session.query(Dashboard)
- dashboard_ids = []
+ dashboard_ids = set()
for dashboard in dashboards:
- dashboard_ids.append(dashboard.id)
+ dashboard_ids.add(dashboard.id)
data = Dashboard.export_dashboards(dashboard_ids)
return data
diff --git a/superset/views/dashboard/views.py b/superset/views/dashboard/views.py
index 71ef212f6d..ba8b8b2fb3 100644
--- a/superset/views/dashboard/views.py
+++ b/superset/views/dashboard/views.py
@@ -78,7 +78,7 @@ class DashboardModelView(
@expose("/export_dashboards_form")
def download_dashboards(self) -> FlaskResponse:
if request.args.get("action") == "go":
- ids = request.args.getlist("id")
+ ids = set(request.args.getlist("id"))
return Response(
DashboardModel.export_dashboards(ids),
headers=generate_download_headers("json"),
diff --git a/tests/integration_tests/superset_test_config.py b/tests/integration_tests/superset_test_config.py
index 77e007a2dd..bcc3146083 100644
--- a/tests/integration_tests/superset_test_config.py
+++ b/tests/integration_tests/superset_test_config.py
@@ -97,6 +97,8 @@ REDIS_CELERY_DB = os.environ.get("REDIS_CELERY_DB", 2)
REDIS_RESULTS_DB = os.environ.get("REDIS_RESULTS_DB", 3)
REDIS_CACHE_DB = os.environ.get("REDIS_CACHE_DB", 4)
+RATELIMIT_ENABLED = False
+
CACHE_CONFIG = {
"CACHE_TYPE": "RedisCache",
[superset] 05/06: fix: Duplicated options in Select when using numerical values (#24906)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit dd53b334d6b0103a26bad75535fbbdc061161f6f
Author: Michael S. Molina <70...@users.noreply.github.com>
AuthorDate: Fri Aug 11 14:22:15 2023 -0300
fix: Duplicated options in Select when using numerical values (#24906)
(cherry picked from commit b621ee92c9124e2e2f7c988302eb0f77f00c9fc9)
---
.../cypress-base/cypress/e2e/dashboard/utils.ts | 2 +-
.../cypress/e2e/explore/advanced_analytics.test.ts | 1 +
.../src/components/Select/AsyncSelect.stories.tsx | 34 ------
.../src/components/Select/AsyncSelect.test.tsx | 63 +++++++---
.../src/components/Select/AsyncSelect.tsx | 75 +++++++-----
.../src/components/Select/Select.stories.tsx | 5 +
.../src/components/Select/Select.test.tsx | 135 +++++++++++++++------
superset-frontend/src/components/Select/Select.tsx | 96 +++++++++------
8 files changed, 260 insertions(+), 151 deletions(-)
diff --git a/superset-frontend/cypress-base/cypress/e2e/dashboard/utils.ts b/superset-frontend/cypress-base/cypress/e2e/dashboard/utils.ts
index 00d3eda45e..ca539039cf 100644
--- a/superset-frontend/cypress-base/cypress/e2e/dashboard/utils.ts
+++ b/superset-frontend/cypress-base/cypress/e2e/dashboard/utils.ts
@@ -322,7 +322,7 @@ export function applyNativeFilterValueWithIndex(index: number, value: string) {
cy.get(nativeFilters.filterFromDashboardView.filterValueInput)
.eq(index)
.should('exist', { timeout: 10000 })
- .type(`${value}{enter}`);
+ .type(`${value}{enter}`, { force: true });
// click the title to dismiss shown options
cy.get(nativeFilters.filterFromDashboardView.filterName)
.eq(index)
diff --git a/superset-frontend/cypress-base/cypress/e2e/explore/advanced_analytics.test.ts b/superset-frontend/cypress-base/cypress/e2e/explore/advanced_analytics.test.ts
index fd207a64e3..8e52aa6c56 100644
--- a/superset-frontend/cypress-base/cypress/e2e/explore/advanced_analytics.test.ts
+++ b/superset-frontend/cypress-base/cypress/e2e/explore/advanced_analytics.test.ts
@@ -39,6 +39,7 @@ describe('Advanced analytics', () => {
cy.get('[data-test=time_compare]')
.find('input[type=search]')
+ .clear()
.type('1 year{enter}');
cy.get('button[data-test="run-query-button"]').click();
diff --git a/superset-frontend/src/components/Select/AsyncSelect.stories.tsx b/superset-frontend/src/components/Select/AsyncSelect.stories.tsx
index 0bdaf43f2c..4235008fb2 100644
--- a/superset-frontend/src/components/Select/AsyncSelect.stories.tsx
+++ b/superset-frontend/src/components/Select/AsyncSelect.stories.tsx
@@ -26,7 +26,6 @@ import React, {
import Button from 'src/components/Button';
import AsyncSelect from './AsyncSelect';
import {
- SelectOptionsType,
AsyncSelectProps,
AsyncSelectRef,
SelectOptionsTypePage,
@@ -39,40 +38,7 @@ export default {
const DEFAULT_WIDTH = 200;
-const options: SelectOptionsType = [
- {
- label: 'Such an incredibly awesome long long label',
- value: 'Such an incredibly awesome long long label',
- custom: 'Secret custom prop',
- },
- {
- label: 'Another incredibly awesome long long label',
- value: 'Another incredibly awesome long long label',
- },
- {
- label: 'JSX Label',
- customLabel: <div style={{ color: 'red' }}>JSX Label</div>,
- value: 'JSX Label',
- },
- { label: 'A', value: 'A' },
- { label: 'B', value: 'B' },
- { label: 'C', value: 'C' },
- { label: 'D', value: 'D' },
- { label: 'E', value: 'E' },
- { label: 'F', value: 'F' },
- { label: 'G', value: 'G' },
- { label: 'H', value: 'H' },
- { label: 'I', value: 'I' },
-];
-
const ARG_TYPES = {
- options: {
- defaultValue: options,
- description: `It defines the options of the Select.
- The options can be static, an array of options.
- The options can also be async, a promise that returns an array of options.
- `,
- },
ariaLabel: {
description: `It adds the aria-label tag for accessibility standards.
Must be plain English and localized.
diff --git a/superset-frontend/src/components/Select/AsyncSelect.test.tsx b/superset-frontend/src/components/Select/AsyncSelect.test.tsx
index 0b7cafab3a..b964f48ee7 100644
--- a/superset-frontend/src/components/Select/AsyncSelect.test.tsx
+++ b/superset-frontend/src/components/Select/AsyncSelect.test.tsx
@@ -17,7 +17,14 @@
* under the License.
*/
import React from 'react';
-import { render, screen, waitFor, within } from 'spec/helpers/testing-library';
+import {
+ createEvent,
+ fireEvent,
+ render,
+ screen,
+ waitFor,
+ within,
+} from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
import { AsyncSelect } from 'src/components';
@@ -93,6 +100,9 @@ const getElementsByClassName = (className: string) =>
const getSelect = () => screen.getByRole('combobox', { name: ARIA_LABEL });
+const getAllSelectOptions = () =>
+ getElementsByClassName('.ant-select-item-option-content');
+
const findSelectOption = (text: string) =>
waitFor(() =>
within(getElementByClassName('.rc-virtual-list')).getByText(text),
@@ -323,12 +333,14 @@ test('same case should be ranked to the top', async () => {
}));
render(<AsyncSelect {...defaultProps} options={loadOptions} />);
await type('Ac');
- const options = await findAllSelectOptions();
- expect(options.length).toBe(4);
- expect(options[0]?.textContent).toEqual('acbc');
- expect(options[1]?.textContent).toEqual('CAc');
- expect(options[2]?.textContent).toEqual('abac');
- expect(options[3]?.textContent).toEqual('Cac');
+ await waitFor(() => {
+ const options = getAllSelectOptions();
+ expect(options.length).toBe(4);
+ expect(options[0]?.textContent).toEqual('acbc');
+ expect(options[1]?.textContent).toEqual('CAc');
+ expect(options[2]?.textContent).toEqual('abac');
+ expect(options[3]?.textContent).toEqual('Cac');
+ });
});
test('ignores special keys when searching', async () => {
@@ -365,7 +377,13 @@ test('searches for custom fields', async () => {
test('removes duplicated values', async () => {
render(<AsyncSelect {...defaultProps} mode="multiple" allowNewOptions />);
- await type('a,b,b,b,c,d,d');
+ const input = getElementByClassName('.ant-select-selection-search-input');
+ const paste = createEvent.paste(input, {
+ clipboardData: {
+ getData: () => 'a,b,b,b,c,d,d',
+ },
+ });
+ fireEvent(input, paste);
const values = await findAllSelectValues();
expect(values.length).toBe(4);
expect(values[0]).toHaveTextContent('a');
@@ -601,7 +619,9 @@ test('does not show "No data" when allowNewOptions is true and a new option is e
render(<AsyncSelect {...defaultProps} allowNewOptions />);
await open();
await type(NEW_OPTION);
- expect(screen.queryByText(NO_DATA)).not.toBeInTheDocument();
+ await waitFor(() =>
+ expect(screen.queryByText(NO_DATA)).not.toBeInTheDocument(),
+ );
});
test('sets a initial value in single mode', async () => {
@@ -690,12 +710,9 @@ test('does not fire a new request for the same search input', async () => {
<AsyncSelect {...defaultProps} options={loadOptions} fetchOnlyOnSearch />,
);
await type('search');
- expect(await screen.findByText(NO_DATA)).toBeInTheDocument();
- expect(loadOptions).toHaveBeenCalledTimes(1);
- clearAll();
+ await waitFor(() => expect(loadOptions).toHaveBeenCalledTimes(1));
await type('search');
- expect(await screen.findByText(LOADING)).toBeInTheDocument();
- expect(loadOptions).toHaveBeenCalledTimes(1);
+ await waitFor(() => expect(loadOptions).toHaveBeenCalledTimes(1));
});
test('does not fire a new request if all values have been fetched', async () => {
@@ -823,6 +840,24 @@ test('does not fire onChange when searching but no selection', async () => {
expect(onChange).toHaveBeenCalledTimes(1);
});
+test('does not duplicate options when using numeric values', async () => {
+ render(
+ <AsyncSelect
+ {...defaultProps}
+ mode="multiple"
+ options={async () => ({
+ data: [
+ { label: '1', value: 1 },
+ { label: '2', value: 2 },
+ ],
+ totalCount: 2,
+ })}
+ />,
+ );
+ await type('1');
+ await waitFor(() => expect(getAllSelectOptions().length).toBe(1));
+});
+
/*
TODO: Add tests that require scroll interaction. Needs further investigation.
- Fetches more data when scrolling and more data is available
diff --git a/superset-frontend/src/components/Select/AsyncSelect.tsx b/superset-frontend/src/components/Select/AsyncSelect.tsx
index 3453ce2b5f..320d6ec3bd 100644
--- a/superset-frontend/src/components/Select/AsyncSelect.tsx
+++ b/superset-frontend/src/components/Select/AsyncSelect.tsx
@@ -27,14 +27,15 @@ import React, {
useRef,
useCallback,
useImperativeHandle,
+ ClipboardEvent,
} from 'react';
import { ensureIsArray, t, usePrevious } from '@superset-ui/core';
import { LabeledValue as AntdLabeledValue } from 'antd/lib/select';
import debounce from 'lodash/debounce';
-import { isEqual } from 'lodash';
+import { isEqual, uniq } from 'lodash';
import Icons from 'src/components/Icons';
import { getClientErrorObject } from 'src/utils/getClientErrorObject';
-import { SLOW_DEBOUNCE } from 'src/constants';
+import { FAST_DEBOUNCE, SLOW_DEBOUNCE } from 'src/constants';
import {
getValue,
hasOption,
@@ -122,6 +123,7 @@ const AsyncSelect = forwardRef(
onClear,
onDropdownVisibleChange,
onDeselect,
+ onSearch,
onSelect,
optionFilterProps = ['label', 'value'],
options,
@@ -129,7 +131,7 @@ const AsyncSelect = forwardRef(
placeholder = t('Select ...'),
showSearch = true,
sortComparator = DEFAULT_SORT_COMPARATOR,
- tokenSeparators,
+ tokenSeparators = TOKEN_SEPARATORS,
value,
getPopupContainer,
oneLine,
@@ -150,11 +152,7 @@ const AsyncSelect = forwardRef(
const [allValuesLoaded, setAllValuesLoaded] = useState(false);
const selectValueRef = useRef(selectValue);
const fetchedQueries = useRef(new Map<string, number>());
- const mappedMode = isSingleMode
- ? undefined
- : allowNewOptions
- ? 'tags'
- : 'multiple';
+ const mappedMode = isSingleMode ? undefined : 'multiple';
const allowFetch = !fetchOnlyOnSearch || inputValue;
const [maxTagCount, setMaxTagCount] = useState(
propsMaxTagCount ?? MAX_TAG_COUNT,
@@ -253,6 +251,14 @@ const AsyncSelect = forwardRef(
const array = selectValue as (string | number)[];
setSelectValue(array.filter(element => element !== value));
}
+ // removes new option
+ if (option.isNewOption) {
+ setSelectOptions(
+ fullSelectOptions.filter(
+ option => getValue(option.value) !== getValue(value),
+ ),
+ );
+ }
}
fireOnChange();
onDeselect?.(value, option);
@@ -341,9 +347,9 @@ const AsyncSelect = forwardRef(
[fetchPage],
);
- const handleOnSearch = (search: string) => {
+ const handleOnSearch = debounce((search: string) => {
const searchValue = search.trim();
- if (allowNewOptions && isSingleMode) {
+ if (allowNewOptions) {
const newOption = searchValue &&
!hasOption(searchValue, fullSelectOptions, true) && {
label: searchValue,
@@ -368,7 +374,10 @@ const AsyncSelect = forwardRef(
setIsLoading(!(fetchOnlyOnSearch && !searchValue));
}
setInputValue(search);
- };
+ onSearch?.(searchValue);
+ }, FAST_DEBOUNCE);
+
+ useEffect(() => () => handleOnSearch.cancel(), [handleOnSearch]);
const handlePagination = (e: UIEvent<HTMLElement>) => {
const vScroll = e.currentTarget;
@@ -439,19 +448,7 @@ const AsyncSelect = forwardRef(
};
const handleOnBlur = (event: React.FocusEvent<HTMLElement>) => {
- const tagsMode = !isSingleMode && allowNewOptions;
- const searchValue = inputValue.trim();
- // Searched values will be autoselected during onBlur events when in tags mode.
- // We want to make sure a value is only selected if the user has actually selected it
- // by pressing Enter or clicking on it.
- if (
- tagsMode &&
- searchValue &&
- !hasOption(searchValue, selectValue, true)
- ) {
- // The search value will be added so we revert to the previous value
- setSelectValue(selectValue || []);
- }
+ setInputValue('');
onBlur?.(event);
};
@@ -526,6 +523,28 @@ const AsyncSelect = forwardRef(
[ref],
);
+ const onPaste = (e: ClipboardEvent<HTMLInputElement>) => {
+ const pastedText = e.clipboardData.getData('text');
+ if (isSingleMode) {
+ setSelectValue({ label: pastedText, value: pastedText });
+ } else {
+ const token = tokenSeparators.find(token => pastedText.includes(token));
+ const array = token ? uniq(pastedText.split(token)) : [pastedText];
+ setSelectValue(previous => [
+ ...((previous || []) as AntdLabeledValue[]),
+ ...array.map<AntdLabeledValue>(value => ({
+ label: value,
+ value,
+ })),
+ ]);
+ }
+ };
+
+ const shouldRenderChildrenOptions = useMemo(
+ () => hasCustomLabels(fullSelectOptions),
+ [fullSelectOptions],
+ );
+
return (
<StyledContainer headerPosition={headerPosition}>
{header && (
@@ -549,17 +568,17 @@ const AsyncSelect = forwardRef(
onBlur={handleOnBlur}
onDeselect={handleOnDeselect}
onDropdownVisibleChange={handleOnDropdownVisibleChange}
+ // @ts-ignore
+ onPaste={onPaste}
onPopupScroll={handlePagination}
onSearch={showSearch ? handleOnSearch : undefined}
onSelect={handleOnSelect}
onClear={handleClear}
- options={
- hasCustomLabels(fullSelectOptions) ? undefined : fullSelectOptions
- }
+ options={shouldRenderChildrenOptions ? undefined : fullSelectOptions}
placeholder={placeholder}
showSearch={showSearch}
showArrow
- tokenSeparators={tokenSeparators || TOKEN_SEPARATORS}
+ tokenSeparators={tokenSeparators}
value={selectValue}
suffixIcon={getSuffixIcon(isLoading, showSearch, isDropdownVisible)}
menuItemSelectedIcon={
diff --git a/superset-frontend/src/components/Select/Select.stories.tsx b/superset-frontend/src/components/Select/Select.stories.tsx
index 58fa424700..d7a9b6e893 100644
--- a/superset-frontend/src/components/Select/Select.stories.tsx
+++ b/superset-frontend/src/components/Select/Select.stories.tsx
@@ -103,6 +103,11 @@ const ARG_TYPES = {
disable: true,
},
},
+ mappedMode: {
+ table: {
+ disable: true,
+ },
+ },
mode: {
description: `It defines whether the Select should allow for
the selection of multiple options or single. Single by default.
diff --git a/superset-frontend/src/components/Select/Select.test.tsx b/superset-frontend/src/components/Select/Select.test.tsx
index bce753604f..2b204cec1c 100644
--- a/superset-frontend/src/components/Select/Select.test.tsx
+++ b/superset-frontend/src/components/Select/Select.test.tsx
@@ -17,7 +17,14 @@
* under the License.
*/
import React from 'react';
-import { render, screen, waitFor, within } from 'spec/helpers/testing-library';
+import {
+ createEvent,
+ fireEvent,
+ render,
+ screen,
+ waitFor,
+ within,
+} from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
import Select from 'src/components/Select/Select';
import { SELECT_ALL_VALUE } from './utils';
@@ -68,7 +75,6 @@ const defaultProps = {
ariaLabel: ARIA_LABEL,
labelInValue: true,
options: OPTIONS,
- pageSize: 10,
showSearch: true,
};
@@ -93,6 +99,9 @@ const querySelectOption = (text: string) =>
within(getElementByClassName('.rc-virtual-list')).queryByText(text),
);
+const getAllSelectOptions = () =>
+ getElementsByClassName('.ant-select-item-option-content');
+
const findAllSelectOptions = () =>
waitFor(() => getElementsByClassName('.ant-select-item-option-content'));
@@ -134,6 +143,11 @@ const clearTypedText = () => {
const open = () => waitFor(() => userEvent.click(getSelect()));
+const reopen = async () => {
+ await type('{esc}');
+ await open();
+};
+
test('displays a header', async () => {
const headerText = 'Header';
render(<Select {...defaultProps} header={headerText} />);
@@ -201,8 +215,7 @@ test('should sort selected to top when in single mode', async () => {
expect(await matchOrder(originalLabels)).toBe(true);
// order selected to top when reopen
- await type('{esc}');
- await open();
+ await reopen();
let labels = originalLabels.slice();
labels = labels.splice(1, 1).concat(labels);
expect(await matchOrder(labels)).toBe(true);
@@ -211,16 +224,14 @@ test('should sort selected to top when in single mode', async () => {
// original order
userEvent.click(await findSelectOption(originalLabels[5]));
await matchOrder(labels);
- await type('{esc}');
- await open();
+ await reopen();
labels = originalLabels.slice();
labels = labels.splice(5, 1).concat(labels);
expect(await matchOrder(labels)).toBe(true);
// should revert to original order
clearAll();
- await type('{esc}');
- await open();
+ await reopen();
expect(await matchOrder(originalLabels)).toBe(true);
});
@@ -235,8 +246,7 @@ test('should sort selected to the top when in multi mode', async () => {
await matchOrder([selectAllOptionLabel(originalLabels.length), ...labels]),
).toBe(true);
- await type('{esc}');
- await open();
+ await reopen();
labels = labels.splice(2, 1).concat(labels);
expect(
await matchOrder([selectAllOptionLabel(originalLabels.length), ...labels]),
@@ -244,8 +254,7 @@ test('should sort selected to the top when in multi mode', async () => {
await open();
userEvent.click(await findSelectOption(labels[5]));
- await type('{esc}');
- await open();
+ await reopen();
labels = [labels.splice(0, 1)[0], labels.splice(4, 1)[0]].concat(labels);
expect(
await matchOrder([selectAllOptionLabel(originalLabels.length), ...labels]),
@@ -253,8 +262,7 @@ test('should sort selected to the top when in multi mode', async () => {
// should revert to original order
clearAll();
- await type('{esc}');
- await open();
+ await reopen();
expect(
await matchOrder([
selectAllOptionLabel(originalLabels.length),
@@ -276,12 +284,14 @@ test('searches for label or value', async () => {
test('search order exact and startWith match first', async () => {
render(<Select {...defaultProps} />);
await type('Her');
- const options = await findAllSelectOptions();
- expect(options.length).toBe(4);
- expect(options[0]?.textContent).toEqual('Her');
- expect(options[1]?.textContent).toEqual('Herme');
- expect(options[2]?.textContent).toEqual('Cher');
- expect(options[3]?.textContent).toEqual('Guilherme');
+ await waitFor(() => {
+ const options = getAllSelectOptions();
+ expect(options.length).toBe(4);
+ expect(options[0]?.textContent).toEqual('Her');
+ expect(options[1]?.textContent).toEqual('Herme');
+ expect(options[2]?.textContent).toEqual('Cher');
+ expect(options[3]?.textContent).toEqual('Guilherme');
+ });
});
test('ignores case when searching', async () => {
@@ -303,12 +313,14 @@ test('same case should be ranked to the top', async () => {
/>,
);
await type('Ac');
- const options = await findAllSelectOptions();
- expect(options.length).toBe(4);
- expect(options[0]?.textContent).toEqual('acbc');
- expect(options[1]?.textContent).toEqual('CAc');
- expect(options[2]?.textContent).toEqual('abac');
- expect(options[3]?.textContent).toEqual('Cac');
+ await waitFor(() => {
+ const options = getAllSelectOptions();
+ expect(options.length).toBe(4);
+ expect(options[0]?.textContent).toEqual('acbc');
+ expect(options[1]?.textContent).toEqual('CAc');
+ expect(options[2]?.textContent).toEqual('abac');
+ expect(options[3]?.textContent).toEqual('Cac');
+ });
});
test('ignores special keys when searching', async () => {
@@ -338,7 +350,13 @@ test('searches for custom fields', async () => {
test('removes duplicated values', async () => {
render(<Select {...defaultProps} mode="multiple" allowNewOptions />);
- await type('a,b,b,b,c,d,d');
+ const input = getElementByClassName('.ant-select-selection-search-input');
+ const paste = createEvent.paste(input, {
+ clipboardData: {
+ getData: () => 'a,b,b,b,c,d,d',
+ },
+ });
+ fireEvent(input, paste);
const values = await findAllSelectValues();
expect(values.length).toBe(4);
expect(values[0]).toHaveTextContent('a');
@@ -519,7 +537,9 @@ test('does not show "No data" when allowNewOptions is true and a new option is e
render(<Select {...defaultProps} allowNewOptions />);
await open();
await type(NEW_OPTION);
- expect(screen.queryByText(NO_DATA)).not.toBeInTheDocument();
+ await waitFor(() =>
+ expect(screen.queryByText(NO_DATA)).not.toBeInTheDocument(),
+ );
});
test('does not show "Loading..." when allowNewOptions is false and a new option is entered', async () => {
@@ -625,9 +645,11 @@ test('does not render "Select all" when searching', async () => {
render(<Select {...defaultProps} options={OPTIONS} mode="multiple" />);
await open();
await type('Select');
- expect(
- screen.queryByText(selectAllOptionLabel(OPTIONS.length)),
- ).not.toBeInTheDocument();
+ await waitFor(() =>
+ expect(
+ screen.queryByText(selectAllOptionLabel(OPTIONS.length)),
+ ).not.toBeInTheDocument(),
+ );
});
test('does not render "Select all" as one of the tags after selection', async () => {
@@ -707,6 +729,24 @@ test('deselecting a value also deselects "Select all"', async () => {
expect(values[0]).not.toHaveTextContent(selectAllOptionLabel(10));
});
+test('deselecting a new value also removes it from the options', async () => {
+ render(
+ <Select
+ {...defaultProps}
+ options={OPTIONS.slice(0, 10)}
+ mode="multiple"
+ allowNewOptions
+ />,
+ );
+ await open();
+ await type(NEW_OPTION);
+ expect(await findSelectOption(NEW_OPTION)).toBeInTheDocument();
+ await type('{enter}');
+ clearTypedText();
+ userEvent.click(await findSelectOption(NEW_OPTION));
+ expect(await querySelectOption(NEW_OPTION)).not.toBeInTheDocument();
+});
+
test('selecting all values also selects "Select all"', async () => {
render(
<Select
@@ -805,10 +845,10 @@ test('"Select All" is checked when unchecking a newly added option and all the o
userEvent.click(await findSelectOption(selectAllOptionLabel(10)));
expect(await findSelectOption(selectAllOptionLabel(10))).toBeInTheDocument();
// add a new option
- await type(`${NEW_OPTION}{enter}`);
+ await type(NEW_OPTION);
+ expect(await findSelectOption(NEW_OPTION)).toBeInTheDocument();
clearTypedText();
expect(await findSelectOption(selectAllOptionLabel(11))).toBeInTheDocument();
- expect(await findSelectOption(NEW_OPTION)).toBeInTheDocument();
// select all should be selected
let values = await findAllCheckedValues();
expect(values[0]).toHaveTextContent(selectAllOptionLabel(11));
@@ -834,10 +874,18 @@ test('does not render "Select All" when there are 0 or 1 options', async () => {
allowNewOptions
/>,
);
+ await open();
expect(screen.queryByText(selectAllOptionLabel(1))).not.toBeInTheDocument();
- await type(`${NEW_OPTION}{enter}`);
- clearTypedText();
- expect(screen.queryByText(selectAllOptionLabel(2))).toBeInTheDocument();
+ rerender(
+ <Select
+ {...defaultProps}
+ options={OPTIONS.slice(0, 2)}
+ mode="multiple"
+ allowNewOptions
+ />,
+ );
+ await open();
+ expect(screen.getByText(selectAllOptionLabel(2))).toBeInTheDocument();
});
test('do not count unselected disabled options in "Select All"', async () => {
@@ -909,6 +957,21 @@ test('does not fire onChange when searching but no selection', async () => {
expect(onChange).toHaveBeenCalledTimes(1);
});
+test('does not duplicate options when using numeric values', async () => {
+ render(
+ <Select
+ {...defaultProps}
+ mode="multiple"
+ options={[
+ { label: '1', value: 1 },
+ { label: '2', value: 2 },
+ ]}
+ />,
+ );
+ await type('1');
+ await waitFor(() => expect(getAllSelectOptions().length).toBe(1));
+});
+
/*
TODO: Add tests that require scroll interaction. Needs further investigation.
- Fetches more data when scrolling and more data is available
diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx
index d9b022224c..770ef1df4a 100644
--- a/superset-frontend/src/components/Select/Select.tsx
+++ b/superset-frontend/src/components/Select/Select.tsx
@@ -24,6 +24,7 @@ import React, {
useMemo,
useState,
useCallback,
+ ClipboardEvent,
} from 'react';
import {
ensureIsArray,
@@ -33,7 +34,8 @@ import {
usePrevious,
} from '@superset-ui/core';
import AntdSelect, { LabeledValue as AntdLabeledValue } from 'antd/lib/select';
-import { isEqual } from 'lodash';
+import { debounce, isEqual, uniq } from 'lodash';
+import { FAST_DEBOUNCE } from 'src/constants';
import {
getValue,
hasOption,
@@ -50,7 +52,7 @@ import {
mapOptions,
hasCustomLabels,
} from './utils';
-import { SelectOptionsType, SelectProps } from './types';
+import { RawValue, SelectOptionsType, SelectProps } from './types';
import {
StyledCheckOutlined,
StyledContainer,
@@ -103,13 +105,14 @@ const Select = forwardRef(
onClear,
onDropdownVisibleChange,
onDeselect,
+ onSearch,
onSelect,
optionFilterProps = ['label', 'value'],
options,
placeholder = t('Select ...'),
showSearch = true,
sortComparator = DEFAULT_SORT_COMPARATOR,
- tokenSeparators,
+ tokenSeparators = TOKEN_SEPARATORS,
value,
getPopupContainer,
oneLine,
@@ -141,11 +144,7 @@ const Select = forwardRef(
}
}, [isDropdownVisible, oneLine]);
- const mappedMode = isSingleMode
- ? undefined
- : allowNewOptions
- ? 'tags'
- : 'multiple';
+ const mappedMode = isSingleMode ? undefined : 'multiple';
const { Option } = AntdSelect;
@@ -167,8 +166,7 @@ const Select = forwardRef(
);
const initialOptions = useMemo(
- () =>
- options && Array.isArray(options) ? options.slice() : EMPTY_OPTIONS,
+ () => (Array.isArray(options) ? options.slice() : EMPTY_OPTIONS),
[options],
);
const initialOptionsSorted = useMemo(
@@ -210,13 +208,13 @@ const Select = forwardRef(
() =>
!isSingleMode &&
allowSelectAll &&
- selectOptions.length > 0 &&
+ fullSelectOptions.length > 0 &&
enabledOptions.length > 1 &&
!inputValue,
[
isSingleMode,
allowSelectAll,
- selectOptions.length,
+ fullSelectOptions.length,
enabledOptions.length,
inputValue,
],
@@ -295,24 +293,30 @@ const Select = forwardRef(
element => getValue(element) !== getValue(value),
);
// if this was not a new item, deselect select all option
- if (
- selectAllMode &&
- selectOptions.some(opt => opt.value === getValue(value))
- ) {
+ if (selectAllMode && !option.isNewOption) {
array = array.filter(
element => getValue(element) !== SELECT_ALL_VALUE,
);
}
setSelectValue(array);
+
+ // removes new option
+ if (option.isNewOption) {
+ setSelectOptions(
+ fullSelectOptions.filter(
+ option => getValue(option.value) !== getValue(value),
+ ),
+ );
+ }
}
}
fireOnChange();
onDeselect?.(value, option);
};
- const handleOnSearch = (search: string) => {
+ const handleOnSearch = debounce((search: string) => {
const searchValue = search.trim();
- if (allowNewOptions && isSingleMode) {
+ if (allowNewOptions) {
const newOption = searchValue &&
!hasOption(searchValue, fullSelectOptions, true) && {
label: searchValue,
@@ -328,7 +332,10 @@ const Select = forwardRef(
setSelectOptions(newOptions);
}
setInputValue(searchValue);
- };
+ onSearch?.(searchValue);
+ }, FAST_DEBOUNCE);
+
+ useEffect(() => () => handleOnSearch.cancel(), [handleOnSearch]);
const handleFilterOption = (search: string, option: AntdLabeledValue) =>
handleFilterOptionHelper(search, option, optionFilterProps, filterOption);
@@ -390,10 +397,7 @@ const Select = forwardRef(
setSelectValue(
labelInValue
? ([...ensureIsArray(value), selectAllOption] as AntdLabeledValue[])
- : ([
- ...ensureIsArray(value),
- SELECT_ALL_VALUE,
- ] as AntdLabeledValue[]),
+ : ([...ensureIsArray(value), SELECT_ALL_VALUE] as RawValue[]),
);
}
}, [labelInValue, selectAllEligible.length, selectAllEnabled, value]);
@@ -429,19 +433,7 @@ const Select = forwardRef(
);
const handleOnBlur = (event: React.FocusEvent<HTMLElement>) => {
- const tagsMode = !isSingleMode && allowNewOptions;
- const searchValue = inputValue.trim();
- // Searched values will be autoselected during onBlur events when in tags mode.
- // We want to make sure a value is only selected if the user has actually selected it
- // by pressing Enter or clicking on it.
- if (
- tagsMode &&
- searchValue &&
- !hasOption(searchValue, selectValue, true)
- ) {
- // The search value will be added so we revert to the previous value
- setSelectValue(selectValue || []);
- }
+ setInputValue('');
onBlur?.(event);
};
@@ -538,6 +530,32 @@ const Select = forwardRef(
actualMaxTagCount -= 1;
}
+ const onPaste = (e: ClipboardEvent<HTMLInputElement>) => {
+ const pastedText = e.clipboardData.getData('text');
+ if (isSingleMode) {
+ setSelectValue(
+ labelInValue ? { label: pastedText, value: pastedText } : pastedText,
+ );
+ } else {
+ const token = tokenSeparators.find(token => pastedText.includes(token));
+ const array = token ? uniq(pastedText.split(token)) : [pastedText];
+ if (labelInValue) {
+ setSelectValue(previous => [
+ ...((previous || []) as AntdLabeledValue[]),
+ ...array.map<AntdLabeledValue>(value => ({
+ label: value,
+ value,
+ })),
+ ]);
+ } else {
+ setSelectValue(previous => [
+ ...((previous || []) as string[]),
+ ...array,
+ ]);
+ }
+ }
+ };
+
return (
<StyledContainer headerPosition={headerPosition}>
{header && (
@@ -562,6 +580,8 @@ const Select = forwardRef(
onBlur={handleOnBlur}
onDeselect={handleOnDeselect}
onDropdownVisibleChange={handleOnDropdownVisibleChange}
+ // @ts-ignore
+ onPaste={onPaste}
onPopupScroll={undefined}
onSearch={shouldShowSearch ? handleOnSearch : undefined}
onSelect={handleOnSelect}
@@ -569,7 +589,7 @@ const Select = forwardRef(
placeholder={placeholder}
showSearch={shouldShowSearch}
showArrow
- tokenSeparators={tokenSeparators || TOKEN_SEPARATORS}
+ tokenSeparators={tokenSeparators}
value={selectValue}
suffixIcon={getSuffixIcon(
isLoading,
@@ -583,7 +603,7 @@ const Select = forwardRef(
<StyledCheckOutlined iconSize="m" aria-label="check" />
)
}
- {...(!shouldRenderChildrenOptions && { options: fullSelectOptions })}
+ options={shouldRenderChildrenOptions ? undefined : fullSelectOptions}
oneLine={oneLine}
tagRender={customTagRender}
{...props}
[superset] 04/06: fix: Tooltips don't disappear on the Heatmap chart (#24959)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 5c931b19518e0cea0695f6c92f8403c9f6891a2a
Author: Michael S. Molina <70...@users.noreply.github.com>
AuthorDate: Fri Aug 11 13:40:45 2023 -0300
fix: Tooltips don't disappear on the Heatmap chart (#24959)
(cherry picked from commit 97034901291420af844257fc76ac107d4a891f18)
---
.../plugins/legacy-plugin-chart-heatmap/src/ReactHeatmap.jsx | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/superset-frontend/plugins/legacy-plugin-chart-heatmap/src/ReactHeatmap.jsx b/superset-frontend/plugins/legacy-plugin-chart-heatmap/src/ReactHeatmap.jsx
index 22dde813b6..6e016b4774 100644
--- a/superset-frontend/plugins/legacy-plugin-chart-heatmap/src/ReactHeatmap.jsx
+++ b/superset-frontend/plugins/legacy-plugin-chart-heatmap/src/ReactHeatmap.jsx
@@ -21,7 +21,12 @@ import { reactify, css, styled } from '@superset-ui/core';
import { Global } from '@emotion/react';
import Component from './Heatmap';
-const ReactComponent = reactify(Component);
+function componentWillUnmount() {
+ // Removes tooltips from the DOM
+ document.querySelectorAll('.d3-tip').forEach(t => t.remove());
+}
+
+const ReactComponent = reactify(Component, { componentWillUnmount });
const Heatmap = ({ className, ...otherProps }) => (
<div className={className}>