Enforce non-empty metadata keys in forms

webui/package.json

@@ -66,7 +66,9 @@
     "@stoplight/prism-cli": "^5.14.2",
     "@stoplight/prism-core": "^5.8.0",
     "@stoplight/prism-http-server": "^5.12.2",
+    "@testing-library/jest-dom": "^6.9.1",
     "@testing-library/react": "^16.3.0",
+    "@testing-library/user-event": "^14.6.1",
     "@types/bootstrap": "^5.2.10",
     "@types/js-yaml": "^4.0.9",
     "@types/jsonwebtoken": "^9.0.10",

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

@@ -3,6 +3,7 @@ import {refs as refsAPI} from "../../../lib/api";
 import {RefTypeBranch} from "../../../constants";
 import {ActionGroup, ActionsBar, AlertError, RefreshButton} from "../controls";
 import {MetadataFields} from "./metadata";
+import {getMetadataIfValid, touchInvalidFields} from "./metadataHelpers";
 import {GitMergeIcon} from "@primer/octicons-react";
 import Button from "react-bootstrap/Button";
 import Modal from "react-bootstrap/Modal";
@@ -72,9 +73,13 @@ const MergeButton = ({repo, onDone, source, dest, disabled = false}) => {
     }
 
     const onSubmit = async () => {
+        const metadata = getMetadataIfValid(metadataFields);
+        if (!metadata) {
+            setMetadataFields(touchInvalidFields(metadataFields));
+            return;
+        }
+
         const message = textRef.current.value;
-        const metadata = {};
-        metadataFields.forEach(pair => metadata[pair.key] = pair.value)
 
         let strategy = mergeState.strategy;
         if (strategy === "none") {

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

@@ -5,10 +5,11 @@ import Button from "react-bootstrap/Button";
 import Form from "react-bootstrap/Form";
 import Row from "react-bootstrap/Row";
 import Col from "react-bootstrap/Col";
+import { getFieldError } from './metadataHelpers'
 
 /**
  * MetadataFields is a component that allows the user to add/remove key-value pairs of metadata.
- * @param {Array<{key: string, value: string}>} metadataFields - current metadata fields to display
+ * @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
  */
@@ -27,29 +28,42 @@ export const MetadataFields = ({ metadataFields, setMetadataFields, ...rest}) =>
         };
     };
 
+    const onBlurKey = (i) => () => {
+        setMetadataFields(prev => [...prev.slice(0,i),  {...prev[i], touched: true}, ...prev.slice(i+1)]);
+    };
+
     const onRemoveKeyValue = (i) => {
         return () => setMetadataFields(prev => [...prev.slice(0, i), ...prev.slice(i + 1)]);
     };
 
     const onAddKeyValue = () => {
-        setMetadataFields(prev => [...prev, {key: "", value: ""}])
+        setMetadataFields(prev => [...prev, {key: "", value: "", touched: false}]);
     };
 
     return (
         <div className="mt-3 mb-3" {...rest}>
             {metadataFields.map((f, i) => {
+                const fieldError = getFieldError(f)
                 return (
                     <Form.Group key={`commit-metadata-field-${i}`} className="mb-3">
                         <Row>
                             <Col md={{span: 5}}>
-                                <Form.Control type="text" placeholder="Key" value={f.key} onChange={onChangeKey(i)}/>
+                                <Form.Control
+                                    type="text"
+                                    placeholder="Key"
+                                    value={f.key}
+                                    onChange={onChangeKey(i)}
+                                    onBlur={onBlurKey(i)}
+                                    isInvalid={fieldError}
+                                />
+                                {fieldError && <Form.Control.Feedback type="invalid">{fieldError}</Form.Control.Feedback>}
                             </Col>
                             <Col md={{span: 5}}>
                                 <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={onRemoveKeyValue(i)}>
+                                    <Button size="sm" variant="secondary" onClick={onRemoveKeyValue(i)} aria-label={`Remove metadata field ${i + 1}`} >
                                         <XIcon/>
                                     </Button>
                                 </Form.Text>

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

@@ -0,0 +1,100 @@
+import React from "react";
+import { describe, it, expect } from 'vitest';
+import { render, screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import { MetadataFields } from './metadata';
+
+/**
+ * MetadataFieldsWrapper is a component wrapper used for testing the MetadataFields component.
+ * It uses the actual React useState hook to manage the state passed to MetadataFields,
+ * ensuring that state updates automatically trigger re-renders in the test environment.
+ *
+ * @param {object} props
+ * @param {Array<Object>} [props.initialFields=[]] - The initial array of metadata field objects to populate the component's state.
+ * @returns {React.JSX.Element} The MetadataFields component rendered with real state management.
+ */
+const MetadataFieldsWrapper = ({ initialFields }) => {
+    const [fields, setFields] = React.useState(initialFields);
+
+    return (<MetadataFields metadataFields={fields} setMetadataFields={setFields}/>);
+};
+
+describe('MetadataFields validation flow', () => {
+    it('does not show error when key is valid', () => {
+        render(<MetadataFieldsWrapper initialFields={[{ key: 'environment', value: 'prod', touched: false }]} />);
+
+        expect(screen.queryByText('Key is required')).not.toBeInTheDocument();
+    });
+
+    it('shows error when key is empty', () => {
+        render(<MetadataFieldsWrapper initialFields={[{ key: '', value: '', touched: true }]} />);
+
+        expect(screen.getByText('Key is required')).toBeInTheDocument();
+    });
+
+    it('shows error when key is whitespace only', () => {
+        render(<MetadataFieldsWrapper initialFields={[{ key: '  ', value: '', touched: true }]} />);
+
+        expect(screen.getByText('Key is required')).toBeInTheDocument();
+    });
+
+    it('shows error after user blurs empty key field', async () => {
+        const user = userEvent.setup();
+
+        render(<MetadataFieldsWrapper initialFields={[{ key: '', value: '', touched: false }]} />);
+        expect(screen.queryByText('Key is required')).not.toBeInTheDocument();
+
+        const keyInput = screen.getByPlaceholderText('Key');
+        await user.click(keyInput);
+        await user.tab();
+
+        expect(await screen.findByText('Key is required')).toBeInTheDocument();
+    });
+
+    it('clears error when user enters a valid key after blur', async () => {
+        const user = userEvent.setup();
+
+        render(<MetadataFieldsWrapper initialFields={[{ key: '', value: '', touched: false }]} />);
+
+        const keyInput = screen.getByPlaceholderText('Key');
+        await user.click(keyInput);
+        await user.tab();
+        expect(await screen.findByText('Key is required')).toBeInTheDocument();
+
+        await user.type(keyInput, 'env');
+
+        expect(screen.queryByText('Key is required')).not.toBeInTheDocument();
+        expect(keyInput).not.toHaveClass('is-invalid');
+        expect(keyInput).toHaveValue('env');
+    });
+
+    it('adds a new metadata field row when clicking Add button', async () => {
+        const user = userEvent.setup();
+
+        render(<MetadataFieldsWrapper initialFields={[]} />);
+
+        await user.click(screen.getByText(/Add Metadata field/i));
+
+        expect(screen.getAllByPlaceholderText('Key')).toHaveLength(1);
+        expect(screen.getByPlaceholderText('Value')).toHaveValue('');
+    });
+
+    it('removes the correct metadata row', async () => {
+        const user = userEvent.setup();
+
+        render(<MetadataFieldsWrapper initialFields={[
+            { key: 'a', value: '1', touched: false },
+            { key: 'b', value: '2', touched: false }
+        ]} />);
+
+        const firstDeleteButton = screen.getByRole('button', { name: 'Remove metadata field 1' });
+
+        await user.click(firstDeleteButton);
+
+        const keyInputs = screen.getAllByPlaceholderText('Key');
+        expect(keyInputs).toHaveLength(1);
+        expect(keyInputs[0]).toHaveValue('b');
+        expect(screen.queryByDisplayValue('a')).not.toBeInTheDocument();
+    });
+});
+

webui/src/lib/components/repository/metadataHelpers.js

@@ -0,0 +1,73 @@
+const isEmptyKey = (key) => !key || key.trim() === "";
+
+const isInvalidKey = (key) => {
+    // TODO: Add more validation checks, e.g. duplicate keys, invalid characters
+    return isEmptyKey(key);
+};
+
+/**
+ * Checks if there are any invalid keys in the metadata fields.
+ * @param {Array<{key: string, value: string, touched: boolean}>} metadataFields
+ * @returns {boolean} True if there are any invalid keys
+ */
+export const hasInvalidKeys = (metadataFields) => {
+    return metadataFields.some(f => isInvalidKey(f.key));
+};
+
+/**
+ * Converts metadata fields array to a plain key-value object.
+ * @param {Array<{key: string, value: string, touched: boolean}>} metadataFields
+ * @returns {Object} Metadata object with key-value pairs
+ */
+export const fieldsToMetadata = (metadataFields) => {
+    const metadata = {};
+    metadataFields.forEach(pair => metadata[pair.key] = pair.value);
+    return metadata;
+};
+
+/**
+ * Returns a new array with invalid fields marked as touched.
+ * Does not mutate the input array.
+ * @param {Array<{key: string, value: string, touched: boolean}>} metadataFields
+ * @returns {Array<{key: string, value: string, touched: boolean}>} New array with invalid fields touched
+ */
+export const touchInvalidFields = (metadataFields) => {
+    return metadataFields.map(field =>
+        isInvalidKey(field.key) ? { ...field, touched: true } : field
+    );
+};
+
+/**
+ * Validates metadata fields and returns the metadata object if valid, or null if invalid.
+ *
+ * Use this in form submission handlers. If null is returned, call touchInvalidFields()
+ * to mark invalid fields and show errors to the user.
+ *
+ * @param {Array<{key: string, value: string, touched: boolean}>} metadataFields
+ * @returns {Object|null} Metadata object if valid, null if validation failed
+ */
+export const getMetadataIfValid = (metadataFields) => {
+    if (hasInvalidKeys(metadataFields)) {
+        return null;
+    }
+    return fieldsToMetadata(metadataFields);
+};
+
+/**
+ * Returns the validation error message for a metadata field, if any.
+ * Only returns an error if the field has been touched (interacted with by the user).
+ *
+ * Use this function in the MetadataFields component to display field-level errors.
+ *
+ * @param {Object} field - Metadata field object with key, value, and touched properties
+ * @returns {string|null} Error message if field is invalid and touched, null otherwise
+ */
+export const getFieldError = (field) => {
+    if (!field.touched) return null;
+
+    if (isEmptyKey(field.key)) {
+        return "Key is required";
+    }
+
+    return null;
+};