sadpandajoe commented on code in PR #45649:
URL: https://github.com/apache/superset/pull/45649#discussion_r4381840007
##########
superset-frontend/src/features/semanticLayers/SemanticLayerModal.tsx:
##########
@@ -271,6 +304,8 @@ export default function SemanticLayerModal({
useEffect(
() => () => {
+ typeRequestRef.current += 1;
Review Comment:
Closing the dialog while it stays mounted doesn't invalidate in-flight
requests — only unmount does (and the next `fetchTypes`). `DatabaseList`
renders this modal permanently with `show={false}`, so if the user cancels
while the types GET is pending and it then fails, `request ===
typeRequestRef.current` still holds and a "An error occurred while fetching
semantic layer types" toast fires with no dialog open (the same applies to the
schema fetch). That is the late-response case the description says is ignored.
Should the `show === false` branch bump `typeRequestRef`/`schemaRequestRef` as
well?
The stale-request tests only catch this when the dialog reopens first (which
bumps the ref itself), so they would still pass with the close-time guard
missing; asserting `addDangerToast` is not called after close and before
reopening would cover it.
--
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]
