Allow for override of field_error_proc in the FormBuilder

[thebravoman]

Create a form with a custom builder. Generate input fields with additional labels and classes in the form builder. When an active record validation error occurs the ActionView::Base.field_error_proc is called that wraps the field with a span and a class.

But we should be able to override field_error_proc in the FormBuilder since this is where we are building the forms. In my case the field_error_proc was even defined by another engine, so it made it more difficult to debug why an additional span was added. The additional span was breaking the functionality because of the way the toggle checkboxes were generated.

Steps to reproduce

The builder in my case has the following method.

def collection_check_boxes method, collection, value_method, text_method, options = {}, html_options = {}
  super(method, collection, value_method, text_method, options, html_options) do |b|
    @template.content_tag :div, class: "form-group" do 
      b.label(class: "d-flex align-items-center justify-content-between", for: nil) do
        @template.content_tag(:span, b.text) +
        @template.content_tag(:div, class: "u-check") do 
          b.check_box(class: "g-hidden-xs-up g-pos-abs g-top-0 g-right-0")+
          @template.content_tag(:div, class: "u-check-icon-radio-v8") do 
            @template.content_tag(:i, "", class: "fa", "data-check-icon": "")
          end
        end
      end 
    end
  end
end

Actual behavior

It generates

<div class="form-group">
  <label class="d-flex align-items-center justify-content-between">
    <span>organizations_manage</span>
    <div class="u-check">
      <input class="g-hidden-xs-up g-pos-abs g-top-0 g-right-0" type="checkbox" value="organizations_manage" checked="checked" name="organizations_user[roles_name][]" id="organizations_user_roles_name_organizations_manage" />
      <div class="u-check-icon-radio-v8">
        <i class="fa" data-check-icon=""></i>
      </div>
    </div>
  </label>
</div>

This works very well until there is an error in the form. Then the generated html is

<div class="form-group">
  <label class="d-flex align-items-center justify-content-between">
    <span>organizations_manage</span>
    <div class="u-check">
      <span class="fieldWithErrors">
        <input class="g-hidden-xs-up g-pos-abs g-top-0 g-right-0" type="checkbox" value="organizations_manage" checked="checked" name="organizations_user[roles_name][]" id="organizations_user_roles_name_organizations_manage" />
      </span>
      <div class="u-check-icon-radio-v8">
        <i class="fa" data-check-icon=""></i>
      </div>
    </div>
  </label>
</div>

Notice the additional <span class="fieldWithErrors">. Because of this span the checkbox is no longer working.

If there is an error we are calling Base.field_error_proc

actionview-6.0.3.1/lib/action_view/helpers/active_model_helper.rb
   25:         tag_generate_errors?(options) ? error_wrapping(super) : super
   26:       end
   27: 
   28:       def error_wrapping(html_tag)
   29:         if object_has_errors?
=> 30:           Base.field_error_proc.call(html_tag, self)
   31:         else
   32:           html_tag
   33:         end
   34:       end

and in my case this field_error_proc was defined in another engine used by the host app- refinery

refinerycms-d1a8db3061d4/core/lib/refinery/core/engine.rb
   34:       config.to_prepare(&method(:refinery_inclusion!).to_proc)
   35: 
   36:       # Wrap errors in spans
   37:       config.to_prepare do
   38:         ActionView::Base.field_error_proc = Proc.new do |html_tag, instance|
=> 39:           ActionController::Base.helpers.content_tag(:span, html_tag, class: "fieldWithErrors")
   40:         end
   41:       end
   42: 

Expected behavior

I am sure there are ways to hack around this in the collection_check_boxes and generate the field in a different way, but I think active model helper should first call a method of the FormBuilder and probably after that delegate to Base.field_error_proc. This would allow us to override the error classes for a specific form. Because if I now just disable this proc I would break the functionality provided by a different engine.

System configuration

Rails version: 6.0.3.1

Ruby version: 2.6.5

[pixeltrix]

@thebravoman this seems like a good idea and would be a way for engines like Refinery not to stomp on an application's settings as they could create a custom form builder for its UI and just override it there. Is this something you want work on?

[thebravoman]

@pixeltrix yes, I could look at it and prepare a PR.

[jacobdaddario]

Hey there! I just wanted to follow up on this idea. How can I be of help here?

[thebravoman]

Thanks @jacobbednarz . We talked in the forum. I will try to look at this by the end of August.

[jacobdaddario]

Okay! If there’s anyway I could help, please let me know.

[rails-bot]

This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails team are limited, and so we are asking for your help.
If you can still reproduce this error on the 6-0-stable branch or on master, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.

[thebravoman]

Hey, @jacobdaddario @pixeltrix

I wanted to share that I am thinking on an approach to have the logic in the FormBuilder. I have proposed an implementation.

As per Rails Contribution guides I would like to first get feedback before opening a new PR. Let me know your thoughts in https://discuss.rubyonrails.org/t/feature-allow-for-override-of-field-error-proc-in-the-formbuilder-could-prepare-pr-if-the-approach-is-reasonable/79843

[jacobdaddario]

I'm so happy that you came back to this issue. Trying to extract field_error_proc has been something I've brainstormed for on and off for the last two years, and I'd love to see it happen. I'd love to go review what you have over on the Rails forums.