Skip to content

Commit 8584b4a

Browse files
teeohhemteeohhem
authored
fix: Fix default values in SourceForm (#1532)
Fixes: HDX-3116
1 parent 3019bec commit 8584b4a

5 files changed

Lines changed: 268 additions & 64 deletions

File tree

‎.changeset/tender-days-beam.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@hyperdx/app": patch
3+
---
4+
5+
fix: source form was not loading properly for all sources

‎packages/app/src/DBSearchPage.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ function SourceEditMenu({
216216
</Menu.Item>
217217
{IS_LOCAL_MODE ? (
218218
<Menu.Item
219-
data-testid="edit-source-menu-item"
219+
data-testid="edit-sources-menu-item"
220220
leftSection={<IconSettings size={14} />}
221221
onClick={() => setModelFormExpanded(v => !v)}
222222
>

‎packages/app/src/components/SourceForm.tsx‎

Lines changed: 79 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -289,8 +289,11 @@ function HighlightedAttributeExpressionsFormRow({
289289

290290
/** Component for configuring one or more materialized views */
291291
function MaterializedViewsFormSection({ control, setValue }: TableModelProps) {
292-
const databaseName =
293-
useWatch({ control, name: `from.databaseName` }) || DEFAULT_DATABASE;
292+
const databaseName = useWatch({
293+
control,
294+
name: `from.databaseName`,
295+
defaultValue: DEFAULT_DATABASE,
296+
});
294297

295298
const {
296299
fields: materializedViews,
@@ -357,13 +360,21 @@ function MaterializedViewFormSection({
357360
setValue,
358361
}: { mvIndex: number; onRemove: () => void } & TableModelProps) {
359362
const connection = useWatch({ control, name: `connection` });
360-
const sourceDatabaseName =
361-
useWatch({ control, name: `from.databaseName` }) || DEFAULT_DATABASE;
362-
const mvDatabaseName =
363-
useWatch({ control, name: `materializedViews.${mvIndex}.databaseName` }) ||
364-
sourceDatabaseName;
365-
const mvTableName =
366-
useWatch({ control, name: `materializedViews.${mvIndex}.tableName` }) || '';
363+
const sourceDatabaseName = useWatch({
364+
control,
365+
name: `from.databaseName`,
366+
defaultValue: DEFAULT_DATABASE,
367+
});
368+
const mvDatabaseName = useWatch({
369+
control,
370+
name: `materializedViews.${mvIndex}.databaseName`,
371+
defaultValue: sourceDatabaseName,
372+
});
373+
const mvTableName = useWatch({
374+
control,
375+
name: `materializedViews.${mvIndex}.tableName`,
376+
defaultValue: '',
377+
});
367378

368379
return (
369380
<Stack gap="sm">
@@ -615,12 +626,17 @@ function AggregatedColumnRow({
615626
onRemove: () => void;
616627
}) {
617628
const connectionId = useWatch({ control, name: `connection` });
618-
const sourceDatabaseName =
619-
useWatch({ control, name: `from.databaseName` }) || DEFAULT_DATABASE;
629+
const sourceDatabaseName = useWatch({
630+
control,
631+
name: `from.databaseName`,
632+
defaultValue: DEFAULT_DATABASE,
633+
});
620634
const sourceTableName = useWatch({ control, name: `from.tableName` });
621-
const mvDatabaseName =
622-
useWatch({ control, name: `materializedViews.${mvIndex}.databaseName` }) ||
623-
sourceDatabaseName;
635+
const mvDatabaseName = useWatch({
636+
control,
637+
name: `materializedViews.${mvIndex}.databaseName`,
638+
defaultValue: sourceDatabaseName,
639+
});
624640
const mvTableName = useWatch({
625641
control,
626642
name: `materializedViews.${mvIndex}.tableName`,
@@ -1226,7 +1242,7 @@ export function TraceTableModelForm(props: TableModelProps) {
12261242
);
12271243
}
12281244

1229-
export function SessionTableModelForm({ control, setValue }: TableModelProps) {
1245+
export function SessionTableModelForm({ control }: TableModelProps) {
12301246
const databaseName = useWatch({
12311247
control,
12321248
name: 'from.databaseName',
@@ -1411,36 +1427,45 @@ export function TableSourceForm({
14111427
const { data: source } = useSource({ id: sourceId });
14121428
const { data: connections } = useConnections();
14131429

1414-
const {
1415-
control,
1416-
setValue,
1417-
formState,
1418-
handleSubmit,
1419-
resetField,
1420-
setError,
1421-
clearErrors,
1422-
} = useForm<TSourceUnion>({
1423-
defaultValues: {
1424-
kind: SourceKind.Log,
1425-
name: defaultName,
1426-
connection: connections?.[0]?.id,
1427-
from: {
1428-
databaseName: 'default',
1429-
tableName: '',
1430+
const { control, setValue, handleSubmit, resetField, setError, clearErrors } =
1431+
useForm<TSourceUnion>({
1432+
defaultValues: {
1433+
kind: SourceKind.Log,
1434+
name: defaultName,
1435+
connection: connections?.[0]?.id,
1436+
from: {
1437+
databaseName: 'default',
1438+
tableName: '',
1439+
},
14301440
},
1431-
},
1432-
// TODO: HDX-1768 remove type assertion
1433-
values: source as TSourceUnion,
1434-
resetOptions: {
1435-
keepDirtyValues: true,
1436-
keepErrors: true,
1437-
},
1438-
});
1441+
// TODO: HDX-1768 remove type assertion
1442+
values: source as TSourceUnion,
1443+
resetOptions: {
1444+
keepDirtyValues: true,
1445+
keepErrors: true,
1446+
},
1447+
});
14391448

1440-
const watchedConnection = useWatch({ control, name: 'connection' });
1441-
const watchedDatabaseName = useWatch({ control, name: 'from.databaseName' });
1442-
const watchedTableName = useWatch({ control, name: 'from.tableName' });
1443-
const watchedKind = useWatch({ control, name: 'kind' });
1449+
const watchedConnection = useWatch({
1450+
control,
1451+
name: 'connection',
1452+
defaultValue: source?.connection,
1453+
});
1454+
const watchedDatabaseName = useWatch({
1455+
control,
1456+
name: 'from.databaseName',
1457+
defaultValue: source?.from?.databaseName || DEFAULT_DATABASE,
1458+
});
1459+
const watchedTableName = useWatch({
1460+
control,
1461+
name: 'from.tableName',
1462+
defaultValue: source?.from?.tableName,
1463+
});
1464+
const watchedKind = useWatch({
1465+
control,
1466+
name: 'kind',
1467+
defaultValue: source?.kind || SourceKind.Log,
1468+
});
14441469
const prevTableNameRef = useRef(watchedTableName);
14451470

14461471
useEffect(() => {
@@ -1493,7 +1518,11 @@ export function TableSourceForm({
14931518
resetField('connection', { defaultValue: connections?.[0]?.id });
14941519
}, [connections, resetField]);
14951520

1496-
const kind: SourceKind = useWatch({ control, name: 'kind' });
1521+
const kind = useWatch({
1522+
control,
1523+
name: 'kind',
1524+
defaultValue: source?.kind || SourceKind.Log,
1525+
});
14971526

14981527
const createSource = useCreateSource();
14991528
const updateSource = useUpdateSource();
@@ -1751,9 +1780,13 @@ export function TableSourceForm({
17511780
const databaseName = useWatch({
17521781
control,
17531782
name: 'from.databaseName',
1754-
defaultValue: DEFAULT_DATABASE,
1783+
defaultValue: source?.from?.databaseName || DEFAULT_DATABASE,
1784+
});
1785+
const connectionId = useWatch({
1786+
control,
1787+
name: 'connection',
1788+
defaultValue: source?.connection,
17551789
});
1756-
const connectionId = useWatch({ control, name: 'connection' });
17571790

17581791
return (
17591792
<div

‎packages/app/tests/e2e/features/sources.spec.ts‎

Lines changed: 135 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,79 @@
11
import { SearchPage } from '../page-objects/SearchPage';
22
import { expect, test } from '../utils/base-test';
33

4-
test.describe('Sources Functionality', () => {
4+
const COMMON_FIELDS = [
5+
'Name',
6+
'Source Data Type',
7+
'Server Connection',
8+
'Database',
9+
'Table',
10+
];
11+
12+
const LOG_FIELDS = [
13+
...COMMON_FIELDS,
14+
'Service Name Expression',
15+
'Log Level Expression',
16+
'Body Expression',
17+
'Log Attributes Expression',
18+
'Resource Attributes Expression',
19+
'Displayed Timestamp Column',
20+
'Correlated Metric Source',
21+
'Correlated Trace Source',
22+
'Trace Id Expression',
23+
'Span Id Expression',
24+
'Implicit Column Expression',
25+
];
26+
27+
const TRACE_FIELDS = [
28+
...COMMON_FIELDS,
29+
'Duration Expression',
30+
'Duration Precision',
31+
'Trace Id Expression',
32+
'Span Id Expression',
33+
'Parent Span Id Expression',
34+
'Span Name Expression',
35+
'Span Kind Expression',
36+
'Correlated Log Source',
37+
'Correlated Session Source',
38+
'Correlated Metric Source',
39+
'Status Code Expression',
40+
'Status Message Expression',
41+
'Service Name Expression',
42+
'Resource Attributes Expression',
43+
'Event Attributes Expression',
44+
'Span Events Expression',
45+
'Implicit Column Expression',
46+
'Displayed Timestamp Column',
47+
];
48+
49+
const SESSION_FIELDS = [...COMMON_FIELDS, 'Correlated Trace Source'];
50+
51+
const METRIC_FIELDS = [
52+
...COMMON_FIELDS.slice(0, -1), // Remove Table
53+
'gauge Table',
54+
'histogram Table',
55+
'sum Table',
56+
'summary Table',
57+
'exponential histogram Table',
58+
'Correlated Log Source',
59+
];
60+
61+
const editableSourcesData = [
62+
{ name: 'Demo Logs', fields: LOG_FIELDS, radioButtonName: 'Log' },
63+
{ name: 'Demo Traces', fields: TRACE_FIELDS, radioButtonName: 'Trace' },
64+
];
65+
66+
const allSourcesData = [
67+
...editableSourcesData,
68+
{
69+
name: 'Demo Metrics',
70+
fields: METRIC_FIELDS,
71+
radioButtonName: 'OTEL Metrics',
72+
},
73+
{ name: 'Demo Sessions', fields: SESSION_FIELDS, radioButtonName: 'Session' },
74+
];
75+
76+
test.describe('Sources Functionality', { tag: ['@sources'] }, () => {
577
let searchPage: SearchPage;
678

779
test.beforeEach(async ({ page }) => {
@@ -11,21 +83,71 @@ test.describe('Sources Functionality', () => {
1183

1284
test('should open source settings menu', async () => {
1385
// Click source settings menu
14-
const sourceSettingsMenu = searchPage.page.locator(
15-
'[data-testid="source-settings-menu"]',
16-
);
17-
await sourceSettingsMenu.click();
86+
await searchPage.sourceMenu.click();
1887

1988
// Verify create new source menu item is visible
20-
const createNewSourceMenuItem = searchPage.page.locator(
21-
'[data-testid="create-new-source-menu-item"]',
22-
);
23-
await expect(createNewSourceMenuItem).toBeVisible();
89+
await expect(searchPage.createNewSourceItem).toBeVisible();
2490

2591
// Verify edit source menu items are visible
26-
const editSourceMenuItems = searchPage.page.locator(
27-
'[data-testid="edit-source-menu-item"], [data-testid="edit-sources-menu-item"]',
28-
);
29-
await expect(editSourceMenuItems.first()).toBeVisible();
92+
await expect(searchPage.editSourceMenuItem).toBeVisible();
93+
});
94+
95+
test(
96+
'should show the correct source form when modal is open',
97+
{ tag: ['@sources'] },
98+
async () => {
99+
test.skip(
100+
process.env.E2E_FULLSTACK === 'true',
101+
'Skipping source form tests in fullstack mode due to UI differences',
102+
);
103+
for (const sourceData of editableSourcesData) {
104+
await test.step(`Verify ${sourceData.name} fields`, async () => {
105+
// Demo Logs is selected by default, so we don't need to select it again
106+
if (sourceData.name !== 'Demo Logs') {
107+
await searchPage.selectSource(sourceData.name);
108+
}
109+
await searchPage.openEditSourceModal();
110+
await searchPage.sourceModalShowOptionalFields();
111+
112+
for (const field of sourceData.fields) {
113+
await expect(
114+
searchPage.page.getByText(field, { exact: true }),
115+
).toBeVisible();
116+
}
117+
118+
// press escape to close the modal
119+
await searchPage.page.keyboard.press('Escape');
120+
});
121+
}
122+
},
123+
);
124+
125+
test('should show proper fields when creating a new source', async () => {
126+
await searchPage.sourceMenu.click();
127+
await searchPage.createNewSourceItem.click();
128+
// for each source type (log, trace, session, metric), verify the correct fields are shown
129+
for (const sourceData of allSourcesData) {
130+
await test.step(`Verify ${sourceData.radioButtonName} source type`, async () => {
131+
// Find the radio button by its label
132+
const radioButton = searchPage.page.getByLabel(
133+
sourceData.radioButtonName,
134+
{ exact: true },
135+
);
136+
137+
// Click the radio button
138+
await radioButton.click();
139+
140+
// Show optional fields if the button exists
141+
await searchPage.sourceModalShowOptionalFields();
142+
143+
// Verify fields
144+
for (const field of sourceData.fields) {
145+
await expect(
146+
searchPage.page.getByText(field, { exact: true }),
147+
).toBeVisible();
148+
}
149+
});
150+
}
151+
await searchPage.page.keyboard.press('Escape');
30152
});
31153
});

0 commit comments

Comments
 (0)