diff --git a/workspaces/dcm/.changeset/fix-policy-form-validation.md b/workspaces/dcm/.changeset/fix-policy-form-validation.md new file mode 100644 index 0000000000..f439501c28 --- /dev/null +++ b/workspaces/dcm/.changeset/fix-policy-form-validation.md @@ -0,0 +1,25 @@ +--- +'@red-hat-developer-hub/backstage-plugin-dcm': patch +--- + +Fix policy form validation for Description and Priority fields, normalize enabled +checks, and reject whitespace-only required fields. + +- Add a 255-character max-length Yup rule to the Description field, with inline error + feedback matching the Display Name pattern (validation error on blur rather than + hard-blocking input at the DOM level). +- Add `.integer()` and `.required()` Yup rules to the Priority field so decimal values + (e.g. 500.5) and an empty field are rejected with an explicit error message instead of + silently coercing to an integer or defaulting to 500. +- Add `step={1}` to the Priority number input and block `.`/`e`/`E` characters via + `onKeyDown` and `onChange` guards to prevent decimal strings from bypassing the Yup + integer check through JavaScript's `Number()` coercion. +- Update the Priority label to "Priority \*" to indicate it is a required field. +- Update the Priority helper text to surface the API uniqueness constraint + (priority must be unique per policy type) so users are informed before hitting a 409. +- Normalize the three inconsistent `enabled` checks in `PoliciesTabContent` to + `p.enabled ?? true` so the Enabled column, Actions toggle, and edit form all agree + when `enabled` is undefined. +- Add `.trim()` to the `display_name` and `rego_code` Yup rules so whitespace-only + values are rejected client-side instead of passing validation and being silently + rejected by the API after submit. diff --git a/workspaces/dcm/plugins/dcm/src/pages/policies/PoliciesTabContent.tsx b/workspaces/dcm/plugins/dcm/src/pages/policies/PoliciesTabContent.tsx index bedaa1002f..b0588459f2 100644 --- a/workspaces/dcm/plugins/dcm/src/pages/policies/PoliciesTabContent.tsx +++ b/workspaces/dcm/plugins/dcm/src/pages/policies/PoliciesTabContent.tsx @@ -119,7 +119,7 @@ export function PoliciesTabContent() { setTogglingIds(addTogglingId(id)); try { const updated = await policyApi.updatePolicy(id, { - enabled: !p.enabled, + enabled: !(p.enabled ?? true), }); setItems(replacePolicyById(id, updated)); } catch (err) { @@ -172,7 +172,7 @@ export function PoliciesTabContent() { title: 'Enabled', field: 'enabled', render: p => { - const isEnabled = p.enabled === true; + const isEnabled = p.enabled ?? true; return ( { const id = p.id ?? ''; const isToggling = togglingIds.has(id); - const isEnabled = p.enabled !== false; + const isEnabled = p.enabled ?? true; return ( setForm(prev => ({ ...prev, description: e.target.value })) } + onBlur={() => touch('description')} fullWidth variant="outlined" size="small" @@ -111,19 +115,29 @@ export function PolicyFormFields({ setForm(prev => ({ ...prev, priority: e.target.value }))} + onChange={e => { + if (/^\d*$/.test(e.target.value)) { + setForm(prev => ({ ...prev, priority: e.target.value })); + } + }} + onKeyDown={e => { + if (e.key === '.' || e.key === 'e' || e.key === 'E') { + e.preventDefault(); + } + }} onBlur={() => touch('priority')} fullWidth variant="outlined" size="small" type="number" - inputProps={{ min: 1, max: 1000 }} + inputProps={{ min: 1, max: 1000, step: 1 }} /> { expect(errors.display_name).toBeDefined(); }); + it('rejects whitespace-only display_name', () => { + const errors = validatePolicyForm({ ...valid(), display_name: ' ' }); + expect(errors.display_name).toBeDefined(); + }); + it('requires rego_code', () => { const errors = validatePolicyForm({ ...valid(), rego_code: '' }); expect(errors.rego_code).toBeDefined(); }); + it('rejects whitespace-only rego_code', () => { + const errors = validatePolicyForm({ ...valid(), rego_code: ' ' }); + expect(errors.rego_code).toBeDefined(); + }); + it('validates priority range', () => { const tooLow = validatePolicyForm({ ...valid(), priority: '0' }); expect(tooLow.priority).toBeDefined(); @@ -52,6 +62,23 @@ describe('validatePolicyForm', () => { const ok = validatePolicyForm({ ...valid(), priority: '500' }); expect(ok.priority).toBeUndefined(); + + const decimal = validatePolicyForm({ ...valid(), priority: '500.5' }); + expect(decimal.priority).toBeDefined(); + + const empty = validatePolicyForm({ ...valid(), priority: '' }); + expect(empty.priority).toBeDefined(); + }); + + it('rejects description longer than 255 characters', () => { + const errors = validatePolicyForm({ + ...valid(), + description: 'a'.repeat(256), + }); + expect(errors.description).toBeDefined(); + + const ok = validatePolicyForm({ ...valid(), description: 'a'.repeat(255) }); + expect(ok.description).toBeUndefined(); }); it('accepts only GLOBAL or USER as policy_type', () => { diff --git a/workspaces/dcm/plugins/dcm/src/pages/policies/policyFormTypes.ts b/workspaces/dcm/plugins/dcm/src/pages/policies/policyFormTypes.ts index 1682c2fd82..8c6e1bb3b1 100644 --- a/workspaces/dcm/plugins/dcm/src/pages/policies/policyFormTypes.ts +++ b/workspaces/dcm/plugins/dcm/src/pages/policies/policyFormTypes.ts @@ -34,9 +34,13 @@ export type PolicyForm = { const policySchema = yup.object({ display_name: yup .string() + .trim() .required('Display name is required') .min(1, 'Display name cannot be empty') .max(255, 'Display name must be at most 255 characters'), + description: yup + .string() + .max(255, 'Description must be at most 255 characters'), policy_type: yup .string() .required('Policy type is required') @@ -44,10 +48,13 @@ const policySchema = yup.object({ priority: yup .number() .typeError('Priority must be a number') + .required('Priority is required') + .integer('Priority must be a whole number') .min(1, 'Priority must be at least 1') .max(1000, 'Priority must be at most 1000'), rego_code: yup .string() + .trim() .required('Rego code is required') .min(1, 'Rego code cannot be empty'), });