aminghadersohi commented on code in PR #45843:
URL: https://github.com/apache/superset/pull/45843#discussion_r4430473316
##########
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:
My previous reply was incomplete: `fetchDB` was affected by a late edit GET,
and `DatabaseList/index.tsx:559` was not changed by
8271d43868fd2001d5f52f0fdb109bd74d8f321a. I incorrectly listed the former as
not affected and the latter as fixed.
Fixed in eac7fed0f320b5c99547dab6c1b896af8bfc7329. Connection reads capture
the modal's generation; closing, returning to the picker, changing edit targets
or unmounting invalidates that generation. `useSingleViewResource` checks it
before publishing a fetched record or error. A pending read also cannot prevent
fetching a newly selected edit target. The resource-copy effect keeps
`[dbFetched]`, so reopening does not replay the cache, and open multi-step
creates still consume their saved record.
Regression test:
`superset-frontend/src/features/databases/DatabaseModal/index.test.tsx::late
edit fetch after X then creating uses POST without the old id`.
Companion tests in the same file:
- `late edit fetch after X then creating uses POST without the old id`
- `switching edit targets loads the active target while the previous fetch
is pending`
- `keeps a created PostgreSQL field connection on step 3 without a saved
schema: Back`
I retained `closing edit then creating uses POST without the old id`, `X
edit then creating uses POST without the old id`, and `saving edit then
creating uses POST without the old id`. The create regressions check the
picker, POST without an old id, zero PUTs to the old connection,
create-after-create, and reopening edit. The hook tests `useSingleViewResource:
discarded fetch does not replace the active resource` and
`useSingleViewResource: discarded fetch errors do not update state or show a
toast` cover discarded successes/errors and preserve the returned response for
navigation callers.
Before: substituting the two production files from
8271d43868fd2001d5f52f0fdb109bd74d8f321a into the final tests yields five
failing regressions. The delayed-edit case fails at `index.test.tsx:479`:
`Expected length: 0 / Received length: 1`, a PUT to `/api/v1/database/10`
containing the new name and URI.
After: `npx jest src/features/databases/DatabaseModal src/pages/DatabaseList
--runInBand` passes 12 suites / 231 tests; `npx jest
src/views/CRUD/hooks.test.tsx --runInBand` passes 42 tests (both run from
superset-frontend).
Pre-commit passes on staged changes and the branch's changed-file list,
including mypy and frontend type checking. CI was not awaited.
Siblings checked:
- superset-frontend/src/views/CRUD/hooks.ts:430 — fixed in
eac7fed0f320b5c99547dab6c1b896af8bfc7329 (optional response-publication guard;
existing callers default to accepting responses).
- superset-frontend/src/features/databases/DatabaseModal/index.tsx:1178 —
fixed in eac7fed0f320b5c99547dab6c1b896af8bfc7329 (shared guarded read,
including fetchDB and edit-target changes at 1539).
- superset-frontend/src/features/databases/DatabaseModal/index.tsx:926 —
fixed in eac7fed0f320b5c99547dab6c1b896af8bfc7329 (Close and all onHide
variants at 2102, 2146 and 2343 invalidate pending reads, including X/cancel
and successful-save closure).
- superset-frontend/src/features/databases/DatabaseModal/index.tsx:1353 —
fixed in eac7fed0f320b5c99547dab6c1b896af8bfc7329 (Back from Finish uses the
guarded read).
- superset-frontend/src/features/databases/DatabaseModal/index.tsx:1361 —
fixed in eac7fed0f320b5c99547dab6c1b896af8bfc7329 (Back to the picker
invalidates pending reads before resetting the form).
- superset-frontend/src/features/databases/DatabaseModal/index.tsx:1919 —
fixed in eac7fed0f320b5c99547dab6c1b896af8bfc7329 (dataset/SQL Lab CTA reads
cannot repopulate another modal session; returning the response for
local-storage navigation is preserved).
- superset-frontend/src/features/databases/DatabaseModal/index.tsx:1554 —
not affected: the existing resource-copy effect remains dbFetched-only;
obsolete reads are discarded upstream rather than copied into create state.
- superset-frontend/src/features/databases/DatabaseModal/index.tsx:1094 —
not affected: create/update response handling is unchanged; the retained
Close/X/Finish and PostgreSQL step-3 tests cover save closure and open
multi-step creation.
- superset-frontend/src/features/databases/DatabaseModal/index.tsx:1156 —
not affected: import save and the completion effect at 1507 use importResource,
not dbFetched, and close through onClose.
- superset-frontend/src/features/databases/DatabaseModal/index.tsx:1569 —
not affected: available-engine, import-field, SSH and validation effects do not
copy fetched database ids.
- superset-frontend/src/pages/DatabaseList/index.tsx:559 — not affected:
this file is unchanged; its onHide clears the selected database and visibility
together, as modeled by the mounted regression harness at 384.
- superset-frontend/src/features/home/RightMenu.tsx:734 — not affected: this
file is unchanged; it supplies no databaseId and starts no edit fetch; its hide
handler at 323 clears visibility and engine.
-
superset-frontend/packages/superset-ui-core/src/components/Modal/Modal.tsx:369
— not affected: the unchanged onCancel handler delegates to onHide, which
DatabaseModal routes through onClose; backdrop dismissal remains disabled in
all three database modal variants.
--
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]
