Skip to content

Commit ab120e9

Browse files
committed
Validate the aggregator ID with the form, not by hand
Motivation: The create screen tracked the ID in its own state and re-checked it inside the submit handler, which is what react-hook-form already does for every other ID field in the console. The regex and the YAML validation helper were copied into the new editor rather than shared. Modifications: - Register the ID input with react-hook-form (`required` + `pattern`), following NewGroup: the form owns the value and the inline error, and the submit handler no longer re-checks it. - Move the shared ID pattern to XdsTypes and the YAML validation helper to its own module; the resource editor uses both instead of its own copies. Result: One definition of what a valid xDS ID is, and one of how invalid YAML is reported.
1 parent 2632e33 commit ab120e9

5 files changed

Lines changed: 63 additions & 56 deletions

File tree

webapp/src/dogma/features/xds/K8sAggregatorEditor.tsx

Lines changed: 22 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import { FetchBaseQueryError } from '@reduxjs/toolkit/query';
4141
import { default as RouteLink } from 'next/link';
4242
import Router from 'next/router';
4343
import { useEffect, useMemo, useState } from 'react';
44+
import { useForm } from 'react-hook-form';
4445
import { AiOutlineClose, AiOutlineDelete, AiOutlineEdit, AiOutlineEye } from 'react-icons/ai';
4546
import { FiSave } from 'react-icons/fi';
4647
import * as jsYaml from 'js-yaml';
@@ -63,33 +64,24 @@ import { useGroupWriteAccess } from 'dogma/features/xds/useGroupWriteAccess';
6364
import { useAppDispatch } from 'dogma/hooks';
6465
import { newNotification } from 'dogma/features/notification/notificationSlice';
6566
import ErrorMessageParser from 'dogma/features/services/ErrorMessageParser';
66-
67-
// Mirrors XdsResourceManager.RESOURCE_ID_PATTERN_STRING.
68-
const AGGREGATOR_ID_PATTERN = /^[a-z](?:[a-z0-9_.-]*[a-z0-9])?$/;
69-
70-
function validateYamlOrNotify(dispatch: ReturnType<typeof useAppDispatch>, value: string): boolean {
71-
try {
72-
jsYaml.load(value);
73-
return true;
74-
} catch (e) {
75-
dispatch(newNotification('Invalid YAML', (e as Error).message, 'error'));
76-
return false;
77-
}
78-
}
67+
import { validateYamlOrNotify } from 'dogma/features/xds/validateYaml';
68+
import { XDS_ID_PATTERN } from 'dogma/features/xds/XdsTypes';
7969

