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


##########
superset-frontend/src/features/semanticLayers/SemanticLayerModal.tsx:
##########
@@ -333,32 +372,45 @@ export default function SemanticLayerModal({
   };
 
   const handleCreate = async () => {
+    saveRequestRef.current += 1;
+    const request = saveRequestRef.current;
     setSaving(true);
     try {
       if (isEditMode && semanticLayerUuid) {
         await SupersetClient.put({
           endpoint: `/api/v1/semantic_layer/${semanticLayerUuid}`,
           jsonPayload: { name, configuration: formData },
         });
+        if (request !== saveRequestRef.current) {
+          // Cancel refreshed the parent before the update committed.
+          onUpdated?.();
+          return;
+        }
         addSuccessToast(t('Semantic layer updated'));
       } else {
         await SupersetClient.post({
           endpoint: '/api/v1/semantic_layer/',
           jsonPayload: { name, type: selectedType, configuration: formData },
         });
+        // The server created the layer even if this dialog was closed. Only
+        // the parent may refresh the picker; stale dialog UI remains ignored.
+        onCreated?.();

Review Comment:
   Thanks, fixed in 537733d75b. The Database list create modal now passes 
`onCreated={refreshData}`. A new test starts a create, cancels while the POST 
is pending, then resolves it and checks that the Connections list refetches 
after the write.



##########
superset-frontend/src/features/semanticLayers/SemanticLayerModal.tsx:
##########
@@ -479,72 +532,122 @@ export default function SemanticLayerModal({
       width={isTypeStep ? MODAL_STANDARD_WIDTH : MODAL_MEDIUM_WIDTH}
       saveDisabled={
         isTypeStep
-          ? !selectedType
-          : saving || clearing || !name.trim() || hasErrors
+          ? !selectedType || typeStatus !== 'success' || loading
+          : loading || saving || clearing || !name.trim() || hasErrors
       }
       saveText={isTypeStep ? undefined : isEditMode ? t('Save') : t('Create')}
       saveLoading={saving}
-      contentLoading={loading}
     >
       <ModalContent>
-        {isTypeStep ? (
-          <ModalFormField label={t('Type')}>
-            <Select
-              ariaLabel={t('Semantic layer type')}
-              placeholder={t('Select a semantic layer type')}
-              value={selectedType}
-              onChange={value => setSelectedType(value as string)}
-              options={types.map(type => ({
-                value: type.id,
-                label: type.name,
-              }))}
-              getPopupContainer={() => document.body}
-            />
-          </ModalFormField>
+        <SemanticLayerSetupHelp />
+        {loading ? (
+          <Loading />

Review Comment:
   Good catch, fixed in 537733d75b. The schema loader is now in flow 
(`position="normal"`), centered in a body with a 200px minimum height. A 
pending-schema test checks the spinner position and the reserved space.



##########
superset-frontend/src/features/semanticLayers/SemanticLayerModal.test.tsx:
##########
@@ -745,3 +772,379 @@ test('A pending clear prevents edits and a closed editor 
ignores its late respon
   await act(async () => finish({ json: {} }));
   expect(props.addSuccessToast).not.toHaveBeenCalled();
 });
+
+test('a successful edit after Cancel refreshes the parent without stale modal 
UI', async () => {

Review Comment:
   Added in 537733d75b: rejected POST and PUT cases after Cancel, both 
asserting that no danger toast appears. I checked them by temporarily removing 
the stale-request guards in the catch paths; both tests then failed on a danger 
toast, and they pass again with the guards restored.



##########
superset/config.py:
##########
@@ -2936,6 +2936,10 @@ def EMAIL_HEADER_MUTATOR(  # pylint: 
disable=invalid-name,unused-argument  # noq
 
 # Send user to a link where they can read more about Superset
 DOCUMENTATION_URL = None
+
+# Optional setup guide shown in the semantic layer and semantic view dialogs.
+# Use an absolute HTTP(S) URL; unset deployments show no setup-guide link.
+SEMANTIC_LAYER_DOCUMENTATION_URL: str | None = None

Review Comment:
   Moved in 537733d75b. `SEMANTIC_LAYER_DOCUMENTATION_URL` now sits below 
`DOCUMENTATION_ICON`, so the navbar documentation settings stay together.



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