heartcombo / simple_form

Forms made easy for Rails! It's tied to a simple DSL, with no opinion on markup.
http://blog.plataformatec.com.br/tag/simple_form
MIT License
8.21k stars 1.31k forks source link

alias_attribute error messages #1769

Closed cschilbe closed 2 months ago

cschilbe commented 2 years ago

Environment

Current behavior

When using active model attribute aliases, validation errors from the original field are not shown in forms. A model will not be valid if there are failing validations on the original attribute but details about the error won't be shown to users.

# app/models/example.rb
class Example
  include ActiveModel::Model
  include ActiveModel::Attributes

  attribute :name
  alias_attribute :other_name, :name

  validates :name, presence: true

  ...
end
# app/views/example/new.html.erb
<%= simple_form_for @example,  do |f| %>
  <%= f.other_name %>
  <%= f.button :submit %>
<% end %>

Validation of the example would fail and no error would be generated for the other_name field.

Expected behavior

This is just to open up discussions about what might be more optimal behavior.

Current workaround - Explicitly duplicate validations in the model

Developers can add the same validation for the aliased attribute

# app/model/example.rb
...
  validates :other_name, presence: true

This does provide explicit control over validation for each field and allows for customized error messages easily for each attribute and alias and makes the most sense for simple validations. In cases where there may be many complicated validations performed on the underlying field this becomes unnecessary duplication and a possible problematic area down the road.

# app/model/example.rb
  validate :name_is_actual_name
  validate :name_is_not_test_name
  validate :name_isnt_banned
  ...

In this case, we may not want to duplicate the validate declarations and in this case, this might also mean needing to duplicate the methods used to perform the validation since they are specific to the attribute.

Display validation errors from aliased attribute

When fetching errors for an attribute, we could also check if the attribute is an alias and return any errors from the underlying attribute.

This could be done relatively easily with the following change to https://github.com/heartcombo/simple_form/blob/main/lib/simple_form/components/errors.rb#L54

def errors_on_attribute
  object.errors[attribute_name].presence || errors_on_alias || []
end

def errors_on_alias
  return nil unless object.attribute_aliases.key? attribute_name.to_s
  errors_on_explicit_attribute(object.attribute_aliases[attribute_name.to_s])
end

def errors_on_explicit_attribute(attribute)
  object.errors[attribute]
end

Because this may change the behavior of existing applications, an option could be added such as display_aliased_attribute_errors. This could be a simple_form configuration or could be an option set for the field in the view.

I'd be willing to cut a PR if the community feels this would be beneficial. Any feedback about options or configuration required is welcomed.

nashby commented 2 months ago

Hey @cschilbe! Thanks for the detailed report, but I'm not sure it feels right to show validation errors for aliased attributes. My reasoning is that if Rails doesn't add errors for the other_name attribute, then SimpleForm should not do it either.

You can probably start a discussion about implementing it in Rails and if it's approved it'll automatically work in SimpleForm