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

accepts_nested_attributes_for #185

Closed
emrekutlu opened this Issue Nov 15, 2011 · 12 comments

Comments

Projects
None yet
2 participants
@emrekutlu

emrekutlu commented Nov 15, 2011

class Author < ActiveRecord::Base
  belongs_to :publication, :polymorphic => true
  friendly_id :name, :use => :slugged
end
class Book < ActiveRecord::Base
  has_many :authors, :as => :publication
  accepts_nested_attributes_for :authors
end

I am using "4.0.0.beta14". I have a form for Book model which also can add multiple authors. Author model's slug column has an unique index.

When i try to add multiple authors whose name is same, all authors try to get the same slug, so a "Mysql2::Error: Duplicate entry" occurs.

@norman

This comment has been minimized.

Show comment
Hide comment
@norman

norman Nov 15, 2011

Owner

"When i try to add multiple authors"

Thanks for the bug report. Can you show me code that shows me exactly how you're trying to do that? Also, please show me your whole stack trace, not just the error message.

I am more than happy to help you but if I have send you multiple messages to figure out what's going on in your code, or spend 30 minutes figuring out how to reproduce your problem, then I can't help you because I am literally too busy.

Owner

norman commented Nov 15, 2011

"When i try to add multiple authors"

Thanks for the bug report. Can you show me code that shows me exactly how you're trying to do that? Also, please show me your whole stack trace, not just the error message.

I am more than happy to help you but if I have send you multiple messages to figure out what's going on in your code, or spend 30 minutes figuring out how to reproduce your problem, then I can't help you because I am literally too busy.

@emrekutlu

This comment has been minimized.

Show comment
Hide comment
@emrekutlu

emrekutlu Nov 16, 2011

Ok, i'm sorry, you're right. I create a repository to reproduce the error.
http://github.com/emrekutlu/friendly_id_bug_report

I am using Rails 3.1. I try to explain it here in more detail.

class Author < ActiveRecord::Base
  extend FriendlyId
  belongs_to :publication, :polymorphic => true
  friendly_id :name, :use => :slugged
end
class Book < ActiveRecord::Base
  has_many :authors, :as => :publication
  accepts_nested_attributes_for :authors
end

I added a unique index for slug column.

In books controller:

def new
    @book = Book.new
    (1..3).each do
      @book.authors.build
    end

    respond_to do |format|
      format.html # new.html.erb
      format.json { render json: @book }
    end
  end

I have a form for Book model, which i use to add a book title and author names. accepts_nested_attributes_for is used to add author names. For simplicity i just build 3 authors. So i have a form which has 4 fields, one for book title and 3 for author names.

When i try to add multiple authors with the same name, i got an exception.

SQLite3::ConstraintException in BooksController#create
column slug is not unique

Full trace:

