Skip to content

Commit 0e2f173

Browse files
Vchen7629mpawlow
andauthored
fix: restrict ingestion to containers section shows up correctly and error message isn't duplicated (#2071)
* fix: added new showContainer and onAuthModeChange state * fix: updated settings-form to only show "restrict ingestion to containers section" when state is true * fix: adding missing setContainers null onAuthModeChange * fix: updated onAuthModeChange to clear error message aswell when switching auth mode * fix: added missing setContainers(null) to handleTestConnection * fix: reset containers to null on failed retest and dialog reopen * fix: Azure Blob connector dialog clearance of containers section Issue - #2047 Summary - Fix nit: Abort-on-unmount cleanup never fires on dialog close, so a canceled test can still land and repopulate stale containers --------- Co-authored-by: Mike Pawlowski <mpawlow@ca.ibm.com>
1 parent 32f36e9 commit 0e2f173

2 files changed

Lines changed: 43 additions & 13 deletions

File tree

frontend/enhancements/connectors/azure-blob/settings-dialog.tsx

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,18 @@ export default function AzureBlobSettingsDialog({
4949
const [selectedContainers, setSelectedContainers] = useState<string[]>(
5050
defaults?.container_names ?? [],
5151
);
52-
useEffect(() => {
53-
if (defaults?.container_names?.length) {
54-
setContainers(defaults.container_names);
55-
setSelectedContainers(defaults.container_names);
56-
}
57-
}, [defaults]);
52+
const [showContainers, setShowContainers] = useState(
53+
!!defaults?.container_names?.length,
54+
);
55+
56+
const [prevDefaults, setPrevDefaults] = useState(defaults);
57+
if (defaults !== prevDefaults) {
58+
setPrevDefaults(defaults);
59+
const names = defaults?.container_names ?? [];
60+
setContainers(names.length ? names : null);
61+
setSelectedContainers(names);
62+
setShowContainers(names.length > 0);
63+
}
5864

5965
const [isFetchingContainers, setIsFetchingContainers] = useState(false);
6066
const [containersError, setContainersError] = useState<string | null>(null);
@@ -66,13 +72,16 @@ export default function AzureBlobSettingsDialog({
6672
// can be cancelled if the user closes the dialog or starts a new test.
6773
const testAbortRef = useRef<AbortController | null>(null);
6874

69-
// Abort any in-flight test request when the component unmounts (i.e. when
70-
// the dialog is closed, since connector-cards renders it only while open).
75+
// Abort any in-flight test request when the dialog closes. The dialog
76+
// component itself stays mounted (connector-cards renders it unconditionally
77+
// and just toggles `open`), so an unmount-only cleanup would never fire here
78+
// — without this, a canceled test could still resolve and repopulate the
79+
// container list after the user closed the dialog.
7180
useEffect(() => {
72-
return () => {
81+
if (!open) {
7382
testAbortRef.current?.abort();
74-
};
75-
}, []);
83+
}
84+
}, [open]);
7685

7786
const handleTestConnection = handleSubmit(async (data) => {
7887
// Cancel any previous in-flight test before starting a new one.
@@ -107,13 +116,16 @@ export default function AzureBlobSettingsDialog({
107116
const fetched: string[] = json.containers;
108117
setContainers(fetched);
109118
setSelectedContainers((prev) => prev.filter((c) => fetched.includes(c)));
119+
setShowContainers(true);
110120
} catch (err: unknown) {
111121
// Ignore cancellations — they are intentional (dialog closed or new test
112122
// started) and must not surface an error message to the user.
113123
if (err instanceof DOMException && err.name === "AbortError") return;
114124
setContainersError(
115125
err instanceof Error ? err.message : "Connection failed",
116126
);
127+
setShowContainers(false);
128+
setContainers(null);
117129
} finally {
118130
// Only clear the loading flag if this invocation is still the active one.
119131
// If the user started a second test while this one was in flight, the
@@ -127,6 +139,7 @@ export default function AzureBlobSettingsDialog({
127139

128140
const onSubmit = handleSubmit(async (data) => {
129141
setFormError(null);
142+
setContainersError(null);
130143
if (containers === null) {
131144
setFormError("Test the connection first to validate credentials.");
132145
return;
@@ -197,6 +210,13 @@ export default function AzureBlobSettingsDialog({
197210
connectionStringSet={defaults?.connection_string_set}
198211
accountKeySet={defaults?.account_key_set}
199212
formError={formError}
213+
showContainers={showContainers}
214+
onAuthModeChange={() => {
215+
setShowContainers(false);
216+
setContainers(null);
217+
setContainersError(null);
218+
setFormError(null);
219+
}}
200220
/>
201221

202222
<DialogFooter className="mt-4">

frontend/enhancements/connectors/azure-blob/settings-form.tsx

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ interface AzureBlobSettingsFormProps {
2929
connectionStringSet?: boolean;
3030
accountKeySet?: boolean;
3131
formError?: string | null;
32+
showContainers?: boolean;
33+
onAuthModeChange?: () => void;
3234
}
3335

3436
export function AzureBlobSettingsForm({
@@ -41,6 +43,8 @@ export function AzureBlobSettingsForm({
4143
connectionStringSet,
4244
accountKeySet,
4345
formError,
46+
showContainers,
47+
onAuthModeChange,
4448
}: AzureBlobSettingsFormProps) {
4549
const {
4650
register,
@@ -69,7 +73,13 @@ export function AzureBlobSettingsForm({
6973
control={control}
7074
name="auth_mode"
7175
render={({ field }) => (
72-
<Tabs value={field.value} onValueChange={(v) => field.onChange(v)}>
76+
<Tabs
77+
value={field.value}
78+
onValueChange={(v) => {
79+
field.onChange(v);
80+
onAuthModeChange?.();
81+
}}
82+
>
7383
<TabsList className="w-full">
7484
<TabsTrigger value="connection_string">
7585
<span className="font-semibold text-sm">
@@ -213,7 +223,7 @@ export function AzureBlobSettingsForm({
213223
)}
214224

215225
{/* Container selector */}
216-
{containers !== null && (
226+
{containers !== null && showContainers && (
217227
<div className="space-y-2">
218228
<div className="flex items-center justify-between">
219229
<Label className="text-sm font-medium">

0 commit comments

Comments
 (0)