AAE-28918 Fix for start process button is enabled when required widgets are read only without value#11781
AAE-28918 Fix for start process button is enabled when required widgets are read only without value#11781dthornton-hyl wants to merge 4 commits intodevelopfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes start-process form validation so the Start button is not enabled when all required widgets are read-only but empty (avoids backend-side required validation errors).
Changes:
- Updates
FormFieldModel.validate()to mark read-only + required + visible fields as invalid when their value is empty (even when the field type is not in the “validatable” list). - Adds a small helper (
isValueEmpty) to detect empty values across common value shapes. - Expands unit tests to cover read-only required fields with/without values and hidden-field behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
lib/core/src/lib/form/components/widgets/core/form-field.model.ts |
Adds read-only required empty-value validation to prevent enabling Start when required read-only fields are empty. |
lib/core/src/lib/form/components/widgets/core/form-field.model.spec.ts |
Adds/updates tests validating the new read-only required behavior and hidden-field exemption. |
lib/core/src/lib/form/components/widgets/core/form-field.model.ts
Outdated
Show resolved
Hide resolved
lib/core/src/lib/form/components/widgets/core/form-field.model.ts
Outdated
Show resolved
Hide resolved
| @@ -187,6 +190,22 @@ export class FormFieldModel extends FormWidgetModel { | |||
| return !this.readOnly || FormFieldTypes.isValidatableType(this.type); | |||
There was a problem hiding this comment.
I'm surprised we don't have a isFieldOrParentHidden check here - if it is correct/valid to check for field visibility as part of isFieldValidatable I would probably refactor things a bit, but my next question is should we only be doing the required check validation for readonly fields, or should all validation be ran?
There was a problem hiding this comment.
And personal opinion (I know you didn't write this code), but I would rename this something like shouldValidate instead of isFieldValidatable
There was a problem hiding this comment.
I will look into it. Will convert this to a draft PR for the time being.
There was a problem hiding this comment.
FormFieldTypes.isValidatableType just had one field type in it. So isFieldValidatable() was pretty much returning if a field was readOnly or not. Since want to validate either readOnly or not. Can pretty much do away with the isFieldValidatable() check.
The validator.validate() has checks to see if the field is validation is supported (so making the isFieldValidatable check not needed) and it also checks the isFieldOrParentHidden
7120620 to
7f258b2
Compare
|
| return this._isValid; | ||
| } | ||
| const validators = this.form?.fieldValidators || []; | ||
| for (const validator of validators) { |
There was a problem hiding this comment.
I'm good with this change, but lets run it by product or the Automate Front End channel. It seems reasonable to me to require all fields to be valid regardless of being editable.
FormFieldTypes.isValidatableType is kind of an odd check to me because I don't know what a display-external-property type field is, which appears to be the only type it's checking for, but want to confirm the removal of that check was intentional?



Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (check one with "x")
What is the current behaviour? (You can also link to an open issue here)
https://hyland.atlassian.net/browse/AAE-28918
Start button is enable if all fields are read-only and required. Clicking Start button will return validation error from BE.
What is the new behaviour?
Start button is disable if all fields are read only and required
Does this PR introduce a breaking change? (check one with "x")
If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...
Other information: