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

'validates_uniqueness_of' ignored if 'accepts_nested_attributes_for' is used #1572

Closed
adamflorin opened this issue Jun 8, 2011 · 41 comments

Comments

Projects
None yet
@adamflorin
Copy link

commented Jun 8, 2011

When batch creating a number of objects (as is done via mass assignment with accepts_nested_attributes_for), it looks like validates_uniqueness_of is just checking the db for conflicts, but not looking for conflicts within the batch currently being saved.

For example, given these two models:

class Parent < ActiveRecord::Base
  has_many :children
  accepts_nested_attributes_for :children
end

class Child < ActiveRecord::Base
  belongs_to :parent
  validates_uniqueness_of :name
end

Here's my output from the console. See that, despite the uniqueness constraint, two children with the same name were created, and that they then can't be re-saved!

?> p = Parent.create
=> #<Parent id: 3, created_at: "2011-06-08 17:31:54", updated_at: "2011-06-08 17:31:54">
>> p.update_attributes :children_attributes => [{:name => "Kid"}, {:name => "Kid"}]
=> true
>> Child.all.map(&:name)
=> ["Kid", "Kid"]
>> c = Child.first
=> #<Child id: 4, parent_id: 3, name: "Kid", created_at: "2011-06-08 17:32:12", updated_at: "2011-06-08 17:32:12">
>> c.save
=> false
>> c.errors.full_messages
=> ["Name has already been taken"]

I understand it may not be the role of validates_uniqueness_of to validate these objects against each other, but is there at least a reasonable workaround short of hand-coding a list-sorter/de-duper or something? Thanks!

@raviolicode

This comment has been minimized.

Copy link
Contributor

commented Jun 16, 2011

I can reproduce the problem, but I don't think it's directly related to nested_atributes_for. It also happens associating the children manually:

