mikebridge commented on code in PR #45649:
URL: https://github.com/apache/superset/pull/45649#discussion_r4381241927


##########
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:
   Agreed, and fixed in `33fe0325be`. The create response now signals the 
parent even if the layer dialog was cancelled, so the view picker refetches its 
layer list. A stale response can't reopen a dialog or show a success toast. A 
delayed-POST regression test failed before the fix and passes now.



##########
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:
   Agreed, and fixed in `33fe0325be`. The server now leaves credential-bearing, 
non-HTTP(S) and malformed setup URLs out of the common bootstrap (they become 
`None`) and logs one warning, without the URL. The bootstrap tests failed 
before the filter and pass now, and the configuration docs describe the 
behavior.



-- 
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]

Reply via email to