Conditionals mark fields as optional (#1799)

We were assuming that all fields were required when a presence
validation existed. While that makes sense, it's also possible for
validations to be conditional.

Take the following validation as an example:

        validates :phone_number, presence: true, if: :egyptian?

Before this commit, the UI would flag phone_number as required, even for
records who were not egyptian.

We now always flag these fields as optional. This is a bit misleading
too, but it's impossible to know these things when the page is rendered,
and marking them as optional makes for a slightly better user interface,
as the user will most likely be prompted with detailed validation errors
after trying to persist an invalid item, rather than be led to fill some
fields that are, in fact, not mandatory.

lib/administrate/field/base.rb

@@ -52,7 +52,9 @@ def required?
         return false unless resource.class.respond_to?(:validators_on)
 
         resource.class.validators_on(attribute).any? do |v|
-          v.class == ActiveRecord::Validations::PresenceValidator
+          v.class == ActiveRecord::Validations::PresenceValidator &&
+            !v.options.include?(:if) &&
+            !v.options.include?(:unless)
         end
       end
 

spec/example_app/app/models/product.rb

@@ -11,10 +11,10 @@ def self.policy_class
   has_many :pages, dependent: :destroy
   has_one :product_meta_tag, dependent: :destroy
 
-  validates :description, presence: true
+  validates :description, presence: true, unless: -> { false }
   validates :image_url, presence: true
   validates :name, presence: true
-  validates :price, presence: true
+  validates :price, presence: true, if: -> { true }
   validates :release_year,
             numericality: {
               less_than_or_equal_to: ->(_product) { Time.current.year },

spec/features/form_spec.rb

@@ -29,6 +29,21 @@
     end
   end
 
+  it "marks required fields only" do
+    visit new_admin_product_path
+
+    required_field_translations = [
+      Product.human_attribute_name(:name),
+      Product.human_attribute_name(:image_url),
+    ]
+
+    required_field_labels = find_all('.field-unit--required').map do |field|
+      field.text
+    end
+
+    expect(required_field_labels).to match_array(required_field_translations)
+  end
+
   context "include_blank option for belongs_to" do
     before { create_list(:country, 5) }
 

spec/helpers/administrate/application_helper_spec.rb

@@ -80,17 +80,27 @@
 
   describe "#requireness" do
     let(:page) do
-      Administrate::Page::Form.new(Blog::PostDashboard.new, Blog::Post.new)
+      Administrate::Page::Form.new(ProductDashboard.new, Product.new)
     end
 
     it "returns 'required' if field is required" do
-      title = page.attributes.detect { |i| i.attribute == :title }
-      expect(requireness(title)).to eq("required")
+      name = page.attributes.detect { |i| i.attribute == :name }
+      expect(requireness(name)).to eq("required")
     end
 
     it "returns 'optional' if field is not required" do
-      publish_at = page.attributes.detect { |i| i.attribute == :published_at }
-      expect(requireness(publish_at)).to eq("optional")
+      release_year = page.attributes.detect { |i| i.attribute == :release_year }
+      expect(requireness(release_year)).to eq("optional")
+    end
+
+    it "returns 'optional' if field is required if condition is met" do
+      description = page.attributes.detect { |i| i.attribute == :description }
+      expect(requireness(description)).to eq("optional")
+    end
+
+    it "returns 'optional' if field is required unless condition is met" do
+      price = page.attributes.detect { |i| i.attribute == :price }
+      expect(requireness(price)).to eq("optional")
     end
   end