activerecord (3.1.0) lib/active_record/connection_adapters/sqlite_adapter.rb:110:in `close'
activerecord (3.1.0) lib/active_record/connection_adapters/sqlite_adapter.rb:110:in `block in clear_cache!'
activerecord (3.1.0) lib/active_record/connection_adapters/sqlite_adapter.rb:110:in `each'
activerecord (3.1.0) lib/active_record/connection_adapters/sqlite_adapter.rb:110:in `clear_cache!'
activerecord (3.1.0) lib/active_record/connection_adapters/sqlite_adapter.rb:104:in `disconnect!'
activerecord (3.1.0) lib/active_record/connection_adapters/abstract/connection_pool.rb:214:in `block in clear_reloadable_connections!'
activerecord (3.1.0) lib/active_record/connection_adapters/abstract/connection_pool.rb:213:in `each'
activerecord (3.1.0) lib/active_record/connection_adapters/abstract/connection_pool.rb:213:in `clear_reloadable_connections!'
activesupport (3.1.0) lib/active_support/core_ext/module/synchronization.rb:35:in `block in clear_reloadable_connections_with_synchronization!'
/home/emre/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/monitor.rb:201:in `mon_synchronize'
activesupport (3.1.0) lib/active_support/core_ext/module/synchronization.rb:34:in `clear_reloadable_connections_with_synchronization!'
activerecord (3.1.0) lib/active_record/connection_adapters/abstract/connection_pool.rb:391:in `block in clear_reloadable_connections!'
activerecord (3.1.0) lib/active_record/connection_adapters/abstract/connection_pool.rb:391:in `each_value'
activerecord (3.1.0) lib/active_record/connection_adapters/abstract/connection_pool.rb:391:in `clear_reloadable_connections!'
activerecord (3.1.0) lib/active_record/connection_adapters/abstract/connection_specification.rb:123:in `clear_reloadable_connections!'
activerecord (3.1.0) lib/active_record/railtie.rb:84:in `block (3 levels) in <class:Railtie>'
activesupport (3.1.0) lib/active_support/callbacks.rb:404:in `_run_cleanup_callbacks'
activesupport (3.1.0) lib/active_support/callbacks.rb:81:in `run_callbacks'
actionpack (3.1.0) lib/action_dispatch/middleware/reloader.rb:72:in `rescue in call'
actionpack (3.1.0) lib/action_dispatch/middleware/reloader.rb:67:in `call'
rack (1.3.5) lib/rack/sendfile.rb:101:in `call'
actionpack (3.1.0) lib/action_dispatch/middleware/remote_ip.rb:48:in `call'
actionpack (3.1.0) lib/action_dispatch/middleware/show_exceptions.rb:47:in `call'
railties (3.1.0) lib/rails/rack/logger.rb:13:in `call'
rack (1.3.5) lib/rack/methodoverride.rb:24:in `call'
rack (1.3.5) lib/rack/runtime.rb:17:in `call'
activesupport (3.1.0) lib/active_support/cache/strategy/local_cache.rb:72:in `call'
rack (1.3.5) lib/rack/lock.rb:15:in `call'
actionpack (3.1.0) lib/action_dispatch/middleware/static.rb:53:in `call'
railties (3.1.0) lib/rails/engine.rb:455:in `call'
railties (3.1.0) lib/rails/rack/content_length.rb:16:in `call'
railties (3.1.0) lib/rails/rack/log_tailer.rb:14:in `call'
rack (1.3.5) lib/rack/handler/webrick.rb:59:in `service'
/home/emre/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/webrick/httpserver.rb:111:in `service'
/home/emre/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/webrick/httpserver.rb:70:in `run'
/home/emre/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/webrick/server.rb:183:in `block in start_thread'

Maybe request parameters will help:

{"utf8"=>"",
 "authenticity_token"=>"sQmBDRqvOlLqdC9lckojgGMyWELhd6NCdyu4LOGcWQU=",
 "book"=>{"title"=>"book title",
 "authors_attributes"=>{"0"=>{"name"=>"emre"},
 "1"=>{"name"=>"emre"},
 "2"=>{"name"=>"kutlu"}}},
 "commit"=>"Create Book"}

I hope this helps.

emrekutlu commented Nov 16, 2011

Ok, i'm sorry, you're right. I create a repository to reproduce the error.
http://github.com/emrekutlu/friendly_id_bug_report

I am using Rails 3.1. I try to explain it here in more detail.

class Author < ActiveRecord::Base
  extend FriendlyId
  belongs_to :publication, :polymorphic => true
  friendly_id :name, :use => :slugged
end
class Book < ActiveRecord::Base
  has_many :authors, :as => :publication
  accepts_nested_attributes_for :authors
end

I added a unique index for slug column.

In books controller:

def new
    @book = Book.new
    (1..3).each do
      @book.authors.build
    end

    respond_to do |format|
      format.html # new.html.erb
      format.json { render json: @book }
    end
  end

