betodealmeida commented on a change in pull request #14881:
URL: https://github.com/apache/superset/pull/14881#discussion_r660816056
##########
File path:
superset-frontend/cypress-base/cypress/integration/database/modal.test.ts
##########
@@ -17,75 +17,54 @@
* under the License.
*/
import { DATABASE_LIST } from './helper';
-
+// TODO: Add new tests with the modal
Review comment:
Can we assign this TODO to someone?
When I leave a TODO I write:
```
TODO (betodealmeida): do XXX
```
So I can easily `grep` for the things I'm responsible for. I've seen other
people doing the same.
##########
File path: docs/src/pages/docs/Miscellaneous/issue_codes.mdx
##########
@@ -207,3 +207,11 @@ The submitted payload has the incorrect schema.
```
Please check that the request payload has the expected schema.
+
+## Issue 1021
Review comment:
We already have an issue 1021, we need to update this.
##########
File path:
superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx
##########
@@ -68,47 +222,56 @@ const portField = ({
changeMethods,
getValidation,
validationErrors,
+ db,
}: FieldPropTypes) => (
- <ValidatedInput
- id="port"
- name="port"
- required={required}
- validationMethods={{ onBlur: getValidation }}
- errorMessage={validationErrors?.port}
- placeholder="e.g. 5432"
- className="form-group-w-50"
- label="Port"
- onChange={changeMethods.onParametersChange}
- />
+ <>
+ <ValidatedInput
+ id="port"
+ name="port"
+ type="number"
+ required={required}
+ value={db?.parameters?.port as number}
+ validationMethods={{ onBlur: getValidation }}
+ errorMessage={validationErrors?.port}
+ placeholder="e.g. 5432"
+ className="form-group-w-50"
+ label="Port"
+ onChange={changeMethods.onParametersChange}
+ />
+ </>
);
const databaseField = ({
required,
changeMethods,
getValidation,
validationErrors,
+ db,
}: FieldPropTypes) => (
<ValidatedInput
id="database"
name="database"
required={required}
+ value={db?.parameters?.database}
validationMethods={{ onBlur: getValidation }}
errorMessage={validationErrors?.database}
placeholder="e.g. world_population"
label="Database name"
onChange={changeMethods.onParametersChange}
- helpText="Copy the name of the PostgreSQL database you are trying to
connect to."
+ helpText="Copy the name of the database you are trying to connect to."
Review comment:
Same here, `t()`.
##########
File path:
superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx
##########
@@ -34,27 +45,170 @@ export const FormFieldOrder = [
'username',
'password',
'database_name',
+ 'credentials_info',
+ 'query',
+ 'encryption',
];
interface FieldPropTypes {
required: boolean;
+ hasTooltip?: boolean;
+ tooltipText?: (valuse: any) => string;
+ onParametersChange: (value: any) => string;
+ onParametersUploadFileChange: (value: any) => string;
changeMethods: { onParametersChange: (value: any) => string } & {
onChange: (value: any) => string;
- };
+ } & { onParametersUploadFileChange: (value: any) => string };
validationErrors: JsonObject | null;
getValidation: () => void;
+ db?: DatabaseObject;
+ isEditMode?: boolean;
+ sslForced?: boolean;
+ defaultDBName?: string;
+ editNewDb?: boolean;
}
+const CredentialsInfo = ({
+ changeMethods,
+ isEditMode,
+ db,
+ editNewDb,
+}: FieldPropTypes) => {
+ const [uploadOption, setUploadOption] = useState<number>(
+ CredentialInfoOptions.jsonUpload.valueOf(),
+ );
+ const [fileToUpload, setFileToUpload] = useState<string | null | undefined>(
+ null,
+ );
+ return (
+ <CredentialInfoForm>
+ {!isEditMode && (
+ <>
+ <FormLabel required>
+ How do you want to enter service account credentials?
Review comment:
We need to make these strings (here and below) translatable (with `t()`).
##########
File path: superset/errors.py
##########
@@ -156,6 +157,9 @@ class SupersetErrorType(str, Enum):
SupersetErrorType.CONNECTION_PORT_CLOSED_ERROR: [
{"code": 1008, "message": _("Issue 1008 - The port is closed.")},
],
+ SupersetErrorType.CONNECTION_INVALID_PORT_ERROR: [
+ {"code": 1021, "message": _("Issue 1021 - Port number is invalid.")},
Review comment:
Issue 1021 is taken, we need to create a new issue here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
