Add :conditions option to uniqueness validator #5321

Merged
merged 3 commits into from Mar 17, 2012

Conversation

Projects
None yet
5 participants
@pfeiffer
Contributor

pfeiffer commented Mar 7, 2012

This commit adds a :conditions option to the uniqueness validator. The :conditions is added to the lookup used for determining the uniqueness of the attribute.

Example use-case:
An article with a title and a status. Status can be either draft, published or archived. We want to have unique titles - but only for non-archived articles. This cannot be accomplished using :scope => :status, since a draft article and a published article with identical titles both would be valid. This can now be accomplished with:

class Article < AR::Base
  validates :title, :uniqueness => {:conditions => ['status != ?', 'archived']}
end

In this case a draft article can have the same title as an archived article, but not the same name as a published article.

What do you think? Let me know if anything is missing - this is my first pull request to Rails.

@@ -35,8 +35,8 @@ def validate_each(record, attribute, value)
relation = relation.and(table[scope_item].eq(scope_value))
end
- if finder_class.unscoped.where(relation).exists?
- record.errors.add(attribute, :taken, options.except(:case_sensitive, :scope).merge(:value => value))
+ if finder_class.unscoped.where(relation).where(options[:conditions]).exists?

This comment has been minimized.

@pfeiffer

pfeiffer Mar 7, 2012

Contributor

Would prefer only to call the .where(options[:conditions]) if conditions are present - but didn't want to move too much around in this first take. Suggestions?

@pfeiffer

pfeiffer Mar 7, 2012

Contributor

Would prefer only to call the .where(options[:conditions]) if conditions are present - but didn't want to move too much around in this first take. Suggestions?

@josevalim

This comment has been minimized.

Show comment
Hide comment
@josevalim

josevalim Mar 7, 2012

Contributor

Thanks @pfeiffer. I believe something along these lines is convenient but I would rather allow a scope like where("status != 'archived'") than the old conditions hash.

Contributor

josevalim commented Mar 7, 2012

Thanks @pfeiffer. I believe something along these lines is convenient but I would rather allow a scope like where("status != 'archived'") than the old conditions hash.

@pfeiffer

This comment has been minimized.

Show comment
Hide comment
@pfeiffer

pfeiffer Mar 7, 2012

Contributor

@josevalim Great - I've pushed an update supporting where("..") instead of conditions hash.

Contributor

pfeiffer commented Mar 7, 2012

@josevalim Great - I've pushed an update supporting where("..") instead of conditions hash.

@@ -102,6 +108,14 @@ module ClassMethods
# validates_uniqueness_of :teacher_id, :scope => [:semester_id, :class_id]
# end
#
+ # It is also possible to limit the uniqueness constraint to a set of records matching certain conditions.
+ # In this example archived articles with are not being taken into consideration when validating uniqueness

This comment has been minimized.

@vijaydev

vijaydev Mar 7, 2012

Member

Remove the unwanted "with" please.

@vijaydev

vijaydev Mar 7, 2012

Member

Remove the unwanted "with" please.

@pfeiffer

This comment has been minimized.

Show comment
Hide comment
@pfeiffer

pfeiffer Mar 8, 2012

Contributor

@vijaydev Done!

Contributor

pfeiffer commented Mar 8, 2012

@vijaydev Done!

@pfeiffer

This comment has been minimized.

Show comment
Hide comment
@pfeiffer

pfeiffer Mar 17, 2012

Contributor

@josevalim Have you had time to review the changes?

Contributor

pfeiffer commented Mar 17, 2012

@josevalim Have you had time to review the changes?

josevalim added a commit that referenced this pull request Mar 17, 2012

Merge pull request #5321 from pfeiffer/uniqueness_validator_conditions
Add :conditions option to uniqueness validator

@josevalim josevalim merged commit 4711298 into rails:master Mar 17, 2012

@josevalim

This comment has been minimized.

Show comment
Hide comment
@josevalim

josevalim Mar 17, 2012

Contributor

Merged, sorry for the delay. It somehow escaped my inbox. :)

Contributor

josevalim commented Mar 17, 2012

Merged, sorry for the delay. It somehow escaped my inbox. :)

@carlosantoniodasilva

This comment has been minimized.

Show comment
Hide comment
@carlosantoniodasilva

carlosantoniodasilva Jan 27, 2013

Member

@jonleighton hey, given AR is moving to a lazy approach for everything - from scopes to associations - I guess this should be changed to follow the same approach, wdyt?

@jonleighton hey, given AR is moving to a lazy approach for everything - from scopes to associations - I guess this should be changed to follow the same approach, wdyt?

@jonleighton

This comment has been minimized.

Show comment
Hide comment
@jonleighton

jonleighton Jan 27, 2013

Member

@carlosantoniodasilva yeah I guess so.

Member

jonleighton commented Jan 27, 2013

@carlosantoniodasilva yeah I guess so.

@carlosantoniodasilva

This comment has been minimized.

Show comment
Hide comment
@carlosantoniodasilva

carlosantoniodasilva Jan 27, 2013

Member

@pfeiffer hey, perhaps you want to take a look on that? Otherwise let me know and I'll try to take care of it later this week. Thanks!

@pfeiffer hey, perhaps you want to take a look on that? Otherwise let me know and I'll try to take care of it later this week. Thanks!

senny added a commit to senny/rails that referenced this pull request Mar 9, 2013

Uniqueness validation uses a proc to specify the `:conditions` option.
This is a follow up to #5321 and follows the general direction in
AR to make things lazy evaluated.

carlosantoniodasilva added a commit that referenced this pull request Mar 9, 2013

Merge pull request #9633 from senny/5321_make_it_lazy
Uniqueness validation uses a proc to specify the `:conditions` option.

This is a follow up to #5321 and follows the general direction in
AR to make things lazy evaluated.

@al2o3cr al2o3cr referenced this pull request in composite-primary-keys/composite_primary_keys Nov 26, 2014

Closed

Uniqueness validation with a condition fails #261

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment