Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added absence validator for active model. #7155

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions actionpack/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,21 @@
## Rails 4.0.0 (unreleased) ##

* Fix input name when `:multiple => true` and `:index` are set.

Before:

check_box("post", "comment_ids", { :multiple => true, :index => "foo" }, 1)
#=> <input name=\"post[foo][comment_ids]\" type=\"hidden\" value=\"0\" /><input id=\"post_foo_comment_ids_1\" name=\"post[foo][comment_ids]\" type=\"checkbox\" value=\"1\" />

After:

check_box("post", "comment_ids", { :multiple => true, :index => "foo" }, 1)
#=> <input name=\"post[foo][comment_ids][]\" type=\"hidden\" value=\"0\" /><input id=\"post_foo_comment_ids_1\" name=\"post[foo][comment_ids][]\" type=\"checkbox\" value=\"1\" />

Fix #8108

*Daniel Fox, Grant Hutchins & Trace Wax*

* Clear url helpers when reloading routes.

*Santiago Pastorino*
Expand Down
3 changes: 2 additions & 1 deletion actionpack/lib/action_controller/test_case.rb
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ def process(action, http_method = 'GET', *args)
@request.assign_parameters(@routes, controller_class_name, action.to_s, parameters)

@request.session.update(session) if session
@request.session["flash"] = @request.flash.update(flash || {})
@request.flash.update(flash || {})

@controller.request = @request
@controller.response = @response
Expand All @@ -526,6 +526,7 @@ def process(action, http_method = 'GET', *args)
@response.prepare!

@assigns = @controller.respond_to?(:view_assigns) ? @controller.view_assigns : {}
@request.session['flash'] = @request.flash.to_session_value
@request.session.delete('flash') if @request.session['flash'].blank?
@response
end
Expand Down
33 changes: 24 additions & 9 deletions actionpack/lib/action_dispatch/middleware/flash.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class Request < Rack::Request
# read a notice you put there or <tt>flash["notice"] = "hello"</tt>
# to put a new one.
def flash
@env[Flash::KEY] ||= (session["flash"] || Flash::FlashHash.new).tap(&:sweep)
@env[Flash::KEY] ||= Flash::FlashHash.from_session_value(session["flash"])
end
end

Expand Down Expand Up @@ -70,16 +70,31 @@ def notice=(message)
end
end

# Implementation detail: please do not change the signature of the
# FlashHash class. Doing that will likely affect all Rails apps in
# production as the FlashHash currently stored in their sessions will
# become invalid.
class FlashHash
include Enumerable

def initialize #:nodoc:
@discard = Set.new
@flashes = {}
def self.from_session_value(value)
flash = case value
when FlashHash # Rails 3.1, 3.2
new(value.instance_variable_get(:@flashes), value.instance_variable_get(:@used))
when Hash # Rails 4.0
new(value['flashes'], value['discard'])
else
new
end

flash.sweep
flash
end

def to_session_value
return nil if empty?
{ 'discard' => @discard.to_a, 'flashes' => @flashes }
end

def initialize(flashes = {}, discard = []) #:nodoc:
@discard = Set.new(discard)
@flashes = flashes
@now = nil
end

Expand Down Expand Up @@ -223,7 +238,7 @@ def call(env)

if flash_hash
if !flash_hash.empty? || session.key?('flash')
session["flash"] = flash_hash
session["flash"] = flash_hash.to_session_value
new_hash = flash_hash.dup
else
new_hash = flash_hash
Expand Down
4 changes: 1 addition & 3 deletions actionpack/lib/action_dispatch/routing/mapper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -491,9 +491,7 @@ def define_generate_prefix(app, name)
prefix_options = options.slice(*_route.segment_keys)
# we must actually delete prefix segment keys to avoid passing them to next url_for
_route.segment_keys.each { |k| options.delete(k) }
prefix = _routes.url_helpers.send("#{name}_path", prefix_options)
prefix = '' if prefix == '/'
prefix
_routes.url_helpers.send("#{name}_path", prefix_options)
end
end
end
Expand Down
21 changes: 21 additions & 0 deletions actionpack/test/controller/flash_hash_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,27 @@ def test_to_hash
assert_equal({'foo' => 'bar'}, @hash.to_hash)
end

def test_to_session_value
@hash['foo'] = 'bar'
assert_equal({'flashes' => {'foo' => 'bar'}, 'discard' => []}, @hash.to_session_value)

@hash.discard('foo')
assert_equal({'flashes' => {'foo' => 'bar'}, 'discard' => %w[foo]}, @hash.to_session_value)

@hash.now['qux'] = 1
assert_equal({'flashes' => {'foo' => 'bar', 'qux' => 1}, 'discard' => %w[foo qux]}, @hash.to_session_value)

@hash.sweep
assert_equal(nil, @hash.to_session_value)
end

def test_from_session_value
rails_3_2_cookie = 'BAh7B0kiD3Nlc3Npb25faWQGOgZFRkkiJWY4ZTFiODE1MmJhNzYwOWMyOGJiYjE3ZWM5MjYzYmE3BjsAVEkiCmZsYXNoBjsARm86JUFjdGlvbkRpc3BhdGNoOjpGbGFzaDo6Rmxhc2hIYXNoCToKQHVzZWRvOghTZXQGOgpAaGFzaHsAOgxAY2xvc2VkRjoNQGZsYXNoZXN7BkkiDG1lc3NhZ2UGOwBGSSIKSGVsbG8GOwBGOglAbm93MA=='
session = Marshal.load(Base64.decode64(rails_3_2_cookie))
hash = Flash::FlashHash.from_session_value(session['flash'])
assert_equal({'flashes' => {'message' => 'Hello'}, 'discard' => %w[message]}, hash.to_session_value)
end

def test_empty?
assert @hash.empty?
@hash['zomg'] = 'bears'
Expand Down
5 changes: 5 additions & 0 deletions actionpack/test/dispatch/prefix_generation_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,11 @@ def setup
assert_equal "/something/", app_object.root_path
end

test "[OBJECT] generating application's route includes default_url_options[:trailing_slash]" do
RailsApplication.routes.default_url_options[:trailing_slash] = true
assert_equal "/awesome/blog/posts", engine_object.posts_path
end

test "[OBJECT] generating engine's route with url_for" do
path = engine_object.url_for(:controller => "inside_engine_generating",
:action => "show",
Expand Down
18 changes: 18 additions & 0 deletions activemodel/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,23 @@
## Rails 4.0.0 (unreleased) ##

* Add `ActiveModel::Validations::AbsenceValidator`, a validator to check the
absence of attributes.

class Person < ActiveRecord::Base
validates_absence_of :first_name
end

person = Person.new
person.first_name = "John"
person.valid?
=> false
# first_name must be blank

* Added `ActiveModel::Errors#add_on_present` method. Adds error messages to
present attributes.

*Roberto Vasquez Angel*

* Add `ActiveModel::ForbiddenAttributesProtection`, a simple module to
protect attributes from mass assignment when non-permitted attributes are passed.

Expand Down
13 changes: 13 additions & 0 deletions activemodel/lib/active_model/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,19 @@ def add_on_blank(attributes, options = {})
end
end

# Will add an error message to each of the attributes in +attributes+ that
# is present (using Object#present?).
#
# person.errors.add_on_present(:name)
# person.errors.messages
# # => { :name => ["must be blank"] }
def add_on_present(attributes, options = {})
Array(attributes).flatten.each do |attribute|
value = @base.send(:read_attribute_for_validation, attribute)
add(attribute, :not_blank, options) if value.present?
end
end

# Returns +true+ if an error on the attribute with the given message is
# present, +false+ otherwise. +message+ is treated the same as for +add+.
#
Expand Down
1 change: 1 addition & 0 deletions activemodel/lib/active_model/locale/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ en:
accepted: "must be accepted"
empty: "can't be empty"
blank: "can't be blank"
not_blank: "must be blank"
too_long: "is too long (maximum is %{count} characters)"
too_short: "is too short (minimum is %{count} characters)"
wrong_length: "is the wrong length (should be %{count} characters)"
Expand Down
31 changes: 31 additions & 0 deletions activemodel/lib/active_model/validations/absence.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
module ActiveModel
module Validations
# == Active Model Absence Validator
class AbsenceValidator < EachValidator #:nodoc:
def validate(record)
record.errors.add_on_present(attributes, options)
end
end

module HelperMethods
# Validates that the specified attributes are blank (as defined by
# Object#blank?). Happens by default on save.
#
# class Person < ActiveRecord::Base
# validates_absence_of :first_name
# end
#
# The first_name attribute must be in the object and it must be blank.
#
# Configuration options:
# * <tt>:message</tt> - A custom error message (default is: "must be blank").
#
# There is also a list of default options supported by every validator:
# +:if+, +:unless+, +:on+ and +:strict+.
# See <tt>ActiveModel::Validation#validates</tt> for more information
def validates_absence_of(*attr_names)
validates_with AbsenceValidator, _merge_attributes(attr_names)
end
end
end
end
67 changes: 67 additions & 0 deletions activemodel/test/cases/validations/absence_validation_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# encoding: utf-8
require 'cases/helper'
require 'models/topic'
require 'models/person'
require 'models/custom_reader'

class AbsenceValidationTest < ActiveModel::TestCase
teardown do
Topic.reset_callbacks(:validate)
Person.reset_callbacks(:validate)
CustomReader.reset_callbacks(:validate)
end

def test_validate_absences
Topic.validates_absence_of(:title, :content)
t = Topic.new
t.title = "foo"
t.content = "bar"
assert t.invalid?
assert_equal ["must be blank"], t.errors[:title]
assert_equal ["must be blank"], t.errors[:content]
t.title = ""
t.content = "something"
assert t.invalid?
assert_equal ["must be blank"], t.errors[:content]
t.content = ""
assert t.valid?
end

def test_accepts_array_arguments
Topic.validates_absence_of %w(title content)
t = Topic.new
t.title = "foo"
t.content = "bar"
assert t.invalid?
assert_equal ["must be blank"], t.errors[:title]
assert_equal ["must be blank"], t.errors[:content]
end

def test_validates_acceptance_of_with_custom_error_using_quotes
Person.validates_absence_of :karma, :message => "This string contains 'single' and \"double\" quotes"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use 1.9 hash syntax. Thanks.

p = Person.new
p.karma = "good"
assert p.invalid?
assert_equal "This string contains 'single' and \"double\" quotes", p.errors[:karma].last
end

def test_validates_absence_of_for_ruby_class
Person.validates_absence_of :karma
p = Person.new
p.karma = "good"
assert p.invalid?
assert_equal ["must be blank"], p.errors[:karma]
p.karma = nil
assert p.valid?
end

def test_validates_absence_of_for_ruby_class_with_custom_reader
CustomReader.validates_absence_of :karma
p = CustomReader.new
p[:karma] = "excellent"
assert p.invalid?
assert_equal ["must be blank"], p.errors[:karma]
p[:karma] = ""
assert p.valid?
end
end
1 change: 0 additions & 1 deletion activerecord/lib/active_record/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
require 'active_support/core_ext/class/delegating_attributes'
require 'active_support/core_ext/array/extract_options'
require 'active_support/core_ext/hash/deep_merge'
require 'active_support/core_ext/hash/indifferent_access'
require 'active_support/core_ext/hash/slice'
require 'active_support/core_ext/string/behavior'
require 'active_support/core_ext/kernel/singleton_class'
Expand Down
6 changes: 1 addition & 5 deletions activerecord/lib/active_record/relation/finder_methods.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
require 'active_support/core_ext/hash/indifferent_access'

module ActiveRecord
module FinderMethods
# Find by id - This can either be a specific id (1), a list of ids (1, 5, 6), or an array of ids ([5, 6, 10]).
Expand Down Expand Up @@ -225,7 +223,7 @@ def apply_join_dependency(relation, join_dependency)

def construct_limited_ids_condition(relation)
orders = relation.order_values.map { |val| val.presence }.compact
values = @klass.connection.distinct("#{@klass.connection.quote_table_name table_name}.#{primary_key}", orders)
values = @klass.connection.distinct("#{quoted_table_name}.#{primary_key}", orders)

relation = relation.dup

Expand All @@ -234,8 +232,6 @@ def construct_limited_ids_condition(relation)
end

def find_with_ids(*ids)
return to_a.find { |*block_args| yield(*block_args) } if block_given?

expects_array = ids.first.kind_of?(Array)
return ids.first if expects_array && ids.first.empty?

Expand Down
11 changes: 11 additions & 0 deletions activerecord/test/cases/nested_attributes_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,17 @@ def parrot_attributes=(attrs)
assert_equal "James", mean_pirate.parrot.name
assert_equal "blue", mean_pirate.parrot.color
end

def test_accepts_nested_attributes_for_can_be_overridden_in_subclasses
Pirate.accepts_nested_attributes_for(:parrot)

mean_pirate_class = Class.new(Pirate) do
accepts_nested_attributes_for :parrot
end
mean_pirate = mean_pirate_class.new
mean_pirate.parrot_attributes = { :name => "James" }
assert_equal "James", mean_pirate.parrot.name
end
end

class TestNestedAttributesOnAHasOneAssociation < ActiveRecord::TestCase
Expand Down
7 changes: 6 additions & 1 deletion railties/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
## Rails 4.0.0 (unreleased) ##

* Add dummy app Rake tasks when --skip-test-unit and --dummy-path is passed to the plugin generator.
Fix #8121

*Yves Senn*

* Ensure that RAILS_ENV is set when accessing Rails.env *Steve Klabnik*

* Don't eager-load app/assets and app/views *Elia Schito*
Expand All @@ -9,7 +14,7 @@
* New test locations `test/models`, `test/helpers`, `test/controllers`, and
`test/mailers`. Corresponding rake tasks added as well. *Mike Moore*

* Set a different cache per environment for assets pipeline
* Set a different cache per environment for assets pipeline
through `config.assets.cache`.

*Guillermo Iguaran*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ def create_test_files
end

def create_test_dummy_files
return if options[:skip_test_unit] && options[:dummy_path] == 'test/dummy'
return unless with_dummy_app?
create_dummy_app
end

Expand Down Expand Up @@ -279,6 +279,10 @@ def mountable?
options[:mountable]
end

def with_dummy_app?
options[:skip_test_unit].blank? || options[:dummy_path] != 'test/dummy'
end

def self.banner
"rails plugin new #{self.arguments.map(&:usage).join(' ')} [options]"
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ RDoc::Task.new(:rdoc) do |rdoc|
rdoc.rdoc_files.include('lib/**/*.rb')
end

<% if full? && !options[:skip_active_record] && !options[:skip_test_unit] -%>
<% if full? && !options[:skip_active_record] && with_dummy_app? -%>
APP_RAKEFILE = File.expand_path("../<%= dummy_path -%>/Rakefile", __FILE__)
load 'rails/tasks/engine.rake'
<% end %>
Expand Down
Loading