EES-5731: Add character limit validation to data block chart title and alt text. #5497
Conversation
tomjonesdev
requested review from
benoutram,
ntsim,
bennettstuart,
mmyoungman and
jack-hive
| RuleForEach(x => x.Charts).ChildRules(chart => | ||
| { | ||
| chart | ||
| .RuleFor(request => request.Title) | ||
| .NotEmpty() | ||
| .MaximumLength(220); | ||
|
|
||
| chart.RuleFor(request => request.Alt) | ||
| .NotEmpty() | ||
| .MaximumLength(220); | ||
| }); |
There was a problem hiding this comment.
Can be done in separate ticket, but I think there is an opportunity for clean up in these request objects: we can remove Charts from DataBlockCreateRequest. It is only used in tests.
This also makes me wonder if there is anything else in both request objects that aren't being used and can be removed.
Alternatively we could just add the same validation to DataBlockCreateRequest - if we're assuming that we may want to create a datablock that already has a chart in the future, but I'm inclined to think this won't happen.
There was a problem hiding this comment.
It took me a moment to realise that Create is only used by the system when a new block is created, and not triggered by the user clicking "Save" (that's for Update), so I agree that it should be done...in a separate ticket. Maybe bring it up in the dev team meeting on Monday? Concern is that it's no small task to review all request objects for mismatches and update the FE/BE accordingly, there's no simple way to identify them automatically (afaik), and I imagine there's not much value when considering the work needed.
Based on this, I think adding the same validation to Create would be a mistake as we'd be muddying the logic further by implementing logic which isn't used/relevant.
This PR adds character limit validation and dynamic character counts to data block chart titles and alt text. This is to help ensure data is presented in a concise format, and maintain standards and consistency across the service.