I have a form for Book model, which i use to add a book title and author names. accepts_nested_attributes_for is used to add author names. For simplicity i just build 3 authors. So i have a form which has 4 fields, one for book title and 3 for author names.

When i try to add multiple authors with the same name, i got an exception.

SQLite3::ConstraintException in BooksController#create
column slug is not unique

Full trace:

activerecord (3.1.0) lib/active_record/connection_adapters/sqlite_adapter.rb:110:in `close'
activerecord (3.1.0) lib/active_record/connection_adapters/sqlite_adapter.rb:110:in `block in clear_cache!'
activerecord (3.1.0) lib/active_record/connection_adapters/sqlite_adapter.rb:110:in `each'
activerecord (3.1.0) lib/active_record/connection_adapters/sqlite_adapter.rb:110:in `clear_cache!'
activerecord (3.1.0) lib/active_record/connection_adapters/sqlite_adapter.rb:104:in `disconnect!'
activerecord (3.1.0) lib/active_record/connection_adapters/abstract/connection_pool.rb:214:in `block in clear_reloadable_connections!'
activerecord (3.1.0) lib/active_record/connection_adapters/abstract/connection_pool.rb:213:in `each'
activerecord (3.1.0) lib/active_record/connection_adapters/abstract/connection_pool.rb:213:in `clear_reloadable_connections!'
activesupport (3.1.0) lib/active_support/core_ext/module/synchronization.rb:35:in `block in clear_reloadable_connections_with_synchronization!'
/home/emre/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/monitor.rb:201:in `mon_synchronize'
activesupport (3.1.0) lib/active_support/core_ext/module/synchronization.rb:34:in `clear_reloadable_connections_with_synchronization!'
activerecord (3.1.0) lib/active_record/connection_adapters/abstract/connection_pool.rb:391:in `block in clear_reloadable_connections!'
activerecord (3.1.0) lib/active_record/connection_adapters/abstract/connection_pool.rb:391:in `each_value'
activerecord (3.1.0) lib/active_record/connection_adapters/abstract/connection_pool.rb:391:in `clear_reloadable_connections!'
activerecord (3.1.0) lib/active_record/connection_adapters/abstract/connection_specification.rb:123:in `clear_reloadable_connections!'
activerecord (3.1.0) lib/active_record/railtie.rb:84:in `block (3 levels) in <class:Railtie>'
activesupport (3.1.0) lib/active_support/callbacks.rb:404:in `_run_cleanup_callbacks'
activesupport (3.1.0) lib/active_support/callbacks.rb:81:in `run_callbacks'
actionpack (3.1.0) lib/action_dispatch/middleware/reloader.rb:72:in `rescue in call'
actionpack (3.1.0) lib/action_dispatch/middleware/reloader.rb:67:in `call'
rack (1.3.5) lib/rack/sendfile.rb:101:in `call'
actionpack (3.1.0) lib/action_dispatch/middleware/remote_ip.rb:48:in `call'
actionpack (3.1.0) lib/action_dispatch/middleware/show_exceptions.rb:47:in `call'
railties (3.1.0) lib/rails/rack/logger.rb:13:in `call'
rack (1.3.5) lib/rack/methodoverride.rb:24:in `call'
rack (1.3.5) lib/rack/runtime.rb:17:in `call'
activesupport (3.1.0) lib/active_support/cache/strategy/local_cache.rb:72:in `call'
rack (1.3.5) lib/rack/lock.rb:15:in `call'
actionpack (3.1.0) lib/action_dispatch/middleware/static.rb:53:in `call'
railties (3.1.0) lib/rails/engine.rb:455:in `call'
railties (3.1.0) lib/rails/rack/content_length.rb:16:in `call'
railties (3.1.0) lib/rails/rack/log_tailer.rb:14:in `call'
rack (1.3.5) lib/rack/handler/webrick.rb:59:in `service'
/home/emre/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/webrick/httpserver.rb:111:in `service'
/home/emre/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/webrick/httpserver.rb:70:in `run'
/home/emre/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/webrick/server.rb:183:in `block in start_thread'

