Skip to content

Commit bf15109

Browse files
yottahmdyottahmd
committed
fix: standardize conditional swr query enabling
1 parent b5d9a7b commit bf15109

16 files changed

Lines changed: 532 additions & 279 deletions

File tree

‎ui/src/features/cockpit/components/TemplateSelector.tsx‎

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import React, { useState, useEffect, useRef, useCallback, useContext, useMemo }
22
import { useQuery } from '@/hooks/api';
33
import { AppBarContext } from '@/contexts/AppBarContext';
44
import { Badge } from '@/components/ui/badge';
5+
import { whenEnabled } from '@/hooks/queryUtils';
56
import { Search, ChevronDown, X, AlertTriangle, Tags } from 'lucide-react';
67
import { cn } from '@/lib/utils';
78
import type { components } from '@/api/v1/schema';
@@ -50,30 +51,26 @@ export function TemplateSelector({
5051
// the tag filter UI inside the selector.
5152
const { data: tagsData } = useQuery(
5253
'/dags/tags',
53-
isOpen && isTagFilterOpen
54-
? {
55-
params: { query: { remoteNode } },
56-
}
57-
: null
54+
whenEnabled(isOpen && isTagFilterOpen, {
55+
params: { query: { remoteNode } },
56+
})
5857
);
5958
const availableTags = tagsData?.tags ?? [];
6059

6160
// The DAG list only stays live while the selector dropdown is open. The
6261
// closed trigger uses locally cached selection metadata instead.
6362
const { data, isLoading } = useQuery(
6463
'/dags',
65-
isOpen
66-
? {
67-
params: {
68-
query: {
69-
remoteNode,
70-
perPage: 50,
71-
...(debouncedTerm ? { name: debouncedTerm } : {}),
72-
...(selectedTags.length > 0 ? { tags: selectedTags.join(',') } : {}),
73-
},
74-
},
75-
}
76-
: null
64+
whenEnabled(isOpen, {
65+
params: {
66+
query: {
67+
remoteNode,
68+
perPage: 50,
69+
...(debouncedTerm ? { name: debouncedTerm } : {}),
70+
...(selectedTags.length > 0 ? { tags: selectedTags.join(',') } : {}),
71+
},
72+
},
73+
})
7774
);
7875
const dags = data?.dags ?? [];
7976

‎ui/src/features/dag-runs/components/dag-run-details/DAGRunDetailsModal.tsx‎

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { components } from '../../../../api/v1/schema';
77
import { AppBarContext } from '../../../../contexts/AppBarContext';
88
import { usePageContext } from '../../../../contexts/PageContext';
99
import { useQuery } from '../../../../hooks/api';
10+
import { whenEnabled } from '../../../../hooks/queryUtils';
1011
import { shouldIgnoreKeyboardShortcuts } from '../../../../lib/keyboard-shortcuts';
1112
import LoadingIndicator from '../../../../ui/LoadingIndicator';
1213
import { DAGRunContext } from '../../contexts/DAGRunContext';
@@ -74,36 +75,32 @@ function DAGRunDetailsModal({
7475

7576
const subDAGQuery = useQuery(
7677
'/dag-runs/{name}/{dagRunId}/sub-dag-runs/{subDAGRunId}',
77-
subDAGQueryEnabled
78-
? {
79-
params: {
80-
query: { remoteNode },
81-
path: {
82-
name: parentName,
83-
dagRunId: parentDAGRunId ?? '',
84-
subDAGRunId: subDAGRunId ?? '',
85-
},
86-
},
87-
}
88-
: null,
78+
whenEnabled(subDAGQueryEnabled, {
79+
params: {
80+
query: { remoteNode },
81+
path: {
82+
name: parentName,
83+
dagRunId: parentDAGRunId ?? '',
84+
subDAGRunId: subDAGRunId ?? '',
85+
},
86+
},
87+
}),
8988
{
9089
refreshInterval: subDAGQueryEnabled ? 2000 : 0,
9190
}
9291
);
9392

9493
const dagRunQuery = useQuery(
9594
'/dag-runs/{name}/{dagRunId}',
96-
dagRunQueryEnabled
97-
? {
98-
params: {
99-
query: { remoteNode },
100-
path: {
101-
name: name || '',
102-
dagRunId: dagRunId || 'latest',
103-
},
104-
},
105-
}
106-
: null,
95+
whenEnabled(dagRunQueryEnabled, {
96+
params: {
97+
query: { remoteNode },
98+
path: {
99+
name: name || '',
100+
dagRunId: dagRunId || 'latest',
101+
},
102+
},
103+
}),
107104
{
108105
refreshInterval: dagRunQueryEnabled ? 2000 : 0,
109106
}

‎ui/src/features/dag-runs/components/dag-run-details/DAGRunDetailsPanel.tsx‎

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import React from 'react';
44
import { useNavigate } from 'react-router-dom';
55
import { AppBarContext } from '../../../../contexts/AppBarContext';
66
import { useQuery } from '../../../../hooks/api';
7+
import { whenEnabled } from '../../../../hooks/queryUtils';
78
import {
89
liveFallbackOptions,
910
useLiveConnection,
@@ -49,36 +50,32 @@ function DAGRunDetailsPanel({
4950
// Sub-DAG query (only enabled for sub-DAG runs)
5051
const subDAGQuery = useQuery(
5152
'/dag-runs/{name}/{dagRunId}/sub-dag-runs/{subDAGRunId}',
52-
subDAGQueryEnabled
53-
? {
54-
params: {
55-
query: { remoteNode },
56-
path: {
57-
name: parentName as string,
58-
dagRunId: parentDAGRunId as string,
59-
subDAGRunId: subDAGRunId as string,
60-
},
61-
},
62-
}
63-
: null,
53+
whenEnabled(subDAGQueryEnabled, {
54+
params: {
55+
query: { remoteNode },
56+
path: {
57+
name: parentName as string,
58+
dagRunId: parentDAGRunId as string,
59+
subDAGRunId: subDAGRunId as string,
60+
},
61+
},
62+
}),
6463
liveFallbackOptions(liveState)
6564
);
6665
useLiveDAGRuns(subDAGQuery.mutate, subDAGQueryEnabled);
6766

6867
// Regular DAG query — SWR is the single source of truth, refreshed by live invalidations
6968
const dagRunQuery = useQuery(
7069
'/dag-runs/{name}/{dagRunId}',
71-
dagRunQueryEnabled
72-
? {
73-
params: {
74-
query: { remoteNode },
75-
path: {
76-
name: name || '',
77-
dagRunId: dagRunId || 'latest',
78-
},
79-
},
80-
}
81-
: null,
70+
whenEnabled(dagRunQueryEnabled, {
71+
params: {
72+
query: { remoteNode },
73+
path: {
74+
name: name || '',
75+
dagRunId: dagRunId || 'latest',
76+
},
77+
},
78+
}),
8279
liveFallbackOptions(liveState)
8380
);
8481
useLiveDAGRuns(dagRunQuery.mutate, dagRunQueryEnabled);

‎ui/src/features/dag-runs/components/dag-run-details/__tests__/DAGRunDetailsPanel.test.tsx‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
import React from 'react';
2+
import { render, screen } from '@testing-library/react';
3+
import { MemoryRouter } from 'react-router-dom';
4+
import { afterEach, describe, expect, it, vi } from 'vitest';
5+
import { AppBarContext } from '@/contexts/AppBarContext';
6+
import { useQuery } from '@/hooks/api';
7+
import { useLiveConnection } from '@/hooks/useAppLive';
8+
import DAGRunDetailsPanel from '../DAGRunDetailsPanel';
9+
10+
vi.mock('@/hooks/api', () => ({
11+
useQuery: vi.fn(),
12+
}));
13+
14+
vi.mock('@/hooks/useAppLive', () => ({
15+
liveFallbackOptions: vi.fn(() => ({})),
16+
useLiveConnection: vi.fn(),
17+
useLiveDAGRuns: vi.fn(),
18+
}));
19+
20+
vi.mock('../DAGRunDetailsContent', () => ({
21+
default: ({ dagRun }: { dagRun: { dagRunId: string } }) => (
22+
<div>run {dagRun.dagRunId}</div>
23+
),
24+
}));
25+
26+
const appBarValue = {
27+
title: 'Runs',
28+
setTitle: vi.fn(),
29+
remoteNodes: ['local'],
30+
setRemoteNodes: vi.fn(),
31+
selectedRemoteNode: 'local',
32+
selectRemoteNode: vi.fn(),
33+
};
34+
35+
const liveState = {
36+
data: null,
37+
error: null,
38+
isConnected: false,
39+
isConnecting: false,
40+
shouldUseFallback: true,
41+
};
42+
43+
const useQueryMock = useQuery as unknown as {
44+
mockImplementation: (
45+
fn: (path: string, init?: unknown) => unknown
46+
) => void;
47+
};
48+
49+
function renderPanel() {
50+
return render(
51+
<MemoryRouter>
52+
<AppBarContext.Provider value={appBarValue}>
53+
<DAGRunDetailsPanel
54+
name="child-dag"
55+
dagRunId="child-run"
56+
onClose={vi.fn()}
57+
/>
58+
</AppBarContext.Provider>
59+
</MemoryRouter>
60+
);
61+
}
62+
63+
afterEach(() => {
64+
vi.clearAllMocks();
65+
window.history.pushState({}, '', '/');
66+
});
67+
68+
describe('DAGRunDetailsPanel', () => {
69+
it('enables the regular dag-run query and disables the sub-dag query by null init', () => {
70+
const queryCalls: Array<{ path: string; init?: unknown }> = [];
71+
vi.mocked(useLiveConnection).mockReturnValue(liveState);
72+
useQueryMock.mockImplementation((path, init) => {
73+
queryCalls.push({ path, init });
74+
if (path === '/dag-runs/{name}/{dagRunId}') {
75+
return {
76+
data: { dagRunDetails: { dagRunId: 'child-run' } },
77+
mutate: vi.fn(),
78+
} as never;
79+
}
80+
return {
81+
data: undefined,
82+
mutate: vi.fn(),
83+
} as never;
84+
});
85+
86+
renderPanel();
87+
88+
expect(
89+
queryCalls.find((call) => call.path === '/dag-runs/{name}/{dagRunId}')?.init
90+
).toEqual(
91+
expect.objectContaining({
92+
params: expect.objectContaining({
93+
path: { name: 'child-dag', dagRunId: 'child-run' },
94+
}),
95+
})
96+
);
97+
expect(
98+
queryCalls.find(
99+
(call) => call.path === '/dag-runs/{name}/{dagRunId}/sub-dag-runs/{subDAGRunId}'
100+
)?.init
101+
).toBeNull();
102+
expect(screen.getByText('run child-run')).toBeInTheDocument();
103+
});
104+
105+
it('enables the sub-dag query and disables the regular dag-run query by null init', () => {
106+
const queryCalls: Array<{ path: string; init?: unknown }> = [];
107+
window.history.pushState(
108+
{},
109+
'',
110+
'/?subDAGRunId=sub-run&dagRunId=root-run&dagRunName=root-dag'
111+
);
112+
vi.mocked(useLiveConnection).mockReturnValue(liveState);
113+
useQueryMock.mockImplementation((path, init) => {
114+
queryCalls.push({ path, init });
115+
if (path === '/dag-runs/{name}/{dagRunId}/sub-dag-runs/{subDAGRunId}') {
116+
return {
117+
data: { dagRunDetails: { dagRunId: 'sub-run' } },
118+
mutate: vi.fn(),
119+
} as never;
120+
}
121+
return {
122+
data: undefined,
123+
mutate: vi.fn(),
124+
} as never;
125+
});
126+
127+
renderPanel();
128+
129+
expect(
130+
queryCalls.find(
131+
(call) => call.path === '/dag-runs/{name}/{dagRunId}/sub-dag-runs/{subDAGRunId}'
132+
)?.init
133+
).toEqual(
134+
expect.objectContaining({
135+
params: expect.objectContaining({
136+
path: {
137+
name: 'root-dag',
138+
dagRunId: 'root-run',
139+
subDAGRunId: 'sub-run',
140+
},
141+
}),
142+
})
143+
);
144+
expect(
145+
queryCalls.find((call) => call.path === '/dag-runs/{name}/{dagRunId}')?.init
146+
).toBeNull();
147+
expect(screen.getByText('run sub-run')).toBeInTheDocument();
148+
});
149+
});

‎ui/src/features/dags/components/chat-history/StepMessagesTable.tsx‎

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { ChatMessageRole } from '@/api/v1/schema';
22
import { Markdown } from '@/components/ui/markdown';
33
import { AppBarContext } from '@/contexts/AppBarContext';
44
import { useQuery } from '@/hooks/api';
5+
import { whenEnabled } from '@/hooks/queryUtils';
56
import { cn } from '@/lib/utils';
67
import { Check, ChevronRight, Copy, Loader2, Wrench } from 'lucide-react';
78
import { useContext, useEffect, useState } from 'react';
@@ -66,14 +67,12 @@ export function StepMessagesTable({
6667
// Query for regular DAG runs
6768
const regularQuery = useQuery(
6869
'/dag-runs/{name}/{dagRunId}/steps/{stepName}/messages',
69-
isSubDAGRun
70-
? null
71-
: {
72-
params: {
73-
path: { name: effectiveName, dagRunId: effectiveRunId, stepName },
74-
query: { remoteNode: appBarContext.selectedRemoteNode || 'local' },
75-
},
76-
},
70+
whenEnabled(!isSubDAGRun, {
71+
params: {
72+
path: { name: effectiveName, dagRunId: effectiveRunId, stepName },
73+
query: { remoteNode: appBarContext.selectedRemoteNode || 'local' },
74+
},
75+
}),
7776
{
7877
refreshInterval: isActive ? 2000 : 0,
7978
revalidateOnFocus: false,
@@ -83,19 +82,17 @@ export function StepMessagesTable({
8382
// Query for sub-DAG runs
8483
const subDagQuery = useQuery(
8584
'/dag-runs/{name}/{dagRunId}/sub-dag-runs/{subDAGRunId}/steps/{stepName}/messages',
86-
isSubDAGRun
87-
? {
88-
params: {
89-
path: {
90-
name: effectiveName,
91-
dagRunId: effectiveRunId,
92-
subDAGRunId: subDAGRunId || '',
93-
stepName,
94-
},
95-
query: { remoteNode: appBarContext.selectedRemoteNode || 'local' },
96-
},
97-
}
98-
: null,
85+
whenEnabled(isSubDAGRun, {
86+
params: {
87+
path: {
88+
name: effectiveName,
89+
dagRunId: effectiveRunId,
90+
subDAGRunId: subDAGRunId || '',
91+
stepName,
92+
},
93+
query: { remoteNode: appBarContext.selectedRemoteNode || 'local' },
94+
},
95+
}),
9996
{
10097
refreshInterval: isActive ? 2000 : 0,
10198
revalidateOnFocus: false,

0 commit comments

Comments
 (0)