codeant-ai-for-open-source[bot] commented on code in PR #45649:
URL: https://github.com/apache/superset/pull/45649#discussion_r4365359821
##########
superset/views/base.py:
##########
@@ -81,6 +81,7 @@
from .utils import bootstrap_user_data, get_config_value
FRONTEND_CONF_KEYS = (
+ "SEMANTIC_LAYER_DOCUMENTATION_URL",
Review Comment:
Yes. The concern is valid: client-side hiding does not prevent the raw value
from appearing in the bootstrap payload. The server should validate
`SEMANTIC_LAYER_DOCUMENTATION_URL` before adding it to `conf` and return `None`
for URLs with credentials (and, ideally, other invalid URLs). Rejecting the
whole URL is safer than stripping credentials, which could silently change its
destination. Keep the frontend check as defense in depth, and add a bootstrap
test confirming credential-bearing URLs are not exposed.
##########
superset-frontend/src/pages/DatasetList/index.tsx:
##########
@@ -1630,13 +1637,38 @@ const DatasetList: FunctionComponent<DatasetListProps>
= ({
addSuccessToast={addSuccessToast}
semanticView={svCurrentlyEditing}
/>
- <AddSemanticViewModal
- show={showAddSemanticViewModal}
- onHide={() => setShowAddSemanticViewModal(false)}
- onSuccess={refreshData}
- addDangerToast={addDangerToast}
- addSuccessToast={addSuccessToast}
- />
+ {semanticSetup === 'view' && (
+ <AddSemanticViewModal
+ show
+ onHide={() => {
+ setSemanticSetup('closed');
+ newDatasourceButtonRef.current?.focus();
+ }}
+ onCreateLayer={
+ canCreateLayer
+ ? () => {
+ if (
+ isFeatureEnabled(SEMANTIC_LAYERS_FLAG) &&
+ findPermission('can_write', 'SemanticLayer', user?.roles)
+ ) {
+ setSemanticSetup('layer');
+ }
+ }
+ : undefined
+ }
+ onSuccess={refreshData}
+ addDangerToast={addDangerToast}
+ addSuccessToast={addSuccessToast}
+ />
+ )}
+ {semanticSetup === 'layer' && (
+ <SemanticLayerModal
+ show
+ onHide={() => setSemanticSetup('view')}
Review Comment:
Yes. Cancel currently only closes/unmounts the modal; it doesn’t guarantee
the POST was canceled or rolled back. If that POST later succeeds, the parent
should receive a **creation-completed signal even though the modal is stale**,
and use it only to invalidate/refetch the semantic-layer list.
Keep that signal separate from `onHide`: a late completion must not switch
dialogs, close a newer session, or show a stale success toast. A parent-owned
refresh signal can trigger a refetch if the view picker is already open, while
its normal initial fetch handles the case where creation finishes before the
picker remounts. This closes the race without letting the late response take
over the UI.
--
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]
