rusackas commented on code in PR #43458:
URL: https://github.com/apache/superset/pull/43458#discussion_r3982360855
##########
superset-frontend/src/components/ErrorMessage/DatasourceSecurityAccessErrorMessage.tsx:
##########
@@ -55,7 +56,14 @@ export function DatasourceSecurityAccessErrorMessage({
// (e.g. virtual-dataset SQL validation: "Only SELECT statements are
// allowed"). Those errors carry no access payload — render them plainly
// rather than misleading the user with request-access guidance.
- const isAccessDenial = !!extra?.datasource_name || !!extra?.tables?.length;
+ //
+ // `datasource` is the pre-`is_access_denial` shape of the payload; accepting
+ // it keeps this guidance working while a rolling deploy still has older API
+ // pods answering. It never appears on the non-access failures above.
+ const isAccessDenial =
+ !!extra?.tables?.length ||
+ !!extra?.is_access_denial ||
+ isDefined(extra?.datasource);
Review Comment:
sadpandajoe's still-open point above: `extra.link` renders unconditionally
once `isAccessDenial` is true here, and that's also true under the legacy
`datasource`-id-only signal, where an older backend pod could've already baked
the dataset name into the URL before this fix. Might be worth gating the link
render on `extra?.is_access_denial` specifically, since that's the only case
the new backend guarantees the link is clean.
--
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]
