codeant-ai-for-open-source[bot] commented on code in PR #40466:
URL: https://github.com/apache/superset/pull/40466#discussion_r3324594405
##########
superset-frontend/src/views/CRUD/hooks.ts:
##########
@@ -846,8 +846,10 @@ export function useDatabaseValidation() {
return error.json().then(({ errors = [] }) => {
const parsedErrors = errors
.filter((err: { error_type: string }) => {
+ // CONNECTION_MISSING_PARAMETERS_ERROR is intentionally
excluded
+ // here so required-field errors only surface on submit
+ // (onCreate=true), not on blur of empty inputs.
const allowed = [
- 'CONNECTION_MISSING_PARAMETERS_ERROR',
'CONNECTION_ACCESS_DENIED_ERROR',
'INVALID_PAYLOAD_SCHEMA_ERROR',
];
Review Comment:
**Suggestion:** Removing `CONNECTION_MISSING_PARAMETERS_ERROR` from the blur
allow-list makes blur validation return an empty object for missing required
fields, and the save flow currently relies on pre-populated validation state as
a fallback because it checks `errors?.length` (array-style) on submit. This
combination can let invalid dynamic-form database configs proceed to save with
missing required parameters. Keep required-field errors represented in a way
the submit path can reliably block (for example, ensure submit receives/uses
object-key errors, or keep required-field state available for the save guard).
[logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Dynamic-form database submit ignores frontend required-field gating.
- ⚠️ Users see failures only after backend rejects save.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the dynamic-form database connection UI so that `DatabaseModal` is
rendered and
uses `useDatabaseValidation()` from
`superset-frontend/src/views/CRUD/hooks.ts:37-115`,
wiring `[validationErrors, getValidation, ...]` into
`DatabaseConnectionForm` at
`DatabaseModal/index.tsx:621-630` and then into `TableCatalog` via the
`getValidation`
prop at `DatabaseModal/index.tsx:1893-1897`.
2. In the Google Sheets catalog UI, add a sheet row and blur the URL field
without
entering a value; `TableCatalog` uses `ValidatedInput` with
`validationMethods={{ onBlur:
getValidation }}` at `TableCatalog.tsx:7-11`, which calls `getValidation(db,
false)`
(onCreate=false) from `useDatabaseValidation`.
3. The backend responds with a `CONNECTION_MISSING_PARAMETERS_ERROR` for the
missing
required URL/host; in `useDatabaseValidation`, the `.filter` at
`hooks.ts:69-78` excludes
this error type on blur because it only allows
`CONNECTION_ACCESS_DENIED_ERROR` and
`INVALID_PAYLOAD_SCHEMA_ERROR` (see the comment block at `hooks.ts:69-75`
and tests at
`hooks.test.tsx:22-39`), so `parsedErrors` reduces to `{}` and
`validationErrors` state
becomes `{}` even though required fields are missing.
4. Because `hasValidated` is set `true` and `validationErrors` is `{}`, the
"Connect"
button in `DatabaseModal` becomes enabled (`disabled` depends on
`!hasValidated ||
isValidating || (validationErrors && Object.keys(validationErrors).length >
0)` at
`DatabaseModal/index.tsx:1290-23`). Clicking "Connect" calls `onSave` at
`DatabaseModal/index.tsx:869-69`, which re-runs `getValidation(dbToUpdate,
true)`
(onCreate=true) and receives an object of required-field errors; however,
the submit guard
checks `if (!isEmpty(validationErrors) || errors?.length)` at
`DatabaseModal/index.tsx:62-69`: `validationErrors` is still the previous
`{}` from blur
(React state update from this submit call is not visible within the same
render), and
`errors` is an object without a `length` property, so `errors?.length` is
`undefined`. The
condition is false and `onSave` proceeds to call
`createResource`/`updateResource` with
missing required parameters, relying solely on backend enforcement, which
can let the
frontend bypass its intended submit-time gating for dynamic-form database
configs.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b8526a672dc9419cbbcce242231fbd4c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=b8526a672dc9419cbbcce242231fbd4c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(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/views/CRUD/hooks.ts
**Line:** 849:855
**Comment:**
*Logic Error: Removing `CONNECTION_MISSING_PARAMETERS_ERROR` from the
blur allow-list makes blur validation return an empty object for missing
required fields, and the save flow currently relies on pre-populated validation
state as a fallback because it checks `errors?.length` (array-style) on submit.
This combination can let invalid dynamic-form database configs proceed to save
with missing required parameters. Keep required-field errors represented in a
way the submit path can reliably block (for example, ensure submit
receives/uses object-key errors, or keep required-field state available for the
save guard).
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%2F40466&comment_hash=185a114685b7ac213bc58567fc3978d494ab99639b94f5db6af9b9f0144f2eae&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40466&comment_hash=185a114685b7ac213bc58567fc3978d494ab99639b94f5db6af9b9f0144f2eae&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]