Maybe request parameters will help:

{"utf8"=>"",
 "authenticity_token"=>"sQmBDRqvOlLqdC9lckojgGMyWELhd6NCdyu4LOGcWQU=",
 "book"=>{"title"=>"book title",
 "authors_attributes"=>{"0"=>{"name"=>"emre"},
 "1"=>{"name"=>"emre"},
 "2"=>{"name"=>"kutlu"}}},
 "commit"=>"Create Book"}

I hope this helps.

@norman

This comment has been minimized.

Show comment
Hide comment
@norman

norman Nov 16, 2011

Owner

Awesome, thanks for the extensive info. I'll take a look into it today.

Owner

norman commented Nov 16, 2011

Awesome, thanks for the extensive info. I'll take a look into it today.

@norman

This comment has been minimized.

Show comment
Hide comment
@norman

norman Nov 21, 2011

Owner

Sorry I've been slow to respond to this, I'll try to make time today to look into it some more.

Owner

norman commented Nov 21, 2011

Sorry I've been slow to respond to this, I'll try to make time today to look into it some more.

@emrekutlu

This comment has been minimized.

Show comment
Hide comment
@emrekutlu

emrekutlu Nov 22, 2011

I just realized that we have a more basic problem. It is not related to accepts_nested_attributes_for. I am not sure it is a new issue or related to this one.

From the previous models:

Author.create(:name => "emre")
#<Author id: 1, name: "emre", slug: "emre">

Author.create(:name => "emre")
#<Author id: 2, name: "emre", slug: "emre--2">

last_author = Author.find(2)
last_author.save
#<Author id: 2, name: "emre", slug: "emre--2">

Everything is normal up to here. But when i save the first author:

first_author = Author.find(1)
first_author.save
#<Author id: 1, name: "emre", slug: "emre--3">
Author.all
# [#<Author id: 1, name: "emre", slug: "emre--3">, #<Author id: 2, name: "emre", slug: "emre--2">]

emrekutlu commented Nov 22, 2011

I just realized that we have a more basic problem. It is not related to accepts_nested_attributes_for. I am not sure it is a new issue or related to this one.

From the previous models:

Author.create(:name => "emre")
#<Author id: 1, name: "emre", slug: "emre">

Author.create(:name => "emre")
#<Author id: 2, name: "emre", slug: "emre--2">

last_author = Author.find(2)
last_author.save
#<Author id: 2, name: "emre", slug: "emre--2">

Everything is normal up to here. But when i save the first author:

first_author = Author.find(1)
first_author.save
#<Author id: 1, name: "emre", slug: "emre--3">
Author.all
# [#<Author id: 1, name: "emre", slug: "emre--3">, #<Author id: 2, name: "emre", slug: "emre--2">]
@emrekutlu

This comment has been minimized.

Show comment
Hide comment
@emrekutlu

emrekutlu Nov 22, 2011

Some new info:

Author.create(:name => "norman")
#<Author id: 3, name: "norman", slug: "norman">
Author.last.should_generate_new_friendly_id?
# true

should_generate_new_friendly_id? returns true just after i create a record. Is it normal?

I override should_generate_new_friendly_id? for temporary solution.

def should_generate_new_friendly_id?
  send("#{friendly_id_config.base}_changed?")
end

emrekutlu commented Nov 22, 2011

Some new info:

Author.create(:name => "norman")
#<Author id: 3, name: "norman", slug: "norman">
Author.last.should_generate_new_friendly_id?
# true

should_generate_new_friendly_id? returns true just after i create a record. Is it normal?

I override should_generate_new_friendly_id? for temporary solution.

def should_generate_new_friendly_id?
  send("#{friendly_id_config.base}_changed?")
end
@emrekutlu

This comment has been minimized.

Show comment
Hide comment
@emrekutlu

