Skip to content

Commit 5acff90

Browse files
zubronzubron
committed
Mark empty metadata keys as invalid
When creating a commit/merge/import, prevent submission when any metadata key is empty. When a metadata field is added, if the key field is touched but left empty, the field will be marked as invalid. If empty metadata fields are added but not touched, when the user clicks submit, the fields will be validated and marked as invalid. Closes #5251
1 parent b05f8f8 commit 5acff90

File tree

4 files changed

+174
-17
lines changed

4 files changed

+174
-17
lines changed

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

Lines changed: 11 additions & 4 deletions
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";
@@ -43,7 +43,7 @@ const CompareBranchesActionsBar = (
4343

4444
const MergeButton = ({repo, onDone, source, dest, disabled = false}) => {
4545
const textRef = useRef(null);
46-
const [metadataFields, setMetadataFields] = useState([])
46+
const [metadataFields, setMetadataFields] = useState([]);
4747
const initialMerge = {
4848
merging: false,
4949
show: false,
@@ -72,9 +72,12 @@ 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 = {};
77-
metadataFields.forEach(pair => metadata[pair.key] = pair.value)
80+
metadataFields.forEach(pair => metadata[pair.key] = pair.value);
7881

7982
let strategy = mergeState.strategy;
8083
if (strategy === "none") {
@@ -147,7 +150,11 @@ const MergeButton = ({repo, onDone, source, dest, disabled = false}) => {
147150
<Button variant="secondary" disabled={mergeState.merging} onClick={hide}>
148151
Cancel
149152
</Button>
150-
<Button variant="success" disabled={mergeState.merging} onClick={onSubmit}>
153+
<Button
154+
variant="success"
155+
disabled={mergeState.merging}
156+
onClick={onSubmit}
157+
>
151158
{(mergeState.merging) ? 'Merging...' : 'Merge'}
152159
</Button>
153160
</Modal.Footer>

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

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,14 @@ import Form from "react-bootstrap/Form";
66
import Row from "react-bootstrap/Row";
77
import Col from "react-bootstrap/Col";
88

9+
/**
10+
* Helper to check if a metadata key is empty or whitespace-only
11+
*/
12+
const isEmptyKey = (key) => !key || key.trim() === "";
13+
914
/**
1015
* 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
16+
* @param {Array<{key: string, value: string, touched: boolean}>} metadataFields - current metadata fields to display
1217
* @param {Function} setMetadataFields - callback to update the metadata fields
1318
* @param rest - any other props to pass to the component
1419
*/
@@ -27,22 +32,44 @@ export const MetadataFields = ({ metadataFields, setMetadataFields, ...rest}) =>
2732
};
2833
};
2934

35+
const onBlurKey = (i) => () => {
36+
setMetadataFields(prev =>
37+
prev.map((field, idx) =>
38+
idx === i ? {...field, touched: true} : field
39+
)
40+
);
41+
};
42+
43+
3044
const onRemoveKeyValue = (i) => {
3145
return () => setMetadataFields(prev => [...prev.slice(0, i), ...prev.slice(i + 1)]);
3246
};
3347

3448
const onAddKeyValue = () => {
35-
setMetadataFields(prev => [...prev, {key: "", value: ""}])
49+
setMetadataFields(prev => [...prev, {key: "", value: "", touched: false}]);
3650
};
3751

3852
return (
3953
<div className="mt-3 mb-3" {...rest}>
4054
{metadataFields.map((f, i) => {
55+
const showError = isEmptyKey(f.key) && f.touched;
4156
return (
4257
<Form.Group key={`commit-metadata-field-${i}`} className="mb-3">
4358
<Row>
4459
<Col md={{span: 5}}>
45-
<Form.Control type="text" placeholder="Key" value={f.key} onChange={onChangeKey(i)}/>
60+
<Form.Control
61+
type="text"
62+
placeholder="Key"
63+
value={f.key}
64+
onChange={onChangeKey(i)}
65+
onBlur={onBlurKey(i)}
66+
isInvalid={showError}
67+
/>
68+
{showError && (
69+
<Form.Control.Feedback type="invalid">
70+
Key is required
71+
</Form.Control.Feedback>
72+
)}
4673
</Col>
4774
<Col md={{span: 5}}>
4875
<Form.Control type="text" placeholder="Value" value={f.value} onChange={onChangeValue(i)}/>
@@ -65,3 +92,21 @@ export const MetadataFields = ({ metadataFields, setMetadataFields, ...rest}) =>
6592
</div>
6693
)
6794
}
95+
96+
/**
97+
* Validates metadata fields and marks empty keys as touched to show validation errors.
98+
* Use this before submitting to ensure all keys are filled in.
99+
*
100+
* @param {Array<{key: string, value: string, touched: boolean}>} metadataFields - Array of metadata field objects
101+
* @param {Function} setMetadataFields - Setter function to update metadata fields
102+
* @returns {boolean} True if validation passed (no empty keys), false if validation failed
103+
*/
104+
export const validateMetadataKeys = (metadataFields, setMetadataFields) => {
105+
const hasEmptyKeys = metadataFields.some(f => isEmptyKey(f.key));
106+
if (!hasEmptyKeys) return true;
107+
108+
setMetadataFields(prev =>
109+
prev.map(f => isEmptyKey(f.key) ? {...f, touched: true} : f)
110+
);
111+
return false;
112+
};

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

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import { describe, it, expect } from 'vitest';
2+
import { hasEmptyMetadataKeys } from './metadata';
3+
4+
describe('hasEmptyMetadataKeys', () => {
5+
it('should return false when all fields have non-empty keys', () => {
6+
const input = [
7+
{ key: 'key1', value: 'value1' },
8+
{ key: 'key2', value: 'value2' },
9+
];
10+
const result = hasEmptyMetadataKeys(input);
11+
expect(result).toBe(false);
12+
});
13+
14+
it('should return false when fields have non-empty keys and empty values', () => {
15+
const input = [
16+
{ key: 'key_with_empty_value', value: '' },
17+
{ key: 'tag', value: 'v1.0' }
18+
];
19+
const result = hasEmptyMetadataKeys(input);
20+
expect(result).toBe(false);
21+
});
22+
23+
it('should return true when any field has an empty key', () => {
24+
const input = [
25+
{ key: '', value: 'value for empty key' },
26+
{ key: 'tag', value: 'v1.0' }
27+
];
28+
const result = hasEmptyMetadataKeys(input);
29+
expect(result).toBe(true);
30+
});
31+
32+
it('should return true when field has empty key and empty value', () => {
33+
const input = [
34+
{ key: 'key1', value: 'value1' },
35+
{ key: '', value: '' },
36+
{ key: 'key2', value: 'value2' },
37+
];
38+
const result = hasEmptyMetadataKeys(input);
39+
expect(result).toBe(true);
40+
});
41+
42+
it('should return true when field has whitespace-only key', () => {
43+
const input = [
44+
{ key: 'key1', value: 'value1' },
45+
{ key: ' ', value: 'value' },
46+
{ key: 'key2', value: 'value2' },
47+
];
48+
const result = hasEmptyMetadataKeys(input);
49+
expect(result).toBe(true);
50+
});
51+
52+
it('should return false for empty array', () => {
53+
const input = [];
54+
const result = hasEmptyMetadataKeys(input);
55+
expect(result).toBe(false);
56+
});
57+
58+
it('should return true when all fields have empty keys', () => {
59+
const input = [
60+
{ key: '', value: 'value1' },
61+
{ key: ' ', value: 'value2' }
62+
];
63+
const result = hasEmptyMetadataKeys(input);
64+
expect(result).toBe(true);
65+
});
66+
67+
it('should return true when key is whitespace with non-empty value', () => {
68+
const input = [
69+
{ key: ' ', value: 'has value' }
70+
];
71+
const result = hasEmptyMetadataKeys(input);
72+
expect(result).toBe(true);
73+
});
74+
75+
it('should return false when key is non-empty even with whitespace value', () => {
76+
const input = [
77+
{ key: 'haskey', value: ' ' }
78+
];
79+
const result = hasEmptyMetadataKeys(input);
80+
expect(result).toBe(false);
81+
});
82+
83+
it('should return true when at least one key is empty among multiple fields', () => {
84+
const input = [
85+
{ key: 'key1', value: 'value1' },
86+
{ key: 'key2', value: 'value2' },
87+
{ key: '', value: 'value3' },
88+
{ key: 'key4', value: 'value4' },
89+
];
90+
const result = hasEmptyMetadataKeys(input);
91+
expect(result).toBe(true);
92+
});
93+
});

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

Lines changed: 22 additions & 10 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";
@@ -82,20 +82,24 @@ const isAbortedError = (error, controller) => {
8282
const CommitButton = ({repo, onCommit, enabled = false}) => {
8383
const textRef = useRef(null);
8484

85-
const [committing, setCommitting] = useState(false)
86-
const [show, setShow] = useState(false)
87-
const [metadataFields, setMetadataFields] = useState([])
85+
const [committing, setCommitting] = useState(false);
86+
const [show, setShow] = useState(false);
87+
const [metadataFields, setMetadataFields] = useState([]);
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 = {};
97-
metadataFields.forEach(pair => metadata[pair.key] = pair.value)
98-
setCommitting(true)
101+
metadataFields.forEach(pair => metadata[pair.key] = pair.value);
102+
setCommitting(true);
99103
onCommit({message, metadata}, () => {
100104
setCommitting(false)
101105
setShow(false);
@@ -126,7 +130,11 @@ const CommitButton = ({repo, onCommit, enabled = false}) => {
126130
<Button variant="secondary" disabled={committing} onClick={hide}>
127131
Cancel
128132
</Button>
129-
<Button variant="success" disabled={committing} onClick={onSubmit}>
133+
<Button
134+
variant="success"
135+
disabled={committing}
136+
onClick={onSubmit}
137+
>
130138
Commit Changes
131139
</Button>
132140
</Modal.Footer>
@@ -217,10 +225,13 @@ const ImportModal = ({config, repoId, referenceId, referenceType, path = '', onD
217225
};
218226

219227
const doImport = async () => {
228+
if (!validateMetadataKeys(metadataFields, setMetadataFields)) {
229+
return;
230+
}
220231
setImportPhase(ImportPhase.InProgress);
221232
try {
222233
const metadata = {};
223-
metadataFields.forEach(pair => metadata[pair.key] = pair.value)
234+
metadataFields.forEach(pair => metadata[pair.key] = pair.value);
224235
setImportPhase(ImportPhase.InProgress)
225236
await startImport(
226237
setImportID,
@@ -288,7 +299,8 @@ const ImportModal = ({config, repoId, referenceId, referenceType, path = '', onD
288299
importPhase={importPhase}
289300
importFunc={doImport}
290301
doneFunc={hide}
291-
isEnabled={isImportEnabled}/>
302+
isEnabled={isImportEnabled}
303+
/>
292304
</Modal.Footer>
293305
</Modal>
294306
</>

0 commit comments

Comments
 (0)