aminghadersohi commented on PR #42390:
URL: https://github.com/apache/superset/pull/42390#issuecomment-5186117863
Thanks @rebenitez1802 — pushed 0fe4f25, which addresses your deferred nit
and one remaining bot finding.
**1. The headline behavior now has a real end-to-end test.** You were right
that it was only asserted indirectly. New test `repopulates the preview once
the OAuth2 dance completes`: renders with `table_metadata` returning 403 +
`OAUTH2_REDIRECT`, waits for the authorization link, then flips the mock to a
successful response and fires the completion `storage` event that
`OAuth2RedirectMessage` listens for — asserting the preview actually
repopulates with the column rows. Verified non-vacuous: removing
`'TableMetadatas'` from the `invalidateTags` list makes it fail.
**2. Fixed a real error-selection bug** flagged by the bot on
`TablePreview/index.tsx`. `(metadataError || metadataExtrError)` always
preferred the main request, so a generic failure there would mask an actionable
`OAUTH2_REDIRECT` arriving from the extended-metadata request. Selection now
picks whichever response carries a structured `errors[]`, keeping your
plain-message fallback intact when neither does. Also covered by a new
non-vacuous test.
**3. Correction on my earlier verification note.** I previously said the
Jest failures here were environmental (Node 22 vs the required ^24.16.0,
`fetch-mock` not intercepting). That was wrong — this worktree simply had no
`node_modules`. After a proper install the whole suite runs green on Node 22:
```
Tests: 9 passed, 9 total
✓ renders columns / indexes / preview
✓ renders an OAuth2 authorization prompt when metadata errors with
OAUTH2_REDIRECT
✓ surfaces an OAUTH2_REDIRECT from the extended metadata request when the
main request fails generically
✓ repopulates the preview once the OAuth2 dance completes
✓ renders the error message when metadata fails without a structured error
✓ table actions › refreshes table metadata / shows CREATE VIEW statement
```
`src/components/ErrorMessage` also green (14 suites, 69 tests). prettier,
oxlint, stylelint and the custom-rules hook pass on the touched files; the
pre-commit frontend type-check fails only on `TS6305` "output file has not been
built from source" across the whole repo because the workspace `lib/` outputs
aren't built locally — zero type errors reference the files in this diff.
The one bot comment I did **not** act on is the unscoped `'TableMetadatas'`
invalidation; I replied inline with the reasoning (no resource identity in the
OAuth2 error payload, matches the existing unscoped
`'Tables'`/`Schemas`/`Catalogs` siblings, and RTK Query only refetches active
subscribers — one per pinned table tab). Happy to change course if you disagree.
--
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]
