Clients config updates for census reporting #20125
Conversation
ui/app/models/clients/config.js
Outdated
| if (model.reportingEnabled) { | ||
| return 'Retention period must be a minimum of 24 months.'; | ||
| } | ||
| return 'Retention period must be greater than or equal to 0.'; |
There was a problem hiding this comment.
I discussed a nit I had with @zofskeez out-of-band:
These errors will be returned by the backend. If we include them statically here, we'll have to make sure they're updated if the contraints are ever updated on the backend. I wonder if there's a way to use the backend errors instead?
There was a problem hiding this comment.
To align with the designs I wonder if we could leverage the current model validations pattern/object shape and maybe create a util that will convert API errors into the validations format that can be passed in to FormField?
There was a problem hiding this comment.
I think unless the errors from the API can be returned as an object with the key names mapped to the properties that have errors we won't actually know which form field to display the message on though.
There was a problem hiding this comment.
One idea to still leverage the backend errors is to instead warn here, so submission is still possible. This deviates from the design patterns a bit, but just throwing it out there
There was a problem hiding this comment.
Ya that could work but I'm not sure if it's preferable. If the backend value were to change then the form validation might warn that the minimum is 24 and then the API error would say something different like 36 in that message which may be confusing.
There was a problem hiding this comment.
Yeah, exactly. The subtext for the input also contains the minimum value, right? (Edit: nvm, those I guess were the original designs it doesn't seem like the subtext is added here) So either way we have to update the UI if/when the backend changes. Maybe a note in the backend code to update the corresponding UI code (or let the UI team know) if the value is ever changed, @mpalmi ?
There was a problem hiding this comment.
Yeah, that was one option I considered. Another would be to throw the minimum retention months in the config endpoint. I'm okay with either approach. I'll throw a comment in my open PR for now and we can come back to this if we ever need to!
…config save failure
* updates clients config view for census reporting * adds changelog entry * fixes issue with modal staying open and error not showing on clients config save failure * adds min retention months to clients config model and form validation
* updates clients config view for census reporting * adds changelog entry * fixes issue with modal staying open and error not showing on clients config save failure * adds min retention months to clients config model and form validation
* updates clients config view for census reporting * adds changelog entry * fixes issue with modal staying open and error not showing on clients config save failure * adds min retention months to clients config model and form validation
* updates clients config view for census reporting * adds changelog entry * fixes issue with modal staying open and error not showing on clients config save failure * adds min retention months to clients config model and form validation
…1.11.x (#21208) * Clients config updates for census reporting (#20125) * updates clients config view for census reporting * adds changelog entry * fixes issue with modal staying open and error not showing on clients config save failure * adds min retention months to clients config model and form validation * removes model-form-field decorator from clients config model
* updates clients config view for census reporting * adds changelog entry * fixes issue with modal staying open and error not showing on clients config save failure * adds min retention months to clients config model and form validation
The following updates were made to the clients configuration edit view to accommodate census reporting: