x-govuk/govuk-form-builder

Select does not show errors from Rails' belongs_to presence validation

Closed this issue · 6 comments

Rails 6+ adds a presence validation to belongs_to associations by default. The validation is on association name, rather than name_id, but in forms the input is always going to be on name_id rather than name.

This means that the errors on name do not show on the name_id input, and the summary error does not link to the input correctly.

This issue has come up numerous times on stack overflow and other form builders, e.g. bootstrap-ruby/bootstrap_form#318 but no clear consensus on the best solution. Arguably this is Rails' fault.

Is there a recommended solution for govuk-formbuilder? If so, it would be good to documented and add an example to https://govuk-form-builder.netlify.app/form-elements/select/ that uses belongs_to.

Thank you for reporting this @sfnelson. As it's a problem that'd manifest in the same way on a vanilla Rails app I think we might be best just adding a cautionary paragraph to the guide. It feels like suggesting the disabling of the setting and requiring validations to be explicitly added is the way to go.

Thinking about it, this problem will also happen in #govuk_collection_radio_buttons. Maybe the suggestion should go in the setup guide or README.

@peteryates thanks for the reply. That seems like a pragmatic approach at present. It would be good to have some guidance for users in how to resolve this disagreement between ActiveRecord's needs and ActiveView's needs – personally, I would to see the example at https://govuk-form-builder.netlify.app/form-elements/select/#select-field-with-a-label-and-hint to use an ActiveRecord association similar to https://guides.rubyonrails.org/v7.0/form_helpers.html#the-collection-select-helper

It seems like this is the way Rails has implemented belongs_to since at least Rails 4, so it's unlikely they are going to accept a change (e.g. recently proposed and closed rails/rails#46839). Would you consider a PR that uses reflection to identify associations and merges errors from association and association_id? This could be useful for more than just presence validations. Another approach to consider is rails/rails#46564

... As it's a problem that'd manifest in the same way on a vanilla Rails app ...

Vanilla Rails doesn't provide helpers for showing errors next to fields so the problem is not equivalent, e.g. this might be typical for a vanilla rails form:

<%= f.label :department_id %>
<% f.object.errors.messages_for(:department) do |message| %>
  <%= tag.div message class: "error" %>
<% end %>
<%= f.collection_select :department_id, departments, :id, :name %>

As an aside, there's a rubocop to flag and remove presence validations on belongs_to associations: https://docs.rubocop.org/rubocop-rails/cops_rails.html#railsredundantpresencevalidationonbelongsto

Yeah, sorry I meant the underlying problem of the automatic validation being applied to the relationship but the form having to refer to the attribute name.

Would you consider a PR that uses reflection to identify associations and merges errors from association and association_id?

I think it could work but I suspect there's a fair bit of complexity involved. It might make sense for it to be implemented via an error summary presenter as that would allow people to opt in.

I'd still probably go with defining the validations explicitly, I'll create a PR with some amendments to the guide/README in the meantime.

Closing this now as I think the warnings address the problem.