emrekutlu Nov 22, 2011

Sorry for commenting so much but i think i got one step closer to the problem's reason. It is about callback chain.

On Slugged module, set_slug method added to before_validation callbacks. In set_slug method should_generate_new_friendly_id? method is called and in should_generate_new_friendly_id? current_friendly_id method is called. But @current_friendly_id attribute is tried to set on a before_save callback.

So should_generate_new_friendly_id? method always returns true because when it is called current_friendly_id is always nil.

emrekutlu commented Nov 22, 2011

Sorry for commenting so much but i think i got one step closer to the problem's reason. It is about callback chain.

On Slugged module, set_slug method added to before_validation callbacks. In set_slug method should_generate_new_friendly_id? method is called and in should_generate_new_friendly_id? current_friendly_id method is called. But @current_friendly_id attribute is tried to set on a before_save callback.

So should_generate_new_friendly_id? method always returns true because when it is called current_friendly_id is always nil.

@norman

This comment has been minimized.

Show comment
Hide comment
@norman

norman Nov 24, 2011

Owner

should_generate_new_friendly_id? should always return true when creating a new record (unless the slug is nil) so I don't think this is an issue. I'm looking into the app you sent me now, I suspect there's a bug with friendly_id and polymorphic models - will try to have a solution in place today. Thanks for your patience with my slow response.

Owner

norman commented Nov 24, 2011

should_generate_new_friendly_id? should always return true when creating a new record (unless the slug is nil) so I don't think this is an issue. I'm looking into the app you sent me now, I suspect there's a bug with friendly_id and polymorphic models - will try to have a solution in place today. Thanks for your patience with my slow response.

norman added a commit that referenced this issue Nov 24, 2011

@norman

This comment has been minimized.

Show comment
Hide comment
@norman

norman Nov 24, 2011

Owner

Please try your app against FriendlyId master now, I've added a change to address the resequencing issue. You were right - the callback chain was definitely a factor in the bug. Thanks again for the report.

Owner

norman commented Nov 24, 2011

Please try your app against FriendlyId master now, I've added a change to address the resequencing issue. You were right - the callback chain was definitely a factor in the bug. Thanks again for the report.

@emrekutlu

This comment has been minimized.

Show comment
Hide comment
@emrekutlu

emrekutlu Nov 25, 2011

callback chain bug seems resolved but accepts_nested_attributes_for bug remains.
thanks for the patch.

emrekutlu commented Nov 25, 2011

callback chain bug seems resolved but accepts_nested_attributes_for bug remains.
thanks for the patch.

@norman

This comment has been minimized.

Show comment
Hide comment
@norman

norman Nov 30, 2011

Owner

Ok, so I've looked into this at length and it's not a simple problem to solve. I think the best course will be to generate the slug in a before_validation callback, and then generate the sequence in a before_save callback. Otherwise in cases like this, FriendlyId does two queries to check if a slug is being used before doing an insert, so it's impossible to detect collisions.

Owner

norman commented Nov 30, 2011

Ok, so I've looked into this at length and it's not a simple problem to solve. I think the best course will be to generate the slug in a before_validation callback, and then generate the sequence in a before_save callback. Otherwise in cases like this, FriendlyId does two queries to check if a slug is being used before doing an insert, so it's impossible to detect collisions.

norman added a commit that referenced this issue Dec 15, 2011

@norman

This comment has been minimized.

Show comment
Hide comment
@norman

norman Dec 15, 2011

Owner

For now I'm going to leave this issue documented but not fix it, as it would be too significant a change so close to a release, and there are some fairly simple workarounds. I've just added a commit that references this issue, and it can be reopened in the future.

Owner

norman commented Dec 15, 2011

For now I'm going to leave this issue documented but not fix it, as it would be too significant a change so close to a release, and there are some fairly simple workarounds. I've just added a commit that references this issue, and it can be reopened in the future.

@norman norman closed this Dec 15, 2011

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