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

Breaks validates :uniqueness => true #152

Closed
the-architect opened this issue Sep 9, 2011 · 43 comments
Closed

Breaks validates :uniqueness => true #152

the-architect opened this issue Sep 9, 2011 · 43 comments
Assignees

Comments

@the-architect
Copy link

my scenario:

a rails app with a user model which uses friendly_id:

# Gemfile:

gem 'rails', '>=3.1.0'
gem "friendly_id", :git => "git://github.com/norman/friendly_id.git"

# app/models/user.rb:

class User < ActiveRecord::Base
  attr_accessible :username, :email, :password, :password_confirmation
  validates :email, :presence => true,
                    :uniqueness => { :case_sensitive => false }

  extend FriendlyId
  friendly_id :username, :use => :slugged
end

# factories.rb:

FactoryGirl.define do
  factory :user do
    factory :bob do
      username "Example User"
      email "user@example.com"
      password "foobar"
      password_confirmation "foobar"
    end
  end
end

# spec/models/user_spec.rb

it "should reject duplicate email addresses" do
  user = Factory(:bob)
  Factory.build(:bob).should_not be_valid
end

without the lines:

extend FriendlyId
friendly_id :username, :use => :slugged

the spec passes. So I figured it might have something to do with friendly_id.

In fact when I run this with friendly_id enabled

User.exists?(:email => 'marcel.scherf@epicteams.com')

I get the following error:

