From a93eeb1e3774c0e24374eddb722d4e471d941a7f Mon Sep 17 00:00:00 2001 From: Akira Matsuda Date: Sun, 26 Dec 2010 05:36:47 +0900 Subject: [PATCH 1/8] Append HTML5 required="required" on input fields reflecting PresenceValidator on the model's attribute However, the required="required" attribute will not be added if the PresenceValidator has :if or :unless option, since these options could not be evaluated on client side. --- actionpack/lib/action_view/helpers/form_helper.rb | 3 ++ actionpack/test/lib/controller/fake_models.rb | 1 + actionpack/test/template/form_helper_test.rb | 21 ++++++++++++++++++++ .../lib/active_model/validations/presence.rb | 6 +++++ .../cases/validations/presence_validation_test.rb | 14 +++++++++++++ 5 files changed, 45 insertions(+), 0 deletions(-) diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 6f0e2c9..0efc016 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -920,6 +920,7 @@ module ActionView options["type"] ||= field_type options["value"] = options.fetch("value"){ value_before_type_cast(object) } unless field_type == "file" options["value"] &&= ERB::Util.html_escape(options["value"]) + options["required"] ||= object.class.ancestors.include?(ActiveModel::Validations) && object.class.attribute_required?(method_name) add_default_name_and_id(options) tag("input", options) end @@ -943,6 +944,7 @@ module ActionView checked = self.class.radio_button_checked?(value(object), tag_value) end options["checked"] = "checked" if checked + options["required"] ||= object.class.ancestors.include?(ActiveModel::Validations) && object.class.attribute_required?(method_name) add_default_name_and_id_for_value(tag_value, options) tag("input", options) end @@ -954,6 +956,7 @@ module ActionView if size = options.delete("size") options["cols"], options["rows"] = size.split("x") if size.respond_to?(:split) end + options["required"] ||= object.class.ancestors.include?(ActiveModel::Validations) && object.class.attribute_required?(method_name) content_tag("textarea", ERB::Util.html_escape(options.delete('value') || value_before_type_cast(object)), options) end diff --git a/actionpack/test/lib/controller/fake_models.rb b/actionpack/test/lib/controller/fake_models.rb index 67baf36..3ebc39f 100644 --- a/actionpack/test/lib/controller/fake_models.rb +++ b/actionpack/test/lib/controller/fake_models.rb @@ -52,6 +52,7 @@ class Post < Struct.new(:title, :author_name, :body, :secret, :written_on, :cost extend ActiveModel::Naming include ActiveModel::Conversion extend ActiveModel::Translation + include ActiveModel::Validations alias_method :secret?, :secret diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb index 2c60096..e8d4351 100644 --- a/actionpack/test/template/form_helper_test.rb +++ b/actionpack/test/template/form_helper_test.rb @@ -266,6 +266,13 @@ class FormHelperTest < ActionView::TestCase text_field("user", "email", :type => "email") end + def test_text_field_with_presence_validator + Post.stubs(:attribute_required?).with("title").returns(true) + assert_dom_equal( + '', text_field("post", "title") + ) + end + def test_check_box assert_dom_equal( '', @@ -369,6 +376,13 @@ class FormHelperTest < ActionView::TestCase ) end + def test_radio_button_with_presence_validator + Post.stubs(:attribute_required?).with("secret").returns(true) + assert_dom_equal( + '', radio_button("post", "secret", "1") + ) + end + def test_text_area assert_dom_equal( '', @@ -406,6 +420,13 @@ class FormHelperTest < ActionView::TestCase ) end + def test_text_area_with_presence_validator + Post.stubs(:attribute_required?).with("body").returns(true) + assert_dom_equal( + '', text_area("post", "body") + ) + end + def test_search_field expected = %{} assert_dom_equal(expected, search_field("contact", "notes_query")) diff --git a/activemodel/lib/active_model/validations/presence.rb b/activemodel/lib/active_model/validations/presence.rb index 28c4640..8d79882 100644 --- a/activemodel/lib/active_model/validations/presence.rb +++ b/activemodel/lib/active_model/validations/presence.rb @@ -38,6 +38,12 @@ module ActiveModel def validates_presence_of(*attr_names) validates_with PresenceValidator, _merge_attributes(attr_names) end + + def attribute_required?(attribute) + self.validators.grep(PresenceValidator).any? do |v| + v.attributes.include?(attribute.to_sym) && (v.options.keys & [:if, :unless]).empty? + end + end end end end diff --git a/activemodel/test/cases/validations/presence_validation_test.rb b/activemodel/test/cases/validations/presence_validation_test.rb index 510c13a..61b00ad 100644 --- a/activemodel/test/cases/validations/presence_validation_test.rb +++ b/activemodel/test/cases/validations/presence_validation_test.rb @@ -9,6 +9,7 @@ class PresenceValidationTest < ActiveModel::TestCase teardown do Topic.reset_callbacks(:validate) + Topic._validators = Hash.new { |h, k| h[k] = [] } Person.reset_callbacks(:validate) CustomReader.reset_callbacks(:validate) end @@ -70,4 +71,17 @@ class PresenceValidationTest < ActiveModel::TestCase p[:karma] = "Cold" assert p.valid? end + + def test_attribute_required + assert !Topic.attribute_required?(:title) + Topic.validates_presence_of :title + assert Topic.attribute_required?(:title) + end + + def test_attribute_required_with_if_or_unless + Topic.validates_presence_of :title, :if => lambda {|u| true} + assert !Topic.attribute_required?(:title) + Topic.validates_presence_of :title, :unless => lambda {|u| false} + assert !Topic.attribute_required?(:title) + end end -- 1.7.3.4 From c0aefd0b3a88afe1c433335379115739ddc061b4 Mon Sep 17 00:00:00 2001 From: Akira Matsuda Date: Mon, 27 Dec 2010 02:16:11 +0900 Subject: [PATCH 2/8] Don't add DEFAULT_TOKENIZER to every instance of LengthValidator. Just use it only when needed. --- activemodel/lib/active_model/validations/length.rb | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/activemodel/lib/active_model/validations/length.rb b/activemodel/lib/active_model/validations/length.rb index 5a46ecb..c51fe12 100644 --- a/activemodel/lib/active_model/validations/length.rb +++ b/activemodel/lib/active_model/validations/length.rb @@ -16,7 +16,7 @@ module ActiveModel options[:maximum] -= 1 if range.exclude_end? end - super(options.reverse_merge(:tokenizer => DEFAULT_TOKENIZER)) + super end def check_validity! @@ -36,7 +36,7 @@ module ActiveModel end def validate_each(record, attribute, value) - value = options[:tokenizer].call(value) if value.kind_of?(String) + value = (options[:tokenizer] || DEFAULT_TOKENIZER).call(value) if value.kind_of?(String) CHECKS.each do |key, validity_check| next unless check_value = options[key] -- 1.7.3.4 From 48a324b0a677dfad0367551e00ca69d16f230c9d Mon Sep 17 00:00:00 2001 From: Akira Matsuda Date: Mon, 27 Dec 2010 03:13:29 +0900 Subject: [PATCH 3/8] Reset _validators so that each test case does not affect other cases --- .../cases/validations/length_validation_test.rb | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/activemodel/test/cases/validations/length_validation_test.rb b/activemodel/test/cases/validations/length_validation_test.rb index 1e6180a..488a073 100644 --- a/activemodel/test/cases/validations/length_validation_test.rb +++ b/activemodel/test/cases/validations/length_validation_test.rb @@ -8,6 +8,7 @@ class LengthValidationTest < ActiveModel::TestCase def teardown Topic.reset_callbacks(:validate) + Topic._validators = Hash.new { |h, k| h[k] = [] } end def test_validates_length_of_with_allow_nil -- 1.7.3.4 From 6435a4f61ad61ccbfab705d7ae714b3d0eefc1eb Mon Sep 17 00:00:00 2001 From: Akira Matsuda Date: Mon, 27 Dec 2010 03:48:52 +0900 Subject: [PATCH 4/8] Append maxlength="..." on input fields reflecting LengthValidator on the model's attribute --- actionpack/lib/action_view/helpers/form_helper.rb | 1 + actionpack/test/template/form_helper_test.rb | 7 +++++ activemodel/lib/active_model/validations/length.rb | 6 ++++ .../cases/validations/length_validation_test.rb | 28 ++++++++++++++++++++ 4 files changed, 42 insertions(+), 0 deletions(-) diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 0efc016..cc243ac 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -912,6 +912,7 @@ module ActionView def to_input_field_tag(field_type, options = {}) options = options.stringify_keys + options["maxlength"] ||= object.class.attribute_maxlength(method_name) if object.class.ancestors.include?(ActiveModel::Validations) options["size"] = options["maxlength"] || DEFAULT_FIELD_OPTIONS["size"] unless options.key?("size") options = DEFAULT_FIELD_OPTIONS.merge(options) if field_type == "hidden" diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb index e8d4351..0323ab0 100644 --- a/actionpack/test/template/form_helper_test.rb +++ b/actionpack/test/template/form_helper_test.rb @@ -273,6 +273,13 @@ class FormHelperTest < ActionView::TestCase ) end + def test_text_field_with_length_validator_maxlength + Post.stubs(:attribute_maxlength).with("title").returns(12) + assert_dom_equal( + '', text_field("post", "title") + ) + end + def test_check_box assert_dom_equal( '', diff --git a/activemodel/lib/active_model/validations/length.rb b/activemodel/lib/active_model/validations/length.rb index c51fe12..9e8abb3 100644 --- a/activemodel/lib/active_model/validations/length.rb +++ b/activemodel/lib/active_model/validations/length.rb @@ -98,6 +98,12 @@ module ActiveModel end alias_method :validates_size_of, :validates_length_of + + def attribute_maxlength(attribute) + self.validators.grep(LengthValidator).select {|v| + v.attributes.include?(attribute.to_sym) && (v.options.keys & [:maximum, :is]).any? && (v.options.keys & [:if, :unless, :allow_nil, :allow_blank, :tokenizer]).empty? + }.map {|v| v.options.slice(:maximum, :is)}.map(&:values).flatten.max + end end end end diff --git a/activemodel/test/cases/validations/length_validation_test.rb b/activemodel/test/cases/validations/length_validation_test.rb index 488a073..59f3b3a 100644 --- a/activemodel/test/cases/validations/length_validation_test.rb +++ b/activemodel/test/cases/validations/length_validation_test.rb @@ -357,4 +357,32 @@ class LengthValidationTest < ActiveModel::TestCase ensure Person.reset_callbacks(:validate) end + + def test_attribute_maxlength_using_maximum + Topic.validates_length_of :title, :maximum => 30 + assert_equal 30, Topic.attribute_maxlength(:title) + end + + def test_attribute_maxlength_using_is + Topic.validates_length_of :title, :is => 40 + assert_equal 40, Topic.attribute_maxlength(:title) + end + + def test_attribute_maxlength_using_in + Topic.validates_length_of :title, :in => (13..26) + assert_equal 26, Topic.attribute_maxlength(:title) + end + + def test_attribute_maxlength_with_if_or_unless_or_allow_nil_or_allow_blank_or_tokenizer + Topic.validates_length_of :title, :is => 20, :if => lambda {|u| true} + assert_nil Topic.attribute_maxlength(:title) + Topic.validates_length_of :title, :is => 20, :unless => lambda {|u| false} + assert_nil Topic.attribute_maxlength(:title) + Topic.validates_length_of :title, :is => 20, :allow_nil => true + assert_nil Topic.attribute_maxlength(:title) + Topic.validates_length_of :title, :is => 20, :allow_nil => true + assert_nil Topic.attribute_maxlength(:title) + Topic.validates_length_of :title, :is => 20, :tokenizer => lambda {|s| s.scan(/\w+/)} + assert_nil Topic.attribute_maxlength(:title) + end end -- 1.7.3.4 From 9091cdefc01137d4a964bfbe56cd5042bc609e4b Mon Sep 17 00:00:00 2001 From: Akira Matsuda Date: Mon, 27 Dec 2010 04:57:20 +0900 Subject: [PATCH 5/8] Remove unneeded merge with :allow_nil => false --- .../lib/active_model/validations/numericality.rb | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/activemodel/lib/active_model/validations/numericality.rb b/activemodel/lib/active_model/validations/numericality.rb index 95fe20d..0a9479f 100644 --- a/activemodel/lib/active_model/validations/numericality.rb +++ b/activemodel/lib/active_model/validations/numericality.rb @@ -10,7 +10,7 @@ module ActiveModel RESERVED_OPTIONS = CHECKS.keys + [:only_integer] def initialize(options) - super(options.reverse_merge(:only_integer => false, :allow_nil => false)) + super(options.reverse_merge(:only_integer => false)) end def check_validity! -- 1.7.3.4 From 882fc5e869cdfeb3b09dca263c57bfaaf35e4def Mon Sep 17 00:00:00 2001 From: Akira Matsuda Date: Mon, 27 Dec 2010 05:24:23 +0900 Subject: [PATCH 6/8] Reset _validators so that each test case does not affect other cases --- .../validations/numericality_validation_test.rb | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/activemodel/test/cases/validations/numericality_validation_test.rb b/activemodel/test/cases/validations/numericality_validation_test.rb index e1d7d40..4897aed 100644 --- a/activemodel/test/cases/validations/numericality_validation_test.rb +++ b/activemodel/test/cases/validations/numericality_validation_test.rb @@ -10,6 +10,7 @@ class NumericalityValidationTest < ActiveModel::TestCase def teardown Topic.reset_callbacks(:validate) + Topic._validators = Hash.new { |h, k| h[k] = [] } end NIL = [nil] -- 1.7.3.4 From 2cb29380c7106e9551c69fc813b57f26f84cd2a2 Mon Sep 17 00:00:00 2001 From: Akira Matsuda Date: Mon, 27 Dec 2010 05:08:30 +0900 Subject: [PATCH 7/8] Append HTML5 max="..." on input fields reflecting NumericalityValidator on the model's attribute --- actionpack/lib/action_view/helpers/form_helper.rb | 1 + actionpack/test/template/form_helper_test.rb | 7 ++++++ .../lib/active_model/validations/numericality.rb | 6 +++++ .../validations/numericality_validation_test.rb | 21 ++++++++++++++++++++ 4 files changed, 35 insertions(+), 0 deletions(-) diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index cc243ac..7443bcb 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -922,6 +922,7 @@ module ActionView options["value"] = options.fetch("value"){ value_before_type_cast(object) } unless field_type == "file" options["value"] &&= ERB::Util.html_escape(options["value"]) options["required"] ||= object.class.ancestors.include?(ActiveModel::Validations) && object.class.attribute_required?(method_name) + options["max"] ||= object.class.attribute_max(method_name) if object.class.ancestors.include?(ActiveModel::Validations) add_default_name_and_id(options) tag("input", options) end diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb index 0323ab0..cb3c252 100644 --- a/actionpack/test/template/form_helper_test.rb +++ b/actionpack/test/template/form_helper_test.rb @@ -280,6 +280,13 @@ class FormHelperTest < ActionView::TestCase ) end + def test_text_field_with_numericality_validator_max + Post.stubs(:attribute_max).with("cost").returns(1000) + assert_dom_equal( + '', text_field("post", "cost") + ) + end + def test_check_box assert_dom_equal( '', diff --git a/activemodel/lib/active_model/validations/numericality.rb b/activemodel/lib/active_model/validations/numericality.rb index 0a9479f..3806eff 100644 --- a/activemodel/lib/active_model/validations/numericality.rb +++ b/activemodel/lib/active_model/validations/numericality.rb @@ -125,6 +125,12 @@ module ActiveModel def validates_numericality_of(*attr_names) validates_with NumericalityValidator, _merge_attributes(attr_names) end + + def attribute_max(attribute) + self.validators.grep(NumericalityValidator).select {|v| + v.attributes.include?(attribute.to_sym) && (v.options.keys & [:less_than, :less_than_or_equal_to]).any? && (v.options.keys & [:if, :unless, :allow_nil, :allow_blank]).empty? + }.map {|v| v.options.slice(:less_than, :less_than_or_equal_to)}.map(&:values).flatten.max + end end end end diff --git a/activemodel/test/cases/validations/numericality_validation_test.rb b/activemodel/test/cases/validations/numericality_validation_test.rb index 4897aed..4f64c95 100644 --- a/activemodel/test/cases/validations/numericality_validation_test.rb +++ b/activemodel/test/cases/validations/numericality_validation_test.rb @@ -162,6 +162,27 @@ class NumericalityValidationTest < ActiveModel::TestCase assert_raise(ArgumentError){ Topic.validates_numericality_of :approved, :equal_to => "foo" } end + def test_attribute_max_using_less_than + Topic.validates_numericality_of :approved, :less_than => 20 + assert_equal 20, Topic.attribute_max(:approved) + end + + def test_attribute_max_using_less_than_or_equal_to + Topic.validates_numericality_of :approved, :less_than_or_equal_to => 40 + assert_equal 40, Topic.attribute_max(:approved) + end + + def test_attribute_max_with_if_or_unless_or_allow_nil_or_allow_blank + Topic.validates_numericality_of :approved, :less_than => 20, :if => lambda {|u| true} + assert_nil Topic.attribute_max(:approved) + Topic.validates_numericality_of :approved, :less_than => 20, :unless => lambda {|u| false} + assert_nil Topic.attribute_max(:approved) + Topic.validates_numericality_of :approved, :less_than => 20, :allow_nil => true + assert_nil Topic.attribute_max(:approved) + Topic.validates_numericality_of :approved, :less_than => 20, :allow_nil => true + assert_nil Topic.attribute_max(:approved) + end + private def invalid!(values, error = nil) -- 1.7.3.4 From e4914a737e18568b2e70ad218e2f2591e73752cc Mon Sep 17 00:00:00 2001 From: Akira Matsuda Date: Mon, 27 Dec 2010 05:26:34 +0900 Subject: [PATCH 8/8] Append HTML5 min="..." on input fields reflecting NumericalityValidator on the model's attribute --- actionpack/lib/action_view/helpers/form_helper.rb | 1 + actionpack/test/template/form_helper_test.rb | 7 ++++++ .../lib/active_model/validations/numericality.rb | 6 +++++ .../validations/numericality_validation_test.rb | 21 ++++++++++++++++++++ 4 files changed, 35 insertions(+), 0 deletions(-) diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 7443bcb..17b979f 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -923,6 +923,7 @@ module ActionView options["value"] &&= ERB::Util.html_escape(options["value"]) options["required"] ||= object.class.ancestors.include?(ActiveModel::Validations) && object.class.attribute_required?(method_name) options["max"] ||= object.class.attribute_max(method_name) if object.class.ancestors.include?(ActiveModel::Validations) + options["min"] ||= object.class.attribute_min(method_name) if object.class.ancestors.include?(ActiveModel::Validations) add_default_name_and_id(options) tag("input", options) end diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb index cb3c252..886eac6 100644 --- a/actionpack/test/template/form_helper_test.rb +++ b/actionpack/test/template/form_helper_test.rb @@ -287,6 +287,13 @@ class FormHelperTest < ActionView::TestCase ) end + def test_text_field_with_numericality_validator_max + Post.stubs(:attribute_min).with("cost").returns(500) + assert_dom_equal( + '', text_field("post", "cost") + ) + end + def test_check_box assert_dom_equal( '', diff --git a/activemodel/lib/active_model/validations/numericality.rb b/activemodel/lib/active_model/validations/numericality.rb index 3806eff..aa3d3f4 100644 --- a/activemodel/lib/active_model/validations/numericality.rb +++ b/activemodel/lib/active_model/validations/numericality.rb @@ -131,6 +131,12 @@ module ActiveModel v.attributes.include?(attribute.to_sym) && (v.options.keys & [:less_than, :less_than_or_equal_to]).any? && (v.options.keys & [:if, :unless, :allow_nil, :allow_blank]).empty? }.map {|v| v.options.slice(:less_than, :less_than_or_equal_to)}.map(&:values).flatten.max end + + def attribute_min(attribute) + self.validators.grep(NumericalityValidator).select {|v| + v.attributes.include?(attribute.to_sym) && (v.options.keys & [:greater_than, :greater_than_or_equal_to]).any? && (v.options.keys & [:if, :unless, :allow_nil, :allow_blank]).empty? + }.map {|v| v.options.slice(:greater_than, :greater_than_or_equal_to)}.map(&:values).flatten.max + end end end end diff --git a/activemodel/test/cases/validations/numericality_validation_test.rb b/activemodel/test/cases/validations/numericality_validation_test.rb index 4f64c95..0d633a0 100644 --- a/activemodel/test/cases/validations/numericality_validation_test.rb +++ b/activemodel/test/cases/validations/numericality_validation_test.rb @@ -183,6 +183,27 @@ class NumericalityValidationTest < ActiveModel::TestCase assert_nil Topic.attribute_max(:approved) end + def test_attribute_min_using_greater_than + Topic.validates_numericality_of :approved, :greater_than => 20 + assert_equal 20, Topic.attribute_min(:approved) + end + + def test_attribute_min_using_greater_than_or_equal_to + Topic.validates_numericality_of :approved, :greater_than_or_equal_to => 40 + assert_equal 40, Topic.attribute_min(:approved) + end + + def test_attribute_min_with_if_or_unless_or_allow_nil_or_allow_blank + Topic.validates_numericality_of :approved, :greater_than => 20, :if => lambda {|u| true} + assert_nil Topic.attribute_min(:approved) + Topic.validates_numericality_of :approved, :greater_than => 20, :unless => lambda {|u| false} + assert_nil Topic.attribute_min(:approved) + Topic.validates_numericality_of :approved, :greater_than => 20, :allow_nil => true + assert_nil Topic.attribute_min(:approved) + Topic.validates_numericality_of :approved, :greater_than => 20, :allow_nil => true + assert_nil Topic.attribute_min(:approved) + end + private def invalid!(values, error = nil) -- 1.7.3.4