fix(DHIS2-16608): enforce setting validators and reject invalid saves#1478
fix(DHIS2-16608): enforce setting validators and reject invalid saves#1478Chisomchima wants to merge 2 commits intomasterfrom
Conversation
Chisomchima
commented
Apr 5, 2026
Chisomchima
commented
- Add shared buildValidatorsForMapping with min/max range checks
- Block saveKey when validation fails so bad values are not persisted
- Forward TextField onChange to FormBuilder for correct blur validation
- Add positive_number validator for credentialsExpiresReminderInDays
- Add shared buildValidatorsForMapping with min/max range checks - Block saveKey when validation fails so bad values are not persisted - Forward TextField onChange to FormBuilder for correct blur validation - Add positive_number validator for credentialsExpiresReminderInDays Made-with: Cursor
…HIS2-16608-settings-field-validation
|
|
🚀 Deployed on https://pr-1478.settings.netlify.dhis2.org |
tomzemp
left a comment
tomzemp
left a comment
There was a problem hiding this comment.
Looks good to me! Thanks for looking into this!
I've left a couple of comments. Also, if we have a min/max restrictions for a given setting, I think we can probably drop the 'positive_number' validator (e.g. if min:1, max:24, the value -1 would already fail because it's out of this range)? I don't care strongly about this point, but it seems like it would be unneeded.
| mapping.validators.forEach((name) => { | ||
| if (wordToValidatorMap.has(name)) { | ||
| const validator = wordToValidatorMap.get(name) | ||
| console.log('validator', validator.message, mapping) |
| validators.push({ | ||
| validator: (value) => { | ||
| if (value === '' || value === null || value === undefined) { | ||
| return true |
There was a problem hiding this comment.
don't we want this to be false in this case? E.g. if there's a min value, shouldn't it fail validation if it's empty?
| // Can be undefined for some custom sections, i.e. 'keyStopMetadataSync' | ||
| const mapping = settingsKeyMapping[key] | ||
|
|
||
| if (mapping && !locale) { |
There was a problem hiding this comment.
is there a reason to check for !locale? I think locale is only defined for the appearance settings, which don't have validators, so it seems like this won't make a difference
3 participants
