Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Select does not show errors from Rails' belongs_to presence validation #411

Open
sfnelson opened this issue Jan 29, 2023 · 5 comments
Open

Comments

@sfnelson
Copy link

sfnelson commented Jan 29, 2023

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.

@peteryates
Copy link
Member

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.

@peteryates
Copy link
Member

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

@sfnelson
Copy link
Author

@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

@sfnelson
Copy link
Author

... 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

@peteryates
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants