nytai commented on a change in pull request #10745:
URL:
https://github.com/apache/incubator-superset/pull/10745#discussion_r483297347
##########
File path: superset-frontend/src/views/CRUD/data/database/DatabaseModal.tsx
##########
@@ -82,53 +116,147 @@ const DatabaseModal:
FunctionComponent<DatabaseModalProps> = ({
show,
database = null,
}) => {
- // const [disableSave, setDisableSave] = useState(true);
- const [disableSave] = useState<boolean>(true);
- const [db, setDB] = useState<Partial<DatabaseObject> | null>(null);
+ const [disableSave, setDisableSave] = useState<boolean>(true);
+ const [db, setDB] = useState<DatabaseObject | null>(null);
const [isHidden, setIsHidden] = useState<boolean>(true);
+ const isEditMode = database !== null;
+
+ // Database fetch logic
+ const {
+ state: { loading: dbLoading, resource: dbFetched },
+ fetchResource,
+ createResource,
+ updateResource,
+ } = useSingleViewResource<DatabaseObject>(
+ 'database',
+ t('database'),
+ addDangerToast,
+ );
+
// Functions
const hide = () => {
setIsHidden(true);
onHide();
};
const onSave = () => {
- if (onDatabaseAdd) {
- onDatabaseAdd();
- }
+ if (isEditMode) {
+ // Edit
+ const update: DatabaseObject = {
+ database_name: db ? db.database_name : '',
+ sqlalchemy_uri: db ? db.sqlalchemy_uri : '',
+ ...db,
+ };
+
+ // Need to clean update object
+ if (update.id) {
+ delete update.id;
+ }
+
+ if (!update.cache_timeout) {
+ update.cache_timeout = '0';
+ }
- hide();
+ if (!update.encrypted_extra) {
+ update.encrypted_extra = '';
+ }
+
+ if (!update.force_ctas_schema) {
+ update.force_ctas_schema = '';
+ }
+
+ if (!update.server_cert) {
+ update.server_cert = '';
+ }
+
+ if (db && db.id) {
+ updateResource(db.id, update).then(() => {
+ if (onDatabaseAdd) {
+ onDatabaseAdd();
+ }
+
+ hide();
+ });
+ }
+ } else if (db) {
+ // Create
+ createResource(db).then(() => {
+ if (onDatabaseAdd) {
+ onDatabaseAdd();
+ }
+
+ hide();
+ });
+ }
};
const onInputChange = (event: React.ChangeEvent<HTMLInputElement>) => {
const target = event.target;
const data = {
database_name: db ? db.database_name : '',
- uri: db ? db.uri : '',
+ sqlalchemy_uri: db ? db.sqlalchemy_uri : '',
...db,
};
- data[target.name] = target.value;
+ if (target.type === 'checkbox') {
+ data[target.name] = target.checked;
+ } else {
+ data[target.name] = target.value;
+ }
setDB(data);
};
- const isEditMode = database !== null;
+ const onTextChange = (event: React.ChangeEvent<HTMLTextAreaElement>) => {
+ const target = event.target;
+ const data = {
+ database_name: db ? db.database_name : '',
+ sqlalchemy_uri: db ? db.sqlalchemy_uri : '',
+ ...db,
+ };
+
+ data[target.name] = target.value;
+ setDB(data);
+ };
+
+ const validate = () => {
+ if (
+ db &&
+ db.database_name.length &&
+ db.sqlalchemy_uri &&
+ db.sqlalchemy_uri.length
+ ) {
+ setDisableSave(false);
+ } else {
+ setDisableSave(true);
+ }
+ };
// Initialize
if (
isEditMode &&
(!db || !db.id || (database && database.id !== db.id) || (isHidden &&
show))
) {
- setDB(database);
+ if (database && database.id !== null && !dbLoading) {
+ const id = database.id || 0;
+
+ fetchResource(id).then(() => {
+ setDB(dbFetched);
+ });
+ }
} else if (!isEditMode && (!db || db.id || (isHidden && show))) {
setDB({
database_name: '',
- uri: '',
+ sqlalchemy_uri: '',
});
}
+ // Validation
+ useEffect(() => {
+ validate();
Review comment:
Do we really need to call this on every render? It's not an expensive
operation now, but it could become one. Would adding a dependency on `db` be a
sensible optimization?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
