Allow custom error on forms without model #1585

victorperez · 2018-05-24T18:50:42Z

This PR change the has_errors? method to allow custom errors on forms without a model:

Before this PR

= simple_form_for :user, url: users_path do |f|
  = f.input :email, error: 'custom error'

Output

...
<div class="form-group email optional user_email">
  <label class="control-label email optional" for="user_email">Email</label>
  <input class="form-control string email optional" type="email" name="user[email]" id="user_email">
</div>
...

After this PR

= simple_form_for :user, url: users_path do |f|
  = f.input :email, error: 'custom error'

Output

<div class="form-group email optional user_email has-error">
  <label class="control-label email optional" for="user_email">Email</label>
  <input class="form-control string email optional" aria-invalid="true" type="email" name="user[email]" id="user_email">
  <span class="help-block">custom error</span>
</div>

feliperenan · 2018-05-27T17:21:05Z

Hi @victorperez, thanks for the pull request :)

Using the custom error option on a non-model the form ever will be displayed with errors since we don't have validations inside this kind of object, so I'm not sure if it makes sense with a non-model.

Could you explain what use case motivated you to send this PR?

victorperez · 2018-05-28T02:06:20Z

Hi @feliperenan, thanks for the feedback.

Consider the case where you have two applications in your stack, the main application has a view that use simple_form without a model and when the form is submitted it will send a request through an API to the second application where the model/validations exists, the second application will create the record or return the errors in case the form is invalid.

If errors are returned from the second app then will be easy to create the errors in the form if we allow custom errors for forms without a model. Of course the idea is to pass a dynamic variable based in the API response to the input (not a hardcoded string that always display the error in the form) ej:

= simple_form_for :user, url: users_path do |f|
  ...
  = f.input :first_name, error: @errors[:first_name]
  ...

If @errors[:first_name] is nil no error will be displayed and if @errors[:first_name] has a value from the api response then the error will be displayed.

It can be fixed creating a model in the main application to handle the form or using activeresource (https://github.com/rails/activeresource) which is what I finally did in my project, but I consider to open a PR because maybe it could be useful.

I understand that this is not a common scenario or maybe there is a better approach to handle what I'm exposing so feel free to take the best decision :)

feliperenan · 2018-05-28T13:28:34Z

Thanks for the explanation, now it makes sense to me 👍.

feliperenan · 2018-05-28T13:29:50Z

lib/simple_form/components/errors.rb

@@ -34,6 +34,14 @@ def full_error_text
        has_custom_error? ? options[:error] : full_errors.send(error_method)
      end

+      def form_with_object_has_errors?


We could name this method of model_with_errors?, what do you think?

Yes I like more that name, but what you think to call it object_with_errors? (just to respect the same variable name that we use in the method)

Done, I have amended my commit and renamed the method.

feliperenan · 2018-05-28T13:41:35Z

lib/simple_form/components/errors.rb

@@ -11,7 +11,7 @@ def full_error(wrapper_options = nil)
      end

      def has_errors?
-        object && object.respond_to?(:errors) && errors.present?
+        form_with_object_has_errors? || form_without_object_has_errors?


Why do we need to check if the object is nil at the second condition?, just check has_custom_errors? would be enough, is it?

I have to check if the object is nil to respect other logic based in forms using an object and if there is no error on the attribute. If the object.nil? is removed from the second condition then we have two test failing.

Test 1: https://github.com/victorperez/simple_form/blob/master/test/form_builder/error_test.rb#L221
Test 2: https://github.com/victorperez/simple_form/blob/master/test/form_builder/error_test.rb#L274

So we could rename this method or just use the conditions here, something like this:

object_with_errors? || object.nil? && has_custom_error?

Done, I have amended my commit and changed the conditions to what you propose 👍

This PR change the has_errors? method to allow custom errors on forms without a model

victorperez force-pushed the master branch 2 times, most recently from e3fa03c to b1be461 Compare May 24, 2018 18:56

feliperenan added the needs review label May 24, 2018

victorperez force-pushed the master branch from b1be461 to 81aa4d0 Compare May 26, 2018 15:06

feliperenan reviewed May 28, 2018

View reviewed changes

allow custom error when form does not use a model

09c928f

This PR change the has_errors? method to allow custom errors on forms without a model

victorperez force-pushed the master branch from 81aa4d0 to 09c928f Compare May 28, 2018 14:54

feliperenan approved these changes May 28, 2018

View reviewed changes

rafaelfranca approved these changes Jun 9, 2018

View reviewed changes

feliperenan merged commit 501229c into heartcombo:master Jun 9, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow custom error on forms without model #1585

Allow custom error on forms without model #1585

victorperez commented May 24, 2018 •

edited

Loading

feliperenan commented May 27, 2018

victorperez commented May 28, 2018 •

edited

Loading

feliperenan commented May 28, 2018

feliperenan May 28, 2018

victorperez May 28, 2018

victorperez May 28, 2018

feliperenan May 28, 2018

victorperez May 28, 2018 •

edited

Loading

feliperenan May 28, 2018

victorperez May 28, 2018 •

edited

Loading

Allow custom error on forms without model #1585

Allow custom error on forms without model #1585

Conversation

victorperez commented May 24, 2018 • edited Loading

Before this PR

Output

After this PR

Output

feliperenan commented May 27, 2018

victorperez commented May 28, 2018 • edited Loading

feliperenan commented May 28, 2018

feliperenan May 28, 2018

Choose a reason for hiding this comment

victorperez May 28, 2018

Choose a reason for hiding this comment

victorperez May 28, 2018

Choose a reason for hiding this comment

feliperenan May 28, 2018

Choose a reason for hiding this comment

victorperez May 28, 2018 • edited Loading

Choose a reason for hiding this comment

feliperenan May 28, 2018

Choose a reason for hiding this comment

victorperez May 28, 2018 • edited Loading

Choose a reason for hiding this comment

victorperez commented May 24, 2018 •

edited

Loading

victorperez commented May 28, 2018 •

edited

Loading

victorperez May 28, 2018 •

edited

Loading

victorperez May 28, 2018 •

edited

Loading