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

activemodel: ActiveModel::Errors#add accepts String and Proc for type #717

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
9 changes: 9 additions & 0 deletions gems/activemodel/6.0/_test/test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,17 @@ class Person

validates :name, presence: true, length: { maximum: 100 }
validates :email, presence: true, if: [:foo?, -> { age >= 20 }]
validate :should_be_satisfied_special_email_rule

def foo? = true

def should_be_satisfied_special_email_rule
if Time.current >= Time.zone.local(2024, 10)
errors.add(:email, -> (_person, _options) { "must be satisfied at least 3 rules after #{Time.zone.local(2024, 10)}" }) if [/a-z/, /A-Z/, /0-9/, /[+]/].count {|rule| email.match?(rule) } > 3
else
errors.add(:email, 'must be satisfied at least 2 rules') if [/a-z/, /A-Z/, /0-9/].count {|rule| email.match?(rule) } > 2
end
end
end

person = Person.new(name: 'John Doe')
Expand Down
1 change: 1 addition & 0 deletions gems/activemodel/6.0/_test/test.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ class Person
attr_accessor email: String

def foo?: () -> bool
def should_be_satisfied_special_email_rule: () -> void
end
43 changes: 0 additions & 43 deletions gems/activemodel/6.0/activemodel-generated.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -1500,49 +1500,6 @@ module ActiveModel
# person.errors.to_hash(true) # => {:name=>["name cannot be nil"]}
def to_hash: (?bool full_messages) -> untyped

# Adds +message+ to the error messages and used validator type to +details+ on +attribute+.
# More than one error can be added to the same +attribute+.
# If no +message+ is supplied, <tt>:invalid</tt> is assumed.
#
# person.errors.add(:name)
# # => ["is invalid"]
# person.errors.add(:name, :not_implemented, message: "must be implemented")
# # => ["is invalid", "must be implemented"]
#
# person.errors.messages
# # => {:name=>["is invalid", "must be implemented"]}
#
# person.errors.details
# # => {:name=>[{error: :not_implemented}, {error: :invalid}]}
#
# If +message+ is a symbol, it will be translated using the appropriate
# scope (see +generate_message+).
#
# If +message+ is a proc, it will be called, allowing for things like
# <tt>Time.now</tt> to be used within an error.
#
# If the <tt>:strict</tt> option is set to +true+, it will raise
# ActiveModel::StrictValidationFailed instead of adding the error.
# <tt>:strict</tt> option can also be set to any other exception.
#
# person.errors.add(:name, :invalid, strict: true)
# # => ActiveModel::StrictValidationFailed: Name is invalid
# person.errors.add(:name, :invalid, strict: NameIsInvalid)
# # => NameIsInvalid: Name is invalid
#
# person.errors.messages # => {}
#
# +attribute+ should be set to <tt>:base</tt> if the error is not
# directly associated with a single attribute.
#
# person.errors.add(:base, :name_or_email_blank,
# message: "either name or email must be present")
# person.errors.messages
# # => {:base=>["either name or email must be present"]}
# person.errors.details
# # => {:base=>[{error: :name_or_email_blank}]}
def add: (untyped attribute, ?::Symbol message, ?::Hash[untyped, untyped] options) -> untyped

# Returns +true+ if an error on the attribute with the given message is
# present, or +false+ otherwise. +message+ is treated the same as for +add+.
#
Expand Down
49 changes: 48 additions & 1 deletion gems/activemodel/6.0/activemodel.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ module ActiveModel
# person = Person.new
# person.valid? # => false
# person.errors # => #<ActiveModel::Errors:0x007fe603816640 @messages={name:["can't be blank"]}>
def errors: () -> untyped
def errors: () -> Errors

# Runs all the specified validations and returns +true+ if no errors were
# added otherwise +false+.
Expand Down Expand Up @@ -498,3 +498,50 @@ module ActiveModel
end
end
end

module ActiveModel
class Errors
# Adds +message+ to the error messages and used validator type to +details+ on +attribute+.
# More than one error can be added to the same +attribute+.
# If no +message+ is supplied, <tt>:invalid</tt> is assumed.
#
# person.errors.add(:name)
# # => ["is invalid"]
# person.errors.add(:name, :not_implemented, message: "must be implemented")
# # => ["is invalid", "must be implemented"]
#
# person.errors.messages
# # => {:name=>["is invalid", "must be implemented"]}
#
# person.errors.details
# # => {:name=>[{error: :not_implemented}, {error: :invalid}]}
#
# If +message+ is a symbol, it will be translated using the appropriate
# scope (see +generate_message+).
#
# If +message+ is a proc, it will be called, allowing for things like
# <tt>Time.now</tt> to be used within an error.
#
# If the <tt>:strict</tt> option is set to +true+, it will raise
# ActiveModel::StrictValidationFailed instead of adding the error.
# <tt>:strict</tt> option can also be set to any other exception.
#
# person.errors.add(:name, :invalid, strict: true)
# # => ActiveModel::StrictValidationFailed: Name is invalid
# person.errors.add(:name, :invalid, strict: NameIsInvalid)
# # => NameIsInvalid: Name is invalid
#
# person.errors.messages # => {}
#
# +attribute+ should be set to <tt>:base</tt> if the error is not
# directly associated with a single attribute.
#
# person.errors.add(:base, :name_or_email_blank,
# message: "either name or email must be present")
# person.errors.messages
# # => {:base=>["either name or email must be present"]}
# person.errors.details
# # => {:base=>[{error: :name_or_email_blank}]}
def add: (untyped attribute, ?::Symbol | ::String | ^(untyped base, ::Hash[untyped, untyped]) -> (::Symbol | ::String) type, ?::Hash[untyped, untyped] options) -> void
Copy link
Collaborator

Choose a reason for hiding this comment

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

What makes you think return type should be void instead of ActiveModel::Error?

end
end