8070
const NewK8sAggregatorEditor = ({ group }: { group: string }) => {
8171
const dispatch = useAppDispatch();
8272
const { hasWrite, isLoading: accessLoading } = useGroupWriteAccess(group);
83-
const [id, setId] = useState('');
73+
const {
74+
register,
75+
handleSubmit,
76+
formState: { errors },
77+
} = useForm<{ aggregatorId: string }>();
8478
const [content, setContent] = useState('');
8579
const [commitSummary, setCommitSummary] = useState('');
8680
const [createAggregator, { isLoading }] = useCreateK8sAggregatorMutation();
8781
const [previewAggregator, { isLoading: isPreviewing }] = usePreviewK8sAggregatorMutation();
8882
const { isOpen: previewOpen, onOpen: openPreview, onClose: closePreview } = useDisclosure();
8983
const [previewResult, setPreviewResult] = useState<K8sPreviewResult | null>(null);
9084

91-
const idIsInvalid = id.length > 0 && !AGGREGATOR_ID_PATTERN.test(id);
92-
9385
const handlePreview = async () => {
9486
if (!validateYamlOrNotify(dispatch, content)) {
9587
return;
@@ -104,35 +96,18 @@ const NewK8sAggregatorEditor = ({ group }: { group: string }) => {
10496
}
10597
};
10698

107-
const handleCreate = async () => {
108-
if (!hasWrite) {
109-
return;
110-
}
111-
if (!id) {
112-
dispatch(newNotification('ID is required', 'Please enter the aggregator ID', 'error'));
113-
return;
114-
}
115-
if (idIsInvalid) {
116-
dispatch(
117-
newNotification(
118-
'Invalid ID',
119-
'Aggregator ID must match [a-z](?:[a-z0-9_.-]*[a-z0-9])? (dots allowed, slashes not allowed)',
120-
'error',
121-
),
122-
);
123-
return;
124-
}
99+
const handleCreate = async ({ aggregatorId }: { aggregatorId: string }) => {
125100
if (!validateYamlOrNotify(dispatch, content)) {
126101
return;
127102
}
128103
try {
129104
await createAggregator({
130105
group,
131-
aggregatorId: id,
106+
aggregatorId,
132107
body: content,
133108
summary: commitSummary || undefined,
134109
}).unwrap();
135-
dispatch(newNotification('Aggregator created', `Aggregator '${id}' is created`, 'success'));
110+
dispatch(newNotification('Aggregator created', `Aggregator '${aggregatorId}' is created`, 'success'));
136111
Router.push(`/app/xds/group?name=${encodeURIComponent(group)}&type=k8sAggregators`);
137112
} catch (err) {
138113
dispatch(newNotification('Failed to create the aggregator', ErrorMessageParser.parse(err), 'error'));
@@ -153,9 +128,12 @@ const NewK8sAggregatorEditor = ({ group }: { group: string }) => {
153128

154129
return (
155130
<Box>
156-
<FormControl isRequired isInvalid={idIsInvalid} mb={4} maxW="md">
131+
<FormControl isRequired isInvalid={errors.aggregatorId != null} mb={4} maxW="md">
157132
<FormLabel>Aggregator ID</FormLabel>
158-
<Input value={id} onChange={(e) => setId(e.target.value)} placeholder="Enter aggregator ID ..." />
133+
<Input
134+
placeholder="Enter aggregator ID ..."
135+
{...register('aggregatorId', { required: true, pattern: XDS_ID_PATTERN })}
136+
/>
159137
<FormErrorMessage>
160138
ID must match [a-z](?:[a-z0-9_.-]*[a-z0-9])? (dots allowed, slashes not allowed)
161139
</FormErrorMessage>
@@ -175,7 +153,12 @@ const NewK8sAggregatorEditor = ({ group }: { group: string }) => {
175153
>
176154
Preview endpoints
177155
</Button>
178-
<Button colorScheme="teal" leftIcon={<FiSave />} onClick={handleCreate} isLoading={isLoading}>
156+
<Button
157+
colorScheme="teal"
158+
leftIcon={<FiSave />}
159+
onClick={handleSubmit(handleCreate)}
160+
isLoading={isLoading}
161+
>
179162
Create
180163
</Button>
181164
</EditorActionBar>

webapp/src/dogma/features/xds/ResourceEditor.tsx

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ import Router from 'next/router';
4646
import { useEffect, useMemo, useState } from 'react';
4747
import { AiOutlineClose, AiOutlineDelete, AiOutlineEdit } from 'react-icons/ai';
4848
import { FiSave } from 'react-icons/fi';
49-
import * as jsYaml from 'js-yaml';
5049
import { Deferred } from 'dogma/common/components/Deferred';
5150
import { JsonEditor } from 'dogma/common/components/JsonEditor';
5251
import { DeleteConfirmationModal } from 'dogma/common/components/DeleteConfirmationModal';
@@ -56,7 +55,12 @@ import {
5655
useGetResourceQuery,
5756
useUpdateResourceMutation,
5857
} from 'dogma/features/xds/xdsApiSlice';
59-
import { XdsResourceType, XDS_RESOURCE_META, XDS_RESOURCE_TEMPLATES } from 'dogma/features/xds/XdsTypes';
58+
import {
59+
XdsResourceType,
60+
XDS_RESOURCE_META,
61+
XDS_RESOURCE_TEMPLATES,
62+
XDS_ID_PATTERN,
63+
} from 'dogma/features/xds/XdsTypes';
6064
import { extractReferences, referenceHref, resolveReference } from 'dogma/features/xds/xdsReferences';
6165
import { ResourceGraph } from 'dogma/features/xds/ResourceGraph';
6266
import { ResourceHistory } from 'dogma/features/xds/ResourceHistory';
@@ -65,20 +69,9 @@ import { useGroupWriteAccess } from 'dogma/features/xds/useGroupWriteAccess';
6569
import { useAppDispatch } from 'dogma/hooks';
6670
import { newNotification } from 'dogma/features/notification/notificationSlice';
6771
import ErrorMessageParser from 'dogma/features/services/ErrorMessageParser';
72+
import { validateYamlOrNotify } from 'dogma/features/xds/validateYaml';
6873

6974
// Dots are allowed (e.g. "my-service.v1"), but slashes are not.
70-
const RESOURCE_ID_PATTERN = /^[a-z](?:[a-z0-9_.-]*[a-z0-9])?$/;
71-
72-
function validateYamlOrNotify(dispatch: ReturnType<typeof useAppDispatch>, value: string): boolean {
73-
try {
74-
jsYaml.load(value);
75-
return true;
76-
} catch (e) {
77-
dispatch(newNotification('Invalid YAML', (e as Error).message, 'error'));
78-
return false;
79-
}
80-
}
81-
8275
const NewResourceEditor = ({ group, type }: { group: string; type: XdsResourceType }) => {
8376
const meta = XDS_RESOURCE_META[type];
8477
const dispatch = useAppDispatch();
@@ -89,7 +82,7 @@ const NewResourceEditor = ({ group, type }: { group: string; type: XdsResourceTy
8982
const [commitSummary, setCommitSummary] = useState('');
9083
const [createResource, { isLoading }] = useCreateResourceMutation();
9184

92-
const idIsInvalid = id.length > 0 && !RESOURCE_ID_PATTERN.test(id);
85+
const idIsInvalid = id.length > 0 && !XDS_ID_PATTERN.test(id);
9386

9487
const handleCreate = async () => {
9588
if (!hasWrite) {

webapp/src/dogma/features/xds/XdsTypes.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
// The internal Central Dogma project that backs every xDS group.
1818
export const XDS_PROJECT = '@xds';
1919

20+
// Mirrors XdsResourceManager.RESOURCE_ID_PATTERN_STRING.
21+
export const XDS_ID_PATTERN = /^[a-z](?:[a-z0-9_.-]*[a-z0-9])?$/;
22+
2023
// xDS resource types. Each value is also the repository directory name and the
2124
// path segment used by the xDS HTTP API (e.g. /api/v1/xds/groups/{group}/clusters).
2225
export type XdsResourceType = 'listeners' | 'routes' | 'clusters' | 'endpoints';
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
* Copyright 2026 LY Corporation
3+
*
4+
* LY Corporation licenses this file to you under the Apache License,
5+
* version 2.0 (the "License"); you may not use this file except in compliance
6+
* with the License. You may obtain a copy of the License at:
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13+
* License for the specific language governing permissions and limitations
14+
* under the License.
15+
*/
16+
import * as jsYaml from 'js-yaml';
17+
import { useAppDispatch } from 'dogma/hooks';
18+
import { newNotification } from 'dogma/features/notification/notificationSlice';
19+
20+
export function validateYamlOrNotify(dispatch: ReturnType<typeof useAppDispatch>, value: string): boolean {
21+
try {
22+
jsYaml.load(value);
23+
return true;
24+
} catch (e) {
25+
dispatch(newNotification('Invalid YAML', (e as Error).message, 'error'));
26+
return false;
27+
}
28+
}

webapp/tests/dogma/features/xds/K8sAggregatorEditor.test.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,12 +109,12 @@ describe('K8sAggregatorEditor', () => {
109109
describe('new aggregator', () => {
110110
it('rejects a slash ID and does not create', async () => {
111111
const user = userEvent.setup();
112-
const { store } = renderWithProviders(<K8sAggregatorEditor group="foo" isNew />);
112+
renderWithProviders(<K8sAggregatorEditor group="foo" isNew />);
113113

114114
await user.type(screen.getByPlaceholderText('Enter aggregator ID ...'), 'foo/bar');
115115
await user.click(screen.getByRole('button', { name: /^create$/i }));
116116

117-
await waitFor(() => expect(store.getState().notification.title).toBe('Invalid ID'));
117+
await waitFor(() => expect(screen.getByText(/ID must match/)).toBeInTheDocument());
118118
expect(mockCreate).not.toHaveBeenCalled();
119119
});
120120

0 commit comments

Comments
 (0)