From c825741ba0ecba391fa20a44e7f94b99c051dd2b Mon Sep 17 00:00:00 2001
From: "Seth A. Roby" <sroby@uci.edu>
Date: Tue, 23 Apr 2019 10:41:16 -0700
Subject: [PATCH] Skip valid_class check if no class defined

https://github.com/plataformatec/simple_form/issues/1656

Previously, we would check for validity all the time, even if the outcome of the check added no class. This meant that there was no way to opt out of the check in cases where it was expensive (if the file is stored in S3, in our example).

Now, we only check for validity if there is a valid_class defined to be added if the field is valid.
---
 lib/simple_form/wrappers/root.rb  | 11 ++++++++---
 test/form_builder/wrapper_test.rb |  9 +++++++++
 test/support/misc_helpers.rb      |  4 ++--
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/lib/simple_form/wrappers/root.rb b/lib/simple_form/wrappers/root.rb
index f70cb00b4..5486d70d8 100644
--- a/lib/simple_form/wrappers/root.rb
+++ b/lib/simple_form/wrappers/root.rb
@@ -28,11 +28,16 @@ def html_classes(input, options)
         css += SimpleForm.additional_classes_for(:wrapper) do
           input.additional_classes + [input.input_class]
         end
-        css << (options[:wrapper_error_class] || @defaults[:error_class]) if input.has_errors?
-        css << (options[:wrapper_hint_class] || @defaults[:hint_class]) if input.has_hint?
-        css << (options[:wrapper_valid_class] || @defaults[:valid_class]) if input.valid?
+        css << html_class(:error_class, options) { input.has_errors? }
+        css << html_class(:hint_class, options) { input.has_hint? }
+        css << html_class(:valid_class, options) { input.valid? }
         css.compact
       end
+
+      def html_class(key, options)
+        css = (options[:"wrapper_#{key}"] || @defaults[key])
+        css if css && yield
+      end
     end
   end
 end
diff --git a/test/form_builder/wrapper_test.rb b/test/form_builder/wrapper_test.rb
index fd2dc8c47..86e021e08 100644
--- a/test/form_builder/wrapper_test.rb
+++ b/test/form_builder/wrapper_test.rb
@@ -59,6 +59,15 @@ class WrapperTest < ActionView::TestCase
     assert_no_select 'input.is-invalid'
   end
 
+  test 'wrapper does not determine if valid class is needed when it is set to nil' do
+    def @user.name; raise BOOM; end
+    with_form_for @user, :name, as: :file, wrapper: custom_wrapper_with_input_valid_class(valid_class: nil)
+    assert_select 'div'
+    assert_select 'input'
+    assert_no_select 'div.field_with_errors'
+    assert_no_select 'input.is-invalid'
+  end
+
   test 'wrapper adds hint class for attribute with a hint' do
     with_form_for @user, :name, hint: 'hint'
     assert_select 'div.field_with_hint'
diff --git a/test/support/misc_helpers.rb b/test/support/misc_helpers.rb
index 9752504e1..8b629b711 100644
--- a/test/support/misc_helpers.rb
+++ b/test/support/misc_helpers.rb
@@ -213,8 +213,8 @@ def custom_wrapper_with_input_error_class
     end
   end
 
-  def custom_wrapper_with_input_valid_class
-    SimpleForm.build tag: :div, class: "custom_wrapper", valid_class: :field_without_errors do |b|
+  def custom_wrapper_with_input_valid_class(valid_class: :field_without_errors)
+    SimpleForm.build tag: :div, class: "custom_wrapper", valid_class: valid_class do |b|
       b.use :label
       b.use :input, class: 'inline-class', valid_class: 'is-valid'
     end