This is an automated email from the ASF dual-hosted git repository. villebro pushed a commit to branch 1.3 in repository https://gitbox.apache.org/repos/asf/superset.git
commit 52f747b3fcce4dc525d30fb7a20c88dd692c93c0 Author: Beto Dealmeida <[email protected]> AuthorDate: Thu Aug 12 18:05:27 2021 -0700 fix: validate_parameters and query (#16241) * fix: validate_parameters and query * add onQueryChange (cherry picked from commit 5d3d6b6eae819d114e1942f445c5ce8c9933bee8) --- .../DatabaseModal/DatabaseConnectionForm.tsx | 15 ++-- .../data/database/DatabaseModal/ExtraOptions.tsx | 4 +- .../CRUD/data/database/DatabaseModal/index.tsx | 80 +++++++++++----------- .../src/views/CRUD/data/database/types.ts | 16 ++--- 4 files changed, 62 insertions(+), 53 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx index f6997fc..f037abf 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx @@ -61,6 +61,8 @@ interface FieldPropTypes { 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; @@ -415,15 +417,15 @@ const queryField = ({ db, }: FieldPropTypes) => ( <ValidatedInput - id="query" - name="query" + id="query_input" + name="query_input" required={required} - value={db?.parameters?.query} + value={db?.query_input || ''} validationMethods={{ onBlur: getValidation }} errorMessage={validationErrors?.query} placeholder="e.g. param1=value1¶m2=value2" label="Additional Parameters" - onChange={changeMethods.onParametersChange} + onChange={changeMethods.onQueryChange} helpText={t('Add additional custom parameters')} /> ); @@ -475,6 +477,7 @@ const DatabaseConnectionForm = ({ dbModel: { parameters }, onParametersChange, onChange, + onQueryChange, onParametersUploadFileChange, onAddTableCatalog, onRemoveTableCatalog, @@ -496,6 +499,9 @@ const DatabaseConnectionForm = ({ onChange: ( event: FormEvent<InputProps> | { target: HTMLInputElement }, ) => void; + onQueryChange: ( + event: FormEvent<InputProps> | { target: HTMLInputElement }, + ) => void; onParametersUploadFileChange?: ( event: FormEvent<InputProps> | { target: HTMLInputElement }, ) => void; @@ -523,6 +529,7 @@ const DatabaseConnectionForm = ({ changeMethods: { onParametersChange, onChange, + onQueryChange, onParametersUploadFileChange, onAddTableCatalog, onRemoveTableCatalog, 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 0e1e3ce..03c82f4 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx @@ -169,9 +169,9 @@ const ExtraOptions = ({ <StyledInputContainer css={no_margin_bottom}> <div className="input-container"> <IndeterminateCheckbox - id="cost_query_enabled" + id="cost_estimate_enabled" indeterminate={false} - checked={!!db?.extra_json?.cost_query_enabled} + checked={!!db?.extra_json?.cost_estimate_enabled} onChange={onExtraInputChange} labelText={t('Enable query cost estimation')} /> 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 54c9702..7bc50f7 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -146,6 +146,7 @@ enum ActionType { extraEditorChange, addTableCatalogSheet, removeTableCatalogSheet, + queryChange, } interface DBReducerPayloadType { @@ -163,6 +164,7 @@ type DBReducerActionType = | ActionType.extraEditorChange | ActionType.extraInputChange | ActionType.textChange + | ActionType.queryChange | ActionType.inputChange | ActionType.editorChange | ActionType.parametersChange; @@ -205,7 +207,8 @@ function dbReducer( const trimmedState = { ...(state || {}), }; - let query = ''; + let query = {}; + let query_input = ''; let deserializeExtraJSON = {}; let extra_json: DatabaseObject['extra_json']; @@ -318,6 +321,15 @@ function dbReducer( ...trimmedState, [action.payload.name]: action.payload.json, }; + case ActionType.queryChange: + return { + ...trimmedState, + parameters: { + ...trimmedState.parameters, + query: Object.fromEntries(new URLSearchParams(action.payload.value)), + }, + query_input: action.payload.value, + }; case ActionType.textChange: return { ...trimmedState, @@ -339,16 +351,17 @@ function dbReducer( }; } + // convert query to a string and store in query_input + query = action.payload?.parameters?.query || {}; + query_input = Object.entries(query) + .map(([key, value]) => `${key}=${value}`) + .join('&'); + if ( action.payload.backend === 'bigquery' && action.payload.configuration_method === CONFIGURATION_METHOD.DYNAMIC_FORM ) { - // convert query into URI params string - query = new URLSearchParams( - action?.payload?.parameters?.query as string, - ).toString(); - return { ...action.payload, engine: action.payload.backend, @@ -360,6 +373,7 @@ function dbReducer( ), query, }, + query_input, }; } @@ -381,37 +395,18 @@ function dbReducer( name: e, value: engineParamsCatalog[e], })), + query_input, } as DatabaseObject; } - if (action.payload?.parameters?.query) { - // convert query into URI params string - query = new URLSearchParams( - action.payload.parameters.query as string, - ).toString(); - - return { - ...action.payload, - encrypted_extra: action.payload.encrypted_extra || '', - engine: action.payload.backend || trimmedState.engine, - configuration_method: action.payload.configuration_method, - extra_json: deserializeExtraJSON, - parameters: { - ...action.payload.parameters, - query, - }, - }; - } - return { ...action.payload, encrypted_extra: action.payload.encrypted_extra || '', engine: action.payload.backend || trimmedState.engine, configuration_method: action.payload.configuration_method, extra_json: deserializeExtraJSON, - parameters: { - ...action.payload.parameters, - }, + parameters: action.payload.parameters, + query_input, }; case ActionType.dbSelected: @@ -539,17 +534,6 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ const dbToUpdate = JSON.parse(JSON.stringify(update)); if (dbToUpdate.configuration_method === CONFIGURATION_METHOD.DYNAMIC_FORM) { - if (dbToUpdate?.parameters?.query) { - // convert query params into dictionary - const urlParams = new URLSearchParams(dbToUpdate?.parameters?.query); - dbToUpdate.parameters.query = Object.fromEntries(urlParams); - } else if ( - dbToUpdate?.parameters?.query === '' && - 'query' in dbModel.parameters.properties - ) { - dbToUpdate.parameters.query = {}; - } - // Validate DB before saving await getValidation(dbToUpdate, true); if (validationErrors && !isEmpty(validationErrors)) { @@ -974,6 +958,12 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ value: target.value, }) } + onQueryChange={({ target }: { target: HTMLInputElement }) => + onChange(ActionType.queryChange, { + name: target.name, + value: target.value, + }) + } onAddTableCatalog={() => setDB({ type: ActionType.addTableCatalogSheet }) } @@ -1095,6 +1085,12 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ value: target.value, }) } + onQueryChange={({ target }: { target: HTMLInputElement }) => + onChange(ActionType.queryChange, { + name: target.name, + value: target.value, + }) + } onAddTableCatalog={() => setDB({ type: ActionType.addTableCatalogSheet }) } @@ -1241,6 +1237,12 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ onAddTableCatalog={() => { setDB({ type: ActionType.addTableCatalogSheet }); }} + onQueryChange={({ target }: { target: HTMLInputElement }) => + onChange(ActionType.queryChange, { + name: target.name, + value: target.value, + }) + } onRemoveTableCatalog={(idx: number) => { setDB({ type: ActionType.removeTableCatalogSheet, diff --git a/superset-frontend/src/views/CRUD/data/database/types.ts b/superset-frontend/src/views/CRUD/data/database/types.ts index f6341d3..d473ba5 100644 --- a/superset-frontend/src/views/CRUD/data/database/types.ts +++ b/superset-frontend/src/views/CRUD/data/database/types.ts @@ -45,15 +45,12 @@ export type DatabaseObject = { password?: string; encryption?: boolean; credentials_info?: string; - query?: string | object; - catalog?: {}; + query?: Record<string, string>; + catalog?: Record<string, string>; }; configuration_method: CONFIGURATION_METHOD; engine?: string; - // Gsheets temporary storage - catalog?: Array<CatalogObject>; - // Performance cache_timeout?: string; allow_run_async?: boolean; @@ -85,11 +82,14 @@ export type DatabaseObject = { allows_virtual_table_explore?: boolean; // in SQL Lab schemas_allowed_for_csv_upload?: string[]; // in Security cancel_query_on_windows_unload?: boolean; // in Performance - version?: string; - // todo: ask beto where this should live - cost_query_enabled?: boolean; // in SQL Lab + version?: string; + cost_estimate_enabled?: boolean; // in SQL Lab }; + + // Temporary storage + catalog?: Array<CatalogObject>; + query_input?: string; extra?: string; };
