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

Custom error is not rendered when object has no error for the attribute #1748

Closed romaricpascal closed 2 years ago

romaricpascal commented 2 years ago

Precheck

Environment

Current behavior

When rendering an input for a form that's been provided an object, a custom error passed through the error option will only show if the model already has an error for the attribute:

Provided that @model has no errors:

=  simple_form_for @model do |f|
  = f.input(:attribute_name, error: "This won't show")

But if there's already an error for the attribute, the custom message will correctly override the error message from ActiveModel

- @model.errors.add(:attribute_name, :whoops)
=  simple_form_for @model do |f|
  = f.input(:attribute_name, error: "This will show")

Expected behavior

This behaviour is troublesome when using another way of validating data that doesn't fill the object's error (dry-validation, for example). It'd be great if any error provided in the f.input call would show regardless of the state of the object passed to the FormBuilder.

I believe the current behaviour is due to line 14 of the Errors module that checks for object.nil? before trying has_custom_error?. Could this check be lifted so that having a custom error does render the error anyway?

nickcoyne commented 2 years ago

This is causing problems for me too. @romaricpascal have you come up with any sensible workaround?

romaricpascal commented 2 years ago

@nickcoyne, I've managed to work my way around with a custom FormBuilder that includes the following concern. It creates an fake error on the field before rendering if there was none already and clears it up after render. It does the job nicely so far 😁

module SimpleFormHelper::WithExplicitError
  def input(attribute_name, *args, error: nil, **attributes)
    return super unless error.present? && !object.errors.include?(attribute_name)

    object.errors.add(attribute_name, error)

    begin
      super(attribute_name, *args, error: error, **attributes)
    ensure
      object.errors.delete(attribute_name)
    end
  end
end
nickcoyne commented 2 years ago

That's neat, thanks @romaricpascal!

nashby commented 2 years ago

Hey! We can't really change this at this point since it'll break existing applications. Having it as an extension that @romaricpascal provided is good enough for me.