Skip to content

Commit 47c9992

Browse files
zubronzubron
committed
Enforce non-empty metadata keys in forms
Add client-side validation to prevent form submission (commit, merge, import) when metadata keys are empty or whitespace-only. Fields are marked invalid when: - User blurs an empty key input - Form is submitted with empty keys Invalid fields display "Key is required" error message. Added component tests using React Testing Library to verify validation behavior. Closes #5251
1 parent b05f8f8 commit 47c9992

File tree

8 files changed

+477
-207
lines changed

8 files changed

+477
-207
lines changed

‎webui/package-lock.json‎

Lines changed: 282 additions & 200 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎webui/package.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,9 @@
6666
"@stoplight/prism-cli": "^5.14.2",
6767
"@stoplight/prism-core": "^5.8.0",
6868
"@stoplight/prism-http-server": "^5.12.2",
69+
"@testing-library/jest-dom": "^6.9.1",
6970
"@testing-library/react": "^16.3.0",
71+
"@testing-library/user-event": "^14.6.1",
7072
"@types/bootstrap": "^5.2.10",
7173
"@types/js-yaml": "^4.0.9",
7274
"@types/jsonwebtoken": "^9.0.10",

‎webui/src/lib/components/repository/compareBranchesActionBar.jsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import React, {useCallback, useRef, useState} from "react";
22
import {refs as refsAPI} from "../../../lib/api";
33
import {RefTypeBranch} from "../../../constants";
44
import {ActionGroup, ActionsBar, AlertError, RefreshButton} from "../controls";
5-
import {MetadataFields} from "./metadata";
5+
import {MetadataFields, validateMetadataKeys} from "./metadata";
66
import {GitMergeIcon} from "@primer/octicons-react";
77
import Button from "react-bootstrap/Button";
88
import Modal from "react-bootstrap/Modal";
@@ -72,6 +72,9 @@ const MergeButton = ({repo, onDone, source, dest, disabled = false}) => {
7272
}
7373

7474
const onSubmit = async () => {
75+
if (!validateMetadataKeys(metadataFields, setMetadataFields)) {
76+
return;
77+
}
7578
const message = textRef.current.value;
7679
const metadata = {};
7780
metadataFields.forEach(pair => metadata[pair.key] = pair.value)

‎webui/src/lib/components/repository/metadata.jsx‎

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import Col from "react-bootstrap/Col";
88

99
/**
1010
* MetadataFields is a component that allows the user to add/remove key-value pairs of metadata.
11-
* @param {Array<{key: string, value: string}>} metadataFields - current metadata fields to display
11+
* @param {Array<{key: string, value: string, touched: boolean}>} metadataFields - current metadata fields to display
1212
* @param {Function} setMetadataFields - callback to update the metadata fields
1313
* @param rest - any other props to pass to the component
1414
*/
@@ -27,29 +27,46 @@ export const MetadataFields = ({ metadataFields, setMetadataFields, ...rest}) =>
2727
};
2828
};
2929

30+
const onBlurKey = (i) => () => {
31+
setMetadataFields(prev => [...prev.slice(0,i), {...prev[i], touched: true}, ...prev.slice(i+1)]);
32+
};
33+
3034
const onRemoveKeyValue = (i) => {
3135
return () => setMetadataFields(prev => [...prev.slice(0, i), ...prev.slice(i + 1)]);
3236
};
3337

3438
const onAddKeyValue = () => {
35-
setMetadataFields(prev => [...prev, {key: "", value: ""}])
39+
setMetadataFields(prev => [...prev, {key: "", value: "", touched: false}]);
3640
};
3741