?> p = Parent.create
=> #<Parent id: 5, name: nil, age: nil, created_at: "2011-06-16 19:09:53", updated_at: "2011-06-16 19:09:53">
>> p.children.build :name => "Kid"
=> #<Child id: nil, parent_id: 5, name: "Kid", age: nil, created_at: nil, updated_at: nil>
>> p.children.build :name => "Kid"
=> #<Child id: nil, parent_id: 5, name: "Kid", age: nil, created_at: nil, updated_at: nil>
>> p.children
=> [#<Child id: nil, parent_id: 5, name: "Kid", age: nil, created_at: nil, updated_at: nil>, #<Child id: nil, parent_id: 5, name: "Kid", age: nil, created_at: nil, updated_at: nil>]
>> p.save
=> true
>> Child.all.map(&:name)
=> ["Kid", "Kid"]

You could prevent this behavior with validates_associated:

class Parent < ActiveRecord::Base
  has_many :children
  validates_associated :children
  accepts_nested_attributes_for :children
end

Hope that's what you're looking for.

@sikachu

This comment has been minimized.

Copy link
Member

commented Jul 23, 2011

Yep, validates_associated is the key here.

@sikachu sikachu closed this Jul 23, 2011

@verto

This comment has been minimized.

Copy link

commented Sep 18, 2011

I'm sorry guys, but i have same problem. I got reproduced the problem of @adamflorin with the validates_associated defined. The problem only occurs with nested_attributes. So, can't i use nested_attribute? :S

ruby-1.9.2-p290 :001 > p = Parent.create :children_attributes => [{:name => "Kid"}, {:name => "Kid"}]
=> #<Parent id: 1, created_at: "2011-09-18 18:00:08", updated_at: "2011-09-18 18:00:08"> 
ruby-1.9.2-p290 :002 > Child.all.map(&:name)
 => ["Kid", "Kid"] 
ruby-1.9.2-p290 :003 > c = Child.first
 => #<Child id: 1, parent_id: 1, name: "Kid", created_at: "2011-09-18 18:00:08", updated_at: "2011-09-18 18:00:08"> 
ruby-1.9.2-p290 :004 > c.save
 => false

Models:

class Parent < ActiveRecord::Base
  has_many :children
  validates_associated :children
  accepts_nested_attributes_for :children
end

class Child < ActiveRecord::Base
  belongs_to :parent
  validates_uniqueness_of :name
end

i'm using rails 3.1.0.

Thanks!

@celsodantas

This comment has been minimized.

Copy link

commented Sep 19, 2011

Same problem over here.

Even with validates_associated Rails let the Child be saved.

p = Parent.new
p.children << Child.new(:name => "Kid")
p.children << Child.new(:name => "Kid")
p.save
=> true

Rails (ActiveRecord) should avoid it, right? It only check if the Children named "Kid" is already in DB but not in memory.

There's a workaround using this: http://stackoverflow.com/questions/2772236/validates-uniqueness-of-in-nested-model-rails#answers-header

But I think it's a ActiveRecord issue.

@rafamvc

This comment has been minimized.

Copy link

commented Oct 4, 2011

+1

@kevinrood

This comment has been minimized.

Copy link

commented Nov 11, 2011

+1 to fix please

@emptyflask

This comment has been minimized.

Copy link

commented Nov 11, 2011

+1, running into the same problem here, and validates_associated doesn't solve it

@billywatson

This comment has been minimized.

Copy link

commented Nov 17, 2011

+1 can we reopen this issue, please?

@adsummos

This comment has been minimized.

Copy link

commented Nov 17, 2011

validates_associated works fine with validates_uniqueness of if you are adding a single record to a collection. Where it breaks is when you use collection.build to add multiple records. If there are duplicates within the new records, the validation doesn't find them.

@fj

This comment has been minimized.

Copy link

commented Nov 28, 2011

+1 for reopening; validates_associated doesn't work. To summarize, the issue is that if you:

  • are submitting multiple new records as part of an association; and
  • you accepts_nested_attribues_for on that association; and
  • that association has a validates ..., :uniqueness on it

then the uniqueness validator:

  • checks against records already in the database
  • won't find records that are not unique within the collection being submitted, since they aren't in the database yet

Likewise, validates_associated is of no help since it doesn't address the underlying uniqueness issue.

@rares

This comment has been minimized.

Copy link

commented Nov 28, 2011

+1 for a re-open.

At the very least, the validates_uniqueness_of documentation should point out that its behavior is altered significantly when used conjunction with accepts_nested_attributes_for and or when :autosave is enabled.

If I get a chance in the next few days, i'll fork and add the warning myself.

@billywatson

This comment has been minimized.

Copy link

commented Nov 30, 2011

@rares, I think the warning might be unnecessary as this seems to be a bug, not just a slight altering in behavior. The validation just plain doesn't work with acceps_nested_attributes_for.

@rares

This comment has been minimized.

Copy link

commented Nov 30, 2011

I am split on this, as where it seems like a bug, it is actually really hard to solve in the context of accepts_nested_attributes_for because it all executes within the scope of one transaction. The query executed in the validation never can see the data because the data is not committed (default transaction isolation level is READ_COMMITTED on oracle and postgres, not sure about mysql).

One thought I have had is to alter the transaction isolation level (to READ_UNCOMMITTED), but can lead to some scary situations in a write-heavy app.

validates_uniqueness_of can also be extended to look at its in-memory collection for uniqueness as well, but that is more of a hack then actually fixing the problem.

Bottom line, my reasoning for including it in the docs was not to be the eventual solution (as i don't think its easily solvable given how both nested_attributes and the validator works) but as warning so that people are at least educated when using these two features together.

@adsummos

This comment has been minimized.

Copy link

commented Nov 30, 2011

It's important to note that this is not specific to accepts_nested_attributes_for. This is a general problem with validates_uniqueness_of when you are adding multiple records to a collection. The transaction doesn't matter because it's the in-memory objects that are not being validated against each other.

@rares

This comment has been minimized.

Copy link

commented Nov 30, 2011

Yeah, that is much is understood. the question is how to merge that concern with that validates_uniqueness_of does.

@adsummos

This comment has been minimized.

Copy link

commented Nov 30, 2011

It's really not a difficult problem to solve, we already have a patch in our own code. A generic solution for AR is not much more complicated than what we are doing. If nothing else gets done, when I have time I will submit something myself.

@rares

This comment has been minimized.

Copy link

commented Nov 30, 2011

Great! I look forward to it.

@pekeler

This comment has been minimized.

Copy link

commented Dec 2, 2011

+1

@popungco

This comment has been minimized.

Copy link

commented Jan 17, 2012

+1 validates_presence_of too.

@morexchange

This comment has been minimized.

Copy link

commented Feb 23, 2012

For the record, this happens not only for validates_uniqueness_of but for other validations as well (all?).

In my case, I have a validates_numericality_of :x, :only_integer => true on one of the attributes (data type: integer) of the associated object. When value is changed from 200 (valid value) to 200.13 (decimal is invalid), it doesn't trigger "not a number" error.

Surprisingly, when value is changed from 200 to 199.13 (still invalid but the integer portion is different from previous value), the validations run since the field is of type :integer and activerecord is able to detect that integer portion of value has changed.

Rails version: 2.3.14

@Backoo

This comment has been minimized.

Copy link

commented Nov 12, 2012

+1 ! ! !

@jamilabreu

This comment has been minimized.

Copy link

commented Nov 15, 2012

+1

1 similar comment
@birula

This comment has been minimized.

Copy link

commented Nov 21, 2012

+1

@jeyb

This comment has been minimized.

Copy link
Contributor

commented Nov 26, 2012

I opened a pull request with a fix for this bug, take a look. Let me know if you have suggestions or comments.

@sotrachhun

This comment has been minimized.

Copy link

commented May 3, 2013

I have used this patch in my code and it seem works well on Development and Staging Environment.
However, on Production I faced some problems but not sure it is related to this patch or not.
User add nested records after saved it responded ok but it doesn't save any data. After users try 2 or 3 times, it saved. It is happen only some time and not often.

class Author
  has_many :books

  # Could easily be made a validation-style class method of course
  validate :validate_unique_books

  def validate_unique_books
    validate_uniqueness_of_in_memory(
      books, [:title, :isbn], 'Duplicate book.')
  end
end

module ActiveRecord
  class Base
    # Validate that the the objects in +collection+ are unique
    # when compared against all their non-blank +attrs+. If not
    # add +message+ to the base errors.
    def validate_uniqueness_of_in_memory(collection, attrs, message)
      hashes = collection.inject({}) do |hash, record|
        key = attrs.map {|a| record.send(a).to_s }.join
        if key.blank? || record.marked_for_destruction?
          key = record.object_id
        end
        hash[key] = record unless hash[key]
        hash
      end
      if collection.length > hashes.length
        self.errors.add_to_base(message)
      end
    end
  end
end
@mpapis

This comment has been minimized.

Copy link
Contributor

commented Mar 9, 2016

+1 for reopening, a simple iteration / group_by would solve it

@Quintasan

This comment has been minimized.

Copy link

commented Aug 14, 2017

Can we reopen this?

@h8rry

This comment has been minimized.

Copy link

commented Aug 29, 2017

Just got into this problem too

@ethagnawl

This comment has been minimized.

Copy link

commented Oct 31, 2017

This just bit me as well. Thankfully, I had an index in place to prevent the "bad" data from being saved. However, it'd make for a much better user experience if the issue could be caught during the ActiveRecord validation cycle.

@erprincebansal

This comment has been minimized.

Copy link

commented Jan 15, 2018

Can we reopen this?
I faced the same issue.
Rails: 5.1.4

@mshka

This comment has been minimized.

Copy link

commented Feb 4, 2018

+1 for reopening this; validates_associated doesn't work with nested attributes

@kirandc

This comment has been minimized.

Copy link

commented Apr 10, 2018

+1 Yes it does not work with Rails 5.1.5 also. any update on this?

@hanyelbanna

This comment has been minimized.

Copy link

commented Apr 11, 2018

+1 , I have added index on the database to solve it but I get big PG::UniqueViolation: ERROR.
How can I solve that?

@jngarcia

This comment has been minimized.

Copy link

commented Jun 26, 2018

+1

1 similar comment
@SeinopSys

This comment has been minimized.

Copy link

commented Sep 3, 2018

+1

@djezzzl

This comment has been minimized.

Copy link

commented Sep 5, 2018

Hi, I'd suggest you use validates_db_uniqueness_of. It should solve your problem. Let me know if it's not. The validator is not completely ready: some of the options are not supported yet but will be very soon.

Any feedback is appreciated 👍

@alexander-e1off

This comment has been minimized.

Copy link

commented Sep 14, 2018

Hi, I've tried validates_db_uniqueness_of as follows:

class Parent< ApplicationRecord
  has_many :parent_children, inverse_of: :parent
  accepts_nested_attributes_for :parent_children
end
class Child< ApplicationRecord
  has_many :parent_children, inverse_of: :child
end
class ParentChild< ApplicationRecord
  belongs_to parent
  belongs_to child
  validates_db_uniqueness_of :child_id, scope: :parent_id
end

When duplicated ParentChild associations are added to Parent, parent.save returns false but parent.errors.messages is empty so user cannot understand what is the reason of failure.

Thanks

@djezzzl

This comment has been minimized.

Copy link

commented Sep 14, 2018

@alexander-e1off

Hi, thank you for your case!

First of all, I want to highlight that the gem solves the real problem: now we don't save duplicates and save returns false.

About your issue: this is happening because of the implementation of accepts_nested_attributes_for.
The flow is the following:

  • It runs valid? on every sub-entity. Each valid? returns true because there is no duplication yet (we didn't commit the transaction to a database).
  • Because everything is fine it starts an insert transaction (saving).
  • Then the transaction is rolled back because of internal duplicates. And we catch it and returns false.

There is an option (even if a bit ugly) to get the errors:

p = Parent.new(parent_children: [ParentChild.new(child: Child.first), ParentChild.new(child: Child.first)])
p.save
# => false
p.parent_children.map(&:errors)
 # => [#<ActiveModel::Errors:0x00007fa7d2479c40 @base=#<ParentChild id: nil, parent_id: 1, child_id: 1, any_data: nil, created_at: "2018-09-14 20:05:43", updated_at: "2018-09-14 20:05:43">, @messages={}, @details={}>, #<ActiveModel::Errors:0x00007fa7d3466130 @base=#<ParentChild id: nil, parent_id: 1, child_id: 1, any_data: nil, created_at: "2018-09-14 20:05:43", updated_at: "2018-09-14 20:05:43">, @messages={:child_id=>["has already been taken"]}, @details={:child_id=>[{:error=>:taken, :value=>1}]}>] 

As you can see, first sub-entity doesn't have any errors but second does, you can extract it and use.

I hope this explanation helps you.

@alexander-e1off

This comment has been minimized.

Copy link

commented Sep 17, 2018

Hi @djezzzl, thanks a lot for your explanation! I've already went to this direction to iterate children and retrieve error message from there.

@jamesst20

This comment has been minimized.

Copy link

commented Jun 11, 2019

The issue is still present 8 years later in Rails 6.

https://github.com/toptal/database_validations -> validates_db_uniqueness_of did solve the issue.

Thank you

@djezzzl

This comment has been minimized.

Copy link

commented Jun 12, 2019

@jamesst20 Thank you, glad it helps, more cool staff coming soon 👍

P.S. In case, you work with FactoryBot I can suggest you to try https://github.com/djezzzl/factory_trace
And, if you like to have your DB to be consistent with models definition, there is a https://github.com/djezzzl/database_consistency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.