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.

Author: zubron

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

@@ -2,7 +2,7 @@ import React, {useCallback, useRef, useState} from "react";
 import {refs as refsAPI} from "../../../lib/api";
 import {RefTypeBranch} from "../../../constants";
 import {ActionGroup, ActionsBar, AlertError, RefreshButton} from "../controls";
-import {MetadataFields, filterEmptyMetadataFields} from "./metadata";
+import {MetadataFields, validateMetadataKeys} from "./metadata";
 import {GitMergeIcon} from "@primer/octicons-react";
 import Button from "react-bootstrap/Button";
 import Modal from "react-bootstrap/Modal";
@@ -43,7 +43,7 @@ const CompareBranchesActionsBar = (
 
 const MergeButton = ({repo, onDone, source, dest, disabled = false}) => {
     const textRef = useRef(null);
-    const [metadataFields, setMetadataFields] = useState([])
+    const [metadataFields, setMetadataFields] = useState([]);
     const initialMerge = {
         merging: false,
         show: false,
@@ -72,9 +72,12 @@ const MergeButton = ({repo, onDone, source, dest, disabled = false}) => {
     }
 
     const onSubmit = async () => {
+        if (!validateMetadataKeys(metadataFields, setMetadataFields)) {
+            return;
+        }
         const message = textRef.current.value;
         const metadata = {};
-        filterEmptyMetadataFields(metadataFields).forEach(pair => metadata[pair.key] = pair.value)
+        metadataFields.forEach(pair => metadata[pair.key] = pair.value);
 
         let strategy = mergeState.strategy;
         if (strategy === "none") {
@@ -147,7 +150,11 @@ const MergeButton = ({repo, onDone, source, dest, disabled = false}) => {
                     <Button variant="secondary" disabled={mergeState.merging} onClick={hide}>
                         Cancel
                     </Button>
-                    <Button variant="success" disabled={mergeState.merging} onClick={onSubmit}>
+                    <Button
+                        variant="success"
+                        disabled={mergeState.merging}
+                        onClick={onSubmit}
+                    >
                         {(mergeState.merging) ? 'Merging...' : 'Merge'}
                     </Button>
                 </Modal.Footer>

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

@@ -1,53 +1,89 @@
-import React, {useCallback} from "react";
+import React from "react";
 import {PlusIcon, XIcon} from "@primer/octicons-react";
 import Button from "react-bootstrap/Button";
 import Form from "react-bootstrap/Form";
 import Row from "react-bootstrap/Row";
 import Col from "react-bootstrap/Col";
 
 /**
- * MetadataFields component for adding metadata key-value pairs.
- * Used in commit, import, and merge modals.
+ * Helper to check if a metadata key is empty or whitespace-only
+ */
+const isEmptyKey = (key) => !key || key.trim() === "";
+
+/**
+ * MetadataFields is a component that allows the user to add/remove key-value pairs of metadata.
+ * @param {Array<{key: string, value: string, touched: boolean}>} metadataFields - current metadata fields to display
+ * @param {Function} setMetadataFields - callback to update the metadata fields
+ * @param rest - any other props to pass to the component
  */
 export const MetadataFields = ({ metadataFields, setMetadataFields, ...rest}) => {
-    const onChangeKey = useCallback((i) => {
+    const onChangeKey = (i) => {
         return e => {
-            const key = e.currentTarget.value;
-            setMetadataFields(prev => [...prev.slice(0,i), {...prev[i], key}, ...prev.slice(i+1)]);
-            e.preventDefault()
+            const newKey = e.currentTarget.value;
+            setMetadataFields(prev =>
+                prev.map((field, idx) =>
+                    idx === i ? {...field, key: newKey} : field
+                )
+            );
         };
-    }, [setMetadataFields]);
+    };
 
-    const onChangeValue = useCallback((i) => {
+    const onChangeValue = (i) => {
         return e => {
-            const value = e.currentTarget.value;
-            setMetadataFields(prev => [...prev.slice(0,i),  {...prev[i], value}, ...prev.slice(i+1)]);
+            const newValue = e.currentTarget.value;
+            setMetadataFields(prev =>
+                prev.map((field, idx) =>
+                    idx === i ? {...field, value: newValue} : field
+                )
+            );
         };
-    }, [setMetadataFields]);
+    };
+
+    const onBlurKey = (i) => () => {
+        setMetadataFields(prev =>
+            prev.map((field, idx) =>
+                idx === i ? {...field, touched: true} : field
+            )
+        );
+    };
 
-    const onRemovePair = useCallback((i) => {
-        return () => setMetadataFields(prev => [...prev.slice(0, i), ...prev.slice(i + 1)])
-    }, [setMetadataFields])
 
-    const onAddPair = useCallback(() => {
-        setMetadataFields(prev => [...prev, {key: "", value: ""}])
-    }, [setMetadataFields])
+    const onRemoveKeyValue = (i) => () => {
+        setMetadataFields(prev => prev.filter((_, idx) => idx !== i));
+    };
+
+    const onAddKeyValue = () => {
+        setMetadataFields(prev => [...prev, {key: "", value: "", touched: false}]);
+    };
 
     return (
         <div className="mt-3 mb-3" {...rest}>
             {metadataFields.map((f, i) => {
+                const showError = isEmptyKey(f.key) && f.touched;
                 return (
                     <Form.Group key={`commit-metadata-field-${i}`} className="mb-3">
                         <Row>
                             <Col md={{span: 5}}>
-                                <Form.Control type="text" placeholder="Key" defaultValue={f.key} onChange={onChangeKey(i)}/>
+                                <Form.Control
+                                    type="text"
+                                    placeholder="Key"
+                                    value={f.key}
+                                    onChange={onChangeKey(i)}
+                                    onBlur={onBlurKey(i)}
+                                    isInvalid={showError}
+                                />
+                                {showError && (
+                                    <Form.Control.Feedback type="invalid">
+                                        Key is required
+                                    </Form.Control.Feedback>
+                                )}
                             </Col>
                             <Col md={{span: 5}}>
-                                <Form.Control type="text" placeholder="Value" defaultValue={f.value}  onChange={onChangeValue(i)}/>
+                                <Form.Control type="text" placeholder="Value" value={f.value}  onChange={onChangeValue(i)}/>
                             </Col>
                             <Col md={{span: 1}}>
                                 <Form.Text>
-                                    <Button size="sm" variant="secondary" onClick={onRemovePair(i)}>
+                                    <Button size="sm" variant="secondary" onClick={onRemoveKeyValue(i)}>
                                         <XIcon/>
                                     </Button>
                                 </Form.Text>
@@ -56,7 +92,7 @@ export const MetadataFields = ({ metadataFields, setMetadataFields, ...rest}) =>
                     </Form.Group>
                 )
             })}
-            <Button onClick={onAddPair} size="sm" variant="secondary">
+            <Button onClick={onAddKeyValue} size="sm" variant="secondary">
                 <PlusIcon/>{' '}
                 Add Metadata field
             </Button>
@@ -65,15 +101,19 @@ export const MetadataFields = ({ metadataFields, setMetadataFields, ...rest}) =>
 }
 
 /**
- * Filters out metadata fields where both key and value are empty or whitespace-only.
- * Allows fields with empty values (but non-empty keys) or empty keys (but non-empty values) matching the current CLI behavior.
+ * Validates metadata fields and marks empty keys as touched to show validation errors.
+ * Use this before submitting to ensure all keys are filled in.
  *
- * @param {Array<{key: string, value: string}>} metadataFields - Array of metadata field objects
- * @returns {Array<{key: string, value: string}>} Filtered array excluding fields where both key and value are empty
+ * @param {Array<{key: string, value: string, touched: boolean}>} metadataFields - Array of metadata field objects
+ * @param {Function} setMetadataFields - Setter function to update metadata fields
+ * @returns {boolean} True if validation passed (no empty keys), false if validation failed
  */
-export const filterEmptyMetadataFields = (metadataFields) => {
-    return metadataFields.filter(field =>
-        (field.key && field.key.trim() !== "") ||
-        (field.value && field.value.trim() !== "")
+export const validateMetadataKeys = (metadataFields, setMetadataFields) => {
+    const hasEmptyKeys = metadataFields.some(f => isEmptyKey(f.key));
+    if (!hasEmptyKeys) return true;
+
+    setMetadataFields(prev =>
+        prev.map(f => isEmptyKey(f.key) ? {...f, touched: true} : f)
     );
-};
+    return false;
+};

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

@@ -1,91 +1,93 @@
 import { describe, it, expect } from 'vitest';
-import { filterEmptyMetadataFields } from './metadata';
+import { hasEmptyMetadataKeys } from './metadata';
 
-describe('filterEmptyMetadataFields', () => {
-    it('should include fields with both key and value', () => {
+describe('hasEmptyMetadataKeys', () => {
+    it('should return false when all fields have non-empty keys', () => {
         const input = [
             { key: 'key1', value: 'value1' },
             { key: 'key2', value: 'value2' },
         ];
-        const result = filterEmptyMetadataFields(input);
-        expect(result).toEqual(input);
+        const result = hasEmptyMetadataKeys(input);
+        expect(result).toBe(false);
     });
 
-    it('should include fields with non-empty key and empty value', () => {
+    it('should return false when fields have non-empty keys and empty values', () => {
         const input = [
             { key: 'key_with_empty_value', value: '' },
             { key: 'tag', value: 'v1.0' }
         ];
-        const result = filterEmptyMetadataFields(input);
-        expect(result).toEqual(input);
+        const result = hasEmptyMetadataKeys(input);
+        expect(result).toBe(false);
     });
 
-    it('should include fields with empty key and non-empty value', () => {
+    it('should return true when any field has an empty key', () => {
         const input = [
             { key: '', value: 'value for empty key' },
             { key: 'tag', value: 'v1.0' }
         ];
-        const result = filterEmptyMetadataFields(input);
-        expect(result).toEqual(input);
+        const result = hasEmptyMetadataKeys(input);
+        expect(result).toBe(true);
     });
 
-    it('should filter out fields where both key and value are empty', () => {
+    it('should return true when field has empty key and empty value', () => {
         const input = [
             { key: 'key1', value: 'value1' },
             { key: '', value: '' },
             { key: 'key2', value: 'value2' },
         ];
-        const expected = [
-            { key: 'key1', value: 'value1' },
-            { key: 'key2', value: 'value2' },
-        ];
-        const result = filterEmptyMetadataFields(input);
-        expect(result).toEqual(expected);
+        const result = hasEmptyMetadataKeys(input);
+        expect(result).toBe(true);
     });
 
-    it('should filter out fields where both key and value are whitespace only', () => {
+    it('should return true when field has whitespace-only key', () => {
         const input = [
             { key: 'key1', value: 'value1' },
-            { key: '   ', value: '  ' },
-            { key: '\t', value: '\n' },
+            { key: '   ', value: 'value' },
             { key: 'key2', value: 'value2' },
         ];
-        const expected = [
-            { key: 'key1', value: 'value1' },
-            { key: 'key2', value: 'value2' },
-        ];
-        const result = filterEmptyMetadataFields(input);
-        expect(result).toEqual(expected);
+        const result = hasEmptyMetadataKeys(input);
+        expect(result).toBe(true);
     });
 
-    it('should handle empty array', () => {
+    it('should return false for empty array', () => {
         const input = [];
-        const result = filterEmptyMetadataFields(input);
-        expect(result).toEqual([]);
+        const result = hasEmptyMetadataKeys(input);
+        expect(result).toBe(false);
     });
 
-    it('should handle array with only empty fields', () => {
+    it('should return true when all fields have empty keys', () => {
         const input = [
-            { key: '', value: '' },
-            { key: '  ', value: '  ' }
+            { key: '', value: 'value1' },
+            { key: '  ', value: 'value2' }
         ];
-        const result = filterEmptyMetadataFields(input);
-        expect(result).toEqual([]);
+        const result = hasEmptyMetadataKeys(input);
+        expect(result).toBe(true);
     });
 
-    it('should include fields with whitespace in key but non-empty value', () => {
+    it('should return true when key is whitespace with non-empty value', () => {
         const input = [
             { key: '   ', value: 'has value' }
         ];
-        const result = filterEmptyMetadataFields(input);
-        expect(result).toEqual(input);
+        const result = hasEmptyMetadataKeys(input);
+        expect(result).toBe(true);
     });
 
-    it('should include fields with non-empty key but whitespace in value', () => {
+    it('should return false when key is non-empty even with whitespace value', () => {
         const input = [
             { key: 'haskey', value: '   ' }
         ];
-        const result = filterEmptyMetadataFields(input);
-        expect(result).toEqual(input);
+        const result = hasEmptyMetadataKeys(input);
+        expect(result).toBe(false);
+    });
+
+    it('should return true when at least one key is empty among multiple fields', () => {
+        const input = [
+            { key: 'key1', value: 'value1' },
+            { key: 'key2', value: 'value2' },
+            { key: '', value: 'value3' },
+            { key: 'key4', value: 'value4' },
+        ];
+        const result = hasEmptyMetadataKeys(input);
+        expect(result).toBe(true);
     });
 });