mikebridge commented on PR #42845:
URL: https://github.com/apache/superset/pull/42845#issuecomment-5206326476
## Launch-review synthesis
Reviewed head `f9aa95ef5d` with: `react-review`, `clean-code-review`,
`tidy-first-review`, `superset-committer-review`, and `preset-review`.
### Medium
1. **Provide feedback while dependent views are loading** —
`superset-frontend/src/pages/DatabaseList/index.tsx:324`
The delete action does not show modal or loading state until the
datasource lookup settles. On a slow connection, clicking Delete appears to do
nothing, and repeated clicks issue duplicate requests. Open the modal
immediately with an explicit loading state, or disable/show progress on the
action until lookup completion; retain the generation guard for stale
responses.
_Source: preset-review_
2. **Extract the semantic-layer delete description from the page component**
— `superset-frontend/src/pages/DatabaseList/index.tsx:987`
The inline description mixes pluralization, failure-state policy, list
rendering, overflow calculation, and modal composition inside an already large
page component. Extract a focused `SemanticLayerDeleteDescription` receiving
the layer name, count, and names so the policy is independently readable and
testable.
_Source: clean-code-review_
### Low
1. **Replace the fixed timer with deterministic async synchronization** —
`superset-frontend/src/pages/DatabaseList/DatabaseList.semanticLayerDelete.test.tsx:287`
The stale-resolution regression waits 50 ms after releasing the deferred
request. Resolve the gate inside `act()`, await request settlement or the
relevant microtasks, then assert that the dialog remains closed.
_Sources: react-adjacent consensus from clean-code-review,
tidy-first-review, superset-committer-review, preset-review_
2. **Use the shared Redux test wrapper** —
`superset-frontend/src/pages/DatabaseList/DatabaseList.semanticLayerDelete.test.tsx:125`
Pass the per-test store through the shared `render` helper instead of
adding an ad-hoc `Provider`, so the standard provider composition remains
authoritative.
_Source: react-review_
### Summary
0 blocker, 0 high, 2 medium, 2 low. No reviewer found a security,
API-contract, feature-flag, destructive-action, typing, licensing, or
process-gate blocker. The committer lens returned **APPROVED_WITH_NITS**; the
main behavioral recommendation is visible feedback during the pre-delete lookup.
_AI-assisted review synthesis posted on behalf of @mikebridge._
--
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]
