codeant-ai-for-open-source[bot] commented on code in PR #36880:
URL: https://github.com/apache/superset/pull/36880#discussion_r3194560381
##########
superset-frontend/src/features/databases/DatabaseModal/index.tsx:
##########
@@ -811,6 +823,15 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps>
= ({
[onChange, handleClearValidationErrors],
);
+ const getBlurValidation = useCallback(() => {
+ const currentDbSnapshot = JSON.stringify(db);
+ if (currentDbSnapshot === lastValidatedDbSnapshotRef.current) {
+ return Promise.resolve([]);
+ }
+ lastValidatedDbSnapshotRef.current = currentDbSnapshot;
+ return getValidation(db);
+ }, [db, getValidation]);
Review Comment:
**Suggestion:** The blur-validation deduplication cache is updated before
the async validation completes. If that request fails (for example due to a
transient backend/network error), the same form state is still marked as
"already validated", so subsequent blurs return early and never retry
validation. Update the snapshot only after a successful validation response (or
clear/reset it on validation failure) so unchanged values can be revalidated
after failed attempts. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ DatabaseModal blur validation skips retries after network failures.
- ⚠️ SSH tunnel fields may never revalidate unchanged values.
- ⚠️ TableCatalog blur validation can silently stop retrying.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the Database modal, e.g. via the database list page at
`superset-frontend/src/pages/DatabaseList/index.tsx:925-933`, which renders
`<DatabaseModal ... />` from
`superset-frontend/src/features/databases/DatabaseModal/index.tsx`.
2. In the modal, focus and then blur a field wired for blur validation, such
as the SSH
Host input in `SSHTunnelForm` at
`superset-frontend/src/features/databases/DatabaseModal/SSHTunnelForm.tsx:19-31`,
where
`validationMethods={{ onBlur: getValidation }}` delegates to the
`getValidation` prop
coming from `getBlurValidation` in `DatabaseModal` (`index.tsx:826-833`).
3. While that blur-triggered validation runs, cause
`/api/v1/database/validate_parameters/` to fail with a transient
backend/network error so
that `SupersetClient.post(...)` in `useDatabaseValidation().getValidation` at
`superset-frontend/src/views/CRUD/hooks.ts:22-27` rejects without a usable
`error.json()`
function, driving execution into the "Unexpected error during validation"
branch at
`hooks.ts:19-24` which returns `{}`; note that
`lastValidatedDbSnapshotRef.current` was
already set to `currentDbSnapshot` before this failure in
`getBlurValidation` at
`index.tsx:826-833`.
4. Without changing any form fields (so the serialized `db` object is
unchanged), blur the
same field again: `getBlurValidation` recomputes `currentDbSnapshot` and
finds it equal to
`lastValidatedDbSnapshotRef.current`, so it returns `Promise.resolve([])`
early at
`index.tsx:828-829` and never calls `getValidation` again, meaning this
exact database
configuration cannot be revalidated on blur after the failed request unless
the user
changes some field or the snapshot is reset elsewhere.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fsrc%2Ffeatures%2Fdatabases%2FDatabaseModal%2Findex.tsx%0A%2A%2ALine%3A%2A%2A%20826%3A833%0A%2A%2AComment%3A%2A%2A%0A%09%2ALogic%20Error%3A%20The%20blur-validation%20deduplication%20cache%20is%20updated%20before%20the%20async%20validation%20completes.%20If%20that%20request%20fails%20%28for%20example%20due%20to%20a%20transient%20backend%2Fnetwork%20error%29%2C%20the%20same%20form%20state%20is%20still%20marked%20as%20%22already%20validated%22%2C%20so%20subsequent%20blurs%20return%20early%20and%20never%20retry%20validation.%20Update%20the%20snapshot%20only%20after%20a%20successful%20validation%20response%20%28or%20clear%2Freset%20it%20on%20validation%20failure%29%20so%20unchanged%20values%20can%20be%20revalidated%20after%20failed%20attempts.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%2
0correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fsrc%2Ffeatures%2Fdatabases%2FDatabaseModal%2Findex.tsx%0A%2A%2ALine%3A%2A%2A%20826%3A833%0A%2A%2AComment%3A%2A%2A%0A%09%2ALogic%20Error%3A%20The%20blur-validation%20deduplication%20cache%20is%20updated%20before%20the%20async%20validation%20completes.%20If%20that%20request%20fails%20%28for%20example%20due%20to%20a%20transient%20backend
%2Fnetwork%20error%29%2C%20the%20same%20form%20state%20is%20still%20marked%20as%20%22already%20validated%22%2C%20so%20subsequent%20blurs%20return%20early%20and%20never%20retry%20validation.%20Update%20the%20snapshot%20only%20after%20a%20successful%20validation%20response%20%28or%20clear%2Freset%20it%20on%20validation%20failure%29%20so%20unchanged%20values%20can%20be%20revalidated%20after%20failed%20attempts.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/features/databases/DatabaseModal/index.tsx
**Line:** 826:833
**Comment:**
*Logic Error: The blur-validation deduplication cache is updated before
the async validation completes. If that request fails (for example due to a
transient backend/network error), the same form state is still marked as
"already validated", so subsequent blurs return early and never retry
validation. Update the snapshot only after a successful validation response (or
clear/reset it on validation failure) so unchanged values can be revalidated
after failed attempts.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36880&comment_hash=3fb013e1160652cb03e5987c2995d6895dc2f366dc64ebcf465e1743c797d8ff&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36880&comment_hash=3fb013e1160652cb03e5987c2995d6895dc2f366dc64ebcf465e1743c797d8ff&reaction=dislike'>👎</a>
--
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]
