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/14 12:35:16 UTC
(superset) branch 4.0 updated (5b6e5e475e -> 017e0fc733)
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 5b6e5e475e fix: missing shared color in mixed timeseries (#27403)
new 15557f493d fix(deps): resolving canvg and html2canvas module not found (#27315)
new 759863553d fix(dashboard): Only fetch CSS templates for dashboard header menu when in edit mode (#27411)
new 131c254fe7 fix: SSH Tunnel configuration settings (#27186)
new 0ee452e9f9 fix(webpack): remove double-dotted file extensions in webpack config (#27471)
new 017e0fc733 fix: check if guest user modified query (#27484)
The 5 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:
superset-frontend/package-lock.json | 40 +---
.../superset-ui-core/src/ui-overrides/types.ts | 18 +-
.../components/CssEditor/CssEditor.test.tsx | 54 +++---
.../src/dashboard/components/CssEditor/index.jsx | 25 ++-
.../Header/HeaderActionsDropdown/index.jsx | 22 +--
.../src/features/alerts/AlertReportModal.test.tsx | 4 +-
.../DatabaseConnectionForm/CommonParameters.tsx | 35 +---
.../DatabaseConnectionForm/EncryptedField.tsx | 2 +-
.../DatabaseConnectionForm/TableCatalog.tsx | 3 +-
.../DatabaseConnectionForm/ValidatedInputField.tsx | 2 +-
.../DatabaseModal/DatabaseConnectionForm/index.tsx | 130 ++++++-------
.../databases/DatabaseModal/SSHTunnelForm.tsx | 12 +-
.../DatabaseModal/SSHTunnelSwitch.test.tsx | 162 +++++++++++++++++
.../databases/DatabaseModal/SSHTunnelSwitch.tsx | 82 ++++++---
.../databases/DatabaseModal/index.test.tsx | 11 +-
.../src/features/databases/DatabaseModal/index.tsx | 132 ++++++++------
superset-frontend/src/features/databases/types.ts | 80 +++++++-
superset-frontend/src/views/CRUD/hooks.ts | 7 +-
superset-frontend/webpack.config.js | 4 +-
superset/commands/database/create.py | 10 +-
superset/commands/database/ssh_tunnel/create.py | 11 ++
.../commands/database/ssh_tunnel/exceptions.py | 4 +
superset/commands/database/ssh_tunnel/update.py | 25 ++-
superset/commands/database/test_connection.py | 45 +++--
superset/commands/database/update.py | 79 +++++---
superset/databases/api.py | 7 +-
superset/security/manager.py | 85 ++++++---
tests/integration_tests/databases/api_tests.py | 201 +++++++++++++++++++++
.../databases/ssh_tunnel/commands/create_test.py | 45 ++++-
.../databases/ssh_tunnel/commands/update_test.py | 35 +++-
tests/unit_tests/security/manager_test.py | 112 ++++++++++--
31 files changed, 1094 insertions(+), 390 deletions(-)
create mode 100644 superset-frontend/src/features/databases/DatabaseModal/SSHTunnelSwitch.test.tsx
(superset) 05/05: fix: check if guest user modified query (#27484)
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 017e0fc733597af91f94cce43008db222690d88b
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Tue Mar 12 21:28:06 2024 -0400
fix: check if guest user modified query (#27484)
(cherry picked from commit 735b895dd5e409bfc95406e847a82fd786d93a1d)
---
superset/security/manager.py | 85 +++++++++++++++++------
tests/unit_tests/security/manager_test.py | 112 +++++++++++++++++++++++++-----
2 files changed, 156 insertions(+), 41 deletions(-)
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 83e12fb2dc..d56c0ad688 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -143,6 +143,56 @@ RoleModelView.edit_columns = ["name", "permissions", "user"]
RoleModelView.related_views = []
+def query_context_modified(query_context: "QueryContext") -> bool:
+ """
+ Check if a query context has been modified.
+
+ This is used to ensure guest users don't modify the payload and fetch data
+ different from what was shared with them in dashboards.
+ """
+ form_data = query_context.form_data
+ stored_chart = query_context.slice_
+
+ # sanity checks
+ if form_data is None or stored_chart is None:
+ return True
+
+ # cannot request a different chart
+ if form_data.get("slice_id") != stored_chart.id:
+ return True
+
+ # compare form_data
+ requested_metrics = {
+ frozenset(metric.items()) if isinstance(metric, dict) else metric
+ for metric in form_data.get("metrics") or []
+ }
+ stored_metrics = {
+ frozenset(metric.items()) if isinstance(metric, dict) else metric
+ for metric in stored_chart.params_dict.get("metrics") or []
+ }
+ if not requested_metrics.issubset(stored_metrics):
+ return True
+
+ # compare queries in query_context
+ queries_metrics = {
+ frozenset(metric.items()) if isinstance(metric, dict) else metric
+ for query in query_context.queries
+ for metric in query.metrics or []
+ }
+
+ if stored_chart.query_context:
+ stored_query_context = json.loads(cast(str, stored_chart.query_context))
+ for query in stored_query_context.get("queries") or []:
+ stored_metrics.update(
+ {
+ frozenset(metric.items()) if isinstance(metric, dict) else metric
+ for metric in query.get("metrics") or []
+ }
+ )
+
+ return not queries_metrics.issubset(stored_metrics)
+
+
class SupersetSecurityManager( # pylint: disable=too-many-public-methods
SecurityManager
):
@@ -1941,29 +1991,20 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
self.get_table_access_error_object(denied)
)
- if self.is_guest_user() and query_context:
- # Guest users MUST not modify the payload so it's requesting a different
- # chart or different ad-hoc metrics from what's saved.
- form_data = query_context.form_data
- stored_chart = query_context.slice_
-
- if (
- form_data is None
- or stored_chart is None
- or form_data.get("slice_id") != stored_chart.id
- or form_data.get("metrics", []) != stored_chart.params_dict["metrics"]
- or any(
- query.metrics != stored_chart.params_dict["metrics"]
- for query in query_context.queries
- )
- ):
- raise SupersetSecurityException(
- SupersetError(
- error_type=SupersetErrorType.DASHBOARD_SECURITY_ACCESS_ERROR,
- message=_("Guest user cannot modify chart payload"),
- level=ErrorLevel.ERROR,
- )
+ # Guest users MUST not modify the payload so it's requesting a
+ # different chart or different ad-hoc metrics from what's saved.
+ if (
+ query_context
+ and self.is_guest_user()
+ and query_context_modified(query_context)
+ ):
+ raise SupersetSecurityException(
+ SupersetError(
+ error_type=SupersetErrorType.DASHBOARD_SECURITY_ACCESS_ERROR,
+ message=_("Guest user cannot modify chart payload"),
+ level=ErrorLevel.ERROR,
)
+ )
if datasource or query_context or viz:
form_data = None
diff --git a/tests/unit_tests/security/manager_test.py b/tests/unit_tests/security/manager_test.py
index 7d2b9153a3..5a06013a68 100644
--- a/tests/unit_tests/security/manager_test.py
+++ b/tests/unit_tests/security/manager_test.py
@@ -15,6 +15,8 @@
# specific language governing permissions and limitations
# under the License.
+# pylint: disable=invalid-name, unused-argument, redefined-outer-name
+
import pytest
from flask_appbuilder.security.sqla.models import Role, User
from pytest_mock import MockFixture
@@ -25,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.superset_typing import AdhocMetric
from superset.utils.core import override_user
@@ -36,12 +39,29 @@ def test_security_manager(app_context: None) -> None:
assert sm
-def test_raise_for_access_guest_user(
+@pytest.fixture
+def stored_metrics() -> list[AdhocMetric]:
+ """
+ Return a list of metrics.
+ """
+ return [
+ {
+ "column": None,
+ "expressionType": "SQL",
+ "hasCustomLabel": False,
+ "label": "COUNT(*) + 1",
+ "sqlExpression": "COUNT(*) + 1",
+ },
+ ]
+
+
+def test_raise_for_access_guest_user_ok(
mocker: MockFixture,
app_context: None,
+ stored_metrics: list[AdhocMetric],
) -> None:
"""
- Test that guest user can't modify chart payload.
+ Test that guest user can submit an unmodified chart payload.
"""
sm = SupersetSecurityManager(appbuilder)
mocker.patch.object(sm, "is_guest_user", return_value=True)
@@ -49,23 +69,11 @@ def test_raise_for_access_guest_user(
query_context = mocker.MagicMock()
query_context.slice_.id = 42
- stored_metrics = [
- {
- "aggregate": None,
- "column": None,
- "datasourceWarning": False,
- "expressionType": "SQL",
- "hasCustomLabel": False,
- "label": "COUNT(*) + 1",
- "optionName": "metric_ssa1gwimio_cxpyjc7vj3s",
- "sqlExpression": "COUNT(*) + 1",
- }
- ]
+ query_context.slice_.query_context = None
query_context.slice_.params_dict = {
"metrics": stored_metrics,
}
- # normal request
query_context.form_data = {
"slice_id": 42,
"metrics": stored_metrics,
@@ -73,7 +81,26 @@ def test_raise_for_access_guest_user(
query_context.queries = [QueryObject(metrics=stored_metrics)] # type: ignore
sm.raise_for_access(query_context=query_context)
- # tampered requests
+
+def test_raise_for_access_guest_user_tampered_id(
+ mocker: MockFixture,
+ app_context: None,
+ stored_metrics: list[AdhocMetric],
+) -> None:
+ """
+ Test that guest user cannot modify the chart ID.
+ """
+ sm = SupersetSecurityManager(appbuilder)
+ mocker.patch.object(sm, "is_guest_user", return_value=True)
+ mocker.patch.object(sm, "can_access", return_value=True)
+
+ query_context = mocker.MagicMock()
+ query_context.slice_.id = 42
+ query_context.slice_.query_context = None
+ query_context.slice_.params_dict = {
+ "metrics": stored_metrics,
+ }
+
query_context.form_data = {
"slice_id": 43,
"metrics": stored_metrics,
@@ -82,15 +109,32 @@ def test_raise_for_access_guest_user(
with pytest.raises(SupersetSecurityException):
sm.raise_for_access(query_context=query_context)
+
+def test_raise_for_access_guest_user_tampered_form_data(
+ mocker: MockFixture,
+ app_context: None,
+ stored_metrics: list[AdhocMetric],
+) -> None:
+ """
+ Test that guest user cannot modify metrics in the form data.
+ """
+ sm = SupersetSecurityManager(appbuilder)
+ mocker.patch.object(sm, "is_guest_user", return_value=True)
+ mocker.patch.object(sm, "can_access", return_value=True)
+
+ query_context = mocker.MagicMock()
+ query_context.slice_.id = 42
+ query_context.slice_.query_context = None
+ query_context.slice_.params_dict = {
+ "metrics": stored_metrics,
+ }
+
tampered_metrics = [
{
- "aggregate": None,
"column": None,
- "datasourceWarning": False,
"expressionType": "SQL",
"hasCustomLabel": False,
"label": "COUNT(*) + 2",
- "optionName": "metric_ssa1gwimio_cxpyjc7vj3s",
"sqlExpression": "COUNT(*) + 2",
}
]
@@ -102,6 +146,36 @@ def test_raise_for_access_guest_user(
with pytest.raises(SupersetSecurityException):
sm.raise_for_access(query_context=query_context)
+
+def test_raise_for_access_guest_user_tampered_queries(
+ mocker: MockFixture,
+ app_context: None,
+ stored_metrics: list[AdhocMetric],
+) -> None:
+ """
+ Test that guest user cannot modify metrics in the queries.
+ """
+ sm = SupersetSecurityManager(appbuilder)
+ mocker.patch.object(sm, "is_guest_user", return_value=True)
+ mocker.patch.object(sm, "can_access", return_value=True)
+
+ query_context = mocker.MagicMock()
+ query_context.slice_.id = 42
+ query_context.slice_.query_context = None
+ query_context.slice_.params_dict = {
+ "metrics": stored_metrics,
+ }
+
+ tampered_metrics = [
+ {
+ "column": None,
+ "expressionType": "SQL",
+ "hasCustomLabel": False,
+ "label": "COUNT(*) + 2",
+ "sqlExpression": "COUNT(*) + 2",
+ }
+ ]
+
query_context.form_data = {
"slice_id": 42,
"metrics": stored_metrics,
(superset) 03/05: fix: SSH Tunnel configuration settings (#27186)
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 131c254fe786306d4f79fc59cd42dc5a6efd0d54
Author: Geido <60...@users.noreply.github.com>
AuthorDate: Mon Mar 11 16:56:54 2024 +0100
fix: SSH Tunnel configuration settings (#27186)
(cherry picked from commit 89e89de341c555a1fdbe9d3f5bccada58eb08059)
---
.../superset-ui-core/src/ui-overrides/types.ts | 18 +-
.../src/features/alerts/AlertReportModal.test.tsx | 4 +-
.../DatabaseConnectionForm/CommonParameters.tsx | 35 +---
.../DatabaseConnectionForm/EncryptedField.tsx | 2 +-
.../DatabaseConnectionForm/TableCatalog.tsx | 3 +-
.../DatabaseConnectionForm/ValidatedInputField.tsx | 2 +-
.../DatabaseModal/DatabaseConnectionForm/index.tsx | 130 ++++++-------
.../databases/DatabaseModal/SSHTunnelForm.tsx | 12 +-
.../DatabaseModal/SSHTunnelSwitch.test.tsx | 162 +++++++++++++++++
.../databases/DatabaseModal/SSHTunnelSwitch.tsx | 82 ++++++---
.../databases/DatabaseModal/index.test.tsx | 11 +-
.../src/features/databases/DatabaseModal/index.tsx | 132 ++++++++------
superset-frontend/src/features/databases/types.ts | 80 +++++++-
superset-frontend/src/views/CRUD/hooks.ts | 7 +-
superset/commands/database/create.py | 10 +-
superset/commands/database/ssh_tunnel/create.py | 11 ++
.../commands/database/ssh_tunnel/exceptions.py | 4 +
superset/commands/database/ssh_tunnel/update.py | 25 ++-
superset/commands/database/test_connection.py | 45 +++--
superset/commands/database/update.py | 79 +++++---
superset/databases/api.py | 7 +-
tests/integration_tests/databases/api_tests.py | 201 +++++++++++++++++++++
.../databases/ssh_tunnel/commands/create_test.py | 45 ++++-
.../databases/ssh_tunnel/commands/update_test.py | 35 +++-
24 files changed, 871 insertions(+), 271 deletions(-)
diff --git a/superset-frontend/packages/superset-ui-core/src/ui-overrides/types.ts b/superset-frontend/packages/superset-ui-core/src/ui-overrides/types.ts
index 45ec06e90e..60598bd4e1 100644
--- a/superset-frontend/packages/superset-ui-core/src/ui-overrides/types.ts
+++ b/superset-frontend/packages/superset-ui-core/src/ui-overrides/types.ts
@@ -44,15 +44,15 @@ interface MenuObjectChildProps {
disable?: boolean;
}
-export interface SwitchProps {
- isEditMode: boolean;
- dbFetched: any;
- disableSSHTunnelingForEngine?: boolean;
- useSSHTunneling: boolean;
- setUseSSHTunneling: React.Dispatch<React.SetStateAction<boolean>>;
- setDB: React.Dispatch<any>;
- isSSHTunneling: boolean;
-}
+// loose typing to avoid any circular dependencies
+// refer to SSHTunnelSwitch component for strict typing
+type SwitchProps = {
+ db: object;
+ changeMethods: {
+ onParametersChange: (event: any) => void;
+ };
+ clearValidationErrors: () => void;
+};
type ConfigDetailsProps = {
embeddedId: string;
diff --git a/superset-frontend/src/features/alerts/AlertReportModal.test.tsx b/superset-frontend/src/features/alerts/AlertReportModal.test.tsx
index ee9504286d..358aa27df3 100644
--- a/superset-frontend/src/features/alerts/AlertReportModal.test.tsx
+++ b/superset-frontend/src/features/alerts/AlertReportModal.test.tsx
@@ -541,8 +541,8 @@ test('defaults to day when CRON is not selected', async () => {
useRedux: true,
});
userEvent.click(screen.getByTestId('schedule-panel'));
- const days = screen.getAllByTitle(/day/i, { exact: true });
- expect(days.length).toBe(2);
+ const day = screen.getByText('day');
+ expect(day).toBeInTheDocument();
});
// Notification Method Section
diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx
index 7b52eab26c..3f1f5f9625 100644
--- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx
+++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx
@@ -17,12 +17,11 @@
* under the License.
*/
import React from 'react';
-import { isEmpty } from 'lodash';
import { SupersetTheme, t } from '@superset-ui/core';
import { AntdSwitch } from 'src/components';
import InfoTooltip from 'src/components/InfoTooltip';
import ValidatedInput from 'src/components/Form/LabeledErrorBoundInput';
-import { FieldPropTypes } from '.';
+import { FieldPropTypes } from '../../types';
import { toggleStyle, infoTooltip } from '../styles';
export const hostField = ({
@@ -252,35 +251,3 @@ export const forceSSLField = ({
/>
</div>
);
-
-export const SSHTunnelSwitch = ({
- isEditMode,
- changeMethods,
- clearValidationErrors,
- db,
-}: FieldPropTypes) => (
- <div css={(theme: SupersetTheme) => infoTooltip(theme)}>
- <AntdSwitch
- disabled={isEditMode && !isEmpty(db?.ssh_tunnel)}
- checked={db?.parameters?.ssh}
- onChange={changed => {
- changeMethods.onParametersChange({
- target: {
- type: 'toggle',
- name: 'ssh',
- checked: true,
- value: changed,
- },
- });
- clearValidationErrors();
- }}
- data-test="ssh-tunnel-switch"
- />
- <span css={toggleStyle}>{t('SSH Tunnel')}</span>
- <InfoTooltip
- tooltip={t('SSH Tunnel configuration parameters')}
- placement="right"
- viewBox="0 -5 24 24"
- />
- </div>
-);
diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx
index c5e268e569..009afc84ef 100644
--- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx
+++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx
@@ -22,7 +22,7 @@ import { AntdButton, AntdSelect } from 'src/components';
import InfoTooltip from 'src/components/InfoTooltip';
import FormLabel from 'src/components/Form/FormLabel';
import Icons from 'src/components/Icons';
-import { FieldPropTypes } from '.';
+import { FieldPropTypes } from '../../types';
import { infoTooltip, labelMarginBottom, CredentialInfoForm } from '../styles';
enum CredentialInfoOptions {
diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/TableCatalog.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/TableCatalog.tsx
index ed5cc94903..47a0ec1579 100644
--- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/TableCatalog.tsx
+++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/TableCatalog.tsx
@@ -21,9 +21,8 @@ import { css, SupersetTheme, t } from '@superset-ui/core';
import ValidatedInput from 'src/components/Form/LabeledErrorBoundInput';
import FormLabel from 'src/components/Form/FormLabel';
import Icons from 'src/components/Icons';
-import { FieldPropTypes } from '.';
import { StyledFooterButton, StyledCatalogTable } from '../styles';
-import { CatalogObject } from '../../types';
+import { CatalogObject, FieldPropTypes } from '../../types';
export const TableCatalog = ({
required,
diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/ValidatedInputField.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/ValidatedInputField.tsx
index ec2e239ac4..d6794f9a21 100644
--- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/ValidatedInputField.tsx
+++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/ValidatedInputField.tsx
@@ -19,7 +19,7 @@
import React from 'react';
import { t } from '@superset-ui/core';
import ValidatedInput from 'src/components/Form/LabeledErrorBoundInput';
-import { FieldPropTypes } from '.';
+import { FieldPropTypes } from '../../types';
const FIELD_TEXT_MAP = {
account: {
diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx
index e747b3c895..fc076b624f 100644
--- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx
+++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx
@@ -17,7 +17,11 @@
* under the License.
*/
import React, { FormEvent } from 'react';
-import { SupersetTheme, JsonObject } from '@superset-ui/core';
+import {
+ SupersetTheme,
+ JsonObject,
+ getExtensionsRegistry,
+} from '@superset-ui/core';
import { InputProps } from 'antd/lib/input';
import { Form } from 'src/components/Form';
import {
@@ -31,13 +35,13 @@ import {
portField,
queryField,
usernameField,
- SSHTunnelSwitch,
} from './CommonParameters';
import { validatedInputField } from './ValidatedInputField';
import { EncryptedField } from './EncryptedField';
import { TableCatalog } from './TableCatalog';
import { formScrollableStyles, validatedFormStyles } from '../styles';
import { DatabaseForm, DatabaseObject } from '../../types';
+import SSHTunnelSwitch from '../SSHTunnelSwitch';
export const FormFieldOrder = [
'host',
@@ -59,34 +63,10 @@ export const FormFieldOrder = [
'ssh',
];
-export interface FieldPropTypes {
- required: boolean;
- hasTooltip?: boolean;
- tooltipText?: (value: any) => string;
- placeholder?: string;
- onParametersChange: (value: any) => string;
- onParametersUploadFileChange: (value: any) => string;
- changeMethods: { onParametersChange: (value: any) => string } & {
- onChange: (value: any) => string;
- } & {
- onQueryChange: (value: any) => string;
- } & { onParametersUploadFileChange: (value: any) => string } & {
- onAddTableCatalog: () => void;
- onRemoveTableCatalog: (idx: number) => void;
- } & {
- onExtraInputChange: (value: any) => void;
- onSSHTunnelParametersChange: (value: any) => string;
- };
- validationErrors: JsonObject | null;
- getValidation: () => void;
- clearValidationErrors: () => void;
- db?: DatabaseObject;
- field: string;
- isEditMode?: boolean;
- sslForced?: boolean;
- defaultDBName?: string;
- editNewDb?: boolean;
-}
+const extensionsRegistry = getExtensionsRegistry();
+
+const SSHTunnelSwitchComponent =
+ extensionsRegistry.get('ssh_tunnel.form.switch') ?? SSHTunnelSwitch;
const FORM_FIELD_MAP = {
host: hostField,
@@ -105,7 +85,7 @@ const FORM_FIELD_MAP = {
warehouse: validatedInputField,
role: validatedInputField,
account: validatedInputField,
- ssh: SSHTunnelSwitch,
+ ssh: SSHTunnelSwitchComponent,
};
interface DatabaseConnectionFormProps {
@@ -138,7 +118,7 @@ interface DatabaseConnectionFormProps {
}
const DatabaseConnectionForm = ({
- dbModel: { parameters },
+ dbModel,
db,
editNewDb,
getPlaceholder,
@@ -154,47 +134,51 @@ const DatabaseConnectionForm = ({
sslForced,
validationErrors,
clearValidationErrors,
-}: DatabaseConnectionFormProps) => (
- <Form>
- <div
- // @ts-ignore
- css={(theme: SupersetTheme) => [
- formScrollableStyles,
- validatedFormStyles(theme),
- ]}
- >
- {parameters &&
- FormFieldOrder.filter(
- (key: string) =>
- Object.keys(parameters.properties).includes(key) ||
- key === 'database_name',
- ).map(field =>
- FORM_FIELD_MAP[field]({
- required: parameters.required?.includes(field),
- changeMethods: {
- onParametersChange,
- onChange,
- onQueryChange,
- onParametersUploadFileChange,
- onAddTableCatalog,
- onRemoveTableCatalog,
- onExtraInputChange,
- },
- validationErrors,
- getValidation,
- clearValidationErrors,
- db,
- key: field,
- field,
- isEditMode,
- sslForced,
- editNewDb,
- placeholder: getPlaceholder ? getPlaceholder(field) : undefined,
- }),
- )}
- </div>
- </Form>
-);
+}: DatabaseConnectionFormProps) => {
+ const parameters = dbModel?.parameters;
+
+ return (
+ <Form>
+ <div
+ // @ts-ignore
+ css={(theme: SupersetTheme) => [
+ formScrollableStyles,
+ validatedFormStyles(theme),
+ ]}
+ >
+ {parameters &&
+ FormFieldOrder.filter(
+ (key: string) =>
+ Object.keys(parameters.properties).includes(key) ||
+ key === 'database_name',
+ ).map(field =>
+ FORM_FIELD_MAP[field]({
+ required: parameters.required?.includes(field),
+ changeMethods: {
+ onParametersChange,
+ onChange,
+ onQueryChange,
+ onParametersUploadFileChange,
+ onAddTableCatalog,
+ onRemoveTableCatalog,
+ onExtraInputChange,
+ },
+ validationErrors,
+ getValidation,
+ clearValidationErrors,
+ db,
+ key: field,
+ field,
+ isEditMode,
+ sslForced,
+ editNewDb,
+ placeholder: getPlaceholder ? getPlaceholder(field) : undefined,
+ }),
+ )}
+ </div>
+ </Form>
+ );
+};
export const FormFieldMap = FORM_FIELD_MAP;
export default DatabaseConnectionForm;
diff --git a/superset-frontend/src/features/databases/DatabaseModal/SSHTunnelForm.tsx b/superset-frontend/src/features/databases/DatabaseModal/SSHTunnelForm.tsx
index 7823d82faf..e0d1b16ff2 100644
--- a/superset-frontend/src/features/databases/DatabaseModal/SSHTunnelForm.tsx
+++ b/superset-frontend/src/features/databases/DatabaseModal/SSHTunnelForm.tsx
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-import React, { EventHandler, ChangeEvent, useState } from 'react';
+import React, { useState } from 'react';
import { t, styled } from '@superset-ui/core';
import { AntdForm, Col, Row } from 'src/components';
import { Form, FormLabel } from 'src/components/Form';
@@ -24,7 +24,7 @@ import { Radio } from 'src/components/Radio';
import { Input, TextArea } from 'src/components/Input';
import { Input as AntdInput, Tooltip } from 'antd';
import { EyeInvisibleOutlined, EyeOutlined } from '@ant-design/icons';
-import { DatabaseObject } from '../types';
+import { DatabaseObject, FieldPropTypes } from '../types';
import { AuthType } from '.';
const StyledDiv = styled.div`
@@ -54,9 +54,7 @@ const SSHTunnelForm = ({
setSSHTunnelLoginMethod,
}: {
db: DatabaseObject | null;
- onSSHTunnelParametersChange: EventHandler<
- ChangeEvent<HTMLInputElement | HTMLTextAreaElement>
- >;
+ onSSHTunnelParametersChange: FieldPropTypes['changeMethods']['onSSHTunnelParametersChange'];
setSSHTunnelLoginMethod: (method: AuthType) => void;
}) => {
const [usePassword, setUsePassword] = useState<AuthType>(AuthType.Password);
@@ -86,9 +84,9 @@ const SSHTunnelForm = ({
</FormLabel>
<Input
name="server_port"
- type="text"
placeholder={t('22')}
- value={db?.ssh_tunnel?.server_port || ''}
+ type="number"
+ value={db?.ssh_tunnel?.server_port}
onChange={onSSHTunnelParametersChange}
data-test="ssh-tunnel-server_port-input"
/>
diff --git a/superset-frontend/src/features/databases/DatabaseModal/SSHTunnelSwitch.test.tsx b/superset-frontend/src/features/databases/DatabaseModal/SSHTunnelSwitch.test.tsx
new file mode 100644
index 0000000000..fef205acf2
--- /dev/null
+++ b/superset-frontend/src/features/databases/DatabaseModal/SSHTunnelSwitch.test.tsx
@@ -0,0 +1,162 @@
+/**
+ * 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 React from 'react';
+import { render, screen } from 'spec/helpers/testing-library';
+import userEvent from '@testing-library/user-event';
+import SSHTunnelSwitch from './SSHTunnelSwitch';
+import { DatabaseForm, DatabaseObject } from '../types';
+
+jest.mock('@superset-ui/core', () => ({
+ ...jest.requireActual('@superset-ui/core'),
+ isFeatureEnabled: jest.fn().mockReturnValue(true),
+}));
+
+jest.mock('src/components', () => ({
+ AntdSwitch: ({
+ checked,
+ onChange,
+ }: {
+ checked: boolean;
+ onChange: (checked: boolean) => void;
+ }) => (
+ <button
+ onClick={() => onChange(!checked)}
+ aria-checked={checked}
+ role="switch"
+ type="button"
+ >
+ {checked ? 'ON' : 'OFF'}
+ </button>
+ ),
+}));
+
+const mockChangeMethods = {
+ onParametersChange: jest.fn(),
+};
+
+const mockDbModel = {
+ engine: 'mysql',
+ engine_information: {
+ disable_ssh_tunneling: false,
+ },
+} as DatabaseForm;
+
+const defaultDb = {
+ parameters: { ssh: false },
+ ssh_tunnel: {},
+ engine: 'mysql',
+} as DatabaseObject;
+
+afterEach(() => {
+ jest.clearAllMocks();
+});
+
+test('Renders SSH Tunnel switch enabled by default and toggles its state', () => {
+ render(
+ <SSHTunnelSwitch
+ changeMethods={mockChangeMethods}
+ clearValidationErrors={jest.fn}
+ db={defaultDb}
+ dbModel={mockDbModel}
+ />,
+ );
+ const switchButton = screen.getByRole('switch');
+ expect(switchButton).toHaveTextContent('OFF');
+ userEvent.click(switchButton);
+ expect(mockChangeMethods.onParametersChange).toHaveBeenCalledWith({
+ target: { type: 'toggle', name: 'ssh', checked: true, value: true },
+ });
+ expect(switchButton).toHaveTextContent('ON');
+});
+
+test('Does not render if SSH Tunnel is disabled', () => {
+ render(
+ <SSHTunnelSwitch
+ changeMethods={mockChangeMethods}
+ clearValidationErrors={jest.fn}
+ db={defaultDb}
+ dbModel={{
+ ...mockDbModel,
+ engine_information: {
+ disable_ssh_tunneling: true,
+ supports_file_upload: false,
+ },
+ }}
+ />,
+ );
+ expect(screen.queryByRole('switch')).not.toBeInTheDocument();
+});
+
+test('Checks the switch based on db.parameters.ssh', () => {
+ const dbWithSSHTunnelEnabled = {
+ ...defaultDb,
+ parameters: { ssh: true },
+ } as DatabaseObject;
+ render(
+ <SSHTunnelSwitch
+ changeMethods={mockChangeMethods}
+ clearValidationErrors={jest.fn}
+ db={dbWithSSHTunnelEnabled}
+ dbModel={mockDbModel}
+ />,
+ );
+ expect(screen.getByRole('switch')).toHaveTextContent('ON');
+});
+
+test('Calls onParametersChange with true if SSH Tunnel info exists', () => {
+ const dbWithSSHTunnelInfo = {
+ ...defaultDb,
+ parameters: { ssh: undefined },
+ ssh_tunnel: { host: 'example.com' },
+ } as DatabaseObject;
+ render(
+ <SSHTunnelSwitch
+ changeMethods={mockChangeMethods}
+ clearValidationErrors={jest.fn}
+ db={dbWithSSHTunnelInfo}
+ dbModel={mockDbModel}
+ />,
+ );
+ expect(mockChangeMethods.onParametersChange).toHaveBeenCalledWith({
+ target: { type: 'toggle', name: 'ssh', checked: true, value: true },
+ });
+});
+
+test('Displays tooltip text on hover over the InfoTooltip', async () => {
+ const tooltipText = 'SSH Tunnel configuration parameters';
+ render(
+ <SSHTunnelSwitch
+ changeMethods={mockChangeMethods}
+ clearValidationErrors={jest.fn}
+ db={defaultDb}
+ dbModel={mockDbModel}
+ />,
+ );
+
+ const infoTooltipTrigger = screen.getByRole('img', {
+ name: 'info-solid_small',
+ });
+ expect(infoTooltipTrigger).toBeInTheDocument();
+
+ userEvent.hover(infoTooltipTrigger);
+
+ const tooltip = await screen.findByText(tooltipText);
+
+ expect(tooltip).toBeInTheDocument();
+});
diff --git a/superset-frontend/src/features/databases/DatabaseModal/SSHTunnelSwitch.tsx b/superset-frontend/src/features/databases/DatabaseModal/SSHTunnelSwitch.tsx
index 388e3c83b1..cf96864a3d 100644
--- a/superset-frontend/src/features/databases/DatabaseModal/SSHTunnelSwitch.tsx
+++ b/superset-frontend/src/features/databases/DatabaseModal/SSHTunnelSwitch.tsx
@@ -16,35 +16,73 @@
* specific language governing permissions and limitations
* under the License.
*/
-import React from 'react';
-import { t, SupersetTheme, SwitchProps } from '@superset-ui/core';
+import React, { useEffect, useState } from 'react';
+import {
+ t,
+ SupersetTheme,
+ isFeatureEnabled,
+ FeatureFlag,
+} from '@superset-ui/core';
import { AntdSwitch } from 'src/components';
import InfoTooltip from 'src/components/InfoTooltip';
import { isEmpty } from 'lodash';
-import { ActionType } from '.';
import { infoTooltip, toggleStyle } from './styles';
+import { SwitchProps } from '../types';
const SSHTunnelSwitch = ({
- isEditMode,
- dbFetched,
- useSSHTunneling,
- setUseSSHTunneling,
- setDB,
- isSSHTunneling,
-}: SwitchProps) =>
- isSSHTunneling ? (
+ clearValidationErrors,
+ changeMethods,
+ db,
+ dbModel,
+}: SwitchProps) => {
+ const [isChecked, setChecked] = useState(false);
+ const sshTunnelEnabled = isFeatureEnabled(FeatureFlag.SshTunneling);
+ const disableSSHTunnelingForEngine =
+ dbModel?.engine_information?.disable_ssh_tunneling || false;
+ const isSSHTunnelEnabled = sshTunnelEnabled && !disableSSHTunnelingForEngine;
+
+ const handleOnChange = (changed: boolean) => {
+ setChecked(changed);
+ changeMethods.onParametersChange({
+ target: {
+ type: 'toggle',
+ name: 'ssh',
+ checked: true,
+ value: changed,
+ },
+ });
+ clearValidationErrors();
+ };
+
+ useEffect(() => {
+ if (isSSHTunnelEnabled && db?.parameters?.ssh !== undefined) {
+ setChecked(db.parameters.ssh);
+ }
+ }, [db?.parameters?.ssh, isSSHTunnelEnabled]);
+
+ useEffect(() => {
+ if (
+ isSSHTunnelEnabled &&
+ db?.parameters?.ssh === undefined &&
+ !isEmpty(db?.ssh_tunnel)
+ ) {
+ // reflecting the state of the ssh tunnel on first load
+ changeMethods.onParametersChange({
+ target: {
+ type: 'toggle',
+ name: 'ssh',
+ checked: true,
+ value: true,
+ },
+ });
+ }
+ }, [changeMethods, db?.parameters?.ssh, db?.ssh_tunnel, isSSHTunnelEnabled]);
+
+ return isSSHTunnelEnabled ? (
<div css={(theme: SupersetTheme) => infoTooltip(theme)}>
<AntdSwitch
- disabled={isEditMode && !isEmpty(dbFetched?.ssh_tunnel)}
- checked={useSSHTunneling}
- onChange={changed => {
- setUseSSHTunneling(changed);
- if (!changed) {
- setDB({
- type: ActionType.RemoveSSHTunnelConfig,
- });
- }
- }}
+ checked={isChecked}
+ onChange={handleOnChange}
data-test="ssh-tunnel-switch"
/>
<span css={toggleStyle}>{t('SSH Tunnel')}</span>
@@ -55,4 +93,6 @@ const SSHTunnelSwitch = ({
/>
</div>
) : null;
+};
+
export default SSHTunnelSwitch;
diff --git a/superset-frontend/src/features/databases/DatabaseModal/index.test.tsx b/superset-frontend/src/features/databases/DatabaseModal/index.test.tsx
index 0f60857f06..7e8018b25f 100644
--- a/superset-frontend/src/features/databases/DatabaseModal/index.test.tsx
+++ b/superset-frontend/src/features/databases/DatabaseModal/index.test.tsx
@@ -16,6 +16,9 @@
* specific language governing permissions and limitations
* under the License.
*/
+
+// TODO: These tests should be made atomic in separate files
+
import React from 'react';
import fetchMock from 'fetch-mock';
import userEvent from '@testing-library/user-event';
@@ -1227,9 +1230,9 @@ describe('DatabaseModal', () => {
const SSHTunnelServerPortInput = screen.getByTestId(
'ssh-tunnel-server_port-input',
);
- expect(SSHTunnelServerPortInput).toHaveValue('');
+ expect(SSHTunnelServerPortInput).toHaveValue(null);
userEvent.type(SSHTunnelServerPortInput, '22');
- expect(SSHTunnelServerPortInput).toHaveValue('22');
+ expect(SSHTunnelServerPortInput).toHaveValue(22);
const SSHTunnelUsernameInput = screen.getByTestId(
'ssh-tunnel-username-input',
);
@@ -1263,9 +1266,9 @@ describe('DatabaseModal', () => {
const SSHTunnelServerPortInput = screen.getByTestId(
'ssh-tunnel-server_port-input',
);
- expect(SSHTunnelServerPortInput).toHaveValue('');
+ expect(SSHTunnelServerPortInput).toHaveValue(null);
userEvent.type(SSHTunnelServerPortInput, '22');
- expect(SSHTunnelServerPortInput).toHaveValue('22');
+ expect(SSHTunnelServerPortInput).toHaveValue(22);
const SSHTunnelUsernameInput = screen.getByTestId(
'ssh-tunnel-username-input',
);
diff --git a/superset-frontend/src/features/databases/DatabaseModal/index.tsx b/superset-frontend/src/features/databases/DatabaseModal/index.tsx
index 60ae032feb..47c9a8b658 100644
--- a/superset-frontend/src/features/databases/DatabaseModal/index.tsx
+++ b/superset-frontend/src/features/databases/DatabaseModal/index.tsx
@@ -20,8 +20,6 @@ import {
t,
styled,
SupersetTheme,
- FeatureFlag,
- isFeatureEnabled,
getExtensionsRegistry,
} from '@superset-ui/core';
import React, {
@@ -31,6 +29,7 @@ import React, {
useState,
useReducer,
Reducer,
+ useCallback,
} from 'react';
import { useHistory } from 'react-router-dom';
import { setItem, LocalStorageKeys } from 'src/utils/localStorageHelpers';
@@ -65,6 +64,7 @@ import {
CatalogObject,
Engines,
ExtraJson,
+ CustomTextType,
} from '../types';
import ExtraOptions from './ExtraOptions';
import SqlAlchemyForm from './SqlAlchemyForm';
@@ -208,8 +208,8 @@ export type DBReducerActionType =
| {
type:
| ActionType.Reset
- | ActionType.AddTableCatalogSheet
- | ActionType.RemoveSSHTunnelConfig;
+ | ActionType.RemoveSSHTunnelConfig
+ | ActionType.AddTableCatalogSheet;
}
| {
type: ActionType.RemoveTableCatalogSheet;
@@ -595,7 +595,9 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
const SSHTunnelSwitchComponent =
extensionsRegistry.get('ssh_tunnel.form.switch') ?? SSHTunnelSwitch;
- const [useSSHTunneling, setUseSSHTunneling] = useState<boolean>(false);
+ const [useSSHTunneling, setUseSSHTunneling] = useState<boolean | undefined>(
+ undefined,
+ );
let dbConfigExtraExtension = extensionsRegistry.get(
'databaseconnection.extraOption',
@@ -618,14 +620,6 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
const dbImages = getDatabaseImages();
const connectionAlert = getConnectionAlert();
const isEditMode = !!databaseId;
- const disableSSHTunnelingForEngine = (
- availableDbs?.databases?.find(
- (DB: DatabaseObject) =>
- DB.backend === db?.engine || DB.engine === db?.engine,
- ) as DatabaseObject
- )?.engine_information?.disable_ssh_tunneling;
- const isSSHTunneling =
- isFeatureEnabled(FeatureFlag.SshTunneling) && !disableSSHTunnelingForEngine;
const hasAlert =
connectionAlert || !!(db?.engine && engineSpecificAlertMapping[db.engine]);
const useSqlAlchemyForm =
@@ -659,7 +653,13 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
extra: db?.extra,
masked_encrypted_extra: db?.masked_encrypted_extra || '',
server_cert: db?.server_cert || undefined,
- ssh_tunnel: db?.ssh_tunnel || undefined,
+ ssh_tunnel:
+ !isEmpty(db?.ssh_tunnel) && useSSHTunneling
+ ? {
+ ...db.ssh_tunnel,
+ server_port: Number(db.ssh_tunnel!.server_port),
+ }
+ : undefined,
};
setTestInProgress(true);
testDatabaseConnection(
@@ -687,10 +687,36 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
return false;
};
+ const onChange = useCallback(
+ (
+ type: DBReducerActionType['type'],
+ payload: CustomTextType | DBReducerPayloadType,
+ ) => {
+ setDB({ type, payload } as DBReducerActionType);
+ },
+ [],
+ );
+
+ const handleClearValidationErrors = useCallback(() => {
+ setValidationErrors(null);
+ }, [setValidationErrors]);
+
+ const handleParametersChange = useCallback(
+ ({ target }: { target: HTMLInputElement }) => {
+ onChange(ActionType.ParametersChange, {
+ type: target.type,
+ name: target.name,
+ checked: target.checked,
+ value: target.value,
+ });
+ },
+ [onChange],
+ );
+
const onClose = () => {
setDB({ type: ActionType.Reset });
setHasConnectedDb(false);
- setValidationErrors(null); // reset validation errors on close
+ handleClearValidationErrors(); // reset validation errors on close
clearError();
setEditNewDb(false);
setFileList([]);
@@ -705,7 +731,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
setSSHTunnelPrivateKeys({});
setSSHTunnelPrivateKeyPasswords({});
setConfirmedOverwrite(false);
- setUseSSHTunneling(false);
+ setUseSSHTunneling(undefined);
onHide();
};
@@ -729,12 +755,11 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
setImportingErrorMessage(msg);
});
- const onChange = (type: any, payload: any) => {
- setDB({ type, payload } as DBReducerActionType);
- };
-
const onSave = async () => {
let dbConfigExtraExtensionOnSaveError;
+
+ setLoading(true);
+
dbConfigExtraExtension
?.onSave(extraExtensionComponentState, db)
.then(({ error }: { error: any }) => {
@@ -743,6 +768,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
addDangerToast(error);
}
});
+
if (dbConfigExtraExtensionOnSaveError) {
setLoading(false);
return;
@@ -762,17 +788,13 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
});
}
- // only do validation for non ssh tunnel connections
- if (!dbToUpdate?.ssh_tunnel) {
- // make sure that button spinner animates
- setLoading(true);
- const errors = await getValidation(dbToUpdate, true);
- if ((validationErrors && !isEmpty(validationErrors)) || errors) {
- setLoading(false);
- return;
- }
- // end spinner animation
+ const errors = await getValidation(dbToUpdate, true);
+ if (!isEmpty(validationErrors) || errors?.length) {
+ addDangerToast(
+ t('Connection failed, please check your connection settings.'),
+ );
setLoading(false);
+ return;
}
const parameters_schema = isEditMode
@@ -829,7 +851,12 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
});
}
- setLoading(true);
+ // strictly checking for false as an indication that the toggle got unchecked
+ if (useSSHTunneling === false) {
+ // remove ssh tunnel
+ dbToUpdate.ssh_tunnel = null;
+ }
+
if (db?.id) {
const result = await updateResource(
db.id as number,
@@ -1282,10 +1309,10 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
}, [sshPrivateKeyPasswordNeeded]);
useEffect(() => {
- if (db && isSSHTunneling) {
- setUseSSHTunneling(!isEmpty(db?.ssh_tunnel));
+ if (db?.parameters?.ssh !== undefined) {
+ setUseSSHTunneling(db.parameters.ssh);
}
- }, [db, isSSHTunneling]);
+ }, [db?.parameters?.ssh]);
const onDbImport = async (info: UploadChangeParam) => {
setImportingErrorMessage('');
@@ -1550,17 +1577,14 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
const renderSSHTunnelForm = () => (
<SSHTunnelForm
db={db as DatabaseObject}
- onSSHTunnelParametersChange={({
- target,
- }: {
- target: HTMLInputElement | HTMLTextAreaElement;
- }) =>
+ onSSHTunnelParametersChange={({ target }) => {
onChange(ActionType.ParametersSSHTunnelChange, {
type: target.type,
name: target.name,
value: target.value,
- })
- }
+ });
+ handleClearValidationErrors();
+ }}
setSSHTunnelLoginMethod={(method: AuthType) =>
setDB({
type: ActionType.SetSSHTunnelLoginMethod,
@@ -1623,14 +1647,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
payload: { indexToDelete: idx },
});
}}
- onParametersChange={({ target }: { target: HTMLInputElement }) =>
- onChange(ActionType.ParametersChange, {
- type: target.type,
- name: target.name,
- checked: target.checked,
- value: target.value,
- })
- }
+ onParametersChange={handleParametersChange}
onChange={({ target }: { target: HTMLInputElement }) =>
onChange(ActionType.TextChange, {
name: target.name,
@@ -1640,9 +1657,9 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
getValidation={() => getValidation(db)}
validationErrors={validationErrors}
getPlaceholder={getPlaceholder}
- clearValidationErrors={() => setValidationErrors(null)}
+ clearValidationErrors={handleClearValidationErrors}
/>
- {db?.parameters?.ssh && (
+ {useSSHTunneling && (
<SSHTunnelContainer>{renderSSHTunnelForm()}</SSHTunnelContainer>
)}
</>
@@ -1792,13 +1809,12 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
testInProgress={testInProgress}
>
<SSHTunnelSwitchComponent
- isEditMode={isEditMode}
- dbFetched={dbFetched}
- disableSSHTunnelingForEngine={disableSSHTunnelingForEngine}
- useSSHTunneling={useSSHTunneling}
- setUseSSHTunneling={setUseSSHTunneling}
- setDB={setDB}
- isSSHTunneling={isSSHTunneling}
+ dbModel={dbModel}
+ db={db as DatabaseObject}
+ changeMethods={{
+ onParametersChange: handleParametersChange,
+ }}
+ clearValidationErrors={handleClearValidationErrors}
/>
{useSSHTunneling && renderSSHTunnelForm()}
</SqlAlchemyForm>
diff --git a/superset-frontend/src/features/databases/types.ts b/superset-frontend/src/features/databases/types.ts
index 50e535f9b1..58d533c7be 100644
--- a/superset-frontend/src/features/databases/types.ts
+++ b/superset-frontend/src/features/databases/types.ts
@@ -1,3 +1,7 @@
+import { JsonObject } from '@superset-ui/core';
+import { InputProps } from 'antd/lib/input';
+import { ChangeEvent, EventHandler, FormEvent } from 'react';
+
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
@@ -108,7 +112,7 @@ export type DatabaseObject = {
};
// SSH Tunnel information
- ssh_tunnel?: SSHTunnelObject;
+ ssh_tunnel?: SSHTunnelObject | null;
};
export type DatabaseForm = {
@@ -195,6 +199,10 @@ export type DatabaseForm = {
};
preferred: boolean;
sqlalchemy_uri_placeholder: string;
+ engine_information: {
+ supports_file_upload: boolean;
+ disable_ssh_tunneling: boolean;
+ };
};
// the values should align with the database
@@ -231,3 +239,73 @@ export interface ExtraJson {
};
version?: string;
}
+
+export type CustomTextType = {
+ value?: string | boolean | number;
+ type?: string | null;
+ name?: string;
+ checked?: boolean;
+};
+
+type CustomHTMLInputElement = Omit<Partial<CustomTextType>, 'value' | 'type'> &
+ CustomTextType;
+
+type CustomHTMLTextAreaElement = Omit<
+ Partial<CustomTextType>,
+ 'value' | 'type'
+> &
+ CustomTextType;
+
+export type CustomParametersChangeType<T = CustomTextType> =
+ | FormEvent<InputProps>
+ | { target: T };
+
+export type CustomEventHandlerType = EventHandler<
+ ChangeEvent<CustomHTMLInputElement | CustomHTMLTextAreaElement>
+>;
+
+export interface FieldPropTypes {
+ required: boolean;
+ hasTooltip?: boolean;
+ tooltipText?: (value: any) => string;
+ placeholder?: string;
+ onParametersChange: (event: CustomParametersChangeType) => void;
+ onParametersUploadFileChange: (value: any) => string;
+ changeMethods: {
+ onParametersChange: (event: CustomParametersChangeType) => void;
+ } & {
+ onChange: (value: any) => string;
+ } & {
+ onQueryChange: (value: any) => string;
+ } & { onParametersUploadFileChange: (value: any) => string } & {
+ onAddTableCatalog: () => void;
+ onRemoveTableCatalog: (idx: number) => void;
+ } & {
+ onExtraInputChange: (value: any) => void;
+ onSSHTunnelParametersChange: CustomEventHandlerType;
+ };
+ validationErrors: JsonObject | null;
+ getValidation: () => void;
+ clearValidationErrors: () => void;
+ db?: DatabaseObject;
+ dbModel?: DatabaseForm;
+ field: string;
+ isEditMode?: boolean;
+ sslForced?: boolean;
+ defaultDBName?: string;
+ editNewDb?: boolean;
+}
+
+type ChangeMethodsType = FieldPropTypes['changeMethods'];
+
+// changeMethods compatibility with dynamic forms
+type SwitchPropsChangeMethodsType = {
+ onParametersChange: ChangeMethodsType['onParametersChange'];
+};
+
+export type SwitchProps = {
+ dbModel: DatabaseForm;
+ db: DatabaseObject;
+ changeMethods: SwitchPropsChangeMethodsType;
+ clearValidationErrors: () => void;
+};
diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts
index 85f7c60252..8f31f2fcdd 100644
--- a/superset-frontend/src/views/CRUD/hooks.ts
+++ b/superset-frontend/src/views/CRUD/hooks.ts
@@ -35,7 +35,8 @@ import Chart, { Slice } from 'src/types/Chart';
import copyTextToClipboard from 'src/utils/copy';
import { getClientErrorObject } from 'src/utils/getClientErrorObject';
import SupersetText from 'src/utils/textUtils';
-import { FavoriteStatus, ImportResourceName, DatabaseObject } from './types';
+import { DatabaseObject } from 'src/features/databases/types';
+import { FavoriteStatus, ImportResourceName } from './types';
interface ListViewResourceState<D extends object = any> {
loading: boolean;
@@ -691,7 +692,7 @@ export const getDatabaseDocumentationLinks = () =>
SupersetText.DB_CONNECTION_DOC_LINKS;
export const testDatabaseConnection = (
- connection: DatabaseObject,
+ connection: Partial<DatabaseObject>,
handleErrorMsg: (errorMsg: string) => void,
addSuccessToast: (arg0: string) => void,
) => {
@@ -745,7 +746,7 @@ export function useDatabaseValidation() {
const getValidation = useCallback(
(database: Partial<DatabaseObject> | null, onCreate = false) => {
if (database?.parameters?.ssh) {
- // when ssh tunnel is enabled we don't want to render any validation errors
+ // TODO: /validate_parameters/ and related utils should support ssh tunnel
setValidationErrors(null);
return [];
}
diff --git a/superset/commands/database/create.py b/superset/commands/database/create.py
index cde9dd8e88..9efb39b75a 100644
--- a/superset/commands/database/create.py
+++ b/superset/commands/database/create.py
@@ -19,6 +19,7 @@ from typing import Any, Optional
from flask import current_app
from flask_appbuilder.models.sqla import Model
+from flask_babel import gettext as _
from marshmallow import ValidationError
from superset import is_feature_enabled
@@ -33,6 +34,7 @@ from superset.commands.database.exceptions import (
from superset.commands.database.ssh_tunnel.create import CreateSSHTunnelCommand
from superset.commands.database.ssh_tunnel.exceptions import (
SSHTunnelCreateFailedError,
+ SSHTunnelDatabasePortError,
SSHTunnelingNotEnabledError,
SSHTunnelInvalidError,
)
@@ -57,7 +59,11 @@ class CreateDatabaseCommand(BaseCommand):
try:
# Test connection before starting create transaction
TestConnectionDatabaseCommand(self._properties).run()
- except (SupersetErrorsException, SSHTunnelingNotEnabledError) as ex:
+ except (
+ SupersetErrorsException,
+ SSHTunnelingNotEnabledError,
+ SSHTunnelDatabasePortError,
+ ) as ex:
event_logger.log_with_context(
action=f"db_creation_failed.{ex.__class__.__name__}",
engine=self._properties.get("sqlalchemy_uri", "").split(":")[0],
@@ -103,6 +109,7 @@ class CreateDatabaseCommand(BaseCommand):
SSHTunnelInvalidError,
SSHTunnelCreateFailedError,
SSHTunnelingNotEnabledError,
+ SSHTunnelDatabasePortError,
) as ex:
db.session.rollback()
event_logger.log_with_context(
@@ -140,6 +147,7 @@ class CreateDatabaseCommand(BaseCommand):
# Check database_name uniqueness
if not DatabaseDAO.validate_uniqueness(database_name):
exceptions.append(DatabaseExistsValidationError())
+
if exceptions:
exception = DatabaseInvalidError()
exception.extend(exceptions)
diff --git a/superset/commands/database/ssh_tunnel/create.py b/superset/commands/database/ssh_tunnel/create.py
index cbfee3ce2a..287accc5aa 100644
--- a/superset/commands/database/ssh_tunnel/create.py
+++ b/superset/commands/database/ssh_tunnel/create.py
@@ -23,11 +23,13 @@ from marshmallow import ValidationError
from superset.commands.base import BaseCommand
from superset.commands.database.ssh_tunnel.exceptions import (
SSHTunnelCreateFailedError,
+ SSHTunnelDatabasePortError,
SSHTunnelInvalidError,
SSHTunnelRequiredFieldValidationError,
)
from superset.daos.database import SSHTunnelDAO
from superset.daos.exceptions import DAOCreateFailedError
+from superset.databases.utils import make_url_safe
from superset.extensions import event_logger
from superset.models.core import Database
@@ -35,9 +37,12 @@ logger = logging.getLogger(__name__)
class CreateSSHTunnelCommand(BaseCommand):
+ _database: Database
+
def __init__(self, database: Database, data: dict[str, Any]):
self._properties = data.copy()
self._properties["database"] = database
+ self._database = database
def run(self) -> Model:
try:
@@ -57,16 +62,22 @@ class CreateSSHTunnelCommand(BaseCommand):
server_address: Optional[str] = self._properties.get("server_address")
server_port: Optional[int] = self._properties.get("server_port")
username: Optional[str] = self._properties.get("username")
+ password: Optional[str] = self._properties.get("password")
private_key: Optional[str] = self._properties.get("private_key")
private_key_password: Optional[str] = self._properties.get(
"private_key_password"
)
+ url = make_url_safe(self._database.sqlalchemy_uri)
+ if not url.port:
+ raise SSHTunnelDatabasePortError()
if not server_address:
exceptions.append(SSHTunnelRequiredFieldValidationError("server_address"))
if not server_port:
exceptions.append(SSHTunnelRequiredFieldValidationError("server_port"))
if not username:
exceptions.append(SSHTunnelRequiredFieldValidationError("username"))
+ if not private_key and not password:
+ exceptions.append(SSHTunnelRequiredFieldValidationError("password"))
if private_key_password and private_key is None:
exceptions.append(SSHTunnelRequiredFieldValidationError("private_key"))
if exceptions:
diff --git a/superset/commands/database/ssh_tunnel/exceptions.py b/superset/commands/database/ssh_tunnel/exceptions.py
index 0e3f91cae6..a0def8c087 100644
--- a/superset/commands/database/ssh_tunnel/exceptions.py
+++ b/superset/commands/database/ssh_tunnel/exceptions.py
@@ -38,6 +38,10 @@ class SSHTunnelInvalidError(CommandInvalidError):
message = _("SSH Tunnel parameters are invalid.")
+class SSHTunnelDatabasePortError(CommandInvalidError):
+ message = _("A database port is required when connecting via SSH Tunnel.")
+
+
class SSHTunnelUpdateFailedError(UpdateFailedError):
message = _("SSH Tunnel could not be updated.")
diff --git a/superset/commands/database/ssh_tunnel/update.py b/superset/commands/database/ssh_tunnel/update.py
index ae7ee78afe..d0dd14a5b2 100644
--- a/superset/commands/database/ssh_tunnel/update.py
+++ b/superset/commands/database/ssh_tunnel/update.py
@@ -21,6 +21,7 @@ from flask_appbuilder.models.sqla import Model
from superset.commands.base import BaseCommand
from superset.commands.database.ssh_tunnel.exceptions import (
+ SSHTunnelDatabasePortError,
SSHTunnelInvalidError,
SSHTunnelNotFoundError,
SSHTunnelRequiredFieldValidationError,
@@ -29,6 +30,7 @@ from superset.commands.database.ssh_tunnel.exceptions import (
from superset.daos.database import SSHTunnelDAO
from superset.daos.exceptions import DAOUpdateFailedError
from superset.databases.ssh_tunnel.models import SSHTunnel
+from superset.databases.utils import make_url_safe
logger = logging.getLogger(__name__)
@@ -39,20 +41,33 @@ class UpdateSSHTunnelCommand(BaseCommand):
self._model_id = model_id
self._model: Optional[SSHTunnel] = None
- def run(self) -> Model:
+ def run(self) -> Optional[Model]:
self.validate()
try:
- if self._model is not None: # So we dont get incompatible types error
- tunnel = SSHTunnelDAO.update(self._model, self._properties)
+ if self._model is None:
+ return None
+
+ # unset password if private key is provided
+ if self._properties.get("private_key"):
+ self._properties["password"] = None
+
+ # unset private key and password if password is provided
+ if self._properties.get("password"):
+ self._properties["private_key"] = None
+ self._properties["private_key_password"] = None
+
+ tunnel = SSHTunnelDAO.update(self._model, self._properties)
+ return tunnel
except DAOUpdateFailedError as ex:
raise SSHTunnelUpdateFailedError() from ex
- return tunnel
def validate(self) -> None:
# Validate/populate model exists
self._model = SSHTunnelDAO.find_by_id(self._model_id)
if not self._model:
raise SSHTunnelNotFoundError()
+
+ url = make_url_safe(self._model.database.sqlalchemy_uri)
private_key: Optional[str] = self._properties.get("private_key")
private_key_password: Optional[str] = self._properties.get(
"private_key_password"
@@ -61,3 +76,5 @@ class UpdateSSHTunnelCommand(BaseCommand):
raise SSHTunnelInvalidError(
exceptions=[SSHTunnelRequiredFieldValidationError("private_key")]
)
+ if not url.port:
+ raise SSHTunnelDatabasePortError()
diff --git a/superset/commands/database/test_connection.py b/superset/commands/database/test_connection.py
index 0ffdf3ddd9..431918c6bc 100644
--- a/superset/commands/database/test_connection.py
+++ b/superset/commands/database/test_connection.py
@@ -32,7 +32,10 @@ from superset.commands.database.exceptions import (
DatabaseTestConnectionDriverError,
DatabaseTestConnectionUnexpectedError,
)
-from superset.commands.database.ssh_tunnel.exceptions import SSHTunnelingNotEnabledError
+from superset.commands.database.ssh_tunnel.exceptions import (
+ SSHTunnelDatabasePortError,
+ SSHTunnelingNotEnabledError,
+)
from superset.daos.database import DatabaseDAO, SSHTunnelDAO
from superset.databases.ssh_tunnel.models import SSHTunnel
from superset.databases.utils import make_url_safe
@@ -61,20 +64,22 @@ def get_log_connection_action(
class TestConnectionDatabaseCommand(BaseCommand):
+ _model: Optional[Database] = None
+ _context: dict[str, Any]
+ _uri: str
+
def __init__(self, data: dict[str, Any]):
self._properties = data.copy()
- self._model: Optional[Database] = None
- def run(self) -> None: # pylint: disable=too-many-statements, too-many-branches
- self.validate()
- ex_str = ""
+ if (database_name := self._properties.get("database_name")) is not None:
+ self._model = DatabaseDAO.get_database_by_name(database_name)
+
uri = self._properties.get("sqlalchemy_uri", "")
if self._model and uri == self._model.safe_sqlalchemy_uri():
uri = self._model.sqlalchemy_uri_decrypted
- ssh_tunnel = self._properties.get("ssh_tunnel")
- # context for error messages
url = make_url_safe(uri)
+
context = {
"hostname": url.host,
"password": url.password,
@@ -83,6 +88,14 @@ class TestConnectionDatabaseCommand(BaseCommand):
"database": url.database,
}
+ self._context = context
+ self._uri = uri
+
+ def run(self) -> None: # pylint: disable=too-many-statements
+ self.validate()
+ ex_str = ""
+ ssh_tunnel = self._properties.get("ssh_tunnel")
+
serialized_encrypted_extra = self._properties.get(
"masked_encrypted_extra",
"{}",
@@ -103,15 +116,12 @@ class TestConnectionDatabaseCommand(BaseCommand):
encrypted_extra=serialized_encrypted_extra,
)
- database.set_sqlalchemy_uri(uri)
+ database.set_sqlalchemy_uri(self._uri)
database.db_engine_spec.mutate_db_for_connection_test(database)
# Generate tunnel if present in the properties
if ssh_tunnel:
- if not is_feature_enabled("SSH_TUNNELING"):
- raise SSHTunnelingNotEnabledError()
- # If there's an existing tunnel for that DB we need to use the stored
- # password, private_key and private_key_password instead
+ # unmask password while allowing for updated values
if ssh_tunnel_id := ssh_tunnel.pop("id", None):
if existing_ssh_tunnel := SSHTunnelDAO.find_by_id(ssh_tunnel_id):
ssh_tunnel = unmask_password_info(
@@ -186,7 +196,7 @@ class TestConnectionDatabaseCommand(BaseCommand):
engine=database.db_engine_spec.__name__,
)
# check for custom errors (wrong username, wrong password, etc)
- errors = database.db_engine_spec.extract_errors(ex, context)
+ errors = database.db_engine_spec.extract_errors(ex, self._context)
raise SupersetErrorsException(errors) from ex
except SupersetSecurityException as ex:
event_logger.log_with_context(
@@ -221,9 +231,12 @@ class TestConnectionDatabaseCommand(BaseCommand):
),
engine=database.db_engine_spec.__name__,
)
- errors = database.db_engine_spec.extract_errors(ex, context)
+ errors = database.db_engine_spec.extract_errors(ex, self._context)
raise DatabaseTestConnectionUnexpectedError(errors) from ex
def validate(self) -> None:
- if (database_name := self._properties.get("database_name")) is not None:
- self._model = DatabaseDAO.get_database_by_name(database_name)
+ if self._properties.get("ssh_tunnel"):
+ if not is_feature_enabled("SSH_TUNNELING"):
+ raise SSHTunnelingNotEnabledError()
+ if not self._context.get("port"):
+ raise SSHTunnelDatabasePortError()
diff --git a/superset/commands/database/update.py b/superset/commands/database/update.py
index edc0ba1b98..5575d674a8 100644
--- a/superset/commands/database/update.py
+++ b/superset/commands/database/update.py
@@ -18,6 +18,7 @@ import logging
from typing import Any, Optional
from flask_appbuilder.models.sqla import Model
+from flask_babel import gettext as _
from marshmallow import ValidationError
from superset import is_feature_enabled
@@ -30,8 +31,11 @@ from superset.commands.database.exceptions import (
DatabaseUpdateFailedError,
)
from superset.commands.database.ssh_tunnel.create import CreateSSHTunnelCommand
+from superset.commands.database.ssh_tunnel.delete import DeleteSSHTunnelCommand
from superset.commands.database.ssh_tunnel.exceptions import (
SSHTunnelCreateFailedError,
+ SSHTunnelDatabasePortError,
+ SSHTunnelDeleteFailedError,
SSHTunnelingNotEnabledError,
SSHTunnelInvalidError,
SSHTunnelUpdateFailedError,
@@ -47,15 +51,21 @@ logger = logging.getLogger(__name__)
class UpdateDatabaseCommand(BaseCommand):
+ _model: Optional[Database]
+
def __init__(self, model_id: int, data: dict[str, Any]):
self._properties = data.copy()
self._model_id = model_id
self._model: Optional[Database] = None
- def run(self) -> Model:
- self.validate()
+ def run(self) -> Model: # pylint: disable=too-many-statements, too-many-branches
+ self._model = DatabaseDAO.find_by_id(self._model_id)
+
if not self._model:
raise DatabaseNotFoundError()
+
+ self.validate()
+
old_database_name = self._model.database_name
# unmask ``encrypted_extra``
@@ -70,36 +80,59 @@ class UpdateDatabaseCommand(BaseCommand):
database = DatabaseDAO.update(self._model, self._properties, commit=False)
database.set_sqlalchemy_uri(database.sqlalchemy_uri)
- if ssh_tunnel_properties := self._properties.get("ssh_tunnel"):
+ ssh_tunnel = DatabaseDAO.get_ssh_tunnel(database.id)
+
+ if "ssh_tunnel" in self._properties:
if not is_feature_enabled("SSH_TUNNELING"):
db.session.rollback()
raise SSHTunnelingNotEnabledError()
- existing_ssh_tunnel_model = DatabaseDAO.get_ssh_tunnel(database.id)
- if existing_ssh_tunnel_model is None:
- # We couldn't found an existing tunnel so we need to create one
- try:
- CreateSSHTunnelCommand(database, ssh_tunnel_properties).run()
- except (SSHTunnelInvalidError, SSHTunnelCreateFailedError) as ex:
- # So we can show the original message
- raise ex
- except Exception as ex:
- raise DatabaseUpdateFailedError() from ex
- else:
- # We found an existing tunnel so we need to update it
+
+ if self._properties.get("ssh_tunnel") is None and ssh_tunnel:
+ # We need to remove the existing tunnel
try:
- UpdateSSHTunnelCommand(
- existing_ssh_tunnel_model.id, ssh_tunnel_properties
- ).run()
- except (SSHTunnelInvalidError, SSHTunnelUpdateFailedError) as ex:
- # So we can show the original message
+ DeleteSSHTunnelCommand(ssh_tunnel.id).run()
+ ssh_tunnel = None
+ except SSHTunnelDeleteFailedError as ex:
raise ex
except Exception as ex:
raise DatabaseUpdateFailedError() from ex
+ if ssh_tunnel_properties := self._properties.get("ssh_tunnel"):
+ if ssh_tunnel is None:
+ # We couldn't found an existing tunnel so we need to create one
+ try:
+ ssh_tunnel = CreateSSHTunnelCommand(
+ database, ssh_tunnel_properties
+ ).run()
+ except (
+ SSHTunnelInvalidError,
+ SSHTunnelCreateFailedError,
+ SSHTunnelDatabasePortError,
+ ) as ex:
+ # So we can show the original message
+ raise ex
+ except Exception as ex:
+ raise DatabaseUpdateFailedError() from ex
+ else:
+ # We found an existing tunnel so we need to update it
+ try:
+ ssh_tunnel_id = ssh_tunnel.id
+ ssh_tunnel = UpdateSSHTunnelCommand(
+ ssh_tunnel_id, ssh_tunnel_properties
+ ).run()
+ except (
+ SSHTunnelInvalidError,
+ SSHTunnelUpdateFailedError,
+ SSHTunnelDatabasePortError,
+ ) as ex:
+ # So we can show the original message
+ raise ex
+ except Exception as ex:
+ raise DatabaseUpdateFailedError() from ex
+
# adding a new database we always want to force refresh schema list
# TODO Improve this simplistic implementation for catching DB conn fails
try:
- ssh_tunnel = DatabaseDAO.get_ssh_tunnel(database.id)
schemas = database.get_all_schema_names(ssh_tunnel=ssh_tunnel)
except Exception as ex:
db.session.rollback()
@@ -167,10 +200,6 @@ class UpdateDatabaseCommand(BaseCommand):
def validate(self) -> None:
exceptions: list[ValidationError] = []
- # Validate/populate model exists
- self._model = DatabaseDAO.find_by_id(self._model_id)
- if not self._model:
- raise DatabaseNotFoundError()
database_name: Optional[str] = self._properties.get("database_name")
if database_name:
# Check database_name uniqueness
diff --git a/superset/databases/api.py b/superset/databases/api.py
index 2f95bd0442..4d7d4c531a 100644
--- a/superset/databases/api.py
+++ b/superset/databases/api.py
@@ -47,6 +47,7 @@ from superset.commands.database.export import ExportDatabasesCommand
from superset.commands.database.importers.dispatcher import ImportDatabasesCommand
from superset.commands.database.ssh_tunnel.delete import DeleteSSHTunnelCommand
from superset.commands.database.ssh_tunnel.exceptions import (
+ SSHTunnelDatabasePortError,
SSHTunnelDeleteFailedError,
SSHTunnelingNotEnabledError,
)
@@ -415,7 +416,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
exc_info=True,
)
return self.response_422(message=str(ex))
- except SSHTunnelingNotEnabledError as ex:
+ except (SSHTunnelingNotEnabledError, SSHTunnelDatabasePortError) as ex:
return self.response_400(message=str(ex))
except SupersetException as ex:
return self.response(ex.status, message=ex.message)
@@ -500,7 +501,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
exc_info=True,
)
return self.response_422(message=str(ex))
- except SSHTunnelingNotEnabledError as ex:
+ except (SSHTunnelingNotEnabledError, SSHTunnelDatabasePortError) as ex:
return self.response_400(message=str(ex))
@expose("/<int:pk>", methods=("DELETE",))
@@ -918,7 +919,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
try:
TestConnectionDatabaseCommand(item).run()
return self.response(200, message="OK")
- except SSHTunnelingNotEnabledError as ex:
+ except (SSHTunnelingNotEnabledError, SSHTunnelDatabasePortError) as ex:
return self.response_400(message=str(ex))
@expose("/<int:pk>/related_objects/", methods=("GET",))
diff --git a/tests/integration_tests/databases/api_tests.py b/tests/integration_tests/databases/api_tests.py
index ebabc16e87..0f9dc03723 100644
--- a/tests/integration_tests/databases/api_tests.py
+++ b/tests/integration_tests/databases/api_tests.py
@@ -35,6 +35,7 @@ from sqlalchemy.exc import DBAPIError
from sqlalchemy.sql import func
from superset import db, security_manager
+from superset.commands.database.ssh_tunnel.exceptions import SSHTunnelDatabasePortError
from superset.connectors.sqla.models import SqlaTable
from superset.databases.ssh_tunnel.models import SSHTunnel
from superset.databases.utils import make_url_safe
@@ -336,6 +337,58 @@ class TestDatabaseApi(SupersetTestCase):
db.session.delete(model)
db.session.commit()
+ @mock.patch(
+ "superset.commands.database.test_connection.TestConnectionDatabaseCommand.run",
+ )
+ @mock.patch("superset.commands.database.create.is_feature_enabled")
+ @mock.patch(
+ "superset.models.core.Database.get_all_schema_names",
+ )
+ def test_create_database_with_missing_port_raises_error(
+ self,
+ mock_test_connection_database_command_run,
+ mock_create_is_feature_enabled,
+ mock_get_all_schema_names,
+ ):
+ """
+ Database API: Test that missing port raises SSHTunnelDatabaseError
+ """
+ mock_create_is_feature_enabled.return_value = True
+ self.login(username="admin")
+ example_db = get_example_database()
+ if example_db.backend == "sqlite":
+ return
+
+ modified_sqlalchemy_uri = "postgresql://foo:bar@localhost/test-db"
+
+ ssh_tunnel_properties = {
+ "server_address": "123.132.123.1",
+ "server_port": 8080,
+ "username": "foo",
+ "password": "bar",
+ }
+
+ database_data_with_ssh_tunnel = {
+ "database_name": "test-db-with-ssh-tunnel",
+ "sqlalchemy_uri": modified_sqlalchemy_uri,
+ "ssh_tunnel": ssh_tunnel_properties,
+ }
+
+ database_data_with_ssh_tunnel = {
+ "database_name": "test-db-with-ssh-tunnel",
+ "sqlalchemy_uri": modified_sqlalchemy_uri,
+ "ssh_tunnel": ssh_tunnel_properties,
+ }
+
+ uri = "api/v1/database/"
+ rv = self.client.post(uri, json=database_data_with_ssh_tunnel)
+ response = json.loads(rv.data.decode("utf-8"))
+ self.assertEqual(rv.status_code, 400)
+ self.assertEqual(
+ response.get("message"),
+ "A database port is required when connecting via SSH Tunnel.",
+ )
+
@mock.patch(
"superset.commands.database.test_connection.TestConnectionDatabaseCommand.run",
)
@@ -397,6 +450,154 @@ class TestDatabaseApi(SupersetTestCase):
db.session.delete(model)
db.session.commit()
+ @mock.patch(
+ "superset.commands.database.test_connection.TestConnectionDatabaseCommand.run",
+ )
+ @mock.patch("superset.commands.database.create.is_feature_enabled")
+ @mock.patch("superset.commands.database.update.is_feature_enabled")
+ @mock.patch(
+ "superset.models.core.Database.get_all_schema_names",
+ )
+ def test_update_database_with_missing_port_raises_error(
+ self,
+ mock_test_connection_database_command_run,
+ mock_create_is_feature_enabled,
+ mock_update_is_feature_enabled,
+ mock_get_all_schema_names,
+ ):
+ """
+ Database API: Test that missing port raises SSHTunnelDatabaseError
+ """
+ mock_create_is_feature_enabled.return_value = True
+ mock_update_is_feature_enabled.return_value = True
+ self.login(username="admin")
+ example_db = get_example_database()
+ if example_db.backend == "sqlite":
+ return
+
+ modified_sqlalchemy_uri = "postgresql://foo:bar@localhost/test-db"
+
+ ssh_tunnel_properties = {
+ "server_address": "123.132.123.1",
+ "server_port": 8080,
+ "username": "foo",
+ "password": "bar",
+ }
+
+ database_data_with_ssh_tunnel = {
+ "database_name": "test-db-with-ssh-tunnel",
+ "sqlalchemy_uri": modified_sqlalchemy_uri,
+ "ssh_tunnel": ssh_tunnel_properties,
+ }
+
+ database_data = {
+ "database_name": "test-db-with-ssh-tunnel",
+ "sqlalchemy_uri": example_db.sqlalchemy_uri_decrypted,
+ }
+
+ uri = "api/v1/database/"
+ rv = self.client.post(uri, json=database_data)
+ response_create = json.loads(rv.data.decode("utf-8"))
+ self.assertEqual(rv.status_code, 201)
+
+ uri = "api/v1/database/{}".format(response_create.get("id"))
+ rv = self.client.put(uri, json=database_data_with_ssh_tunnel)
+ response = json.loads(rv.data.decode("utf-8"))
+ self.assertEqual(rv.status_code, 400)
+ self.assertEqual(
+ response.get("message"),
+ "A database port is required when connecting via SSH Tunnel.",
+ )
+
+ # Cleanup
+ model = db.session.query(Database).get(response_create.get("id"))
+ db.session.delete(model)
+ db.session.commit()
+
+ @mock.patch(
+ "superset.commands.database.test_connection.TestConnectionDatabaseCommand.run",
+ )
+ @mock.patch("superset.commands.database.create.is_feature_enabled")
+ @mock.patch("superset.commands.database.update.is_feature_enabled")
+ @mock.patch("superset.commands.database.ssh_tunnel.delete.is_feature_enabled")
+ @mock.patch(
+ "superset.models.core.Database.get_all_schema_names",
+ )
+ def test_delete_ssh_tunnel(
+ self,
+ mock_test_connection_database_command_run,
+ mock_create_is_feature_enabled,
+ mock_update_is_feature_enabled,
+ mock_delete_is_feature_enabled,
+ mock_get_all_schema_names,
+ ):
+ """
+ Database API: Test deleting a SSH tunnel via Database update
+ """
+ mock_create_is_feature_enabled.return_value = True
+ mock_update_is_feature_enabled.return_value = True
+ mock_delete_is_feature_enabled.return_value = True
+ self.login(username="admin")
+ example_db = get_example_database()
+ if example_db.backend == "sqlite":
+ return
+
+ ssh_tunnel_properties = {
+ "server_address": "123.132.123.1",
+ "server_port": 8080,
+ "username": "foo",
+ "password": "bar",
+ }
+ database_data = {
+ "database_name": "test-db-with-ssh-tunnel",
+ "sqlalchemy_uri": example_db.sqlalchemy_uri_decrypted,
+ }
+ database_data_with_ssh_tunnel = {
+ "database_name": "test-db-with-ssh-tunnel",
+ "sqlalchemy_uri": example_db.sqlalchemy_uri_decrypted,
+ "ssh_tunnel": ssh_tunnel_properties,
+ }
+
+ uri = "api/v1/database/"
+ rv = self.client.post(uri, json=database_data)
+ response = json.loads(rv.data.decode("utf-8"))
+ self.assertEqual(rv.status_code, 201)
+
+ uri = "api/v1/database/{}".format(response.get("id"))
+ rv = self.client.put(uri, json=database_data_with_ssh_tunnel)
+ response_update = json.loads(rv.data.decode("utf-8"))
+ self.assertEqual(rv.status_code, 200)
+
+ model_ssh_tunnel = (
+ db.session.query(SSHTunnel)
+ .filter(SSHTunnel.database_id == response_update.get("id"))
+ .one()
+ )
+ self.assertEqual(model_ssh_tunnel.database_id, response_update.get("id"))
+
+ database_data_with_ssh_tunnel_null = {
+ "database_name": "test-db-with-ssh-tunnel",
+ "sqlalchemy_uri": example_db.sqlalchemy_uri_decrypted,
+ "ssh_tunnel": None,
+ }
+
+ rv = self.client.put(uri, json=database_data_with_ssh_tunnel_null)
+ response_update = json.loads(rv.data.decode("utf-8"))
+ self.assertEqual(rv.status_code, 200)
+
+ model_ssh_tunnel = (
+ db.session.query(SSHTunnel)
+ .filter(SSHTunnel.database_id == response_update.get("id"))
+ .one_or_none()
+ )
+
+ assert model_ssh_tunnel is None
+
+ # Cleanup
+ model = db.session.query(Database).get(response.get("id"))
+ db.session.delete(model)
+ db.session.commit()
+
@mock.patch(
"superset.commands.database.test_connection.TestConnectionDatabaseCommand.run",
)
diff --git a/tests/unit_tests/databases/ssh_tunnel/commands/create_test.py b/tests/unit_tests/databases/ssh_tunnel/commands/create_test.py
index 4b05cce637..c80b52931d 100644
--- a/tests/unit_tests/databases/ssh_tunnel/commands/create_test.py
+++ b/tests/unit_tests/databases/ssh_tunnel/commands/create_test.py
@@ -19,7 +19,10 @@
import pytest
from sqlalchemy.orm.session import Session
-from superset.commands.database.ssh_tunnel.exceptions import SSHTunnelInvalidError
+from superset.commands.database.ssh_tunnel.exceptions import (
+ SSHTunnelDatabasePortError,
+ SSHTunnelInvalidError,
+)
def test_create_ssh_tunnel_command() -> None:
@@ -27,7 +30,11 @@ def test_create_ssh_tunnel_command() -> None:
from superset.databases.ssh_tunnel.models import SSHTunnel
from superset.models.core import Database
- database = Database(id=1, database_name="my_database", sqlalchemy_uri="sqlite://")
+ database = Database(
+ id=1,
+ database_name="my_database",
+ sqlalchemy_uri="postgresql://u:p@localhost:5432/db",
+ )
properties = {
"database_id": database.id,
@@ -48,7 +55,11 @@ def test_create_ssh_tunnel_command_invalid_params() -> None:
from superset.databases.ssh_tunnel.models import SSHTunnel
from superset.models.core import Database
- database = Database(id=1, database_name="my_database", sqlalchemy_uri="sqlite://")
+ database = Database(
+ id=1,
+ database_name="my_database",
+ sqlalchemy_uri="postgresql://u:p@localhost:5432/db",
+ )
# If we are trying to create a tunnel with a private_key_password
# then a private_key is mandatory
@@ -65,3 +76,31 @@ def test_create_ssh_tunnel_command_invalid_params() -> None:
with pytest.raises(SSHTunnelInvalidError) as excinfo:
command.run()
assert str(excinfo.value) == ("SSH Tunnel parameters are invalid.")
+
+
+def test_create_ssh_tunnel_command_no_port() -> None:
+ from superset.commands.database.ssh_tunnel.create import CreateSSHTunnelCommand
+ from superset.databases.ssh_tunnel.models import SSHTunnel
+ from superset.models.core import Database
+
+ database = Database(
+ id=1,
+ database_name="my_database",
+ sqlalchemy_uri="postgresql://u:p@localhost/db",
+ )
+
+ properties = {
+ "database": database,
+ "server_address": "123.132.123.1",
+ "server_port": "3005",
+ "username": "foo",
+ "password": "bar",
+ }
+
+ command = CreateSSHTunnelCommand(database, properties)
+
+ with pytest.raises(SSHTunnelDatabasePortError) as excinfo:
+ command.run()
+ assert str(excinfo.value) == (
+ "A database port is required when connecting via SSH Tunnel."
+ )
diff --git a/tests/unit_tests/databases/ssh_tunnel/commands/update_test.py b/tests/unit_tests/databases/ssh_tunnel/commands/update_test.py
index 54e54d05da..66684eb8de 100644
--- a/tests/unit_tests/databases/ssh_tunnel/commands/update_test.py
+++ b/tests/unit_tests/databases/ssh_tunnel/commands/update_test.py
@@ -20,11 +20,14 @@ from collections.abc import Iterator
import pytest
from sqlalchemy.orm.session import Session
-from superset.commands.database.ssh_tunnel.exceptions import SSHTunnelInvalidError
+from superset.commands.database.ssh_tunnel.exceptions import (
+ SSHTunnelDatabasePortError,
+ SSHTunnelInvalidError,
+)
@pytest.fixture
-def session_with_data(session: Session) -> Iterator[Session]:
+def session_with_data(request, session: Session) -> Iterator[Session]:
from superset.connectors.sqla.models import SqlaTable
from superset.databases.ssh_tunnel.models import SSHTunnel
from superset.models.core import Database
@@ -32,7 +35,8 @@ def session_with_data(session: Session) -> Iterator[Session]:
engine = session.get_bind()
SqlaTable.metadata.create_all(engine) # pylint: disable=no-member
- database = Database(database_name="my_database", sqlalchemy_uri="sqlite://")
+ sqlalchemy_uri = getattr(request, "param", "postgresql://u:p@localhost:5432/db")
+ database = Database(database_name="my_database", sqlalchemy_uri=sqlalchemy_uri)
sqla_table = SqlaTable(
table_name="my_sqla_table",
columns=[],
@@ -93,3 +97,28 @@ def test_update_shh_tunnel_invalid_params(session_with_data: Session) -> None:
with pytest.raises(SSHTunnelInvalidError) as excinfo:
command.run()
assert str(excinfo.value) == ("SSH Tunnel parameters are invalid.")
+
+
+@pytest.mark.parametrize(
+ "session_with_data", ["postgresql://u:p@localhost/testdb"], indirect=True
+)
+def test_update_shh_tunnel_no_port(session_with_data: Session) -> None:
+ from superset.commands.database.ssh_tunnel.update import UpdateSSHTunnelCommand
+ from superset.daos.database import DatabaseDAO
+ from superset.databases.ssh_tunnel.models import SSHTunnel
+
+ result = DatabaseDAO.get_ssh_tunnel(1)
+
+ assert result
+ assert isinstance(result, SSHTunnel)
+ assert 1 == result.database_id
+ assert "Test" == result.server_address
+
+ update_payload = {"server_address": "Test update"}
+ command = UpdateSSHTunnelCommand(1, update_payload)
+
+ with pytest.raises(SSHTunnelDatabasePortError) as excinfo:
+ command.run()
+ assert str(excinfo.value) == (
+ "A database port is required when connecting via SSH Tunnel."
+ )
(superset) 01/05: fix(deps): resolving canvg and html2canvas module not found (#27315)
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 15557f493d7330a4a88d63dbb75f86c29952f3f7
Author: Jack <41...@users.noreply.github.com>
AuthorDate: Sun Mar 3 18:26:17 2024 -0600
fix(deps): resolving canvg and html2canvas module not found (#27315)
(cherry picked from commit 5915851ba308ce06a914f173fba8b0c47c4e32c0)
---
superset-frontend/package-lock.json | 40 ++++++++++---------------------------
1 file changed, 10 insertions(+), 30 deletions(-)
diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json
index 50d0a97dad..38d97377c3 100644
--- a/superset-frontend/package-lock.json
+++ b/superset-frontend/package-lock.json
@@ -22716,8 +22716,7 @@
"node_modules/@types/raf": {
"version": "3.4.2",
"resolved": "https://registry.npmjs.org/@types/raf/-/raf-3.4.2.tgz",
- "integrity": "sha512-sM4HyDVlDFl4goOXPF+g9nNHJFZQGot+HgySjM4cRjqXzjdatcEvYrtG4Ia8XumR9T6k8G2tW9B7hnUj51Uf0A==",
- "optional": true
+ "integrity": "sha512-sM4HyDVlDFl4goOXPF+g9nNHJFZQGot+HgySjM4cRjqXzjdatcEvYrtG4Ia8XumR9T6k8G2tW9B7hnUj51Uf0A=="
},
"node_modules/@types/range-parser": {
"version": "1.2.4",
@@ -26817,7 +26816,6 @@
"version": "1.0.2",
"resolved": "https://registry.npmjs.org/base64-arraybuffer/-/base64-arraybuffer-1.0.2.tgz",
"integrity": "sha512-I3yl4r9QB5ZRY3XuJVEPfc2XhZO6YweFPI+UovAzn+8/hb3oJ6lnysaFcjVpkCPfVWFUDvoZ8kmVDP7WyRtYtQ==",
- "optional": true,
"engines": {
"node": ">= 0.6.0"
}
@@ -27575,7 +27573,6 @@
"version": "3.0.10",
"resolved": "https://registry.npmjs.org/canvg/-/canvg-3.0.10.tgz",
"integrity": "sha512-qwR2FRNO9NlzTeKIPIKpnTY6fqwuYSequ8Ru8c0YkYU7U0oW+hLUvWadLvAu1Rl72OMNiFhoLu4f8eUjQ7l/+Q==",
- "optional": true,
"dependencies": {
"@babel/runtime": "^7.12.5",
"@types/raf": "^3.4.0",
@@ -30510,7 +30507,6 @@
"version": "2.1.0",
"resolved": "https://registry.npmjs.org/css-line-break/-/css-line-break-2.1.0.tgz",
"integrity": "sha512-FHcKFCZcAha3LwfVBhCQbW2nCNbkZXn7KVUJcsT5/P8YmfsVja0FMPJr0B903j/E69HUphKiV9iQArX8SDYA4w==",
- "optional": true,
"dependencies": {
"utrie": "^1.0.2"
}
@@ -39067,7 +39063,6 @@
"version": "1.4.1",
"resolved": "https://registry.npmjs.org/html2canvas/-/html2canvas-1.4.1.tgz",
"integrity": "sha512-fPU6BHNpsyIhr8yyMpTLLxAbkaK8ArIBcmZIRiBLiDhjeqvXolaEmDGmELFuX9I4xDcaKKcJl+TKZLqruBbmWA==",
- "optional": true,
"dependencies": {
"css-line-break": "^2.1.0",
"text-segmentation": "^1.0.3"
@@ -46672,13 +46667,13 @@
"@babel/runtime": "^7.14.0",
"atob": "^2.1.2",
"btoa": "^1.2.1",
- "fflate": "^0.4.8"
+ "canvg": "^3.0.6",
+ "fflate": "^0.4.8",
+ "html2canvas": "^1.0.0-rc.5"
},
"optionalDependencies": {
- "canvg": "^3.0.6",
"core-js": "^3.6.0",
- "dompurify": "^2.2.0",
- "html2canvas": "^1.0.0-rc.5"
+ "dompurify": "^2.2.0"
}
},
"node_modules/jsprim": {
@@ -60369,7 +60364,6 @@
"version": "1.0.1",
"resolved": "https://registry.npmjs.org/rgbcolor/-/rgbcolor-1.0.1.tgz",
"integrity": "sha512-9aZLIrhRaD97sgVhtJOW6ckOEh6/GnvQtdVNfdZ6s67+3/XwLS9lBcQYzEEhYVeUowN7pRzMLsyGhK2i/xvWbw==",
- "optional": true,
"engines": {
"node": ">= 0.8.15"
}
@@ -61884,7 +61878,6 @@
"version": "2.6.0",
"resolved": "https://registry.npmjs.org/stackblur-canvas/-/stackblur-canvas-2.6.0.tgz",
"integrity": "sha512-8S1aIA+UoF6erJYnglGPug6MaHYGo1Ot7h5fuXx4fUPvcvQfcdw2o/ppCse63+eZf8PPidSu4v1JnmEVtEDnpg==",
- "optional": true,
"engines": {
"node": ">=0.1.14"
}
@@ -62593,7 +62586,6 @@
"version": "6.0.3",
"resolved": "https://registry.npmjs.org/svg-pathdata/-/svg-pathdata-6.0.3.tgz",
"integrity": "sha512-qsjeeq5YjBZ5eMdFuUa4ZosMLxgr5RZ+F+Y1OrDhuOCEInRMA3x74XdBtggJcj9kOeInz0WE+LgCPDkZFlBYJw==",
- "optional": true,
"engines": {
"node": ">=12.0.0"
}
@@ -63117,7 +63109,6 @@
"version": "1.0.3",
"resolved": "https://registry.npmjs.org/text-segmentation/-/text-segmentation-1.0.3.tgz",
"integrity": "sha512-iOiPUo/BGnZ6+54OsWxZidGCsdU8YbE4PSpdPinp7DeMtUJNJBoJ/ouUSTJjHkh1KntHaltHl/gDs2FC4i5+Nw==",
- "optional": true,
"dependencies": {
"utrie": "^1.0.2"
}
@@ -64590,7 +64581,6 @@
"version": "1.0.2",
"resolved": "https://registry.npmjs.org/utrie/-/utrie-1.0.2.tgz",
"integrity": "sha512-1MLa5ouZiOmQzUbjbu9VmjLzn1QLXBhwpUa7kdLUQK+KQ5KA9I1vk5U4YHe/X2Ch7PYnJfWuWT+VbuxbGwljhw==",
- "optional": true,
"dependencies": {
"base64-arraybuffer": "^1.0.2"
}
@@ -89473,8 +89463,7 @@
"@types/raf": {
"version": "3.4.2",
"resolved": "https://registry.npmjs.org/@types/raf/-/raf-3.4.2.tgz",
- "integrity": "sha512-sM4HyDVlDFl4goOXPF+g9nNHJFZQGot+HgySjM4cRjqXzjdatcEvYrtG4Ia8XumR9T6k8G2tW9B7hnUj51Uf0A==",
- "optional": true
+ "integrity": "sha512-sM4HyDVlDFl4goOXPF+g9nNHJFZQGot+HgySjM4cRjqXzjdatcEvYrtG4Ia8XumR9T6k8G2tW9B7hnUj51Uf0A=="
},
"@types/range-parser": {
"version": "1.2.4",
@@ -92753,8 +92742,7 @@
"base64-arraybuffer": {
"version": "1.0.2",
"resolved": "https://registry.npmjs.org/base64-arraybuffer/-/base64-arraybuffer-1.0.2.tgz",
- "integrity": "sha512-I3yl4r9QB5ZRY3XuJVEPfc2XhZO6YweFPI+UovAzn+8/hb3oJ6lnysaFcjVpkCPfVWFUDvoZ8kmVDP7WyRtYtQ==",
- "optional": true
+ "integrity": "sha512-I3yl4r9QB5ZRY3XuJVEPfc2XhZO6YweFPI+UovAzn+8/hb3oJ6lnysaFcjVpkCPfVWFUDvoZ8kmVDP7WyRtYtQ=="
},
"base64-js": {
"version": "1.5.1",
@@ -93330,7 +93318,6 @@
"version": "3.0.10",
"resolved": "https://registry.npmjs.org/canvg/-/canvg-3.0.10.tgz",
"integrity": "sha512-qwR2FRNO9NlzTeKIPIKpnTY6fqwuYSequ8Ru8c0YkYU7U0oW+hLUvWadLvAu1Rl72OMNiFhoLu4f8eUjQ7l/+Q==",
- "optional": true,
"requires": {
"@babel/runtime": "^7.12.5",
"@types/raf": "^3.4.0",
@@ -95644,7 +95631,6 @@
"version": "2.1.0",
"resolved": "https://registry.npmjs.org/css-line-break/-/css-line-break-2.1.0.tgz",
"integrity": "sha512-FHcKFCZcAha3LwfVBhCQbW2nCNbkZXn7KVUJcsT5/P8YmfsVja0FMPJr0B903j/E69HUphKiV9iQArX8SDYA4w==",
- "optional": true,
"requires": {
"utrie": "^1.0.2"
}
@@ -102082,7 +102068,6 @@
"version": "1.4.1",
"resolved": "https://registry.npmjs.org/html2canvas/-/html2canvas-1.4.1.tgz",
"integrity": "sha512-fPU6BHNpsyIhr8yyMpTLLxAbkaK8ArIBcmZIRiBLiDhjeqvXolaEmDGmELFuX9I4xDcaKKcJl+TKZLqruBbmWA==",
- "optional": true,
"requires": {
"css-line-break": "^2.1.0",
"text-segmentation": "^1.0.3"
@@ -118253,8 +118238,7 @@
"rgbcolor": {
"version": "1.0.1",
"resolved": "https://registry.npmjs.org/rgbcolor/-/rgbcolor-1.0.1.tgz",
- "integrity": "sha512-9aZLIrhRaD97sgVhtJOW6ckOEh6/GnvQtdVNfdZ6s67+3/XwLS9lBcQYzEEhYVeUowN7pRzMLsyGhK2i/xvWbw==",
- "optional": true
+ "integrity": "sha512-9aZLIrhRaD97sgVhtJOW6ckOEh6/GnvQtdVNfdZ6s67+3/XwLS9lBcQYzEEhYVeUowN7pRzMLsyGhK2i/xvWbw=="
},
"rimraf": {
"version": "3.0.2",
@@ -119462,8 +119446,7 @@
"stackblur-canvas": {
"version": "2.6.0",
"resolved": "https://registry.npmjs.org/stackblur-canvas/-/stackblur-canvas-2.6.0.tgz",
- "integrity": "sha512-8S1aIA+UoF6erJYnglGPug6MaHYGo1Ot7h5fuXx4fUPvcvQfcdw2o/ppCse63+eZf8PPidSu4v1JnmEVtEDnpg==",
- "optional": true
+ "integrity": "sha512-8S1aIA+UoF6erJYnglGPug6MaHYGo1Ot7h5fuXx4fUPvcvQfcdw2o/ppCse63+eZf8PPidSu4v1JnmEVtEDnpg=="
},
"stackframe": {
"version": "1.3.4",
@@ -119997,8 +119980,7 @@
"svg-pathdata": {
"version": "6.0.3",
"resolved": "https://registry.npmjs.org/svg-pathdata/-/svg-pathdata-6.0.3.tgz",
- "integrity": "sha512-qsjeeq5YjBZ5eMdFuUa4ZosMLxgr5RZ+F+Y1OrDhuOCEInRMA3x74XdBtggJcj9kOeInz0WE+LgCPDkZFlBYJw==",
- "optional": true
+ "integrity": "sha512-qsjeeq5YjBZ5eMdFuUa4ZosMLxgr5RZ+F+Y1OrDhuOCEInRMA3x74XdBtggJcj9kOeInz0WE+LgCPDkZFlBYJw=="
},
"svgo": {
"version": "3.2.0",
@@ -120383,7 +120365,6 @@
"version": "1.0.3",
"resolved": "https://registry.npmjs.org/text-segmentation/-/text-segmentation-1.0.3.tgz",
"integrity": "sha512-iOiPUo/BGnZ6+54OsWxZidGCsdU8YbE4PSpdPinp7DeMtUJNJBoJ/ouUSTJjHkh1KntHaltHl/gDs2FC4i5+Nw==",
- "optional": true,
"requires": {
"utrie": "^1.0.2"
}
@@ -121476,7 +121457,6 @@
"version": "1.0.2",
"resolved": "https://registry.npmjs.org/utrie/-/utrie-1.0.2.tgz",
"integrity": "sha512-1MLa5ouZiOmQzUbjbu9VmjLzn1QLXBhwpUa7kdLUQK+KQ5KA9I1vk5U4YHe/X2Ch7PYnJfWuWT+VbuxbGwljhw==",
- "optional": true,
"requires": {
"base64-arraybuffer": "^1.0.2"
}
(superset) 02/05: fix(dashboard): Only fetch CSS templates for dashboard header menu when in edit mode (#27411)
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 759863553d342f9c72a7dcd66661b39adeaeb707
Author: Mark Skelton <ma...@federato.ai>
AuthorDate: Fri Mar 8 07:21:36 2024 -0600
fix(dashboard): Only fetch CSS templates for dashboard header menu when in edit mode (#27411)
Co-authored-by: Michael S. Molina <mi...@gmail.com>
(cherry picked from commit fde93dcf08122d8b41ca296213e09616d1b71782)
---
.../components/CssEditor/CssEditor.test.tsx | 54 +++++++++++++---------
.../src/dashboard/components/CssEditor/index.jsx | 25 ++++++++--
.../Header/HeaderActionsDropdown/index.jsx | 22 +--------
3 files changed, 55 insertions(+), 46 deletions(-)
diff --git a/superset-frontend/src/dashboard/components/CssEditor/CssEditor.test.tsx b/superset-frontend/src/dashboard/components/CssEditor/CssEditor.test.tsx
index 16b2a1afbb..28ac7672f6 100644
--- a/superset-frontend/src/dashboard/components/CssEditor/CssEditor.test.tsx
+++ b/superset-frontend/src/dashboard/components/CssEditor/CssEditor.test.tsx
@@ -21,6 +21,7 @@ import { render, screen, waitFor } from 'spec/helpers/testing-library';
import { CssEditor as AceCssEditor } from 'src/components/AsyncAceEditor';
import { IAceEditorProps } from 'react-ace';
import userEvent from '@testing-library/user-event';
+import fetchMock from 'fetch-mock';
import CssEditor from '.';
jest.mock('src/components/AsyncAceEditor', () => ({
@@ -33,46 +34,59 @@ jest.mock('src/components/AsyncAceEditor', () => ({
}));
const templates = [
- { label: 'Template A', css: 'background-color: red;' },
- { label: 'Template B', css: 'background-color: blue;' },
- { label: 'Template C', css: 'background-color: yellow;' },
+ { template_name: 'Template A', css: 'background-color: red;' },
+ { template_name: 'Template B', css: 'background-color: blue;' },
+ { template_name: 'Template C', css: 'background-color: yellow;' },
];
+fetchMock.get('glob:*/csstemplateasyncmodelview/api/read', {
+ result: templates,
+});
+
AceCssEditor.preload = () => new Promise(() => {});
-test('renders with default props', () => {
- render(<CssEditor triggerNode={<>Click</>} />);
+const defaultProps = {
+ triggerNode: <>Click</>,
+ addDangerToast: jest.fn(),
+};
+
+test('renders with default props', async () => {
+ await waitFor(() => render(<CssEditor {...defaultProps} />));
expect(screen.getByRole('button', { name: 'Click' })).toBeInTheDocument();
});
-test('renders with initial CSS', () => {
+test('renders with initial CSS', async () => {
const initialCss = 'margin: 10px;';
- render(<CssEditor triggerNode={<>Click</>} initialCss={initialCss} />);
+ await waitFor(() =>
+ render(<CssEditor {...defaultProps} initialCss={initialCss} />),
+ );
userEvent.click(screen.getByRole('button', { name: 'Click' }));
expect(screen.getByText(initialCss)).toBeInTheDocument();
});
test('renders with templates', async () => {
- render(<CssEditor triggerNode={<>Click</>} templates={templates} />);
+ await waitFor(() => render(<CssEditor {...defaultProps} />));
userEvent.click(screen.getByRole('button', { name: 'Click' }));
userEvent.hover(screen.getByText('Load a CSS template'));
await waitFor(() => {
templates.forEach(template =>
- expect(screen.getByText(template.label)).toBeInTheDocument(),
+ expect(screen.getByText(template.template_name)).toBeInTheDocument(),
);
});
});
-test('triggers onChange when using the editor', () => {
+test('triggers onChange when using the editor', async () => {
const onChange = jest.fn();
const initialCss = 'margin: 10px;';
const additionalCss = 'color: red;';
- render(
- <CssEditor
- triggerNode={<>Click</>}
- initialCss={initialCss}
- onChange={onChange}
- />,
+ await waitFor(() =>
+ render(
+ <CssEditor
+ {...defaultProps}
+ initialCss={initialCss}
+ onChange={onChange}
+ />,
+ ),
);
userEvent.click(screen.getByRole('button', { name: 'Click' }));
expect(onChange).not.toHaveBeenCalled();
@@ -82,12 +96,8 @@ test('triggers onChange when using the editor', () => {
test('triggers onChange when selecting a template', async () => {
const onChange = jest.fn();
- render(
- <CssEditor
- triggerNode={<>Click</>}
- templates={templates}
- onChange={onChange}
- />,
+ await waitFor(() =>
+ render(<CssEditor {...defaultProps} onChange={onChange} />),
);
userEvent.click(screen.getByRole('button', { name: 'Click' }));
userEvent.click(screen.getByText('Load a CSS template'));
diff --git a/superset-frontend/src/dashboard/components/CssEditor/index.jsx b/superset-frontend/src/dashboard/components/CssEditor/index.jsx
index ad12cb6c78..9fcd1768a8 100644
--- a/superset-frontend/src/dashboard/components/CssEditor/index.jsx
+++ b/superset-frontend/src/dashboard/components/CssEditor/index.jsx
@@ -21,7 +21,7 @@ import PropTypes from 'prop-types';
import { AntdDropdown } from 'src/components';
import { Menu } from 'src/components/Menu';
import Button from 'src/components/Button';
-import { t, styled } from '@superset-ui/core';
+import { t, styled, SupersetClient } from '@superset-ui/core';
import ModalTrigger from 'src/components/ModalTrigger';
import { CssEditor as AceCssEditor } from 'src/components/AsyncAceEditor';
@@ -47,7 +47,7 @@ const propTypes = {
initialCss: PropTypes.string,
triggerNode: PropTypes.node.isRequired,
onChange: PropTypes.func,
- templates: PropTypes.array,
+ addDangerToast: PropTypes.func.isRequired,
};
const defaultProps = {
@@ -60,6 +60,7 @@ class CssEditor extends React.PureComponent {
super(props);
this.state = {
css: props.initialCss,
+ templates: [],
};
this.changeCss = this.changeCss.bind(this);
this.changeCssTemplate = this.changeCssTemplate.bind(this);
@@ -67,6 +68,22 @@ class CssEditor extends React.PureComponent {
componentDidMount() {
AceCssEditor.preload();
+
+ SupersetClient.get({ endpoint: '/csstemplateasyncmodelview/api/read' })
+ .then(({ json }) => {
+ const templates = json.result.map(row => ({
+ value: row.template_name,
+ css: row.css,
+ label: row.template_name,
+ }));
+
+ this.setState({ templates });
+ })
+ .catch(() => {
+ this.props.addDangerToast(
+ t('An error occurred while fetching available CSS templates'),
+ );
+ });
}
changeCss(css) {
@@ -80,10 +97,10 @@ class CssEditor extends React.PureComponent {
}
renderTemplateSelector() {
- if (this.props.templates) {
+ if (this.state.templates) {
const menu = (
<Menu onClick={this.changeCssTemplate}>
- {this.props.templates.map(template => (
+ {this.state.templates.map(template => (
<Menu.Item key={template.css}>{template.label}</Menu.Item>
))}
</Menu>
diff --git a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx
index f1a3f59039..1926a975bb 100644
--- a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx
+++ b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx
@@ -19,7 +19,7 @@
import React from 'react';
import PropTypes from 'prop-types';
import { isEmpty } from 'lodash';
-import { SupersetClient, t } from '@superset-ui/core';
+import { t } from '@superset-ui/core';
import { Menu } from 'src/components/Menu';
import { URL_PARAMS } from 'src/constants';
import ShareMenuItems from 'src/dashboard/components/menu/ShareMenuItems';
@@ -99,7 +99,6 @@ class HeaderActionsDropdown extends React.PureComponent {
super(props);
this.state = {
css: props.customCss,
- cssTemplates: [],
showReportSubMenu: null,
};
@@ -109,23 +108,6 @@ class HeaderActionsDropdown extends React.PureComponent {
this.setShowReportSubMenu = this.setShowReportSubMenu.bind(this);
}
- UNSAFE_componentWillMount() {
- SupersetClient.get({ endpoint: '/csstemplateasyncmodelview/api/read' })
- .then(({ json }) => {
- const cssTemplates = json.result.map(row => ({
- value: row.template_name,
- css: row.css,
- label: row.template_name,
- }));
- this.setState({ cssTemplates });
- })
- .catch(() => {
- this.props.addDangerToast(
- t('An error occurred while fetching available CSS templates'),
- );
- });
- }
-
UNSAFE_componentWillReceiveProps(nextProps) {
if (this.props.customCss !== nextProps.customCss) {
this.setState({ css: nextProps.customCss }, () => {
@@ -257,8 +239,8 @@ class HeaderActionsDropdown extends React.PureComponent {
<CssEditor
triggerNode={<span>{t('Edit CSS')}</span>}
initialCss={this.state.css}
- templates={this.state.cssTemplates}
onChange={this.changeCss}
+ addDangerToast={addDangerToast}
/>
</Menu.Item>
)}
(superset) 04/05: fix(webpack): remove double-dotted file extensions in webpack config (#27471)
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 0ee452e9f90198fe8e5e7d5cd2cbd4f31300f173
Author: Evan Rusackas <ev...@preset.io>
AuthorDate: Tue Mar 12 11:26:49 2024 -0600
fix(webpack): remove double-dotted file extensions in webpack config (#27471)
(cherry picked from commit 47ae9d4cc3ca94332cb14359fb8a306f91c2da60)
---
superset-frontend/webpack.config.js | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/superset-frontend/webpack.config.js b/superset-frontend/webpack.config.js
index d531a01cc9..d71fa2a195 100644
--- a/superset-frontend/webpack.config.js
+++ b/superset-frontend/webpack.config.js
@@ -403,7 +403,7 @@ const config = {
},
type: 'asset',
generator: {
- filename: '[name].[contenthash:8].[ext]',
+ filename: '[name].[contenthash:8][ext]',
},
},
{
@@ -431,7 +431,7 @@ const config = {
test: /\.(jpg|gif)$/,
type: 'asset/resource',
generator: {
- filename: '[name].[contenthash:8].[ext]',
+ filename: '[name].[contenthash:8][ext]',
},
},
/* for font-awesome */