You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by el...@apache.org on 2022/12/12 20:36:50 UTC
[superset] branch master updated: fix: make database connection modal ace fields uncontrolled (#22350)
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 608ffcbfb9 fix: make database connection modal ace fields uncontrolled (#22350)
608ffcbfb9 is described below
commit 608ffcbfb9d91aa44cdca77cc1b08fcb610209b8
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Mon Dec 12 12:36:41 2022 -0800
fix: make database connection modal ace fields uncontrolled (#22350)
---
.../data/database/DatabaseModal/ExtraOptions.tsx | 35 ++++++++-----
.../data/database/DatabaseModal/index.test.tsx | 60 +++++++++-------------
.../CRUD/data/database/DatabaseModal/index.tsx | 12 ++++-
3 files changed, 57 insertions(+), 50 deletions(-)
diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx
index 8bff977d13..c8c03878cd 100644
--- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx
+++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx
@@ -50,7 +50,16 @@ const ExtraOptions = ({
const createAsOpen = !!(db?.allow_ctas || db?.allow_cvas);
const isFileUploadSupportedByEngine =
db?.engine_information?.supports_file_upload;
- const extraJson: ExtraJson = JSON.parse(db?.extra || '{}');
+
+ // JSON.parse will deep parse engine_params
+ // if it's an object, and we want to keep it a string
+ const extraJson: ExtraJson = JSON.parse(db?.extra || '{}', (key, value) => {
+ if (key === 'engine_params' && typeof value === 'object') {
+ // keep this as a string
+ return JSON.stringify(value);
+ }
+ return value;
+ });
return (
<Collapse
@@ -123,9 +132,9 @@ const ExtraOptions = ({
<input
type="text"
name="force_ctas_schema"
- value={db?.force_ctas_schema || ''}
placeholder={t('Create or select schema...')}
onChange={onInputChange}
+ value={db?.force_ctas_schema || ''}
/>
</div>
<div className="helper">
@@ -442,17 +451,17 @@ const ExtraOptions = ({
<div className="input-container">
<StyledJsonEditor
name="metadata_params"
- value={
- !Object.keys(extraJson?.metadata_params || {}).length
- ? ''
- : extraJson?.metadata_params
- }
placeholder={t('Metadata Parameters')}
onChange={(json: string) =>
onExtraEditorChange({ json, name: 'metadata_params' })
}
width="100%"
height="160px"
+ defaultValue={
+ !Object.keys(extraJson?.metadata_params || {}).length
+ ? ''
+ : extraJson?.metadata_params
+ }
/>
</div>
<div className="helper">
@@ -468,17 +477,17 @@ const ExtraOptions = ({
<div className="input-container">
<StyledJsonEditor
name="engine_params"
- value={
- !Object.keys(extraJson?.engine_params || {}).length
- ? ''
- : extraJson?.engine_params
- }
placeholder={t('Engine Parameters')}
onChange={(json: string) =>
onExtraEditorChange({ json, name: 'engine_params' })
}
width="100%"
height="160px"
+ defaultValue={
+ !Object.keys(extraJson?.engine_params || {}).length
+ ? ''
+ : extraJson?.engine_params
+ }
/>
</div>
<div className="helper">
@@ -497,9 +506,9 @@ const ExtraOptions = ({
<input
type="number"
name="version"
- value={extraJson?.version || ''}
placeholder={t('Version number')}
onChange={onExtraInputChange}
+ value={extraJson?.version || ''}
/>
</div>
<div className="helper">
diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.tsx
index c279ea8851..79b0bd6269 100644
--- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.tsx
+++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.tsx
@@ -25,12 +25,13 @@ import {
within,
cleanup,
act,
+ waitFor,
} from 'spec/helpers/testing-library';
-import * as hooks from 'src/views/CRUD/hooks';
import {
DatabaseObject,
CONFIGURATION_METHOD,
} from 'src/views/CRUD/data/database/types';
+import * as hooks from 'src/views/CRUD/hooks';
import DatabaseModal, {
dbReducer,
DBReducerActionType,
@@ -47,6 +48,20 @@ const dbProps = {
const DATABASE_FETCH_ENDPOINT = 'glob:*/api/v1/database/10';
const AVAILABLE_DB_ENDPOINT = 'glob:*/api/v1/database/available*';
const VALIDATE_PARAMS_ENDPOINT = 'glob:*/api/v1/database/validate_parameters*';
+const DATABASE_CONNECT_ENDPOINT = 'glob:*/api/v1/database/';
+
+fetchMock.post(DATABASE_CONNECT_ENDPOINT, {
+ id: 10,
+ result: {
+ configuration_method: 'sqlalchemy_form',
+ database_name: 'Other2',
+ driver: 'apsw',
+ expose_in_sqllab: true,
+ extra: '{"allows_virtual_table_explore":true}',
+ sqlalchemy_uri: 'gsheets://',
+ },
+ json: 'foo',
+});
fetchMock.config.overwriteRoutes = true;
fetchMock.get(DATABASE_FETCH_ENDPOINT, {
@@ -1178,6 +1193,7 @@ describe('DatabaseModal', () => {
const databaseNameField = textboxes[1];
const usernameField = textboxes[2];
const passwordField = textboxes[3];
+ const connectButton = screen.getByRole('button', { name: 'Connect' });
expect(hostField).toHaveValue('');
expect(portField).toHaveValue(null);
@@ -1198,19 +1214,10 @@ describe('DatabaseModal', () => {
expect(usernameField).toHaveValue('testdb');
expect(passwordField).toHaveValue('demoPassword');
- /* ---------- 🐞 TODO (lyndsiWilliams): function mock is not currently working 🐞 ----------
-
- // Mock useSingleViewResource
- const mockUseSingleViewResource = jest.fn();
- mockUseSingleViewResource.mockImplementation(useSingleViewResource);
-
- const { fetchResource } = mockUseSingleViewResource('database');
-
- // Invalid hook call?
- userEvent.click(screen.getByRole('button', { name: 'Connect' }));
- expect(fetchResource).toHaveBeenCalled();
-
- */
+ userEvent.click(connectButton);
+ await waitFor(() => {
+ expect(fetchMock.calls(VALIDATE_PARAMS_ENDPOINT).length).toEqual(6);
+ });
});
});
@@ -1325,23 +1332,6 @@ describe('DatabaseModal', () => {
...jest.requireActual('src/views/CRUD/hooks'),
useSingleViewResource: jest.fn(),
}));
- const useSingleViewResourceMock = jest.spyOn(
- hooks,
- 'useSingleViewResource',
- );
-
- useSingleViewResourceMock.mockReturnValue({
- state: {
- loading: false,
- resource: null,
- error: { _schema: 'Test Error With Object' },
- },
- fetchResource: jest.fn(),
- createResource: jest.fn(),
- updateResource: jest.fn(),
- clearError: jest.fn(),
- setResource: jest.fn(),
- });
const renderAndWait = async () => {
const mounted = act(async () => {
@@ -1442,7 +1432,7 @@ describe('dbReducer', () => {
test('it will set state to payload from extra editor', () => {
const action: DBReducerActionType = {
type: ActionType.extraEditorChange,
- payload: { name: 'foo', json: { bar: 1 } },
+ payload: { name: 'foo', json: JSON.stringify({ bar: 1 }) },
};
const currentState = dbReducer(databaseFixture, action);
// extra should be serialized
@@ -1455,20 +1445,20 @@ describe('dbReducer', () => {
test('it will set state to payload from editor', () => {
const action: DBReducerActionType = {
type: ActionType.editorChange,
- payload: { name: 'foo', json: { bar: 1 } },
+ payload: { name: 'foo', json: JSON.stringify({ bar: 1 }) },
};
const currentState = dbReducer(databaseFixture, action);
// extra should be serialized
expect(currentState).toEqual({
...databaseFixture,
- foo: { bar: 1 },
+ foo: JSON.stringify({ bar: 1 }),
});
});
test('it will add extra payload to existing extra data', () => {
const action: DBReducerActionType = {
type: ActionType.extraEditorChange,
- payload: { name: 'foo', json: { bar: 1 } },
+ payload: { name: 'foo', json: JSON.stringify({ bar: 1 }) },
};
// extra should be a string
const currentState = dbReducer(
diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
index 7cd030dab4..095e4f0225 100644
--- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
+++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
@@ -150,7 +150,7 @@ export enum ActionType {
interface DBReducerPayloadType {
target?: string;
name: string;
- json?: {};
+ json?: string;
type?: string;
checked?: boolean;
value?: string;
@@ -215,20 +215,28 @@ export function dbReducer(
let query = {};
let query_input = '';
let parametersCatalog;
+ let actionPayloadJson;
const extraJson: ExtraJson = JSON.parse(trimmedState.extra || '{}');
switch (action.type) {
case ActionType.extraEditorChange:
// "extra" payload in state is a string
+ try {
+ // we don't want to stringify encoded strings twice
+ actionPayloadJson = JSON.parse(action.payload.json || '{}');
+ } catch (e) {
+ actionPayloadJson = action.payload.json;
+ }
return {
...trimmedState,
extra: JSON.stringify({
...extraJson,
- [action.payload.name]: action.payload.json,
+ [action.payload.name]: actionPayloadJson,
}),
};
case ActionType.extraInputChange:
// "extra" payload in state is a string
+
if (
action.payload.name === 'schema_cache_timeout' ||
action.payload.name === 'table_cache_timeout'