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 f6d3e239a6664..243ee27fe2241 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx @@ -300,7 +300,7 @@ const ExtraOptions = ({ 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 a65263db5a4d2..2bd7ed18664c8 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 @@ -27,7 +27,6 @@ import { act, } from 'spec/helpers/testing-library'; import * as hooks from 'src/views/CRUD/hooks'; -import DatabaseModal from './index'; import { DatabaseObject, CONFIGURATION_METHOD, @@ -658,8 +657,6 @@ describe('DatabaseModal', () => { checkboxOffSVGs[2], checkboxOffSVGs[3], checkboxOffSVGs[4], - checkboxOffSVGs[5], - checkboxOffSVGs[6], tooltipIcons[0], tooltipIcons[1], tooltipIcons[2], @@ -688,14 +685,13 @@ describe('DatabaseModal', () => { allowDbExplorationCheckbox, disableSQLLabDataPreviewQueriesCheckbox, ]; - visibleComponents.forEach(component => { expect(component).toBeVisible(); }); invisibleComponents.forEach(component => { expect(component).not.toBeVisible(); }); - expect(checkboxOffSVGs).toHaveLength(7); + expect(checkboxOffSVGs).toHaveLength(5); expect(tooltipIcons).toHaveLength(7); }); @@ -1311,6 +1307,7 @@ describe('DatabaseModal', () => { createResource: jest.fn(), updateResource: jest.fn(), clearError: jest.fn(), + setResource: jest.fn(), }); const renderAndWait = async () => { @@ -1355,6 +1352,7 @@ describe('DatabaseModal', () => { createResource: jest.fn(), updateResource: jest.fn(), clearError: jest.fn(), + setResource: jest.fn(), }); const renderAndWait = async () => { @@ -1603,6 +1601,88 @@ describe('dbReducer', () => { foo: true, }); }); + + test('it will change state to payload from input change for checkbox', () => { + const action: DBReducerActionType = { + type: ActionType.inputChange, + payload: { name: 'allow_ctas', type: 'checkbox', checked: false }, + }; + const currentState = dbReducer( + { + ...databaseFixture, + allow_ctas: true, + }, + action, + ); + + expect(currentState).toEqual({ + ...databaseFixture, + allow_ctas: false, + }); + }); + + test('it will add a parameter', () => { + const action: DBReducerActionType = { + type: ActionType.parametersChange, + payload: { name: 'host', value: '127.0.0.1' }, + }; + const currentState = dbReducer(databaseFixture, action); + + expect(currentState).toEqual({ + ...databaseFixture, + parameters: { + host: '127.0.0.1', + }, + }); + }); + + test('it will add a parameter with existing parameters', () => { + const action: DBReducerActionType = { + type: ActionType.parametersChange, + payload: { name: 'port', value: '1234' }, + }; + const currentState = dbReducer( + { + ...databaseFixture, + parameters: { + host: '127.0.0.1', + }, + }, + action, + ); + + expect(currentState).toEqual({ + ...databaseFixture, + parameters: { + host: '127.0.0.1', + port: '1234', + }, + }); + }); + + test('it will change a parameter with existing parameters', () => { + const action: DBReducerActionType = { + type: ActionType.parametersChange, + payload: { name: 'host', value: 'localhost' }, + }; + const currentState = dbReducer( + { + ...databaseFixture, + parameters: { + host: '127.0.0.1', + }, + }, + action, + ); + + expect(currentState).toEqual({ + ...databaseFixture, + parameters: { + host: 'localhost', + }, + }); + }); + test('it will set state to payload from parametersChange with catalog', () => { const action: DBReducerActionType = { type: ActionType.parametersChange, @@ -1633,7 +1713,9 @@ describe('dbReducer', () => { expect(currentState).toEqual({ ...databaseFixture, parameters: { - foo: 'bar', + catalog: { + '': 'bar', + }, }, }); }); 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 d1db4d313f92b..1c7c5e0d05fd7 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -89,6 +89,8 @@ import { } from './styles'; import ModalHeader, { DOCUMENTATION_LINK } from './ModalHeader'; +const DEFAULT_EXTRA = JSON.stringify({ allows_virtual_table_explore: true }); + const engineSpecificAlertMapping = { [Engines.GSheet]: { message: 'Why do I need to create a database?', @@ -211,12 +213,12 @@ export function dbReducer( }; let query = {}; let query_input = ''; - let extraJson: ExtraJson; + let parametersCatalog; + const extraJson: ExtraJson = JSON.parse(trimmedState.extra || '{}'); switch (action.type) { case ActionType.extraEditorChange: // "extra" payload in state is a string - extraJson = JSON.parse(trimmedState.extra || '{}'); return { ...trimmedState, extra: JSON.stringify({ @@ -226,7 +228,6 @@ export function dbReducer( }; case ActionType.extraInputChange: // "extra" payload in state is a string - extraJson = JSON.parse(trimmedState.extra || '{}'); if ( action.payload.name === 'schema_cache_timeout' || action.payload.name === 'table_cache_timeout' @@ -275,26 +276,44 @@ export function dbReducer( [action.payload.name]: action.payload.value, }; case ActionType.parametersChange: - if ( - trimmedState.catalog !== undefined && - action.payload.type?.startsWith('catalog') - ) { - // Formatting wrapping google sheets table catalog - const idx = action.payload.type?.split('-')[1]; - const catalogToUpdate = trimmedState?.catalog[idx] || {}; - catalogToUpdate[action.payload.name] = action.payload.value; - - const paramatersCatalog = {}; - // eslint-disable-next-line array-callback-return - trimmedState.catalog?.map((item: CatalogObject) => { - paramatersCatalog[item.name] = item.value; - }); - + if (action.payload.type?.startsWith('catalog')) { + if (trimmedState.catalog !== undefined) { + // Formatting wrapping google sheets table catalog + const catalogCopy: CatalogObject[] = [...trimmedState.catalog]; + const idx = action.payload.type?.split('-')[1]; + const catalogToUpdate: CatalogObject = catalogCopy[idx] || {}; + catalogToUpdate[action.payload.name] = action.payload.value; + + // insert updated catalog to existing state + catalogCopy.splice(parseInt(idx, 10), 1, catalogToUpdate); + + // format catalog for state + // eslint-disable-next-line array-callback-return + parametersCatalog = catalogCopy.reduce((obj, item: any) => { + const catalog = { ...obj }; + catalog[item.name] = item.value; + return catalog; + }, {}); + + return { + ...trimmedState, + catalog: catalogCopy, + parameters: { + ...trimmedState.parameters, + catalog: parametersCatalog, + }, + }; + } + if (action.payload.name === 'name') { + parametersCatalog = { [action.payload.value as string]: undefined }; + } else { + parametersCatalog = { '': action.payload.value }; + } return { ...trimmedState, parameters: { ...trimmedState.parameters, - catalog: paramatersCatalog, + catalog: parametersCatalog, }, }; } @@ -305,6 +324,7 @@ export function dbReducer( [action.payload.name]: action.payload.value, }, }; + case ActionType.addTableCatalogSheet: if (trimmedState.catalog !== undefined) { return { @@ -353,21 +373,25 @@ export function dbReducer( CONFIGURATION_METHOD.DYNAMIC_FORM ) { // "extra" payload from the api is a string - extraJson = { + const extraJsonPayload: ExtraJson = { ...JSON.parse((action.payload.extra as string) || '{}'), }; - const engineParamsCatalog = Object.entries( - extraJson?.engine_params?.catalog || {}, - ).map(([key, value]) => ({ - name: key, - value, - })); + + const payloadCatalog = extraJsonPayload.engine_params?.catalog; + + const engineRootCatalog = Object.entries(payloadCatalog || {}).map( + ([name, value]: string[]) => ({ name, value }), + ); + return { ...action.payload, engine: action.payload.backend || trimmedState.engine, configuration_method: action.payload.configuration_method, - catalog: engineParamsCatalog, - parameters: action.payload.parameters || trimmedState.parameters, + catalog: engineRootCatalog, + parameters: { + ...(action.payload.parameters || trimmedState.parameters), + catalog: payloadCatalog, + }, query_input, }; } @@ -381,6 +405,12 @@ export function dbReducer( }; case ActionType.dbSelected: + // set initial state for blank form + return { + ...action.payload, + extra: DEFAULT_EXTRA, + expose_in_sqllab: true, + }; case ActionType.configMethodChange: return { ...action.payload, @@ -545,20 +575,17 @@ const DatabaseModal: FunctionComponent = ({ // Clone DB object const dbToUpdate = { ...(db || {}) }; - if (dbToUpdate.catalog) { - // convert catalog to fit /validate_parameters endpoint - dbToUpdate.catalog = Object.assign( - {}, - ...dbToUpdate.catalog.map((x: { name: string; value: string }) => ({ - [x.name]: x.value, - })), - ); - } else { - dbToUpdate.catalog = {}; - } - if (dbToUpdate.configuration_method === CONFIGURATION_METHOD.DYNAMIC_FORM) { // Validate DB before saving + if (dbToUpdate?.parameters?.catalog) { + // need to stringify gsheets catalog to allow it to be serialized + dbToUpdate.extra = JSON.stringify({ + ...JSON.parse(dbToUpdate.extra || '{}'), + engine_params: { + catalog: dbToUpdate.parameters.catalog, + }, + }); + } const errors = await getValidation(dbToUpdate, true); if ((validationErrors && !isEmpty(validationErrors)) || errors) { return; @@ -607,6 +634,16 @@ const DatabaseModal: FunctionComponent = ({ } } + if (dbToUpdate?.parameters?.catalog) { + // need to stringify gsheets catalog to allow it to be seralized + dbToUpdate.extra = JSON.stringify({ + ...JSON.parse(dbToUpdate.extra || '{}'), + engine_params: { + catalog: dbToUpdate.parameters.catalog, + }, + }); + } + setLoading(true); if (db?.id) { const result = await updateResource( diff --git a/superset-frontend/src/views/CRUD/data/database/types.ts b/superset-frontend/src/views/CRUD/data/database/types.ts index de74333040be0..373e0dbf83981 100644 --- a/superset-frontend/src/views/CRUD/data/database/types.ts +++ b/superset-frontend/src/views/CRUD/data/database/types.ts @@ -51,7 +51,7 @@ export type DatabaseObject = { credentials_info?: string; service_account_info?: string; query?: Record; - catalog?: Record; + catalog?: Record; properties?: Record; warehouse?: string; role?: string; @@ -158,7 +158,7 @@ export interface ExtraJson { cost_estimate_enabled?: boolean; // in SQL Lab disable_data_preview?: boolean; // in SQL Lab engine_params?: { - catalog?: Record; + catalog?: Record; connect_args?: { http_path?: string; };