3842
return (
3943
<div className="mt-3 mb-3" {...rest}>
4044
{metadataFields.map((f, i) => {
45+
const showError = isEmptyKey(f.key) && f.touched;
4146
return (
4247
<Form.Group key={`commit-metadata-field-${i}`} className="mb-3">
4348
<Row>
4449
<Col md={{span: 5}}>
45-
<Form.Control type="text" placeholder="Key" value={f.key} onChange={onChangeKey(i)}/>
50+
<Form.Control
51+
type="text"
52+
placeholder="Key"
53+
value={f.key}
54+
onChange={onChangeKey(i)}
55+
onBlur={onBlurKey(i)}
56+
isInvalid={showError}
57+
/>
58+
{showError && (
59+
<Form.Control.Feedback type="invalid">
60+
Key is required
61+
</Form.Control.Feedback>
62+
)}
4663
</Col>
4764
<Col md={{span: 5}}>
4865
<Form.Control type="text" placeholder="Value" value={f.value} onChange={onChangeValue(i)}/>
4966
</Col>
5067
<Col md={{span: 1}}>
5168
<Form.Text>
52-
<Button size="sm" variant="secondary" onClick={onRemoveKeyValue(i)}>
69+
<Button size="sm" variant="secondary" onClick={onRemoveKeyValue(i)} aria-label={`Remove metadata field ${i + 1}`} >
5370
<XIcon/>
5471
</Button>
5572
</Form.Text>
@@ -65,3 +82,23 @@ export const MetadataFields = ({ metadataFields, setMetadataFields, ...rest}) =>
6582
</div>
6683
)
6784
}
85+
86+
const isEmptyKey = (key) => !key || key.trim() === "";
87+
88+
/**
89+
* Validates metadata fields and marks empty keys as touched to show validation errors.
90+
* Use this before submitting to ensure all keys are filled in.
91+
*
92+
* @param {Array<{key: string, value: string, touched: boolean}>} metadataFields - Array of metadata field objects
93+
* @param {Function} setMetadataFields - Setter function to update metadata fields
94+
* @returns {boolean} True if validation passed (no empty keys), false if validation failed
95+
*/
96+
export const validateMetadataKeys = (metadataFields, setMetadataFields) => {
97+
const hasEmptyKeys = metadataFields.some(f => isEmptyKey(f.key));
98+
if (!hasEmptyKeys) return true;
99+
100+
setMetadataFields(prev =>
101+
prev.map(f => isEmptyKey(f.key) ? {...f, touched: true} : f)
102+
);
103+
return false;
104+
};

‎webui/src/lib/components/repository/metadata.test.jsx‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
import React from "react";
2+
import { describe, it, expect, vi } from 'vitest';
3+
import { render, screen } from '@testing-library/react';
4+
import userEvent from '@testing-library/user-event';
5+
import { MetadataFields, validateMetadataKeys } from './metadata';
6+
7+
/**
8+
* MetadataFieldsWrapper is a component wrapper used for testing the MetadataFields component.
9+
* It uses the actual React useState hook to manage the state passed to MetadataFields,
10+
* ensuring that state updates automatically trigger re-renders in the test environment.
11+
*
12+
* @param {object} props
13+
* @param {Array<Object>} [props.initialFields=[]] - The initial array of metadata field objects to populate the component's state.
14+
* @returns {React.JSX.Element} The MetadataFields component rendered with real state management.
15+
*/
16+
const MetadataFieldsWrapper = ({ initialFields }) => {
17+
const [fields, setFields] = React.useState(initialFields);
18+
19+
return (<MetadataFields metadataFields={fields} setMetadataFields={setFields}/>);
20+
};
21+
22+
describe('MetadataFields validation flow', () => {
23+
it('does not show error when key is valid', () => {
24+
render(<MetadataFieldsWrapper initialFields={[{ key: 'environment', value: 'prod', touched: false }]} />);
25+
26+
expect(screen.queryByText('Key is required')).not.toBeInTheDocument();
27+
});
28+
29+
it('shows error when key is empty', () => {
30+
render(<MetadataFieldsWrapper initialFields={[{ key: '', value: '', touched: true }]} />);
31+
32+
expect(screen.getByText('Key is required')).toBeInTheDocument();
33+
});
34+
35+
it('shows error when key is whitespace only', () => {
36+
render(<MetadataFieldsWrapper initialFields={[{ key: ' ', value: '', touched: true }]} />);
37+
38+
expect(screen.getByText('Key is required')).toBeInTheDocument();
39+
});
40+
41+
it('shows error after user blurs empty key field', async () => {
42+
const user = userEvent.setup();
43+
44+
render(<MetadataFieldsWrapper initialFields={[{ key: '', value: '', touched: false }]} />);
45+
expect(screen.queryByText('Key is required')).not.toBeInTheDocument();
46+
47+
const keyInput = screen.getByPlaceholderText('Key');
48+
await user.click(keyInput);
49+
await user.tab();
50+
51+
expect(await screen.findByText('Key is required')).toBeInTheDocument();
52+
});
53+
54+
it('clears error when user enters a valid key after blur', async () => {
55+
const user = userEvent.setup();
56+
57+
render(<MetadataFieldsWrapper initialFields={[{ key: '', value: '', touched: false }]} />);
58+
59+
const keyInput = screen.getByPlaceholderText('Key');
60+
await user.click(keyInput);
61+
await user.tab();
62+
expect(await screen.findByText('Key is required')).toBeInTheDocument();
63+
64+
await user.type(keyInput, 'env');
65+
66+
expect(screen.queryByText('Key is required')).not.toBeInTheDocument();
67+
expect(keyInput).not.toHaveClass('is-invalid');
68+
expect(keyInput).toHaveValue('env');
69+
});
70+
71+
it('adds a new metadata field row when clicking Add button', async () => {
72+
const user = userEvent.setup();
73+
74+
render(<MetadataFieldsWrapper initialFields={[]} />);
75+
76+
await user.click(screen.getByText(/Add Metadata field/i));
77+
78+
expect(screen.getAllByPlaceholderText('Key')).toHaveLength(1);
79+
expect(screen.getByPlaceholderText('Value')).toHaveValue('');
80+
});
81+
82+
it('removes the correct metadata row', async () => {
83+
const user = userEvent.setup();
84+
85+
render(<MetadataFieldsWrapper initialFields={[
86+
{ key: 'a', value: '1', touched: false },
87+
{ key: 'b', value: '2', touched: false }
88+
]} />);
89+
90+
const firstDeleteButton = screen.getByRole('button', { name: 'Remove metadata field 1' });
91+
92+
await user.click(firstDeleteButton);
93+
94+
const keyInputs = screen.getAllByPlaceholderText('Key');
95+
expect(keyInputs).toHaveLength(1);
96+
expect(keyInputs[0]).toHaveValue('b');
97+
expect(screen.queryByDisplayValue('a')).not.toBeInTheDocument();
98+
});
99+
});
100+
101+
describe('validateMetadataKeys', () => {
102+
it('returns true for empty array', () => {
103+
const setMetadataFields = vi.fn();
104+
expect(validateMetadataKeys([], setMetadataFields)).toBe(true);
105+
expect(setMetadataFields).not.toHaveBeenCalled();
106+
});
107+
108+
it('returns true when all keys are valid', () => {
109+
const setMetadataFields = vi.fn();
110+
const fields = [
111+
{ key: "key1", value: "value1", touched: false },
112+
{ key: "key2", value: "", touched: false },
113+
];
114+
115+
expect(validateMetadataKeys(fields, setMetadataFields)).toBe(true);
116+
expect(setMetadataFields).not.toHaveBeenCalled();
117+
});
118+
119+
it('returns false when any key is empty or whitespace', () => {
120+
const setMetadataFields = vi.fn();
121+
const fields = [
122+
{ key: "", value: "value", touched: false },
123+
{ key: " ", value: "value", touched: false },
124+
];
125+
126+
expect(validateMetadataKeys(fields, setMetadataFields)).toBe(false);
127+
expect(setMetadataFields).toHaveBeenCalledTimes(1);
128+
});
129+
});

‎webui/src/pages/repositories/repository/objects.jsx‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ import {useDropzone} from "react-dropzone";
4545
import pMap from "p-map";
4646
import {formatAlertText} from "../../../lib/components/repository/errors";
4747
import {ChangesTreeContainer} from "../../../lib/components/repository/changes";
48-
import {MetadataFields} from "../../../lib/components/repository/metadata";
48+
import {MetadataFields, validateMetadataKeys} from "../../../lib/components/repository/metadata";
4949
import {ConfirmationModal} from "../../../lib/components/modals";
5050
import { Link } from "../../../lib/components/nav";
5151
import Card from "react-bootstrap/Card";
@@ -88,10 +88,14 @@ const CommitButton = ({repo, onCommit, enabled = false}) => {
8888

8989
const hide = () => {
9090
if (committing) return;
91-
setShow(false)
91+
setShow(false);
92+
setMetadataFields([]);
9293
}
9394

9495
const onSubmit = () => {
96+
if (!validateMetadataKeys(metadataFields, setMetadataFields)) {
97+
return;
98+
}
9599
const message = textRef.current.value;
96100
const metadata = {};
97101
metadataFields.forEach(pair => metadata[pair.key] = pair.value)
@@ -217,6 +221,9 @@ const ImportModal = ({config, repoId, referenceId, referenceType, path = '', onD
217221
};
218222

219223
const doImport = async () => {
224+
if (!validateMetadataKeys(metadataFields, setMetadataFields)) {
225+
return;
226+
}
220227
setImportPhase(ImportPhase.InProgress);
221228
try {
222229
const metadata = {};

‎webui/test/setup.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import { expect, afterEach } from 'vitest';
2+
import { cleanup } from '@testing-library/react';
3+
import * as matchers from '@testing-library/jest-dom/matchers';
4+
5+
expect.extend(matchers);
6+
7+
afterEach(() => {
8+
cleanup();
9+
});

‎webui/vite.config.mts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ export default ({ command }) => {
99
test: {
1010
environment: 'happy-dom',
1111
exclude: ["./test/e2e/**/*", "./node_modules/**/*"],
12+
setupFiles: './test/setup.js',
1213
},
1314
plugins: [
1415
replace({

0 commit comments

Comments
 (0)