rebenitez1802 commented on code in PR #45843:
URL: https://github.com/apache/superset/pull/45843#discussion_r4427915423
##########
superset-frontend/src/features/databases/DatabaseModal/index.tsx:
##########
@@ -1522,13 +1540,14 @@ const DatabaseModal:
FunctionComponent<DatabaseModalProps> = ({
if (dbFetched) {
setDB({
type: ActionType.Fetched,
+ isEditMode,
payload: dbFetched,
});
// keep a copy of the name separate for display purposes
// because it shouldn't change when the form is updated
setDbName(dbFetched.database_name);
}
- }, [dbFetched]);
+ }, [dbFetched, isEditMode]);
Review Comment:
🔴 High — Closing an edit modal reloads that connection into "+ Database",
and saving overwrites it
`DatabaseList` keeps a single `<DatabaseModal
databaseId={currentDatabase?.id}>` mounted for both edit and create, and
`useSingleViewResource` never clears `dbFetched`. When an edit closes,
`onClose()` dispatches `Reset` and then `onHide()` clears `currentDatabase`.
That flips `isEditMode` from true to false, so this effect runs again with the
stale `dbFetched` and puts the old record (including its `id`) back into `db`
while the modal is hidden. Clicking "+ Database" then skips the picker (`!db`
is false), shows the old name and URI, and `onSave` follows the `db?.id`
branch, calling `updateResource(<old id>)`. The existing connection, and every
dataset and chart on it, is repointed at the new URI.
Repro (Jest harness that mirrors `DatabaseList`): open edit for id 10,
close, open "+ Database", enter a new name and URI, Test, Connect. At HEAD this
sends `PUT /api/v1/database/10` with `{"id":10,"database_name":"My new
sandbox",...}`. With `[dbFetched]` (the deps on master and at 54f95ada) no PUT
is sent and the blank picker renders.
`isEditMode` only needs its value at the moment the fetch lands, so it can
stay out of the deps. The closure already has the current value.
(`react-hooks/exhaustive-deps` is a warning, and CI runs oxlint with
`--quiet`.) A regression test along the lines of the repro above would pin this.
```suggestion
}, [dbFetched]);
```
--
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]
