sadpandajoe commented on code in PR #45649:
URL: https://github.com/apache/superset/pull/45649#discussion_r4386167311
##########
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:
The after-Cancel tests only resolve the pending save; none rejects a POST or
PUT after Cancel. If the stale-request checks in `handleCreate`'s catch were
removed, these tests would still pass while an obsolete failure shows a danger
toast over the reopened view picker. Could a case reject the deferred save
after Cancel and assert `addDangerToast` is not called?
##########
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:
The previous `contentLoading` path wrapped the spinner in a container with a
200px minimum height, but `<Loading />` here defaults to the absolutely
positioned `floating` mode. While the configuration schema or an existing layer
loads, nothing reserves height in the modal body, so it can collapse to its
padding and the spinner can sit over the header or footer. Should this use an
in-flow loader inside a height-reserving container, as `StandardModal` does?
##########
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:
In the Database list, creating a layer and then clicking Cancel while the
POST is pending refreshes the list before the layer exists. When the POST later
succeeds this branch calls `onCreated?.()`, but the creation modal in
`DatabaseList` passes no `onCreated`, so the new layer stays missing from the
list until another refresh. Should that caller pass `onCreated={refreshData}`,
as `DatasetList` does?
--
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]
