govuk_date_field caused non-numerical values to automatically be cast to 0

[patrick-laa]

In general, if a user enters invalid data in a form field, on submission we want to re-render the form with what they entered pre-populated as entered, and an error message saying what is wrong with it.

Because govuk_date_field appends an i to the identifier of each field (c.f. https://github.com/x-govuk/govuk-form-builder/blob/main/lib/govuk_design_system_formbuilder/elements/date.rb#L11), the name of the field comes out as something like person[date_of_trade(3i)], and that i tells Rails to cast the provided value as an integer before it is assigned to an attribute on a model (c.f. https://github.com/rails/rails/blob/d90b4e2/activerecord/lib/active_record/base.rb#L1884) , meaning that by the time validations run, "four" has been cast to 0, and there's no way for a standard validator to know that what was originally entered was a string, so the best we can do is show a validation saying "The value must be greater than zero" instead of "The value must be a number".

If you removed that i, then it would be up to the attribute type to cast to integer or not, and that would allow non-valid strings to be left as strings, allowing for a more intuitive UX when showing an error message.

[peteryates]

Hey @patrick-laa, thanks for reporting this.

I just had a play and wasn't able to get it to work without the i.

1 error(s) on assignment of multiparameter attributes [error on assignment ["2", "March", "2023"] to joined_on (invalid value for Integer(): "March")]

I removed the i with the following change:

diff --git a/lib/govuk_design_system_formbuilder/elements/date.rb b/lib/govuk_design_system_formbuilder/elements/date.rb
index b7f574b..84b0a2c 100644
--- a/lib/govuk_design_system_formbuilder/elements/date.rb
+++ b/lib/govuk_design_system_formbuilder/elements/date.rb
@@ -8,7 +8,7 @@ module GOVUKDesignSystemFormBuilder
       include Traits::Supplemental
       include Traits::HTMLClasses

-      SEGMENTS = { day: '3i', month: '2i', year: '1i' }.freeze
+      SEGMENTS = { day: '3', month: '2', year: '1' }.freeze
       MULTIPARAMETER_KEY = { day: 3, month: 2, year: 1 }.freeze

       def initialize(builder, object_name, attribute_name, legend:, caption:, hint:, omit_day:, maxlength_enabled:, form_group:, date_of_birth: false, **kwargs, &block)

I haven't delved right into how Rails deals with multiparameter attributes behind the scenes but based on this comment it looks like it's expecting either a integer or float only.

[patrick-laa]

@peteryates so the issue you have in your error is that the month is being represented as the string "March" rather than the string "3". Without seeing your attributes I can't comment on what might be going on there beyond that. The comment in the Rails codebase you highlighted says "You can also specify a typecast character" (my italics) - you can see here in the ternary operator that if one isn't provided, the input string is left as a string.

In our codebases we've built a couple of custom multiparam fields that don't use i or f and have them working satisfactorily. I also have a PR open that subclasses your Date class to remove the i to get the behaviour that we want, and it works fine, although please note that we're using attributes on our models with custom types, and in particular the MultiparamDate type we use has some extra logic so that, once it's made it through Rails's multiparam assignment, we don't force invalid values into a Date object, so that we can preserve and re-render invalid values.

[peteryates]

Ah, that makes sense. I didn't realise you'd created and integrated a custom type too! I'll have a closer look.

Really appreciate this by the way, wasn't sure of the best approach for tackling this problem and this looks most promising.

[peteryates]

I've released the change that makes teh segments configurable last week and have just pushed a follow up that makes those defaults overridable.