EnxDev commented on PR #41685:
URL: https://github.com/apache/superset/pull/41685#issuecomment-4874643934
## EnxDev's Review Agent โ apache/superset#41685 ยท HEAD 5b3493f
comment โ the fix is correct and verified end-to-end, but the added test
doesn't guard the changed code path.
Verified the chain: the saved_query GET API exposes `description` in
`show_columns`, `convertQueryToClient` passes the key through unrenamed, and
`SaveQuery` already selects `description` from the query editor state and
initializes the field null-safely (`query.description || ''`). Adding
`description` to `tmpAdaptedProps` is the right fix at the right place.
### ๐ก Should-fix
-
**`superset-frontend/src/SqlLab/components/SaveQuery/SaveQuery.test.tsx:175`**
โ this test passes without the fix. It seeds `description` directly into the
mocked store and asserts the modal pre-fills โ behavior `SaveQuery` already had
on master (`SaveQuery/index.tsx:88,108`). The changed path is `popSavedQuery`,
which the test never dispatches. Move the regression guard to
`src/SqlLab/actions/sqlLab.test.ts` (`popSavedQuery` block, ~line 795): set a
non-empty `description` in `mockSavedQueryApiResponse` and add `description` to
`expectedParams` โ that assertion is red without this fix. (Codecov's "all
modified lines covered" comes from the existing `popSavedQuery` tests executing
the line without asserting on it.)
### ๐ต Nits
- `superset-frontend/src/SqlLab/components/SaveQuery/SaveQuery.test.tsx:203`
โ `screen.getAllByRole('textbox')[1]` breaks if the modal's field order
changes; the field has an accessible label: `screen.getByRole('textbox', {
name: 'Description' })`.
### ๐ Praise
- PR body pins the exact root cause (hand-picked `tmpAdaptedProps` dropping
`description`) โ made verification fast.
<!-- enxdev-review-agent:5b3493f -->
_Reviewed by EnxDev's Review Agent โ @EnxDev ยท HEAD 5b3493f._
--
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]