ActiveRecord::StatementInvalid: SQLite3::SQLException: no such column: slug.email: SELECT  1 FROM "users"  WHERE "slug"."email" = 'john.doe@example.com' LIMIT 1
        from /Users/work/.rvm/gems/ruby-1.9.2-p290@project/gems/sqlite3-1.3.4/lib/sqlite3/database.rb:91:in `initialize'
        from /Users/work/.rvm/gems/ruby-1.9.2-p290@project/gems/sqlite3-1.3.4/lib/sqlite3/database.rb:91:in `new'
        from /Users/work/.rvm/gems/ruby-1.9.2-p290@project/gems/sqlite3-1.3.4/lib/sqlite3/database.rb:91:in `prepare'
        from /Users/work/.rvm/gems/ruby-1.9.2-p290@project/gems/activerecord-3.1.0/lib/active_record/connection_adapters/sqlite_adapter.rb:175:in `block in exec_query'
        from /Users/work/.rvm/gems/ruby-1.9.2-p290@project/gems/activerecord-3.1.0/lib/active_record/connection_adapters/abstract_adapter.rb:244:in `block in log'
        from /Users/work/.rvm/gems/ruby-1.9.2-p290@project/gems/activesupport-3.1.0/lib/active_support/notifications/instrumenter.rb:21:in `instrument'
        from /Users/work/.rvm/gems/ruby-1.9.2-p290@project/gems/activerecord-3.1.0/lib/active_record/connection_adapters/abstract_adapter.rb:239:in `log'
        from /Users/work/.rvm/gems/ruby-1.9.2-p290@project/gems/activerecord-3.1.0/lib/active_record/connection_adapters/sqlite_adapter.rb:171:in `exec_query'
        from /Users/work/.rvm/gems/ruby-1.9.2-p290@project/gems/activerecord-3.1.0/lib/active_record/connection_adapters/sqlite_adapter.rb:382:in `select'
        from /Users/work/.rvm/gems/ruby-1.9.2-p290@project/gems/activerecord-3.1.0/lib/active_record/connection_adapters/abstract/database_statements.rb:18:in `select_all'
        from /Users/work/.rvm/gems/ruby-1.9.2-p290@project/gems/activerecord-3.1.0/lib/active_record/connection_adapters/abstract/query_cache.rb:63:in `select_all'
        from /Users/work/.rvm/gems/ruby-1.9.2-p290@project/gems/activerecord-3.1.0/lib/active_record/connection_adapters/abstract/database_statements.rb:24:in `select_one'
        from /Users/work/.rvm/gems/ruby-1.9.2-p290@project/gems/activerecord-3.1.0/lib/active_record/connection_adapters/abstract/database_statements.rb:30:in `select_value'
        from /Users/work/.rvm/gems/ruby-1.9.2-p290@project/gems/activerecord-3.1.0/lib/active_record/relation/finder_methods.rb:197:in `exists?'
        from /Users/work/.rvm/gems/ruby-1.9.2-p290@project/bundler/gems/friendly_id-c03879ac0227/lib/friendly_id/finder_methods.rb:30:in `exists?'
        from /Users/work/.rvm/gems/ruby-1.9.2-p290@project/gems/activerecord-3.1.0/lib/active_record/base.rb:441:in `exists?'
        from (irb):1
        from /Users/work/.rvm/gems/ruby-1.9.2-p290@project/gems/railties-3.1.0/lib/rails/commands/console.rb:45:in `start'
        from /Users/work/.rvm/gems/ruby-1.9.2-p290@project/gems/railties-3.1.0/lib/rails/commands/console.rb:8:in `start'
        from /Users/work/.rvm/gems/ruby-1.9.2-p290@project/gems/railties-3.1.0/lib/rails/commands.rb:40:in `<top (required)>'
        from script/rails:6:in `require'
        from script/rails:6:in `<main>'>>

Is this a known issue or is there an easy way around the problem?

@someoneinomaha
Copy link

I'm having this problem as well - pretty much the same setup.

@recurser
Copy link
Contributor

+1

@borama
Copy link

borama commented Sep 11, 2011

I also have this problem. The validates_uniqueness_of calls exists?(nil) which is overriden by friendly_id and adds an invalid condition to the query, making the validation always succeed. I think, nil should be considered an "unfriendly_id" in object_utils.rb but I am not sure about other consequences.

Anyway, I made my specs work again by the following monkey patch that I put to config/initializers/friendly_id_patch.rb:

module FriendlyIdPatch

  def friendly_id?
    if [Numeric, Symbol, ActiveRecord::Base, NilClass].detect {|klass| self.class <= klass}
      false
    elsif respond_to?(:to_i) && to_i.to_s != to_s
      true
    end
  end

end

Object.send :include, FriendlyIdPatch

It detects NillClass instances as ids that friendly_id should not handle.

@recurser
Copy link
Contributor

The exists?() implementation also breaks Hash and Array ids :

Person.exists?({:name => 'joe'})
Person.exists?(['name = ?', 'joe'])

@recurser
Copy link
Contributor

I've submitted a patch that i think fixes this - if a couple of you guys have time to try it out and confirm would be much appreciated.

#153

@someoneinomaha
Copy link

After applying your patch, all my tests worked. Everything seems to be working as expected. Well played!

@borama
Copy link

borama commented Sep 12, 2011

The patch works nicely for me too, thanks a lot!

@the-architect
Copy link
Author

patch works, thank you guys :)

@norman
Copy link
Owner

norman commented Sep 13, 2011

Will pull @recurser's patch soon. Thanks everybody for the help and feedback.

@ghost ghost assigned norman Sep 13, 2011
norman pushed a commit that referenced this issue Sep 13, 2011
@norman norman closed this as completed Sep 13, 2011
@bensie
Copy link
Contributor

bensie commented Sep 21, 2011

I'm still encountering this issue (Rails 3.1.1.rc1 and friendly_id at b5b1870) with the following model:

class AbstractUser < ActiveRecord::Base
  extend FriendlyId

  set_table_name "users"

  validates_presence_of     :login
  validates_uniqueness_of   :login
  validates_format_of       :login, with: /\A[a-z0-9-]+\z/i # Letters, numbers, dashes

  friendly_id :login
end

This happens to be the base of an STI configuration, but that doesn't seem to be the problem -- other classes are failing as well that are not part of the STI hierarchy.

@josepjaume
Copy link

This is happening to me when upgraded my app to Rails 3.1.1 (also happens in 3-1-stable branch)

@norman
Copy link
Owner

norman commented Sep 30, 2011

Hmm. This has been WAY more problematic than I anticipated. I'm contemplating removing the override on exists? - this was something I added only fairly recently and it seems it would be less invasive to leave it out. Thoughts?

@bensie
Copy link
Contributor

bensie commented Sep 30, 2011

Would removing that override prevent passing IDs instead of friendly IDs on a friendly_id column when necessary? User.find(123) vs User.find("joe")

Did you track down the commit that caused the issue?

@norman
Copy link
Owner

norman commented Sep 30, 2011

I mean, just removing the override on #exists?. I would leave it on #find. I don't know the specific commit, no. I haven't had time to look into it this week but should be able to spend a few hours on Monday to resolve this and the other open issues.

@norman norman reopened this Sep 30, 2011
@bensie
Copy link
Contributor

bensie commented Sep 30, 2011

Got it -- I don't have a problem with removing the override on #exists?, but others may want to chime in.

@invaino
Copy link

invaino commented Oct 6, 2011

Same issue here, breaks all my validates_uniqueness tests :(

@znakaska
Copy link

znakaska commented Oct 8, 2011

It seems that resolving issue #143 introduced #exists? in 485a19e. After removing #exists?, uniqueness validation works again in Rails 3.1.1.

@sush
Copy link

sush commented Oct 8, 2011

+1 for removing the overrive on #exists? to make it work with Rails 3.1.1.

@lankz
Copy link
Contributor

lankz commented Oct 10, 2011

+1

@oriolgual
Copy link

+1

With Rails 3.1.1 the uniqueness validation is still broken

norman added a commit that referenced this issue Oct 11, 2011
@norman
Copy link
Owner

norman commented Oct 11, 2011

I'm trying to add a test for this, but it seems to be working fine.

Can somebody please try this with the latest FriendlyId (just release beta 13) and if it fails, provide a stack trace and steps to reproduce, or better, modify the test I just added to make it fail?

@sush
Copy link

sush commented Oct 11, 2011

I have no stack trace to give as it fails silently.

But I have just ran your new test with a fresh friendly_id (4.0.0.beta13) and it fails :

bundle exec rake test gives me :

1) Failure:
test_should_not_break_validates_uniqueness_of(SluggedTest) [/tmp/friendly_id/test/slugged_test.rb:58]:
Failed refutation, no message given

@norman
Copy link
Owner

norman commented Oct 11, 2011

Bizarre, the tests run for me. But I just changed the test to use a column other than the slug base and am able to reproduce the failure now. I'm determined to fix this today one way or another, it's been hard to get time to hack on this for the last few weeks and right now I have a brief window of opportunity. Will post another update soon.

@sush
Copy link

sush commented Oct 11, 2011

Thanks a lot for your work :)

@norman
Copy link
Owner

norman commented Oct 11, 2011

I take it back, it's still working fine for me.

@oriolgual
Copy link

I've cloned the repo and the tests fails both for the same slugged column and a different column (MRI 1.9.2-p290). Thanks for your work!

@sush
Copy link

sush commented Oct 11, 2011

Same here, just tested on a different column than the slugged one and fails too.

Weird that you can't reproduce it :s

@norman
Copy link
Owner

norman commented Oct 11, 2011

I'm trying this in a Rails app rather than my Unit tests now, and getting the errors. Not sure why I should see them in an app but not my tests, but that's an issue for later. Will get a fix in soon.

norman added a commit that referenced this issue Oct 11, 2011
Fixes GH issue #152
@norman
Copy link
Owner

norman commented Oct 11, 2011

Ok, a fix has now been pushed to master. The problem was that the method signature for #exists? changed in this commit to Rails, and FriendlyId needed to be updated accordingly.

@norman norman closed this as completed Oct 11, 2011
@sush
Copy link

sush commented Oct 11, 2011

Great !

Will be in the beta13 ?

Thanks a lot !

@norman
Copy link
Owner

norman commented Oct 11, 2011

I just pushed beta 14 actually, after having done a 13 a few hours ago. This week I'll be adding an improved i18n module and then will do a release candidate. Hopefully I can get the stable release out next week, I just want to be really sure there are no embarrassing showstoppers like this one.

@bensie
Copy link
Contributor

bensie commented Oct 11, 2011

Looking good now @norman, thanks for the fix!

@znakaska
Copy link

Thank you for this fix, everything is working great now.

@mattgibb
Copy link

Hey guys,

I've upgraded to rc2 but this test is still failing:
describe User do
it "should reject duplicate email addresses" do
user = FactoryGirl.create :user
user_with_duplicate_email = FactoryGirl.build :user, email: user.email
# user_with_duplicate_email.should_not be_valid
end
end

with the message:

 ActiveRecord::RecordNotUnique:
   PGError: ERROR:  duplicate key value violates unique constraint "index_users_on_email"
   DETAIL:  Key (email)=(user2@test.com) already exists.

Any help would be greatly appreciated :-)

@norman
Copy link
Owner

norman commented Dec 20, 2011

I don't think this has anything to do with FriendlyId. Try generating a random email with Faker, make sure your database is being reset properly between tests, or consider using a sequence in the email address to ensure its uniqueness.

@bensie
Copy link
Contributor

bensie commented Dec 20, 2011

Yeah all uniqueness issues have been resolved for awhile now, no new test failures surrounding this issue for me.

@mattgibb
Copy link

It looks like I've misunderstood something. I'd already set up sequences for the email, that's why I explicitly set the email in the second call to FactoryGirl in the code above. I already tested other models in the same project through 'rails console test' that aren't using friendly_id and found that the FactoryGirl.build :charity call didn't hit the database, but the FactoryGirl.build :user hits the db to insert a user. Furthermore, FactoryGirl.create hits the database with an insert call followed by 2 selects and an update (apologies if this is all obvious).

I'm not sure how to determine if Devise is getting in the way here, but I guess the key thing I'm confused about is, why would FactoryGirl.build insert a record in the first place?

Thanks for a great gem!

@aalbagarcia
Copy link

Hi:

I'm having problems with 'validates uniqueness:true' in my Rails application. I see this issue was closed two years ago so I don't know if it's me doing something wrong.

I'm using rails 4.0.0-rc1. This is my Gemfile:

ruby '2.0.0'
gem 'rails', '4.0.0.rc1'
gem 'friendly_id', github: 'FriendlyId/friendly_id', branch: 'master'
gem 'devise', github: 'plataformatec/devise', branch: 'rails4'

This is my User model:

class User < ActiveRecord::Base
  include PictureProfilable
  include FriendlyId
  friendly_id :name, use: [:slugged]

  # Include default devise modules. Others available are:
  # :token_authenticatable, :encryptable, :confirmable, :lockable, :timeoutable and :omniauthable

  devise :database_authenticatable, :registerable, :omniauthable,
         :recoverable, :rememberable, :validatable, :trackable, :confirmable

  has_and_belongs_to_many :roles, join_table: :roles_users #Work around for https://github.com/thoughtbot/shoulda/issues/225

  has_many :services, :dependent => :destroy

...(some public and private methods)

I've written this spec:

  describe 'email should be unique' do
    it {
      user = FactoryGirl.create :user
      new_user = User.new first_name: 'New User', email: user.email, password: 'password'
      new_user.valid?.should be_false
    }
  end

Which only passes if I remove friendly_id from the User model.

I've tried other models in my application not using Devise and I have the same problem.

From the Rails console, if I try to create a User with an existing email, I see the following:

user = user = User.new first_name: 'nuevo', email: 'existing@email.com', password: 'password'
user.valid?

  User Load (0.3ms)  SELECT "users".* FROM "users" WHERE ("slug" = 'nuevo' OR "slug" LIKE 'nuevo--%'ESCAPE '\') ORDER BY LENGTH("slug") DESC, "slug" DESC LIMIT 1
=> true

Apparently there is no checking for the uniqueness of the email.

Any idea of what can be wrong, or am I doing wrong?.

@parndt
Copy link
Collaborator

parndt commented May 15, 2013

@aalbagarcia we have a rails4 branch with work in progress support for Rails 4:

gem 'friendly_id', github: 'norman/friendly_id', branch: 'rails4'

@aalbagarcia
Copy link

I've tried both of them. I think the problem is related to a new change in the signature of the exists? function. In ActiveRecord/relation/finder_methods.rb (4.0.0-rc1) it is

def exists?(conditions = :none)

while in friendly_id/finder_methods.rb is

def exists?(id = false)

Could it be related?

@parndt
Copy link
Collaborator

parndt commented May 15, 2013

@aalbagarcia yeah, that could have something to do with it..

parndt added a commit that referenced this issue May 15, 2013
@aalbagarcia
Copy link

Thanks!! problem solved.

@vivipoit
Copy link

Hello!

I have friendly_id 5.1.0 on Rails 5.1.6 and it seems like it breaks just in terms of case sensitivity.

class Event < ApplicationRecord

  extend FriendlyId
  friendly_id :name, use: :slugged

  belongs_to :admin

  validates_presence_of :name
  validates_uniqueness_of :name
  
end

With this code, I can't create 2 records with 'Test' as name, but I'm able to create 'Test' and 'test'.

Any idea if the issue could come back to the gem?

I've tried setting :case_sensitive => true explicitly, but that didn't seem to help anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests